Add validations for Composer 2/3 only fields

mmv1/third_party/terraform/services/composer/resource_composer_environment.go.erb

@@ -7,6 +7,7 @@ import (
 	"regexp"
 	"strings"
 	"time"
+	"context"
 
 	"github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
@@ -167,6 +168,10 @@ func ResourceComposerEnvironment() *schema.Resource {
 			tpgresource.DefaultProviderProject,
 			tpgresource.DefaultProviderRegion,
 			tpgresource.SetLabelsDiff,
+<% unless version == "ga" -%>
+		  customdiff.ValidateChange("config.0.software_config.0.image_version", imageVersionChangeValidationFunc),
+			versionValidationCustomizeDiffFunc,
+<% end -%>
 		),
 
 		Schema: map[string]*schema.Schema{
@@ -1548,10 +1553,14 @@ func flattenComposerEnvironmentConfig(envCfg *composer.EnvironmentConfig) interf
 	transformed["airflow_uri"] = envCfg.AirflowUri
 	transformed["node_config"] = flattenComposerEnvironmentConfigNodeConfig(envCfg.NodeConfig)
 	transformed["software_config"] = flattenComposerEnvironmentConfigSoftwareConfig(envCfg.SoftwareConfig)
-	transformed["private_environment_config"] = flattenComposerEnvironmentConfigPrivateEnvironmentConfig(envCfg.PrivateEnvironmentConfig)
+	if !isComposer3(envCfg){
+		transformed["private_environment_config"] = flattenComposerEnvironmentConfigPrivateEnvironmentConfig(envCfg.PrivateEnvironmentConfig)
+	}
 <% unless version == "ga" -%>
-	transformed["enable_private_environment"] = envCfg.PrivateEnvironmentConfig.EnablePrivateEnvironment
-	transformed["enable_private_builds_only"] = envCfg.PrivateEnvironmentConfig.EnablePrivateBuildsOnly
+	if isComposer3(envCfg){
+		transformed["enable_private_environment"] = envCfg.PrivateEnvironmentConfig.EnablePrivateEnvironment
+		transformed["enable_private_builds_only"] = envCfg.PrivateEnvironmentConfig.EnablePrivateBuildsOnly
+	}
 <% end -%>
 	transformed["web_server_network_access_control"] = flattenComposerEnvironmentConfigWebServerNetworkAccessControl(envCfg.WebServerNetworkAccessControl)
 	transformed["database_config"] = flattenComposerEnvironmentConfigDatabaseConfig(envCfg.DatabaseConfig)
@@ -1736,7 +1745,9 @@ func flattenComposerEnvironmentConfigWorkloadsConfig(workloadsConfig *composer.W
 	transformed["web_server"] = []interface{}{transformedWebServer}
 	transformed["worker"] = []interface{}{transformedWorker}
 <% unless version == "ga" -%>
-	transformed["dag_processor"] = []interface{}{transformedDagProcessor}
+	if transformedDagProcessor != nil {
+		transformed["dag_processor"] = []interface{}{transformedDagProcessor}
+	}
 <% end -%>
 
 
@@ -1915,7 +1926,7 @@ func expandComposerEnvironmentConfig(v interface{}, d *schema.ResourceData, conf
 		composer.PrivateEnvironmentConfig.EnablePrivateEnvironment in API.
 		Check image version to avoid overriding EnablePrivateEnvironment in case of other versions.
 	*/
-	if isComposer3(d, config) {
+	if isComposer3(d) {
 		transformed.PrivateEnvironmentConfig = &composer.PrivateEnvironmentConfig{}
 		if enablePrivateEnvironmentRaw, ok := original["enable_private_environment"]; ok {
 			transformed.PrivateEnvironmentConfig.EnablePrivateEnvironment = enablePrivateEnvironmentRaw.(bool)
@@ -2812,7 +2823,60 @@ func versionsEqual(old, new string) (bool, error) {
 	return o.Equal(n), nil
 }
 
-func isComposer3(d *schema.ResourceData, config *transport_tpg.Config)	bool {
-	image_version := d.Get("config.0.software_config.0.image_version").(string)
-	return strings.Contains(image_version, "composer-3")
+func isComposer3(v interface{})	bool {
+	var imageVersion string
+	switch v.(type) {
+	case *schema.ResourceData:
+		imageVersion = v.(*schema.ResourceData).Get("config.0.software_config.0.image_version").(string)
+	case *schema.ResourceDiff:
+		imageVersion = v.(*schema.ResourceDiff).Get("config.0.software_config.0.image_version").(string)
+	case *composer.EnvironmentConfig:
+		imageVersion = v.(*composer.EnvironmentConfig).SoftwareConfig.ImageVersion
+	case string:
+		imageVersion = v.(string)
+	default:
+		log.Printf("[ERROR] error in plugin: unexpected argument type in isComposer3")
+	}
+	return strings.Contains(imageVersion, "composer-3")
+}
+
+func imageVersionChangeValidationFunc(ctx context.Context, old, new, meta any) error {
+	if old.(string) != "" && !isComposer3(old.(string)) && isComposer3(new.(string)) {
+		return fmt.Errorf("upgrade to composer 3 is not yet supported")
+	}
+	return nil
+}
+
+func validateComposer3FieldUsage(d *schema.ResourceDiff, key string, requireComposer3 bool) error {
+	_, ok := d.GetOk(key)
+	if ok && ( isComposer3(d) != requireComposer3 ) {
+		if requireComposer3 {
+			return fmt.Errorf("error in configuration, %s should only be used in Composer 3", key)
+		} else {
+			return fmt.Errorf("error in configuration, %s should not be used in Composer 3", key)
+		}
+	}
+	return nil
 }
+
+func versionValidationCustomizeDiffFunc(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error {
+	composer3FieldUsagePolicy := map[string]bool{
+		"config.0.node_config.0.max_pods_per_node": false, // not allowed in composer 3
+		"config.0.node_config.0.enable_ip_masq_agent": false,
+		"config.0.node_config.0.config.0.node_config.0.ip_allocation_policy": false,
+		"config.0.private_environment_config": false,
+		"config.0.master_authorized_networks_config": false,
+		"config.0.node_config.0.composer_network_attachment": true, // allowed only in composer 3
+		"config.0.node_config.0.composer_internal_ipv4_cidr_block": true,
+		"config.0.software_config.0.web_server_plugins_mode": true,
+		"config.0.enable_private_environment": true,
+		"config.0.enable_private_builds_only": true,
+		"config.0.workloads_config.0.dag_processor": true,
+	}
+	for key, allowed := range composer3FieldUsagePolicy {
+		if err := validateComposer3FieldUsage(d, key, allowed); err != nil {
+			return err
+		}
+	}
+	return nil
+}

mmv1/third_party/terraform/services/composer/resource_composer_environment_test.go.erb

@@ -1186,6 +1186,77 @@ func TestAccComposerEnvironmentComposer3_update(t *testing.T) {
 	})
 }
 
+func TestAccComposerEnvironmentComposer3_upgrade_expectError(t *testing.T) {
+	t.Parallel()
+
+	envName := fmt.Sprintf("%s-%d", testComposerEnvironmentPrefix, acctest.RandInt(t))
+	network := fmt.Sprintf("%s-%d", testComposerNetworkPrefix, acctest.RandInt(t))
+	subnetwork := network + "-1"
+	errorRegExp, _ := regexp.Compile(".*upgrade to composer 3 is not yet supported.*")
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:		          func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccComposerEnvironmentDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config:             testAccComposerEnvironmentComposer2_empty(envName, network, subnetwork),
+			},
+			{
+				Config:             testAccComposerEnvironmentComposer3_empty(envName, network, subnetwork),
+				ExpectError:        errorRegExp,
+			},
+			// This is a terrible clean-up step in order to get destroy to succeed,
+			// due to dangling firewall rules left by the Composer Environment blocking network deletion.
+			// TODO: Remove this check if firewall rules bug gets fixed by Composer.
+			{
+				PlanOnly:           true,
+				ExpectNonEmptyPlan: false,
+				Config:             testAccComposerEnvironmentComposer2_empty(envName, network, subnetwork),
+				Check:              testAccCheckClearComposerEnvironmentFirewalls(t, network),
+			},
+		},
+	})
+}
+
+func TestAccComposerEnvironmentComposer2_usesUnsupportedField_expectError(t *testing.T) {
+	t.Parallel()
+
+	envName := fmt.Sprintf("%s-%d", testComposerEnvironmentPrefix, acctest.RandInt(t))
+	errorRegExp, _ := regexp.Compile(".*error in configuration, .* should only be used in Composer 3.*")
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:		          func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccComposerEnvironmentDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config:             testAccComposerEnvironmentComposer2_usesUnsupportedField(envName),
+				ExpectError:        errorRegExp,
+			},
+		},
+	})
+}
+
+func TestAccComposerEnvironmentComposer3_usesUnsupportedField_expectError(t *testing.T) {
+	t.Parallel()
+
+	envName := fmt.Sprintf("%s-%d", testComposerEnvironmentPrefix, acctest.RandInt(t))
+	errorRegExp, _ := regexp.Compile(".*error in configuration, .* should not be used in Composer 3.*")
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:		          func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccComposerEnvironmentDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config:             testAccComposerEnvironmentComposer3_usesUnsupportedField(envName),
+				ExpectError:        errorRegExp,
+			},
+		},
+	})
+}
+
 // Checks Composer 3 specific updatable fields.
 func TestAccComposerEnvironmentComposer3_updateToEmpty(t *testing.T) {
 	t.Parallel()
@@ -2825,6 +2896,34 @@ resource "google_project_iam_member" "composer-worker" {
 }
 
 <% unless version == "ga" -%>
+func testAccComposerEnvironmentComposer2_empty(name, network, subnetwork string) string {
+	return fmt.Sprintf(`
+resource "google_composer_environment" "test" {
+  name   = "%s"
+  region = "us-central1"
+  config {
+    software_config {
+      image_version = "composer-2-airflow-2"
+    }
+  }
+}
+
+// use a separate network to avoid conflicts with other tests running in parallel
+// that use the default network/subnet
+resource "google_compute_network" "test" {
+  name                    = "%s"
+  auto_create_subnetworks = false
+}
+
+resource "google_compute_subnetwork" "test" {
+  name          = "%s"
+  ip_cidr_range = "10.2.0.0/16"
+  region        = "us-central1"
+  network       = google_compute_network.test.self_link
+}
+`, name, network, subnetwork)
+}
+
 func testAccComposerEnvironmentComposer3_empty(name, network, subnetwork string) string {
 	return fmt.Sprintf(`
 resource "google_composer_environment" "test" {
@@ -2853,6 +2952,38 @@ resource "google_compute_subnetwork" "test" {
 `, name, network, subnetwork)
 }
 
+func testAccComposerEnvironmentComposer2_usesUnsupportedField(name string) string {
+return fmt.Sprintf(`
+resource "google_composer_environment" "test" {
+  name   = "%s"
+  region = "us-central1"
+  config {
+    software_config {
+      image_version = "composer-2-airflow-2"
+			web_server_plugins_mode = "ENABLED"
+    }
+  }
+}
+`, name)
+}
+
+func testAccComposerEnvironmentComposer3_usesUnsupportedField(name string) string {
+return fmt.Sprintf(`
+resource "google_composer_environment" "test" {
+  name   = "%s"
+  region = "us-central1"
+  config {
+	  node_config {
+	    enable_ip_masq_agent = true
+	  }
+    software_config {
+      image_version = "composer-3-airflow-2"
+    }
+  }
+}
+`, name)
+}
+
 func testAccComposerEnvironmentComposer3_basic(name, network, subnetwork string) string {
 	return fmt.Sprintf(`
 resource "google_composer_environment" "test" {