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

TF 0.12: conditional expressions does not seem to work with complex types #21465

Closed
burakovsky opened this issue May 26, 2019 · 3 comments · Fixed by #21957
Closed

TF 0.12: conditional expressions does not seem to work with complex types #21465

burakovsky opened this issue May 26, 2019 · 3 comments · Fixed by #21957

Comments

@burakovsky
Copy link

Terraform Version

v0.12.0

Terraform Configuration Files

The failure happens in a relatively complicated code. I tried to write general-purpose module for IAM role policy using aws_iam_policy_document data source and dynamic block for statement configuration. The main idea to have possibility not to provide some optional arguments if it not necessary. Statements have principals, not_principals and condition nested configuration blocks which can be used multiple times, that makes it even more complicated. Here is code example:

variable "policy" {
  default = [
    {
      effect  = "Allow"
      actions = ["ec2:*", "ecs:*"]
      principals = [
        {
          type        = "Service",
          identifiers = ["ec2.amazonaws.com"]
        },
        {
          type        = "AWS",
          identifiers = ["*"]
        }
      ]
      condition = []
    },
    {
      effect      = "Deny"
      actions = ["s3:*"]
      resources = [
        "arn:aws:s3:::somebucket",
        "arn:aws:s3:::somebucket/*",
      ]
      condition = [
        {
        test     = "StringLike"
        variable = "s3:prefix"
        values = [
            "",
            "home/",
            "home/&{aws:username}/",
          ]
        }
      ]
    }
  ]
}

data "aws_iam_policy_document" "policy" {
  dynamic "statement" {
    for_each = [for s in var.policy : {
      sid            = contains(keys(s), "sid") ? s.sid : null
      effect         = contains(keys(s), "effect") ? s.effect : null
      actions        = contains(keys(s), "actions") ? s.actions : null
      resources      = contains(keys(s), "resources") ? s.resources : null
      principals     = contains(keys(s), "principals") ? s.principals : null
      condition      = contains(keys(s), "condition") ? s.condition : null
    }]

    content {
      sid           = statement.value.sid
      effect        = statement.value.effect
      actions       = statement.value.actions
      resources     = statement.value.resources
      
      dynamic "principals" {
        for_each = statement.value.principals

        content {
          type        = principals.value.type
          identifiers = principals.value.identifiers
        }
      }

      dynamic "condition" {
        for_each = statement.value.condition

        content {
          test     = condition.value.test
          variable = condition.value.variable
          values   = condition.value.values
        }
      }
    }
  }
}

output "policy_text" {
  value = data.aws_iam_policy_document.policy.json
}

Because some arguments can be not provided, I used for_each = [for s in var.policy : {...} to check if it provided or not.

Expected Behavior

terraform apply works regardless of whether I specified some optional arguments or not.

Actual Behavior

Error: configuration for data.aws_iam_policy_document.policy still contains unknown values during apply (this is a bug in Terraform; please report it!)

Here is a few workarounds:

  1. If argument has list(string) value, I tried to use such variant:
...
actions = contains(keys(s), "actions") ? s.actions : []
...

It works well if I provided actions argument, but if not I got the same error. However, It works fine in such case:

output "test" {
  value = [for s in var.policy : {
      actions        = contains(keys(s), "actions") ? s.actions : []
  }]
}

For my case the workaround is to use additional variable with empty list as default value:

variable "empty_list" {
  type    = list(any)
  default = []
}
...
actions = contains(keys(s), "actions") ? s.actions : var.empty_list

2.principals, not_principals has only 2 parameters (type and identifiers) and I created a map like { type = identifiers } and use empty map as false result of condition expression:

variable "empty_map" {
  type    = map(any)
  default = {}
}

data "aws_iam_policy_document" "policy" {
...
  dynamic "statement" {
    for_each = [for s in var.policy : {
    ...
      principals     = contains(keys(s), "principals") ? { for o in s.principals : o.type => o.identifiers } : var.empty_map
    ...}]
    content {
    ...
      dynamic "principals" {
        for_each = statement.value.principals

        content {
          type        = principals.key
          identifiers = principals.value
        }
      }
    ...
    }
  }
}
...
  1. condition block has 3 arguments and I can't use workaround with map as for principal. As workaround currently I provided empty list for every policy if I don't need to configure it (can see it above in variable default value).

Steps to Reproduce

  1. terraform init
  2. terraform apply

References

Probably related to #21455, #19180

@apparentlymart
Copy link
Contributor

Thanks for reporting this, @burakovsky!

It does indeed seem like something odd is going on here. I'm not sure exactly what yet, but will dig in and debug it further soon.

In the meantime, I'd generally prefer to use lookup as the idiom for "use a default if a key/attribute isn't set", rather than a conditional expression with contains and keys, and switching to that may help you work around this bug:

  resources = lookup(s, "resources", null)

lookup is still supported in Terraform 0.12, and although it's now deprecated to use it with only two arguments (because index syntax is the preferred way to write that), we intend to retain it for this very common situation where a default value must be provided if a key isn't present.


I'm sure this was just an illustrative example to show the problem, but I'd also tend to warn against making a module that exposes the interface of a particular existing resource so closely.

If the caller of the module needs such specific control over the resource, I think it's better to let the caller instantiate the resource itself and then pass the necessary parts into the module, so that the interfaces between the different sub-components can be simpler and we don't need such a big wall of conditional expressions covering the entire configuration surface of a resource.

The Terraform 0.12 features intend to make it possible to do things like this for the inevitable situation where it's necessary, but I think this sort of thing is best considered a last resort, instead preferring to use modules to create higher-level abstractions over complex concepts, such as a module that creates a particular kind of IAM policy tailored for a specific use-case, where the input variables are therefore tailored to that particular problem.

@burakovsky
Copy link
Author

Thanks a lot, @apparentlymart! lookup works great in this situation!

@ghost
Copy link

ghost commented Aug 13, 2019

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 Aug 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants