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

Conditionally setting attributes #380

Closed
evanstoddard23 opened this issue Aug 28, 2024 · 8 comments
Closed

Conditionally setting attributes #380

evanstoddard23 opened this issue Aug 28, 2024 · 8 comments

Comments

@evanstoddard23
Copy link

evanstoddard23 commented Aug 28, 2024

Is your request related to a problem? Please describe.

The try() function is used extensively throughout this module to evaluate if a key is set. However I find myself in many cases wanting to conditionally set these keys. When setting a key to null, it does not result in an error, so the for_each ends up trying to create a block it shouldn't, resulting in an error.

Here's an example:

module "alb" {
  source  = "terraform-aws-modules/alb/aws"
  version = "9.11.0"
  for_each = {...}
  listeners = {
    https = {
      forward = each.value.some_condition ? null : {
        target_group_key = "default"
      }

      fixed_response = each.value.some_condition ? {
        content_type = "text/plain"
        message_body = "Access denied"
        status_code  = "403"
      } : null
...

Results in errors like:

│ Error: Attempt to get attribute from null value
│ 
│   on .terraform/modules/frontends.alb/main.tf line 141, in resource "aws_lb_listener" "this":
│  141:         content_type = default_action.value.content_type
│     ├────────────────
│     │ default_action.value is null
│ 
│ This value is null, so it does not have any attributes.

Describe the solution you'd like.

Maybe these blocks should look something like this to allow for null values?

dynamic "default_action" {
  for_each = try([each.value.fixed_response != null ? each.value.fixed_response : (1 / 0)], [])
...

Describe alternatives you've considered.

I haven't been able to think of other ways to handle this gracefully.

@bryantbiggs
Copy link
Member

However I find myself in many cases wanting to conditionally set these keys.

Why? This seems like you are trying to re-wrap this module so you'd have to handle that differently

@evanstoddard23
Copy link
Author

evanstoddard23 commented Aug 29, 2024

@bryantbiggs In my case I'm using a for_each loop to create albs alongside other associated infrastructure such as cloudfront, waf, and route 53 for similar services. v8.7.0 worked well like this:

...
https_listeners = [{
  ...
  fixed_response = each.value.some_condition ? {
    content_type = "text/plain"
    message_body = "Access denied"
    status_code  = 403
  } : {}

I was able to get around this in v9.x by doing a higher level map merge, but I think it makes the code a bit messier and it seems (maybe just to me) that null is a valid value that should be able to be passed in.

...
https = merge(each.value.some_condition ? {
  fixed_response = {
    content_type = "text/plain"
    message_body = "Access denied"
    status_code  = "403"
  }
  } :
  {
    forward = {
      target_group_key = "default"
    }
  },
  {
    port            = 443
    ...

@bryantbiggs
Copy link
Member

this feels like an x-y type problem so I'm not sure how much guidance I can provide given the narrow context. but in general, 99.9% of the time we've received issues like this - the answer has always been - "you should adjust how you are doing that"

@evanstoddard23
Copy link
Author

I was able to more or less get around it for what I’m doing, although it’s not pretty. It does seem to me like a valid part of HCL that’s not being properly handled by this module though given this in the docs “null is most useful in conditional expressions, so you can dynamically omit an argument if a condition isn't met.”

Copy link

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Sep 30, 2024
@AkshayB-ops
Copy link

Any solution for this?

@github-actions github-actions bot removed the stale label Oct 4, 2024
Copy link

github-actions bot commented Nov 3, 2024

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Nov 3, 2024
Copy link

This issue was automatically closed because of stale in 10 days

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants