Skip to content

Commit 89c734f

Browse files
lantolilantoli
authored
feat: Handle errors for unsupported dynamic block use cases (#69)
1 parent 8fec80a commit 89c734f

12 files changed

+243
-27
lines changed

‎internal/convert/adv2v2.go

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,13 @@ func convertRepSpecs(resourceb *hclwrite.Body, diskSizeGB hclwrite.Tokens) error
8080
blockb := block.Body()
8181
shardsAttr := blockb.GetAttribute(nNumShards)
8282
blockb.RemoveAttribute(nNumShards)
83-
dConfig, err := getDynamicBlock(blockb, nConfig)
83+
dConfig, err := convertConfigsWithDynamicBlock(blockb, diskSizeGB, false)
8484
if err != nil {
8585
return err
8686
}
8787
if dConfig.IsPresent() {
88-
transformReferences(dConfig.content.Body(), getResourceName(dConfig.block), nRegion)
89-
copyAttributesSorted(dConfig.content.Body(), dConfig.content.Body().Attributes())
90-
processAllSpecs(dConfig.content.Body(), diskSizeGB)
91-
tokens := hcl.TokensFromExpr(buildForExpr(nRegion, hcl.GetAttrExpr(dConfig.forEach), false))
92-
tokens = append(tokens, hcl.TokensObject(dConfig.content.Body())...)
93-
blockb.SetAttributeRaw(nConfig, hcl.EncloseBracketsNewLines(tokens))
9488
blockb.RemoveBlock(dConfig.block)
89+
blockb.SetAttributeRaw(nConfig, dConfig.tokens)
9590
} else {
9691
var configs []*hclwrite.Body
9792
for _, configBlock := range collectBlocks(blockb, nConfig) {
@@ -125,12 +120,12 @@ func convertRepSpecs(resourceb *hclwrite.Body, diskSizeGB hclwrite.Tokens) error
125120
}
126121

127122
func convertRepSpecsWithDynamicBlock(resourceb *hclwrite.Body, diskSizeGB hclwrite.Tokens) (dynamicBlock, error) {
128-
dSpec, err := getDynamicBlock(resourceb, nRepSpecs)
123+
dSpec, err := getDynamicBlock(resourceb, nRepSpecs, true)
129124
if err != nil || !dSpec.IsPresent() {
130125
return dynamicBlock{}, err
131126
}
132127
transformReferences(dSpec.content.Body(), nRepSpecs, nSpec)
133-
dConfig, err := convertConfigsWithDynamicBlock(dSpec.content.Body(), diskSizeGB)
128+
dConfig, err := convertConfigsWithDynamicBlock(dSpec.content.Body(), diskSizeGB, true)
134129
if err != nil {
135130
return dynamicBlock{}, err
136131
}
@@ -139,9 +134,10 @@ func convertRepSpecsWithDynamicBlock(resourceb *hclwrite.Body, diskSizeGB hclwri
139134
return dSpec, nil
140135
}
141136

142-
func convertConfigsWithDynamicBlock(specbSrc *hclwrite.Body, diskSizeGB hclwrite.Tokens) (dynamicBlock, error) {
143-
d, err := getDynamicBlock(specbSrc, nConfig)
144-
if err != nil {
137+
func convertConfigsWithDynamicBlock(specbSrc *hclwrite.Body, diskSizeGB hclwrite.Tokens,
138+
insideDynamicRepSpec bool) (dynamicBlock, error) {
139+
d, err := getDynamicBlock(specbSrc, nConfig, true)
140+
if err != nil || !d.IsPresent() {
145141
return dynamicBlock{}, err
146142
}
147143
configBody := d.content.Body()
@@ -158,17 +154,25 @@ func convertConfigsWithDynamicBlock(specbSrc *hclwrite.Body, diskSizeGB hclwrite
158154
}
159155
regionConfigBody.SetAttributeRaw(blockType, hcl.TokensObject(blockBody))
160156
}
157+
forEach := hcl.GetAttrExpr(d.forEach)
158+
if insideDynamicRepSpec {
159+
forEach = fmt.Sprintf("%s.%s", nSpec, nConfig)
160+
}
161+
regionTokens := hcl.TokensFromExpr(buildForExpr(nRegion, forEach, false))
162+
regionTokens = append(regionTokens, hcl.TokensObject(regionConfigBody)...)
163+
if !insideDynamicRepSpec {
164+
d.tokens = hcl.EncloseBracketsNewLines(regionTokens)
165+
return d, nil
166+
}
161167
repSpecb := hclwrite.NewEmptyFile().Body()
162168
if zoneNameAttr := specbSrc.GetAttribute(nZoneName); zoneNameAttr != nil {
163-
repSpecb.SetAttributeRaw(nZoneName, hcl.TokensFromExpr(
164-
transformReference(hcl.GetAttrExpr(zoneNameAttr), nRepSpecs, nSpec)))
169+
zoneNameExpr := transformReference(hcl.GetAttrExpr(zoneNameAttr), nRepSpecs, nSpec)
170+
repSpecb.SetAttributeRaw(nZoneName, hcl.TokensFromExpr(zoneNameExpr))
165171
}
166-
regionTokens := hcl.TokensFromExpr(buildForExpr(nRegion, fmt.Sprintf("%s.%s", nSpec, nConfig), false))
167-
regionTokens = append(regionTokens, hcl.TokensObject(regionConfigBody)...)
168172
repSpecb.SetAttributeRaw(nConfig, hcl.EncloseBracketsNewLines(regionTokens))
169173
if numShardsAttr := specbSrc.GetAttribute(nNumShards); numShardsAttr != nil {
170-
tokens := hcl.TokensFromExpr(buildForExpr("i",
171-
fmt.Sprintf("range(%s)", transformReference(hcl.GetAttrExpr(numShardsAttr), nRepSpecs, nSpec)), false))
174+
numShardsExpr := transformReference(hcl.GetAttrExpr(numShardsAttr), nRepSpecs, nSpec)
175+
tokens := hcl.TokensFromExpr(buildForExpr("i", fmt.Sprintf("range(%s)", numShardsExpr), false))
172176
tokens = append(tokens, hcl.TokensObject(repSpecb)...)
173177
return dynamicBlock{tokens: hcl.EncloseBracketsNewLines(tokens)}, nil
174178
}

‎internal/convert/clu2adv.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func convertResource(block *hclwrite.Block) (bool, error) {
7575
}
7676

7777
func isFreeTierCluster(resourceb *hclwrite.Body) bool {
78-
d, _ := getDynamicBlock(resourceb, nRepSpecs)
78+
d, _ := getDynamicBlock(resourceb, nRepSpecs, true)
7979
return resourceb.FirstMatchingBlock(nRepSpecs, nil) == nil && !d.IsPresent()
8080
}
8181

@@ -221,7 +221,7 @@ func fillRepSpecs(resourceb *hclwrite.Body, root attrVals) error {
221221

222222
// fillRepSpecsWithDynamicBlock used for dynamic blocks in replication_specs
223223
func fillRepSpecsWithDynamicBlock(resourceb *hclwrite.Body, root attrVals) (dynamicBlock, error) {
224-
dSpec, err := getDynamicBlock(resourceb, nRepSpecs)
224+
dSpec, err := getDynamicBlock(resourceb, nRepSpecs, true)
225225
if err != nil || !dSpec.IsPresent() {
226226
return dynamicBlock{}, err
227227
}
@@ -239,7 +239,7 @@ func fillRepSpecsWithDynamicBlock(resourceb *hclwrite.Body, root attrVals) (dyna
239239

240240
// fillConfigsWithDynamicRegion is used for dynamic blocks in region_configs
241241
func fillConfigsWithDynamicRegion(specbSrc *hclwrite.Body, root attrVals, changeReferences bool) (dynamicBlock, error) {
242-
d, err := getDynamicBlock(specbSrc, nConfigSrc)
242+
d, err := getDynamicBlock(specbSrc, nConfigSrc, true)
243243
if err != nil || !d.IsPresent() {
244244
return dynamicBlock{}, err
245245
}

‎internal/convert/convert_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ func runConvertTests(t *testing.T, cmdName string, convert func(testName string,
2727
require.NoError(t, err)
2828
err = json.Unmarshal(errContent, &errMap)
2929
require.NoError(t, err)
30+
unusedErrors := make(map[string]struct{})
31+
for name := range errMap {
32+
unusedErrors[name] = struct{}{}
33+
}
34+
3035
g := goldie.New(t,
3136
goldie.WithFixtureDir(root),
3237
goldie.WithNameSuffix(outSuffix))
@@ -46,7 +51,9 @@ func runConvertTests(t *testing.T, cmdName string, convert func(testName string,
4651
errMsg, found := errMap[testName]
4752
assert.True(t, found, "error not found in file %s for test %s, errMsg: %v", errFilename, testName, err)
4853
assert.Contains(t, err.Error(), errMsg)
54+
delete(unusedErrors, testName)
4955
}
5056
})
5157
}
58+
assert.Empty(t, unusedErrors, "some errors are not being used")
5259
}

‎internal/convert/shared.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package convert
22

33
import (
4+
"errors"
45
"fmt"
56
"slices"
67
"strings"
@@ -9,6 +10,10 @@ import (
910
"github.com/mongodb-labs/atlas-cli-plugin-terraform/internal/hcl"
1011
)
1112

13+
var (
14+
errDynamicBlockAlone = errors.New("dynamic block must be the only block, see docs for more information")
15+
)
16+
1217
// hasVariableNumShards checks if any block has a variable (non-literal) num_shards attribute
1318
func hasVariableNumShards(blocks []*hclwrite.Block) bool {
1419
for _, block := range blocks {
@@ -51,8 +56,13 @@ func (d dynamicBlock) IsPresent() bool {
5156
}
5257

5358
// getDynamicBlock finds and returns a dynamic block with the given name from the body
54-
func getDynamicBlock(body *hclwrite.Body, name string) (dynamicBlock, error) {
59+
func getDynamicBlock(body *hclwrite.Body, name string, checkAlone bool) (dynamicBlock, error) {
60+
var db dynamicBlock
61+
staticBlockCount := 0
5562
for _, block := range body.Blocks() {
63+
if block.Type() == name {
64+
staticBlockCount++
65+
}
5666
if block.Type() != nDynamic || name != getResourceName(block) {
5767
continue
5868
}
@@ -65,9 +75,14 @@ func getDynamicBlock(body *hclwrite.Body, name string) (dynamicBlock, error) {
6575
if content == nil {
6676
return dynamicBlock{}, fmt.Errorf("dynamic block %s: block %s not found", name, nContent)
6777
}
68-
return dynamicBlock{forEach: forEach, block: block, content: content}, nil
78+
if !db.IsPresent() {
79+
db = dynamicBlock{forEach: forEach, block: block, content: content}
80+
}
81+
}
82+
if checkAlone && db.IsPresent() && staticBlockCount > 0 {
83+
return dynamicBlock{}, errDynamicBlockAlone
6984
}
70-
return dynamicBlock{}, nil
85+
return db, nil
7186
}
7287

7388
func checkDynamicBlock(body *hclwrite.Body) error {
@@ -180,7 +195,7 @@ func fillTagsLabelsOpt(resourceb *hclwrite.Body, name string) error {
180195
}
181196

182197
func extractTagsLabelsDynamicBlock(resourceb *hclwrite.Body, name string) (hclwrite.Tokens, error) {
183-
d, err := getDynamicBlock(resourceb, name)
198+
d, err := getDynamicBlock(resourceb, name, false)
184199
if err != nil || !d.IsPresent() {
185200
return nil, err
186201
}

‎internal/convert/testdata/adv2v2/dynamic_regions_config_invalid_multiple_blocks.in.tf

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
resource "mongodbatlas_advanced_cluster" "multiple_blocks" {
2+
project_id = var.project_id
3+
name = var.cluster_name
4+
cluster_type = var.cluster_type
5+
replication_specs {
6+
num_shards = var.replication_specs.num_shards
7+
dynamic "region_configs" {
8+
for_each = var.replication_specs.region_configs
9+
content {
10+
priority = region_configs.value.priority
11+
provider_name = region_configs.value.provider_name
12+
region_name = region_configs.value.region_name
13+
electable_specs {
14+
instance_size = region_configs.value.instance_size
15+
node_count = region_configs.value.electable_node_count
16+
}
17+
}
18+
}
19+
region_configs { # inline block is not allowed with dynamic blocks
20+
priority = 0
21+
provider_name = "AWS"
22+
region_name = "US_EAST_1"
23+
read_only_specs {
24+
instance_size = var.instance_size
25+
node_count = 1
26+
}
27+
}
28+
}
29+
}

‎internal/convert/testdata/adv2v2/dynamic_replication_specs_invalid_multiple_blocks.in.tf

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
resource "mongodbatlas_advanced_cluster" "multiple_blocks" {
2+
project_id = var.project_id
3+
name = var.cluster_name
4+
cluster_type = var.cluster_type
5+
dynamic "replication_specs" {
6+
for_each = var.replication_specs
7+
content {
8+
num_shards = replication_specs.value.num_shards
9+
dynamic "region_configs" {
10+
for_each = replication_specs.value.region_configs
11+
content {
12+
priority = region_configs.value.priority
13+
provider_name = region_configs.value.provider_name
14+
region_name = region_configs.value.region_name
15+
electable_specs {
16+
instance_size = region_configs.value.instance_size
17+
node_count = region_configs.value.electable_node_count
18+
}
19+
}
20+
}
21+
}
22+
}
23+
replication_specs { # inline block is not allowed with dynamic blocks
24+
region_configs {
25+
priority = 7
26+
provider_name = "AWS"
27+
region_name = "EU_WEST_1"
28+
electable_specs {
29+
instance_size = "M10"
30+
node_count = 3
31+
}
32+
}
33+
}
34+
}

‎internal/convert/testdata/adv2v2/dynamic_replication_specs_invalid_multiple_config_blocks.in.tf

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
resource "mongodbatlas_advanced_cluster" "multiple_blocks" {
2+
project_id = var.project_id
3+
name = var.cluster_name
4+
cluster_type = var.cluster_type
5+
dynamic "replication_specs" {
6+
for_each = var.replication_specs
7+
content {
8+
num_shards = replication_specs.value.num_shards
9+
dynamic "region_configs" {
10+
for_each = replication_specs.value.region_configs
11+
content {
12+
priority = region_configs.value.priority
13+
provider_name = region_configs.value.provider_name
14+
region_name = region_configs.value.region_name
15+
electable_specs {
16+
instance_size = region_configs.value.instance_size
17+
node_count = region_configs.value.electable_node_count
18+
}
19+
}
20+
}
21+
region_configs { # inline block is not allowed with dynamic blocks
22+
priority = 0
23+
provider_name = "AWS"
24+
region_name = "US_EAST_1"
25+
read_only_specs {
26+
instance_size = var.instance_size
27+
node_count = 1
28+
}
29+
}
30+
}
31+
}
32+
}

‎internal/convert/testdata/adv2v2/errors.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,8 @@
22
"configuration_file_error": "failed to parse Terraform config file",
33
"replication_specs_missing_region_configs": "replication_specs must have at least one region_configs",
44
"missing_replication_specs": "must have at least one replication_specs",
5-
"dynamic_unsupported_tag": "dynamic blocks are not supported for advanced_configuration"
5+
"dynamic_unsupported_tag": "dynamic blocks are not supported for advanced_configuration",
6+
"dynamic_regions_config_invalid_multiple_blocks": "dynamic block must be the only block",
7+
"dynamic_replication_specs_invalid_multiple_blocks": "dynamic block must be the only block",
8+
"dynamic_replication_specs_invalid_multiple_config_blocks": "dynamic block must be the only block"
69
}

‎internal/convert/testdata/clu2adv/dynamic_regions_config_invalid_multiple_blocks.in.tf

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
resource "mongodbatlas_cluster" "multiple_blocks" {
2+
project_id = var.project_id
3+
name = "cluster"
4+
cluster_type = "SHARDED"
5+
provider_name = "AWS"
6+
provider_instance_size_name = "M10"
7+
replication_specs {
8+
num_shards = var.replication_specs.num_shards
9+
dynamic "regions_config" {
10+
for_each = var.replication_specs.regions_config
11+
content {
12+
region_name = regions_config.value.region_name
13+
electable_nodes = regions_config.value.electable_nodes
14+
priority = regions_config.value.prio
15+
read_only_nodes = regions_config.value.read_only_nodes
16+
}
17+
}
18+
regions_config { # inline block is not allowed with dynamic blocks
19+
region_name = "US_EAST_1"
20+
read_only_nodes = 1
21+
}
22+
}
23+
}

‎internal/convert/testdata/clu2adv/dynamic_replication_specs_invalid_multiple_blocks.in.tf

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Based on https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/examples/migrate_cluster_to_advanced_cluster/module_maintainer/v1/main.tf
2+
resource "mongodbatlas_cluster" "this" {
3+
project_id = var.project_id
4+
name = var.cluster_name
5+
auto_scaling_disk_gb_enabled = var.auto_scaling_disk_gb_enabled
6+
cluster_type = var.cluster_type
7+
disk_size_gb = var.disk_size
8+
mongo_db_major_version = var.mongo_db_major_version
9+
provider_instance_size_name = var.instance_size
10+
provider_name = var.provider_name
11+
dynamic "replication_specs" {
12+
for_each = var.replication_specs
13+
content {
14+
num_shards = replication_specs.value.num_shards
15+
zone_name = replication_specs.value.zone_name
16+
dynamic "regions_config" {
17+
for_each = replication_specs.value.regions_config
18+
content {
19+
electable_nodes = regions_config.value.electable_nodes
20+
priority = regions_config.value.priority
21+
read_only_nodes = regions_config.value.read_only_nodes
22+
region_name = regions_config.value.region_name
23+
}
24+
}
25+
}
26+
}
27+
replication_specs { # inline block is not allowed with dynamic blocks
28+
num_shards = 1
29+
regions_config {
30+
region_name = "US_EAST_1"
31+
electable_nodes = 3
32+
priority = 7
33+
}
34+
}
35+
}

0 commit comments

Comments
 (0)