-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
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
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?
@@ -26,7 +26,7 @@ resource "aws_vpc_endpoint" "this" { | |||
vpc_endpoint_type = lookup(each.value, "service_type", "Interface") | |||
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 |
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_ids
argument toaws_vpc_endpoint
to takenull
values when the endpoint is of Interface typeMotivation and Context
When defining a VPC endpoint of type Interface, if no
security_group_ids
are defined either as defaults or in the individual endpoint definition, the value passed to theaws_vpc_endpoint
resource is an empty list.This PR modifies that behaviour so that if no
security_group_ids
are defined, theaws_vpc_endpoint
receives anull
value 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, anull
value enables the use ofaws_vpc_endpoint_security_group_association
resources without triggering changes on theaws_vpc_endpoint
resource.How Has This Been Tested?
I have tested this changes against an environment where the side effects that this change addresses were occuring