K8SPXC-1030: Don't delete cert-manager certs by default (#1171)

* K8SPXC-1030: Don't delete cert-manager certs by default

We shouldn't set owner references to cert-manager objects if we don't
want to delete secrets too. This way, after the PXC cluster is deleted
issuers, certificates and their secrets will remain intact in the
cluster.

If users want to cleanup objects created for SSL, we introduce a new
finalizer: `delete-ssl`. If this finalizer is set, the operator will
delete secrets, certificates and issuers. Unfortunately, cert-manager
doesn't set owner reference to the secret it created and this behaviour
can only configured by command line flag in the controller. Since we
can't control how users deploy cert-manager to their clusters, we
shouldn't rely on this feature and cleanup certificates and secrets
altogether. Hopefully,
cert-manager/cert-manager#5158 will merged and
we can configure this behaviour on certificate level.

* fix tests

Co-authored-by: Viacheslav Sarzhan <[email protected]>

Author: egegunes

deploy/cr.yaml

@@ -4,6 +4,7 @@ metadata:
   name: cluster1
   finalizers:
     - delete-pxc-pods-in-order
+#    - delete-ssl
 #    - delete-proxysql-pvc
 #    - delete-pxc-pvc
 #  annotations:

e2e-tests/init-deploy/compare/certificate_no-proxysql-ssl-internal.yml

@@ -3,10 +3,6 @@ kind: Certificate
 metadata:
   generation: 1
   name: no-proxysql-ssl-internal
-  ownerReferences:
-    - controller: true
-      kind: PerconaXtraDBCluster
-      name: no-proxysql
 spec:
   commonName: no-proxysql-pxc
   dnsNames:

e2e-tests/init-deploy/compare/certificate_no-proxysql-ssl.yml

@@ -3,10 +3,6 @@ kind: Certificate
 metadata:
   generation: 1
   name: no-proxysql-ssl
-  ownerReferences:
-    - controller: true
-      kind: PerconaXtraDBCluster
-      name: no-proxysql
 spec:
   commonName: no-proxysql-proxysql
   dnsNames:

e2e-tests/init-deploy/compare/issuer_no-proxysql-pxc-issuer.yml

@@ -3,10 +3,6 @@ kind: Issuer
 metadata:
   generation: 1
   name: no-proxysql-pxc-issuer
-  ownerReferences:
-    - controller: true
-      kind: PerconaXtraDBCluster
-      name: no-proxysql
 spec:
   ca:
     secretName: no-proxysql-ca-cert

e2e-tests/tls-issue-cert-manager-ref/compare/certificate_some-name-tls-issueref-ssl.yml

@@ -3,10 +3,6 @@ kind: Certificate
 metadata:
   generation: 1
   name: some-name-tls-issueref-ssl
-  ownerReferences:
-    - controller: true
-      kind: PerconaXtraDBCluster
-      name: some-name-tls-issueref
 spec:
   commonName: some-name-tls-issueref-proxysql
   dnsNames:

e2e-tests/tls-issue-cert-manager/compare/certificate_some-name-tls-issue-ssl.yml

@@ -3,10 +3,6 @@ kind: Certificate
 metadata:
   generation: 1
   name: some-name-tls-issue-ssl
-  ownerReferences:
-    - controller: true
-      kind: PerconaXtraDBCluster
-      name: some-name-tls-issue
 spec:
   commonName: some-name-tls-issue-proxysql
   dnsNames:

e2e-tests/tls-issue-cert-manager/compare/issuer_some-name-tls-issue-pxc-ca-issuer.yml

@@ -3,9 +3,5 @@ kind: Issuer
 metadata:
   generation: 1
   name: some-name-tls-issue-pxc-ca-issuer
-  ownerReferences:
-    - controller: true
-      kind: PerconaXtraDBCluster
-      name: some-name-tls-issue
 spec:
   selfSigned: {}

e2e-tests/tls-issue-cert-manager/compare/issuer_some-name-tls-issue-pxc-issuer.yml

@@ -3,10 +3,6 @@ kind: Issuer
 metadata:
   generation: 1
   name: some-name-tls-issue-pxc-issuer
-  ownerReferences:
-    - controller: true
-      kind: PerconaXtraDBCluster
-      name: some-name-tls-issue
 spec:
   ca:
     secretName: some-name-tls-issue-ca-cert

pkg/controller/pxc/controller.go

@@ -10,6 +10,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	cm "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	"github.com/go-logr/logr"
 	"github.com/go-logr/zapr"
 	"github.com/pkg/errors"
@@ -228,6 +229,8 @@ func (r *ReconcilePerconaXtraDBCluster) Reconcile(_ context.Context, request rec
 		for _, fnlz := range o.GetFinalizers() {
 			var sfs api.StatefulApp
 			switch fnlz {
+			case "delete-ssl":
+				err = r.deleteCerts(o)
 			case "delete-proxysql-pvc":
 				sfs = statefulset.NewProxy(o)
 				// deletePVC is always true on this stage
@@ -1172,6 +1175,67 @@ func (r *ReconcilePerconaXtraDBCluster) deleteSecrets(cr *api.PerconaXtraDBClust
 	return nil
 }
 
+func (r *ReconcilePerconaXtraDBCluster) deleteCerts(cr *api.PerconaXtraDBCluster) error {
+	issuers := []string{
+		cr.Name + "-pxc-ca-issuer",
+		cr.Name + "-pxc-issuer",
+	}
+	for _, issuerName := range issuers {
+		issuer := &cm.Issuer{}
+		err := r.client.Get(context.TODO(), types.NamespacedName{
+			Namespace: cr.Namespace,
+			Name:      issuerName,
+		}, issuer)
+		if err != nil {
+			continue
+		}
+
+		err = r.client.Delete(context.TODO(), issuer, &client.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &issuer.UID}})
+		if err != nil {
+			return errors.Wrapf(err, "delete issuer %s", issuerName)
+		}
+	}
+
+	certs := []string{
+		cr.Name + "-ssl",
+		cr.Name + "-ssl-internal",
+		cr.Name + "-ca-cert",
+	}
+	for _, certName := range certs {
+		secret := &corev1.Secret{}
+		err := r.client.Get(context.TODO(), types.NamespacedName{
+			Namespace: cr.Namespace,
+			Name:      certName,
+		}, secret)
+		if client.IgnoreNotFound(err) != nil {
+			continue
+		}
+
+		err = r.client.Delete(context.TODO(), secret,
+			&client.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &secret.UID}})
+		if client.IgnoreNotFound(err) != nil {
+			return errors.Wrapf(err, "delete secret %s", certName)
+		}
+
+		cert := &cm.Certificate{}
+		err = r.client.Get(context.TODO(), types.NamespacedName{
+			Namespace: cr.Namespace,
+			Name:      certName,
+		}, cert)
+		if err != nil {
+			continue
+		}
+
+		err = r.client.Delete(context.TODO(), cert,
+			&client.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &cert.UID}})
+		if err != nil {
+			return errors.Wrapf(err, "delete certificate %s", certName)
+		}
+	}
+
+	return nil
+}
+
 func setControllerReference(ro runtime.Object, obj metav1.Object, scheme *runtime.Scheme) error {
 	ownerRef, err := OwnerRef(ro, scheme)
 	if err != nil {

pkg/controller/pxc/tls.go

@@ -62,12 +62,6 @@ func (r *ReconcilePerconaXtraDBCluster) reconcileSSL(cr *api.PerconaXtraDBCluste
 }
 
 func (r *ReconcilePerconaXtraDBCluster) createSSLByCertManager(cr *api.PerconaXtraDBCluster) error {
-	owner, err := OwnerRef(cr, r.scheme)
-	if err != nil {
-		return err
-	}
-	ownerReferences := []metav1.OwnerReference{owner}
-
 	issuerName := cr.Name + "-pxc-issuer"
 	caIssuerName := cr.Name + "-pxc-ca-issuer"
 	issuerKind := "Issuer"
@@ -77,15 +71,14 @@ func (r *ReconcilePerconaXtraDBCluster) createSSLByCertManager(cr *api.PerconaXt
 		issuerName = cr.Spec.TLS.IssuerConf.Name
 		issuerGroup = cr.Spec.TLS.IssuerConf.Group
 	} else {
-		if err := r.createIssuer(ownerReferences, cr.Namespace, caIssuerName, ""); err != nil {
+		if err := r.createIssuer(cr.Namespace, caIssuerName, ""); err != nil {
 			return err
 		}
 
 		caCert := &cm.Certificate{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:            cr.Name + "-ca-cert",
-				Namespace:       cr.Namespace,
-				OwnerReferences: ownerReferences,
+				Name:      cr.Name + "-ca-cert",
+				Namespace: cr.Namespace,
 			},
 			Spec: cm.CertificateSpec{
 				SecretName: cr.Name + "-ca-cert",
@@ -101,7 +94,7 @@ func (r *ReconcilePerconaXtraDBCluster) createSSLByCertManager(cr *api.PerconaXt
 			},
 		}
 
-		err = r.client.Create(context.TODO(), caCert)
+		err := r.client.Create(context.TODO(), caCert)
 		if err != nil && !k8serr.IsAlreadyExists(err) {
 			return fmt.Errorf("create CA certificate: %v", err)
 		}
@@ -110,16 +103,15 @@ func (r *ReconcilePerconaXtraDBCluster) createSSLByCertManager(cr *api.PerconaXt
 			return err
 		}
 
-		if err := r.createIssuer(ownerReferences, cr.Namespace, issuerName, caCert.Spec.SecretName); err != nil {
+		if err := r.createIssuer(cr.Namespace, issuerName, caCert.Spec.SecretName); err != nil {
 			return err
 		}
 	}
 
 	kubeCert := &cm.Certificate{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:            cr.Name + "-ssl",
-			Namespace:       cr.Namespace,
-			OwnerReferences: ownerReferences,
+			Name:      cr.Name + "-ssl",
+			Namespace: cr.Namespace,
 		},
 		Spec: cm.CertificateSpec{
 			SecretName: cr.Spec.PXC.SSLSecretName,
@@ -143,7 +135,7 @@ func (r *ReconcilePerconaXtraDBCluster) createSSLByCertManager(cr *api.PerconaXt
 		kubeCert.Spec.DNSNames = append(kubeCert.Spec.DNSNames, cr.Spec.TLS.SANs...)
 	}
 
-	err = r.client.Create(context.TODO(), kubeCert)
+	err := r.client.Create(context.TODO(), kubeCert)
 	if err != nil && !k8serr.IsAlreadyExists(err) {
 		return fmt.Errorf("create certificate: %v", err)
 	}
@@ -154,9 +146,8 @@ func (r *ReconcilePerconaXtraDBCluster) createSSLByCertManager(cr *api.PerconaXt
 
 	kubeCert = &cm.Certificate{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:            cr.Name + "-ssl-internal",
-			Namespace:       cr.Namespace,
-			OwnerReferences: ownerReferences,
+			Name:      cr.Name + "-ssl-internal",
+			Namespace: cr.Namespace,
 		},
 		Spec: cm.CertificateSpec{
 			SecretName: cr.Spec.PXC.SSLInternalSecretName,
@@ -220,8 +211,7 @@ func (r *ReconcilePerconaXtraDBCluster) waitForCerts(namespace string, secretsLi
 	}
 }
 
-func (r *ReconcilePerconaXtraDBCluster) createIssuer(ownRef []metav1.OwnerReference, namespace, issuer string, caCertSecret string) error {
-
+func (r *ReconcilePerconaXtraDBCluster) createIssuer(namespace, issuer string, caCertSecret string) error {
 	spec := cm.IssuerSpec{}
 
 	if caCertSecret == "" {
@@ -240,9 +230,8 @@ func (r *ReconcilePerconaXtraDBCluster) createIssuer(ownRef []metav1.OwnerRefere
 
 	err := r.client.Create(context.TODO(), &cm.Issuer{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:            issuer,
-			Namespace:       namespace,
-			OwnerReferences: ownRef,
+			Name:      issuer,
+			Namespace: namespace,
 		},
 		Spec: spec,
 	})