feat: Handle errors for unsupported dynamic block use cases

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
 	}

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
 	}

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")
 }

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
 	}

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
+      }
+    }
+  }
+}

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
+      }
+    }
+  }
+}

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
+        }
+      }
+    }
+  }
+}

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"
 }

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
+    }
+  }
+}

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
+    }
+  }
+}

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
+      }
+    }
+  }
+}