From 8166c7fb8a55e06acec81334ff89ceeacad48683 Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Wed, 15 Nov 2023 10:59:18 +0100 Subject: [PATCH] fix: make disk_iops a computed attribute in advanced_cluster resource (#1620) * fix: make disk_iops a computed attribute in advanced_cluster resource * define migration test for advanced cluster using single and multi cloud provider configurations --- .github/workflows/migration-tests.yml | 35 ++++ .../resource_mongodbatlas_advanced_cluster.go | 5 +- ...dbatlas_advanced_cluster_migration_test.go | 197 ++++++++---------- ...tlas_advanced_cluster_schema_migration.go} | 0 ..._advanced_cluster_schema_migration_test.go | 132 ++++++++++++ ...urce_mongodbatlas_advanced_cluster_test.go | 16 +- mongodbatlas/testutils/attribute_checks.go | 14 ++ .../testutils/attribute_checks_test.go | 31 +++ 8 files changed, 308 insertions(+), 122 deletions(-) rename mongodbatlas/{resource_mongodbatlas_advanced_cluster_migrate.go => resource_mongodbatlas_advanced_cluster_schema_migration.go} (100%) create mode 100644 mongodbatlas/resource_mongodbatlas_advanced_cluster_schema_migration_test.go create mode 100644 mongodbatlas/testutils/attribute_checks_test.go diff --git a/.github/workflows/migration-tests.yml b/.github/workflows/migration-tests.yml index f8cfe61eae..c2fd0b579d 100644 --- a/.github/workflows/migration-tests.yml +++ b/.github/workflows/migration-tests.yml @@ -19,6 +19,7 @@ jobs: outputs: project: ${{ steps.filter.outputs.project }} config: ${{ steps.filter.outputs.config }} + advanced_cluster: ${{ steps.filter.outputs.advanced_cluster }} backup_online_archive: ${{ steps.filter.outputs.backup_online_archive }} shouldTriggerResourceTest: ${{ github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || github.event.label.name == 'run-testacc' || inputs.parent-event-name == 'release' }} steps: @@ -39,6 +40,8 @@ jobs: - 'mongodbatlas/fw_resource_mongodbatlas_database_user*.go' backup_online_archive: - 'mongodbatlas/**online_archive**.go' + advanced_cluster: + - 'mongodbatlas/**advanced_cluster**.go' project: needs: [ change-detection ] @@ -146,3 +149,35 @@ jobs: PARALLEL_GO_TEST: 20 TEST_REGEX: "^TestAccMigrationBackup" run: make testacc + + advanced_cluster: + needs: [ change-detection ] + if: ${{ needs.change-detection.outputs.advanced_cluster == 'true' || github.event.label.name == 'run-testacc-advanced-cluster'|| needs.change-detection.outputs.shouldTriggerResourceTest == 'true'}} + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Get Last Release + id: get_last_release + run: | + LAST_RELEASE=$(curl -sSfL -X GET https://api.github.com/repos/mongodb/terraform-provider-mongodbatlas/releases/latest | jq -r '.tag_name | ltrimstr("v")') + echo "Last release: $LAST_RELEASE" + echo "MONGODB_ATLAS_LAST_VERSION=$LAST_RELEASE" >> $GITHUB_ENV + shell: bash + - name: Set up Go + uses: actions/setup-go@v4 + with: + go-version-file: 'go.mod' + - name: Migration Tests + env: + MONGODB_ATLAS_PUBLIC_KEY: ${{ secrets.MONGODB_ATLAS_PUBLIC_KEY_CLOUD_DEV }} + MONGODB_ATLAS_PRIVATE_KEY: ${{ secrets.MONGODB_ATLAS_PRIVATE_KEY_CLOUD_DEV }} + MONGODB_ATLAS_ORG_ID: ${{ vars.MONGODB_ATLAS_ORG_ID_CLOUD_DEV }} + MONGODB_ATLAS_BASE_URL: ${{ vars.MONGODB_ATLAS_BASE_URL }} + SKIP_TEST_EXTERNAL_CREDENTIALS: ${{ vars.SKIP_TEST_EXTERNAL_CREDENTIALS }} + ACCTEST_TIMEOUT: ${{ vars.ACCTEST_TIMEOUT }} + TF_LOG: ${{ vars.LOG_LEVEL }} + TF_ACC: 1 + PARALLEL_GO_TEST: 20 + TEST_REGEX: "^TestAccMigrationAdvancedCluster" + run: make testacc diff --git a/mongodbatlas/resource_mongodbatlas_advanced_cluster.go b/mongodbatlas/resource_mongodbatlas_advanced_cluster.go index 3353cc41a4..701247957d 100644 --- a/mongodbatlas/resource_mongodbatlas_advanced_cluster.go +++ b/mongodbatlas/resource_mongodbatlas_advanced_cluster.go @@ -356,6 +356,7 @@ func advancedClusterRegionConfigsSpecsSchema() *schema.Schema { "disk_iops": { Type: schema.TypeInt, Optional: true, + Computed: true, }, "ebs_volume_type": { Type: schema.TypeString, @@ -1219,9 +1220,7 @@ func flattenAdvancedReplicationSpecRegionConfigSpec(apiObject *matlas.Specs, pro if providerName == "AWS" { if cast.ToInt64(apiObject.DiskIOPS) > 0 { - if v, ok := tfMapObject["disk_iops"]; ok && v.(int) > 0 { - tfMap["disk_iops"] = apiObject.DiskIOPS - } + tfMap["disk_iops"] = apiObject.DiskIOPS } if v, ok := tfMapObject["ebs_volume_type"]; ok && v.(string) != "" { tfMap["ebs_volume_type"] = apiObject.EbsVolumeType diff --git a/mongodbatlas/resource_mongodbatlas_advanced_cluster_migration_test.go b/mongodbatlas/resource_mongodbatlas_advanced_cluster_migration_test.go index a193b3db76..856e1c27fe 100644 --- a/mongodbatlas/resource_mongodbatlas_advanced_cluster_migration_test.go +++ b/mongodbatlas/resource_mongodbatlas_advanced_cluster_migration_test.go @@ -1,132 +1,103 @@ package mongodbatlas import ( - "fmt" + "os" "testing" - "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + matlas "go.mongodb.org/atlas/mongodbatlas" ) -func TestAccClusterRSAdvancedClusterMigrateState_empty_advancedConfig(t *testing.T) { - v0State := map[string]any{ - "project_id": "test-id", - "name": "test-cluster", - "cluster_type": "REPLICASET", - "replication_specs": []any{ - map[string]any{ - "region_configs": []any{ - map[string]any{ - "electable_specs": []any{ - map[string]any{ - "instance_size": "M30", - "node_count": 3, - }, - }, - "provider_name": "AWS", - "region_name": "US_EAST_1", - "priority": 7, +func TestAccMigrationAdvancedClusterRS_singleAWSProvider(t *testing.T) { + var ( + cluster matlas.AdvancedCluster + resourceName = "mongodbatlas_advanced_cluster.test" + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + rName = acctest.RandomWithPrefix("test-acc") + lastVersionConstraint = os.Getenv("MONGODB_ATLAS_LAST_VERSION") + ) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccMigrationPreCheckBasic(t) }, + CheckDestroy: testAccCheckMongoDBAtlasAdvancedClusterDestroy, + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "mongodbatlas": { + VersionConstraint: lastVersionConstraint, + Source: "mongodb/mongodbatlas", }, }, + Config: testAccMongoDBAtlasAdvancedClusterConfigSingleProvider(orgID, projectName, rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckMongoDBAtlasAdvancedClusterExists(resourceName, &cluster), + testAccCheckMongoDBAtlasAdvancedClusterAttributes(&cluster, rName), + resource.TestCheckResourceAttrSet(resourceName, "project_id"), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "retain_backups_enabled", "true"), + resource.TestCheckResourceAttrSet(resourceName, "replication_specs.#"), + resource.TestCheckResourceAttrSet(resourceName, "replication_specs.0.region_configs.#"), + ), }, - }, - "bi_connector": []any{ - map[string]any{ - "enabled": 1, - "read_preference": "secondary", + { + ProtoV6ProviderFactories: testAccProviderV6Factories, + Config: testAccMongoDBAtlasAdvancedClusterConfigSingleProvider(orgID, projectName, rName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPreRefresh: []plancheck.PlanCheck{ + DebugPlan(), + }, + }, + PlanOnly: true, }, }, - } - - v0Config := terraform.NewResourceConfigRaw(v0State) - diags := resourceMongoDBAtlasAdvancedClusterResourceV0().Validate(v0Config) - - if len(diags) > 0 { - t.Error("test precondition failed - invalid mongodb cluster v0 config") - - return - } - - // test migrate function - v1State := migrateBIConnectorConfig(v0State) - - v1Config := terraform.NewResourceConfigRaw(v1State) - diags = resourceMongoDBAtlasAdvancedCluster().Validate(v1Config) - if len(diags) > 0 { - fmt.Println(diags) - t.Error("migrated cluster advanced config is invalid") - - return - } + }) } -func TestAccClusterRSAdvancedClusterV0StateUpgrade_ReplicationSpecs(t *testing.T) { - v0State := map[string]any{ - "project_id": "test-id", - "name": "test-cluster", - "cluster_type": "REPLICASET", - "backup_enabled": true, - "disk_size_gb": 256, - "replication_specs": []any{ - map[string]any{ - "zone_name": "Test Zone", - "region_configs": []any{ - map[string]any{ - "priority": 7, - "provider_name": "AWS", - "region_name": "US_EAST_1", - "electable_specs": []any{ - map[string]any{ - "instance_size": "M30", - "node_count": 3, - }, - }, - "read_only_specs": []any{ - map[string]any{ - "disk_iops": 0, - "instance_size": "M30", - "node_count": 0, - }, - }, - "auto_scaling": []any{ - map[string]any{ - "compute_enabled": true, - "compute_max_instance_size": "M60", - "compute_min_instance_size": "M30", - "compute_scale_down_enabled": true, - "disk_gb_enabled": false, - }, - }, +func TestAccMigrationAdvancedClusterRS_multiCloud(t *testing.T) { + var ( + cluster matlas.AdvancedCluster + resourceName = "mongodbatlas_advanced_cluster.test" + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + rName = acctest.RandomWithPrefix("test-acc") + lastVersionConstraint = os.Getenv("MONGODB_ATLAS_LAST_VERSION") + ) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccMigrationPreCheckBasic(t) }, + CheckDestroy: testAccCheckMongoDBAtlasAdvancedClusterDestroy, + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "mongodbatlas": { + VersionConstraint: lastVersionConstraint, + Source: "mongodb/mongodbatlas", + }, + }, + Config: testAccMongoDBAtlasAdvancedClusterConfigMultiCloud(orgID, projectName, rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckMongoDBAtlasAdvancedClusterExists(resourceName, &cluster), + testAccCheckMongoDBAtlasAdvancedClusterAttributes(&cluster, rName), + resource.TestCheckResourceAttrSet(resourceName, "project_id"), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "retain_backups_enabled", "false"), + resource.TestCheckResourceAttrSet(resourceName, "replication_specs.#"), + resource.TestCheckResourceAttrSet(resourceName, "replication_specs.0.region_configs.#"), + ), + }, + { + ProtoV6ProviderFactories: testAccProviderV6Factories, + Config: testAccMongoDBAtlasAdvancedClusterConfigMultiCloud(orgID, projectName, rName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPreRefresh: []plancheck.PlanCheck{ + DebugPlan(), }, }, + PlanOnly: true, }, }, - } - - v0Config := terraform.NewResourceConfigRaw(v0State) - diags := resourceMongoDBAtlasAdvancedClusterResourceV0().Validate(v0Config) - - if len(diags) > 0 { - fmt.Println(diags) - t.Error("test precondition failed - invalid mongodb cluster v0 config") - - return - } - - // test migrate function - v1State := migrateBIConnectorConfig(v0State) - - v1Config := terraform.NewResourceConfigRaw(v1State) - diags = resourceMongoDBAtlasAdvancedCluster().Validate(v1Config) - if len(diags) > 0 { - fmt.Println(diags) - t.Error("migrated advanced cluster replication_specs invalid") - - return - } - - if len(v1State["replication_specs"].([]any)) != len(v0State["replication_specs"].([]any)) { - t.Error("migrated replication specs did not contain the same number of elements") - - return - } + }) } diff --git a/mongodbatlas/resource_mongodbatlas_advanced_cluster_migrate.go b/mongodbatlas/resource_mongodbatlas_advanced_cluster_schema_migration.go similarity index 100% rename from mongodbatlas/resource_mongodbatlas_advanced_cluster_migrate.go rename to mongodbatlas/resource_mongodbatlas_advanced_cluster_schema_migration.go diff --git a/mongodbatlas/resource_mongodbatlas_advanced_cluster_schema_migration_test.go b/mongodbatlas/resource_mongodbatlas_advanced_cluster_schema_migration_test.go new file mode 100644 index 0000000000..a193b3db76 --- /dev/null +++ b/mongodbatlas/resource_mongodbatlas_advanced_cluster_schema_migration_test.go @@ -0,0 +1,132 @@ +package mongodbatlas + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" +) + +func TestAccClusterRSAdvancedClusterMigrateState_empty_advancedConfig(t *testing.T) { + v0State := map[string]any{ + "project_id": "test-id", + "name": "test-cluster", + "cluster_type": "REPLICASET", + "replication_specs": []any{ + map[string]any{ + "region_configs": []any{ + map[string]any{ + "electable_specs": []any{ + map[string]any{ + "instance_size": "M30", + "node_count": 3, + }, + }, + "provider_name": "AWS", + "region_name": "US_EAST_1", + "priority": 7, + }, + }, + }, + }, + "bi_connector": []any{ + map[string]any{ + "enabled": 1, + "read_preference": "secondary", + }, + }, + } + + v0Config := terraform.NewResourceConfigRaw(v0State) + diags := resourceMongoDBAtlasAdvancedClusterResourceV0().Validate(v0Config) + + if len(diags) > 0 { + t.Error("test precondition failed - invalid mongodb cluster v0 config") + + return + } + + // test migrate function + v1State := migrateBIConnectorConfig(v0State) + + v1Config := terraform.NewResourceConfigRaw(v1State) + diags = resourceMongoDBAtlasAdvancedCluster().Validate(v1Config) + if len(diags) > 0 { + fmt.Println(diags) + t.Error("migrated cluster advanced config is invalid") + + return + } +} + +func TestAccClusterRSAdvancedClusterV0StateUpgrade_ReplicationSpecs(t *testing.T) { + v0State := map[string]any{ + "project_id": "test-id", + "name": "test-cluster", + "cluster_type": "REPLICASET", + "backup_enabled": true, + "disk_size_gb": 256, + "replication_specs": []any{ + map[string]any{ + "zone_name": "Test Zone", + "region_configs": []any{ + map[string]any{ + "priority": 7, + "provider_name": "AWS", + "region_name": "US_EAST_1", + "electable_specs": []any{ + map[string]any{ + "instance_size": "M30", + "node_count": 3, + }, + }, + "read_only_specs": []any{ + map[string]any{ + "disk_iops": 0, + "instance_size": "M30", + "node_count": 0, + }, + }, + "auto_scaling": []any{ + map[string]any{ + "compute_enabled": true, + "compute_max_instance_size": "M60", + "compute_min_instance_size": "M30", + "compute_scale_down_enabled": true, + "disk_gb_enabled": false, + }, + }, + }, + }, + }, + }, + } + + v0Config := terraform.NewResourceConfigRaw(v0State) + diags := resourceMongoDBAtlasAdvancedClusterResourceV0().Validate(v0Config) + + if len(diags) > 0 { + fmt.Println(diags) + t.Error("test precondition failed - invalid mongodb cluster v0 config") + + return + } + + // test migrate function + v1State := migrateBIConnectorConfig(v0State) + + v1Config := terraform.NewResourceConfigRaw(v1State) + diags = resourceMongoDBAtlasAdvancedCluster().Validate(v1Config) + if len(diags) > 0 { + fmt.Println(diags) + t.Error("migrated advanced cluster replication_specs invalid") + + return + } + + if len(v1State["replication_specs"].([]any)) != len(v0State["replication_specs"].([]any)) { + t.Error("migrated replication specs did not contain the same number of elements") + + return + } +} diff --git a/mongodbatlas/resource_mongodbatlas_advanced_cluster_test.go b/mongodbatlas/resource_mongodbatlas_advanced_cluster_test.go index b4ff76f36c..9de746b90f 100644 --- a/mongodbatlas/resource_mongodbatlas_advanced_cluster_test.go +++ b/mongodbatlas/resource_mongodbatlas_advanced_cluster_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas/testutils" "github.com/mwielbut/pointy" matlas "go.mongodb.org/atlas/mongodbatlas" ) @@ -79,11 +80,12 @@ func TestAccClusterAdvancedCluster_basicTenant(t *testing.T) { func TestAccClusterAdvancedCluster_singleProvider(t *testing.T) { var ( - cluster matlas.AdvancedCluster - resourceName = "mongodbatlas_advanced_cluster.test" - orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") - projectName = acctest.RandomWithPrefix("test-acc") - rName = acctest.RandomWithPrefix("test-acc") + cluster matlas.AdvancedCluster + resourceName = "mongodbatlas_advanced_cluster.test" + dataSourceName = fmt.Sprintf("data.%s", resourceName) + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acctest.RandomWithPrefix("test-acc") + rName = acctest.RandomWithPrefix("test-acc") ) resource.ParallelTest(t, resource.TestCase{ @@ -101,6 +103,8 @@ func TestAccClusterAdvancedCluster_singleProvider(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "retain_backups_enabled", "true"), resource.TestCheckResourceAttrSet(resourceName, "replication_specs.#"), resource.TestCheckResourceAttrSet(resourceName, "replication_specs.0.region_configs.#"), + resource.TestCheckResourceAttrWith(resourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", testutils.IntGreatThan(0)), + resource.TestCheckResourceAttrWith(dataSourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", testutils.IntGreatThan(0)), ), }, { @@ -675,7 +679,7 @@ var tagsMap3 = map[string]string{ func testAccCheckMongoDBAtlasAdvancedClusterExists(resourceName string, cluster *matlas.AdvancedCluster) resource.TestCheckFunc { return func(s *terraform.State) error { - conn := testAccProviderSdkV2.Meta().(*MongoDBClient).Atlas + conn := testMongoDBClient.(*MongoDBClient).Atlas rs, ok := s.RootModule().Resources[resourceName] if !ok { diff --git a/mongodbatlas/testutils/attribute_checks.go b/mongodbatlas/testutils/attribute_checks.go index b0cdc443be..438899552f 100644 --- a/mongodbatlas/testutils/attribute_checks.go +++ b/mongodbatlas/testutils/attribute_checks.go @@ -3,6 +3,7 @@ package testutils import ( "fmt" "regexp" + "strconv" "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) @@ -19,3 +20,16 @@ func MatchesExpression(expr string) resource.CheckResourceAttrWithFunc { return nil } } + +func IntGreatThan(value int) resource.CheckResourceAttrWithFunc { + return func(input string) error { + inputInt, err := strconv.Atoi(input) + if err != nil { + return err + } + if inputInt <= value { + return fmt.Errorf("%d is not greater than %d", inputInt, value) + } + return nil + } +} diff --git a/mongodbatlas/testutils/attribute_checks_test.go b/mongodbatlas/testutils/attribute_checks_test.go new file mode 100644 index 0000000000..1e3c8867ec --- /dev/null +++ b/mongodbatlas/testutils/attribute_checks_test.go @@ -0,0 +1,31 @@ +package testutils + +import "testing" + +func TestIntGreaterThan(t *testing.T) { + testCases := []struct { + name string + input string + errorMsg string + value int + wantErr bool + }{ + {"ValidGreater", "10", "", 5, false}, + {"ValidEqual", "5", "5 is not greater than 5", 5, true}, + {"ValidLess", "3", "3 is not greater than 5", 5, true}, + {"InvalidInput", "abc", "strconv.Atoi: parsing \"abc\": invalid syntax", 5, true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + checkFunc := IntGreatThan(tc.value) + err := checkFunc(tc.input) + if (err != nil) != tc.wantErr { + t.Errorf("IntGreatThan() error = %v, wantErr %v", err, tc.wantErr) + } + if err != nil && err.Error() != tc.errorMsg { + t.Errorf("IntGreatThan() error message = %v, want %v", err, tc.errorMsg) + } + }) + } +}