From aaec014967f645cf7316412fe52f1e706a94e641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wilson=20J=C3=BAnior?= Date: Mon, 19 Aug 2024 11:44:03 -0300 Subject: [PATCH] Use SecretTemplate instead of manual label propagation --- .../controllers/certificates/cert_manager.go | 13 ++-- .../certificates/cert_manager_test.go | 42 +++++++++++++ .../controllers/certificates/certificates.go | 34 ---------- .../certificates/certificates_test.go | 62 ------------------- 4 files changed, 49 insertions(+), 102 deletions(-) diff --git a/internal/controllers/certificates/cert_manager.go b/internal/controllers/certificates/cert_manager.go index 672bb5d8..2b60a98b 100644 --- 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 index 6caf511c..98ca859f 100644 --- 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 index f80b7ba6..af5e80e8 100644 --- 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 index 026a9185..ba2fd084 100644 --- 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