Operator certificate handling refactor

- Move the ServiceServingCertCAFile var from options.go to util.go
- Fix the MountPath of the OCp injected CA bundle and improve handling of the configmap
- Readd the global refreshing transport to the S3 clients in prepareAWSBackingStore
- Rename AddToRootCAs to CombineCaBundle for added clarity
- Add an explanation about the current certificate flow and its implication on other pods, and their HTTP agents

Signed-off-by: Ben <[email protected]>

Author: Neon-White

pkg/options/options.go

@@ -48,9 +48,6 @@ const (
 
 	// SystemName is a constant as we want just a single system per namespace
 	SystemName = "noobaa"
-
-	// ServiceServingCertCAFile points to OCP root CA to be added to the default root CA list
-	ServiceServingCertCAFile = "/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt"
 )
 
 // Namespace is the target namespace for locating the noobaa system

pkg/system/phase2_creating.go

@@ -603,10 +603,11 @@ func (r *Reconciler) SetDesiredCoreApp() error {
 				util.MergeVolumeMountList(&c.VolumeMounts, &dbSecretVolumeMounts)
 			}
 
-			if util.KubeCheckQuiet(r.CaBundleConf) {
+			// we want to check that the cm exists and also that it has data in it
+			if util.KubeCheckQuiet(r.CaBundleConf) && len(r.CaBundleConf.Data) > 0 {
 				configMapVolumeMounts := []corev1.VolumeMount{{
 					Name:      r.CaBundleConf.Name,
-					MountPath: "/etc/ocp-injected-ca-bundle.crt",
+					MountPath: "/etc/ocp-injected-ca-bundle",
 					ReadOnly:  true,
 				}}
 				util.MergeVolumeMountList(&c.VolumeMounts, &configMapVolumeMounts)
@@ -686,10 +687,11 @@ func (r *Reconciler) SetDesiredCoreApp() error {
 					Limits:   logResourceList,
 				}
 			}
-			if util.KubeCheckQuiet(r.CaBundleConf) {
+			// we want to check that the cm exists and also that it has data in it
+			if util.KubeCheckQuiet(r.CaBundleConf) && len(r.CaBundleConf.Data) > 0 {
 				configMapVolumeMounts := []corev1.VolumeMount{{
 					Name:      r.CaBundleConf.Name,
-					MountPath: "/etc/ocp-injected-ca-bundle.crt",
+					MountPath: "/etc/ocp-injected-ca-bundle",
 					ReadOnly:  true,
 				}}
 				util.MergeVolumeMountList(&c.VolumeMounts, &configMapVolumeMounts)
@@ -729,7 +731,8 @@ func (r *Reconciler) SetDesiredCoreApp() error {
 
 	r.CoreApp.Spec.Template.Annotations["noobaa.io/configmap-hash"] = r.CoreAppConfig.Annotations["noobaa.io/configmap-hash"]
 
-	if util.KubeCheckQuiet(r.CaBundleConf) {
+	// we want to check that the cm exists and also that it has data in it
+	if util.KubeCheckQuiet(r.CaBundleConf) && len(r.CaBundleConf.Data) > 0 {
 		configMapVolumes := []corev1.Volume{{
 			Name: r.CaBundleConf.Name,
 			VolumeSource: corev1.VolumeSource{

pkg/system/phase4_configuring.go

@@ -546,7 +546,8 @@ func (r *Reconciler) setDesiredEndpointMounts(podSpec *corev1.PodSpec, container
 		util.MergeVolumeMountList(&container.VolumeMounts, &dbSecretVolumeMounts)
 	}
 
-	if util.KubeCheckQuiet(r.CaBundleConf) {
+	// we want to check that the cm exists and also that it has data in it
+	if util.KubeCheckQuiet(r.CaBundleConf) && len(r.CaBundleConf.Data) > 0 {
 		configMapVolumes := []corev1.Volume{{
 			Name: r.CaBundleConf.Name,
 			VolumeSource: corev1.VolumeSource{
@@ -564,7 +565,7 @@ func (r *Reconciler) setDesiredEndpointMounts(podSpec *corev1.PodSpec, container
 		util.MergeVolumeList(&podSpec.Volumes, &configMapVolumes)
 		configMapVolumeMounts := []corev1.VolumeMount{{
 			Name:      r.CaBundleConf.Name,
-			MountPath: "/etc/ocp-injected-ca-bundle.crt",
+			MountPath: "/etc/ocp-injected-ca-bundle",
 			ReadOnly:  true,
 		}}
 		util.MergeVolumeMountList(&container.VolumeMounts, &configMapVolumeMounts)
@@ -1108,6 +1109,10 @@ func (r *Reconciler) prepareAWSBackingStore() error {
 				*result.Credentials.SecretAccessKey,
 				*result.Credentials.SessionToken,
 			),
+			HTTPClient: &http.Client{
+				Transport: util.GlobalCARefreshingTransport,
+				Timeout:   10 * time.Second,
+			},
 			Region: &region,
 		}
 	} else { // handle AWS long-lived credentials (not STS)
@@ -1117,6 +1122,10 @@ func (r *Reconciler) prepareAWSBackingStore() error {
 				cloudCredsSecret.StringData["aws_secret_access_key"],
 				"",
 			),
+			HTTPClient: &http.Client{
+				Transport: util.GlobalCARefreshingTransport,
+				Timeout:   10 * time.Second,
+			},
 			Region: &region,
 		}
 	}

pkg/system/reconciler.go

@@ -68,7 +68,7 @@ type Reconciler struct {
 	OperatorVersion          string
 	OAuthEndpoints           *util.OAuth2Endpoints
 	PostgresConnectionString string
-	ApplyCAsToPods           string
+	ApplyCAsToPods           string // the path that will be applied to the core and endpoint pods in NODE_EXTRA_CA_CERTS
 
 	NooBaa                    *nbv1.NooBaa
 	ServiceAccount            *corev1.ServiceAccount
@@ -286,7 +286,7 @@ func NewReconciler(
 	r.RouteS3.Name = r.ServiceS3.Name
 	r.RouteSts.Name = r.ServiceSts.Name
 	r.DeploymentEndpoint.Name = r.Request.Name + "-endpoint"
-	r.CaBundleConf.Name = r.Request.Name + "-ca-inject"
+	r.CaBundleConf.Name = "ocp-injected-ca-bundle"
 	r.KedaScaled.Name = r.Request.Name
 	r.AdapterHPA.Name = r.Request.Name + "-hpav2"
 	r.BucketLoggingPVC.Name = r.Request.Name + "-bucket-logging-pvc"
@@ -409,9 +409,30 @@ func (r *Reconciler) Reconcile() (reconcile.Result, error) {
 		}
 	}
 
-	err = util.AddToRootCAs(options.ServiceServingCertCAFile)
+	/*
+	This code is problematic due to the way other parts of the product work.
+	On the core side, get_unsecured_agent() relies on the presence of the NODE_EXTRA_CA_CERTS
+	environment variable to determine whether an HTTP or HTTPS client should be used.
+
+	At the time of writing this comment, if the environment variable is not set, an HTTP agent
+	will be used for *all* S3-compatible domains that aren't under amazonaws.com - including
+	domains that are already present by default in the system's certificate store.
+
+	Forcing the environment variable to always be set leads to a different problem where
+	some things might fail - e.g. the admission tests that rely on creating a namespacestore
+	that points towards NooBaa's (self-signed) S3 service. In that case, the HTTPS agent fails
+	due to the self-signed certificate.	
+
+	Also, note that the code that combines certificates only applies to the operator.
+	Based on whether the certificate bundling was successful, the operator will set the value of
+	NODE_EXTRA_CA_CERTS in endpoints and core pods to point to *the system generated service-serving certs*.
+
+	At the time of writing, user certs are not included at any point.
+	*/
+	
+	err = util.CombineCaBundle(util.ServiceServingCertCAFile)
 	if err == nil {
-		r.ApplyCAsToPods = options.ServiceServingCertCAFile
+		r.ApplyCAsToPods = util.ServiceServingCertCAFile
 	} else if !os.IsNotExist(err) {
 		log.Errorf("❌ NooBaa %q failed to add root CAs to system default", r.NooBaa.Name)
 		res.RequeueAfter = 3 * time.Second

pkg/util/util.go

@@ -84,6 +84,12 @@ const (
 
 	topologyConstraintsEnabledKubeVersion = "1.26.0"
 	trueStr                               = "true"
+
+	// ServiceServingCertCAFile points to OCP default root CA list
+	ServiceServingCertCAFile = "/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt"
+
+	// InjectedBundleCertCAFile points to OCP root CA to be added to the default root CA list
+	InjectedBundleCertCAFile = "/etc/ocp-injected-ca-bundle/ca-bundle.crt"
 )
 
 // OAuth2Endpoints holds OAuth2 endpoints information.
@@ -143,15 +149,15 @@ var (
 	}
 )
 
-// AddToRootCAs adds a local cert file to Our GlobalCARefreshingTransport
-func AddToRootCAs(localCertFile string) error {
+// CombineCaBundle combines a local cert file to Our GlobalCARefreshingTransport
+func CombineCaBundle(localCertFile string) error {
 	rootCAs, _ := x509.SystemCertPool()
 	if rootCAs == nil {
 		rootCAs = x509.NewCertPool()
 	}
 
 	var certFiles = []string{
-		"/etc/ocp-injected-ca-bundle.crt",
+		InjectedBundleCertCAFile,
 		localCertFile,
 	}
 
@@ -169,7 +175,7 @@ func AddToRootCAs(localCertFile string) error {
 		}
 
 		// Trust the augmented cert pool in our client
-		log.Infof("Successfuly appended %q to RootCAs", certFile)
+		log.Infof("Successfully appended %q to RootCAs", certFile)
 	}
 	GlobalCARefreshingTransport.TLSClientConfig.RootCAs = rootCAs
 	return nil