From f7c4ae5e503a140d33165c2e66b606fdd5543217 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wilson=20J=C3=BAnior?= Date: Mon, 16 Sep 2024 11:24:58 -0300 Subject: [PATCH] Add support to validation using multiple cert-manager requests --- api/v1alpha1/rpaasinstance.go | 20 ++++----- api/v1alpha1/rpaasinstance_test.go | 2 +- controllers/rpaasinstance_controller.go | 2 +- controllers/validation_controller.go | 41 +++++++++++++++---- controllers/validation_controller_test.go | 19 +++++---- .../controllers/certificates/cert_manager.go | 29 ++++++++++++- .../controllers/certificates/certificates.go | 6 +-- internal/pkg/rpaas/certificates.go | 2 +- internal/pkg/rpaas/k8s.go | 2 +- 9 files changed, 88 insertions(+), 35 deletions(-) diff --git a/api/v1alpha1/rpaasinstance.go b/api/v1alpha1/rpaasinstance.go index 267445af7..0c160a6e2 100644 --- a/api/v1alpha1/rpaasinstance.go +++ b/api/v1alpha1/rpaasinstance.go @@ -64,17 +64,17 @@ func (i *RpaasInstance) BelongsToCluster(clusterName string) bool { return clusterName == instanceCluster } -func (i *RpaasInstance) CertManagerRequests() (reqs []CertManager) { - if i == nil || i.Spec.DynamicCertificates == nil { +func (s *RpaasInstanceSpec) CertManagerRequests(name string) (reqs []CertManager) { + if s == nil || s.DynamicCertificates == nil { return } uniqueCertsByIssuer := make(map[string]*CertManager) uniqueCertsByName := make(map[string]*CertManager) - if req := i.Spec.DynamicCertificates.CertManager; req != nil { + if req := s.DynamicCertificates.CertManager; req != nil { r := req.DeepCopy() - r.DNSNames = r.dnsNames(i) + r.DNSNames = r.dnsNames(name, s) if req.Name != "" { uniqueCertsByName[req.Name] = r @@ -83,12 +83,12 @@ func (i *RpaasInstance) CertManagerRequests() (reqs []CertManager) { } } - for _, req := range i.Spec.DynamicCertificates.CertManagerRequests { + for _, req := range s.DynamicCertificates.CertManagerRequests { if req.Name != "" { r, found := uniqueCertsByName[req.Name] if found { - r.DNSNames = append(r.DNSNames, req.dnsNames(i)...) + r.DNSNames = append(r.DNSNames, req.dnsNames(name, s)...) r.IPAddresses = append(r.IPAddresses, req.IPAddresses...) } else { uniqueCertsByName[req.Name] = req.DeepCopy() @@ -103,7 +103,7 @@ func (i *RpaasInstance) CertManagerRequests() (reqs []CertManager) { continue } - r.DNSNames = append(r.DNSNames, req.dnsNames(i)...) + r.DNSNames = append(r.DNSNames, req.dnsNames(name, s)...) r.IPAddresses = append(r.IPAddresses, req.IPAddresses...) } @@ -125,14 +125,14 @@ func (i *RpaasInstance) CertManagerRequests() (reqs []CertManager) { return } -func (c *CertManager) dnsNames(i *RpaasInstance) (names []string) { +func (c *CertManager) dnsNames(name string, spec *RpaasInstanceSpec) (names []string) { if c == nil { return } names = append(names, c.DNSNames...) - if c.DNSNamesDefault && i.Spec.DNS != nil && i.Spec.DNS.Zone != "" { - names = append(names, fmt.Sprintf("%s.%s", i.Name, i.Spec.DNS.Zone)) + if c.DNSNamesDefault && spec.DNS != nil && spec.DNS.Zone != "" { + names = append(names, fmt.Sprintf("%s.%s", name, spec.DNS.Zone)) } return diff --git a/api/v1alpha1/rpaasinstance_test.go b/api/v1alpha1/rpaasinstance_test.go index b9437dca3..c01d9e144 100644 --- a/api/v1alpha1/rpaasinstance_test.go +++ b/api/v1alpha1/rpaasinstance_test.go @@ -105,5 +105,5 @@ func TestCertManagerRequests(t *testing.T) { IPAddresses: []string{"10.1.1.1", "10.1.1.2"}, DNSNamesDefault: true, }, - }, instance.CertManagerRequests()) + }, instance.Spec.CertManagerRequests(instance.Name)) } diff --git a/controllers/rpaasinstance_controller.go b/controllers/rpaasinstance_controller.go index f591b28a0..18bfb09f5 100644 --- a/controllers/rpaasinstance_controller.go +++ b/controllers/rpaasinstance_controller.go @@ -157,7 +157,7 @@ func (r *RpaasInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Reques changes := map[string]bool{} - certificateSecrets, err := certificates.ListCertificateSecrets(ctx, r.Client, instanceMergedWithFlavors) + certificateSecrets, err := certificates.ListCertificateSecrets(ctx, r.Client, instanceMergedWithFlavors.Namespace, instanceMergedWithFlavors.Name) if err != nil { return reportError(err) } diff --git a/controllers/validation_controller.go b/controllers/validation_controller.go index e4285af12..f72317ea9 100644 --- a/controllers/validation_controller.go +++ b/controllers/validation_controller.go @@ -18,7 +18,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" + nginxv1alpha1 "github.com/tsuru/nginx-operator/api/v1alpha1" "github.com/tsuru/rpaas-operator/api/v1alpha1" + "github.com/tsuru/rpaas-operator/internal/controllers/certificates" "github.com/tsuru/rpaas-operator/internal/pkg/rpaas/nginx" ) @@ -46,6 +48,9 @@ func (r *RpaasValidationReconciler) Reconcile(ctx context.Context, req ctrl.Requ return reconcile.Result{}, err } + logger := r.Log.WithName("Reconcile"). + WithValues("RpaasValidation", types.NamespacedName{Name: validation.Name, Namespace: validation.Namespace}) + if validation.Status.ObservedGeneration == validation.ObjectMeta.Generation && validation.Status.Valid != nil { return reconcile.Result{}, nil } @@ -76,7 +81,19 @@ func (r *RpaasValidationReconciler) Reconcile(ctx context.Context, req ctrl.Requ } } - rendered, err := r.renderTemplate(ctx, validationMergedWithFlavors, plan) + certificateSecrets, err := certificates.ListCertificateSecrets(ctx, r.Client, validationMergedWithFlavors.Namespace, validationMergedWithFlavors.Name) + if err != nil { + return reconcile.Result{}, err + } + + certManagerCertificates, err := certificates.CertManagerCertificates(ctx, r.Client, validationMergedWithFlavors.Namespace, validationMergedWithFlavors.Name, &validationMergedWithFlavors.Spec) + if err != nil { + return reconcile.Result{}, err + } + + podAnnotations, nginxTLS := newNginxTLS(&logger, certificateSecrets, validationMergedWithFlavors.Spec.TLS, certManagerCertificates) + + rendered, err := r.renderTemplate(ctx, validationMergedWithFlavors, plan, nginxTLS) if err != nil { return reconcile.Result{}, err } @@ -92,7 +109,13 @@ func (r *RpaasValidationReconciler) Reconcile(ctx context.Context, req ctrl.Requ return reconcile.Result{}, err } - pod := newValidationPod(validationMergedWithFlavors, validationHash, plan, configMap) + pod := newValidationPod(validationMergedWithFlavors, validationHash, plan, configMap, nginxTLS) + if pod.Annotations == nil { + pod.Annotations = make(map[string]string) + } + for k, v := range podAnnotations { + pod.Annotations[k] = v + } existingPod, err := r.getPod(ctx, pod.Namespace, pod.Name) if err != nil && !k8sErrors.IsNotFound(err) { @@ -270,7 +293,7 @@ func mergeValidationWithFlavor(validation *v1alpha1.RpaasValidation, flavor v1al return nil } -func (r *RpaasValidationReconciler) renderTemplate(ctx context.Context, validation *v1alpha1.RpaasValidation, plan *v1alpha1.RpaasPlan) (string, error) { +func (r *RpaasValidationReconciler) renderTemplate(ctx context.Context, validation *v1alpha1.RpaasValidation, plan *v1alpha1.RpaasPlan, nginxTLS []nginxv1alpha1.NginxTLS) (string, error) { rf := &referenceFinder{ spec: &validation.Spec, client: r.Client, @@ -291,11 +314,15 @@ func (r *RpaasValidationReconciler) renderTemplate(ctx context.Context, validati return "", err } + validationWithNginxTLS := validation.DeepCopy() + validationWithNginxTLS.Spec.TLS = nginxTLS + config := nginx.ConfigurationData{ Instance: &v1alpha1.RpaasInstance{ - Spec: validation.Spec, + Spec: validationWithNginxTLS.Spec, }, - Config: &plan.Spec.Config, + Config: &plan.Spec.Config, + NginxTLS: nginxTLS, } return cr.Render(config) @@ -371,7 +398,7 @@ func (r *RpaasValidationReconciler) reconcilePod(ctx context.Context, pod *corev return true, nil } -func newValidationPod(validationMergedWithFlavors *v1alpha1.RpaasValidation, validationHash string, plan *v1alpha1.RpaasPlan, configMap *corev1.ConfigMap) *corev1.Pod { +func newValidationPod(validationMergedWithFlavors *v1alpha1.RpaasValidation, validationHash string, plan *v1alpha1.RpaasPlan, configMap *corev1.ConfigMap, nginxTLS []nginxv1alpha1.NginxTLS) *corev1.Pod { n := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: configMap.Name, @@ -466,7 +493,7 @@ func newValidationPod(validationMergedWithFlavors *v1alpha1.RpaasValidation, val }) } - for index, t := range validationMergedWithFlavors.Spec.TLS { + for index, t := range nginxTLS { volumeName := fmt.Sprintf("nginx-certs-%d", index) n.Spec.Volumes = append(n.Spec.Volumes, corev1.Volume{ diff --git a/controllers/validation_controller_test.go b/controllers/validation_controller_test.go index fa9c85786..82d8c2903 100644 --- a/controllers/validation_controller_test.go +++ b/controllers/validation_controller_test.go @@ -39,6 +39,7 @@ func TestNewValidationPod(t *testing.T) { Name: "valid-config", }, }, + []nginxv1alpha1.NginxTLS{}, ) assert.Equal(t, &corev1.Pod{ @@ -115,15 +116,6 @@ func TestNewValidationPodFullFeatured(t *testing.T) { }, }, }, - TLS: []nginxv1alpha1.NginxTLS{ - { - SecretName: "secret-tls", - Hosts: []string{ - "host1", - "host2", - }, - }, - }, TLSSessionResumption: &v1alpha1.TLSSessionResumption{ SessionTicket: &v1alpha1.TLSSessionTicket{ @@ -146,6 +138,15 @@ func TestNewValidationPodFullFeatured(t *testing.T) { Name: "valid-config", }, }, + []nginxv1alpha1.NginxTLS{ + { + SecretName: "secret-tls", + Hosts: []string{ + "host1", + "host2", + }, + }, + }, ) assert.Equal(t, &corev1.Pod{ diff --git a/internal/controllers/certificates/cert_manager.go b/internal/controllers/certificates/cert_manager.go index 1681a687b..05d74d830 100644 --- a/internal/controllers/certificates/cert_manager.go +++ b/internal/controllers/certificates/cert_manager.go @@ -34,7 +34,7 @@ func ReconcileCertManager(ctx context.Context, client client.Client, instance, i certManagerCerts := []cmv1.Certificate{} - for _, req := range instanceMergedWithFlavors.CertManagerRequests() { + for _, req := range instanceMergedWithFlavors.Spec.CertManagerRequests(instance.Name) { issuer, err := getCertManagerIssuer(ctx, client, req, instanceMergedWithFlavors.Namespace) if err != nil { return nil, err @@ -77,6 +77,31 @@ func ReconcileCertManager(ctx context.Context, client client.Client, instance, i return certManagerCerts, nil } +func CertManagerCertificates(ctx context.Context, client client.Client, namespace, name string, spec *v1alpha1.RpaasInstanceSpec) ([]cmv1.Certificate, error) { + certManagerCerts := []cmv1.Certificate{} + + for _, req := range spec.CertManagerRequests(name) { + certName := CertManagerCertificateNameForInstance(name, req) + + var cert cmv1.Certificate + err := client.Get(ctx, types.NamespacedName{Name: certName, Namespace: namespace}, &cert) + if err != nil { + if k8serrors.IsNotFound(err) { + continue + } + return nil, err + } + + if !isCertificateReady(&cert) { + continue + } + + certManagerCerts = append(certManagerCerts, cert) + } + + return certManagerCerts, nil +} + func removeOldCertificates(ctx context.Context, c client.Client, instance, instanceMergedWithFlavors *v1alpha1.RpaasInstance) error { certs, err := getCertificates(ctx, c, instanceMergedWithFlavors) if err != nil { @@ -88,7 +113,7 @@ func removeOldCertificates(ctx context.Context, c client.Client, instance, insta toRemove[cert.Name] = true } - for _, req := range instanceMergedWithFlavors.CertManagerRequests() { + for _, req := range instanceMergedWithFlavors.Spec.CertManagerRequests(instanceMergedWithFlavors.Name) { delete(toRemove, CertManagerCertificateNameForInstance(instance.Name, req)) } diff --git a/internal/controllers/certificates/certificates.go b/internal/controllers/certificates/certificates.go index af5e80e82..35c4ed0b1 100644 --- a/internal/controllers/certificates/certificates.go +++ b/internal/controllers/certificates/certificates.go @@ -135,13 +135,13 @@ func updateInstanceSpecWithCertificateInfos(ctx context.Context, c client.Client return nil } -func ListCertificateSecrets(ctx context.Context, c client.Client, instance *v1alpha1.RpaasInstance) ([]corev1.Secret, error) { +func ListCertificateSecrets(ctx context.Context, c client.Client, namespace, instance string) ([]corev1.Secret, error) { var sl corev1.SecretList err := c.List(ctx, &sl, &client.ListOptions{ LabelSelector: labels.Set{ - "rpaas.extensions.tsuru.io/instance-name": instance.Name, + "rpaas.extensions.tsuru.io/instance-name": instance, }.AsSelector(), - Namespace: instance.Namespace, + Namespace: namespace, }) if err != nil { diff --git a/internal/pkg/rpaas/certificates.go b/internal/pkg/rpaas/certificates.go index 3f0e4f87b..7c2d6bdd7 100644 --- a/internal/pkg/rpaas/certificates.go +++ b/internal/pkg/rpaas/certificates.go @@ -28,7 +28,7 @@ func (m *k8sRpaasManager) GetCertManagerRequests(ctx context.Context, instanceNa } var requests []clientTypes.CertManager - for _, r := range instance.CertManagerRequests() { + for _, r := range instance.Spec.CertManagerRequests(instance.Name) { requests = append(requests, clientTypes.CertManager{ Name: r.Name, Issuer: r.Issuer, diff --git a/internal/pkg/rpaas/k8s.go b/internal/pkg/rpaas/k8s.go index 5dcc4552e..03d2a58b3 100644 --- a/internal/pkg/rpaas/k8s.go +++ b/internal/pkg/rpaas/k8s.go @@ -713,7 +713,7 @@ func (m *k8sRpaasManager) GetCertificates(ctx context.Context, instanceName stri events := make([]clientTypes.Event, 0) - for _, certManagerRequest := range instance.CertManagerRequests() { + for _, certManagerRequest := range instance.Spec.CertManagerRequests(instance.Name) { certName := certManagerRequest.RequiredName() certManagerCertificateName := certificates.CertManagerCertificateNameForInstance(instance.Name, certManagerRequest) certificateEvents, err := m.eventsForObjectName(ctx, instance.Namespace, "Certificate", certManagerCertificateName)