[feat aga] Add cross-namespace reference support for AGA #4547
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shraddhabang 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 |
k8s-ci-robot
added
approved
shraddhabang
force-pushed
the
agacrossns
branch
from
b47c1fb to
3dffec2
Compare
shraddhabang
force-pushed
the
agacrossns
branch
from
3dffec2 to
30112d0
Compare
| for _, from := range impactedFroms { | ||
|
|
||
| var gaList agaapi.GlobalAcceleratorList | ||
| if err := h.k8sClient.List(ctx, &gaList, &client.ListOptions{Namespace: string(from.Namespace)}); err != nil { |
There was a problem hiding this comment.
Is there any caching you can do here to prevent calling List() on the same namespace multiple times? I think the client should handle this case, but adding an explicit cache might make things cleaner here.
There was a problem hiding this comment.
Ohh are you concerned they might have multiple same namespace references in single grant or you thinking across grants?
There was a problem hiding this comment.
I took a look at the Reference Grant structure, and the comment I made doesn't make sense (sorry). I see the existing reference grant logic also does this same listing.
|
|
||
| !!!note "To use cross-namespace references" | ||
|
|
||
| 1. The Gateway API CRDs must be installed in your cluster (specifically the ReferenceGrant CRD) |
There was a problem hiding this comment.
You can be explicit here and say that Reference grant is installed via "standard crds" https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/guide/gateway/gateway/#prerequisites
| } | ||
|
|
||
| // grantAllowsReference checks if a specific ReferenceGrant allows the reference | ||
| func (v *ReferenceGrantValidator) grantAllowsReference(grant gwv1beta1.ReferenceGrant, |
There was a problem hiding this comment.
Can you refactor this reference grant logic and the gateway reference grant logic into a shared resource?
There was a problem hiding this comment.
If you choose to not refactor this, I think it should have private visibility.
| } | ||
|
|
||
| // Validate cross-namespace reference using ReferenceGrant | ||
| if err := l.crossNamespaceValidator.ValidateCrossNamespaceReference(ctx, |
There was a problem hiding this comment.
I suppose you want a more graceful failure mode here when the user is attempting to use cross namespace referencing but they have forgotten to install the proper CRD.
| ) | ||
|
|
||
| BeforeEach(func() { | ||
| if tf.Options.IPFamily == framework.IPv6 { |
There was a problem hiding this comment.
why is ipv6 not supported?
There was a problem hiding this comment.
The dualstack NLBs with Client IP preservations enabled are not supported hence we have disabled those in our tests.
| Expect(err).NotTo(HaveOccurred()) | ||
|
|
||
| currentTestName := CurrentSpecReport().LeafNodeText | ||
| if !strings.Contains(currentTestName, "cross-namespace") { |
3 participants
This PR adds support for cross-namespace references to the AWS Global Accelerator controller, allowing a GlobalAccelerator resource to reference endpoints (Ingress, Service, Gateway) located in different namespaces using ReferenceGrant resources from the Gateway API.
Changes
How It Works
The controller now checks for the existence of ReferenceGrant resources that explicitly allow cross-namespace references. When a GlobalAccelerator attempts to reference an endpoint in another namespace:
Testing
Added comprehensive e2e tests for:
Checklist
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯