Certificate support for image registry

Updates the ClusterExtension API to support a certificate to retreive a
bundle from an image registry.

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

api/v1alpha1/clusterextension_types.go

@@ -43,6 +43,25 @@ const (
 	UpgradeConstraintPolicyIgnore UpgradeConstraintPolicy = "Ignore"
 )
 
+// Similar to NamespacedName, but with json
+type ClusterExtensionSecretRef struct {
+	// Name of the secret
+	Name string `json:"name"`
+	// Namespace of the secret
+	Namespace string `json:"namespace,omitempty"`
+}
+
+type ClusterExtensionTLS struct {
+	//+optional
+	// InsecureSkipTLSVerify allows the HTTPS client to ignore the server certificate
+	InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"`
+	// +optional
+	// CertificateSecretRef references a Secret that contains the tls.crt certificate that
+	// can verify the server certificate.
+	// This fits the definition of NamespacedName (but that doesn't have json tags)
+	CertificateSecretRef *ClusterExtensionSecretRef `json:"certificateSecretRef,omitempty"`
+}
+
 // ClusterExtensionSpec defines the desired state of ClusterExtension
 type ClusterExtensionSpec struct {
 	//+kubebuilder:validation:MaxLength:=48
@@ -78,6 +97,10 @@ type ClusterExtensionSpec struct {
 	// the bundle may contain resources that are cluster-scoped or that are
 	// installed in a different namespace. This namespace is expected to exist.
 	InstallNamespace string `json:"installNamespace"`
+
+	//+optional
+	// RegistryTLS defines the connection parameters to retrieve an image from a registry
+	RegistryTLS *ClusterExtensionTLS `json:"registryTLS,omitempty"`
 }
 
 const (

cmd/manager/main.go

@@ -150,7 +150,6 @@ func main() {
 		setupLog.Error(err, "unable to start manager")
 		os.Exit(1)
 	}
-
 	httpClient, err := httputil.BuildHTTPClient(caCert)
 	if err != nil {
 		setupLog.Error(err, "unable to create catalogd http client")
@@ -210,6 +209,7 @@ func main() {
 
 	if err = (&controllers.ClusterExtensionReconciler{
 		Client:                cl,
+		Reader:                mgr.GetAPIReader(),
 		BundleProvider:        catalogClient,
 		ActionClientGetter:    acg,
 		Unpacker:              unpacker,

config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml

@@ -56,6 +56,30 @@ spec:
                 maxLength: 48
                 pattern: ^[a-z0-9]+(-[a-z0-9]+)*$
                 type: string
+              registryTLS:
+                description: RegistryTLS defines the connection parameters to retrieve
+                  an image from a registry
+                properties:
+                  certificateSecretRef:
+                    description: |-
+                      CertificateSecretRef references a Secret that contains the tls.crt certificate that
+                      can verify the server certificate.
+                      This fits the definition of NamespacedName (but that doesn't have json tags)
+                    properties:
+                      name:
+                        description: Name of the secret
+                        type: string
+                      namespace:
+                        description: Namespace of the secret
+                        type: string
+                    required:
+                    - name
+                    type: object
+                  insecureSkipTLSVerify:
+                    description: InsecureSkipTLSVerify allows the HTTPS client to
+                      ignore the server certificate
+                    type: boolean
+                type: object
               upgradeConstraintPolicy:
                 default: Enforce
                 description: Defines the policy for how to handle upgrade constraints

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/operator-framework/catalogd v0.14.0
 	github.com/operator-framework/helm-operator-plugins v0.2.2-0.20240520180534-f463c36fedf9
 	github.com/operator-framework/operator-registry v1.43.1
-	github.com/operator-framework/rukpak v0.23.1
+	github.com/operator-framework/rukpak v0.23.2-0.20240618133950-e1d8b0e32344
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.9.0
 	go.uber.org/zap v1.27.0
@@ -123,7 +123,7 @@ require (
 	github.com/google/btree v1.1.2 // indirect
 	github.com/google/cel-go v0.17.8 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
-	github.com/google/go-containerregistry v0.19.1 // indirect
+	github.com/google/go-containerregistry v0.19.2 // indirect
 	github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20240505154900-ff385a972813 // indirect
 	github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20240505154900-ff385a972813 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
@@ -195,7 +195,7 @@ require (
 	github.com/sirupsen/logrus v1.9.3 // indirect
 	github.com/skeema/knownhosts v1.2.2 // indirect
 	github.com/spf13/cast v1.5.0 // indirect
-	github.com/spf13/cobra v1.8.0 // indirect
+	github.com/spf13/cobra v1.8.1 // indirect
 	github.com/stoewer/go-strcase v1.3.0 // indirect
 	github.com/stretchr/objx v0.5.2 // indirect
 	github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect

go.sum

@@ -152,6 +152,7 @@ github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03V
 github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs=
 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
 github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
+github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
 github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
 github.com/cyphar/filepath-securejoin v0.2.5 h1:6iR5tXJ/e6tJZzzdMc1km3Sa7RRIVBKAK32O2s7AYfo=
@@ -307,6 +308,8 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-containerregistry v0.19.1 h1:yMQ62Al6/V0Z7CqIrrS1iYoA5/oQCm88DeNujc7C1KY=
 github.com/google/go-containerregistry v0.19.1/go.mod h1:YCMFNQeeXeLF+dnhhWkqDItx/JSkH01j1Kis4PsjzFI=
+github.com/google/go-containerregistry v0.19.2 h1:TannFKE1QSajsP6hPWb5oJNgKe1IKjHukIKDUmvsV6w=
+github.com/google/go-containerregistry v0.19.2/go.mod h1:YCMFNQeeXeLF+dnhhWkqDItx/JSkH01j1Kis4PsjzFI=
 github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20240505154900-ff385a972813 h1:PNR/Dkh697bQVCKLUekdd6LSNU9XkmXjrUbvH+wpfTg=
 github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20240505154900-ff385a972813/go.mod h1:5UXYZJNyCPf2YD+6J76geTiLAXA8fJbDy7mGQa5m5Vc=
 github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20240505154900-ff385a972813 h1:irEChX0pAmED+6auieJELA0JKeCakr6iDCTLjJUiT8k=
@@ -485,6 +488,8 @@ github.com/operator-framework/operator-registry v1.43.1 h1:ACahVHGIL/hINBXd3RKWq
 github.com/operator-framework/operator-registry v1.43.1/go.mod h1:qhssAIYWXDIW+nTg0C5i4iD9zpMtiXtfXqGUuUmGz5c=
 github.com/operator-framework/rukpak v0.23.1 h1:lam6+wysaVjZVpMdtl7DUbc+8ibCdlCfp+nB62K0aSU=
 github.com/operator-framework/rukpak v0.23.1/go.mod h1:DrQRNduAm0DWRSXpFhz8FA5g2GrJJ88sWpG5GiWmvPU=
+github.com/operator-framework/rukpak v0.23.2-0.20240618133950-e1d8b0e32344 h1:L2jGtjx9g3EgXUTDiVNgoqyBrP4m4RCyQspDjoDIFjg=
+github.com/operator-framework/rukpak v0.23.2-0.20240618133950-e1d8b0e32344/go.mod h1:peTAGzf4gmU+in2RJen84ZQ8lkdB3m6qy+nfNiVv0RY=
 github.com/otiai10/copy v1.14.0 h1:dCI/t1iTdYGtkvCuBG2BgR6KZa83PTclw4U5n2wAllU=
 github.com/otiai10/copy v1.14.0/go.mod h1:ECfuL02W+/FkTWZWgQqXPWZgW9oeKCSQ5qVfSc4qc4w=
 github.com/otiai10/mint v1.5.1 h1:XaPLeE+9vGbuyEHem1JNk3bYc7KKqyI/na0/mLd/Kks=
@@ -554,6 +559,8 @@ github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w=
 github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU=
 github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0=
 github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho=
+github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM=
+github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y=
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs=

internal/controllers/clusterextension_controller.go

@@ -36,6 +36,7 @@ import (
 	"helm.sh/helm/v3/pkg/postrender"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -79,6 +80,7 @@ import (
 // ClusterExtensionReconciler reconciles a ClusterExtension object
 type ClusterExtensionReconciler struct {
 	client.Client
+	client.Reader
 	BundleProvider        BundleProvider
 	Unpacker              rukpaksource.Unpacker
 	ActionClientGetter    helmclient.ActionClientGetter
@@ -96,10 +98,6 @@ 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())
@@ -533,7 +531,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 := r.getCertificateData(ctx, ce)
+	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,
@@ -550,21 +552,54 @@ func (r *ClusterExtensionReconciler) generateBundleDeploymentForUnpack(bundlePat
 				Image: &rukpakv1alpha2.ImageSource{
 					Ref:                   bundlePath,
 					InsecureSkipTLSVerify: isInsecureSkipTLSVerifySet(ce),
+					CertificateData:       certData,
 				},
 			},
 		},
 	}
 }
 
 func isInsecureSkipTLSVerifySet(ce *ocv1alpha1.ClusterExtension) bool {
-	if ce == nil {
+	if ce == nil || ce.Spec.RegistryTLS == nil {
 		return false
 	}
-	value, ok := ce.Annotations[bundleConnectionAnnotation]
-	if !ok {
-		return false
+	return ce.Spec.RegistryTLS.InsecureSkipTLSVerify
+}
+
+func (r *ClusterExtensionReconciler) getCertificateData(ctx context.Context, ce *ocv1alpha1.ClusterExtension) (string, error) {
+	if ce == nil || ce.Spec.RegistryTLS == nil || ce.Spec.RegistryTLS.CertificateSecretRef == nil {
+		return "", nil
+	}
+	secretName := getNamespacedName(*ce.Spec.RegistryTLS.CertificateSecretRef)
+	var secret = &corev1.Secret{}
+	if err := r.Reader.Get(ctx, secretName, secret); err != nil {
+		return "", fmt.Errorf("unable to get secret %v: %w", secretName, err)
+	}
+	if secret.Type != corev1.SecretTypeTLS {
+		return "", fmt.Errorf("invalid type in secret %v: %v", secretName, secret.Type)
+	}
+	var certs []string
+	// Get any 'ca.crt'
+	data, ok := secret.Data[corev1.ServiceAccountRootCAKey]
+	if ok && len(data) > 0 {
+		certs = append(certs, string(data[:]))
+	}
+	// Get any 'tls.crt'
+	data, ok = secret.Data[corev1.TLSCertKey]
+	if ok && len(data) > 0 {
+		certs = append(certs, string(data[:]))
+	}
+	if len(certs) == 0 {
+		return "", fmt.Errorf("no data found in secret: %v", secretName)
+	}
+	return strings.Join(certs, "\n"), nil
+}
+
+func getNamespacedName(name ocv1alpha1.ClusterExtensionSecretRef) types.NamespacedName {
+	if name.Namespace == "" {
+		return types.NamespacedName{Namespace: "default", Name: name.Name}
 	}
-	return value == "true"
+	return types.NamespacedName{Namespace: name.Namespace, Name: name.Name}
 }
 
 // SetupWithManager sets up the controller with the Manager.

test/e2e/cluster_extension_install_test.go

@@ -45,13 +45,18 @@ 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
 }
+func addCertToSpec(ce *ocv1alpha1.ClusterExtension) {
+	ce.Spec.RegistryTLS = &ocv1alpha1.ClusterExtensionTLS{
+		CertificateSecretRef: &ocv1alpha1.ClusterExtensionSecretRef{
+			Name:      "operator-controller-e2e-registry",
+			Namespace: "operator-controller-e2e",
+		},
+	}
+}
 
 func testCleanup(t *testing.T, cat *catalogd.ClusterCatalog, clusterExtension *ocv1alpha1.ClusterExtension) {
 	require.NoError(t, c.Delete(context.Background(), cat))
@@ -78,6 +83,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) {
 		PackageName:      "prometheus",
 		InstallNamespace: "default",
 	}
+	addCertToSpec(clusterExtension)
 	t.Log("It resolves the specified package with correct bundle path")
 	t.Log("By creating the ClusterExtension resource")
 	require.NoError(t, c.Create(context.Background(), clusterExtension))
@@ -135,6 +141,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
 		PackageName:      pkgName,
 		InstallNamespace: "default",
 	}
+	addCertToSpec(clusterExtension)
 
 	t.Log("By deleting the catalog first")
 	require.NoError(t, c.Delete(context.Background(), extensionCatalog))
@@ -202,6 +209,7 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
 		Version:          "1.0.0",
 		InstallNamespace: "default",
 	}
+	addCertToSpec(clusterExtension)
 	require.NoError(t, c.Create(context.Background(), clusterExtension))
 	t.Log("By eventually reporting a successful installation")
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
@@ -248,6 +256,7 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
 		Version:          "1.0.0",
 		InstallNamespace: "default",
 	}
+	addCertToSpec(clusterExtension)
 	require.NoError(t, c.Create(context.Background(), clusterExtension))
 	t.Log("By eventually reporting a successful resolution")
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
@@ -293,6 +302,7 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
 		Version:          "1.0.0",
 		InstallNamespace: "default",
 	}
+	addCertToSpec(clusterExtension)
 	require.NoError(t, c.Create(context.Background(), clusterExtension))
 	t.Log("By eventually reporting a successful resolution")
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {

test/e2e/cluster_extension_registryV1_limitations_test.go

@@ -24,6 +24,7 @@ func TestClusterExtensionPackagesWithWebhooksAreNotAllowed(t *testing.T) {
 		Version:          "1.0.0",
 		InstallNamespace: "default",
 	}
+	addCertToSpec(clusterExtension)
 	require.NoError(t, c.Create(ctx, clusterExtension))
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
 		assert.NoError(ct, c.Get(ctx, types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))