Skip to content

Conversation

@tao12345666333
Copy link
Member

@tao12345666333 tao12345666333 commented Nov 20, 2025

What this PR does / why we need it:

Which issue this PR fixes

Fixes #2646

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect significant changes

@tao12345666333 tao12345666333 self-assigned this Nov 20, 2025
@tao12345666333 tao12345666333 force-pushed the fix-secret-watch branch 2 times, most recently from eff2299 to b643fee Compare November 20, 2025 15:12
@tao12345666333 tao12345666333 marked this pull request as ready for review November 21, 2025 13:04
@tao12345666333 tao12345666333 requested a review from a team as a code owner November 21, 2025 13:04
@tao12345666333 tao12345666333 force-pushed the fix-secret-watch branch 2 times, most recently from 3234f73 to d071cba Compare November 21, 2025 19:28
Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nits, overall this is a solid fix 👍

Can we add a changelog entry for this?

@sandromodarelli
Copy link
Contributor

please, backport this in 2.0.x so we can release it with #2707

tao12345666333 and others added 7 commits November 26, 2025 18:43
…listeners.tls.certificateRef

Signed-off-by: Jintao Zhang <[email protected]>
Signed-off-by: Jintao Zhang <[email protected]>
Co-authored-by: Patryk Małek <[email protected]>
Signed-off-by: Jintao Zhang <[email protected]>
Signed-off-by: Jintao Zhang <[email protected]>
Signed-off-by: Jintao Zhang <[email protected]>
Comment on lines +55 to +73
require.Eventually(t, func() bool {
var cur gatewayv1.GatewayClass
if err := c.Get(ctx, types.NamespacedName{Name: gc.Name}, &cur); err != nil {
return false
}
cond := metav1.Condition{
Type: string(gatewayv1.GatewayClassConditionStatusAccepted),
Status: metav1.ConditionTrue,
Reason: string(gatewayv1.GatewayClassReasonAccepted),
ObservedGeneration: cur.Generation,
LastTransitionTime: metav1.Now(),
}
// Use shared helper to set/merge the condition.
gwcd := gwcdecor.DecorateGatewayClass(&cur)
k8sutils.SetCondition(cond, gwcd)
if err := c.Status().Update(ctx, &cur); err != nil {
return false
}
return true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like something that we could reuse. We already have a bunch of similar helpers in https://github.com/Kong/kong-operator/blob/e90fd984547cd60e3ce8c481f368514dd1936411/pkg/utils/test/predicates.go, e.g.

// DataPlaneUpdateEventually is a helper function for tests that returns a function
// that can be used to update the DataPlane.
// Should be used in conjunction with require.Eventually or assert.Eventually.
func DataPlaneUpdateEventually(t *testing.T, ctx context.Context, dataplaneNN types.NamespacedName, clients K8sClients, updateFunc func(*operatorv1beta1.DataPlane)) func() bool {
return func() bool {
cl := clients.OperatorClient.GatewayOperatorV1beta1().DataPlanes(dataplaneNN.Namespace)
dp, err := cl.Get(ctx, dataplaneNN.Name, metav1.GetOptions{})
if err != nil {
t.Logf("error getting dataplane: %v", err)
return false
}
updateFunc(dp)
_, err = cl.Update(ctx, dp, metav1.UpdateOptions{})
if err != nil {
t.Logf("error updating dataplane: %v", err)
return false
}
return true
}
}
.

I think it would make sense to put it there and use it here. WDYT?

Comment on lines +128 to +155
// waitForGatewayResolvedRefsStatus waits until the Gateway's listener ResolvedRefs condition
// matches the expected status, or fails after the default timeout.
func waitForGatewayResolvedRefsStatus(
t *testing.T,
ctx context.Context,
c client.Client,
namespace, name string,
status metav1.ConditionStatus,
) {
t.Helper()
require.Eventually(t, func() bool {
var cur gatewayv1.Gateway
if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, &cur); err != nil {
return false
}
if len(cur.Status.Listeners) == 0 {
return false
}
for _, ls := range cur.Status.Listeners {
for _, cond := range ls.Conditions {
if cond.Type == string(gatewayv1.ListenerConditionResolvedRefs) && cond.Status == status {
return true
}
}
}
return false
}, waitTime, tickTime)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto about defining a reusable test assertion helper.

Comment on lines +148 to +167
handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, s *corev1.Secret) []reconcile.Request {
// Use field index to list only Gateways that reference this Secret.
var gwList gwtypes.GatewayList
nn := client.ObjectKeyFromObject(s)
if err := r.List(ctx, &gwList, client.MatchingFields{idx.TLSCertificateSecretsOnGatewayIndex: nn.String()}); err != nil {
ctrllog.FromContext(ctx).Error(err, "failed to list indexed gateways for Secret watch", "secret", nn)
return nil
}
recs := make([]reconcile.Request, 0, len(gwList.Items))
for i := range gwList.Items {
gw := gwList.Items[i]
// Optional pre-filter: only enqueue Gateways managed by this controller.
if !r.gatewayHasMatchingGatewayClass(&gw) {
continue
}
recs = append(recs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&gw)})
}
return recs
}),
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about extracting this into a function and adding a test for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway controllers do not watching Secrets referenced by spec.listeners.tls.certificateRef

4 participants