fix: Fixed service_region argument in the VPC endpoint module #1162
Conversation
modules/vpc-endpoints/main.tf
Outdated
| @@ -13,7 +13,7 @@ data "aws_vpc_endpoint_service" "this" { | |||
|
|
|||
| service = try(each.value.service, null) | |||
| service_name = try(each.value.service_name, null) | |||
| service_regions = try([each.value.service_region], null) | |||
| service_regions = try(each.value.service_region != null ? [each.value.service_region] : null, null) | |||
There was a problem hiding this comment.
| service_regions = try(each.value.service_region != null ? [each.value.service_region] : null, null) | |
| service_regions = one([each.value.service_region]) |
Much simpler, but please test it. :)
There was a problem hiding this comment.
Thanks for the feedback. So I tested
service_regions = one([each.value.service_region])this does not quite work. If the endpoints maps' service_region value is a valid string, it evaluates just to the string, but the data sources' service_regions expects a list of strings. Further, if service _region is not included in the endpoints map, it also fails.
There was a problem hiding this comment.
I think we could change it to this which is a bit easier to understand and read and I successfully tested for all 3 cases (service_region not included in the endpoints map, service_region=null or service_region="valid-region-string":
service_regions = try(each.value.service_region, null) != null ? [each.value.service_region] : nullThere was a problem hiding this comment.
I think this is going to work as expected:
service_regions = try(coalescelist(compact([each.value.service_region])), null)Let's use it if it does.
There was a problem hiding this comment.
It does indeed, thank you! Updated the PR accordingly.
There was a problem hiding this comment.
Thank you for fixing an issue!
antonbabenko
changed the title
## [5.18.1](v5.18.0...v5.18.1) (2025-01-28) ### Bug Fixes * Fixed service_region argument in the VPC endpoint module ([#1162](#1162)) ([5415dee](5415dee))
|
This PR is included in version 5.18.1 🎉 |
|
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
Currently, if we pass
service_region=null(rather than not passing it at all), the module fails as it passes on[null]rather thannullto the underlying data source.Motivation and Context
See description and the comment in this PR that added cross-region support in the first place.
Breaking Changes
Non breaking, makes things more robust and covers all all cases (no
service_regionpassed,service_region=nullpassed, valid service region passed)How Has This Been Tested?
Tested passing no
service_region,service_region=null, or a valid service region passed