✨ Certificate support for image registry (#960)

* Certificate support for image registry

Remove the InsecureSkipTLSVerify annotations.

* Create a ClusterIssuer CA (via openssl) that is used by OLMv1 e2e
* Update the operator controller to specify a cert directory, rather than a
  single file.
* Use this directory for catalogd and image-registries
* Update the deployment to reference CAs appropriately

Signed-off-by: Todd Short <tshort@redhat.com>

* fixup! Certificate support for image registry

Signed-off-by: Todd Short <tshort@redhat.com>

* fixup! Certificate support for image registry

Signed-off-by: Todd Short <tshort@redhat.com>

* fixup! Certificate support for image registry

Signed-off-by: Todd Short <tshort@redhat.com>

* fixup! Certificate support for image registry

* fixup! Certificate support for image registry

* fixup! Certificate support for image registry

---------

Signed-off-by: Todd Short <tshort@redhat.com>

Tiltfile

@@ -1,7 +1,7 @@
 if not os.path.exists('../tilt-support'):
     fail('Please clone https://github.com/operator-framework/tilt-support to ../tilt-support')
 
-load('../tilt-support/Tiltfile', 'deploy_repo')
+load('../tilt-support/Tiltfile', 'deploy_repo', 'process_yaml')
 
 config.define_string_list('repos', args=True)
 cfg = config.parse()
@@ -16,6 +16,8 @@ repo = {
     'starting_debug_port': 30000,
 }
 
+process_yaml("testdata/certs/issuers.yaml")
+
 for r in repos:
     if r == 'operator-controller':
         deploy_repo('operator-controller', repo)

cmd/manager/main.go

@@ -80,11 +80,11 @@ func main() {
 		cachePath                 string
 		operatorControllerVersion bool
 		systemNamespace           string
-		caCert                    string
+		caCertDir                 string
 	)
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
-	flag.StringVar(&caCert, "ca-cert", "", "The TLS certificate to use for verifying HTTPS connections to the Catalogd web server.")
+	flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of TLS certificate to use for verifying HTTPS connections to the Catalogd and docker-registry web servers.")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
@@ -153,7 +153,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	httpClient, err := httputil.BuildHTTPClient(caCert)
+	httpClient, err := httputil.BuildHTTPClient(caCertDir)
 	if err != nil {
 		setupLog.Error(err, "unable to create catalogd http client")
 	}
@@ -224,6 +224,7 @@ func main() {
 		InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg},
 		Handler:               registryv1handler.HandlerFunc(registry.HandleBundleDeployment),
 		Finalizers:            clusterExtensionFinalizers,
+		CaCertDir:             caCertDir,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension")
 		os.Exit(1)

config/overlays/tls/kustomization.yaml

@@ -12,9 +12,11 @@ apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 resources:
 - ../../base
+- resources/manager_cert.yaml
 
 patches:
 - target:
     kind: Deployment
     name: controller-manager
-  path: patches/manager_deployment_cert.yaml
+  path: patches/manager_deployment_cert.yaml
+- path: patches/manager_cert_patch.yaml

config/overlays/tls/patches/manager_cert_patch.yaml

@@ -0,0 +1,23 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: controller-manager
+  namespace: system
+spec:
+  template:
+    spec:
+      containers:
+      - name: kube-rbac-proxy
+      - name: manager
+        volumeMounts:
+        - name: e2e-cert
+          mountPath: /var/certs/olm-ca.crt
+          subPath: olm-ca.crt
+          readOnly: true
+      volumes:
+      - name: e2e-cert
+        secret:
+          secretName: olmv1-cert
+          items:
+            - key: ca.crt
+              path: olm-ca.crt

config/overlays/tls/patches/manager_deployment_cert.yaml

@@ -1,9 +1,9 @@
 - op: add
   path: /spec/template/spec/volumes/-
-  value: {"name":"ca-certificate", "secret":{"secretName":"catalogd-catalogserver-cert", "optional": false, "items": [{"key": "tls.crt", "path": "tls.crt"}]}}
+  value: {"name":"catalogd-certificate", "secret":{"secretName":"catalogd-catalogserver-cert", "optional": false, "items": [{"key": "ca.crt", "path": "catalogd.crt"}]}}
 - op: add
   path: /spec/template/spec/containers/0/volumeMounts/-
-  value: {"name":"ca-certificate", "readOnly": true, "mountPath":"/var/certs"}
+  value: {"name":"catalogd-certificate", "readOnly": true, "mountPath":"/var/certs/catalogd.crt", "subPath":"catalogd.crt"}
 - op: add
   path: /spec/template/spec/containers/0/args/-
-  value: "--ca-cert=/var/certs/tls.crt"
+  value: "--ca-certs-dir=/var/certs"

config/overlays/tls/resources/manager_cert.yaml

@@ -0,0 +1,16 @@
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: olmv1-cert
+spec:
+  secretName: olmv1-cert
+  dnsNames:
+    - operator-controller.olmv1-system.svc
+    - operator-controller.olmv1-system.svc.cluster.local
+  privateKey:
+    algorithm: ECDSA
+    size: 256
+  issuerRef:
+    name: olmv1-ca
+    kind: ClusterIssuer
+    group: cert-manager.io

internal/controllers/clusterextension_controller.go

@@ -73,6 +73,7 @@ import (
 	catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
 	catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
 	"github.com/operator-framework/operator-controller/internal/conditionsets"
+	"github.com/operator-framework/operator-controller/internal/httputil"
 	"github.com/operator-framework/operator-controller/internal/labels"
 )
 
@@ -90,16 +91,13 @@ type ClusterExtensionReconciler struct {
 	cache                 cache.Cache
 	InstalledBundleGetter InstalledBundleGetter
 	Finalizers            crfinalizer.Finalizers
+	CaCertDir             string
 }
 
 type InstalledBundleGetter interface {
 	GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error)
 }
 
-const (
-	bundleConnectionAnnotation string = "bundle.connection.config/insecureSkipTLSVerify"
-)
-
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/status,verbs=update;patch
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/finalizers,verbs=update
@@ -249,7 +247,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 	// Generate a BundleDeployment from the ClusterExtension to Unpack.
 	// Note: The BundleDeployment here is not a k8s API, its a simple Go struct which
 	// necessary embedded values.
-	bd := r.generateBundleDeploymentForUnpack(bundle.Image, ext)
+	bd := r.generateBundleDeploymentForUnpack(ctx, bundle.Image, ext)
 	unpackResult, err := r.Unpacker.Unpack(ctx, bd)
 	if err != nil {
 		setStatusUnpackFailed(ext, err.Error())
@@ -532,7 +530,11 @@ func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetad
 	}
 }
 
-func (r *ClusterExtensionReconciler) generateBundleDeploymentForUnpack(bundlePath string, ce *ocv1alpha1.ClusterExtension) *rukpakv1alpha2.BundleDeployment {
+func (r *ClusterExtensionReconciler) generateBundleDeploymentForUnpack(ctx context.Context, bundlePath string, ce *ocv1alpha1.ClusterExtension) *rukpakv1alpha2.BundleDeployment {
+	certData, err := httputil.LoadCerts(r.CaCertDir)
+	if err != nil {
+		log.FromContext(ctx).WithName("operator-controller").WithValues("cluster-extension", ce.GetName()).Error(err, "unable to get TLS certificate")
+	}
 	return &rukpakv1alpha2.BundleDeployment{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       ce.Kind,
@@ -547,25 +549,14 @@ func (r *ClusterExtensionReconciler) generateBundleDeploymentForUnpack(bundlePat
 			Source: rukpakv1alpha2.BundleSource{
 				Type: rukpakv1alpha2.SourceTypeImage,
 				Image: &rukpakv1alpha2.ImageSource{
-					Ref:                   bundlePath,
-					InsecureSkipTLSVerify: isInsecureSkipTLSVerifySet(ce),
+					Ref:             bundlePath,
+					CertificateData: certData,
 				},
 			},
 		},
 	}
 }
 
-func isInsecureSkipTLSVerifySet(ce *ocv1alpha1.ClusterExtension) bool {
-	if ce == nil {
-		return false
-	}
-	value, ok := ce.Annotations[bundleConnectionAnnotation]
-	if !ok {
-		return false
-	}
-	return value == "true"
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	controller, err := ctrl.NewControllerManagedBy(mgr).

internal/httputil/httputil.go

@@ -5,30 +5,57 @@ import (
 	"crypto/x509"
 	"net/http"
 	"os"
+	"path/filepath"
+	"strings"
 	"time"
 )
 
-func BuildHTTPClient(caCert string) (*http.Client, error) {
-	httpClient := &http.Client{Timeout: 10 * time.Second}
-
-	if caCert != "" {
-		// tlsFileWatcher, err := certwatcher.New(caCert, "")
+func LoadCerts(caDir string) (string, error) {
+	if caDir == "" {
+		return "", nil
+	}
 
-		cert, err := os.ReadFile(caCert)
-		if err != nil {
-			return nil, err
-		}
-		caCertPool := x509.NewCertPool()
-		caCertPool.AppendCertsFromPEM(cert)
-		tlsConfig := &tls.Config{
-			RootCAs:    caCertPool,
-			MinVersion: tls.VersionTLS12,
+	certs := []string{}
+	dirEntries, err := os.ReadDir(caDir)
+	if err != nil {
+		return "", err
+	}
+	for _, e := range dirEntries {
+		if e.IsDir() {
+			continue
 		}
-		tlsTransport := &http.Transport{
-			TLSClientConfig: tlsConfig,
+		data, err := os.ReadFile(filepath.Join(caDir, e.Name()))
+		if err != nil {
+			return "", err
 		}
-		httpClient.Transport = tlsTransport
+		certs = append(certs, string(data))
+	}
+	return strings.Join(certs, "\n"), nil
+}
+
+func BuildHTTPClient(caDir string) (*http.Client, error) {
+	httpClient := &http.Client{Timeout: 10 * time.Second}
+
+	// use the SystemCertPool as a default
+	caCertPool, err := x509.SystemCertPool()
+	if err != nil {
+		return nil, err
+	}
+
+	certs, err := LoadCerts(caDir)
+	if err != nil {
+		return nil, err
+	}
+
+	caCertPool.AppendCertsFromPEM([]byte(certs))
+	tlsConfig := &tls.Config{
+		RootCAs:    caCertPool,
+		MinVersion: tls.VersionTLS12,
+	}
+	tlsTransport := &http.Transport{
+		TLSClientConfig: tlsConfig,
 	}
+	httpClient.Transport = tlsTransport
 
 	return httpClient, nil
 }

scripts/install.tpl.sh

@@ -35,6 +35,42 @@ function kubectl_wait() {
 kubectl apply -f "https://github.com/cert-manager/cert-manager/releases/download/${cert_mgr_version}/cert-manager.yaml"
 kubectl_wait "cert-manager" "deployment/cert-manager-webhook" "60s"
 
+# Create a self-signed ClusterIssuer
+kubectl apply -f - <<EOF
+apiVersion: cert-manager.io/v1
+kind: Issuer
+metadata:
+  name: self-sign-issuer
+  namespace: cert-manager
+spec:
+  selfSigned: {}
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: olmv1-ca
+  namespace: cert-manager
+spec:
+  isCA: true
+  commonName: olmv1-ca
+  secretName: olmv1-ca
+  privateKey:
+    algorithm: ECDSA
+    size: 256
+  issuerRef:
+    name: self-sign-issuer
+    kind: Issuer
+    group: cert-manager.io
+---
+apiVersion: cert-manager.io/v1
+kind: ClusterIssuer
+metadata:
+  name: olmv1-ca
+spec:
+  ca:
+    secretName: olmv1-ca
+EOF
+
 kubectl apply -f "https://github.com/operator-framework/catalogd/releases/download/${catalogd_version}/catalogd.yaml"
 kubectl_wait "olmv1-system" "deployment/catalogd-controller-manager" "60s"
 

test/e2e/cluster_extension_install_test.go

@@ -45,9 +45,6 @@ func testInit(t *testing.T) (*ocv1alpha1.ClusterExtension, *catalogd.ClusterCata
 	clusterExtension := &ocv1alpha1.ClusterExtension{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: clusterExtensionName,
-			Annotations: map[string]string{
-				"bundle.connection.config/insecureSkipTLSVerify": "true",
-			},
 		},
 	}
 	return clusterExtension, extensionCatalog

test/extension-developer-e2e/extension_developer_test.go

@@ -35,9 +35,6 @@ func TestExtensionDeveloper(t *testing.T) {
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "registryv1",
-				Annotations: map[string]string{
-					"bundle.connection.config/insecureSkipTLSVerify": "true",
-				},
 			},
 			Spec: ocv1alpha1.ClusterExtensionSpec{
 				PackageName:      os.Getenv("REG_PKG_NAME"),

test/tools/image-registry.sh

@@ -30,14 +30,6 @@ metadata:
   name: ${namespace}
 ---
 apiVersion: cert-manager.io/v1
-kind: Issuer
-metadata:
-  name: selfsigned-issuer
-  namespace: ${namespace}
-spec:
-  selfSigned: {}
----
-apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
   name: ${namespace}-registry
@@ -52,8 +44,8 @@ spec:
     algorithm: ECDSA
     size: 256
   issuerRef:
-    name: selfsigned-issuer
-    kind: Issuer
+    name: olmv1-ca
+    kind: ClusterIssuer
     group: cert-manager.io
 ---
 apiVersion: apps/v1

testdata/certs/issuers.yaml

@@ -0,0 +1,32 @@
+apiVersion: cert-manager.io/v1
+kind: Issuer
+metadata:
+  name: self-sign-issuer
+  namespace: cert-manager
+spec:
+  selfSigned: {}
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: olmv1-ca
+  namespace: cert-manager
+spec:
+  isCA: true
+  commonName: olmv1-ca
+  secretName: olmv1-ca
+  privateKey:
+    algorithm: ECDSA
+    size: 256
+  issuerRef:
+    name: self-sign-issuer
+    kind: Issuer
+    group: cert-manager.io
+---
+apiVersion: cert-manager.io/v1
+kind: ClusterIssuer
+metadata:
+  name: olmv1-ca
+spec:
+  ca:
+    secretName: olmv1-ca