Use trusted bundle with root CAs for multi-tenant manager pods (#2990)

* Use trusted bundle with root CAs for multi-tenant manager pods

* Fix tests

* Add ut

* Add basic UT for tenant secret controller

* Load the correct bundle

* Fix static checks

pkg/controller/certificatemanager/certificatemanager.go

@@ -89,12 +89,17 @@ type CertificateManager interface {
 	// - A bundle with Calico's root certificates + any user supplied certificates in /etc/pki/tls/certs/tigera-ca-bundle.crt.
 	// - A system root certificate bundle in /etc/pki/tls/certs/ca-bundle.crt.
 	CreateTrustedBundleWithSystemRootCertificates(certificates ...certificatemanagement.CertificateInterface) (certificatemanagement.TrustedBundle, error)
+	// CreateMultiTenantTrustedBundleWithSystemRootCertificates is an alternative to CreateTrustedBundleWithSystemRootCertificates that is appropriate for
+	// multi-tenant management clusters.
+	CreateMultiTenantTrustedBundleWithSystemRootCertificates(certificates ...certificatemanagement.CertificateInterface) (certificatemanagement.TrustedBundle, error)
 	// AddToStatusManager lets the status manager monitor pending CSRs if the certificate management is enabled.
 	AddToStatusManager(manager status.StatusManager, namespace string)
 	// KeyPair Returns the CA KeyPairInterface, so it can be rendered in the operator namespace.
 	KeyPair() certificatemanagement.KeyPairInterface
-	// Loads an existing trusted bundle to pass to render.
+	// LoadTrustedBundle loads an existing trusted bundle to pass to render.
 	LoadTrustedBundle(context.Context, client.Client, string) (certificatemanagement.TrustedBundleRO, error)
+	// LoadMultiTenantTrustedBundleWithRootCertificates loads an existing trusted bundle with system root certificates to pass to render.
+	LoadMultiTenantTrustedBundleWithRootCertificates(context.Context, client.Client, string) (certificatemanagement.TrustedBundleRO, error)
 }
 
 type Option func(cm *certificateManager) error
@@ -488,13 +493,33 @@ func (cm *certificateManager) CreateTrustedBundleWithSystemRootCertificates(cert
 	return certificatemanagement.CreateTrustedBundleWithSystemRootCertificates(append([]certificatemanagement.CertificateInterface{cm.keyPair}, certificates...)...)
 }
 
+func (cm *certificateManager) CreateMultiTenantTrustedBundleWithSystemRootCertificates(certificates ...certificatemanagement.CertificateInterface) (certificatemanagement.TrustedBundle, error) {
+	return certificatemanagement.CreateMultiTenantTrustedBundleWithSystemRootCertificates(append([]certificatemanagement.CertificateInterface{cm.keyPair}, certificates...)...)
+}
+
 func (cm *certificateManager) LoadTrustedBundle(ctx context.Context, client client.Client, ns string) (certificatemanagement.TrustedBundleRO, error) {
+	return cm.loadTrustedBundle(ctx, client, ns, certificatemanagement.TrustedCertConfigMapName)
+}
+
+func (cm *certificateManager) LoadMultiTenantTrustedBundleWithRootCertificates(ctx context.Context, client client.Client, ns string) (certificatemanagement.TrustedBundleRO, error) {
+	return cm.loadTrustedBundle(ctx, client, ns, certificatemanagement.TrustedCertConfigMapNamePublic)
+}
+
+func (cm *certificateManager) loadTrustedBundle(ctx context.Context, client client.Client, ns string, name string) (certificatemanagement.TrustedBundleRO, error) {
+	// Get the ConfigMap containing the actual certificates.
 	obj := &corev1.ConfigMap{}
-	k := types.NamespacedName{Name: certificatemanagement.TrustedCertConfigMapName, Namespace: ns}
+	k := types.NamespacedName{Name: name, Namespace: ns}
 	if err := client.Get(ctx, k, obj); err != nil {
 		return nil, err
 	}
-	a := newReadOnlyTrustedBundle(cm, len(obj.Data[certificatemanagement.RHELRootCertificateBundleName]) > 0)
+
+	// Create a new readOnlyTrustedBundle based on the given configuration.
+	includeSystemCerts := len(obj.Data[certificatemanagement.RHELRootCertificateBundleName]) > 0
+	useMultiTenantName := name == certificatemanagement.TrustedCertConfigMapNamePublic
+	a := newReadOnlyTrustedBundle(cm, includeSystemCerts, useMultiTenantName)
+
+	// Augment it with annotations from the actual ConfigMap so that we inherit the hash annotations used to
+	// detect changes to the ConfigMap's contents.
 	for key, val := range obj.Annotations {
 		if strings.HasPrefix(key, "hash.operator.tigera.io/") {
 			a.annotations[key] = val
@@ -506,9 +531,13 @@ func (cm *certificateManager) LoadTrustedBundle(ctx context.Context, client clie
 // newReadOnlyTrustedBundle creates a new readOnlyTrustedBundle. If system is true, the bundle will include a system root certificate bundle.
 // TrustedBundleRO is useful for mounting a bundle of certificates to trust in a pod without the ability to modify the bundle, and allows
 // one controller to create the bundle and another to mount it.
-func newReadOnlyTrustedBundle(cm CertificateManager, system bool) *readOnlyTrustedBundle {
-	if system {
+func newReadOnlyTrustedBundle(cm CertificateManager, includeSystemCerts, multiTenant bool) *readOnlyTrustedBundle {
+	if includeSystemCerts {
 		bundle, _ := cm.CreateTrustedBundleWithSystemRootCertificates()
+		if multiTenant {
+			// For multi-tenant clusters, the system root certificate bundle uses a different name. Load it instead.
+			bundle, _ = cm.CreateMultiTenantTrustedBundleWithSystemRootCertificates()
+		}
 		return &readOnlyTrustedBundle{annotations: map[string]string{}, bundle: bundle}
 	}
 	return &readOnlyTrustedBundle{annotations: map[string]string{}, bundle: cm.CreateTrustedBundle()}

pkg/controller/certificatemanager/certificatemanager_test.go

@@ -603,6 +603,50 @@ var _ = Describe("Test CertificateManagement suite", func() {
 			numBlocks := strings.Count(bundle, "-----BEGIN CERTIFICATE-----")
 			Expect(numBlocks > 1).To(BeTrue()) // We expect tens of them most likely.
 		})
+
+		It("should load the system certificates into a multi-tenant bundle", func() {
+			if runtime.GOOS != "linux" {
+				Skip("Skip for users that run this test outside of a container on incompatible systems.")
+			}
+			trustedBundle, err := certificateManager.CreateMultiTenantTrustedBundleWithSystemRootCertificates()
+			Expect(err).NotTo(HaveOccurred())
+
+			configMap := trustedBundle.ConfigMap(appNs)
+			Expect(configMap.Name).To(Equal("tigera-ca-bundle-system-certs"))
+
+			Expect(configMap.Namespace).To(Equal(appNs))
+			Expect(configMap.Annotations).To(HaveKey("tigera-operator.hash.operator.tigera.io/tigera-ca-private"))
+			Expect(configMap.Annotations).To(HaveKey("hash.operator.tigera.io/system"))
+			Expect(configMap.TypeMeta).To(Equal(metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}))
+
+			By("counting the number of pem blocks in the configmap")
+			bundle := configMap.Data[certificatemanagement.RHELRootCertificateBundleName]
+			numBlocks := strings.Count(bundle, "-----BEGIN CERTIFICATE-----")
+			Expect(numBlocks > 1).To(BeTrue()) // We expect tens of them most likely.
+
+			By("verifying the volume is correct")
+			volume := trustedBundle.Volume()
+			Expect(volume.ConfigMap).NotTo(BeNil())
+			Expect(volume.Name).To(Equal("tigera-ca-bundle-system-certs"))
+			Expect(volume.VolumeSource.ConfigMap.Name).To(Equal("tigera-ca-bundle-system-certs"))
+
+			By("verifying the volume mount is correct")
+			mounts := trustedBundle.VolumeMounts(rmeta.OSTypeLinux)
+			Expect(mounts).To(HaveLen(2))
+			Expect(mounts).To(Equal([]corev1.VolumeMount{
+				{
+					Name:      "tigera-ca-bundle-system-certs",
+					MountPath: "/etc/pki/tls/certs",
+					ReadOnly:  true,
+				},
+				{
+					Name:      "tigera-ca-bundle-system-certs",
+					MountPath: "/etc/pki/tls/cert.pem",
+					SubPath:   "ca-bundle.crt",
+					ReadOnly:  true,
+				},
+			}))
+		})
 	})
 })
 

pkg/controller/manager/manager_controller.go

@@ -599,8 +599,9 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ
 
 	trustedBundle := bundleMaker.(certificatemanagement.TrustedBundleRO)
 	if r.multiTenant {
-		// for multi-tenant systems, we load the pre-created bundle for this tenant instead of using the one we built here.
-		trustedBundle, err = certificateManager.LoadTrustedBundle(ctx, r.client, helper.InstallNamespace())
+		// For multi-tenant systems, we load the pre-created bundle for this tenant instead of using the one we built here.
+		// Multi-tenant managers need the bundle variant that includes system root certificates, in order to verify external auth providers.
+		trustedBundle, err = certificateManager.LoadMultiTenantTrustedBundleWithRootCertificates(ctx, r.client, helper.InstallNamespace())
 		if err != nil {
 			r.status.SetDegraded(operatorv1.ResourceReadError, "Error getting trusted bundle", err, logc)
 			return reconcile.Result{}, err

pkg/controller/manager/manager_controller_test.go

@@ -1079,15 +1079,19 @@ var _ = Describe("Manager controller tests", func() {
 				managerTLSTenantA, err := certificateManagerTenantA.GetOrCreateKeyPair(c, render.ManagerInternalTLSSecretName, tenantANamespace, []string{render.ManagerInternalTLSSecretName})
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c.Create(ctx, managerTLSTenantA.Secret(tenantANamespace))).NotTo(HaveOccurred())
-				Expect(c.Create(ctx, certificateManagerTenantA.CreateTrustedBundle().ConfigMap(tenantANamespace))).NotTo(HaveOccurred())
+				bundleA, err := certificateManagerTenantA.CreateMultiTenantTrustedBundleWithSystemRootCertificates()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(c.Create(ctx, bundleA.ConfigMap(tenantANamespace))).NotTo(HaveOccurred())
 
 				certificateManagerTenantB, err := certificatemanager.Create(c, nil, "", tenantBNamespace, certificatemanager.AllowCACreation(), certificatemanager.WithTenant(tenantB))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c.Create(ctx, certificateManagerTenantB.KeyPair().Secret(tenantBNamespace)))
 				managerTLSTenantB, err := certificateManagerTenantB.GetOrCreateKeyPair(c, render.ManagerInternalTLSSecretName, tenantBNamespace, []string{render.ManagerInternalTLSSecretName})
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c.Create(ctx, managerTLSTenantB.Secret(tenantBNamespace))).NotTo(HaveOccurred())
-				Expect(c.Create(ctx, certificateManagerTenantB.CreateTrustedBundle().ConfigMap(tenantBNamespace))).NotTo(HaveOccurred())
+				bundleB, err := certificateManagerTenantB.CreateMultiTenantTrustedBundleWithSystemRootCertificates()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(c.Create(ctx, bundleB.ConfigMap(tenantBNamespace))).NotTo(HaveOccurred())
 
 				err = c.Create(ctx, &operatorv1.Manager{
 					ObjectMeta: metav1.ObjectMeta{

pkg/controller/secrets/secrets_suite_test.go

@@ -0,0 +1,34 @@
+// Copyright (c) 2023 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 secrets
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"github.com/onsi/ginkgo/reporters"
+	uzap "go.uber.org/zap"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+)
+
+func TestStatus(t *testing.T) {
+	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true), zap.Level(uzap.NewAtomicLevelAt(uzap.DebugLevel))))
+	RegisterFailHandler(Fail)
+	junitReporter := reporters.NewJUnitReporter("../../../report/ut/secrets_controller_suite.xml")
+	RunSpecsWithDefaultAndCustomReporters(t, "pkg/controller/secrets Suite", []Reporter{junitReporter})
+}

pkg/controller/secrets/tenant_controller.go

@@ -170,7 +170,15 @@ func (r *TenantController) Reconcile(ctx context.Context, request reconcile.Requ
 	trustedBundle := cm.CreateTrustedBundle()
 	trustedBundle.AddCertificates(clusterCA)
 
-	// TODO: Provision a trusted bundle that includes system certificates for components that need it.
+	// We also need a trusted bundle that includes the system root certificates in addition to the certificates
+	// listed above, so that components that talk to public endpoints can verify them. In a multi-tenant cluster, this
+	// bundle will co-exist in the same namespace as the default trusted bundle, but with a different name.
+	trustedBundleWithSystemCAs, err := cm.CreateMultiTenantTrustedBundleWithSystemRootCertificates()
+	if err != nil {
+		r.status.SetDegraded(operatorv1.ResourceReadError, "Error querying system root certificates", err, logc)
+		return reconcile.Result{}, err
+	}
+	trustedBundleWithSystemCAs.AddCertificates(clusterCA)
 
 	component := rcertificatemanagement.CertificateManagement(&rcertificatemanagement.Config{
 		Namespace:      tenant.Namespace,
@@ -182,12 +190,21 @@ func (r *TenantController) Reconcile(ctx context.Context, request reconcile.Requ
 		KeyPairOptions: keyPairOptions,
 		TrustedBundle:  trustedBundle,
 	})
+	systemRootsComponent := rcertificatemanagement.CertificateManagement(&rcertificatemanagement.Config{
+		Namespace:      tenant.Namespace,
+		TruthNamespace: tenant.Namespace,
+		TrustedBundle:  trustedBundleWithSystemCAs,
+	})
 
 	hdler := utils.NewComponentHandler(logc, r.client, r.scheme, tenant)
 	if err = hdler.CreateOrUpdateOrDelete(ctx, component, r.status); err != nil {
 		r.status.SetDegraded(operatorv1.ResourceUpdateError, "Error creating / updating resource", err, logc)
 		return reconcile.Result{}, err
 	}
+	if err = hdler.CreateOrUpdateOrDelete(ctx, systemRootsComponent, r.status); err != nil {
+		r.status.SetDegraded(operatorv1.ResourceUpdateError, "Error creating / updating trusted bundle with public CAs", err, logc)
+		return reconcile.Result{}, err
+	}
 
 	return reconcile.Result{}, nil
 }