From 9d8fc9a3a6b2be93033c01355aaee384cc392c20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wilson=20J=C3=BAnior?= Date: Wed, 23 Oct 2024 09:51:12 -0300 Subject: [PATCH] Drop cert-manager certificate when the certissuer is unset --- kubernetes/ingress.go | 60 ++++++++++++--- kubernetes/ingress_test.go | 148 +++++++++++++++++++++++++++++++++++++ router/service.go | 8 ++ 3 files changed, 206 insertions(+), 10 deletions(-) diff --git a/kubernetes/ingress.go b/kubernetes/ingress.go index 8bde657..d310c64 100644 --- a/kubernetes/ingress.go +++ b/kubernetes/ingress.go @@ -264,7 +264,8 @@ func (k *IngressService) mergeIngresses(ctx context.Context, ingress *networking if existingIngress.Spec.DefaultBackend != nil { ingress.Spec.DefaultBackend = existingIngress.Spec.DefaultBackend } - if existingIngress.Spec.TLS != nil && len(existingIngress.Spec.TLS) > 0 { + + if existingIngress.Spec.TLS != nil && len(existingIngress.Spec.TLS) > 0 && !isManagedByCertManager(existingIngress.Annotations) { k.fillIngressTLS(ingress, id) } _, err := ingressClient.Update(ctx, ingress, metav1.UpdateOptions{}) @@ -381,16 +382,14 @@ func (k *IngressService) ensureCNameBackend(ctx context.Context, opts ensureCNam } k.fillIngressMeta(ingress, opts.routerOpts, opts.id, opts.team, opts.tags) - if opts.routerOpts.AcmeCName { + + if opts.routerOpts.HTTPOnly { + k.cleanupCertManagerAnnotations(ingress) + } else if opts.routerOpts.AcmeCName { k.fillIngressTLS(ingress, opts.id) ingress.ObjectMeta.Annotations[AnnotationsACMEKey] = "true" - } else { - k.cleanupCertManagerAnnotations(ingress) - } - - if opts.routerOpts.Acme { - err = k.ensureCertManagerIssuer(ctx, opts, ingress) + err = k.ensureCNAMECertManagerIssuer(ctx, opts, ingress) if err != nil { return err } @@ -402,12 +401,35 @@ func (k *IngressService) ensureCNameBackend(ctx context.Context, opts ensureCNam } if ingressHasChanges(span, existingIngress, ingress) { - return k.mergeIngresses(ctx, ingress, existingIngress, opts.id, ingressClient, span) + err = k.mergeIngresses(ctx, ingress, existingIngress, opts.id, ingressClient, span) + if err != nil { + return err + } + } + + if len(ingress.Spec.TLS) == 0 { + certificateName := k.secretName(opts.id, opts.cname) + return k.ensureCertmanagerCertificateDeleted(ctx, opts.namespace, certificateName) + } + + return nil +} + +func (k *IngressService) ensureCertmanagerCertificateDeleted(ctx context.Context, namespace, certificateName string) error { + certManagerClient, err := k.getCertManagerClient() + if err != nil { + return err + } + + err = certManagerClient.CertmanagerV1().Certificates(namespace).Delete(ctx, certificateName, metav1.DeleteOptions{}) + if err != nil && !k8sErrors.IsNotFound(err) { + return err } + return nil } -func (k *IngressService) ensureCertManagerIssuer(ctx context.Context, opts ensureCNameBackendOpts, ingress *networkingV1.Ingress) error { +func (k *IngressService) ensureCNAMECertManagerIssuer(ctx context.Context, opts ensureCNameBackendOpts, ingress *networkingV1.Ingress) error { if opts.certIssuer == "" { // If no cert issuer is provided, we should remove any existing cert issuer annotation k.cleanupCertManagerAnnotations(ingress) @@ -608,6 +630,10 @@ func (k *IngressService) AddCertificate(ctx context.Context, id router.InstanceI return err } + if isManagedByCertManager(ingress.Annotations) { + return fmt.Errorf("cannot add certificate to ingress %s, it is managed by cert-manager", ingress.Name) + } + foundCname := false foundCNames := []string{} for _, rules := range ingress.Spec.Rules { @@ -741,6 +767,11 @@ func (k *IngressService) RemoveCertificate(ctx context.Context, id router.Instan if ingress.Annotations[AnnotationsACMEKey] == "true" { return fmt.Errorf("cannot remove certificate from ingress %s, it is managed by ACME", ingress.Name) } + + if isManagedByCertManager(ingress.Annotations) { + return fmt.Errorf("cannot remove certificate to ingress %s, it is managed by cert-manager", ingress.Name) + } + secret, err := k.secretClient(ns) if err != nil { return err @@ -993,3 +1024,12 @@ func isIngressReady(ingress *networkingV1.Ingress) bool { } return ingress.Status.LoadBalancer.Ingress[0].IP != "" || ingress.Status.LoadBalancer.Ingress[0].Hostname != "" } + +func isManagedByCertManager(annotations map[string]string) bool { + for _, annotation := range certManagerAnnotations { + if _, ok := annotations[annotation]; ok { + return true + } + } + return false +} diff --git a/kubernetes/ingress_test.go b/kubernetes/ingress_test.go index d81d999..cfeaf5c 100644 --- a/kubernetes/ingress_test.go +++ b/kubernetes/ingress_test.go @@ -575,6 +575,154 @@ func TestIngressEnsureWithTags(t *testing.T) { assert.Equal(t, expectedIngress, foundIngress) } +func TestIngressEnsureRemoveCertAnnotations(t *testing.T) { + svc := createFakeService(false) + + svc.CertManagerClient.CertmanagerV1().Certificates(svc.Namespace).Create(context.TODO(), &certmanagerv1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kr-test-test.io", + }, + Spec: certmanagerv1.CertificateSpec{ + CommonName: "test.io", + }, + }, metav1.CreateOptions{}) + + createCertManagerIssuer(svc.CertManagerClient, svc.Namespace, "letsencrypt") + + svc.Labels = map[string]string{"controller": "my-controller", "XPTO": "true"} + svc.Annotations = map[string]string{"ann1": "val1", "ann2": "val2"} + err := svc.Ensure(ctx, idForApp("test"), router.EnsureBackendOpts{ + Opts: router.Opts{}, + Team: "default", + CNames: []string{"test.io"}, + CertIssuers: map[string]string{ + "test.io": "letsencrypt", + }, + Prefixes: []router.BackendPrefix{ + { + Target: router.BackendTarget{ + Service: "test-web", + Namespace: "default", + }, + }, + }, + }) + require.NoError(t, err) + foundIngress, err := svc.Client.NetworkingV1().Ingresses(svc.Namespace).Get(ctx, "kubernetes-router-cname-test.io", metav1.GetOptions{}) + require.NoError(t, err) + + expectedIngress := defaultIngress("test", "default") + expectedIngress.ObjectMeta.Name = "kubernetes-router-cname-test.io" + expectedIngress.Labels["controller"] = "my-controller" + expectedIngress.Labels["XPTO"] = "true" + expectedIngress.Labels["tsuru.io/app-name"] = "test" + expectedIngress.Labels["tsuru.io/app-team"] = "default" + expectedIngress.Labels["router.tsuru.io/is-cname-ingress"] = "true" + expectedIngress.Annotations["ann1"] = "val1" + expectedIngress.Annotations["ann2"] = "val2" + expectedIngress.Annotations["cert-manager.io/common-name"] = "test.io" + expectedIngress.Annotations["cert-manager.io/issuer"] = "letsencrypt" + expectedIngress.Spec.Rules[0].Host = "test.io" + expectedIngress.Spec.TLS = []networkingV1.IngressTLS{ + { + Hosts: []string{"test.io"}, + SecretName: "kr-test-test.io", + }, + } + + assert.Equal(t, expectedIngress, foundIngress) + + cert, err := svc.CertManagerClient.CertmanagerV1().Certificates(svc.Namespace).Get(context.TODO(), "kr-test-test.io", metav1.GetOptions{}) + require.NoError(t, err) + assert.NotNil(t, cert) + + err = svc.Ensure(ctx, idForApp("test"), router.EnsureBackendOpts{ + Opts: router.Opts{}, + Team: "default", + CNames: []string{"test.io"}, + CertIssuers: nil, // drop certIssuers + Prefixes: []router.BackendPrefix{ + { + Target: router.BackendTarget{ + Service: "test-web", + Namespace: "default", + }, + }, + }, + }) + + require.NoError(t, err) + foundIngress, err = svc.Client.NetworkingV1().Ingresses(svc.Namespace).Get(ctx, "kubernetes-router-cname-test.io", metav1.GetOptions{}) + require.NoError(t, err) + + expectedIngress = defaultIngress("test", "default") + expectedIngress.ObjectMeta.Name = "kubernetes-router-cname-test.io" + expectedIngress.Labels["controller"] = "my-controller" + expectedIngress.Labels["XPTO"] = "true" + expectedIngress.Labels["tsuru.io/app-name"] = "test" + expectedIngress.Labels["tsuru.io/app-team"] = "default" + expectedIngress.Labels["router.tsuru.io/is-cname-ingress"] = "true" + expectedIngress.Annotations["ann1"] = "val1" + expectedIngress.Annotations["ann2"] = "val2" + delete(expectedIngress.Annotations, "cert-manager.io/common-name") + delete(expectedIngress.Annotations, "cert-manager.io/issuer") + expectedIngress.Spec.Rules[0].Host = "test.io" + expectedIngress.Spec.TLS = nil + + assert.Equal(t, expectedIngress, foundIngress) + + cert, err = svc.CertManagerClient.CertmanagerV1().Certificates(svc.Namespace).Get(context.TODO(), "kr-test-test.io", metav1.GetOptions{}) + require.True(t, k8sErrors.IsNotFound(err)) + assert.Nil(t, cert) + +} + +func TestIngressEnsureHTTPOnly(t *testing.T) { + svc := createFakeService(false) + + createCertManagerIssuer(svc.CertManagerClient, svc.Namespace, "letsencrypt") + + svc.Labels = map[string]string{"controller": "my-controller", "XPTO": "true"} + svc.Annotations = map[string]string{"ann1": "val1", "ann2": "val2"} + err := svc.Ensure(ctx, idForApp("test"), router.EnsureBackendOpts{ + Opts: router.Opts{ + HTTPOnly: true, + }, + Team: "default", + CNames: []string{"test.io"}, + CertIssuers: map[string]string{ + "test.io": "letsencrypt", + }, + Prefixes: []router.BackendPrefix{ + { + Target: router.BackendTarget{ + Service: "test-web", + Namespace: "default", + }, + }, + }, + }) + require.NoError(t, err) + + expectedIngress := defaultIngress("test", "default") + expectedIngress.ObjectMeta.Name = "kubernetes-router-cname-test.io" + expectedIngress.Labels["controller"] = "my-controller" + expectedIngress.Labels["XPTO"] = "true" + expectedIngress.Labels["tsuru.io/app-name"] = "test" + expectedIngress.Labels["tsuru.io/app-team"] = "default" + expectedIngress.Labels["router.tsuru.io/is-cname-ingress"] = "true" + expectedIngress.Annotations["ann1"] = "val1" + expectedIngress.Annotations["ann2"] = "val2" + delete(expectedIngress.Annotations, "cert-manager.io/common-name") + delete(expectedIngress.Annotations, "cert-manager.io/issuer") + expectedIngress.Spec.Rules[0].Host = "test.io" + expectedIngress.Spec.TLS = nil + + foundIngress, err := svc.Client.NetworkingV1().Ingresses(svc.Namespace).Get(ctx, "kubernetes-router-cname-test.io", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, expectedIngress, foundIngress) +} + func TestEnsureCertManagerIssuer(t *testing.T) { svc := createFakeService(false) diff --git a/router/service.go b/router/service.go index 84e8da9..942ab86 100644 --- a/router/service.go +++ b/router/service.go @@ -32,6 +32,8 @@ const ( // Acme is the acme option name Acme = "tls-acme" + HTTPOnly = "http-only" + // AcmeCName is the acme option for cnames AcmeCName = "tls-acme-cname" @@ -94,6 +96,7 @@ type Opts struct { AdditionalOpts map[string]string `json:",omitempty"` HeaderOpts []string `json:",omitempty"` Acme bool `json:",omitempty"` + HTTPOnly bool `json:",omitempty"` AcmeCName bool `json:",omitempty"` ExposeAllServices bool `json:",omitempty"` } @@ -192,6 +195,11 @@ func (o *Opts) UnmarshalJSON(bs []byte) (err error) { o.Route = strV case ExternalTrafficPolicy: o.ExternalTrafficPolicy = strV + case HTTPOnly: + o.HTTPOnly, err = strconv.ParseBool(strV) + if err != nil { + o.HTTPOnly = false + } case Acme: o.Acme, err = strconv.ParseBool(strV) if err != nil {