hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
41.83k stars 9.44k forks source link

Unable to add allow or block empty block dynamically based on variable value #26701

Open fazith27 opened 3 years ago

fazith27 commented 3 years ago

Current Terraform Version

Terraform v0.13.2

Use-cases

Hi Team, I am using terraform to manage WAF v2 for AWS. I am following the document https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl to create the wafv2. I am looking to dynamically create the default_action for WebACL based on the variable value. If the variable value for default_action is allow, I need to create WebACL with default_action as allow {} and for the value block default action should be block {}. I have searched the terraform docs and couldn't see an option to have a if-else loop to achieve this. I doubt the feature is available so it might be a feature request on how dynamically add a block based on variable value.

I have given a sample code

variable "default_action" {
    type="string"
    default="allow"
}

resource "aws_wafv2_web_acl" "web_acl_for_cloudfront" {
    name        = var.waf_acl_name
    description = var.waf_acl_description
    scope       = var.waf_acl_scope  
    default_action {
        allow {} # this piece of code should be dynamically updated during terraform plan and apply based on default_action variable
    }
}

Attempted Solutions

Tried dynamic block with two variables one for block and another one for allow, but no luck. Got the error Attribute supports 1 item maximum, config has 2 declared

variable "default_action_allow" {
    type="list"
    default="[]"
}

variable "default_action_block" {
    type="list"
    default="[true]"
}

resource "aws_wafv2_web_acl" "web_acl_for_cloudfront" {
    name        = var.waf_acl.name
    description = var.waf_acl.description
    scope       = var.waf_acl.scope
    dynamic "default_action" {
    for_each = var.default_action_allow
    content {
      allow{}
    }
  }
    dynamic "default_action" {
    for_each = var.default_action_block
    content {
      block{}
    }
  }
}

Proposal

This can be kept in the same way like aws_waf_web_acl. Reference https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/waf_web_acl.

default_action {
    type = "ALLOW"
  }

References

jbardin commented 3 years ago

Hi @fazith27

Thanks for filing the issue. Your attempted solution is likely the closest we can come to supporting this, but there seems to be some inconsistency when terraform is handling a dynamic block with no values and the block itself has no attributes set.

The schema the provider has chosen for this option is quite unusual, and unfortunately that is going to make conditionally setting the option awkward in the config. I think it may be possible to get this working with the dynamic block construct, but it's going to take some investigation to see what is going wrong, as empty blocks are not typically used as values in terraform.

fazith27 commented 3 years ago

Hi @fazith27

Thanks for filing the issue. Your attempted solution is likely the closest we can come to supporting this, but there seems to be some inconsistency when terraform is handling a dynamic block with no values and the block itself has no attributes set.

The schema the provider has chosen for this option is quite unusual, and unfortunately that is going to make conditionally setting the option awkward in the config. I think it may be possible to get this working with the dynamic block construct, but it's going to take some investigation to see what is going wrong, as empty blocks are not typically used as values in terraform.

Thanks @jbardin for your response. I will be trying different options with dynamic block at my end and will update here if I found any options. If you come with the working piece of code before me, please keep me posted.

jbardin commented 3 years ago

Updating with the current info from the initial investigation. The failure here is occurring during validation, where a configuration containing both blocks is sent to the provider which fails the provider's own validation of MaxItems: 1.

Dynamic block validation relies on the fact that unknown block values will contain unknown attributes. Since the provider took the unusual approach of using a block itself as the configuration value, there are no attributes in this configuration block to be unknown, hence there is no way for core to signal that these default_action blocks may be dynamic. It may be possible to somehow insert an unknown value for the blocks themselves during evaluation when there are no attributes, but that is quite different from how any evaluation currently works, as blocks themselves are never handled as unknown values.

apparentlymart commented 3 years ago

From looking at the schema of this resource, it looks like it might be possible to make this work by making the inner empty blocks be what is dynamic, rather than the outer default_action blocks. Although I expect it's illegal to have both an allow and a block block at the same time, enforcing that seems to be delegated to the remote API rather than handled in the provider schema itself, and so I think an error would be returned only if there are two blocks present at apply time, which would not be true in this case because var.default_action_allow and var.default_action_block should be known by the apply step:

resource "aws_wafv2_web_acl" "web_acl_for_cloudfront" {
  name        = var.waf_acl.name
  description = var.waf_acl.description
  scope       = var.waf_acl.scope

  default_action {
    dynamic "allow" {
      for_each = var.default_action_allow
      content {}
    }
    dynamic "deny" {
      for_each = var.default_action_deny
      content {}
    }
  }
}

Note that this is relying on an implementation detail of this resource type, so although I expect it will work today and thus hopefully allow you to make progress, there is some risk that a future version of the AWS provider may choose to validate at plan time that allow and deny are mutually exclusive, in which case this would start failing again.

A change to the provider like @jbardin was alluding to (declaring at least one argument in there so Terraform can mark it as unknown) would likely be the best way to make this work reliably for both cases, and avoid a future provider change raising an incorrect error for my newer example above too.

fazith27 commented 3 years ago

From looking at the schema of this resource, it looks like it might be possible to make this work by making the inner empty blocks be what is dynamic, rather than the outer default_action blocks. Although I expect it's illegal to have both an allow and a block block at the same time, enforcing that seems to be delegated to the remote API rather than handled in the provider schema itself, and so I think an error would be returned only if there are two blocks present at apply time, which would not be true in this case because var.default_action_allow and var.default_action_block should be known by the apply step:

resource "aws_wafv2_web_acl" "web_acl_for_cloudfront" {
  name        = var.waf_acl.name
  description = var.waf_acl.description
  scope       = var.waf_acl.scope

  default_action {
    dynamic "allow" {
      for_each = var.default_action_allow
      content {}
    }
    dynamic "deny" {
      for_each = var.default_action_deny
      content {}
    }
  }
}

Note that this is relying on an implementation detail of this resource type, so although I expect it will work today and thus hopefully allow you to make progress, there is some risk that a future version of the AWS provider may choose to validate at plan time that allow and deny are mutually exclusive, in which case this would start failing again.

A change to the provider like @jbardin was alluding to (declaring at least one argument in there so Terraform can mark it as unknown) would likely be the best way to make this work reliably for both cases, and avoid a future provider change raising an incorrect error for my newer example above too.

Thanks @apparentlymart for the details. I will just try it out the way you explained with inner empty block. In the meantime, will wait if any change from the provider for the best possible way in handling the default action dynamically.

jackbatzner commented 3 years ago

Just following up on this, @apparentlymart 's solution does work! Thanks very much for the suggestion, you saved me some time debugging 😄 .

jruizatscopely commented 1 year ago

Just following up on this, @apparentlymart 's solution does work! Thanks very much for the suggestion, you saved me some time debugging smile .

Not for me, it tries to add both block and allow as predicted by @apparentlymart