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()