feat(tls): generate client certificates for agents (#938)

* feat(tls): generate client certificates for agents

* Tests for agent certs

Makefile

@@ -546,7 +546,9 @@ undeploy_bundle: operator-sdk ## Undeploy the controller in the bundle format wi
 
 .PHONY: create_cryostat_cr
 create_cryostat_cr: destroy_cryostat_cr ## Create a namespaced Cryostat instance.
-	$(CLUSTER_CLIENT) create -f config/samples/operator_v1beta2_cryostat.yaml
+	target_ns_json=$$(jq -nc '$$ARGS.positional' --args -- $(TARGET_NAMESPACES)) && \
+	$(CLUSTER_CLIENT) patch -f config/samples/operator_v1beta2_cryostat.yaml --local=true --type=merge \
+	-p "{\"spec\": {\"targetNamespaces\": $$target_ns_json}}" -o yaml | oc apply -f -
 
 .PHONY: destroy_cryostat_cr
 destroy_cryostat_cr: ## Delete a namespaced Cryostat instance.

internal/controllers/certmanager.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 
 	certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	"github.com/cryostatio/cryostat-operator/internal/controllers/common"
@@ -89,25 +90,26 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) (
 	if err != nil {
 		return nil, err
 	}
-	tlsConfig := &resources.TLSConfig{
-		CryostatSecret:     cryostatCert.Spec.SecretName,
-		ReportsSecret:      reportsCert.Spec.SecretName,
-		KeystorePassSecret: cryostatCert.Spec.Keystores.PKCS12.PasswordSecretRef.Name,
-	}
+
+	// List of certificates whose secrets should be owned by this CR
 	certificates := []*certv1.Certificate{caCert, cryostatCert, reportsCert}
 
-	// Update owner references of TLS secrets created by cert-manager to ensure proper cleanup
-	err = r.setCertSecretOwner(ctx, cr.Object, certificates...)
+	// Get the Cryostat CA certificate bytes from certificate secret
+	caBytes, err := r.getCertficateBytes(ctx, caCert)
 	if err != nil {
 		return nil, err
 	}
 
-	secret, err := r.GetCertificateSecret(ctx, caCert)
-	if err != nil {
-		return nil, err
+	tlsConfig := &resources.TLSConfig{
+		CryostatSecret:     cryostatCert.Spec.SecretName,
+		ReportsSecret:      reportsCert.Spec.SecretName,
+		KeystorePassSecret: cryostatCert.Spec.Keystores.PKCS12.PasswordSecretRef.Name,
+		CACert:             caBytes,
 	}
-	// Copy Cryostat CA secret in each target namespace
+
+	agentCertsNotReady := []string{}
 	for _, ns := range cr.TargetNamespaces {
+		// Copy Cryostat CA secret in each target namespace
 		if ns != cr.InstallNamespace {
 			namespaceSecret := &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
@@ -116,14 +118,41 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) (
 				},
 				Type: corev1.SecretTypeOpaque,
 			}
-			err = r.createOrUpdateCertSecret(ctx, namespaceSecret, secret.Data[corev1.TLSCertKey])
+			err = r.createOrUpdateCertSecret(ctx, namespaceSecret, caBytes)
 			if err != nil {
 				return nil, err
 			}
 		}
+
+		// Create a certificate for Cryostat agents in each target namespace
+		agentCert := resources.NewAgentCert(cr, ns, r.gvk)
+		err := r.reconcileAgentCertificate(ctx, agentCert, cr, ns)
+		if err != nil {
+			if err == common.ErrCertNotReady {
+				// Continue with other namespaces if the cert isn't ready
+				agentCertsNotReady = append(agentCertsNotReady, agentCert.Name)
+			} else {
+				return nil, err
+			}
+		}
+		certificates = append(certificates, agentCert)
 	}
-	// Delete any Cryostat CA secrets in target namespaces that are no longer requested
+
+	if len(agentCertsNotReady) > 0 {
+		// One or more agent certificates weren't ready, so log a message and return
+		r.Log.Info("Not all agent certificates were ready", "not ready", strings.Join(agentCertsNotReady, ", "))
+		return nil, common.ErrCertNotReady
+	}
+
+	// Update owner references of TLS secrets created by cert-manager to ensure proper cleanup
+	err = r.setCertSecretOwner(ctx, cr.Object, certificates...)
+	if err != nil {
+		return nil, err
+	}
+
+	// Clean up resources from target namespaces that are no longer requested
 	for _, ns := range toDelete(cr) {
+		// Delete any Cryostat CA secret copies in removed namespaces
 		if ns != cr.InstallNamespace {
 			namespaceSecret := &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
@@ -136,14 +165,31 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) (
 				return nil, err
 			}
 		}
-	}
 
-	// Get the Cryostat CA certificate bytes from certificate secret
-	caBytes, err := r.getCertficateBytes(ctx, caCert)
-	if err != nil {
-		return nil, err
+		// Delete any agent certificates removed target namespaces
+		agentCert := resources.NewAgentCert(cr, ns, r.gvk)
+
+		// Delete namespace copy
+		if ns != cr.InstallNamespace {
+			namespaceAgentSecret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      agentCert.Spec.SecretName,
+					Namespace: ns,
+				},
+			}
+			err = r.deleteSecret(ctx, namespaceAgentSecret)
+			if err != nil {
+				return nil, err
+			}
+		}
+
+		// Delete certificate with original secret
+		err := r.deleteCertWithSecret(ctx, agentCert)
+		if err != nil {
+			return nil, err
+		}
 	}
-	tlsConfig.CACert = caBytes
+
 	return tlsConfig, nil
 }
 
@@ -161,8 +207,22 @@ func (r *Reconciler) finalizeTLS(ctx context.Context, cr *model.CryostatInstance
 			if err != nil {
 				return err
 			}
+
+			// Delete any agent certificate secrets in target namespaces
+			agentCert := resources.NewAgentCert(cr, ns, r.gvk)
+			namespaceAgentSecret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      agentCert.Spec.SecretName,
+					Namespace: ns,
+				},
+			}
+			err = r.deleteSecret(ctx, namespaceAgentSecret)
+			if err != nil {
+				return err
+			}
 		}
 	}
+
 	return nil
 }
 
@@ -265,20 +325,7 @@ func (r *Reconciler) deleteCertChain(ctx context.Context, namespace string, caSe
 	for i, cert := range certs.Items {
 		// Is the certificate owned by this CR, and not the CA itself?
 		if metav1.IsControlledBy(&certs.Items[i], owner) && cert.Spec.SecretName != caSecretName {
-			// Clean up secret referenced by the cert
-			secret := &corev1.Secret{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      cert.Spec.SecretName,
-					Namespace: cert.Namespace,
-				},
-			}
-
-			err := r.deleteSecret(ctx, secret)
-			if err != nil {
-				return err
-			}
-			// Delete the certificate
-			err = r.deleteCertificate(ctx, &certs.Items[i])
+			err := r.deleteCertWithSecret(ctx, &certs.Items[i])
 			if err != nil {
 				return err
 			}
@@ -288,11 +335,65 @@ func (r *Reconciler) deleteCertChain(ctx context.Context, namespace string, caSe
 	return nil
 }
 
+func (r *Reconciler) deleteCertWithSecret(ctx context.Context, cert *certv1.Certificate) error {
+	// Clean up secret referenced by the cert
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      cert.Spec.SecretName,
+			Namespace: cert.Namespace,
+		},
+	}
+	err := r.deleteSecret(ctx, secret)
+	if err != nil {
+		return err
+	}
+
+	// Delete the certificate
+	err = r.deleteCertificate(ctx, cert)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func (r *Reconciler) reconcileAgentCertificate(ctx context.Context, cert *certv1.Certificate, cr *model.CryostatInstance, namespace string) error {
+	// Create the Agent certificate in the install namespace
+	err := r.createOrUpdateCertificate(ctx, cert, cr.Object)
+	if err != nil {
+		return err
+	}
+
+	// Fetch the certificate secret and create a copy in the target namespace (if not the install namespace)
+	if namespace != cr.InstallNamespace {
+		secret, err := r.GetCertificateSecret(ctx, cert)
+		if err != nil {
+			return err
+		}
+
+		targetSecret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      secret.Name,
+				Namespace: namespace,
+			},
+		}
+		err = r.createOrUpdateSecret(ctx, targetSecret, nil, func() error {
+			targetSecret.Data = secret.Data
+			return nil
+		})
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (r *Reconciler) createOrUpdateCertificate(ctx context.Context, cert *certv1.Certificate, owner metav1.Object) error {
 	certSpec := cert.Spec.DeepCopy()
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, cert, func() error {
-		if err := controllerutil.SetControllerReference(owner, cert, r.Scheme); err != nil {
-			return err
+		if owner != nil {
+			if err := controllerutil.SetControllerReference(owner, cert, r.Scheme); err != nil {
+				return err
+			}
 		}
 		// Update Certificate spec
 		cert.Spec = *certSpec

internal/controllers/common/common_utils.go

@@ -26,7 +26,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/types"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
@@ -69,17 +68,28 @@ func ClusterUniqueName(gvk *schema.GroupVersionKind, name string, namespace stri
 	return ClusterUniqueNameWithPrefix(gvk, "", name, namespace)
 }
 
-// ClusterUniqueName returns a name for cluster-scoped objects that is
+// ClusterUniqueNameWithPrefix returns a name for cluster-scoped objects that is
 // uniquely identified by a namespace and name. Appends the prefix to the
 // provided Kind.
 func ClusterUniqueNameWithPrefix(gvk *schema.GroupVersionKind, prefix string, name string, namespace string) string {
+	return ClusterUniqueNameWithPrefixTargetNS(gvk, prefix, name, namespace, "")
+}
+
+// ClusterUniqueNameWithPrefixTargetNS returns a name for cluster-scoped objects that is
+// uniquely identified by a namespace and name, and a target namespace.
+// Appends the prefix to the provided Kind.
+func ClusterUniqueNameWithPrefixTargetNS(gvk *schema.GroupVersionKind, prefix string, name string, namespace string,
+	targetNS string) string {
 	prefixWithKind := strings.ToLower(gvk.Kind)
 	if len(prefix) > 0 {
 		prefixWithKind += "-" + prefix
 	}
+	toHash := namespace + "/" + name
+	if len(targetNS) > 0 {
+		toHash += "/" + targetNS
+	}
 	// Use the SHA256 checksum of the namespaced name as a suffix
-	nn := types.NamespacedName{Namespace: namespace, Name: name}
-	suffix := fmt.Sprintf("%x", sha256.Sum256([]byte(nn.String())))
+	suffix := fmt.Sprintf("%x", sha256.Sum256([]byte(toHash)))
 	return prefixWithKind + "-" + suffix
 }
 

internal/controllers/common/resource_definitions/certificates.go

@@ -131,3 +131,27 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate {
 		},
 	}
 }
+
+func NewAgentCert(cr *model.CryostatInstance, namespace string, gvk *schema.GroupVersionKind) *certv1.Certificate {
+	name := common.ClusterUniqueNameWithPrefixTargetNS(gvk, "agent", cr.Name, cr.InstallNamespace, namespace)
+	return &certv1.Certificate{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: cr.InstallNamespace,
+		},
+		Spec: certv1.CertificateSpec{
+			CommonName: fmt.Sprintf("*.%s.pod", namespace),
+			DNSNames: []string{
+				fmt.Sprintf("*.%s.pod", namespace),
+			},
+			SecretName: name,
+			IssuerRef: certMeta.ObjectReference{
+				Name: cr.Name + "-ca",
+			},
+			Usages: append(certv1.DefaultKeyUsages(),
+				certv1.UsageServerAuth,
+				certv1.UsageClientAuth,
+			),
+		},
+	}
+}

internal/controllers/reconciler.go

@@ -165,10 +165,12 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn
 				return reconcile.Result{}, err
 			}
 
-			// Finalizer for CA Cert secrets
-			err = r.finalizeTLS(ctx, cr)
-			if err != nil {
-				return reconcile.Result{}, err
+			// Finalizer for certificates and associated secrets
+			if r.IsCertManagerEnabled(cr) {
+				err = r.finalizeTLS(ctx, cr)
+				if err != nil {
+					return reconcile.Result{}, err
+				}
 			}
 
 			err = common.RemoveFinalizer(ctx, r.Client, cr.Object, cryostatFinalizer)