feat: add SupportedRegions field to VPCEndpointServiceConfiguration#337
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: michaelhtm The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
michaelhtm
force-pushed
the
feat/add-supported-regions-to-vpc-endpoint-service
branch
2 times, most recently
from
9946080 to
b29ad48
Compare
|
Hello, I tested this PR against a real AWS integration environment (built the controller from this branch, applied the updated CRD), and it works. Sharing results in case it helps move the review along. What I validated:
Thanks for putting this together, would love to see it land. 🙏 |
knottnt
left a comment
knottnt
left a comment
There was a problem hiding this comment.
Thanks @michaelhtm implementation looks good, but would be good to validate the custom update logic in an e2e test.
| return nil | ||
| } | ||
|
|
||
| func customPreCompare( |
There was a problem hiding this comment.
Q: What's the reason for a custom delta here?
There was a problem hiding this comment.
it allows us to compare supported regions regardless of order.
We can add a helper function in Runtime later to support orderless list comparison
| } | ||
| } | ||
|
|
||
| if delta.DifferentAt("Spec.SupportedRegions") { |
There was a problem hiding this comment.
Since, we're adding custom update behavior it would be good to validate the functionality in an e2e test that checks.
- That supported regions can be added/removed
- Updates to other fields applied at the same time are also applied.
michaelhtm
force-pushed
the
feat/add-supported-regions-to-vpc-endpoint-service
branch
from
b29ad48 to
0b09dd1
Compare
The field was previously ignored. Users can now specify which regions service consumers can access the endpoint service from. Uses set.from to extract Region from SupportedRegionDetail on read, and Add/Remove semantics on update via ModifyVpcEndpointServiceConfiguration. Resolves aws-controllers-k8s/community#2902
michaelhtm
force-pushed
the
feat/add-supported-regions-to-vpc-endpoint-service
branch
from
0b09dd1 to
efb144e
Compare
|
@michaelhtm: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Description of changes:
The field was previously ignored. Users can now specify which regions
service consumers can access the endpoint service from. Uses set.from
to extract Region from SupportedRegionDetail on read, and Add/Remove
semantics on update via ModifyVpcEndpointServiceConfiguration.
Resolves aws-controllers-k8s/community#2902
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.