-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix: Allow security_group_ids to take null values
#825
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: Allow security_group_ids to take null values
#825
Conversation
security_group_ids to take null valuessecurity_group_ids to take null values
security_group_ids to take null valuessecurity_group_ids to take null values
antonbabenko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@bryantbiggs WDYT?
| auto_accept = lookup(each.value, "auto_accept", null) | ||
|
|
||
| security_group_ids = lookup(each.value, "service_type", "Interface") == "Interface" ? distinct(concat(var.security_group_ids, lookup(each.value, "security_group_ids", []))) : null | ||
| security_group_ids = lookup(each.value, "service_type", "Interface") == "Interface" ? length(distinct(concat(var.security_group_ids, lookup(each.value, "security_group_ids", [])))) > 0 ? distinct(concat(var.security_group_ids, lookup(each.value, "security_group_ids", []))) : null : null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit shorter way:
| security_group_ids = lookup(each.value, "service_type", "Interface") == "Interface" ? length(distinct(concat(var.security_group_ids, lookup(each.value, "security_group_ids", [])))) > 0 ? distinct(concat(var.security_group_ids, lookup(each.value, "security_group_ids", []))) : null : null | |
| security_group_ids = lookup(each.value, "service_type", "Interface") == "Interface" ? try(coalescelist(distinct(concat(var.security_group_ids, lookup(each.value, "security_group_ids", [])))), null) : null |
bryantbiggs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks @Jorge-Rodriguez !
### [3.14.3](v3.14.2...v3.14.3) (2022-09-02) ### Bug Fixes * Allow `security_group_ids` to take `null` values ([#825](#825)) ([67ef09a](67ef09a))
|
This PR is included in version 3.14.3 🎉 |
|
I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
Allow the
security_group_idsargument toaws_vpc_endpointto takenullvalues when the endpoint is of Interface typeMotivation and Context
When defining a VPC endpoint of type Interface, if no
security_group_idsare defined either as defaults or in the individual endpoint definition, the value passed to theaws_vpc_endpointresource is an empty list.This PR modifies that behaviour so that if no
security_group_idsare defined, theaws_vpc_endpointreceives anullvalue instead of an empty list. Semantically, it makes no difference and it causes the default security group to be associated with the endpoint. However, functionally, anullvalue enables the use ofaws_vpc_endpoint_security_group_associationresources without triggering changes on theaws_vpc_endpointresource.How Has This Been Tested?
I have tested this changes against an environment where the side effects that this change addresses were occuring