feat: Supports dynamic blocks in regions_config #33
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,7 +57,8 @@ Given the different ways of using dynamic blocks, we recommend reviewing the out | |
|
|
||
| #### Dynamic blocks in tags and labels | ||
|
|
||
| You can use `dynamic` blocks for `tags` and `labels`. The plugin assumes that `for_each` has an expression which is evaluated to a `map` of strings. | ||
| You can also combine the use of dynamic blocks in `tags` and `labels` with individual blocks in the same cluster definition, e.g.: | ||
| ```hcl | ||
| tags { | ||
| key = "environment" | ||
|
|
@@ -72,12 +73,34 @@ dynamic "tags" { | |
| } | ||
| ``` | ||
|
|
||
| #### Dynamic blocks in regions_config | ||
|
|
||
| You can use `dynamic` blocks for `regions_config`. The plugin assumes that `for_each` has an expression which is evaluated to a `list` or `set` of objects. | ||
| Dynamic block and individual blocks for `regions_config` are not supported at the same time in a `replication_specs`. This is an example of how to use dynamic blocks in `regions_config`: | ||
| ```hcl | ||
| replication_specs { | ||
| num_shards = var.replication_specs.num_shards | ||
| zone_name = var.replication_specs.zone_name # only needed if you're using zones | ||
| dynamic "regions_config" { | ||
| for_each = var.replication_specs.regions_config | ||
| content { | ||
| priority = regions_config.value.priority | ||
| region_name = regions_config.value.region_name | ||
| electable_nodes = regions_config.value.electable_nodes | ||
| read_only_nodes = regions_config.value.read_only_nodes | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Limitations | ||
|
|
||
| - [`num_shards`](https://registry.terraform.io/providers/mongodb/mongodbatlas/latest/docs/resources/cluster#num_shards-2) in `replication_specs` must be a numeric [literal expression](https://developer.hashicorp.com/nomad/docs/job-specification/hcl2/expressions#literal-expressions), e.g. `var.num_shards` is not supported. This is to allow creating a `replication_specs` element per shard in `mongodbatlas_advanced_cluster`. This limitation doesn't apply if you're using `dynamic` blocks in `regions_config` or `replication_specs`. | ||
|
There was a problem hiding this comment. so great to see our limitations going away. Great stuff @lantoli There was a problem hiding this comment.
I am not fully clear why we can support this case but not when is a regular literal replication_specs block. There was a problem hiding this comment. because in the literal case we physically replicate the block num_shard times so we avoid to introduce for loops and the adv_cluster is straighthforward. However in the module case as we need to create the loops and introduce some complexity, it's ok to also iterate through the priorities. We could potentially support that case if some customers are interested |
||
| - `dynamic` blocks are currently supported only for `tags`, `labels` and `regions_config`. **Coming soon**: support for `replication_specs`. | ||
|
|
||
| ## Feedback | ||
|
|
||
| If you find any issues or have any suggestions, please open an [issue](https://github.com/mongodb-labs/atlas-cli-plugin-terraform/issues) in this repository. | ||
|
|
||
| ## Contributing | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,4 +54,5 @@ const ( | |
| nDynamic = "dynamic" | ||
| nForEach = "for_each" | ||
| nContent = "content" | ||
| nRegion = "region" | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,22 +22,21 @@ const ( | |
| advClusterPlural = "mongodbatlas_advanced_clusters" | ||
| valClusterType = "REPLICASET" | ||
| valMaxPriority = 7 | ||
| valMinPriority = 0 | ||
| errFreeCluster = "free cluster (because no " + nRepSpecs + ")" | ||
| errRepSpecs = "setting " + nRepSpecs | ||
| errConfigs = "setting " + nConfig | ||
| errPriority = "setting " + nPriority | ||
| errNumShards = "setting " + nNumShards | ||
|
|
||
| commentGeneratedBy = "Generated by atlas-cli-plugin-terraform." | ||
| commentConfirmReferences = "Please review the changes and confirm that references to this resource are updated." | ||
| commentMovedBlock = "Moved blocks" | ||
| commentRemovedOld = "Note: Remember to remove or comment out the old cluster definitions." | ||
| ) | ||
|
|
||
| var ( | ||
| dynamicBlockAllowList = []string{nTags, nLabels, nConfigSrc} | ||
| ) | ||
|
|
||
| type attrVals struct { | ||
|
|
@@ -129,8 +128,8 @@ func fillMovedBlocks(body *hclwrite.Body, moveLabels []string) { | |
| for i, moveLabel := range moveLabels { | ||
| block := body.AppendNewBlock(nMoved, nil) | ||
| blockb := block.Body() | ||
| blockb.SetAttributeRaw(nFrom, hcl.TokensFromExpr(fmt.Sprintf("%s.%s", cluster, moveLabel))) | ||
| blockb.SetAttributeRaw(nTo, hcl.TokensFromExpr(fmt.Sprintf("%s.%s", advCluster, moveLabel))) | ||
| if i < len(moveLabels)-1 { | ||
| body.AppendNewline() | ||
| } | ||
|
|
@@ -202,9 +201,15 @@ func fillReplicationSpecs(resourceb *hclwrite.Body, root attrVals) error { | |
| break | ||
| } | ||
| specbSrc := specSrc.Body() | ||
| d, err := fillRegionConfigsDynamicBlock(specbSrc, root) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if d.IsPresent() { | ||
| resourceb.RemoveBlock(specSrc) | ||
| resourceb.SetAttributeRaw(nRepSpecs, d.tokens) | ||
| return nil | ||
| } | ||
| // ok to fail as zone_name is optional | ||
| _ = hcl.MoveAttr(specbSrc, specb, nZoneName, nZoneName, errRepSpecs) | ||
| shards := specbSrc.GetAttribute(nNumShards) | ||
|
|
@@ -251,7 +256,7 @@ func fillTagsLabelsOpt(resourceb *hclwrite.Body, name string) error { | |
|
|
||
| func extractTagsLabelsDynamicBlock(resourceb *hclwrite.Body, name string) (hclwrite.Tokens, error) { | ||
| d, err := getDynamicBlock(resourceb, name) | ||
| if err != nil || !d.IsPresent() { | ||
| return nil, err | ||
| } | ||
| key := d.content.Body().GetAttribute(nKey) | ||
|
|
@@ -306,14 +311,43 @@ func fillBlockOpt(resourceb *hclwrite.Body, name string) { | |
| resourceb.SetAttributeRaw(name, hcl.TokensObject(block.Body())) | ||
| } | ||
|
|
||
| // fillRegionConfigsDynamicBlock is used for dynamic blocks in region_configs | ||
| func fillRegionConfigsDynamicBlock(specbSrc *hclwrite.Body, root attrVals) (dynamicBlock, error) { | ||
| d, err := getDynamicBlock(specbSrc, nConfigSrc) | ||
| if err != nil || !d.IsPresent() { | ||
| return dynamicBlock{}, err | ||
| } | ||
| repSpec := hclwrite.NewEmptyFile() | ||
| repSpecb := repSpec.Body() | ||
| if zoneName := hcl.GetAttrExpr(specbSrc.GetAttribute(nZoneName)); zoneName != "" { | ||
EspenAlbert marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| repSpecb.SetAttributeRaw(nZoneName, hcl.TokensFromExpr(zoneName)) | ||
| } | ||
| regionFor, err := getDynamicBlockRegionConfigsRegionArray(d, root) | ||
| if err != nil { | ||
| return dynamicBlock{}, err | ||
| } | ||
| priorityFor := hcl.TokensFromExpr(fmt.Sprintf("for %s in range(%d, %d, -1) : ", nPriority, valMaxPriority, valMinPriority)) | ||
| priorityFor = append(priorityFor, regionFor...) | ||
| repSpecb.SetAttributeRaw(nConfig, hcl.TokensFuncFlatten(priorityFor)) | ||
|
|
||
| shards := specbSrc.GetAttribute(nNumShards) | ||
| if shards == nil { | ||
| return dynamicBlock{}, fmt.Errorf("%s: %s not found", errRepSpecs, nNumShards) | ||
EspenAlbert marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| tokens := hcl.TokensFromExpr(fmt.Sprintf("for i in range(%s) :", hcl.GetAttrExpr(shards))) | ||
| tokens = append(tokens, hcl.EncloseBraces(repSpec.BuildTokens(nil), true)...) | ||
| d.tokens = hcl.EncloseBracketsNewLines(tokens) | ||
| return d, nil | ||
| } | ||
|
|
||
| func fillRegionConfigs(specb, specbSrc *hclwrite.Body, root attrVals) error { | ||
| var configs []*hclwrite.Body | ||
| for { | ||
| configSrc := specbSrc.FirstMatchingBlock(nConfigSrc, nil) | ||
| if configSrc == nil { | ||
| break | ||
| } | ||
| config, err := getRegionConfig(configSrc, root, false) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -323,34 +357,28 @@ func fillRegionConfigs(specb, specbSrc *hclwrite.Body, root attrVals) error { | |
| if len(configs) == 0 { | ||
| return fmt.Errorf("%s: %s not found", errRepSpecs, nConfigSrc) | ||
| } | ||
| configs = sortConfigsByPriority(configs) | ||
| specb.SetAttributeRaw(nConfig, hcl.TokensArray(configs)) | ||
| return nil | ||
| } | ||
|
|
||
| func getRegionConfig(configSrc *hclwrite.Block, root attrVals, isDynamicBlock bool) (*hclwrite.File, error) { | ||
| file := hclwrite.NewEmptyFile() | ||
| fileb := file.Body() | ||
| fileb.SetAttributeRaw(nProviderName, root.req[nProviderName]) | ||
| if err := hcl.MoveAttr(configSrc.Body(), fileb, nRegionName, nRegionName, errRepSpecs); err != nil { | ||
| return nil, err | ||
| } | ||
| if err := hcl.MoveAttr(configSrc.Body(), fileb, nPriority, nPriority, errRepSpecs); err != nil { | ||
| return nil, err | ||
| } | ||
| if electable, _ := getSpecs(configSrc, nElectableNodes, root, isDynamicBlock); electable != nil { | ||
| fileb.SetAttributeRaw(nElectableSpecs, electable) | ||
| } | ||
| if readOnly, _ := getSpecs(configSrc, nReadOnlyNodes, root, isDynamicBlock); readOnly != nil { | ||
| fileb.SetAttributeRaw(nReadOnlySpecs, readOnly) | ||
| } | ||
| if analytics, _ := getSpecs(configSrc, nAnalyticsNodes, root, isDynamicBlock); analytics != nil { | ||
| fileb.SetAttributeRaw(nAnalyticsSpecs, analytics) | ||
| } | ||
| if autoScaling := getAutoScalingOpt(root.opt); autoScaling != nil { | ||
|
|
@@ -359,7 +387,7 @@ func getRegionConfig(configSrc *hclwrite.Block, root attrVals) (*hclwrite.File, | |
| return file, nil | ||
| } | ||
|
|
||
| func getSpecs(configSrc *hclwrite.Block, countName string, root attrVals, isDynamicBlock bool) (hclwrite.Tokens, error) { | ||
| var ( | ||
| file = hclwrite.NewEmptyFile() | ||
| fileb = file.Body() | ||
|
|
@@ -382,7 +410,11 @@ func getSpecs(configSrc *hclwrite.Block, countName string, root attrVals) (hclwr | |
| if root.opt[nDiskIOPSSrc] != nil { | ||
| fileb.SetAttributeRaw(nDiskIOPS, root.opt[nDiskIOPSSrc]) | ||
| } | ||
| tokens := hcl.TokensObject(fileb) | ||
| if isDynamicBlock { | ||
| tokens = encloseDynamicBlockRegionSpec(tokens, countName) | ||
| } | ||
| return tokens, nil | ||
| } | ||
|
|
||
| func getAutoScalingOpt(opt map[string]hclwrite.Tokens) hclwrite.Tokens { | ||
|
|
@@ -440,6 +472,17 @@ func getResourceLabel(resource *hclwrite.Block) string { | |
| return labels[1] | ||
| } | ||
|
|
||
| type dynamicBlock struct { | ||
| block *hclwrite.Block | ||
| forEach *hclwrite.Attribute | ||
| content *hclwrite.Block | ||
| tokens hclwrite.Tokens | ||
| } | ||
|
|
||
| func (d dynamicBlock) IsPresent() bool { | ||
| return d.block != nil | ||
| } | ||
|
|
||
| func checkDynamicBlock(body *hclwrite.Body) error { | ||
| for _, block := range body.Blocks() { | ||
| name := getResourceName(block) | ||
|
|
@@ -451,12 +494,6 @@ func checkDynamicBlock(body *hclwrite.Body) error { | |
| return nil | ||
| } | ||
|
|
||
| func getDynamicBlock(body *hclwrite.Body, name string) (dynamicBlock, error) { | ||
| for _, block := range body.Blocks() { | ||
| if block.Type() != nDynamic || name != getResourceName(block) { | ||
|
|
@@ -481,6 +518,55 @@ func replaceDynamicBlockExpr(attr *hclwrite.Attribute, blockName, attrName strin | |
| return strings.ReplaceAll(expr, fmt.Sprintf("%s.%s", blockName, attrName), attrName) | ||
| } | ||
|
|
||
| func encloseDynamicBlockRegionSpec(specTokens hclwrite.Tokens, countName string) hclwrite.Tokens { | ||
| tokens := hcl.TokensFromExpr(fmt.Sprintf("%s.%s > 0 ?", nRegion, countName)) | ||
| tokens = append(tokens, specTokens...) | ||
| return append(tokens, hcl.TokensFromExpr(": null")...) | ||
| } | ||
|
|
||
| // getDynamicBlockRegionConfigsRegionArray returns the region array for a dynamic block in replication_specs. | ||
| // e.g. [ for region in var.replication_specs.regions_config : { ... } if priority == region.priority ] | ||
| func getDynamicBlockRegionConfigsRegionArray(d dynamicBlock, root attrVals) (hclwrite.Tokens, error) { | ||
| transformDynamicBlockReferences(d.content.Body()) | ||
| priorityStr := hcl.GetAttrExpr(d.content.Body().GetAttribute(nPriority)) | ||
| if priorityStr == "" { | ||
| return nil, fmt.Errorf("%s: %s not found", errRepSpecs, nPriority) | ||
| } | ||
| region, err := getRegionConfig(d.content, root, true) | ||
|
There was a problem hiding this comment. @AgustinBettati FYI region config object creation is reused in individual and dynamic block |
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| tokens := hcl.TokensFromExpr(fmt.Sprintf("for %s in %s :", nRegion, hcl.GetAttrExpr(d.forEach))) | ||
| tokens = append(tokens, hcl.EncloseBraces(region.BuildTokens(nil), true)...) | ||
| tokens = append(tokens, hcl.TokensFromExpr(fmt.Sprintf("if %s == %s", nPriority, priorityStr))...) | ||
| return hcl.EncloseBracketsNewLines(tokens), nil | ||
| } | ||
|
|
||
| // transformDynamicBlockReferences changes value references in all attributes, e.g. regions_config.value.electable_nodes to region.electable_nodes | ||
| func transformDynamicBlockReferences(configSrcb *hclwrite.Body) { | ||
| for name, attr := range configSrcb.Attributes() { | ||
| expr := hcl.GetAttrExpr(attr) | ||
| expr = strings.ReplaceAll(expr, | ||
| fmt.Sprintf("%s.%s.", nConfigSrc, nValue), | ||
| fmt.Sprintf("%s.", nRegion)) | ||
| configSrcb.SetAttributeRaw(name, hcl.TokensFromExpr(expr)) | ||
| } | ||
| } | ||
|
|
||
| func sortConfigsByPriority(configs []*hclwrite.Body) []*hclwrite.Body { | ||
| for _, config := range configs { | ||
| if _, err := hcl.GetAttrInt(config.GetAttribute(nPriority), errPriority); err != nil { | ||
| return configs // don't sort priorities if any is not a numerical literal | ||
| } | ||
| } | ||
| sort.Slice(configs, func(i, j int) bool { | ||
| pi, _ := hcl.GetAttrInt(configs[i].GetAttribute(nPriority), errPriority) | ||
| pj, _ := hcl.GetAttrInt(configs[j].GetAttribute(nPriority), errPriority) | ||
| return pi > pj | ||
| }) | ||
| return configs | ||
| } | ||
|
|
||
| func setKeyValue(body *hclwrite.Body, key, value *hclwrite.Attribute) { | ||
| keyStr, err := hcl.GetAttrString(key, "") | ||
| if err == nil { | ||
|
|
@@ -494,21 +580,6 @@ func setKeyValue(body *hclwrite.Body, key, value *hclwrite.Attribute) { | |
| body.SetAttributeRaw(keyStr, value.Expr().BuildTokens(nil)) | ||
| } | ||
|
|
||
| // popRootAttrs deletes the attributes common to all replication_specs/regions_config and returns them. | ||
| func popRootAttrs(body *hclwrite.Body) (attrVals, error) { | ||
| var ( | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a comment in the previous commit regarding this so they can give feedback if some customer needs it. For example we support it in tags and label as some customers are using it, e.g:
atlas-cli-plugin-terraform/internal/convert/testdata/clu2adv/dynamic_tags_labels.out.tf
Line 142 in e9c2134
mergefunction like in tags.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point I was trying to get is: customers will come at us with questions. For limitations we are aware, it's useful to have clear instructions on "why" the limitation and "what is the alternative". (this internal thread should teach us).
Said that, it looks like the "why" is more on "we are not investing on it", which is fine. Can we at least have a "what's the alternative" clearly written in the docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added here: 2bec1f9