diff --git a/internal/convert/adv2v2.go b/internal/convert/adv2v2.go index 2e3e3c3..734da45 100644 --- a/internal/convert/adv2v2.go +++ b/internal/convert/adv2v2.go @@ -80,18 +80,13 @@ func convertRepSpecs(resourceb *hclwrite.Body, diskSizeGB hclwrite.Tokens) error blockb := block.Body() shardsAttr := blockb.GetAttribute(nNumShards) blockb.RemoveAttribute(nNumShards) - dConfig, err := getDynamicBlock(blockb, nConfig) + dConfig, err := convertConfigsWithDynamicBlock(blockb, diskSizeGB, false) if err != nil { return err } if dConfig.IsPresent() { - transformReferences(dConfig.content.Body(), getResourceName(dConfig.block), nRegion) - copyAttributesSorted(dConfig.content.Body(), dConfig.content.Body().Attributes()) - processAllSpecs(dConfig.content.Body(), diskSizeGB) - tokens := hcl.TokensFromExpr(buildForExpr(nRegion, hcl.GetAttrExpr(dConfig.forEach), false)) - tokens = append(tokens, hcl.TokensObject(dConfig.content.Body())...) - blockb.SetAttributeRaw(nConfig, hcl.EncloseBracketsNewLines(tokens)) blockb.RemoveBlock(dConfig.block) + blockb.SetAttributeRaw(nConfig, dConfig.tokens) } else { var configs []*hclwrite.Body for _, configBlock := range collectBlocks(blockb, nConfig) { @@ -125,12 +120,12 @@ func convertRepSpecs(resourceb *hclwrite.Body, diskSizeGB hclwrite.Tokens) error } func convertRepSpecsWithDynamicBlock(resourceb *hclwrite.Body, diskSizeGB hclwrite.Tokens) (dynamicBlock, error) { - dSpec, err := getDynamicBlock(resourceb, nRepSpecs) + dSpec, err := getDynamicBlock(resourceb, nRepSpecs, true) if err != nil || !dSpec.IsPresent() { return dynamicBlock{}, err } transformReferences(dSpec.content.Body(), nRepSpecs, nSpec) - dConfig, err := convertConfigsWithDynamicBlock(dSpec.content.Body(), diskSizeGB) + dConfig, err := convertConfigsWithDynamicBlock(dSpec.content.Body(), diskSizeGB, true) if err != nil { return dynamicBlock{}, err } @@ -139,9 +134,10 @@ func convertRepSpecsWithDynamicBlock(resourceb *hclwrite.Body, diskSizeGB hclwri return dSpec, nil } -func convertConfigsWithDynamicBlock(specbSrc *hclwrite.Body, diskSizeGB hclwrite.Tokens) (dynamicBlock, error) { - d, err := getDynamicBlock(specbSrc, nConfig) - if err != nil { +func convertConfigsWithDynamicBlock(specbSrc *hclwrite.Body, diskSizeGB hclwrite.Tokens, + insideDynamicRepSpec bool) (dynamicBlock, error) { + d, err := getDynamicBlock(specbSrc, nConfig, true) + if err != nil || !d.IsPresent() { return dynamicBlock{}, err } configBody := d.content.Body() @@ -158,17 +154,25 @@ func convertConfigsWithDynamicBlock(specbSrc *hclwrite.Body, diskSizeGB hclwrite } regionConfigBody.SetAttributeRaw(blockType, hcl.TokensObject(blockBody)) } + forEach := hcl.GetAttrExpr(d.forEach) + if insideDynamicRepSpec { + forEach = fmt.Sprintf("%s.%s", nSpec, nConfig) + } + regionTokens := hcl.TokensFromExpr(buildForExpr(nRegion, forEach, false)) + regionTokens = append(regionTokens, hcl.TokensObject(regionConfigBody)...) + if !insideDynamicRepSpec { + d.tokens = hcl.EncloseBracketsNewLines(regionTokens) + return d, nil + } repSpecb := hclwrite.NewEmptyFile().Body() if zoneNameAttr := specbSrc.GetAttribute(nZoneName); zoneNameAttr != nil { - repSpecb.SetAttributeRaw(nZoneName, hcl.TokensFromExpr( - transformReference(hcl.GetAttrExpr(zoneNameAttr), nRepSpecs, nSpec))) + zoneNameExpr := transformReference(hcl.GetAttrExpr(zoneNameAttr), nRepSpecs, nSpec) + repSpecb.SetAttributeRaw(nZoneName, hcl.TokensFromExpr(zoneNameExpr)) } - regionTokens := hcl.TokensFromExpr(buildForExpr(nRegion, fmt.Sprintf("%s.%s", nSpec, nConfig), false)) - regionTokens = append(regionTokens, hcl.TokensObject(regionConfigBody)...) repSpecb.SetAttributeRaw(nConfig, hcl.EncloseBracketsNewLines(regionTokens)) if numShardsAttr := specbSrc.GetAttribute(nNumShards); numShardsAttr != nil { - tokens := hcl.TokensFromExpr(buildForExpr("i", - fmt.Sprintf("range(%s)", transformReference(hcl.GetAttrExpr(numShardsAttr), nRepSpecs, nSpec)), false)) + numShardsExpr := transformReference(hcl.GetAttrExpr(numShardsAttr), nRepSpecs, nSpec) + tokens := hcl.TokensFromExpr(buildForExpr("i", fmt.Sprintf("range(%s)", numShardsExpr), false)) tokens = append(tokens, hcl.TokensObject(repSpecb)...) return dynamicBlock{tokens: hcl.EncloseBracketsNewLines(tokens)}, nil } diff --git a/internal/convert/clu2adv.go b/internal/convert/clu2adv.go index 7fae2a3..620c990 100644 --- a/internal/convert/clu2adv.go +++ b/internal/convert/clu2adv.go @@ -75,7 +75,7 @@ func convertResource(block *hclwrite.Block) (bool, error) { } func isFreeTierCluster(resourceb *hclwrite.Body) bool { - d, _ := getDynamicBlock(resourceb, nRepSpecs) + d, _ := getDynamicBlock(resourceb, nRepSpecs, true) return resourceb.FirstMatchingBlock(nRepSpecs, nil) == nil && !d.IsPresent() } @@ -221,7 +221,7 @@ func fillRepSpecs(resourceb *hclwrite.Body, root attrVals) error { // fillRepSpecsWithDynamicBlock used for dynamic blocks in replication_specs func fillRepSpecsWithDynamicBlock(resourceb *hclwrite.Body, root attrVals) (dynamicBlock, error) { - dSpec, err := getDynamicBlock(resourceb, nRepSpecs) + dSpec, err := getDynamicBlock(resourceb, nRepSpecs, true) if err != nil || !dSpec.IsPresent() { return dynamicBlock{}, err } @@ -239,7 +239,7 @@ func fillRepSpecsWithDynamicBlock(resourceb *hclwrite.Body, root attrVals) (dyna // fillConfigsWithDynamicRegion is used for dynamic blocks in region_configs func fillConfigsWithDynamicRegion(specbSrc *hclwrite.Body, root attrVals, changeReferences bool) (dynamicBlock, error) { - d, err := getDynamicBlock(specbSrc, nConfigSrc) + d, err := getDynamicBlock(specbSrc, nConfigSrc, true) if err != nil || !d.IsPresent() { return dynamicBlock{}, err } diff --git a/internal/convert/convert_test.go b/internal/convert/convert_test.go index da2b1cc..63c47a7 100644 --- a/internal/convert/convert_test.go +++ b/internal/convert/convert_test.go @@ -27,6 +27,11 @@ func runConvertTests(t *testing.T, cmdName string, convert func(testName string, require.NoError(t, err) err = json.Unmarshal(errContent, &errMap) require.NoError(t, err) + unusedErrors := make(map[string]struct{}) + for name := range errMap { + unusedErrors[name] = struct{}{} + } + g := goldie.New(t, goldie.WithFixtureDir(root), goldie.WithNameSuffix(outSuffix)) @@ -46,7 +51,9 @@ func runConvertTests(t *testing.T, cmdName string, convert func(testName string, errMsg, found := errMap[testName] assert.True(t, found, "error not found in file %s for test %s, errMsg: %v", errFilename, testName, err) assert.Contains(t, err.Error(), errMsg) + delete(unusedErrors, testName) } }) } + assert.Empty(t, unusedErrors, "some errors are not being used") } diff --git a/internal/convert/shared.go b/internal/convert/shared.go index 9d11e77..ffdff9f 100644 --- a/internal/convert/shared.go +++ b/internal/convert/shared.go @@ -1,6 +1,7 @@ package convert import ( + "errors" "fmt" "slices" "strings" @@ -9,6 +10,10 @@ import ( "github.com/mongodb-labs/atlas-cli-plugin-terraform/internal/hcl" ) +var ( + errDynamicBlockAlone = errors.New("dynamic block must be the only block, see docs for more information") +) + // hasVariableNumShards checks if any block has a variable (non-literal) num_shards attribute func hasVariableNumShards(blocks []*hclwrite.Block) bool { for _, block := range blocks { @@ -51,8 +56,13 @@ func (d dynamicBlock) IsPresent() bool { } // getDynamicBlock finds and returns a dynamic block with the given name from the body -func getDynamicBlock(body *hclwrite.Body, name string) (dynamicBlock, error) { +func getDynamicBlock(body *hclwrite.Body, name string, checkAlone bool) (dynamicBlock, error) { + var db dynamicBlock + staticBlockCount := 0 for _, block := range body.Blocks() { + if block.Type() == name { + staticBlockCount++ + } if block.Type() != nDynamic || name != getResourceName(block) { continue } @@ -65,9 +75,14 @@ func getDynamicBlock(body *hclwrite.Body, name string) (dynamicBlock, error) { if content == nil { return dynamicBlock{}, fmt.Errorf("dynamic block %s: block %s not found", name, nContent) } - return dynamicBlock{forEach: forEach, block: block, content: content}, nil + if !db.IsPresent() { + db = dynamicBlock{forEach: forEach, block: block, content: content} + } + } + if checkAlone && db.IsPresent() && staticBlockCount > 0 { + return dynamicBlock{}, errDynamicBlockAlone } - return dynamicBlock{}, nil + return db, nil } func checkDynamicBlock(body *hclwrite.Body) error { @@ -180,7 +195,7 @@ func fillTagsLabelsOpt(resourceb *hclwrite.Body, name string) error { } func extractTagsLabelsDynamicBlock(resourceb *hclwrite.Body, name string) (hclwrite.Tokens, error) { - d, err := getDynamicBlock(resourceb, name) + d, err := getDynamicBlock(resourceb, name, false) if err != nil || !d.IsPresent() { return nil, err } diff --git a/internal/convert/testdata/adv2v2/dynamic_regions_config_invalid_multiple_blocks.in.tf b/internal/convert/testdata/adv2v2/dynamic_regions_config_invalid_multiple_blocks.in.tf new file mode 100644 index 0000000..dfc77d4 --- /dev/null +++ b/internal/convert/testdata/adv2v2/dynamic_regions_config_invalid_multiple_blocks.in.tf @@ -0,0 +1,29 @@ +resource "mongodbatlas_advanced_cluster" "multiple_blocks" { + project_id = var.project_id + name = var.cluster_name + cluster_type = var.cluster_type + replication_specs { + num_shards = var.replication_specs.num_shards + dynamic "region_configs" { + for_each = var.replication_specs.region_configs + content { + priority = region_configs.value.priority + provider_name = region_configs.value.provider_name + region_name = region_configs.value.region_name + electable_specs { + instance_size = region_configs.value.instance_size + node_count = region_configs.value.electable_node_count + } + } + } + region_configs { # inline block is not allowed with dynamic blocks + priority = 0 + provider_name = "AWS" + region_name = "US_EAST_1" + read_only_specs { + instance_size = var.instance_size + node_count = 1 + } + } + } +} diff --git a/internal/convert/testdata/adv2v2/dynamic_replication_specs_invalid_multiple_blocks.in.tf b/internal/convert/testdata/adv2v2/dynamic_replication_specs_invalid_multiple_blocks.in.tf new file mode 100644 index 0000000..04f23fd --- /dev/null +++ b/internal/convert/testdata/adv2v2/dynamic_replication_specs_invalid_multiple_blocks.in.tf @@ -0,0 +1,34 @@ +resource "mongodbatlas_advanced_cluster" "multiple_blocks" { + project_id = var.project_id + name = var.cluster_name + cluster_type = var.cluster_type + dynamic "replication_specs" { + for_each = var.replication_specs + content { + num_shards = replication_specs.value.num_shards + dynamic "region_configs" { + for_each = replication_specs.value.region_configs + content { + priority = region_configs.value.priority + provider_name = region_configs.value.provider_name + region_name = region_configs.value.region_name + electable_specs { + instance_size = region_configs.value.instance_size + node_count = region_configs.value.electable_node_count + } + } + } + } + } + replication_specs { # inline block is not allowed with dynamic blocks + region_configs { + priority = 7 + provider_name = "AWS" + region_name = "EU_WEST_1" + electable_specs { + instance_size = "M10" + node_count = 3 + } + } + } +} diff --git a/internal/convert/testdata/adv2v2/dynamic_replication_specs_invalid_multiple_config_blocks.in.tf b/internal/convert/testdata/adv2v2/dynamic_replication_specs_invalid_multiple_config_blocks.in.tf new file mode 100644 index 0000000..3641ffe --- /dev/null +++ b/internal/convert/testdata/adv2v2/dynamic_replication_specs_invalid_multiple_config_blocks.in.tf @@ -0,0 +1,32 @@ +resource "mongodbatlas_advanced_cluster" "multiple_blocks" { + project_id = var.project_id + name = var.cluster_name + cluster_type = var.cluster_type + dynamic "replication_specs" { + for_each = var.replication_specs + content { + num_shards = replication_specs.value.num_shards + dynamic "region_configs" { + for_each = replication_specs.value.region_configs + content { + priority = region_configs.value.priority + provider_name = region_configs.value.provider_name + region_name = region_configs.value.region_name + electable_specs { + instance_size = region_configs.value.instance_size + node_count = region_configs.value.electable_node_count + } + } + } + region_configs { # inline block is not allowed with dynamic blocks + priority = 0 + provider_name = "AWS" + region_name = "US_EAST_1" + read_only_specs { + instance_size = var.instance_size + node_count = 1 + } + } + } + } +} diff --git a/internal/convert/testdata/adv2v2/errors.json b/internal/convert/testdata/adv2v2/errors.json index dae5a07..a744913 100644 --- a/internal/convert/testdata/adv2v2/errors.json +++ b/internal/convert/testdata/adv2v2/errors.json @@ -2,5 +2,8 @@ "configuration_file_error": "failed to parse Terraform config file", "replication_specs_missing_region_configs": "replication_specs must have at least one region_configs", "missing_replication_specs": "must have at least one replication_specs", - "dynamic_unsupported_tag": "dynamic blocks are not supported for advanced_configuration" + "dynamic_unsupported_tag": "dynamic blocks are not supported for advanced_configuration", + "dynamic_regions_config_invalid_multiple_blocks": "dynamic block must be the only block", + "dynamic_replication_specs_invalid_multiple_blocks": "dynamic block must be the only block", + "dynamic_replication_specs_invalid_multiple_config_blocks": "dynamic block must be the only block" } diff --git a/internal/convert/testdata/clu2adv/dynamic_regions_config_invalid_multiple_blocks.in.tf b/internal/convert/testdata/clu2adv/dynamic_regions_config_invalid_multiple_blocks.in.tf new file mode 100644 index 0000000..6e246c6 --- /dev/null +++ b/internal/convert/testdata/clu2adv/dynamic_regions_config_invalid_multiple_blocks.in.tf @@ -0,0 +1,23 @@ +resource "mongodbatlas_cluster" "multiple_blocks" { + project_id = var.project_id + name = "cluster" + cluster_type = "SHARDED" + provider_name = "AWS" + provider_instance_size_name = "M10" + replication_specs { + num_shards = var.replication_specs.num_shards + dynamic "regions_config" { + for_each = var.replication_specs.regions_config + content { + region_name = regions_config.value.region_name + electable_nodes = regions_config.value.electable_nodes + priority = regions_config.value.prio + read_only_nodes = regions_config.value.read_only_nodes + } + } + regions_config { # inline block is not allowed with dynamic blocks + region_name = "US_EAST_1" + read_only_nodes = 1 + } + } +} diff --git a/internal/convert/testdata/clu2adv/dynamic_replication_specs_invalid_multiple_blocks.in.tf b/internal/convert/testdata/clu2adv/dynamic_replication_specs_invalid_multiple_blocks.in.tf new file mode 100644 index 0000000..d01ab87 --- /dev/null +++ b/internal/convert/testdata/clu2adv/dynamic_replication_specs_invalid_multiple_blocks.in.tf @@ -0,0 +1,35 @@ +# Based on https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/examples/migrate_cluster_to_advanced_cluster/module_maintainer/v1/main.tf +resource "mongodbatlas_cluster" "this" { + project_id = var.project_id + name = var.cluster_name + auto_scaling_disk_gb_enabled = var.auto_scaling_disk_gb_enabled + cluster_type = var.cluster_type + disk_size_gb = var.disk_size + mongo_db_major_version = var.mongo_db_major_version + provider_instance_size_name = var.instance_size + provider_name = var.provider_name + dynamic "replication_specs" { + for_each = var.replication_specs + content { + num_shards = replication_specs.value.num_shards + zone_name = replication_specs.value.zone_name + dynamic "regions_config" { + for_each = replication_specs.value.regions_config + content { + electable_nodes = regions_config.value.electable_nodes + priority = regions_config.value.priority + read_only_nodes = regions_config.value.read_only_nodes + region_name = regions_config.value.region_name + } + } + } + } + replication_specs { # inline block is not allowed with dynamic blocks + num_shards = 1 + regions_config { + region_name = "US_EAST_1" + electable_nodes = 3 + priority = 7 + } + } +} diff --git a/internal/convert/testdata/clu2adv/dynamic_replication_specs_invalid_multiple_config_blocks.in.tf b/internal/convert/testdata/clu2adv/dynamic_replication_specs_invalid_multiple_config_blocks.in.tf new file mode 100644 index 0000000..6516d28 --- /dev/null +++ b/internal/convert/testdata/clu2adv/dynamic_replication_specs_invalid_multiple_config_blocks.in.tf @@ -0,0 +1,31 @@ +# Based on https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/examples/migrate_cluster_to_advanced_cluster/module_maintainer/v1/main.tf +resource "mongodbatlas_cluster" "this" { + project_id = var.project_id + name = var.cluster_name + auto_scaling_disk_gb_enabled = var.auto_scaling_disk_gb_enabled + cluster_type = var.cluster_type + disk_size_gb = var.disk_size + mongo_db_major_version = var.mongo_db_major_version + provider_instance_size_name = var.instance_size + provider_name = var.provider_name + dynamic "replication_specs" { + for_each = var.replication_specs + content { + num_shards = replication_specs.value.num_shards + zone_name = replication_specs.value.zone_name + dynamic "regions_config" { + for_each = replication_specs.value.regions_config + content { + electable_nodes = regions_config.value.electable_nodes + priority = regions_config.value.priority + read_only_nodes = regions_config.value.read_only_nodes + region_name = regions_config.value.region_name + } + } + regions_config { # inline block is not allowed with dynamic blocks + region_name = "US_EAST_1" + read_only_nodes = 1 + } + } + } +} diff --git a/internal/convert/testdata/clu2adv/errors.json b/internal/convert/testdata/clu2adv/errors.json index 266aa5e..fcee178 100644 --- a/internal/convert/testdata/clu2adv/errors.json +++ b/internal/convert/testdata/clu2adv/errors.json @@ -5,5 +5,8 @@ "regions_config_missing_priority": "setting replication_specs: attribute priority not found", "replication_specs_missing_num_shards": "num_shards not found", "replication_specs_missing_regions_config": "setting replication_specs: regions_config not found", - "dynamic_unsupported_tag": "dynamic blocks are not supported for advanced_configuration" + "dynamic_unsupported_tag": "dynamic blocks are not supported for advanced_configuration", + "dynamic_regions_config_invalid_multiple_blocks": "dynamic block must be the only block", + "dynamic_replication_specs_invalid_multiple_blocks": "dynamic block must be the only block", + "dynamic_replication_specs_invalid_multiple_config_blocks": "dynamic block must be the only block" }