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

Validation bug when using local variables combined with data sources #19284

Closed
edmundcraske opened this issue Nov 5, 2018 · 3 comments
Closed

Comments

@edmundcraske
Copy link
Contributor

edmundcraske commented Nov 5, 2018

Hi, I'm seeing a weird issue that smells to me like a general configuration/input validation issue, although I've caused it when trying to conditionally enable logging for an AWS CloudFront distribution, so it could be a quirk of the way that the aws provider is implemented.

I'm trying to write a resource definition for aws_cloudfront_distribution where 'logging_config' is defined conditionally based upon a variable being set to "true".

When running the below configuration (edited for brevity - can share my full config or try and come up with a minimal version showing the bug if needed), regardless of setting 'var.cloudfront_logging_enabled', I get the following error:
Error: aws_cloudfront_distribution.cloudfront: "logging_config.0.bucket": required field is not set

However, if I remove the bucket = "${data.terraform_remote_state.cloudfront_logs_s3_bucket.bucket_name}" and replace with bucket = "arbitrarystring" then the code works as expected - when 'var.cloudfront_logging_enabled' is false, it does not want to enable logging, and when true, it wants to enable it using the bucket string specified.

If I hardcode the logging_config to use the remote state data source, that works - it appears that the combination of conditionally selecting between the output of a remote state data source and an empty configuration triggers a bug that causes terraform to decide that there is a missing field in the configuration, possibly because it has not retrieved the required data to calculate what that field is yet?

Terraform Version

Terraform v0.11.10

Terraform Configuration Files

locals {
  cloudfront_logging_config_map = {
    enabled = [{
      bucket          = "${data.terraform_remote_state.cloudfront_logs_s3_bucket.bucket_name}"
      include_cookies = "false"
      prefix          = "${var.environment}/"
    }]

    disabled = []
  }

  enable_logging = "${var.cloudfront_logging_enabled == "true" ? "enabled" : "disabled"}"
}

resource "aws_cloudfront_distribution" "cloudfront" {
  logging_config = ["${local.cloudfront_logging_config_map[local.enable_logging]}"]
  # other config here...
}

Debug Output

Can provide debug output if needed but does not seem to contain anything useful regarding this validation.

Crash Output

Does not crash

Expected Behavior

Depending on the setting of 'var.cloudfront_logging_enabled', logging should either be enabled with the bucket name from the remote state data source ("true"), or not configured ("false").

Actual Behavior

Regardless of setting of 'var.cloudfront_logging_enabled', terraform gives the error:
Error: aws_cloudfront_distribution.cloudfront: "logging_config.0.bucket": required field is not set

This does not happen if the bucket name is hard coded, or if the conditional element is removed (i.e. always setting logging_config to take the bucket argument from the remote state data source) - it is the combination of the two that appears to trigger the issue.

Steps to Reproduce

  1. terraform init
  2. terraform plan

Additional Context

Using s3 backend and remote state.

@edmundcraske
Copy link
Contributor Author

Have since built a minimal example configuration that exhibits the issue - it's reproducible without using remote state, just any data source, and appears to be more of an issue of interpolation of local variables - the validation is deciding that an as-yet-uncalculated local variable is empty, which terraform treats the same as not being set.

This does not occur for string-type local variables, but does occur for map keys within local variables, regardless of nesting.

See below minimal configuration - if either the bucket key within the map is changed to be a hardcoded string (or indeed a non-local string variable), or the logging_config is coded to use the data source directly, it works.

provider "aws" {}
  
data "aws_caller_identity" "current" {}

variable "cloudfront_logging_enabled" {
  type = "string"
  default = "false"
}

locals {
  cloudfront_logging_config = {
    bucket = "test${data.aws_caller_identity.current.account_id}"
    #bucket = "working-test"
  }
}

resource "aws_cloudfront_distribution" "cloudfront" {
  enabled = true
  is_ipv6_enabled = true
  price_class = "PriceClass_100"

  logging_config = ["${local.cloudfront_logging_config}"]

  # this works
  #logging_config {
  #  bucket = "${data.aws_caller_identity.current.account_id}"
  #}

  origin {
    origin_id = "Test-S3-Origin"
    domain_name = "awsmedia.s3.amazonaws.com"
  }

  default_cache_behavior {
    allowed_methods  = ["GET", "HEAD"]
    cached_methods   = ["GET", "HEAD"]
    target_origin_id = "Test-S3-Origin"
    viewer_protocol_policy = "redirect-to-https"

    forwarded_values {
      query_string = false

      cookies {
        forward = "none"
      }
    }
  }

  restrictions {
    geo_restriction {
      restriction_type = "none"
    }
  }

  viewer_certificate {
    cloudfront_default_certificate = "true"
    minimum_protocol_version = "TLSv1"
    ssl_support_method       = "sni-only"
  }
}

@edmundcraske edmundcraske changed the title Possible validation bug when using conditionals combined with remote state? Validation bug when using local variables combined with data sources Nov 5, 2018
@apparentlymart
Copy link
Contributor

Hi @edmundcraske! Sorry for this strange behavior.

Terraform is acting weird here because logging_config is defined as a nested block rather than as a map. Terraform actually ought to be rejecting what you did here outright, but due to a validation bug Terraform allows it but then gets confused during subsequent validation.

In Terraform v0.11 and prior there isn't any official solution for dynamically setting a block like this. The good news is that such a feature is already in master in preparation for the forthcoming v0.12.0 release. The new way to write what you wanted to express here would be:

locals {
  cloudfront_logging_configs = {
    enabled = [{
      bucket          = data.terraform_remote_state.cloudfront_logs_s3_bucket.bucket_name
      include_cookies = false
      prefix          = "${var.environment}/"
    }]
    disabled = []
  }

  cloudfront_logging_config_key = var.cloudfront_logging_enabled ? "enabled" : "disabled"
}

resource "aws_cloudfront_distribution" "cloudfront" {
  dynamic "logging_config" {
    for_each = local.cloudfront_logging_configs[local.cloudfront_logging_config_key]
    content {
      bucket          = logging_config.value.bucket
      include_cookies = logging_config.value.include_cookies
      prefix          = logging_config.value.prefix
    }
  }

  # other config here...
}

This new dynamic block feature allows blocks to be dynamically created while retaining the usual characteristics of blocks, allowing for sub-blocks (where appropriate) and most importantly allowing Terraform to do the usual validation of the contents of the generated content of the blocks, even if some of the values in there will not be known until apply time.

Since this work is already landed in master ready to be included in the release, I'm going to close this out. Thanks for reporting this, and sorry for the unfortunate timing of you running into this during this period where this fix is ready but other fixes are blocking the final release.

@ghost
Copy link

ghost commented Mar 31, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants