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

Stop replacing single_header & single_query_argument with an int #19

Merged
merged 2 commits into from
Feb 7, 2023

Conversation

paulerickson
Copy link
Contributor

what

  • Some parts of statements get replaced by the number 1, resulting in an error when trying to access its name attribute

why

  • single_header and single_query_argument are unusable without a name attribute

references

@paulerickson paulerickson requested review from a team as code owners January 31, 2023 21:46
@jamengual
Copy link

/test all

@@ -753,15 +753,15 @@ resource "aws_wafv2_web_acl" "default" {
}

dynamic "single_header" {
for_each = lookup(field_to_match.value, "single_header", null) != null ? [1] : []
for_each = lookup(field_to_match.value, "single_header", null) != null ? [field_to_match.value.single_header] : []

content {
name = single_header.value.name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be fixed w/o changing the for_each but instead using

name = field_to_match.value.single_header.value.name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this for_each does not follow the pattern that @paulerickson is referring too and the change proposed will make it more consistant

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see comment

@jamengual jamengual merged commit a31a965 into cloudposse:master Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lookup of field_to_match conditional assignment issue
4 participants