From 661e8804ba01fd76f33d6cc8c06df54f75534037 Mon Sep 17 00:00:00 2001 From: Leo Antoli <430982+lantoli@users.noreply.github.com> Date: Fri, 29 Nov 2024 17:49:49 +0100 Subject: [PATCH] chore: Includes the rest of blocks in TPF test converter (#2841) * don't remove double lines * enable TestAccClusterAdvancedCluster_withTags * tests for labels and tests * label and tags converter * common generateAllAttributeSpecs * change getVal return type * simplify generateAllAttrSpecs call * don't run data source checks for TPF yet * test for advanced_configuration * converter for advanced_configuration * bi_connector_config transform * test for bi_connector_config * fix tests * make explicit dependency from ds --- .../resource_advanced_cluster_test.go | 232 ++++++++++++++---- internal/testutil/acc/advanced_cluster.go | 14 ++ internal/testutil/acc/tpf_converter.go | 38 +-- internal/testutil/acc/tpf_converter_test.go | 82 +++++++ 4 files changed, 303 insertions(+), 63 deletions(-) diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index 70bdca054a..06b716cea4 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -210,6 +210,8 @@ func TestAccClusterAdvancedCluster_pausedToUnpaused(t *testing.T) { } func TestAccClusterAdvancedCluster_advancedConfig(t *testing.T) { + // TODO: Already prepared for TPF but getting this error: + // unexpected new value: .advanced_configuration.fail_index_key_too_long: was cty.False, but now null acc.SkipIfTPFAdvancedCluster(t) var ( projectID = acc.ProjectIDExecution(t) @@ -247,11 +249,11 @@ func TestAccClusterAdvancedCluster_advancedConfig(t *testing.T) { CheckDestroy: acc.CheckDestroyCluster, Steps: []resource.TestStep{ { - Config: configAdvanced(projectID, clusterName, processArgs, nil), + Config: acc.ConvertAdvancedClusterToTPF(t, configAdvanced(projectID, clusterName, processArgs, nil)), Check: checkAdvanced(clusterName, "TLS1_1", "-1"), }, { - Config: configAdvanced(projectID, clusterNameUpdated, processArgsUpdated, conversion.IntPtr(100)), + Config: acc.ConvertAdvancedClusterToTPF(t, configAdvanced(projectID, clusterNameUpdated, processArgsUpdated, conversion.IntPtr(100))), Check: checkAdvanced(clusterNameUpdated, "TLS1_2", "100"), }, }, @@ -417,7 +419,6 @@ func TestAccClusterAdvancedClusterConfig_singleShardedTransitionToOldSchemaExpec } func TestAccClusterAdvancedCluster_withTags(t *testing.T) { - acc.SkipIfTPFAdvancedCluster(t) var ( orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") projectName = acc.RandomProjectName() // No ProjectIDExecution to check correctly plural data source in the different test steps @@ -430,16 +431,44 @@ func TestAccClusterAdvancedCluster_withTags(t *testing.T) { CheckDestroy: acc.CheckDestroyCluster, Steps: []resource.TestStep{ { - Config: configWithTags(orgID, projectName, clusterName), - Check: checkTags(clusterName), + Config: acc.ConvertAdvancedClusterToTPF(t, configWithKeyValueBlocks(orgID, projectName, clusterName, "tags")), + Check: checkKeyValueBlocks(clusterName, "tags"), }, { - Config: configWithTags(orgID, projectName, clusterName, acc.ClusterTagsMap1, acc.ClusterTagsMap2), - Check: checkTags(clusterName, acc.ClusterTagsMap1, acc.ClusterTagsMap2), + Config: acc.ConvertAdvancedClusterToTPF(t, configWithKeyValueBlocks(orgID, projectName, clusterName, "tags", acc.ClusterTagsMap1, acc.ClusterTagsMap2)), + Check: checkKeyValueBlocks(clusterName, "tags", acc.ClusterTagsMap1, acc.ClusterTagsMap2), }, { - Config: configWithTags(orgID, projectName, clusterName, acc.ClusterTagsMap3), - Check: checkTags(clusterName, acc.ClusterTagsMap3), + Config: acc.ConvertAdvancedClusterToTPF(t, configWithKeyValueBlocks(orgID, projectName, clusterName, "tags", acc.ClusterTagsMap3)), + Check: checkKeyValueBlocks(clusterName, "tags", acc.ClusterTagsMap3), + }, + }, + }) +} + +func TestAccClusterAdvancedCluster_withLabels(t *testing.T) { + var ( + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acc.RandomProjectName() // No ProjectIDExecution to check correctly plural data source in the different test steps + clusterName = acc.RandomClusterName() + ) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckBasic(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + CheckDestroy: acc.CheckDestroyCluster, + Steps: []resource.TestStep{ + { + Config: acc.ConvertAdvancedClusterToTPF(t, configWithKeyValueBlocks(orgID, projectName, clusterName, "labels")), + Check: checkKeyValueBlocks(clusterName, "labels"), + }, + { + Config: acc.ConvertAdvancedClusterToTPF(t, configWithKeyValueBlocks(orgID, projectName, clusterName, "labels", acc.ClusterLabelsMap1, acc.ClusterLabelsMap2)), + Check: checkKeyValueBlocks(clusterName, "labels", acc.ClusterLabelsMap1, acc.ClusterLabelsMap2), + }, + { + Config: acc.ConvertAdvancedClusterToTPF(t, configWithKeyValueBlocks(orgID, projectName, clusterName, "labels", acc.ClusterLabelsMap3)), + Check: checkKeyValueBlocks(clusterName, "labels", acc.ClusterLabelsMap3), }, }, }) @@ -830,6 +859,28 @@ func TestAccClusterAdvancedCluster_priorityNewSchema(t *testing.T) { }) } +func TestAccClusterAdvancedCluster_biConnectorConfig(t *testing.T) { + var ( + projectID = acc.ProjectIDExecution(t) + clusterName = acc.RandomClusterName() + ) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: acc.PreCheckBasicSleep(t, nil, projectID, clusterName), + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + CheckDestroy: acc.CheckDestroyCluster, + Steps: []resource.TestStep{ + { + Config: acc.ConvertAdvancedClusterToTPF(t, configBiConnectorConfig(projectID, clusterName, false)), + Check: checkTenantBiConnectorConfig(projectID, clusterName, false), + }, + { + Config: acc.ConvertAdvancedClusterToTPF(t, configBiConnectorConfig(projectID, clusterName, true)), + Check: checkTenantBiConnectorConfig(projectID, clusterName, true), + }, + }, + }) +} + func checkAggr(attrsSet []string, attrsMap map[string]string, extra ...resource.TestCheckFunc) resource.TestCheckFunc { checks := make([]resource.TestCheckFunc, 0) if !acc.IsTPFAdvancedCluster() { // TODO: checkExists not implemented for TPF yet @@ -906,26 +957,25 @@ func checkTenant(projectID, name string) resource.TestCheckFunc { "project_id": projectID, "name": name, "termination_protection_enabled": "false", - "global_cluster_self_managed_sharding": "false", - "labels.#": "0"}, + "global_cluster_self_managed_sharding": "false"}, pluralChecks...) } -func configWithTags(orgID, projectName, name string, tags ...map[string]string) string { - var tagsConf string - for _, label := range tags { - tagsConf += fmt.Sprintf(` - tags { - key = "%s" - value = "%s" +func configWithKeyValueBlocks(orgID, projectName, clusterName, blockName string, blocks ...map[string]string) string { + var extraConfig string + for _, block := range blocks { + extraConfig += fmt.Sprintf(` + %[1]s { + key = %[2]q + value = %[3]q } - `, label["key"], label["value"]) + `, blockName, block["key"], block["value"]) } return fmt.Sprintf(` resource "mongodbatlas_project" "cluster_project" { - name = %[2]q org_id = %[1]q + name = %[2]q } resource "mongodbatlas_advanced_cluster" "test" { @@ -960,28 +1010,37 @@ func configWithTags(orgID, projectName, name string, tags ...map[string]string) data "mongodbatlas_advanced_clusters" "test" { project_id = mongodbatlas_advanced_cluster.test.project_id } - `, orgID, projectName, name, tagsConf) + `, orgID, projectName, clusterName, extraConfig) } -func checkTags(name string, tags ...map[string]string) resource.TestCheckFunc { - lenStr := strconv.Itoa(len(tags)) - tagChecks := []resource.TestCheckFunc{ - resource.TestCheckResourceAttr(resourceName, "tags.#", lenStr), - resource.TestCheckResourceAttr(dataSourceName, "tags.#", lenStr), - resource.TestCheckResourceAttr(dataSourcePluralName, "results.0.tags.#", lenStr), +func checkKeyValueBlocks(clusterName, blockName string, blocks ...map[string]string) resource.TestCheckFunc { + const pluralPrefix = "results.0." + lenStr := strconv.Itoa(len(blocks)) + keyHash := fmt.Sprintf("%s.#", blockName) + keyStar := fmt.Sprintf("%s.*", blockName) + checks := []resource.TestCheckFunc{ + resource.TestCheckResourceAttr(resourceName, keyHash, lenStr), } - for _, tag := range tags { - tagChecks = append(tagChecks, - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "tags.*", tag), - resource.TestCheckTypeSetElemNestedAttrs(dataSourceName, "tags.*", tag), - resource.TestCheckTypeSetElemNestedAttrs(dataSourcePluralName, "results.0.tags.*", tag)) + if !acc.IsTPFAdvancedCluster() { // TODO: data sources not implemented for TPF yet + checks = append(checks, + resource.TestCheckResourceAttr(dataSourceName, keyHash, lenStr), + resource.TestCheckResourceAttr(dataSourcePluralName, pluralPrefix+keyHash, lenStr), + ) + } + for _, block := range blocks { + checks = append(checks, resource.TestCheckTypeSetElemNestedAttrs(resourceName, keyStar, block)) + if !acc.IsTPFAdvancedCluster() { // TODO: data sources not implemented for TPF yet + checks = append(checks, + resource.TestCheckTypeSetElemNestedAttrs(dataSourceName, keyStar, block), + resource.TestCheckTypeSetElemNestedAttrs(dataSourcePluralName, pluralPrefix+keyStar, block)) + } } return checkAggr( []string{"project_id"}, map[string]string{ - "name": name, + "name": clusterName, }, - tagChecks...) + checks...) } func configReplicaSetAWSProvider(projectID, name string, diskSizeGB, nodeCountElectable int) string { @@ -1346,22 +1405,30 @@ func configAdvanced(projectID, clusterName string, p *admin20240530.ClusterDescr } func checkAdvanced(name, tls, changeStreamOptions string) resource.TestCheckFunc { + pluralChecks := []resource.TestCheckFunc{ + resource.TestCheckResourceAttrSet(dataSourcePluralName, "results.#"), + resource.TestCheckResourceAttrSet(dataSourcePluralName, "results.0.replication_specs.#"), + resource.TestCheckResourceAttrSet(dataSourcePluralName, "results.0.name"), + } + prefix := "advanced_configuration.0." + if acc.IsTPFAdvancedCluster() { // TODO: data sources not implemented for TPF yet + pluralChecks = nil + prefix = "advanced_configuration." + } return checkAggr( []string{"project_id", "replication_specs.#", "replication_specs.0.region_configs.#"}, map[string]string{ - "name": name, - "advanced_configuration.0.minimum_enabled_tls_protocol": tls, - "advanced_configuration.0.fail_index_key_too_long": "false", - "advanced_configuration.0.javascript_enabled": "true", - "advanced_configuration.0.no_table_scan": "false", - "advanced_configuration.0.oplog_size_mb": "1000", - "advanced_configuration.0.sample_refresh_interval_bi_connector": "310", - "advanced_configuration.0.sample_size_bi_connector": "110", - "advanced_configuration.0.transaction_lifetime_limit_seconds": "300", - "advanced_configuration.0.change_stream_options_pre_and_post_images_expire_after_seconds": changeStreamOptions}, - resource.TestCheckResourceAttrSet(dataSourcePluralName, "results.#"), - resource.TestCheckResourceAttrSet(dataSourcePluralName, "results.0.replication_specs.#"), - resource.TestCheckResourceAttrSet(dataSourcePluralName, "results.0.name")) + "name": name, + prefix + "minimum_enabled_tls_protocol": tls, + prefix + "fail_index_key_too_long": "false", + prefix + "javascript_enabled": "true", + prefix + "no_table_scan": "false", + prefix + "oplog_size_mb": "1000", + prefix + "sample_refresh_interval_bi_connector": "310", + prefix + "sample_size_bi_connector": "110", + prefix + "transaction_lifetime_limit_seconds": "300", + prefix + "change_stream_options_pre_and_post_images_expire_after_seconds": changeStreamOptions}, + pluralChecks...) } func configAdvancedDefaultWrite(projectID, clusterName string, p *admin20240530.ClusterDescriptionProcessArgs) string { @@ -2238,3 +2305,74 @@ func configPriority(orgID, projectName, clusterName string, oldSchema, swapPrior } `, orgID, projectName, clusterName, strType, strNumShards, strConfigs) } + +func configBiConnectorConfig(projectID, name string, enabled bool) string { + additionalConfig := ` + bi_connector_config { + enabled = false + } + ` + if enabled { + additionalConfig = ` + bi_connector_config { + enabled = true + read_preference = "secondary" + } + ` + } + + return fmt.Sprintf(` + resource "mongodbatlas_advanced_cluster" "test" { + project_id = %[1]q + name = %[2]q + cluster_type = "REPLICASET" + + replication_specs { + region_configs { + electable_specs { + instance_size = "M10" + node_count = 3 + } + analytics_specs { + instance_size = "M10" + node_count = 1 + } + provider_name = "AWS" + priority = 7 + region_name = "US_WEST_2" + } + } + + %[3]s + } + + data "mongodbatlas_advanced_cluster" "test" { + project_id = mongodbatlas_advanced_cluster.test.project_id + name = mongodbatlas_advanced_cluster.test.name + depends_on = [mongodbatlas_advanced_cluster.test] + } + + data "mongodbatlas_advanced_clusters" "test" { + project_id = mongodbatlas_advanced_cluster.test.project_id + depends_on = [mongodbatlas_advanced_cluster.test] + } + `, projectID, name, additionalConfig) +} + +func checkTenantBiConnectorConfig(projectID, name string, enabled bool) resource.TestCheckFunc { + prefix := "bi_connector_config.0." + if acc.IsTPFAdvancedCluster() { + prefix = "bi_connector_config." + } + attrsMap := map[string]string{ + "project_id": projectID, + "name": name, + } + if enabled { + attrsMap[prefix+"enabled"] = "true" + attrsMap[prefix+"read_preference"] = "secondary" + } else { + attrsMap[prefix+"enabled"] = "false" + } + return checkAggr(nil, attrsMap) +} diff --git a/internal/testutil/acc/advanced_cluster.go b/internal/testutil/acc/advanced_cluster.go index 28daa95ec3..025b11724c 100644 --- a/internal/testutil/acc/advanced_cluster.go +++ b/internal/testutil/acc/advanced_cluster.go @@ -26,6 +26,20 @@ var ( "key": "key 3", "value": "value 3", } + ClusterLabelsMap1 = map[string]string{ + "key": "label key 1", + "value": "label value 1", + } + + ClusterLabelsMap2 = map[string]string{ + "key": "label key 2", + "value": "label value 2", + } + + ClusterLabelsMap3 = map[string]string{ + "key": "label key 3", + "value": "label value 3", + } ) func CheckDestroyCluster(s *terraform.State) error { diff --git a/internal/testutil/acc/tpf_converter.go b/internal/testutil/acc/tpf_converter.go index 439cda5762..ea67dbde3b 100644 --- a/internal/testutil/acc/tpf_converter.go +++ b/internal/testutil/acc/tpf_converter.go @@ -1,7 +1,6 @@ package acc import ( - "bytes" "testing" "github.com/hashicorp/hcl/v2" @@ -27,11 +26,13 @@ func ConvertAdvancedClusterToTPF(t *testing.T, def string) string { continue } writeBody := resource.Body() - generateAllReplicationSpecs(t, writeBody) + convertAttrs(t, "labels", writeBody, true, getAttrVal) + convertAttrs(t, "tags", writeBody, true, getAttrVal) + convertAttrs(t, "replication_specs", writeBody, true, getReplicationSpecs) + convertAttrs(t, "advanced_configuration", writeBody, false, getAttrVal) + convertAttrs(t, "bi_connector_config", writeBody, false, getAttrVal) } content := parse.Bytes() - // RemoveBlock is not deleting the newline at the end of the block - content = bytes.ReplaceAll(content, []byte("\n\n"), []byte("\n")) return string(content) } @@ -40,37 +41,42 @@ func AssertEqualHCL(t *testing.T, expected, actual string, msgAndArgs ...interfa assert.Equal(t, canonicalHCL(t, expected), canonicalHCL(t, actual), msgAndArgs...) } -func generateAllReplicationSpecs(t *testing.T, writeBody *hclwrite.Body) { +func convertAttrs(t *testing.T, name string, writeBody *hclwrite.Body, isList bool, getOneAttr func(*testing.T, *hclsyntax.Body) cty.Value) { t.Helper() - const name = "replication_specs" var vals []cty.Value for { match := writeBody.FirstMatchingBlock(name, nil) if match == nil { break } - vals = append(vals, getOneReplicationSpecs(t, getBlockBody(t, match))) - writeBody.RemoveBlock(match) + vals = append(vals, getOneAttr(t, getBlockBody(t, match))) + writeBody.RemoveBlock(match) // TODO: RemoveBlock doesn't remove newline just after the block so an extra line is added + } + if len(vals) == 0 { + return + } + if isList { + writeBody.SetAttributeValue(name, cty.TupleVal(vals)) + } else { + assert.Len(t, vals, 1, "can be only one of %s", name) + writeBody.SetAttributeValue(name, vals[0]) } - require.NotEmpty(t, vals, "there must be at least one %s block", name) - writeBody.SetAttributeValue(name, cty.TupleVal(vals)) } -func getOneReplicationSpecs(t *testing.T, body *hclsyntax.Body) cty.Value { +func getReplicationSpecs(t *testing.T, body *hclsyntax.Body) cty.Value { t.Helper() const name = "region_configs" var vals []cty.Value for _, block := range body.Blocks { assert.Equal(t, name, block.Type, "unexpected block type: %s", block.Type) - oneRegionConfigs := cty.ObjectVal(getVal(t, block.Body)) - vals = append(vals, oneRegionConfigs) + vals = append(vals, getAttrVal(t, block.Body)) } return cty.ObjectVal(map[string]cty.Value{ name: cty.TupleVal(vals), }) } -func getVal(t *testing.T, body *hclsyntax.Body) map[string]cty.Value { +func getAttrVal(t *testing.T, body *hclsyntax.Body) cty.Value { t.Helper() ret := make(map[string]cty.Value) for name, attr := range body.Attributes { @@ -79,9 +85,9 @@ func getVal(t *testing.T, body *hclsyntax.Body) map[string]cty.Value { ret[name] = val } for _, block := range body.Blocks { - ret[block.Type] = cty.ObjectVal(getVal(t, block.Body)) + ret[block.Type] = getAttrVal(t, block.Body) } - return ret + return cty.ObjectVal(ret) } func canonicalHCL(t *testing.T, def string) string { diff --git a/internal/testutil/acc/tpf_converter_test.go b/internal/testutil/acc/tpf_converter_test.go index 4abc4b5fef..30d963383e 100644 --- a/internal/testutil/acc/tpf_converter_test.go +++ b/internal/testutil/acc/tpf_converter_test.go @@ -58,6 +58,48 @@ func TestConvertAdvancedClusterToTPF(t *testing.T) { region_name = "EU_WEST_1" } } + + tags { + key = "Key Tag 2" + value = "Value Tag 2" + } + + labels { + key = "Key Label 1" + value = "Value Label 1" + } + + tags { + key = "Key Tag 1" + value = "Value Tag 1" + } + + labels { + key = "Key Label 2" + value = "Value Label 2" + } + + labels { + key = "Key Label 3" + value = "Value Label 3" + } + + advanced_configuration { + fail_index_key_too_long = false + javascript_enabled = true + minimum_enabled_tls_protocol = "TLS1_1" + no_table_scan = false + oplog_size_mb = 1000 + sample_size_bi_connector = 110 + sample_refresh_interval_bi_connector = 310 + transaction_lifetime_limit_seconds = 300 + change_stream_options_pre_and_post_images_expire_after_seconds = 100 + } + + bi_connector_config { + enabled = true + read_preference = "secondary" + } } ` // expected has the attributes sorted alphabetically to match the output of ConvertAdvancedClusterToTPF @@ -67,6 +109,31 @@ func TestConvertAdvancedClusterToTPF(t *testing.T) { name = "cluster2" cluster_type = "SHARDED" + + + + + + + + + labels = [{ + key = "Key Label 1" + value = "Value Label 1" + }, { + key = "Key Label 2" + value = "Value Label 2" + }, { + key = "Key Label 3" + value = "Value Label 3" + }] + tags = [{ + key = "Key Tag 2" + value = "Value Tag 2" + }, { + key = "Key Tag 1" + value = "Value Tag 1" + }] replication_specs = [{ region_configs = [{ analytics_specs = { @@ -108,6 +175,21 @@ func TestConvertAdvancedClusterToTPF(t *testing.T) { region_name = "EU_WEST_1" }] }] + advanced_configuration = { + change_stream_options_pre_and_post_images_expire_after_seconds = 100 + fail_index_key_too_long = false + javascript_enabled = true + minimum_enabled_tls_protocol = "TLS1_1" + no_table_scan = false + oplog_size_mb = 1000 + sample_refresh_interval_bi_connector = 310 + sample_size_bi_connector = 110 + transaction_lifetime_limit_seconds = 300 + } + bi_connector_config = { + enabled = true + read_preference = "secondary" + } } ` )