From 493f04e81a0293d29064a47338d9f35a48cc70a2 Mon Sep 17 00:00:00 2001 From: Seth Malaki Date: Fri, 27 Feb 2026 10:25:12 +0000 Subject: [PATCH 1/3] feat(istio): pass imagePullSecrets to istiod via Helm values Add ImagePullSecrets field to GlobalConfig and populate it from Installation pull secrets when rendering istiod Helm values. This makes istiod inject imagePullSecrets references into waypoint pod specs it creates, enabling image pulls from private registries. --- pkg/render/istio/config.go | 1 + pkg/render/istio/istio.go | 9 ++++++ pkg/render/istio/istio_test.go | 50 ++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/pkg/render/istio/config.go b/pkg/render/istio/config.go index c8ebcd11b8..d18d9b31bb 100644 --- a/pkg/render/istio/config.go +++ b/pkg/render/istio/config.go @@ -22,6 +22,7 @@ type GlobalConfig struct { Proxy *ProxyConfig `json:"proxy,omitempty"` ProxyInit *ProxyInitConfig `json:"proxy_init,omitempty"` Platform string `json:"platform,omitempty"` + ImagePullSecrets []string `json:"imagePullSecrets,omitempty"` } type ProxyConfig struct { diff --git a/pkg/render/istio/istio.go b/pkg/render/istio/istio.go index 97dac3fe82..76eca8b4f3 100644 --- a/pkg/render/istio/istio.go +++ b/pkg/render/istio/istio.go @@ -121,6 +121,15 @@ func Istio(cfg *Configuration) (*IstioComponentCRDs, *IstioComponent, error) { }, }, } + // Pass imagePullSecrets to istiod so it injects them into waypoint pod specs. + if len(cfg.PullSecrets) > 0 { + secretNames := make([]string, 0, len(cfg.PullSecrets)) + for _, s := range cfg.PullSecrets { + secretNames = append(secretNames, s.Name) + } + istioResOpts.IstiodOpts.Global.ImagePullSecrets = secretNames + } + // Set platform on all charts that have platform-specific behavior. // The embedded Helm charts use zzz_profile.yaml to load platform profiles // (e.g., profile-platform-openshift.yaml) which configure CNI paths, SCC diff --git a/pkg/render/istio/istio_test.go b/pkg/render/istio/istio_test.go index f17ec48d41..b017538429 100644 --- a/pkg/render/istio/istio_test.go +++ b/pkg/render/istio/istio_test.go @@ -15,6 +15,8 @@ package istio_test import ( + "strings" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -326,6 +328,54 @@ var _ = Describe("Istio Component Rendering", func() { Expect(err).ShouldNot(HaveOccurred()) Expect(ztunnelDS.Spec.Template.Spec.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: "test-pull-secret"})) }) + + It("should include imagePullSecrets in istiod Helm values", func() { + pullSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-pull-secret", + Namespace: istio.IstioNamespace, + }, + } + cfg.PullSecrets = []*corev1.Secret{pullSecret} + + _, component, err := istio.Istio(cfg) + Expect(err).ShouldNot(HaveOccurred()) + + objsToCreate, _ := component.Objects() + + // The "values" ConfigMap contains the serialized Helm values for istiod. + // Verify that imagePullSecrets appears in its data. + valuesConfigMap, err := rtest.GetResourceOfType[*corev1.ConfigMap](objsToCreate, "values", istio.IstioNamespace) + Expect(err).ShouldNot(HaveOccurred()) + + // The values ConfigMap should contain the imagePullSecrets key with the secret name + found := false + for _, v := range valuesConfigMap.Data { + if strings.Contains(v, "imagePullSecrets") && strings.Contains(v, "my-pull-secret") { + found = true + break + } + } + Expect(found).To(BeTrue(), "Expected imagePullSecrets with 'my-pull-secret' in istiod values ConfigMap") + }) + + It("should not include secret names in imagePullSecrets Helm values when no pull secrets configured", func() { + cfg.PullSecrets = nil + + _, component, err := istio.Istio(cfg) + Expect(err).ShouldNot(HaveOccurred()) + + objsToCreate, _ := component.Objects() + + valuesConfigMap, err := rtest.GetResourceOfType[*corev1.ConfigMap](objsToCreate, "values", istio.IstioNamespace) + Expect(err).ShouldNot(HaveOccurred()) + + // When no pull secrets are configured, imagePullSecrets should be + // an empty array (the Helm chart default), not populated with names. + for _, v := range valuesConfigMap.Data { + Expect(v).NotTo(ContainSubstring("my-pull-secret"), "Expected no secret names in imagePullSecrets when none configured") + } + }) }) Describe("Deployment Overrides", func() { From 492e2d4e7256dc7fc1d82d39c039f49eb7396386 Mon Sep 17 00:00:00 2001 From: Seth Malaki Date: Fri, 27 Feb 2026 10:25:22 +0000 Subject: [PATCH 2/3] feat(istio): add waypoint pull secret sub-controller Add a new controller that watches for istio-waypoint Gateway resources and copies pull secrets from the operator namespace to waypoint namespaces. This ensures waypoint pods can pull images from private registries. Secrets are tracked with a label for cleanup when gateways are removed or the Istio CR is deleted. --- internal/controller/istio_controller.go | 1 + pkg/controller/istio/istio_controller.go | 5 + .../istio/waypoint/waypoint_controller.go | 222 +++++++++++++ .../waypoint/waypoint_controller_test.go | 293 ++++++++++++++++++ .../istio/waypoint/waypoint_suite_test.go | 29 ++ 5 files changed, 550 insertions(+) create mode 100644 pkg/controller/istio/waypoint/waypoint_controller.go create mode 100644 pkg/controller/istio/waypoint/waypoint_controller_test.go create mode 100644 pkg/controller/istio/waypoint/waypoint_suite_test.go diff --git a/internal/controller/istio_controller.go b/internal/controller/istio_controller.go index 1b5dc9edee..9922bc3158 100644 --- a/internal/controller/istio_controller.go +++ b/internal/controller/istio_controller.go @@ -34,6 +34,7 @@ type IstioReconciler struct { // +kubebuilder:rbac:groups=operator.tigera.io,resources=istios,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=operator.tigera.io,resources=istios/status,verbs=get;update;patch // +kubebuilder:rbac:groups=operator.tigera.io,resources=istios/finalizers,verbs=update +// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch // SetupWithManager sets up the controller with the Manager. func (r *IstioReconciler) SetupWithManager(mgr ctrl.Manager, opts options.ControllerOptions) error { diff --git a/pkg/controller/istio/istio_controller.go b/pkg/controller/istio/istio_controller.go index ec03fbb150..ed04aeb227 100644 --- a/pkg/controller/istio/istio_controller.go +++ b/pkg/controller/istio/istio_controller.go @@ -34,6 +34,7 @@ import ( v3 "github.com/tigera/api/pkg/apis/projectcalico/v3" "github.com/tigera/api/pkg/lib/numorstring" operatorv1 "github.com/tigera/operator/api/v1" + "github.com/tigera/operator/pkg/controller/istio/waypoint" "github.com/tigera/operator/pkg/controller/options" "github.com/tigera/operator/pkg/controller/status" "github.com/tigera/operator/pkg/controller/utils" @@ -95,6 +96,10 @@ func Add(mgr manager.Manager, opts options.ControllerOptions) error { return fmt.Errorf("istio-controller failed to create periodic reconcile watch: %w", err) } + if err := waypoint.Add(mgr, opts); err != nil { + return fmt.Errorf("failed to add waypoint pull secrets controller: %w", err) + } + return nil } diff --git a/pkg/controller/istio/waypoint/waypoint_controller.go b/pkg/controller/istio/waypoint/waypoint_controller.go new file mode 100644 index 0000000000..131ce6ec41 --- /dev/null +++ b/pkg/controller/istio/waypoint/waypoint_controller.go @@ -0,0 +1,222 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package waypoint + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + gapi "sigs.k8s.io/gateway-api/apis/v1" + + operatorv1 "github.com/tigera/operator/api/v1" + "github.com/tigera/operator/pkg/controller/options" + "github.com/tigera/operator/pkg/controller/utils" + "github.com/tigera/operator/pkg/ctrlruntime" + "github.com/tigera/operator/pkg/render" + "github.com/tigera/operator/pkg/render/common/secret" +) + +const ( + // IstioWaypointClassName is the GatewayClass name used by Istio waypoints. + IstioWaypointClassName = "istio-waypoint" + + // WaypointPullSecretLabel is the label applied to secrets copied by this controller + // for tracking and cleanup purposes. + WaypointPullSecretLabel = "operator.tigera.io/istio-waypoint-pull-secret" +) + +var log = logf.Log.WithName("controller_istio_waypoint") + +// Add creates the waypoint pull secrets controller and adds it to the Manager. +func Add(mgr manager.Manager, opts options.ControllerOptions) error { + if !opts.EnterpriseCRDExists { + return nil + } + + r := &ReconcileWaypointSecrets{ + Client: mgr.GetClient(), + scheme: mgr.GetScheme(), + } + + c, err := ctrlruntime.NewController("istio-waypoint-secrets-controller", mgr, controller.Options{Reconciler: r}) + if err != nil { + return fmt.Errorf("failed to create istio-waypoint-secrets-controller: %w", err) + } + + // Watch Gateway resources, filtering for istio-waypoint class only. + err = c.WatchObject(&gapi.Gateway{}, &handler.EnqueueRequestForObject{}, predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + gw, ok := e.Object.(*gapi.Gateway) + return ok && string(gw.Spec.GatewayClassName) == IstioWaypointClassName + }, + UpdateFunc: func(e event.UpdateEvent) bool { + gw, ok := e.ObjectNew.(*gapi.Gateway) + return ok && string(gw.Spec.GatewayClassName) == IstioWaypointClassName + }, + DeleteFunc: func(e event.DeleteEvent) bool { + gw, ok := e.Object.(*gapi.Gateway) + return ok && string(gw.Spec.GatewayClassName) == IstioWaypointClassName + }, + GenericFunc: func(e event.GenericEvent) bool { + gw, ok := e.Object.(*gapi.Gateway) + return ok && string(gw.Spec.GatewayClassName) == IstioWaypointClassName + }, + }) + if err != nil { + return fmt.Errorf("istio-waypoint-secrets-controller failed to watch Gateway resource: %w", err) + } + + // Watch Istio CR for pull secret config changes. + err = c.WatchObject(&operatorv1.Istio{}, &handler.EnqueueRequestForObject{}) + if err != nil { + return fmt.Errorf("istio-waypoint-secrets-controller failed to watch Istio resource: %w", err) + } + + // Watch Installation for pull secret changes. + if err = utils.AddInstallationWatch(c); err != nil { + return fmt.Errorf("istio-waypoint-secrets-controller failed to watch Installation resource: %w", err) + } + + // Periodic reconcile as a backstop. + if err = utils.AddPeriodicReconcile(c, utils.PeriodicReconcileTime, &handler.EnqueueRequestForObject{}); err != nil { + return fmt.Errorf("istio-waypoint-secrets-controller failed to create periodic reconcile watch: %w", err) + } + + return nil +} + +// ReconcileWaypointSecrets copies pull secrets to namespaces that contain +// istio-waypoint Gateways so that waypoint pods can pull images from private registries. +type ReconcileWaypointSecrets struct { + client.Client + scheme *runtime.Scheme +} + +func (r *ReconcileWaypointSecrets) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { + reqLogger := log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) + reqLogger.V(1).Info("Reconciling waypoint pull secrets") + + // Get the Istio CR - if not found or being deleted, clean up all secrets. + instance := &operatorv1.Istio{} + err := r.Get(ctx, utils.DefaultInstanceKey, instance) + if err != nil { + if errors.IsNotFound(err) { + reqLogger.V(1).Info("Istio CR not found, cleaning up all waypoint pull secrets") + return reconcile.Result{}, r.cleanupAllSecrets(ctx) + } + return reconcile.Result{}, err + } + if !instance.DeletionTimestamp.IsZero() { + reqLogger.V(1).Info("Istio CR being deleted, cleaning up all waypoint pull secrets") + return reconcile.Result{}, r.cleanupAllSecrets(ctx) + } + + // Get Installation and pull secrets. + _, installation, err := utils.GetInstallation(ctx, r) + if err != nil { + if errors.IsNotFound(err) { + reqLogger.V(1).Info("Installation not found") + return reconcile.Result{}, nil + } + return reconcile.Result{}, err + } + + pullSecrets, err := utils.GetNetworkingPullSecrets(installation, r) + if err != nil { + return reconcile.Result{}, err + } + + // If no pull secrets configured, clean up any previously copied secrets. + if len(pullSecrets) == 0 { + reqLogger.V(1).Info("No pull secrets configured, cleaning up waypoint pull secrets") + return reconcile.Result{}, r.cleanupAllSecrets(ctx) + } + + // List all Gateway resources and filter for istio-waypoint class. + gatewayList := &gapi.GatewayList{} + if err := r.List(ctx, gatewayList); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to list Gateways: %w", err) + } + + // Build set of target namespaces (deduplicated). + targetNamespaces := map[string]bool{} + for i := range gatewayList.Items { + gw := &gatewayList.Items[i] + if string(gw.Spec.GatewayClassName) == IstioWaypointClassName { + targetNamespaces[gw.Namespace] = true + } + } + + // For each target namespace, copy pull secrets and apply the tracking label. + for ns := range targetNamespaces { + copied := secret.CopyToNamespace(ns, pullSecrets...) + var objs []client.Object + for _, s := range copied { + if s.Labels == nil { + s.Labels = map[string]string{} + } + s.Labels[WaypointPullSecretLabel] = "true" + objs = append(objs, s) + } + + hdlr := utils.NewComponentHandler(log, r, r.scheme, nil) + component := render.NewPassthrough(objs, nil) + if err := hdlr.CreateOrUpdateOrDelete(ctx, component, nil); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to create/update pull secrets in namespace %s: %w", ns, err) + } + } + + // Clean up stale secrets from namespaces that no longer have waypoints. + if err := r.cleanupStaleSecrets(ctx, targetNamespaces); err != nil { + return reconcile.Result{}, err + } + + return reconcile.Result{}, nil +} + +// cleanupAllSecrets removes all secrets created by this controller. +func (r *ReconcileWaypointSecrets) cleanupAllSecrets(ctx context.Context) error { + return r.cleanupStaleSecrets(ctx, nil) +} + +// cleanupStaleSecrets removes tracking-labeled secrets from namespaces not in the active set. +func (r *ReconcileWaypointSecrets) cleanupStaleSecrets(ctx context.Context, activeNamespaces map[string]bool) error { + secretList := &corev1.SecretList{} + if err := r.List(ctx, secretList, client.MatchingLabels{WaypointPullSecretLabel: "true"}); err != nil { + return fmt.Errorf("failed to list waypoint pull secrets: %w", err) + } + + for i := range secretList.Items { + s := &secretList.Items[i] + if !activeNamespaces[s.Namespace] { + if err := r.Delete(ctx, s); err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("failed to delete stale pull secret %s/%s: %w", s.Namespace, s.Name, err) + } + } + } + + return nil +} diff --git a/pkg/controller/istio/waypoint/waypoint_controller_test.go b/pkg/controller/istio/waypoint/waypoint_controller_test.go new file mode 100644 index 0000000000..57d219cdb9 --- /dev/null +++ b/pkg/controller/istio/waypoint/waypoint_controller_test.go @@ -0,0 +1,293 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package waypoint + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + gapi "sigs.k8s.io/gateway-api/apis/v1" + + operatorv1 "github.com/tigera/operator/api/v1" + "github.com/tigera/operator/pkg/apis" + "github.com/tigera/operator/pkg/common" + "github.com/tigera/operator/pkg/controller/certificatemanager" + ctrlrfake "github.com/tigera/operator/pkg/ctrlruntime/client/fake" +) + +var _ = Describe("Waypoint pull secrets controller tests", func() { + var ( + cli client.Client + scheme *runtime.Scheme + ctx context.Context + r *ReconcileWaypointSecrets + installation *operatorv1.Installation + istioCR *operatorv1.Istio + ) + + BeforeEach(func() { + scheme = runtime.NewScheme() + Expect(apis.AddToScheme(scheme, false)).ShouldNot(HaveOccurred()) + + ctx = context.Background() + cli = ctrlrfake.DefaultFakeClientBuilder(scheme).Build() + + // Create certificate manager prerequisites. + certificateManager, err := certificatemanager.Create(cli, nil, "cluster.local", common.OperatorNamespace(), certificatemanager.AllowCACreation()) + Expect(err).NotTo(HaveOccurred()) + Expect(cli.Create(ctx, certificateManager.KeyPair().Secret(common.OperatorNamespace()))).NotTo(HaveOccurred()) + + r = &ReconcileWaypointSecrets{ + Client: cli, + scheme: scheme, + } + + installation = &operatorv1.Installation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: operatorv1.InstallationSpec{ + Variant: operatorv1.Calico, + }, + Status: operatorv1.InstallationStatus{ + Variant: operatorv1.Calico, + }, + } + + istioCR = &operatorv1.Istio{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + } + }) + + createNamespace := func(name string) { + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: name}} + err := cli.Create(ctx, ns) + if err == nil { + return + } + // Ignore already-exists + Expect(client.IgnoreAlreadyExists(err)).ShouldNot(HaveOccurred()) + } + + createPullSecret := func(name string) { + s := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: common.OperatorNamespace(), + }, + Data: map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{"registry.example.com":{"auth":"dGVzdDp0ZXN0"}}}`), + }, + Type: corev1.SecretTypeDockerConfigJson, + } + Expect(cli.Create(ctx, s)).NotTo(HaveOccurred()) + } + + createWaypointGateway := func(name, namespace string) { + createNamespace(namespace) + gw := &gapi.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: gapi.GatewaySpec{ + GatewayClassName: gapi.ObjectName(IstioWaypointClassName), + Listeners: []gapi.Listener{ + { + Name: "mesh", + Port: 15008, + Protocol: gapi.ProtocolType("HBONE"), + }, + }, + }, + } + Expect(cli.Create(ctx, gw)).NotTo(HaveOccurred()) + } + + createNonWaypointGateway := func(name, namespace string) { + createNamespace(namespace) + gw := &gapi.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: gapi.GatewaySpec{ + GatewayClassName: "some-other-class", + Listeners: []gapi.Listener{ + { + Name: "http", + Port: 80, + Protocol: gapi.HTTPProtocolType, + }, + }, + }, + } + Expect(cli.Create(ctx, gw)).NotTo(HaveOccurred()) + } + + doReconcile := func() (reconcile.Result, error) { + return r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + } + + listTrackedSecrets := func() []corev1.Secret { + secretList := &corev1.SecretList{} + err := cli.List(ctx, secretList, client.MatchingLabels{WaypointPullSecretLabel: "true"}) + Expect(err).NotTo(HaveOccurred()) + return secretList.Items + } + + Context("when no pull secrets are configured", func() { + It("should not create any secrets", func() { + Expect(cli.Create(ctx, installation)).NotTo(HaveOccurred()) + Expect(cli.Create(ctx, istioCR)).NotTo(HaveOccurred()) + createWaypointGateway("waypoint", "user-ns") + + _, err := doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + + secrets := listTrackedSecrets() + Expect(secrets).To(BeEmpty()) + }) + }) + + Context("when pull secrets are configured", func() { + BeforeEach(func() { + createPullSecret("my-pull-secret") + installation.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ + {Name: "my-pull-secret"}, + } + Expect(cli.Create(ctx, installation)).NotTo(HaveOccurred()) + Expect(cli.Create(ctx, istioCR)).NotTo(HaveOccurred()) + }) + + It("should copy pull secrets to waypoint gateway namespace", func() { + createWaypointGateway("waypoint", "user-ns") + + _, err := doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + + secrets := listTrackedSecrets() + Expect(secrets).To(HaveLen(1)) + Expect(secrets[0].Namespace).To(Equal("user-ns")) + Expect(secrets[0].Name).To(Equal("my-pull-secret")) + Expect(secrets[0].Labels[WaypointPullSecretLabel]).To(Equal("true")) + }) + + It("should copy pull secrets only once for multiple gateways in same namespace", func() { + createWaypointGateway("waypoint-1", "user-ns") + createWaypointGateway("waypoint-2", "user-ns") + + _, err := doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + + secrets := listTrackedSecrets() + Expect(secrets).To(HaveLen(1)) + Expect(secrets[0].Namespace).To(Equal("user-ns")) + }) + + It("should copy pull secrets to all namespaces with waypoint gateways", func() { + createWaypointGateway("waypoint-a", "ns-a") + createWaypointGateway("waypoint-b", "ns-b") + + _, err := doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + + secrets := listTrackedSecrets() + Expect(secrets).To(HaveLen(2)) + + namespaces := map[string]bool{} + for _, s := range secrets { + namespaces[s.Namespace] = true + } + Expect(namespaces).To(HaveKey("ns-a")) + Expect(namespaces).To(HaveKey("ns-b")) + }) + + It("should clean up stale secrets when gateway is deleted", func() { + createWaypointGateway("waypoint", "user-ns") + + _, err := doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + Expect(listTrackedSecrets()).To(HaveLen(1)) + + // Delete the gateway. + gw := &gapi.Gateway{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "waypoint", Namespace: "user-ns"}, gw)).NotTo(HaveOccurred()) + Expect(cli.Delete(ctx, gw)).NotTo(HaveOccurred()) + + // Reconcile again. + _, err = doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + + // Secrets should be cleaned up. + Expect(listTrackedSecrets()).To(BeEmpty()) + }) + + It("should not take action for non-matching gatewayClassName", func() { + createNonWaypointGateway("other-gateway", "user-ns") + + _, err := doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + + secrets := listTrackedSecrets() + Expect(secrets).To(BeEmpty()) + }) + }) + + Context("when Istio CR is deleted", func() { + It("should clean up all copied secrets", func() { + createPullSecret("my-pull-secret") + installation.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ + {Name: "my-pull-secret"}, + } + Expect(cli.Create(ctx, installation)).NotTo(HaveOccurred()) + Expect(cli.Create(ctx, istioCR)).NotTo(HaveOccurred()) + createWaypointGateway("waypoint", "user-ns") + + _, err := doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + Expect(listTrackedSecrets()).To(HaveLen(1)) + + // Delete the Istio CR. + Expect(cli.Delete(ctx, istioCR)).NotTo(HaveOccurred()) + + // Reconcile again. + _, err = doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + + // All secrets should be cleaned up. + Expect(listTrackedSecrets()).To(BeEmpty()) + }) + }) + + Context("when Installation resource is missing", func() { + It("should return gracefully without error", func() { + Expect(cli.Create(ctx, istioCR)).NotTo(HaveOccurred()) + + _, err := doReconcile() + Expect(err).ShouldNot(HaveOccurred()) + }) + }) +}) diff --git a/pkg/controller/istio/waypoint/waypoint_suite_test.go b/pkg/controller/istio/waypoint/waypoint_suite_test.go new file mode 100644 index 0000000000..04d9159427 --- /dev/null +++ b/pkg/controller/istio/waypoint/waypoint_suite_test.go @@ -0,0 +1,29 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package waypoint + +import ( + "testing" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" +) + +func TestWaypoint(t *testing.T) { + gomega.RegisterFailHandler(ginkgo.Fail) + suiteConfig, reporterConfig := ginkgo.GinkgoConfiguration() + reporterConfig.JUnitReport = "../../../../report/ut/istio_waypoint_suite.xml" + ginkgo.RunSpecs(t, "pkg/controller/istio/waypoint Suite", suiteConfig, reporterConfig) +} From df38dfb25992b1a81fbaea904e96d903b2d6abd1 Mon Sep 17 00:00:00 2001 From: Seth Malaki Date: Thu, 5 Mar 2026 09:38:42 +0000 Subject: [PATCH 3/3] refactor(istio): use NewPassthrough objectsToDelete for waypoint secret cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace manual cleanupAllSecrets/cleanupStaleSecrets methods with a single NewPassthrough(toCreate, toDelete) call. This follows the existing codebase pattern for managing object lifecycle through the component handler. The explicit len(pullSecrets)==0 early return is removed — the mainline reconcile logic now handles this naturally: when no secrets are needed, toCreate is empty and all existing labeled secrets flow into toDelete. Also expand the WaypointPullSecretLabel comment to explain why labels are used instead of owner references (cross-namespace cleanup needs). --- .../istio/waypoint/waypoint_controller.go | 140 ++++++++---------- 1 file changed, 59 insertions(+), 81 deletions(-) diff --git a/pkg/controller/istio/waypoint/waypoint_controller.go b/pkg/controller/istio/waypoint/waypoint_controller.go index 131ce6ec41..5df4de053f 100644 --- a/pkg/controller/istio/waypoint/waypoint_controller.go +++ b/pkg/controller/istio/waypoint/waypoint_controller.go @@ -43,8 +43,10 @@ const ( // IstioWaypointClassName is the GatewayClass name used by Istio waypoints. IstioWaypointClassName = "istio-waypoint" - // WaypointPullSecretLabel is the label applied to secrets copied by this controller - // for tracking and cleanup purposes. + // WaypointPullSecretLabel labels secrets copied by this controller. We use a label rather + // than owner references because cleanup must also occur when cross-namespace resources change + // (e.g. the Istio CR is deleted or pull secrets are removed from Installation), and Kubernetes + // garbage collection only handles same-namespace owner references. WaypointPullSecretLabel = "operator.tigera.io/istio-waypoint-pull-secret" ) @@ -119,104 +121,80 @@ func (r *ReconcileWaypointSecrets) Reconcile(ctx context.Context, request reconc reqLogger := log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) reqLogger.V(1).Info("Reconciling waypoint pull secrets") - // Get the Istio CR - if not found or being deleted, clean up all secrets. + // Determine which secrets need to exist (toCreate) based on current state, + // and which existing secrets are stale (toDelete). + var toCreate []client.Object + var toDelete []client.Object + + // Get the Istio CR - if not found or being deleted, all existing secrets are stale. instance := &operatorv1.Istio{} err := r.Get(ctx, utils.DefaultInstanceKey, instance) - if err != nil { - if errors.IsNotFound(err) { - reqLogger.V(1).Info("Istio CR not found, cleaning up all waypoint pull secrets") - return reconcile.Result{}, r.cleanupAllSecrets(ctx) - } + istioActive := err == nil && instance.DeletionTimestamp.IsZero() + if err != nil && !errors.IsNotFound(err) { return reconcile.Result{}, err } - if !instance.DeletionTimestamp.IsZero() { - reqLogger.V(1).Info("Istio CR being deleted, cleaning up all waypoint pull secrets") - return reconcile.Result{}, r.cleanupAllSecrets(ctx) - } - // Get Installation and pull secrets. - _, installation, err := utils.GetInstallation(ctx, r) - if err != nil { - if errors.IsNotFound(err) { - reqLogger.V(1).Info("Installation not found") - return reconcile.Result{}, nil + // Build the desired set of secrets if Istio is active. + targetNamespaces := map[string]bool{} + if istioActive { + _, installation, err := utils.GetInstallation(ctx, r) + if err != nil { + if errors.IsNotFound(err) { + reqLogger.V(1).Info("Installation not found") + return reconcile.Result{}, nil + } + return reconcile.Result{}, err } - return reconcile.Result{}, err - } - - pullSecrets, err := utils.GetNetworkingPullSecrets(installation, r) - if err != nil { - return reconcile.Result{}, err - } - - // If no pull secrets configured, clean up any previously copied secrets. - if len(pullSecrets) == 0 { - reqLogger.V(1).Info("No pull secrets configured, cleaning up waypoint pull secrets") - return reconcile.Result{}, r.cleanupAllSecrets(ctx) - } - // List all Gateway resources and filter for istio-waypoint class. - gatewayList := &gapi.GatewayList{} - if err := r.List(ctx, gatewayList); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to list Gateways: %w", err) - } + pullSecrets, err := utils.GetNetworkingPullSecrets(installation, r) + if err != nil { + return reconcile.Result{}, err + } - // Build set of target namespaces (deduplicated). - targetNamespaces := map[string]bool{} - for i := range gatewayList.Items { - gw := &gatewayList.Items[i] - if string(gw.Spec.GatewayClassName) == IstioWaypointClassName { - targetNamespaces[gw.Namespace] = true + // List all Gateway resources and filter for istio-waypoint class. + gatewayList := &gapi.GatewayList{} + if err := r.List(ctx, gatewayList); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to list Gateways: %w", err) } - } - // For each target namespace, copy pull secrets and apply the tracking label. - for ns := range targetNamespaces { - copied := secret.CopyToNamespace(ns, pullSecrets...) - var objs []client.Object - for _, s := range copied { - if s.Labels == nil { - s.Labels = map[string]string{} + for i := range gatewayList.Items { + gw := &gatewayList.Items[i] + if string(gw.Spec.GatewayClassName) == IstioWaypointClassName { + targetNamespaces[gw.Namespace] = true } - s.Labels[WaypointPullSecretLabel] = "true" - objs = append(objs, s) } - hdlr := utils.NewComponentHandler(log, r, r.scheme, nil) - component := render.NewPassthrough(objs, nil) - if err := hdlr.CreateOrUpdateOrDelete(ctx, component, nil); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to create/update pull secrets in namespace %s: %w", ns, err) + // Build desired secrets for each target namespace. + for ns := range targetNamespaces { + copied := secret.CopyToNamespace(ns, pullSecrets...) + for _, s := range copied { + if s.Labels == nil { + s.Labels = map[string]string{} + } + s.Labels[WaypointPullSecretLabel] = "true" + toCreate = append(toCreate, s) + } } } - // Clean up stale secrets from namespaces that no longer have waypoints. - if err := r.cleanupStaleSecrets(ctx, targetNamespaces); err != nil { - return reconcile.Result{}, err + // List all existing secrets managed by this controller and mark stale ones for deletion. + existingSecrets := &corev1.SecretList{} + if err := r.List(ctx, existingSecrets, client.MatchingLabels{WaypointPullSecretLabel: "true"}); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to list waypoint pull secrets: %w", err) } - - return reconcile.Result{}, nil -} - -// cleanupAllSecrets removes all secrets created by this controller. -func (r *ReconcileWaypointSecrets) cleanupAllSecrets(ctx context.Context) error { - return r.cleanupStaleSecrets(ctx, nil) -} - -// cleanupStaleSecrets removes tracking-labeled secrets from namespaces not in the active set. -func (r *ReconcileWaypointSecrets) cleanupStaleSecrets(ctx context.Context, activeNamespaces map[string]bool) error { - secretList := &corev1.SecretList{} - if err := r.List(ctx, secretList, client.MatchingLabels{WaypointPullSecretLabel: "true"}); err != nil { - return fmt.Errorf("failed to list waypoint pull secrets: %w", err) + for i := range existingSecrets.Items { + s := &existingSecrets.Items[i] + if !targetNamespaces[s.Namespace] { + toDelete = append(toDelete, s) + } } - for i := range secretList.Items { - s := &secretList.Items[i] - if !activeNamespaces[s.Namespace] { - if err := r.Delete(ctx, s); err != nil && !errors.IsNotFound(err) { - return fmt.Errorf("failed to delete stale pull secret %s/%s: %w", s.Namespace, s.Name, err) - } - } + // Use a single passthrough component to handle both creation and deletion. + hdlr := utils.NewComponentHandler(log, r, r.scheme, nil) + component := render.NewPassthrough(toCreate, toDelete) + if err := hdlr.CreateOrUpdateOrDelete(ctx, component, nil); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to reconcile waypoint pull secrets: %w", err) } - return nil + return reconcile.Result{}, nil }