K8SPG-1017 replication certificate was not covered by cert manager path (#1599)

Author: gkech

e2e-tests/tests/cert-manager-tls/10-verify-cert-manager-resources.yaml

@@ -49,6 +49,12 @@ commands:
           exit 1
       fi
 
+      replication_cert_ready=$(kubectl -n "$NAMESPACE" get certificate cert-manager-tls-replication-cert -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}')
+      if [[ "$replication_cert_ready" != "True" ]]; then
+          echo "Replication Certificate is not ready: $replication_cert_ready"
+          exit 1
+      fi
+
       instance_sts=$(kubectl -n "$NAMESPACE" get sts -l postgres-operator.crunchydata.com/cluster=cert-manager-tls,postgres-operator.crunchydata.com/instance-set=instance1 -o jsonpath='{.items[*].metadata.name}')
       for sts_name in $instance_sts; do
           cert_name="${sts_name}-cert"

e2e-tests/tests/cert-manager-tls/11-verify-tls-secrets.yaml

@@ -62,6 +62,20 @@ commands:
           exit 1
       fi
 
+      retry 12 5 verify_secret_data cert-manager-tls-replication-cert tls.crt tls.key ca.crt
+
+      repl_secret_type=$(kubectl -n "$NAMESPACE" get secret cert-manager-tls-replication-cert -o jsonpath='{.type}')
+      if [[ "$repl_secret_type" != "kubernetes.io/tls" ]]; then
+          echo "Replication TLS secret type is incorrect: $repl_secret_type (expected kubernetes.io/tls)"
+          exit 1
+      fi
+
+      repl_issuer_name=$(kubectl -n "$NAMESPACE" get secret cert-manager-tls-replication-cert -o jsonpath='{.metadata.annotations.cert-manager\.io/issuer-name}')
+      if [[ "$repl_issuer_name" != "cert-manager-tls-tls-issuer" ]]; then
+          echo "Replication TLS secret issuer annotation is incorrect: $repl_issuer_name"
+          exit 1
+      fi
+
       retry 12 5 verify_secret_data cert-manager-tls-pgbackrest-client-tls tls.crt tls.key
 
       pgbr_client_issuer=$(kubectl -n "$NAMESPACE" get secret cert-manager-tls-pgbackrest-client-tls -o jsonpath='{.metadata.annotations.cert-manager\.io/issuer-name}')

internal/controller/postgrescluster/patroni.go

@@ -379,6 +379,23 @@ func (r *Reconciler) reconcileReplicationSecret(
 		return custom, err
 	}
 
+	certManagerManaged, err := r.isRootCACertManagerManaged(ctx, cluster)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to check if cert-manager manages root CA")
+	}
+
+	if certManagerManaged {
+		return r.reconcileCertManagerReplicationSecret(ctx, cluster)
+	}
+
+	return r.reconcileInternalReplicationSecret(ctx, cluster, root)
+}
+
+// reconcileInternalReplicationSecret creates a replication certificate using internal PKI.
+func (r *Reconciler) reconcileInternalReplicationSecret(
+	ctx context.Context, cluster *v1beta1.PostgresCluster,
+	root *pki.RootCertificateAuthority,
+) (*corev1.Secret, error) {
 	existing := &corev1.Secret{ObjectMeta: naming.ReplicationClientCertSecret(cluster)}
 	err := errors.WithStack(client.IgnoreNotFound(
 		r.Client.Get(ctx, client.ObjectKeyFromObject(existing), existing)))
@@ -436,6 +453,20 @@ func (r *Reconciler) reconcileReplicationSecret(
 	return intent, err
 }
 
+// reconcileCertManagerReplicationSecret creates the replication certificate using cert-manager.
+func (r *Reconciler) reconcileCertManagerReplicationSecret(
+	ctx context.Context, cluster *v1beta1.PostgresCluster,
+) (*corev1.Secret, error) {
+	c := r.CertManagerCtrlFunc(r.Client, r.Scheme, false)
+
+	if err := c.ApplyReplicationCertificate(ctx, cluster); err != nil {
+		return nil, errors.Wrap(err, "failed to apply replication certificate")
+	}
+
+	secret := &corev1.Secret{ObjectMeta: naming.ReplicationClientCertSecret(cluster)}
+	return secret, nil
+}
+
 // replicationCertSecretProjection returns a secret projection of the postgrescluster's
 // client certificate and key to include in the instance configuration volume.
 func replicationCertSecretProjection(certificate *corev1.Secret) *corev1.SecretProjection {

internal/controller/postgrescluster/pki_test.go

@@ -412,6 +412,9 @@ func (m *mockCertManagerController) ApplyInstanceCertificate(context.Context, *v
 func (m *mockCertManagerController) ApplyPGBouncerCertificate(context.Context, *v1beta1.PostgresCluster, []string) error {
 	panic("unexpected call")
 }
+func (m *mockCertManagerController) ApplyReplicationCertificate(context.Context, *v1beta1.PostgresCluster) error {
+	panic("unexpected call")
+}
 func (m *mockCertManagerController) ApplyPGBackRestClientCertificate(context.Context, *v1beta1.PostgresCluster) error {
 	panic("unexpected call")
 }

percona/certmanager/certmanager.go

@@ -32,6 +32,7 @@ type Controller interface {
 	ApplyClusterCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster, dnsNames []string) error
 	ApplyInstanceCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster, instanceName string, dnsNames []string) error
 	ApplyPGBouncerCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster, dnsNames []string) error
+	ApplyReplicationCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster) error
 	ApplyPGBackRestClientCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster) error
 	ApplyPGBackRestRepoCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster, dnsNames []string) error
 }
@@ -590,6 +591,104 @@ func (c *controller) ApplyPGBouncerCertificate(ctx context.Context, cluster *v1b
 	return nil
 }
 
+// ApplyReplicationCertificate creates a cert-manager Certificate resource for the replication client.
+func (c *controller) ApplyReplicationCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster) error {
+	secretMeta := naming.ReplicationClientCertSecret(cluster)
+	certName := cluster.Name + "-replication-cert"
+	commonName := "_crunchyrepl"
+
+	certDuration := DefaultCertDuration
+	if cluster.Spec.TLS != nil && cluster.Spec.TLS.CertValidityDuration != nil {
+		certDuration = cluster.Spec.TLS.CertValidityDuration.Duration
+	}
+
+	existing := &v1.Certificate{}
+	err := c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing)
+	if err == nil {
+		needsUpdate := false
+
+		hasOwnerRef, err := controllerutil.HasOwnerReference(existing.OwnerReferences, cluster, c.scheme)
+		if err != nil {
+			return errors.Wrap(err, "check owner reference")
+		}
+
+		if !hasOwnerRef {
+			gvk := v1beta1.SchemeBuilder.GroupVersion.WithKind("PostgresCluster")
+			existing.OwnerReferences = []metav1.OwnerReference{{
+				APIVersion:         gvk.GroupVersion().String(),
+				Kind:               gvk.Kind,
+				Name:               cluster.GetName(),
+				UID:                cluster.GetUID(),
+				BlockOwnerDeletion: ptr.To(true),
+				Controller:         ptr.To(true),
+			}}
+			needsUpdate = true
+		}
+
+		if existing.Spec.Duration != nil && existing.Spec.Duration.Duration != certDuration {
+			existing.Spec.Duration = &metav1.Duration{Duration: certDuration}
+			needsUpdate = true
+		}
+
+		if !needsUpdate {
+			return nil
+		}
+
+		return errors.Wrap(c.cl.Update(ctx, existing), "failed to update replication certificate")
+	}
+	if !k8serrors.IsNotFound(err) {
+		return errors.Wrap(err, "failed to get replication certificate")
+	}
+
+	cert := &v1.Certificate{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      certName,
+			Namespace: cluster.Namespace,
+			Labels: naming.WithPerconaLabels(map[string]string{
+				naming.LabelCluster:            cluster.Name,
+				naming.LabelClusterCertificate: "replication-client-tls",
+			}, cluster.Name, "", cluster.Labels[naming.LabelVersion]),
+		},
+		Spec: v1.CertificateSpec{
+			SecretName: secretMeta.Name,
+			CommonName: commonName,
+			DNSNames:   []string{commonName},
+			IssuerRef: cmmeta.IssuerReference{
+				Name: naming.TLSIssuer(cluster).Name,
+				Kind: v1.IssuerKind,
+			},
+			Duration:    &metav1.Duration{Duration: certDuration},
+			RenewBefore: &metav1.Duration{Duration: DefaultRenewBefore},
+			PrivateKey: &v1.CertificatePrivateKey{
+				Algorithm:      v1.ECDSAKeyAlgorithm,
+				Size:           256,
+				RotationPolicy: v1.RotationPolicyNever,
+			},
+			Usages: []v1.KeyUsage{
+				v1.UsageClientAuth,
+				v1.UsageDigitalSignature,
+				v1.UsageKeyEncipherment,
+			},
+			SecretTemplate: &v1.CertificateSecretTemplate{
+				Labels: naming.WithPerconaLabels(map[string]string{
+					naming.LabelCluster:            cluster.Name,
+					naming.LabelClusterCertificate: "replication-client-tls",
+				}, cluster.Name, "", cluster.Labels[naming.LabelVersion]),
+			},
+		},
+	}
+
+	if err := controllerutil.SetControllerReference(cluster, cert, c.scheme); err != nil {
+		return errors.Wrap(err, "failed to set controller reference")
+	}
+
+	if err := c.cl.Create(ctx, cert); err != nil {
+		return errors.Wrap(err, "failed to create replication certificate")
+	}
+
+	return nil
+}
+
 // ApplyPGBackRestClientCertificate creates a cert-manager Certificate resource
 // for the pgBackRest client used by all PostgreSQL instances to connect to the
 // repository host.

percona/certmanager/certmanager_test.go

@@ -685,6 +685,84 @@ func TestApplyPGBackRestClientCertificate(t *testing.T) {
 	})
 }
 
+func TestApplyReplicationCertificate(t *testing.T) {
+	t.Run("create replication certificate successfully", func(t *testing.T) {
+		cluster := testCluster()
+		cluster.Name = "replication-cert-test"
+		client := setupFakeClient(t, cluster)
+		ctrl := NewController(client, client.Scheme(), false)
+
+		err := ctrl.ApplyReplicationCertificate(t.Context(), cluster)
+		require.NoError(t, err)
+
+		cert := &v1.Certificate{}
+		certName := cluster.Name + "-replication-cert"
+		err = client.Get(t.Context(), sigs.ObjectKey{
+			Namespace: cluster.Namespace,
+			Name:      certName,
+		}, cert)
+		require.NoError(t, err)
+
+		assert.Equal(t, certName, cert.Name)
+		assert.Equal(t, cluster.Namespace, cert.Namespace)
+		assert.Equal(t, naming.ReplicationClientCertSecret(cluster).Name, cert.Spec.SecretName)
+		assert.Equal(t, "_crunchyrepl", cert.Spec.CommonName)
+		assert.Equal(t, []string{"_crunchyrepl"}, cert.Spec.DNSNames)
+		assert.Equal(t, naming.TLSIssuer(cluster).Name, cert.Spec.IssuerRef.Name)
+		assert.Equal(t, v1.IssuerKind, cert.Spec.IssuerRef.Kind)
+		assert.NotNil(t, cert.Spec.Duration)
+		assert.Equal(t, DefaultCertDuration, cert.Spec.Duration.Duration)
+		assert.NotNil(t, cert.Spec.RenewBefore)
+		assert.Equal(t, DefaultRenewBefore, cert.Spec.RenewBefore.Duration)
+		assert.NotNil(t, cert.Spec.PrivateKey)
+		assert.Equal(t, v1.ECDSAKeyAlgorithm, cert.Spec.PrivateKey.Algorithm)
+		assert.Equal(t, 256, cert.Spec.PrivateKey.Size)
+		assert.Equal(t, v1.RotationPolicyNever, cert.Spec.PrivateKey.RotationPolicy)
+
+		assert.Contains(t, cert.Spec.Usages, v1.UsageClientAuth)
+		assert.Contains(t, cert.Spec.Usages, v1.UsageDigitalSignature)
+		assert.Contains(t, cert.Spec.Usages, v1.UsageKeyEncipherment)
+		assert.NotContains(t, cert.Spec.Usages, v1.UsageServerAuth)
+
+		assert.NotNil(t, cert.Spec.SecretTemplate)
+		assert.Equal(t, cluster.Name, cert.Spec.SecretTemplate.Labels[naming.LabelCluster])
+		assert.Equal(t, "replication-client-tls", cert.Spec.SecretTemplate.Labels[naming.LabelClusterCertificate])
+
+		assert.Equal(t, cluster.Name, cert.Labels[naming.LabelCluster])
+		assert.Equal(t, "replication-client-tls", cert.Labels[naming.LabelClusterCertificate])
+		assert.NotEmpty(t, cert.Labels[naming.LabelPerconaManagedBy])
+
+		require.Len(t, cert.OwnerReferences, 1)
+		assert.Equal(t, cluster.Name, cert.OwnerReferences[0].Name)
+
+		// return nil when certificate already exists
+		err = ctrl.ApplyReplicationCertificate(t.Context(), cluster)
+		require.NoError(t, err)
+	})
+
+	t.Run("uses CertValidityDuration when set", func(t *testing.T) {
+		customDuration := 4320 * time.Hour // 180 days
+		cluster := testCluster()
+		cluster.Name = "replication-cert-dur"
+		cluster.Spec.TLS = &v1beta1.TLSSpec{
+			CertValidityDuration: &metav1.Duration{Duration: customDuration},
+		}
+		client := setupFakeClient(t, cluster)
+		ctrl := NewController(client, client.Scheme(), false)
+
+		err := ctrl.ApplyReplicationCertificate(t.Context(), cluster)
+		require.NoError(t, err)
+
+		cert := &v1.Certificate{}
+		err = client.Get(t.Context(), sigs.ObjectKey{
+			Namespace: cluster.Namespace,
+			Name:      cluster.Name + "-replication-cert",
+		}, cert)
+		require.NoError(t, err)
+		assert.Equal(t, customDuration, cert.Spec.Duration.Duration)
+	})
+}
+
 func TestUpdateCertificateDuration(t *testing.T) {
 	initialDuration := 2160 * time.Hour    // 90 days
 	updatedDuration := 4320 * time.Hour    // 180 days
@@ -798,6 +876,31 @@ func TestUpdateCertificateDuration(t *testing.T) {
 		assert.Equal(t, updatedDuration, cert.Spec.Duration.Duration)
 	})
 
+	t.Run("replication certificate duration updated when spec changes", func(t *testing.T) {
+		cluster := testCluster()
+		cluster.Name = "update-repl-dur"
+		cluster.Spec.TLS = &v1beta1.TLSSpec{
+			CertValidityDuration: &metav1.Duration{Duration: initialDuration},
+		}
+		client := setupFakeClient(t, cluster)
+		ctrl := NewController(client, client.Scheme(), false)
+
+		err := ctrl.ApplyReplicationCertificate(t.Context(), cluster)
+		require.NoError(t, err)
+
+		cluster.Spec.TLS.CertValidityDuration = &metav1.Duration{Duration: updatedDuration}
+		err = ctrl.ApplyReplicationCertificate(t.Context(), cluster)
+		require.NoError(t, err)
+
+		cert := &v1.Certificate{}
+		err = client.Get(t.Context(), sigs.ObjectKey{
+			Namespace: cluster.Namespace,
+			Name:      cluster.Name + "-replication-cert",
+		}, cert)
+		require.NoError(t, err)
+		assert.Equal(t, updatedDuration, cert.Spec.Duration.Duration)
+	})
+
 	t.Run("pgbackrest client certificate duration updated when pgBackRestCertValidityDuration changes", func(t *testing.T) {
 		cluster := testCluster()
 		cluster.Name = "update-pgbr-client-dur"