From fa4383d18772afe5ece7b5636ebaf5248bcf455b Mon Sep 17 00:00:00 2001 From: Erik Stidham Date: Wed, 5 Aug 2026 10:12:50 -0500 Subject: [PATCH] Give the manager its own trusted bundle for the Tigera OIDC type For Authentication.spec.oidc.type: Tigera, handleCloudReconcile threw away the manager's component-named trusted bundle and replaced it with one from CreateTrustedBundleWithSystemRootCertificates(), which renders the default-named tigera-ca-bundle. That ConfigMap is owned by the core controller, which renders it into the same namespace - render.ManagerNamespace is common.CalicoNamespace - with a different set of certificates: the node and typha key pairs, plus the legacy typha-ca ConfigMap on clusters old enough to still carry one. Two controllers writing one ConfigMap with different contents overwrite each other, so the bundle alternates between two values. calico-manager mounts it directly and es-calico-kube-controllers inherits its hash annotations via LoadTrustedBundle, so both deployments alternate between two pod templates and roll repeatedly. The rollouts arrive in bursts rather than as a steady loop, since neither controller rebuilds the bundle on every reconcile. Both deployments stay available throughout, because each flip is an ordinary rolling update, so the cost is unnecessary restarts, log noise and a revision history that grows without bound. Kubernetes reuses the two ReplicaSets and bumps deployment.kubernetes.io/revision on each switch back, so the revision number can climb a long way while only a handful of ReplicaSets are ever created. The only thing the Tigera OIDC type actually needs is the system root certificates, so pass that as a flag to the one call that already builds the manager's bundle instead of rebuilding it. The manager is then always backed by a bundle named for itself, and handleCloudReconcile no longer takes part in building it - it just checks that the certificates the bundle trusts are available. That also drops a redundant AddCertificates loop which, on the cloud path, added every trusted secret a second time and so duplicated any certificate not signed by our own CA. The mount paths inside the container are constants, so only the ConfigMap and volume names change. Co-Authored-By: Claude Opus 5 --- pkg/controller/manager/manager_controller.go | 11 ++++++-- .../manager/manager_controller_cloud.go | 17 ++--------- .../manager/manager_controller_test.go | 28 +++++++++++++++++++ 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/pkg/controller/manager/manager_controller.go b/pkg/controller/manager/manager_controller.go index b326b4e45c..802514a828 100644 --- a/pkg/controller/manager/manager_controller.go +++ b/pkg/controller/manager/manager_controller.go @@ -446,8 +446,16 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ trustedSecretNames = append(trustedSecretNames, render.DexTLSSecretName) } + // Voltron has to validate a public OIDC issuer for the Tigera OIDC type, so that configuration also + // needs the system root certificates in its trusted bundle. Either way this stays a bundle named + // for this component: the default-named tigera-ca-bundle belongs to the core controller, which + // renders it into the same namespace with a different set of certificates, and two controllers + // writing one ConfigMap with different contents overwrite each other on every reconcile. + includeSystemRoots := authenticationCR != nil && authenticationCR.Spec.OIDC != nil && + authenticationCR.Spec.OIDC.Type == operatorv1.OIDCTypeTigera + bundleMaker, err := certificateManager.CreateNamedTrustedBundleFromSecrets(TrustedBundlePrefix, r.client, - helper.TruthNamespace(), false, trustedSecretNames...) + helper.TruthNamespace(), includeSystemRoots, trustedSecretNames...) if err != nil { r.status.SetDegraded(operatorv1.ResourceCreateError, "Error creating trusted bundle for manager", err, logc) } @@ -462,7 +470,6 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ logc, helper, tenant, - authenticationCR, certificateManager, bundleMaker, trustedSecretNames, diff --git a/pkg/controller/manager/manager_controller_cloud.go b/pkg/controller/manager/manager_controller_cloud.go index 5f0c56591b..39d1f177e6 100644 --- a/pkg/controller/manager/manager_controller_cloud.go +++ b/pkg/controller/manager/manager_controller_cloud.go @@ -70,22 +70,15 @@ func (r *ReconcileManager) handleCloudReconcile( reqLogger logr.Logger, helper utils.NamespaceHelper, tenant *operatorv1.Tenant, - authenticationCR *operatorv1.Authentication, certificateManager certificatemanager.CertificateManager, bundleMaker certificatemanagement.TrustedBundle, trustedSecretNames []string, requestNamespace string, ) (certificatemanagement.TrustedBundle, render.ManagerCloudResources, *operatorv1.Tenant, *reconcile.Result, error) { - if authenticationCR != nil && authenticationCR.Spec.OIDC != nil && authenticationCR.Spec.OIDC.Type == operatorv1.OIDCTypeTigera { - var err error - bundleMaker, err = certificateManager.CreateTrustedBundleWithSystemRootCertificates() - if err != nil { - r.status.SetDegraded(operatorv1.ResourceCreateError, "failed to create trusted bundle with system root certs", err, reqLogger) - return nil, render.ManagerCloudResources{}, nil, nil, err - } - } - + // The trusted bundle is built by the caller, which decides both its name and whether it carries the + // system root certificates. All that is left to do here is confirm the certificates it trusts are + // available before the manager is rendered. for _, secret := range trustedSecretNames { certificate, err := certificateManager.GetCertificate(r.client, secret, helper.TruthNamespace()) if err != nil { @@ -97,10 +90,6 @@ func (r *ReconcileManager) handleCloudReconcile( // stop reconciler iteration with no error as it is waiting for a resource to become available return nil, render.ManagerCloudResources{}, nil, &reconcile.Result{}, nil } - - if bundleMaker != nil { - bundleMaker.AddCertificates(certificate) - } } mcr := render.ManagerCloudResources{ diff --git a/pkg/controller/manager/manager_controller_test.go b/pkg/controller/manager/manager_controller_test.go index da3e7e77fa..46aed58b56 100644 --- a/pkg/controller/manager/manager_controller_test.go +++ b/pkg/controller/manager/manager_controller_test.go @@ -578,6 +578,34 @@ var _ = Describe("Manager controller tests", func() { Expect(namespace.Labels["pod-security.kubernetes.io/enforce-version"]).To(Equal("latest")) }) + It("should leave the manager's trusted bundle alone on the cloud path", func() { + // The default-named tigera-ca-bundle belongs to the core controller, which renders it + // into the same namespace with a different set of certificates. If the manager writes + // that same ConfigMap the two controllers overwrite each other on every reconcile, + // which rolls every workload that mounts or inherits the bundle. The cloud path must + // therefore hand back the component-named bundle it was given, not substitute its own. + helper := utils.NewNamespaceHelper(false, render.ManagerNamespace, "") + + // handleCloudReconcile resolves the tenant from this ConfigMap. + Expect(c.Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: utils.CloudAuthConfig, Namespace: common.OperatorNamespace()}, + Data: map[string]string{"tenantID": "Auth0ID"}, + })).NotTo(HaveOccurred()) + + in, err := certificateManager.CreateNamedTrustedBundleFromSecrets(TrustedBundlePrefix, c, helper.TruthNamespace(), true) + Expect(err).NotTo(HaveOccurred()) + + out, _, _, res, err := r.handleCloudReconcile(ctx, log, helper, nil, certificateManager, in, nil, "") + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(BeNil()) + + Expect(out).NotTo(BeNil()) + Expect(out.ConfigMap(helper.InstallNamespace()).Name). + To(Equal(certificatemanagement.TrustedBundleName(render.ManagerName, true))) + Expect(out.ConfigMap(helper.InstallNamespace()).Name). + NotTo(Equal(certificatemanagement.TrustedCertConfigMapName)) + }) + Context("image reconciliation", func() { It("should use builtin images", func() { mockStatus.On("RemoveCertificateSigningRequests", mock.Anything).Return()