Use SecretTemplate instead of manual label propagation

tsuru · Aug 19, 2024 · aaec014 · aaec014
1 parent 6610bd5
commit aaec014
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 102 deletions.
diff --git a/internal/controllers/certificates/cert_manager.go b/internal/controllers/certificates/cert_manager.go
@@ -67,16 +67,11 @@ func ReconcileCertManager(ctx context.Context, client client.Client, instance, i
 			}
 		}
 
-		certManagerCerts = append(certManagerCerts, cert)
-
 		if !isCertificateReady(&cert) {
 			continue
 		}
 
-		err = UpdateCertificateFromSecret(ctx, client, instance, cmCertificateName(req), newCert.Spec.SecretName)
-		if err != nil {
-			return nil, err
-		}
+		certManagerCerts = append(certManagerCerts, cert)
 	}
 
 	return certManagerCerts, nil
@@ -214,6 +209,12 @@ func newCertificate(instance *v1alpha1.RpaasInstance, issuer *cmmeta.ObjectRefer
 			DNSNames:    req.DNSNames,
 			IPAddresses: req.IPAddresses,
 			SecretName:  fmt.Sprintf("%s-%s", instance.Name, cmCertificateName(req)),
+			SecretTemplate: &cmv1.CertificateSecretTemplate{
+				Labels: map[string]string{
+					"rpaas.extensions.tsuru.io/certificate-name": cmCertificateName(req),
+					"rpaas.extensions.tsuru.io/instance-name":    instance.Name,
+				},
+			},
 		},
 	}, nil
 }

diff --git a/internal/controllers/certificates/cert_manager_test.go b/internal/controllers/certificates/cert_manager_test.go
@@ -232,6 +232,12 @@ wg4cGbIbBPs=
 					SecretName:  cert.Name,
 					DNSNames:    []string{"my-instance.example.com"},
 					IPAddresses: []string{"169.196.1.100"},
+					SecretTemplate: &cmv1.CertificateSecretTemplate{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/certificate-name": "cert-manager-issuer-1",
+							"rpaas.extensions.tsuru.io/instance-name":    "my-instance",
+						},
+					},
 				}, cert.Spec)
 			},
 		},
@@ -314,6 +320,12 @@ wg4cGbIbBPs=
 					},
 					SecretName: "my-instance-cert-01",
 					DNSNames:   []string{"my-instance.example.com"},
+					SecretTemplate: &cmv1.CertificateSecretTemplate{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/certificate-name": "cert-01",
+							"rpaas.extensions.tsuru.io/instance-name":    "my-instance",
+						},
+					},
 				}, certs[0].Spec)
 
 				assert.Equal(t, cmv1.CertificateSpec{
@@ -324,6 +336,12 @@ wg4cGbIbBPs=
 					},
 					SecretName: "my-instance-cert-02",
 					DNSNames:   []string{"my-instance2.example.com"},
+					SecretTemplate: &cmv1.CertificateSecretTemplate{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/certificate-name": "cert-02",
+							"rpaas.extensions.tsuru.io/instance-name":    "my-instance",
+						},
+					},
 				}, certs[1].Spec)
 
 				assert.Equal(t, cmv1.CertificateSpec{
@@ -334,6 +352,12 @@ wg4cGbIbBPs=
 					},
 					SecretName: "my-instance-cert-03",
 					DNSNames:   []string{"my-instance3.example.org"},
+					SecretTemplate: &cmv1.CertificateSecretTemplate{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/certificate-name": "cert-03",
+							"rpaas.extensions.tsuru.io/instance-name":    "my-instance",
+						},
+					},
 				}, certs[2].Spec)
 			},
 		},
@@ -402,6 +426,12 @@ wg4cGbIbBPs=
 					},
 					SecretName: "my-instance-take-over-test-my-instance-take-over",
 					DNSNames:   []string{"my-instance.example.com"},
+					SecretTemplate: &cmv1.CertificateSecretTemplate{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/certificate-name": "my-instance-take-over",
+							"rpaas.extensions.tsuru.io/instance-name":    "my-instance-take-over-test",
+						},
+					},
 				}, certs[0].Spec)
 
 				secret := corev1.Secret{}
@@ -468,6 +498,12 @@ wg4cGbIbBPs=
 					},
 					SecretName: cert.Name,
 					DNSNames:   []string{"my-instance.rpaasv2.example.org"},
+					SecretTemplate: &cmv1.CertificateSecretTemplate{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/certificate-name": "cert-manager-issuer-1",
+							"rpaas.extensions.tsuru.io/instance-name":    "my-instance",
+						},
+					},
 				}, cert.Spec)
 			},
 		},
@@ -522,6 +558,12 @@ wg4cGbIbBPs=
 					SecretName:  cert.Name,
 					DNSNames:    []string{"my-instance-2.example.com", "app1.example.com"},
 					IPAddresses: []string{"2001:db8:dead:beef::"},
+					SecretTemplate: &cmv1.CertificateSecretTemplate{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/certificate-name": "cert-manager-cluster-issuer-1",
+							"rpaas.extensions.tsuru.io/instance-name":    "my-instance-2",
+						},
+					},
 				}, cert.Spec)
 			},
 		},

diff --git a/internal/controllers/certificates/certificates.go b/internal/controllers/certificates/certificates.go
@@ -15,7 +15,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/tsuru/rpaas-operator/api/v1alpha1"
@@ -28,39 +27,6 @@ var (
 	ErrTooManyTLSSecretsFound = fmt.Errorf("too many TLS secrets found")
 )
 
-func UpdateCertificateFromSecret(ctx context.Context, c client.Client, instance *v1alpha1.RpaasInstance, certificateName, secretName string) error {
-	if c == nil {
-		return fmt.Errorf("kubernetes client cannot be nil")
-	}
-
-	if instance == nil {
-		return fmt.Errorf("rpaasinstance cannot be nil")
-	}
-
-	var s corev1.Secret
-	err := c.Get(ctx, types.NamespacedName{Name: secretName, Namespace: instance.Namespace}, &s)
-	if err != nil {
-		return err
-	}
-
-	originalSecret := s.DeepCopy()
-
-	if s.Labels == nil {
-		s.Labels = make(map[string]string)
-	}
-
-	s.Labels[CertificateNameLabel] = certificateName
-	s.Labels["rpaas.extensions.tsuru.io/instance-name"] = instance.Name
-
-	if !reflect.DeepEqual(originalSecret.Labels, s.Labels) {
-		if err = c.Update(ctx, &s); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 func UpdateCertificate(ctx context.Context, c client.Client, instance *v1alpha1.RpaasInstance, certificateName string, certData, keyData []byte) error {
 	if c == nil {
 		return fmt.Errorf("kubernetes client cannot be nil")

diff --git a/internal/controllers/certificates/certificates_test.go b/internal/controllers/certificates/certificates_test.go
@@ -122,68 +122,6 @@ cHPInBo/AIYDW+WuoTwDOgAE3zfoDUUeS7WV7ZmC2A1kWxGvzKU6nOFBPmTxdCT2
 	}
 }
 
-func TestUpdateCertificateFromSecret(t *testing.T) {
-	tests := map[string]struct {
-		instance        string
-		certificateName string
-		secretName      string
-		expectedError   string
-		assert          func(t *testing.T, c client.Client)
-	}{
-		"adding a certificate from secret (e.g. cert manager)": {
-			instance:        "my-instance-3",
-			certificateName: "cert-manager",
-			secretName:      "my-instance-3-cert-manager",
-			assert: func(t *testing.T, c client.Client) {
-				var s corev1.Secret
-				err := c.Get(context.TODO(), types.NamespacedName{Name: "my-instance-3-cert-manager", Namespace: "rpaasv2"}, &s)
-				require.NoError(t, err)
-
-				assert.Equal(t, "cert-manager", s.Labels["rpaas.extensions.tsuru.io/certificate-name"])
-				assert.Equal(t, "my-instance-3", s.Labels["rpaas.extensions.tsuru.io/instance-name"])
-
-				var i v1alpha1.RpaasInstance
-				err = c.Get(context.TODO(), types.NamespacedName{Name: "my-instance-3", Namespace: "rpaasv2"}, &i)
-				assert.NoError(t, err)
-
-				assert.Nil(t, i.Spec.TLS)
-			},
-		},
-	}
-
-	for name, tt := range tests {
-		t.Run(name, func(t *testing.T) {
-			resources := k8sResources()
-
-			client := fake.NewClientBuilder().
-				WithScheme(runtime.NewScheme()).
-				WithRuntimeObjects(resources...).
-				Build()
-
-			var instance *v1alpha1.RpaasInstance
-			for _, object := range resources {
-				if i, found := object.(*v1alpha1.RpaasInstance); found && i.Name == tt.instance {
-					instance = i
-					break
-				}
-			}
-
-			require.NotNil(t, instance, "you should select a RpaasInstance from resources")
-
-			err := certificates.UpdateCertificateFromSecret(context.TODO(), client, instance, tt.certificateName, tt.secretName)
-			if tt.expectedError != "" {
-				assert.EqualError(t, err, tt.expectedError)
-				return
-			}
-
-			require.NoError(t, err)
-
-			require.NotNil(t, tt.assert, "you must provide an assert function")
-			tt.assert(t, client)
-		})
-	}
-}
-
 func TestUpdateCertificate(t *testing.T) {
 	tests := map[string]struct {
 		instance        string