Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for dynamic blocks #1583

Merged
merged 2 commits into from
Nov 18, 2022
Merged

Add support for dynamic blocks #1583

merged 2 commits into from
Nov 18, 2022

Conversation

wata727
Copy link
Member

@wata727 wata727 commented Nov 5, 2022

Fixes #1576

This PR adds support for dynamic blocks. TFLint already has given special consideration to dynamic blocks, but it was imperfect: blocks were not expanded, and iterators were not evaluated.

resource "aws_instance" "main" {
  ebs_block_device {
    encrypted = false
  }

  dynamic "ebs_block_device" {
    for_each = toset([10, 20]) # => This should expand into two blocks, but only one is returned.
    content {
      encrypted   = false                  # => Only static values could be retrieved with previous versions.
      volume_size = ebs_block_device.value # => This value is always unknown.
    }
  }
}

Since this PR fully expands dynamic blocks, the above example expands to:

resource "aws_instance" "main" {
  ebs_block_device {
    encrypted = false
  }

  dynamic "ebs_block_device" {
    for_each = toset([10, 20]) # => Two "ebs_block_device" blocks are returned
    content {
      encrypted   = false                  # => Static values are still supported
      volume_size = ebs_block_device.value # => 10 and 20 are assigned in each block.
    }
  }
}

If the for_each is set to an unknown value, it behaves the same as when an empty object/set is assigned. This is the same behavior as expanding resources/modules by the count/for_each meta-arguments.

From a compatibility perspective, this change allows dynamic blocks to be expanded correctly, so in some cases, the block is not returned (e.g. empty object/set, unknown variables). In that case, plugin developers should use ExpandModeNone:

runner.GetResourceContent("aws_instance", &hclext.BodySchema{
    Blocks: []hclext.BlockSchema{
        {
            Type: "dynamic",
            Labels: []string{"type"},
            Body: &hclext.BodySchema{
                Blocks: []hclext.BlockSchema{
                    {
                        Type: "content",
                        Body: &hclext.BodySchema{
                            Attributes: []hclext.AttributeSchema{{Name: "encrypted"}, {Name: "volume_size"}}
                        },
                    },
                },
            },
        },
    },
}, &tflint.GetModuleContentOption{ExpandMode: tflint.ExpandModeNone})

Previously, differences in dynamic block schema were flattened even when the ExpandModeNone was set, but after this change, the actual dynamic block schema must be set.

Implementation

Dynamic block expansion support uses the hcl/ext/dynblock like Terraform. This is an implementation of hcl.Body that internally duplicates blocks when returning content and wraps expressions with iterator evaluation contexts.

However, there were the following problems when adopting it in TFLint:

  • If the for_each is unknown, "unknown body" is returned.
    • This is not suitable for static analysis requirements as all expressions inside the unknown body are unknown (including static values).
  • If count.* and each.* is included inside the dynamic block, it cannot be evaluated properly.
    • Since the current implementation evaluates count.* and each.* when expanding the hclext.BodyContent obtained by PartialContent, it is not possible to include these values in the EvalContext when expanding the dynamic block.
  • Iterator evaluation result cannot be bound to expression.
    • Due to wire protocol support, expressions that depend on anything other than the global EvalContext must have their results bound beforehand. However, dynblock.exprWrap is private, so we cannot efficiently bind the expression.

For the reasons above, TFLint introduces the terraform/tfhcl package as a fork of hcl/ext/dynblock.

Also, for the support of count.* and each.* inside dynamic blocks, expansion by meta-arguments is now the responsibility of the terraform/tfhcl package. It internally duplicates and returns resources and modules, as an implementation of hcl.Body, just like a dynamic block. Since the "resource" block and the "module" block is Terraform language spec, they are implemented as the terraform/tfhcl package, not the hclext package.

ToDo

  • Add support for each.* and count.* in dynamic blocks
  • Tweak default behavior and consider how to get a static "dynamic" block
  • Improve handling unknown dynamic blocks
  • Handle unknown dynamic labels
  • Handle sensitive values
  • Add tests
  • Revise GoDoc
  • Revise documentation

@wata727 wata727 force-pushed the support_dynblock branch 3 times, most recently from 33ffc19 to b0072fd Compare November 12, 2022 13:38
@wata727 wata727 force-pushed the support_dynblock branch 2 times, most recently from b2844e0 to dea598b Compare November 12, 2022 18:36
@wata727 wata727 marked this pull request as ready for review November 13, 2022 08:30
@wata727 wata727 merged commit 3588e3c into master Nov 18, 2022
@wata727 wata727 deleted the support_dynblock branch November 18, 2022 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic block ignores block schema
1 participant