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

fix: sg arn output breaks when providing security groups IDs #67

Merged
merged 4 commits into from
Mar 1, 2024

Conversation

pcn
Copy link
Contributor

@pcn pcn commented Mar 1, 2024

what

Currently a join() is run over what is assumed to be all of the created security groups created by the module.aws_security_group. This join fails when there are no security groups being created. What's more, my reading is that only one security group will ever be created, so only one security group ID should ever need to be extracted, so this is probably a leftover from older logic.

why

When using create_security_group = false and associated_security_groups = [<something>] , the module fails with

│ Invalid value for "lists" parameter: element 0 is null; cannot concatenate
│ null values.

This breaks a documented supported configuration (that we happen to use and be blocked by).

references

This is just putting the fix from https://github.com/dmitrijn/terraform-aws-elasticache-memcached per the request in #56 (comment) to get this fix into main.

@pcn pcn requested review from a team as code owners March 1, 2024 18:24
@pcn pcn requested review from Gowiem and joe-niland March 1, 2024 18:24
@pcn
Copy link
Contributor Author

pcn commented Mar 1, 2024

@Gowiem ping, can you see if this can be tested and released?

@pcn pcn mentioned this pull request Mar 1, 2024
@Gowiem
Copy link
Member

Gowiem commented Mar 1, 2024

/terratest

@Gowiem Gowiem added bugfix Change that restores intended behavior patch A minor, backward compatible change labels Mar 1, 2024
@Gowiem
Copy link
Member

Gowiem commented Mar 1, 2024

/terratest

@Gowiem Gowiem enabled auto-merge (squash) March 1, 2024 20:44
@Gowiem
Copy link
Member

Gowiem commented Mar 1, 2024

Closes #56

@Gowiem Gowiem merged commit 32b97ec into cloudposse:main Mar 1, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Change that restores intended behavior patch A minor, backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants