diff --git a/mmv1/third_party/terraform/services/composer/resource_composer_environment.go.erb b/mmv1/third_party/terraform/services/composer/resource_composer_environment.go.erb index 536af7709b4b..0369ce49f4d0 100644 --- a/mmv1/third_party/terraform/services/composer/resource_composer_environment.go.erb +++ b/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" @@ -168,6 +169,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{ @@ -1592,10 +1597,15 @@ 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) + imageVersion := envCfg.SoftwareConfig.ImageVersion + if !isComposer3(imageVersion){ + 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(imageVersion) && envCfg.PrivateEnvironmentConfig != nil { + 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) @@ -1803,7 +1813,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 -%> @@ -1982,7 +1994,8 @@ 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) { + imageVersion := d.Get("config.0.software_config.0.image_version").(string) + if isComposer3(imageVersion) { transformed.PrivateEnvironmentConfig = &composer.PrivateEnvironmentConfig{} if enablePrivateEnvironmentRaw, ok := original["enable_private_environment"]; ok { transformed.PrivateEnvironmentConfig.EnablePrivateEnvironment = enablePrivateEnvironmentRaw.(bool) @@ -2934,7 +2947,48 @@ 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(imageVersion string) bool { + 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) + imageVersion := d.Get("config.0.software_config.0.image_version").(string) + if ok && ( isComposer3(imageVersion) != 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 +} \ No newline at end of file diff --git a/mmv1/third_party/terraform/services/composer/resource_composer_environment_test.go.erb b/mmv1/third_party/terraform/services/composer/resource_composer_environment_test.go.erb index 80e870eac07a..1ddb9257f35e 100644 --- a/mmv1/third_party/terraform/services/composer/resource_composer_environment_test.go.erb +++ b/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() @@ -2835,6 +2906,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" { @@ -2863,6 +2962,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" {