From 1066f90e1feb54757ba5fb6695e9c9d24db4ba74 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Tue, 24 Feb 2026 20:40:49 -0800 Subject: [PATCH 01/17] feat: shared tls secret for default listeners --- internal/config/config.go | 15 ++ internal/controller/gateway_controller.go | 45 +++- .../controller/gateway_controller_test.go | 243 ++++++++++++++++++ .../trafficprotectionpolicy_controller.go | 11 +- 4 files changed, 301 insertions(+), 13 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 76534797..8a2a3d79 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -526,6 +526,21 @@ type GatewayConfig struct { // // Defaults to false. EnableDNSIntegration bool `json:"enableDNSIntegration,omitempty"` + + // DefaultListenerTLSSecretName, if provided, is the name of a + // pre-provisioned TLS certificate secret to use for the default HTTPS + // listener (named "default-https"). When set, this listener references + // the shared secret instead of requesting an individual certificate + // via cert-manager. + // + // The secret must exist in every downstream gateway namespace. + DefaultListenerTLSSecretName string `json:"defaultListenerTLSSecretName,omitempty"` +} + +// HasDefaultListenerTLSSecret returns true when a shared TLS certificate +// secret has been configured for default HTTPS listeners. +func (c *GatewayConfig) HasDefaultListenerTLSSecret() bool { + return c.DefaultListenerTLSSecretName != "" } // ShouldDeleteErroredChallenges returns whether the operator should automatically diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 1e516eb9..5db425a8 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -357,7 +357,12 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( var listeners []gatewayv1.Listener for listenerIndex, l := range upstreamGateway.Spec.Listeners { - if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" { + useSharedTLS := gatewayutil.IsDefaultListener(l) && r.Config.Gateway.HasDefaultListenerTLSSecret() + + // Only configure cert-manager annotations for listeners that need + // individual certificates (i.e. not default listeners using a shared + // wildcard certificate). + if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" && !useSharedTLS { if r.Config.Gateway.PerGatewayCertificateIssuer { if !metav1.HasAnnotation(downstreamGateway.ObjectMeta, "cert-manager.io/issuer") { metav1.SetMetaDataAnnotation(&downstreamGateway.ObjectMeta, "cert-manager.io/issuer", upstreamGateway.Name) @@ -387,7 +392,10 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( } } - // Add custom hostnames if they are verified + // Only include listeners whose hostname has been claimed. The default + // listener's UID-based hostname is always auto-claimed; custom hostnames + // must pass Domain verification first. The useSharedTLS flag only applies + // to the default listener. if l.Hostname != nil { if !slices.Contains(claimedHostnames, string(*l.Hostname)) { logger.Info("skipping downstream gateway listener with unclaimed hostname", "upstream_listener_index", listenerIndex, "hostname", *l.Hostname) @@ -399,17 +407,30 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( delete(listenerCopy.TLS.Options, certificateIssuerTLSOption) tlsMode := gatewayv1.TLSModeTerminate - listenerCopy.TLS = &gatewayv1.GatewayTLSConfig{ - Mode: &tlsMode, - // TODO(jreese) investigate secret deletion when Cert (gateway) is deleted - // See: https://cert-manager.io/docs/usage/certificate/#cleaning-up-secrets-when-certificates-are-deleted - CertificateRefs: []gatewayv1.SecretObjectReference{ - { - Group: ptr.To(gatewayv1.Group("")), - Kind: ptr.To(gatewayv1.Kind("Secret")), - Name: gatewayv1.ObjectName(resourcename.GetValidDNS1123Name(fmt.Sprintf("%s-%s", upstreamGateway.Name, l.Name))), + if useSharedTLS { + listenerCopy.TLS = &gatewayv1.GatewayTLSConfig{ + Mode: &tlsMode, + CertificateRefs: []gatewayv1.SecretObjectReference{ + { + Group: ptr.To(gatewayv1.Group("")), + Kind: ptr.To(gatewayv1.Kind("Secret")), + Name: gatewayv1.ObjectName(r.Config.Gateway.DefaultListenerTLSSecretName), + }, }, - }, + } + } else { + listenerCopy.TLS = &gatewayv1.GatewayTLSConfig{ + Mode: &tlsMode, + // TODO(jreese) investigate secret deletion when Cert (gateway) is deleted + // See: https://cert-manager.io/docs/usage/certificate/#cleaning-up-secrets-when-certificates-are-deleted + CertificateRefs: []gatewayv1.SecretObjectReference{ + { + Group: ptr.To(gatewayv1.Group("")), + Kind: ptr.To(gatewayv1.Kind("Secret")), + Name: gatewayv1.ObjectName(resourcename.GetValidDNS1123Name(fmt.Sprintf("%s-%s", upstreamGateway.Name, l.Name))), + }, + }, + } } } diff --git a/internal/controller/gateway_controller_test.go b/internal/controller/gateway_controller_test.go index 58fcdecb..a52ed74f 100644 --- a/internal/controller/gateway_controller_test.go +++ b/internal/controller/gateway_controller_test.go @@ -238,6 +238,249 @@ func TestEnsureDownstreamGateway(t *testing.T) { } } +func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { + testScheme := runtime.NewScheme() + assert.NoError(t, scheme.AddToScheme(testScheme)) + assert.NoError(t, gatewayv1.Install(testScheme)) + assert.NoError(t, discoveryv1.AddToScheme(testScheme)) + assert.NoError(t, networkingv1alpha.AddToScheme(testScheme)) + + upstreamNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + UID: uuid.NewUUID(), + }, + } + + defaultTLSSecretName := "wildcard-test-suite-tls" + + baseConfig := config.GatewayConfig{ + DownstreamGatewayClassName: "test-suite", + DownstreamHostnameAccountingNamespace: "default", + TargetDomain: "test-suite.com", + IPFamilies: []networkingv1alpha.IPFamily{ + networkingv1alpha.IPv4Protocol, + networkingv1alpha.IPv6Protocol, + }, + ListenerTLSOptions: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ + gatewayv1.AnnotationKey(certificateIssuerTLSOption): "test-issuer", + }, + } + + tests := []struct { + name string + defaultTLSSecretName string + upstreamGateway *gatewayv1.Gateway + existingUpstreamObjects []client.Object + existingDownstreamObjects []client.Object + assert func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) + }{ + { + name: "default https listener uses shared TLS secret", + defaultTLSSecretName: defaultTLSSecretName, + upstreamGateway: newGateway(config.NetworkServicesOperator{Gateway: baseConfig}, upstreamNamespace.Name, "test-gw", func(g *gatewayv1.Gateway) { + }), + assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { + httpsListener := gatewayutil.GetListenerByName( + downstreamGateway.Spec.Listeners, + gatewayutil.DefaultHTTPSListenerName, + ) + if assert.NotNil(t, httpsListener, "default HTTPS listener should exist on downstream gateway") { + if assert.NotNil(t, httpsListener.TLS, "TLS config should be set") { + if assert.Len(t, httpsListener.TLS.CertificateRefs, 1) { + assert.Equal(t, + gatewayv1.ObjectName(defaultTLSSecretName), + httpsListener.TLS.CertificateRefs[0].Name, + "default listener should reference the shared TLS secret", + ) + } + } + } + + assert.Empty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], + "cert-manager cluster-issuer annotation should not be set when only default listeners exist", + ) + }, + }, + { + name: "custom hostname gets individual cert with shared TLS secret enabled", + defaultTLSSecretName: defaultTLSSecretName, + upstreamGateway: newGateway(config.NetworkServicesOperator{Gateway: baseConfig}, upstreamNamespace.Name, "test-gw", func(g *gatewayv1.Gateway) { + g.Spec.Listeners = append(g.Spec.Listeners, + gatewayv1.Listener{ + Name: "https-hostname-0", + Protocol: gatewayv1.HTTPSProtocolType, + Port: DefaultHTTPSPort, + Hostname: ptr.To(gatewayv1.Hostname("custom.example.com")), + AllowedRoutes: &gatewayv1.AllowedRoutes{ + Namespaces: &gatewayv1.RouteNamespaces{ + From: ptr.To(gatewayv1.NamespacesFromSame), + }, + }, + TLS: &gatewayv1.GatewayTLSConfig{ + Mode: ptr.To(gatewayv1.TLSModeTerminate), + Options: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ + gatewayv1.AnnotationKey(certificateIssuerTLSOption): "test-issuer", + }, + }, + }, + ) + }), + existingUpstreamObjects: []client.Object{ + newDomain(upstreamNamespace.Name, "custom.example.com", func(d *networkingv1alpha.Domain) { + d.Spec.DomainName = "custom.example.com" + apimeta.SetStatusCondition(&d.Status.Conditions, metav1.Condition{ + Type: networkingv1alpha.DomainConditionVerified, + Status: metav1.ConditionTrue, + }) + }), + }, + assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { + // Default HTTPS listener should use shared TLS secret + httpsListener := gatewayutil.GetListenerByName( + downstreamGateway.Spec.Listeners, + gatewayutil.DefaultHTTPSListenerName, + ) + if assert.NotNil(t, httpsListener, "default HTTPS listener should exist") { + if assert.NotNil(t, httpsListener.TLS) { + if assert.Len(t, httpsListener.TLS.CertificateRefs, 1) { + assert.Equal(t, + gatewayv1.ObjectName(defaultTLSSecretName), + httpsListener.TLS.CertificateRefs[0].Name, + "default listener should reference the shared TLS secret", + ) + } + } + } + + // Custom hostname listener should get its own per-listener cert + customListener := gatewayutil.GetListenerByName( + downstreamGateway.Spec.Listeners, + "https-hostname-0", + ) + if assert.NotNil(t, customListener, "custom HTTPS listener should exist") { + if assert.NotNil(t, customListener.TLS) { + if assert.Len(t, customListener.TLS.CertificateRefs, 1) { + assert.Equal(t, + gatewayv1.ObjectName("test-gw-https-hostname-0"), + customListener.TLS.CertificateRefs[0].Name, + "custom listener should reference a per-listener cert secret", + ) + assert.NotEqual(t, + gatewayv1.ObjectName(defaultTLSSecretName), + customListener.TLS.CertificateRefs[0].Name, + "custom listener should NOT use the shared TLS secret", + ) + } + } + } + + // cert-manager annotations should be present for the custom hostname + assert.NotEmpty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], + "cert-manager annotation should be set for custom hostname listener", + ) + }, + }, + { + name: "without shared TLS secret all listeners get individual certs", + defaultTLSSecretName: "", + upstreamGateway: newGateway(config.NetworkServicesOperator{Gateway: baseConfig}, upstreamNamespace.Name, "test-gw", func(g *gatewayv1.Gateway) { + }), + assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { + httpsListener := gatewayutil.GetListenerByName( + downstreamGateway.Spec.Listeners, + gatewayutil.DefaultHTTPSListenerName, + ) + if assert.NotNil(t, httpsListener, "default HTTPS listener should exist") { + if assert.NotNil(t, httpsListener.TLS) { + if assert.Len(t, httpsListener.TLS.CertificateRefs, 1) { + assert.Equal(t, + gatewayv1.ObjectName("test-gw-default-https"), + httpsListener.TLS.CertificateRefs[0].Name, + "without wildcard config, default listener should get a per-listener cert", + ) + } + } + } + + assert.NotEmpty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], + "cert-manager annotation should be set when wildcard is not configured", + ) + }, + }, + } + + logger := zap.New(zap.UseFlagOptions(&zap.Options{Development: true})) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testCfg := config.NetworkServicesOperator{Gateway: *baseConfig.DeepCopy()} + testCfg.Gateway.DefaultListenerTLSSecretName = tt.defaultTLSSecretName + + tt.existingUpstreamObjects = append(tt.existingUpstreamObjects, &gatewayv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: gatewayv1.GatewayClassSpec{ + ControllerName: gatewayv1.GatewayController("test"), + }, + }) + + for _, obj := range append(tt.existingUpstreamObjects, tt.existingDownstreamObjects...) { + obj.SetUID(uuid.NewUUID()) + obj.SetCreationTimestamp(metav1.Now()) + } + + fakeUpstreamClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(tt.upstreamGateway, upstreamNamespace). + WithObjects(tt.existingUpstreamObjects...). + WithStatusSubresource(tt.upstreamGateway). + WithStatusSubresource(tt.existingUpstreamObjects...). + Build() + + fakeDownstreamClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(tt.existingDownstreamObjects...). + WithStatusSubresource(&gatewayv1.Gateway{}). + WithStatusSubresource(tt.existingDownstreamObjects...). + Build() + + ctx := context.Background() + ctx = log.IntoContext(ctx, logger) + + mgr := &fakeMockManager{cl: fakeUpstreamClient} + + reconciler := &GatewayReconciler{ + mgr: mgr, + Config: testCfg, + DownstreamCluster: &fakeCluster{cl: fakeDownstreamClient}, + } + + downstreamStrategy := downstreamclient.NewMappedNamespaceResourceStrategy("test", fakeUpstreamClient, fakeDownstreamClient) + + reconciler.prepareUpstreamGateway(tt.upstreamGateway) + result, downstreamGateway := reconciler.ensureDownstreamGateway( + ctx, + "test-suite", + fakeUpstreamClient, + tt.upstreamGateway, + downstreamStrategy, + ) + assert.NoError(t, result.Err, "failed ensuring downstream gateway") + + _, err := result.Complete(ctx) + assert.NoError(t, err, "failed completing result") + + if tt.assert != nil { + updatedUpstreamGateway := &gatewayv1.Gateway{} + assert.NoError(t, fakeUpstreamClient.Get(ctx, client.ObjectKeyFromObject(tt.upstreamGateway), updatedUpstreamGateway)) + tt.assert(t, updatedUpstreamGateway, downstreamGateway) + } + }) + } +} + func TestEnsureDownstreamGatewayHTTPRoutes(t *testing.T) { testScheme := runtime.NewScheme() assert.NoError(t, scheme.AddToScheme(testScheme)) diff --git a/internal/controller/trafficprotectionpolicy_controller.go b/internal/controller/trafficprotectionpolicy_controller.go index 2a09b05c..ad362fd0 100644 --- a/internal/controller/trafficprotectionpolicy_controller.go +++ b/internal/controller/trafficprotectionpolicy_controller.go @@ -38,6 +38,7 @@ import ( "go.datum.net/network-services-operator/internal/config" downstreamclient "go.datum.net/network-services-operator/internal/downstreamclient" gatewaystatus "go.datum.net/network-services-operator/internal/gatewayapi/status" + gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" "go.datum.net/network-services-operator/internal/util/resourcename" ) @@ -294,13 +295,18 @@ func (r *TrafficProtectionPolicyReconciler) checkHTTPSListenerCertificatesReady( ) (*certificateReadinessResult, error) { logger := log.FromContext(ctx) - // Collect unique HTTPS listeners from attachments + // Collect unique HTTPS listeners from attachments, skipping default + // listeners that use a shared TLS secret (which is pre-provisioned and + // doesn't have a per-listener Certificate resource). httpsListeners := make(map[string]struct{}) for _, attachment := range attachments { if attachment.Listener != nil { // Check if this specific listener is HTTPS for _, l := range attachment.Gateway.Spec.Listeners { if l.Name == *attachment.Listener && l.Protocol == gatewayv1.HTTPSProtocolType { + if gatewayutil.IsDefaultListener(l) && r.Config.Gateway.HasDefaultListenerTLSSecret() { + continue + } certName := resourcename.GetValidDNS1123Name(fmt.Sprintf("%s-%s", attachment.Gateway.Name, l.Name)) httpsListeners[certName] = struct{}{} } @@ -309,6 +315,9 @@ func (r *TrafficProtectionPolicyReconciler) checkHTTPSListenerCertificatesReady( // Policy targets all listeners on the gateway for _, l := range attachment.Gateway.Spec.Listeners { if l.Protocol == gatewayv1.HTTPSProtocolType { + if gatewayutil.IsDefaultListener(l) && r.Config.Gateway.HasDefaultListenerTLSSecret() { + continue + } certName := resourcename.GetValidDNS1123Name(fmt.Sprintf("%s-%s", attachment.Gateway.Name, l.Name)) httpsListeners[certName] = struct{}{} } From 7ac412a55f755d82dc7b83eecac1960669be2ca7 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 14:40:23 -0800 Subject: [PATCH 02/17] fix: rate limits --- cmd/main.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/main.go b/cmd/main.go index 4372f5f4..09cb0b02 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -121,6 +121,8 @@ func main() { // TODO(jreese) validate the config cfg := ctrl.GetConfigOrDie() + cfg.QPS = 10 + cfg.Burst = 20 deploymentCluster, err := cluster.New(cfg, func(o *cluster.Options) { o.Scheme = scheme @@ -187,6 +189,8 @@ func main() { setupLog.Error(err, "unable to load control plane kubeconfig") os.Exit(1) } + downstreamRestConfig.QPS = 10 + downstreamRestConfig.Burst = 20 downstreamCluster, err := cluster.New(downstreamRestConfig, func(o *cluster.Options) { o.Scheme = scheme @@ -439,11 +443,15 @@ func initializeClusterDiscovery( if err != nil { return nil, nil, fmt.Errorf("unable to get discovery rest config: %w", err) } + discoveryRestConfig.QPS = 50 + discoveryRestConfig.Burst = 100 projectRestConfig, err := serverConfig.Discovery.ProjectRestConfig() if err != nil { return nil, nil, fmt.Errorf("unable to get project rest config: %w", err) } + projectRestConfig.QPS = 10 + projectRestConfig.Burst = 20 discoveryManager, err := manager.New(discoveryRestConfig, manager.Options{ Client: client.Options{ From e1e97177143e66da649e248dcf457b99ca94bc22 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 14:42:53 -0800 Subject: [PATCH 03/17] fix: rate limits --- cmd/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 09cb0b02..00d9ebd4 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -443,8 +443,8 @@ func initializeClusterDiscovery( if err != nil { return nil, nil, fmt.Errorf("unable to get discovery rest config: %w", err) } - discoveryRestConfig.QPS = 50 - discoveryRestConfig.Burst = 100 + discoveryRestConfig.QPS = 10 + discoveryRestConfig.Burst = 20 projectRestConfig, err := serverConfig.Discovery.ProjectRestConfig() if err != nil { From 7ad052667a6444974141478aa1453fe65047bf64 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 14:51:36 -0800 Subject: [PATCH 04/17] fix: rate limits --- cmd/main.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 00d9ebd4..60be74bb 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -121,8 +121,8 @@ func main() { // TODO(jreese) validate the config cfg := ctrl.GetConfigOrDie() - cfg.QPS = 10 - cfg.Burst = 20 + cfg.QPS = 50 + cfg.Burst = 100 deploymentCluster, err := cluster.New(cfg, func(o *cluster.Options) { o.Scheme = scheme @@ -189,8 +189,8 @@ func main() { setupLog.Error(err, "unable to load control plane kubeconfig") os.Exit(1) } - downstreamRestConfig.QPS = 10 - downstreamRestConfig.Burst = 20 + downstreamRestConfig.QPS = 50 + downstreamRestConfig.Burst = 100 downstreamCluster, err := cluster.New(downstreamRestConfig, func(o *cluster.Options) { o.Scheme = scheme @@ -443,15 +443,15 @@ func initializeClusterDiscovery( if err != nil { return nil, nil, fmt.Errorf("unable to get discovery rest config: %w", err) } - discoveryRestConfig.QPS = 10 - discoveryRestConfig.Burst = 20 + discoveryRestConfig.QPS = 50 + discoveryRestConfig.Burst = 100 projectRestConfig, err := serverConfig.Discovery.ProjectRestConfig() if err != nil { return nil, nil, fmt.Errorf("unable to get project rest config: %w", err) } - projectRestConfig.QPS = 10 - projectRestConfig.Burst = 20 + projectRestConfig.QPS = 50 + projectRestConfig.Burst = 100 discoveryManager, err := manager.New(discoveryRestConfig, manager.Options{ Client: client.Options{ From a4eb38848086ff3cf4e07062d632901ca8c1cc86 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 16:16:57 -0800 Subject: [PATCH 05/17] clean up stale annotations --- internal/controller/gateway_controller.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 721a056a..e557e115 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -269,6 +269,22 @@ func (r *GatewayReconciler) ensureDownstreamGateway( return result, nil } } else { + // Remove cert-manager annotations when the desired state no longer + // includes them (e.g. all listeners switched to a shared TLS secret). + // maps.Copy is additive, so stale annotations must be pruned explicitly + // to prevent cert-manager from overwriting the shared secret with a + // per-listener certificate. + for _, key := range []string{ + "cert-manager.io/cluster-issuer", + "cert-manager.io/secret-template", + } { + if _, existsOnCurrent := downstreamGateway.Annotations[key]; existsOnCurrent { + if _, existsOnDesired := desiredDownstreamGateway.Annotations[key]; !existsOnDesired { + delete(downstreamGateway.Annotations, key) + } + } + } + if !equality.Semantic.DeepEqual(downstreamGateway.Annotations, desiredDownstreamGateway.Annotations) || !equality.Semantic.DeepEqual(downstreamGateway.Spec, desiredDownstreamGateway.Spec) { // Take care not to clobber other annotations From f27034c9cba48ce93c5ee3653f2a5a151aeebc4d Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 16:30:27 -0800 Subject: [PATCH 06/17] fix: check for custom listeners --- internal/controller/gateway_controller.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index e557e115..0c0917cd 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -372,8 +372,21 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( var listeners []gatewayv1.Listener + // When any listener requires an individual certificate (custom hostname), + // the gateway-level cert-manager annotation must be set. Since cert-manager + // processes ALL HTTPS listeners on an annotated gateway, the default listener + // must fall back to a per-listener cert ref to prevent cert-manager from + // overwriting the shared wildcard secret. + hasCustomTLSListeners := false + for _, l := range upstreamGateway.Spec.Listeners { + if !gatewayutil.IsDefaultListener(l) && l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" { + hasCustomTLSListeners = true + break + } + } + for listenerIndex, l := range upstreamGateway.Spec.Listeners { - useSharedTLS := gatewayutil.IsDefaultListener(l) && r.Config.Gateway.HasDefaultListenerTLSSecret() + useSharedTLS := gatewayutil.IsDefaultListener(l) && r.Config.Gateway.HasDefaultListenerTLSSecret() && !hasCustomTLSListeners // Only configure cert-manager annotations for listeners that need // individual certificates (i.e. not default listeners using a shared From 69fc156cf2c35473bde1d91ed0a209755253c97c Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 16:41:07 -0800 Subject: [PATCH 07/17] fix: annotation check --- internal/controller/gateway_controller.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 0c0917cd..c0d5bb70 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -274,6 +274,7 @@ func (r *GatewayReconciler) ensureDownstreamGateway( // maps.Copy is additive, so stale annotations must be pruned explicitly // to prevent cert-manager from overwriting the shared secret with a // per-listener certificate. + annotationsChanged := false for _, key := range []string{ "cert-manager.io/cluster-issuer", "cert-manager.io/secret-template", @@ -281,11 +282,13 @@ func (r *GatewayReconciler) ensureDownstreamGateway( if _, existsOnCurrent := downstreamGateway.Annotations[key]; existsOnCurrent { if _, existsOnDesired := desiredDownstreamGateway.Annotations[key]; !existsOnDesired { delete(downstreamGateway.Annotations, key) + annotationsChanged = true } } } - if !equality.Semantic.DeepEqual(downstreamGateway.Annotations, desiredDownstreamGateway.Annotations) || + if annotationsChanged || + !equality.Semantic.DeepEqual(downstreamGateway.Annotations, desiredDownstreamGateway.Annotations) || !equality.Semantic.DeepEqual(downstreamGateway.Spec, desiredDownstreamGateway.Spec) { // Take care not to clobber other annotations maps.Copy(downstreamGateway.Annotations, desiredDownstreamGateway.Annotations) From ec43ffca3364362425c58f75bd2de10440163a25 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 16:55:09 -0800 Subject: [PATCH 08/17] fix: tests --- internal/controller/gateway_controller_test.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/internal/controller/gateway_controller_test.go b/internal/controller/gateway_controller_test.go index a52ed74f..77cd0bad 100644 --- a/internal/controller/gateway_controller_test.go +++ b/internal/controller/gateway_controller_test.go @@ -336,7 +336,9 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { }), }, assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { - // Default HTTPS listener should use shared TLS secret + // When custom HTTPS listeners exist, all listeners fall back to + // per-listener certs to prevent cert-manager from overwriting + // the shared wildcard secret. httpsListener := gatewayutil.GetListenerByName( downstreamGateway.Spec.Listeners, gatewayutil.DefaultHTTPSListenerName, @@ -345,15 +347,14 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { if assert.NotNil(t, httpsListener.TLS) { if assert.Len(t, httpsListener.TLS.CertificateRefs, 1) { assert.Equal(t, - gatewayv1.ObjectName(defaultTLSSecretName), + gatewayv1.ObjectName("test-gw-default-https"), httpsListener.TLS.CertificateRefs[0].Name, - "default listener should reference the shared TLS secret", + "default listener should fall back to per-listener cert when custom listeners exist", ) } } } - // Custom hostname listener should get its own per-listener cert customListener := gatewayutil.GetListenerByName( downstreamGateway.Spec.Listeners, "https-hostname-0", @@ -366,18 +367,12 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { customListener.TLS.CertificateRefs[0].Name, "custom listener should reference a per-listener cert secret", ) - assert.NotEqual(t, - gatewayv1.ObjectName(defaultTLSSecretName), - customListener.TLS.CertificateRefs[0].Name, - "custom listener should NOT use the shared TLS secret", - ) } } } - // cert-manager annotations should be present for the custom hostname assert.NotEmpty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], - "cert-manager annotation should be set for custom hostname listener", + "cert-manager annotation should be set when custom hostname listeners exist", ) }, }, From f2985c0fe9bacfad67fbaa29a814cdce1e180d0c Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 17:06:06 -0800 Subject: [PATCH 09/17] fix: remove issuer annotation when in shared mode as well --- internal/controller/gateway_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index c0d5bb70..13fc23fc 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -276,6 +276,7 @@ func (r *GatewayReconciler) ensureDownstreamGateway( // per-listener certificate. annotationsChanged := false for _, key := range []string{ + "cert-manager.io/issuer", "cert-manager.io/cluster-issuer", "cert-manager.io/secret-template", } { From 9dfa785eb304d3140cec96ad1a8eef280451f689 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 17:49:32 -0800 Subject: [PATCH 10/17] fix: make service only patch changed spec fields it cares about --- internal/controller/gateway_controller.go | 46 +++++++++--- .../controller/gateway_controller_test.go | 73 +++++++++++++++++++ 2 files changed, 108 insertions(+), 11 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 13fc23fc..411b3f2c 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -376,21 +376,40 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( var listeners []gatewayv1.Listener - // When any listener requires an individual certificate (custom hostname), - // the gateway-level cert-manager annotation must be set. Since cert-manager - // processes ALL HTTPS listeners on an annotated gateway, the default listener - // must fall back to a per-listener cert ref to prevent cert-manager from - // overwriting the shared wildcard secret. - hasCustomTLSListeners := false + // Determine whether any listener on this gateway requires an individual + // certificate that cert-manager must issue. A listener needs its own cert + // only when its hostname falls outside the wildcard's scope (i.e. it's not + // a subdomain of the target domain). Hostnames covered by the wildcard + // (*.targetDomain) can share the pre-provisioned secret. + // + // When any listener requires cert-manager, the gateway-level annotation + // must be set. Since cert-manager processes ALL HTTPS listeners on an + // annotated gateway, every listener that would otherwise use the shared + // secret must fall back to a per-listener cert ref to prevent cert-manager + // from overwriting the wildcard secret. + wildcardSuffix := "." + r.Config.Gateway.TargetDomain + hasExternalTLSListeners := false for _, l := range upstreamGateway.Spec.Listeners { - if !gatewayutil.IsDefaultListener(l) && l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" { - hasCustomTLSListeners = true - break + if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" && l.Hostname != nil { + hostname := string(*l.Hostname) + if !strings.HasSuffix(hostname, wildcardSuffix) && hostname != r.Config.Gateway.TargetDomain { + hasExternalTLSListeners = true + break + } } } for listenerIndex, l := range upstreamGateway.Spec.Listeners { - useSharedTLS := gatewayutil.IsDefaultListener(l) && r.Config.Gateway.HasDefaultListenerTLSSecret() && !hasCustomTLSListeners + // A listener can use the shared wildcard when: + // 1. Its hostname is under the target domain (covered by the wildcard) + // 2. The shared TLS secret is configured + // 3. No external hostname on this gateway forces cert-manager annotations + hostnameUnderWildcard := false + if l.Hostname != nil { + h := string(*l.Hostname) + hostnameUnderWildcard = strings.HasSuffix(h, wildcardSuffix) || h == r.Config.Gateway.TargetDomain + } + useSharedTLS := hostnameUnderWildcard && r.Config.Gateway.HasDefaultListenerTLSSecret() && !hasExternalTLSListeners // Only configure cert-manager annotations for listeners that need // individual certificates (i.e. not default listeners using a shared @@ -1294,7 +1313,12 @@ func (r *GatewayReconciler) ensureDownstreamHTTPRoute( resourceResult, err := controllerutil.CreateOrUpdate(ctx, downstreamClient, resource, func() error { switch obj := resource.(type) { case *corev1.Service: - obj.Spec = desiredDownstreamResource.(*corev1.Service).Spec + desired := desiredDownstreamResource.(*corev1.Service) + obj.Spec.Type = desired.Spec.Type + obj.Spec.ClusterIP = desired.Spec.ClusterIP + obj.Spec.Ports = desired.Spec.Ports + obj.Spec.InternalTrafficPolicy = desired.Spec.InternalTrafficPolicy + obj.Spec.TrafficDistribution = desired.Spec.TrafficDistribution case *discoveryv1.EndpointSlice: desiredEndpointSlice := desiredDownstreamResource.(*discoveryv1.EndpointSlice) // Since endpointslices get duplicated for routes, add them as a controller diff --git a/internal/controller/gateway_controller_test.go b/internal/controller/gateway_controller_test.go index 77cd0bad..9324cfc7 100644 --- a/internal/controller/gateway_controller_test.go +++ b/internal/controller/gateway_controller_test.go @@ -376,6 +376,79 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { ) }, }, + { + name: "subdomain of target domain uses shared wildcard cert", + defaultTLSSecretName: defaultTLSSecretName, + upstreamGateway: newGateway(config.NetworkServicesOperator{Gateway: baseConfig}, upstreamNamespace.Name, "test-gw", func(g *gatewayv1.Gateway) { + g.Spec.Listeners = append(g.Spec.Listeners, + gatewayv1.Listener{ + Name: "https-hostname-0", + Protocol: gatewayv1.HTTPSProtocolType, + Port: DefaultHTTPSPort, + Hostname: ptr.To(gatewayv1.Hostname("canary.test-suite.com")), + AllowedRoutes: &gatewayv1.AllowedRoutes{ + Namespaces: &gatewayv1.RouteNamespaces{ + From: ptr.To(gatewayv1.NamespacesFromSame), + }, + }, + TLS: &gatewayv1.GatewayTLSConfig{ + Mode: ptr.To(gatewayv1.TLSModeTerminate), + Options: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ + gatewayv1.AnnotationKey(certificateIssuerTLSOption): "test-issuer", + }, + }, + }, + ) + }), + existingUpstreamObjects: []client.Object{ + newDomain(upstreamNamespace.Name, "canary.test-suite.com", func(d *networkingv1alpha.Domain) { + d.Spec.DomainName = "canary.test-suite.com" + apimeta.SetStatusCondition(&d.Status.Conditions, metav1.Condition{ + Type: networkingv1alpha.DomainConditionVerified, + Status: metav1.ConditionTrue, + }) + }), + }, + assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { + // Both default and subdomain listeners are covered by the wildcard, + // so both should reference the shared TLS secret. + httpsListener := gatewayutil.GetListenerByName( + downstreamGateway.Spec.Listeners, + gatewayutil.DefaultHTTPSListenerName, + ) + if assert.NotNil(t, httpsListener, "default HTTPS listener should exist") { + if assert.NotNil(t, httpsListener.TLS) { + if assert.Len(t, httpsListener.TLS.CertificateRefs, 1) { + assert.Equal(t, + gatewayv1.ObjectName(defaultTLSSecretName), + httpsListener.TLS.CertificateRefs[0].Name, + "default listener should use shared wildcard cert", + ) + } + } + } + + customListener := gatewayutil.GetListenerByName( + downstreamGateway.Spec.Listeners, + "https-hostname-0", + ) + if assert.NotNil(t, customListener, "subdomain listener should exist") { + if assert.NotNil(t, customListener.TLS) { + if assert.Len(t, customListener.TLS.CertificateRefs, 1) { + assert.Equal(t, + gatewayv1.ObjectName(defaultTLSSecretName), + customListener.TLS.CertificateRefs[0].Name, + "subdomain listener covered by wildcard should use shared cert", + ) + } + } + } + + assert.Empty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], + "cert-manager annotation should NOT be set when all hostnames are under the wildcard", + ) + }, + }, { name: "without shared TLS secret all listeners get individual certs", defaultTLSSecretName: "", From 9fde082111014eb77d9472f2956dcf7d4152350a Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 22:43:56 -0800 Subject: [PATCH 11/17] fix: continue cleaning up dupe reconciles --- internal/controller/gateway_controller.go | 18 +- .../controller/gateway_controller_test.go | 155 ++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index fb313a28..674e2fe7 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -1317,9 +1317,25 @@ func (r *GatewayReconciler) ensureDownstreamHTTPRoute( desired := desiredDownstreamResource.(*corev1.Service) obj.Spec.Type = desired.Spec.Type obj.Spec.ClusterIP = desired.Spec.ClusterIP - obj.Spec.Ports = desired.Spec.Ports obj.Spec.InternalTrafficPolicy = desired.Spec.InternalTrafficPolicy obj.Spec.TrafficDistribution = desired.Spec.TrafficDistribution + + // Merge ports by name rather than overwriting the slice, so + // server-defaulted fields like TargetPort are preserved. + for _, dp := range desired.Spec.Ports { + found := false + for i, ep := range obj.Spec.Ports { + if ep.Name == dp.Name { + obj.Spec.Ports[i].Port = dp.Port + obj.Spec.Ports[i].Protocol = dp.Protocol + found = true + break + } + } + if !found { + obj.Spec.Ports = append(obj.Spec.Ports, dp) + } + } case *discoveryv1.EndpointSlice: desiredEndpointSlice := desiredDownstreamResource.(*discoveryv1.EndpointSlice) // Since endpointslices get duplicated for routes, add them as a controller diff --git a/internal/controller/gateway_controller_test.go b/internal/controller/gateway_controller_test.go index 9324cfc7..29a44b41 100644 --- a/internal/controller/gateway_controller_test.go +++ b/internal/controller/gateway_controller_test.go @@ -7,16 +7,19 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -1134,3 +1137,155 @@ func newHTTPRoute(namespace, name string, opts ...func(*gatewayv1.HTTPRoute)) *g return route } + +// TestServiceSpecClobberingCausesReconciliationStorm proves that overwriting the +// entire Service.Spec during CreateOrUpdate causes spurious updates when the +// existing Service contains server-defaulted fields that the desired spec omits. +// This is the root cause of the gateway controller reconciliation storm: every +// reconciliation produces an "updated" result, which re-enqueues the gateway, +// creating an infinite loop. +func TestServiceSpecClobberingCausesReconciliationStorm(t *testing.T) { + testScheme := runtime.NewScheme() + require.NoError(t, scheme.AddToScheme(testScheme)) + + // This is what the NSO controller builds as the desired Service — it only + // sets the fields it cares about. + desiredService := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-svc", + Namespace: "default", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + ClusterIP: "None", + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: 80, + Protocol: corev1.ProtocolTCP, + }, + }, + InternalTrafficPolicy: ptr.To(corev1.ServiceInternalTrafficPolicyCluster), + }, + } + + // This is what the API server returns after creating the Service — it has + // server-defaulted fields like sessionAffinity, ipFamilyPolicy, ipFamilies, + // clusterIPs, and port targetPort that the controller never explicitly sets. + existingService := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-svc", + Namespace: "default", + ResourceVersion: "1", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + ClusterIP: "None", + ClusterIPs: []string{ + "None", + }, + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: 80, + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromInt32(80), + }, + }, + InternalTrafficPolicy: ptr.To(corev1.ServiceInternalTrafficPolicyCluster), + SessionAffinity: corev1.ServiceAffinityNone, + IPFamilies: []corev1.IPFamily{corev1.IPv4Protocol}, + IPFamilyPolicy: ptr.To(corev1.IPFamilyPolicySingleStack), + }, + } + + t.Run("whole_spec_overwrite_causes_perpetual_updates", func(t *testing.T) { + cl := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(existingService.DeepCopy()). + Build() + + svc := desiredService.DeepCopy() + result, err := controllerutil.CreateOrUpdate(context.Background(), cl, svc, func() error { + svc.Spec = desiredService.Spec + return nil + }) + require.NoError(t, err) + assert.Equal(t, controllerutil.OperationResultUpdated, result, + "BUG: whole-spec overwrite returns 'updated' even when intent is unchanged, "+ + "because server-defaulted fields (sessionAffinity, ipFamilies, clusterIPs, "+ + "targetPort, ipFamilyPolicy) are stripped") + + // On a real API server, the server re-defaults these fields on every + // write, so the next GET returns the full spec again and the cycle + // repeats forever — creating an infinite reconciliation storm. + }) + + t.Run("field_level_patching_preserves_server_defaults", func(t *testing.T) { + cl := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(existingService.DeepCopy()). + Build() + + svc := desiredService.DeepCopy() + result, err := controllerutil.CreateOrUpdate(context.Background(), cl, svc, func() error { + desired := desiredService + svc.Spec.Type = desired.Spec.Type + svc.Spec.ClusterIP = desired.Spec.ClusterIP + svc.Spec.InternalTrafficPolicy = desired.Spec.InternalTrafficPolicy + svc.Spec.TrafficDistribution = desired.Spec.TrafficDistribution + + // Merge ports: update matching ports by name, preserving + // server-defaulted fields like TargetPort. + for _, dp := range desired.Spec.Ports { + found := false + for i, ep := range svc.Spec.Ports { + if ep.Name == dp.Name { + svc.Spec.Ports[i].Port = dp.Port + svc.Spec.Ports[i].Protocol = dp.Protocol + found = true + break + } + } + if !found { + svc.Spec.Ports = append(svc.Spec.Ports, dp) + } + } + + return nil + }) + require.NoError(t, err) + assert.Equal(t, controllerutil.OperationResultNone, result, + "FIX: field-level patching returns 'unchanged' because server-defaulted "+ + "fields (sessionAffinity, ipFamilies, clusterIPs, targetPort, ipFamilyPolicy) are preserved") + + // Run it a second time to prove idempotency. + result, err = controllerutil.CreateOrUpdate(context.Background(), cl, svc, func() error { + desired := desiredService + svc.Spec.Type = desired.Spec.Type + svc.Spec.ClusterIP = desired.Spec.ClusterIP + svc.Spec.InternalTrafficPolicy = desired.Spec.InternalTrafficPolicy + svc.Spec.TrafficDistribution = desired.Spec.TrafficDistribution + + for _, dp := range desired.Spec.Ports { + found := false + for i, ep := range svc.Spec.Ports { + if ep.Name == dp.Name { + svc.Spec.Ports[i].Port = dp.Port + svc.Spec.Ports[i].Protocol = dp.Protocol + found = true + break + } + } + if !found { + svc.Spec.Ports = append(svc.Spec.Ports, dp) + } + } + + return nil + }) + require.NoError(t, err) + assert.Equal(t, controllerutil.OperationResultNone, result, + "idempotent: second call also returns 'unchanged'") + }) +} From 7a014b373dc2117b472f3a766f82d2a3d3e6c931 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 26 Feb 2026 23:35:14 -0800 Subject: [PATCH 12/17] add logs --- internal/controller/gateway_controller.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 674e2fe7..ed26ef23 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -89,16 +89,23 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req mcreconcile.Reque logger := log.FromContext(ctx, "cluster", req.ClusterName, "namespace", req.Namespace, "name", req.Name) ctx = log.IntoContext(ctx, logger) + logger.Info("gateway reconcile dequeued") + cl, err := r.mgr.GetCluster(ctx, req.ClusterName) if err != nil { + logger.Error(err, "failed to get cluster") return ctrl.Result{}, err } + logger.Info("got cluster, fetching gateway") + var gateway gatewayv1.Gateway if err := cl.GetClient().Get(ctx, req.NamespacedName, &gateway); err != nil { if apierrors.IsNotFound(err) { + logger.Info("gateway not found, skipping") return ctrl.Result{}, nil } + logger.Error(err, "failed to get gateway") return ctrl.Result{}, err } @@ -106,12 +113,15 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req mcreconcile.Reque var upstreamGatewayClass gatewayv1.GatewayClass if err := cl.GetClient().Get(ctx, types.NamespacedName{Name: string(gateway.Spec.GatewayClassName)}, &upstreamGatewayClass); err != nil { if apierrors.IsNotFound(err) { + logger.Info("gateway class not found, skipping") return ctrl.Result{}, nil } + logger.Error(err, "failed to get gateway class") return ctrl.Result{}, err } if upstreamGatewayClass.Spec.ControllerName != r.Config.Gateway.ControllerName { + logger.Info("gateway class controller name mismatch, skipping", "expected", r.Config.Gateway.ControllerName, "actual", upstreamGatewayClass.Spec.ControllerName) return ctrl.Result{}, nil } @@ -133,7 +143,9 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req mcreconcile.Reque } if r.prepareUpstreamGateway(&gateway) { + logger.Info("preparing upstream gateway (adding finalizer/defaults)") if err := cl.GetClient().Update(ctx, &gateway); err != nil { + logger.Error(err, "failed preparing upstream gateway") return ctrl.Result{}, fmt.Errorf("failed preparing upstream gateway: %w", err) } From 6a6a592855e03dedbcb55722ba4f479dfa6688b2 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Fri, 27 Feb 2026 11:44:42 -0800 Subject: [PATCH 13/17] fix: wildcard cert --- internal/controller/gateway_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 42bbb122..1d4517cb 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -373,8 +373,8 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( var listeners []gatewayv1.Listener - // Determine whether any listener on this gateway requires an individual - // certificate that cert-manager must issue. A listener needs its own cert + // Determine whether any claimed listener on this gateway requires an + // individual certificate from cert-manager. A listener needs its own cert // only when its hostname falls outside the wildcard's scope (i.e. it's not // a subdomain of the target domain). Hostnames covered by the wildcard // (*.targetDomain) can share the pre-provisioned secret. From 0e9b00ea5e7ca3f280f5524962d5fc6ffa8ebfef Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Fri, 27 Feb 2026 13:21:53 -0800 Subject: [PATCH 14/17] feat: gateway controller certificates --- cmd/main.go | 2 + config/rbac/role.yaml | 4 + internal/config/config.go | 11 - internal/controller/challenge_controller.go | 11 +- .../controller/challenge_controller_test.go | 26 +- internal/controller/gateway_controller.go | 292 +++++++++++++----- .../controller/gateway_controller_test.go | 163 ++-------- 7 files changed, 250 insertions(+), 259 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 60be74bb..1e6e414d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -13,6 +13,7 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" cmacmev1 "github.com/cert-manager/cert-manager/pkg/apis/acme/v1" + cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" envoygatewayv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1" multiclusterproviders "go.miloapis.com/milo/pkg/multicluster-runtime" milomulticluster "go.miloapis.com/milo/pkg/multicluster-runtime/milo" @@ -65,6 +66,7 @@ func init() { utilruntime.Must(envoygatewayv1alpha1.AddToScheme(scheme)) utilruntime.Must(networkingv1alpha1.AddToScheme(scheme)) utilruntime.Must(cmacmev1.AddToScheme(scheme)) + utilruntime.Must(cmv1.AddToScheme(scheme)) utilruntime.Must(dnsv1alpha1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 82e382b1..c4cef16e 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -52,8 +52,12 @@ rules: resources: - certificates verbs: + - create + - delete - get - list + - patch + - update - watch - apiGroups: - coordination.k8s.io diff --git a/internal/config/config.go b/internal/config/config.go index ee81d6e6..42acb6bb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -486,17 +486,6 @@ type GatewayConfig struct { // issuer name, the operator will use the value as is. ClusterIssuerMap map[string]string `json:"clusterIssuerMap,omitempty"` - // PerGatewayCertificateIssuer will result in the operator to expect a - // cert-manager Issuer to exist with the same name as the gateway. Any value - // provided for the "gateway.networking.datumapis.com/certificate-issuer" - // option will be replaced with the gateway's name. The Issuer resources will - // be managed by Kyverno policies, and not by this operator. - // - // TODO(jreese) Remove this once we've either implemented DNS validation, - // found a path to attach cert-manager generated routes to the gateway they're - // needed for, or implement our own ACME integration. - PerGatewayCertificateIssuer bool `json:"perGatewayCertificateIssuer,omitempty"` - // ListenerTLSOptions specifies the TLS options to program on generated // TLS listeners. // +default={"gateway.networking.datumapis.com/certificate-issuer": "auto"} diff --git a/internal/controller/challenge_controller.go b/internal/controller/challenge_controller.go index c1fdfd7c..a6d77255 100644 --- a/internal/controller/challenge_controller.go +++ b/internal/controller/challenge_controller.go @@ -81,13 +81,9 @@ func (r *ChallengeReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } // isGatewayRelatedIssuer checks if the given issuer reference is for a Gateway-managed -// certificate issuer. This includes: -// - ClusterIssuers that are mapped in the ClusterIssuerMap configuration -// - Issuers (namespace-scoped) when PerGatewayCertificateIssuer mode is enabled +// certificate issuer (ClusterIssuers that are mapped in the ClusterIssuerMap configuration). func (r *ChallengeReconciler) isGatewayRelatedIssuer(ref cmmeta.ObjectReference) bool { - // Check if ClusterIssuer is in the configured map if ref.Kind == "ClusterIssuer" || ref.Kind == "" { - // Default kind is ClusterIssuer for cert-manager for _, mappedIssuer := range r.Config.Gateway.ClusterIssuerMap { if mappedIssuer == ref.Name { return true @@ -95,11 +91,6 @@ func (r *ChallengeReconciler) isGatewayRelatedIssuer(ref cmmeta.ObjectReference) } } - // In per-gateway mode, any namespace-scoped Issuer is gateway-related - if r.Config.Gateway.PerGatewayCertificateIssuer && ref.Kind == "Issuer" { - return true - } - return false } diff --git a/internal/controller/challenge_controller_test.go b/internal/controller/challenge_controller_test.go index 7464c800..ee69665b 100644 --- a/internal/controller/challenge_controller_test.go +++ b/internal/controller/challenge_controller_test.go @@ -75,7 +75,7 @@ func TestChallengeReconciler(t *testing.T) { expectDeleted: true, }, { - name: "errored challenge with Issuer in per-gateway mode is deleted", + name: "errored challenge with namespace-scoped Issuer is not deleted", challenge: &cmacmev1.Challenge{ ObjectMeta: metav1.ObjectMeta{ Namespace: upstreamNamespace.Name, @@ -95,11 +95,9 @@ func TestChallengeReconciler(t *testing.T) { }, }, config: config.NetworkServicesOperator{ - Gateway: config.GatewayConfig{ - PerGatewayCertificateIssuer: true, - }, + Gateway: config.GatewayConfig{}, }, - expectDeleted: true, + expectDeleted: false, }, { name: "errored challenge with non-gateway ClusterIssuer is not deleted", @@ -258,7 +256,6 @@ func TestChallengeReconciler(t *testing.T) { }, config: config.NetworkServicesOperator{ Gateway: config.GatewayConfig{ - PerGatewayCertificateIssuer: false, ClusterIssuerMap: map[string]string{ "letsencrypt": "letsencrypt-prod", }, @@ -368,25 +365,12 @@ func TestIsGatewayRelatedIssuer(t *testing.T) { expected: true, }, { - name: "Issuer in per-gateway mode returns true", - ref: cmmeta.ObjectReference{ - Kind: "Issuer", - Name: "gateway-issuer", - }, - config: config.GatewayConfig{ - PerGatewayCertificateIssuer: true, - }, - expected: true, - }, - { - name: "Issuer not in per-gateway mode returns false", + name: "namespace-scoped Issuer returns false", ref: cmmeta.ObjectReference{ Kind: "Issuer", Name: "gateway-issuer", }, - config: config.GatewayConfig{ - PerGatewayCertificateIssuer: false, - }, + config: config.GatewayConfig{}, expected: false, }, { diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 1d4517cb..8131273e 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -10,6 +10,8 @@ import ( "strings" "time" + cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" envoygatewayv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" @@ -81,6 +83,8 @@ type GatewayReconciler struct { // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=backendtlspolicies/status,verbs=get;update;patch // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=backendtlspolicies/finalizers,verbs=update +// +kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch;create;update;patch;delete + // +kubebuilder:rbac:groups=externaldns.k8s.io,resources=dnsendpoints,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=externaldns.k8s.io,resources=dnsendpoints/status,verbs=get;update;patch // +kubebuilder:rbac:groups=externaldns.k8s.io,resources=dnsendpoints/finalizers,verbs=update @@ -283,12 +287,15 @@ func (r *GatewayReconciler) ensureDownstreamGateway( return result, nil } } else { - // Sync cert-manager annotations: add desired, remove stale - syncCertManagerAnnotations(downstreamGateway, desiredDownstreamGateway) + // Run the two-phase cert-manager annotation migration before diffing. + // This mutates downstreamGateway.Annotations in place, which the diff + // check below will pick up and persist via Update. + migrationRequeue := migrateLegacyCertManagerAnnotations(downstreamGateway) if !equality.Semantic.DeepEqual(downstreamGateway.Annotations, desiredDownstreamGateway.Annotations) || !equality.Semantic.DeepEqual(downstreamGateway.Spec, desiredDownstreamGateway.Spec) { - // Take care not to clobber other annotations + // maps.Copy merges desired over existing; migration-added keys + // (like the phase marker) survive because they aren't in desired. maps.Copy(downstreamGateway.Annotations, desiredDownstreamGateway.Annotations) downstreamGateway.Spec = desiredDownstreamGateway.Spec if err := downstreamClient.Update(ctx, downstreamGateway); err != nil { @@ -296,6 +303,22 @@ func (r *GatewayReconciler) ensureDownstreamGateway( return result, nil } } + + if migrationRequeue > 0 { + result.RequeueAfter = migrationRequeue + } + } + + certResult := r.ensureListenerCertificates( + ctx, + upstreamClusterName, + upstreamGateway, + downstreamGateway, + downstreamClient, + claimedHostnames, + ) + if certResult.ShouldReturn() { + return certResult.Merge(result), nil } dnsResult := r.ensureDownstreamGatewayDNSEndpoints( @@ -373,85 +396,28 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( var listeners []gatewayv1.Listener - // Determine whether any claimed listener on this gateway requires an - // individual certificate from cert-manager. A listener needs its own cert - // only when its hostname falls outside the wildcard's scope (i.e. it's not - // a subdomain of the target domain). Hostnames covered by the wildcard - // (*.targetDomain) can share the pre-provisioned secret. - // - // When any listener requires cert-manager, the gateway-level annotation - // must be set. Since cert-manager processes ALL HTTPS listeners on an - // annotated gateway, every listener that would otherwise use the shared - // secret must fall back to a per-listener cert ref to prevent cert-manager - // from overwriting the wildcard secret. wildcardSuffix := "." + r.Config.Gateway.TargetDomain - hasExternalTLSListeners := false - for _, l := range upstreamGateway.Spec.Listeners { - if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" && l.Hostname != nil { - hostname := string(*l.Hostname) - if !slices.Contains(claimedHostnames, hostname) { - continue - } - if !strings.HasSuffix(hostname, wildcardSuffix) && hostname != r.Config.Gateway.TargetDomain { - hasExternalTLSListeners = true - break - } - } - } for listenerIndex, l := range upstreamGateway.Spec.Listeners { - // Skip unclaimed custom hostnames — they must not influence cert-manager - // annotations or downstream listener configuration. if l.Hostname != nil && !slices.Contains(claimedHostnames, string(*l.Hostname)) { logger.Info("skipping downstream gateway listener with unclaimed hostname", "upstream_listener_index", listenerIndex, "hostname", *l.Hostname) continue } - // A listener can use the shared wildcard when: - // 1. Its hostname is under the target domain (covered by the wildcard) - // 2. The shared TLS secret is configured - // 3. No claimed external hostname on this gateway forces cert-manager annotations + // Per-listener TLS decision: hostnames covered by the wildcard + // (*.targetDomain) reference the pre-provisioned shared secret; + // all others reference a per-listener secret populated by a + // Certificate resource created in ensureListenerCertificates. hostnameUnderWildcard := false if l.Hostname != nil { h := string(*l.Hostname) hostnameUnderWildcard = strings.HasSuffix(h, wildcardSuffix) || h == r.Config.Gateway.TargetDomain } - useSharedTLS := hostnameUnderWildcard && r.Config.Gateway.HasDefaultListenerTLSSecret() && !hasExternalTLSListeners - - if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" && !useSharedTLS { - if r.Config.Gateway.PerGatewayCertificateIssuer { - if !metav1.HasAnnotation(downstreamGateway.ObjectMeta, "cert-manager.io/issuer") { - metav1.SetMetaDataAnnotation(&downstreamGateway.ObjectMeta, "cert-manager.io/issuer", upstreamGateway.Name) - } - } else { - clusterIssuerName := string(l.TLS.Options[certificateIssuerTLSOption]) - if r.Config.Gateway.ClusterIssuerMap[clusterIssuerName] != "" { - clusterIssuerName = r.Config.Gateway.ClusterIssuerMap[clusterIssuerName] - } - if !metav1.HasAnnotation(downstreamGateway.ObjectMeta, "cert-manager.io/cluster-issuer") { - metav1.SetMetaDataAnnotation(&downstreamGateway.ObjectMeta, "cert-manager.io/cluster-issuer", clusterIssuerName) - } - - // Add labels so that secrets created by cert-manager can be propagated - // See: https://cert-manager.io/docs/reference/annotations/#cert-manageriosecret-template - if !metav1.HasAnnotation(downstreamGateway.ObjectMeta, "cert-manager.io/secret-template") { - metav1.SetMetaDataAnnotation( - &downstreamGateway.ObjectMeta, - "cert-manager.io/secret-template", - fmt.Sprintf( - `{"labels": {"%s": "%s"}}`, - downstreamclient.UpstreamOwnerClusterNameLabel, - fmt.Sprintf("cluster-%s", strings.ReplaceAll(upstreamClusterName, "/", "_")), - ), - ) - } - } - } + useSharedTLS := hostnameUnderWildcard && r.Config.Gateway.HasDefaultListenerTLSSecret() if l.Hostname != nil { listenerCopy := l.DeepCopy() if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" { - // Translate upstream TLS settings to downstream TLS settings delete(listenerCopy.TLS.Options, certificateIssuerTLSOption) tlsMode := gatewayv1.TLSModeTerminate @@ -467,15 +433,15 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( }, } } else { + // Secret name must match the Certificate created by + // ensureListenerCertificates for this listener. listenerCopy.TLS = &gatewayv1.GatewayTLSConfig{ Mode: &tlsMode, - // TODO(jreese) investigate secret deletion when Cert (gateway) is deleted - // See: https://cert-manager.io/docs/usage/certificate/#cleaning-up-secrets-when-certificates-are-deleted CertificateRefs: []gatewayv1.SecretObjectReference{ { Group: ptr.To(gatewayv1.Group("")), Kind: ptr.To(gatewayv1.Kind("Secret")), - Name: gatewayv1.ObjectName(resourcename.GetValidDNS1123Name(fmt.Sprintf("%s-%s", upstreamGateway.Name, l.Name))), + Name: gatewayv1.ObjectName(listenerCertificateSecretName(upstreamGateway.Name, l.Name)), }, }, } @@ -494,26 +460,190 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( return &downstreamGateway } -// certManagerAnnotationKeys are the annotations managed by the gateway controller -// for cert-manager integration. These are synced from the desired state and stale -// entries are removed when custom hostnames become unclaimed. -var certManagerAnnotationKeys = []string{ +// legacyCertManagerAnnotations are annotations previously set by NSO on +// downstream gateways for cert-manager's gateway-shim. NSO now creates +// Certificate resources directly, so these must be stripped from existing +// gateways to prevent gateway-shim from creating conflicting Certificates. +var legacyCertManagerAnnotations = []string{ "cert-manager.io/issuer", "cert-manager.io/cluster-issuer", "cert-manager.io/secret-template", } -// syncCertManagerAnnotations ensures the downstream gateway's cert-manager -// annotations match the desired state: desired keys are set, and keys that -// are no longer needed are removed. -func syncCertManagerAnnotations(downstream, desired *gatewayv1.Gateway) { - for _, key := range certManagerAnnotationKeys { - if v, ok := desired.Annotations[key]; ok { - metav1.SetMetaDataAnnotation(&downstream.ObjectMeta, key, v) - } else { - delete(downstream.Annotations, key) +const certManagerMigrationAnnotation = "networking.datumapis.com/cert-manager-annotation-migration" + +// migrateLegacyCertManagerAnnotations runs a two-phase migration to ensure +// Karmada propagates the removal of legacy cert-manager annotations to member +// clusters. +// +// Background: Karmada only removes an annotation from a member cluster object +// if that annotation key was previously recorded in the Work object's +// resourcetemplate.karmada.io/managed-annotations. For gateways that were +// adopted by Karmada after being created on the member cluster, the +// cert-manager annotations were never tracked. This migration forces tracking +// by temporarily seeding the annotation keys (phase 1), giving Karmada time to +// record them, then removing them (phase 2) so Karmada propagates the deletion. +// +// Returns a non-zero duration if a requeue is needed to complete the migration. +func migrateLegacyCertManagerAnnotations(gw *gatewayv1.Gateway) time.Duration { + if gw.Annotations == nil { + gw.Annotations = make(map[string]string) + } + + switch gw.Annotations[certManagerMigrationAnnotation] { + case "": + // Phase 1: Seed cert-manager annotation keys with empty values so + // Karmada's RecordManagedAnnotations includes them when building the + // Work object. Empty values prevent gateway-shim from issuing real + // certificates during the migration window. + for _, key := range legacyCertManagerAnnotations { + gw.Annotations[key] = "" + } + gw.Annotations[certManagerMigrationAnnotation] = "seeded" + return 60 * time.Second + + case "seeded": + // Phase 2: Remove the seeded annotations. Karmada has recorded them in + // managed-annotations, so getDeletedAnnotationKeys will detect the + // removal and propagate it to member clusters. + for _, key := range legacyCertManagerAnnotations { + delete(gw.Annotations, key) + } + gw.Annotations[certManagerMigrationAnnotation] = "complete" + return 0 + + default: + // Migration complete. Strip any lingering cert-manager annotations as a + // safety net (e.g., if the gateway was updated externally). + for _, key := range legacyCertManagerAnnotations { + delete(gw.Annotations, key) + } + return 0 + } +} + +// listenerCertificateSecretName returns the deterministic Secret name that a +// Certificate resource will populate for a given gateway listener. +func listenerCertificateSecretName(gatewayName string, listenerName gatewayv1.SectionName) string { + return resourcename.GetValidDNS1123Name(fmt.Sprintf("%s-%s", gatewayName, listenerName)) +} + +// listenerCertificateName returns the deterministic Certificate resource name +// for a given gateway listener. +func listenerCertificateName(gatewayName string, listenerName gatewayv1.SectionName) string { + return resourcename.GetValidDNS1123Name(fmt.Sprintf("%s-%s", gatewayName, listenerName)) +} + +// ensureListenerCertificates creates, updates, or deletes cert-manager +// Certificate resources for each listener that requires an individual TLS +// certificate. Listeners whose hostnames fall under the wildcard target domain +// use the shared TLS secret and do not get a Certificate. +func (r *GatewayReconciler) ensureListenerCertificates( + ctx context.Context, + upstreamClusterName string, + upstreamGateway *gatewayv1.Gateway, + downstreamGateway *gatewayv1.Gateway, + downstreamClient client.Client, + claimedHostnames []string, +) (result Result) { + logger := log.FromContext(ctx) + wildcardSuffix := "." + r.Config.Gateway.TargetDomain + + desiredCerts := make(map[string]bool) + + // Only create Certificates for listeners with custom hostnames outside the + // wildcard scope. Wildcard-covered listeners use the shared TLS secret and + // don't need individual Certificates. + for _, l := range upstreamGateway.Spec.Listeners { + if l.TLS == nil || l.TLS.Options[certificateIssuerTLSOption] == "" || l.Hostname == nil { + continue + } + hostname := string(*l.Hostname) + if !slices.Contains(claimedHostnames, hostname) { + continue + } + // Skip hostnames covered by the wildcard — they use the shared secret. + if strings.HasSuffix(hostname, wildcardSuffix) || hostname == r.Config.Gateway.TargetDomain { + continue + } + + clusterIssuerName := string(l.TLS.Options[certificateIssuerTLSOption]) + if mapped := r.Config.Gateway.ClusterIssuerMap[clusterIssuerName]; mapped != "" { + clusterIssuerName = mapped } + + certName := listenerCertificateName(upstreamGateway.Name, l.Name) + secretName := listenerCertificateSecretName(upstreamGateway.Name, l.Name) + desiredCerts[certName] = true + + cert := &cmv1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: downstreamGateway.Namespace, + }, + } + + opResult, err := controllerutil.CreateOrUpdate(ctx, downstreamClient, cert, func() error { + // Owner reference gives us Kubernetes GC: when the downstream + // gateway is deleted, its Certificates are cleaned up automatically. + if err := controllerutil.SetControllerReference(downstreamGateway, cert, downstreamClient.Scheme()); err != nil { + return err + } + + cert.Spec = cmv1.CertificateSpec{ + SecretName: secretName, + // Propagation label on the issued Secret so Karmada can + // distribute it to member clusters. + SecretTemplate: &cmv1.CertificateSecretTemplate{ + Labels: map[string]string{ + downstreamclient.UpstreamOwnerClusterNameLabel: fmt.Sprintf("cluster-%s", strings.ReplaceAll(upstreamClusterName, "/", "_")), + }, + }, + DNSNames: []string{hostname}, + IssuerRef: cmmeta.ObjectReference{ + Name: clusterIssuerName, + Kind: "ClusterIssuer", + }, + } + + return nil + }) + if err != nil { + result.Err = fmt.Errorf("failed to ensure Certificate %s: %w", certName, err) + return result + } + if opResult != controllerutil.OperationResultNone { + logger.Info("Certificate reconciled", "certificate", certName, "operation", opResult) + } + } + + // Clean up Certificate resources for listeners that no longer need them. + // Certificates owned by this gateway are identified via ownerReferences + // set by controllerutil.SetControllerReference above. + var certList cmv1.CertificateList + if err := downstreamClient.List(ctx, &certList, + client.InNamespace(downstreamGateway.Namespace), + ); err != nil { + result.Err = fmt.Errorf("failed to list Certificates: %w", err) + return result } + + for i := range certList.Items { + cert := &certList.Items[i] + if !metav1.IsControlledBy(cert, downstreamGateway) { + continue + } + if desiredCerts[cert.Name] { + continue + } + logger.Info("deleting stale Certificate", "certificate", cert.Name) + if err := downstreamClient.Delete(ctx, cert); client.IgnoreNotFound(err) != nil { + result.Err = fmt.Errorf("failed to delete stale Certificate %s: %w", cert.Name, err) + return result + } + } + + return result } func (r *GatewayReconciler) reconcileGatewayStatus( diff --git a/internal/controller/gateway_controller_test.go b/internal/controller/gateway_controller_test.go index 5bb5d04e..17d6ea80 100644 --- a/internal/controller/gateway_controller_test.go +++ b/internal/controller/gateway_controller_test.go @@ -6,6 +6,7 @@ import ( "slices" "testing" + cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -36,6 +37,7 @@ func TestEnsureDownstreamGateway(t *testing.T) { assert.NoError(t, gatewayv1.Install(testScheme)) assert.NoError(t, discoveryv1.AddToScheme(testScheme)) assert.NoError(t, networkingv1alpha.AddToScheme(testScheme)) + assert.NoError(t, cmv1.AddToScheme(testScheme)) upstreamNamespace := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -247,6 +249,7 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { assert.NoError(t, gatewayv1.Install(testScheme)) assert.NoError(t, discoveryv1.AddToScheme(testScheme)) assert.NoError(t, networkingv1alpha.AddToScheme(testScheme)) + assert.NoError(t, cmv1.AddToScheme(testScheme)) upstreamNamespace := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -301,7 +304,7 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { } assert.Empty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], - "cert-manager cluster-issuer annotation should not be set when only default listeners exist", + "cert-manager annotation should not be set; Certificates are created directly", ) }, }, @@ -339,9 +342,6 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { }), }, assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { - // When custom HTTPS listeners exist, all listeners fall back to - // per-listener certs to prevent cert-manager from overwriting - // the shared wildcard secret. httpsListener := gatewayutil.GetListenerByName( downstreamGateway.Spec.Listeners, gatewayutil.DefaultHTTPSListenerName, @@ -350,9 +350,9 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { if assert.NotNil(t, httpsListener.TLS) { if assert.Len(t, httpsListener.TLS.CertificateRefs, 1) { assert.Equal(t, - gatewayv1.ObjectName("test-gw-default-https"), + gatewayv1.ObjectName(defaultTLSSecretName), httpsListener.TLS.CertificateRefs[0].Name, - "default listener should fall back to per-listener cert when custom listeners exist", + "default listener under wildcard should still use shared TLS even with custom listeners", ) } } @@ -374,8 +374,8 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { } } - assert.NotEmpty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], - "cert-manager annotation should be set when custom hostname listeners exist", + assert.Empty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], + "cert-manager annotation should not be set; Certificates are created directly", ) }, }, @@ -448,7 +448,7 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { } assert.Empty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], - "cert-manager annotation should NOT be set when all hostnames are under the wildcard", + "cert-manager annotation should not be set; Certificates are created directly", ) }, }, @@ -474,8 +474,8 @@ func TestEnsureDownstreamGatewayWildcardCert(t *testing.T) { } } - assert.NotEmpty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], - "cert-manager annotation should be set when wildcard is not configured", + assert.Empty(t, downstreamGateway.Annotations["cert-manager.io/cluster-issuer"], + "cert-manager annotation should not be set; Certificates are created directly", ) }, }, @@ -1121,22 +1121,18 @@ func newGateway( return gw } -func TestGetDesiredDownstreamGateway_UnclaimedHostnameNoAnnotations(t *testing.T) { +func TestGetDesiredDownstreamGateway_UnclaimedHostnameSkipped(t *testing.T) { logger := zap.New(zap.UseFlagOptions(&zap.Options{Development: true})) ctx := log.IntoContext(context.Background(), logger) tests := []struct { - name string - perGatewayCertIssuer bool - listeners []gatewayv1.Listener - claimedHostnames []string - expectIssuerAnnotation bool - expectClusterAnnotation bool - expectListeners int + name string + listeners []gatewayv1.Listener + claimedHostnames []string + expectListeners int }{ { - name: "unclaimed hostname with TLS option does not produce annotations", - perGatewayCertIssuer: true, + name: "unclaimed hostname is skipped", listeners: []gatewayv1.Listener{ { Name: "custom-https", @@ -1150,35 +1146,11 @@ func TestGetDesiredDownstreamGateway_UnclaimedHostnameNoAnnotations(t *testing.T }, }, }, - claimedHostnames: nil, - expectIssuerAnnotation: false, - expectClusterAnnotation: false, - expectListeners: 0, - }, - { - name: "claimed hostname with TLS option produces issuer annotation (per-gateway mode)", - perGatewayCertIssuer: true, - listeners: []gatewayv1.Listener{ - { - Name: "custom-https", - Port: 443, - Protocol: gatewayv1.HTTPSProtocolType, - Hostname: ptr.To(gatewayv1.Hostname("claimed.example.com")), - TLS: &gatewayv1.GatewayTLSConfig{ - Options: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ - certificateIssuerTLSOption: "letsencrypt", - }, - }, - }, - }, - claimedHostnames: []string{"claimed.example.com"}, - expectIssuerAnnotation: true, - expectClusterAnnotation: false, - expectListeners: 1, + claimedHostnames: nil, + expectListeners: 0, }, { - name: "claimed hostname with TLS option produces cluster-issuer annotation (cluster mode)", - perGatewayCertIssuer: false, + name: "claimed hostname produces listener", listeners: []gatewayv1.Listener{ { Name: "custom-https", @@ -1192,14 +1164,11 @@ func TestGetDesiredDownstreamGateway_UnclaimedHostnameNoAnnotations(t *testing.T }, }, }, - claimedHostnames: []string{"claimed.example.com"}, - expectIssuerAnnotation: false, - expectClusterAnnotation: true, - expectListeners: 1, + claimedHostnames: []string{"claimed.example.com"}, + expectListeners: 1, }, { - name: "mix: only claimed hostname contributes annotations", - perGatewayCertIssuer: true, + name: "mix: only claimed hostname produces listener", listeners: []gatewayv1.Listener{ { Name: "unclaimed-https", @@ -1224,10 +1193,8 @@ func TestGetDesiredDownstreamGateway_UnclaimedHostnameNoAnnotations(t *testing.T }, }, }, - claimedHostnames: []string{"claimed.example.com"}, - expectIssuerAnnotation: true, - expectClusterAnnotation: false, - expectListeners: 1, + claimedHostnames: []string{"claimed.example.com"}, + expectListeners: 1, }, } @@ -1236,8 +1203,7 @@ func TestGetDesiredDownstreamGateway_UnclaimedHostnameNoAnnotations(t *testing.T reconciler := &GatewayReconciler{ Config: config.NetworkServicesOperator{ Gateway: config.GatewayConfig{ - DownstreamGatewayClassName: "envoy", - PerGatewayCertificateIssuer: tt.perGatewayCertIssuer, + DownstreamGatewayClassName: "envoy", }, }, } @@ -1249,87 +1215,12 @@ func TestGetDesiredDownstreamGateway_UnclaimedHostnameNoAnnotations(t *testing.T desired := reconciler.getDesiredDownstreamGateway(ctx, "test-cluster", upstream, tt.claimedHostnames) - _, hasIssuer := desired.Annotations["cert-manager.io/issuer"] - _, hasClusterIssuer := desired.Annotations["cert-manager.io/cluster-issuer"] - - assert.Equal(t, tt.expectIssuerAnnotation, hasIssuer, "cert-manager.io/issuer annotation presence") - assert.Equal(t, tt.expectClusterAnnotation, hasClusterIssuer, "cert-manager.io/cluster-issuer annotation presence") + assert.Empty(t, desired.Annotations, "desired gateway should have no cert-manager annotations") assert.Len(t, desired.Spec.Listeners, tt.expectListeners, "downstream listener count") }) } } -func TestSyncCertManagerAnnotations(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - existingAnnotations map[string]string - desiredAnnotations map[string]string - wantAnnotations map[string]string - }{ - { - name: "stale issuer annotation removed", - existingAnnotations: map[string]string{ - "cert-manager.io/issuer": "old-issuer", - "other-annotation": "keep-me", - }, - desiredAnnotations: map[string]string{}, - wantAnnotations: map[string]string{ - "other-annotation": "keep-me", - }, - }, - { - name: "issuer annotation updated to cluster-issuer", - existingAnnotations: map[string]string{ - "cert-manager.io/issuer": "old-issuer", - }, - desiredAnnotations: map[string]string{ - "cert-manager.io/cluster-issuer": "new-cluster-issuer", - }, - wantAnnotations: map[string]string{ - "cert-manager.io/cluster-issuer": "new-cluster-issuer", - }, - }, - { - name: "no-op when annotations match", - existingAnnotations: map[string]string{"cert-manager.io/issuer": "my-issuer"}, - desiredAnnotations: map[string]string{"cert-manager.io/issuer": "my-issuer"}, - wantAnnotations: map[string]string{"cert-manager.io/issuer": "my-issuer"}, - }, - { - name: "all three cert-manager annotations cleaned up", - existingAnnotations: map[string]string{ - "cert-manager.io/issuer": "old", - "cert-manager.io/cluster-issuer": "old", - "cert-manager.io/secret-template": "old", - "unrelated": "preserved", - }, - desiredAnnotations: map[string]string{}, - wantAnnotations: map[string]string{ - "unrelated": "preserved", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - existing := &gatewayv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{Annotations: tt.existingAnnotations}, - } - desired := &gatewayv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{Annotations: tt.desiredAnnotations}, - } - - syncCertManagerAnnotations(existing, desired) - - assert.Equal(t, tt.wantAnnotations, existing.Annotations) - }) - } -} - func newHTTPRoute(namespace, name string, opts ...func(*gatewayv1.HTTPRoute)) *gatewayv1.HTTPRoute { route := &gatewayv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ From 41cc4058b5b5d2460b7503ea0bbccfc22a126775 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Fri, 27 Feb 2026 14:48:44 -0800 Subject: [PATCH 15/17] watch certificate --- internal/controller/gateway_controller.go | 163 +++++++------------ internal/downstreamclient/mappednamespace.go | 19 ++- 2 files changed, 71 insertions(+), 111 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 8131273e..3866592f 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -5,7 +5,6 @@ package controller import ( "context" "fmt" - "maps" "slices" "strings" "time" @@ -287,34 +286,23 @@ func (r *GatewayReconciler) ensureDownstreamGateway( return result, nil } } else { - // Run the two-phase cert-manager annotation migration before diffing. - // This mutates downstreamGateway.Annotations in place, which the diff - // check below will pick up and persist via Update. - migrationRequeue := migrateLegacyCertManagerAnnotations(downstreamGateway) - if !equality.Semantic.DeepEqual(downstreamGateway.Annotations, desiredDownstreamGateway.Annotations) || !equality.Semantic.DeepEqual(downstreamGateway.Spec, desiredDownstreamGateway.Spec) { - // maps.Copy merges desired over existing; migration-added keys - // (like the phase marker) survive because they aren't in desired. - maps.Copy(downstreamGateway.Annotations, desiredDownstreamGateway.Annotations) + downstreamGateway.Annotations = desiredDownstreamGateway.Annotations downstreamGateway.Spec = desiredDownstreamGateway.Spec if err := downstreamClient.Update(ctx, downstreamGateway); err != nil { result.Err = fmt.Errorf("failed updating downstream gateway: %w", err) return result, nil } } - - if migrationRequeue > 0 { - result.RequeueAfter = migrationRequeue - } } certResult := r.ensureListenerCertificates( ctx, - upstreamClusterName, upstreamGateway, downstreamGateway, downstreamClient, + downstreamStrategy, claimedHostnames, ) if certResult.ShouldReturn() { @@ -460,67 +448,6 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( return &downstreamGateway } -// legacyCertManagerAnnotations are annotations previously set by NSO on -// downstream gateways for cert-manager's gateway-shim. NSO now creates -// Certificate resources directly, so these must be stripped from existing -// gateways to prevent gateway-shim from creating conflicting Certificates. -var legacyCertManagerAnnotations = []string{ - "cert-manager.io/issuer", - "cert-manager.io/cluster-issuer", - "cert-manager.io/secret-template", -} - -const certManagerMigrationAnnotation = "networking.datumapis.com/cert-manager-annotation-migration" - -// migrateLegacyCertManagerAnnotations runs a two-phase migration to ensure -// Karmada propagates the removal of legacy cert-manager annotations to member -// clusters. -// -// Background: Karmada only removes an annotation from a member cluster object -// if that annotation key was previously recorded in the Work object's -// resourcetemplate.karmada.io/managed-annotations. For gateways that were -// adopted by Karmada after being created on the member cluster, the -// cert-manager annotations were never tracked. This migration forces tracking -// by temporarily seeding the annotation keys (phase 1), giving Karmada time to -// record them, then removing them (phase 2) so Karmada propagates the deletion. -// -// Returns a non-zero duration if a requeue is needed to complete the migration. -func migrateLegacyCertManagerAnnotations(gw *gatewayv1.Gateway) time.Duration { - if gw.Annotations == nil { - gw.Annotations = make(map[string]string) - } - - switch gw.Annotations[certManagerMigrationAnnotation] { - case "": - // Phase 1: Seed cert-manager annotation keys with empty values so - // Karmada's RecordManagedAnnotations includes them when building the - // Work object. Empty values prevent gateway-shim from issuing real - // certificates during the migration window. - for _, key := range legacyCertManagerAnnotations { - gw.Annotations[key] = "" - } - gw.Annotations[certManagerMigrationAnnotation] = "seeded" - return 60 * time.Second - - case "seeded": - // Phase 2: Remove the seeded annotations. Karmada has recorded them in - // managed-annotations, so getDeletedAnnotationKeys will detect the - // removal and propagate it to member clusters. - for _, key := range legacyCertManagerAnnotations { - delete(gw.Annotations, key) - } - gw.Annotations[certManagerMigrationAnnotation] = "complete" - return 0 - - default: - // Migration complete. Strip any lingering cert-manager annotations as a - // safety net (e.g., if the gateway was updated externally). - for _, key := range legacyCertManagerAnnotations { - delete(gw.Annotations, key) - } - return 0 - } -} // listenerCertificateSecretName returns the deterministic Secret name that a // Certificate resource will populate for a given gateway listener. @@ -540,10 +467,10 @@ func listenerCertificateName(gatewayName string, listenerName gatewayv1.SectionN // use the shared TLS secret and do not get a Certificate. func (r *GatewayReconciler) ensureListenerCertificates( ctx context.Context, - upstreamClusterName string, upstreamGateway *gatewayv1.Gateway, downstreamGateway *gatewayv1.Gateway, downstreamClient client.Client, + downstreamStrategy downstreamclient.ResourceStrategy, claimedHostnames []string, ) (result Result) { logger := log.FromContext(ctx) @@ -583,43 +510,58 @@ func (r *GatewayReconciler) ensureListenerCertificates( }, } - opResult, err := controllerutil.CreateOrUpdate(ctx, downstreamClient, cert, func() error { - // Owner reference gives us Kubernetes GC: when the downstream - // gateway is deleted, its Certificates are cleaned up automatically. - if err := controllerutil.SetControllerReference(downstreamGateway, cert, downstreamClient.Scheme()); err != nil { - return err + if err := downstreamClient.Get(ctx, client.ObjectKeyFromObject(cert), cert); client.IgnoreNotFound(err) != nil { + result.Err = fmt.Errorf("failed to get Certificate %s: %w", certName, err) + return result + } + + isNew := cert.CreationTimestamp.IsZero() + if isNew { + if err := downstreamStrategy.SetControllerReference(ctx, upstreamGateway, cert); err != nil { + result.Err = fmt.Errorf("failed to set controller reference on Certificate %s: %w", certName, err) + return result } + } - cert.Spec = cmv1.CertificateSpec{ - SecretName: secretName, - // Propagation label on the issued Secret so Karmada can - // distribute it to member clusters. - SecretTemplate: &cmv1.CertificateSecretTemplate{ - Labels: map[string]string{ - downstreamclient.UpstreamOwnerClusterNameLabel: fmt.Sprintf("cluster-%s", strings.ReplaceAll(upstreamClusterName, "/", "_")), - }, + desiredSpec := cmv1.CertificateSpec{ + SecretName: secretName, + SecretTemplate: &cmv1.CertificateSecretTemplate{ + Labels: map[string]string{ + downstreamclient.UpstreamOwnerClusterNameLabel: cert.Labels[downstreamclient.UpstreamOwnerClusterNameLabel], }, - DNSNames: []string{hostname}, - IssuerRef: cmmeta.ObjectReference{ - Name: clusterIssuerName, - Kind: "ClusterIssuer", - }, - } + }, + DNSNames: []string{hostname}, + IssuerRef: cmmeta.ObjectReference{ + Name: clusterIssuerName, + Kind: "ClusterIssuer", + }, + } - return nil - }) + var opResult string + var err error + if isNew { + cert.Spec = desiredSpec + err = downstreamClient.Create(ctx, cert) + opResult = "created" + } else if !equality.Semantic.DeepEqual(cert.Spec, desiredSpec) { + cert.Spec = desiredSpec + err = downstreamClient.Update(ctx, cert) + opResult = "updated" + } if err != nil { result.Err = fmt.Errorf("failed to ensure Certificate %s: %w", certName, err) return result } - if opResult != controllerutil.OperationResultNone { + if opResult != "" { logger.Info("Certificate reconciled", "certificate", certName, "operation", opResult) } } // Clean up Certificate resources for listeners that no longer need them. - // Certificates owned by this gateway are identified via ownerReferences - // set by controllerutil.SetControllerReference above. + // Two ownership patterns are checked: + // 1. Certificates created by NSO via the downstream strategy (upstream-owner labels) + // 2. Legacy Certificates created by cert-manager's gateway-shim (controller ownerRef + // to the downstream gateway, no upstream-owner labels) var certList cmv1.CertificateList if err := downstreamClient.List(ctx, &certList, client.InNamespace(downstreamGateway.Namespace), @@ -630,12 +572,19 @@ func (r *GatewayReconciler) ensureListenerCertificates( for i := range certList.Items { cert := &certList.Items[i] - if !metav1.IsControlledBy(cert, downstreamGateway) { + if desiredCerts[cert.Name] { continue } - if desiredCerts[cert.Name] { + + ownedByStrategy := cert.Labels[downstreamclient.UpstreamOwnerKindLabel] == "Gateway" && + cert.Labels[downstreamclient.UpstreamOwnerNameLabel] == upstreamGateway.Name && + cert.Labels[downstreamclient.UpstreamOwnerNamespaceLabel] == upstreamGateway.Namespace + ownedByGatewayShim := metav1.IsControlledBy(cert, downstreamGateway) + + if !ownedByStrategy && !ownedByGatewayShim { continue } + logger.Info("deleting stale Certificate", "certificate", cert.Name) if err := downstreamClient.Delete(ctx, cert); client.IgnoreNotFound(err) != nil { result.Err = fmt.Errorf("failed to delete stale Certificate %s: %w", cert.Name, err) @@ -1822,6 +1771,13 @@ func (r *GatewayReconciler) SetupWithManager(mgr mcmanager.Manager) error { downstreamHTTPRouteClusterSource, _ := downstreamHTTPRouteSource.ForCluster("", r.DownstreamCluster) + downstreamCertificateSource := mcsource.TypedKind( + &cmv1.Certificate{}, + downstreamclient.TypedEnqueueRequestForUpstreamOwner[*cmv1.Certificate](&gatewayv1.Gateway{}), + ) + + downstreamCertificateClusterSource, _ := downstreamCertificateSource.ForCluster("", r.DownstreamCluster) + builder := mcbuilder.ControllerManagedBy(mgr). For(&gatewayv1.Gateway{}). Watches( @@ -1841,7 +1797,8 @@ func (r *GatewayReconciler) SetupWithManager(mgr mcmanager.Manager) error { r.listGatewaysForHTTPRouteFilterFunc, ). WatchesRawSource(downstreamGatewayClusterSource). - WatchesRawSource(downstreamHTTPRouteClusterSource) + WatchesRawSource(downstreamHTTPRouteClusterSource). + WatchesRawSource(downstreamCertificateClusterSource) if r.Config.Gateway.EnableDNSIntegration { builder = builder. diff --git a/internal/downstreamclient/mappednamespace.go b/internal/downstreamclient/mappednamespace.go index 372cd14c..e2dccfdc 100644 --- a/internal/downstreamclient/mappednamespace.go +++ b/internal/downstreamclient/mappednamespace.go @@ -146,17 +146,20 @@ func (c *mappedNamespaceResourceStrategy) SetControllerReference(ctx context.Con downstreamClient := c.GetClient() var anchorConfigMap corev1.ConfigMap - if err := downstreamClient.Get(ctx, client.ObjectKey{Namespace: controlled.GetNamespace(), Name: anchorName}, &anchorConfigMap); client.IgnoreNotFound(err) != nil { - return fmt.Errorf("failed listing configmaps: %w", err) - } - - if anchorConfigMap.CreationTimestamp.IsZero() { - anchorConfigMap.Name = anchorName - anchorConfigMap.Labels = anchorLabels - anchorConfigMap.Namespace = controlled.GetNamespace() + err = downstreamClient.Get(ctx, client.ObjectKey{Namespace: controlled.GetNamespace(), Name: anchorName}, &anchorConfigMap) + if apierrors.IsNotFound(err) { + anchorConfigMap = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: anchorName, + Namespace: controlled.GetNamespace(), + Labels: anchorLabels, + }, + } if err := downstreamClient.Create(ctx, &anchorConfigMap); err != nil { return fmt.Errorf("failed creating anchor configmap: %w", err) } + } else if err != nil { + return fmt.Errorf("failed getting anchor configmap: %w", err) } if err := controllerutil.SetOwnerReference(&anchorConfigMap, controlled, downstreamClient.Scheme(), opts...); err != nil { From b8cb054e35d6868104ff9786bb10f6c457b0e3f4 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Fri, 27 Feb 2026 15:04:02 -0800 Subject: [PATCH 16/17] fix: rbac --- config/rbac_downstream/role.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/rbac_downstream/role.yaml b/config/rbac_downstream/role.yaml index a6de2cb8..c1880d33 100644 --- a/config/rbac_downstream/role.yaml +++ b/config/rbac_downstream/role.yaml @@ -139,8 +139,12 @@ rules: resources: - certificates verbs: + - create + - delete - get - list + - patch + - update - watch - apiGroups: - gateway.envoyproxy.io From 10884b1ca2d9f263831c8a5e3a9d3177c5574b10 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Mon, 2 Mar 2026 10:09:24 -0800 Subject: [PATCH 17/17] fix: lint --- internal/controller/gateway_controller.go | 5 +---- internal/controller/gateway_controller_test.go | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 3866592f..0735e74f 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -267,7 +267,6 @@ func (r *GatewayReconciler) ensureDownstreamGateway( desiredDownstreamGateway := r.getDesiredDownstreamGateway( ctx, - upstreamClusterName, upstreamGateway, claimedHostnames, ) @@ -375,7 +374,6 @@ func (r *GatewayReconciler) ensureDownstreamGateway( func (r *GatewayReconciler) getDesiredDownstreamGateway( ctx context.Context, - upstreamClusterName string, upstreamGateway *gatewayv1.Gateway, claimedHostnames []string, ) *gatewayv1.Gateway { @@ -448,7 +446,6 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( return &downstreamGateway } - // listenerCertificateSecretName returns the deterministic Secret name that a // Certificate resource will populate for a given gateway listener. func listenerCertificateSecretName(gatewayName string, listenerName gatewayv1.SectionName) string { @@ -576,7 +573,7 @@ func (r *GatewayReconciler) ensureListenerCertificates( continue } - ownedByStrategy := cert.Labels[downstreamclient.UpstreamOwnerKindLabel] == "Gateway" && + ownedByStrategy := cert.Labels[downstreamclient.UpstreamOwnerKindLabel] == KindGateway && cert.Labels[downstreamclient.UpstreamOwnerNameLabel] == upstreamGateway.Name && cert.Labels[downstreamclient.UpstreamOwnerNamespaceLabel] == upstreamGateway.Namespace ownedByGatewayShim := metav1.IsControlledBy(cert, downstreamGateway) diff --git a/internal/controller/gateway_controller_test.go b/internal/controller/gateway_controller_test.go index 17d6ea80..18072480 100644 --- a/internal/controller/gateway_controller_test.go +++ b/internal/controller/gateway_controller_test.go @@ -1213,7 +1213,7 @@ func TestGetDesiredDownstreamGateway_UnclaimedHostnameSkipped(t *testing.T) { Spec: gatewayv1.GatewaySpec{Listeners: tt.listeners}, } - desired := reconciler.getDesiredDownstreamGateway(ctx, "test-cluster", upstream, tt.claimedHostnames) + desired := reconciler.getDesiredDownstreamGateway(ctx, upstream, tt.claimedHostnames) assert.Empty(t, desired.Annotations, "desired gateway should have no cert-manager annotations") assert.Len(t, desired.Spec.Listeners, tt.expectListeners, "downstream listener count")