Skip to content

Commit 3234f73

Browse files
resolve reviewer comments
Signed-off-by: Jintao Zhang <[email protected]> Co-authored-by: Patryk Małek <[email protected]>
1 parent b643fee commit 3234f73

File tree

6 files changed

+96
-47
lines changed

6 files changed

+96
-47
lines changed

controller/gateway/controller.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
operatorerrors "github.com/kong/kong-operator/internal/errors"
4848
gwtypes "github.com/kong/kong-operator/internal/types"
4949
"github.com/kong/kong-operator/internal/utils/gatewayclass"
50+
idx "github.com/kong/kong-operator/internal/utils/index"
5051
"github.com/kong/kong-operator/modules/manager/logging"
5152
"github.com/kong/kong-operator/pkg/consts"
5253
gatewayutils "github.com/kong/kong-operator/pkg/utils/gateway"
@@ -146,10 +147,11 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) err
146147
mgr.GetCache(),
147148
&corev1.Secret{},
148149
handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, s *corev1.Secret) []reconcile.Request {
149-
// List Gateways and select those that reference the Secret in any listener certificateRef.
150+
// Use field index to list only Gateways that reference this Secret.
150151
var gwList gwtypes.GatewayList
151-
if err := r.List(ctx, &gwList); err != nil {
152-
ctrllog.FromContext(ctx).Error(err, "failed to list gateways for Secret watch", "secret", client.ObjectKeyFromObject(s))
152+
key := fmt.Sprintf("%s/%s", s.Namespace, s.Name)
153+
if err := r.List(ctx, &gwList, client.MatchingFields{idx.TLSCertificateSecretsOnGatewayIndex: key}); err != nil {
154+
ctrllog.FromContext(ctx).Error(err, "failed to list indexed gateways for Secret watch", "secret", client.ObjectKeyFromObject(s))
153155
return nil
154156
}
155157
recs := make([]reconcile.Request, 0, len(gwList.Items))
@@ -159,9 +161,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) err
159161
if !r.gatewayHasMatchingGatewayClass(&gw) {
160162
continue
161163
}
162-
if secretReferencedByGateway(&gw, s.Namespace, s.Name) {
163-
recs = append(recs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&gw)})
164-
}
164+
recs = append(recs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&gw)})
165165
}
166166
return recs
167167
}),

controller/gateway/controller_secret_watch_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package gateway
33
import (
44
"testing"
55

6+
"github.com/samber/lo"
67
corev1 "k8s.io/api/core/v1"
78
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
89
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -23,18 +24,15 @@ func gwWithListenerCertRef(name string, ref gatewayv1.SecretObjectReference) *gw
2324
Port: 443,
2425
Protocol: gwtypes.HTTPSProtocolType,
2526
TLS: &gatewayv1.GatewayTLSConfig{
26-
Mode: gatewayTLSModePtr(gatewayv1.TLSModeTerminate),
27+
Mode: lo.ToPtr(gatewayv1.TLSModeTerminate),
2728
CertificateRefs: []gatewayv1.SecretObjectReference{ref},
2829
},
29-
},
3030
},
3131
},
3232
}
3333
return gw
3434
}
3535

36-
func gatewayTLSModePtr(m gatewayv1.TLSModeType) *gatewayv1.TLSModeType { return &m }
37-
3836
func Test_secretReferencedByGateway_SameNamespace_DefaultGroupKind(t *testing.T) {
3937
// No Group/Kind set => treated as core/v1 Secret
4038
ref := gatewayv1.SecretObjectReference{

ingress-controller/internal/controllers/gateway/gateway_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error {
147147
}
148148
recs := make([]reconcile.Request, 0, len(referrers))
149149
for _, obj := range referrers {
150-
nn := k8stypes.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}
150+
nn := client.ObjectKeyFromObject(obj)
151151
if !r.GatewayNN.MatchesNN(nn) { // respect --gateway-to-reconcile if set
152152
continue
153153
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package index
2+
3+
import (
4+
"github.com/samber/lo"
5+
corev1 "k8s.io/api/core/v1"
6+
"sigs.k8s.io/controller-runtime/pkg/client"
7+
8+
gwtypes "github.com/kong/kong-operator/internal/types"
9+
)
10+
11+
const (
12+
// TLSCertificateSecretsOnGatewayIndex maps Secret "namespace/name" to Gateway objects that
13+
// reference them in listeners.tls.certificateRefs.
14+
TLSCertificateSecretsOnGatewayIndex = "TLSCertificateSecretsOnGateway"
15+
)
16+
17+
// OptionsForGatewayTLSSecret returns index options for Gateways referencing Secrets via TLS certificateRefs.
18+
func OptionsForGatewayTLSSecret() []Option {
19+
return []Option{
20+
{
21+
Object: &gwtypes.Gateway{},
22+
Field: TLSCertificateSecretsOnGatewayIndex,
23+
ExtractValueFn: tlsCertificateSecretsOnGateway,
24+
},
25+
}
26+
}
27+
28+
// tlsCertificateSecretsOnGateway extracts Secret references (namespace/name) from a Gateway's listeners.tls.certificateRefs.
29+
func tlsCertificateSecretsOnGateway(o client.Object) []string {
30+
gw, ok := o.(*gwtypes.Gateway)
31+
if !ok {
32+
return nil
33+
}
34+
35+
var out []string
36+
for _, l := range gw.Spec.Listeners {
37+
if l.TLS == nil {
38+
continue
39+
}
40+
for _, ref := range l.TLS.CertificateRefs {
41+
// Only index core/v1 Secret references (or defaulted ones when group/kind are nil).
42+
if ref.Group != nil && string(*ref.Group) != corev1.GroupName {
43+
continue
44+
}
45+
if ref.Kind != nil && string(*ref.Kind) != "Secret" {
46+
continue
47+
}
48+
ns := gw.Namespace
49+
if ref.Namespace != nil {
50+
ns = string(*ref.Namespace)
51+
}
52+
if ref.Name == "" {
53+
continue
54+
}
55+
out = append(out, ns+"/"+string(ref.Name))
56+
}
57+
}
58+
return lo.Uniq(out)
59+
}

modules/manager/controller_setup.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ func SetupCacheIndexes(ctx context.Context, mgr manager.Manager, cfg Config) err
9898

9999
if cfg.GatewayControllerEnabled {
100100
indexOptions = append(indexOptions, index.OptionsForGatewayClass()...)
101+
// Index Gateways by referenced TLS Secrets so the Gateway controller can
102+
// efficiently list Gateways for a Secret event.
103+
indexOptions = append(indexOptions, index.OptionsForGatewayTLSSecret()...)
101104
}
102105

103106
if cfg.KonnectControllersEnabled {

test/envtest/gateway_secret_watch_envtest_test.go

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55
"time"
66

7+
"github.com/samber/lo"
78
"github.com/stretchr/testify/require"
89
corev1 "k8s.io/api/core/v1"
910
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -15,6 +16,8 @@ import (
1516
certhelper "github.com/kong/kong-operator/ingress-controller/test/helpers/certificate"
1617
"github.com/kong/kong-operator/modules/manager/logging"
1718
managerscheme "github.com/kong/kong-operator/modules/manager/scheme"
19+
gwcdecor "github.com/kong/kong-operator/internal/utils/gatewayclass"
20+
k8sutils "github.com/kong/kong-operator/pkg/utils/kubernetes"
1821
"github.com/kong/kong-operator/pkg/vars"
1922
)
2023

@@ -62,18 +65,9 @@ func TestGatewaySecretWatch_UpdatesResolvedRefsOnSecretRotation(t *testing.T) {
6265
ObservedGeneration: cur.Generation,
6366
LastTransitionTime: metav1.Now(),
6467
}
65-
// Replace existing condition if present; otherwise append.
66-
updated := false
67-
for i := range cur.Status.Conditions {
68-
if cur.Status.Conditions[i].Type == cond.Type {
69-
cur.Status.Conditions[i] = cond
70-
updated = true
71-
break
72-
}
73-
}
74-
if !updated {
75-
cur.Status.Conditions = append(cur.Status.Conditions, cond)
76-
}
68+
// Use shared helper to set/merge the condition.
69+
gwcd := gwcdecor.DecorateGatewayClass(&cur)
70+
k8sutils.SetCondition(cond, gwcd)
7771
if err := c.Status().Update(ctx, &cur); err != nil {
7872
return false
7973
}
@@ -103,35 +97,18 @@ func TestGatewaySecretWatch_UpdatesResolvedRefsOnSecretRotation(t *testing.T) {
10397
Port: 443,
10498
Protocol: gatewayv1.HTTPSProtocolType,
10599
TLS: &gatewayv1.GatewayTLSConfig{
106-
Mode: gatewayTLSModePtr(gatewayv1.TLSModeTerminate),
100+
Mode: lo.ToPtr(gatewayv1.TLSModeTerminate),
107101
CertificateRefs: []gatewayv1.SecretObjectReference{{
108102
Name: gatewayv1.ObjectName(secretName),
109103
}},
110104
},
111-
},
112105
},
113106
},
114107
}
115108
require.NoError(t, c.Create(ctx, gw))
116109

117110
// Initially, the invalid Secret should result in ResolvedRefs=False (InvalidCertificateRef).
118-
require.Eventually(t, func() bool {
119-
var cur gatewayv1.Gateway
120-
if err := c.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: gw.Name}, &cur); err != nil {
121-
return false
122-
}
123-
if len(cur.Status.Listeners) == 0 {
124-
return false
125-
}
126-
for _, ls := range cur.Status.Listeners {
127-
for _, cond := range ls.Conditions {
128-
if cond.Type == string(gatewayv1.ListenerConditionResolvedRefs) && cond.Status == metav1.ConditionFalse {
129-
return true
130-
}
131-
}
132-
}
133-
return false
134-
}, 30*time.Second, 300*time.Millisecond)
111+
waitForGatewayResolvedRefsStatus(t, ctx, c, ns.Name, gw.Name, metav1.ConditionFalse, 30*time.Second, 300*time.Millisecond)
135112

136113
// Now rotate the Secret with a valid TLS certificate and key.
137114
certPEM, keyPEM := certhelper.MustGenerateCertPEMFormat()
@@ -145,23 +122,35 @@ func TestGatewaySecretWatch_UpdatesResolvedRefsOnSecretRotation(t *testing.T) {
145122
}, client.MergeFrom(bad)))
146123

147124
// After rotation, the Secret watch should enqueue the Gateway and ResolvedRefs should become True.
125+
waitForGatewayResolvedRefsStatus(t, ctx, c, ns.Name, gw.Name, metav1.ConditionTrue, 60*time.Second, 500*time.Millisecond)
126+
}
127+
128+
// waitForGatewayResolvedRefsStatus waits until the Gateway's listener ResolvedRefs condition
129+
// matches the expected status, or fails after the provided timeout.
130+
func waitForGatewayResolvedRefsStatus(
131+
t *testing.T,
132+
ctx context.Context,
133+
c client.Client,
134+
namespace, name string,
135+
status metav1.ConditionStatus,
136+
timeout, interval time.Duration,
137+
) {
138+
t.Helper()
148139
require.Eventually(t, func() bool {
149140
var cur gatewayv1.Gateway
150-
if err := c.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: gw.Name}, &cur); err != nil {
141+
if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, &cur); err != nil {
151142
return false
152143
}
153144
if len(cur.Status.Listeners) == 0 {
154145
return false
155146
}
156147
for _, ls := range cur.Status.Listeners {
157148
for _, cond := range ls.Conditions {
158-
if cond.Type == string(gatewayv1.ListenerConditionResolvedRefs) && cond.Status == metav1.ConditionTrue {
149+
if cond.Type == string(gatewayv1.ListenerConditionResolvedRefs) && cond.Status == status {
159150
return true
160151
}
161152
}
162153
}
163154
return false
164-
}, 60*time.Second, 500*time.Millisecond)
155+
}, timeout, interval)
165156
}
166-
167-
func gatewayTLSModePtr(m gatewayv1.TLSModeType) *gatewayv1.TLSModeType { return &m }

0 commit comments

Comments
 (0)