diff --git a/internal/cli/cli.go b/internal/cli/cli.go index f15de0c..38f8311 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -8,6 +8,7 @@ import ( "github.com/kanopy-platform/gateway-certificate-controller/internal/admission" v1beta1controllers "github.com/kanopy-platform/gateway-certificate-controller/internal/controllers/v1beta1" + v1beta1gc "github.com/kanopy-platform/gateway-certificate-controller/internal/controllers/v1beta1/garbagecollection" logzap "github.com/kanopy-platform/gateway-certificate-controller/internal/log/zap" "github.com/spf13/cobra" @@ -136,6 +137,12 @@ func (c *RootCommand) runE(cmd *cobra.Command, args []string) error { return err } + if err := v1beta1gc.NewGarbageCollectionController(ic, cmc, + v1beta1gc.WithDryRun(dryRun)). + SetupWithManager(ctx, mgr); err != nil { + return err + } + admission.NewGatewayMutationHook(ic).SetupWithManager(mgr) return mgr.Start(ctx) diff --git a/internal/controllers/v1beta1/garbagecollection/garbagecollection.go b/internal/controllers/v1beta1/garbagecollection/garbagecollection.go new file mode 100644 index 0000000..bc10adf --- /dev/null +++ b/internal/controllers/v1beta1/garbagecollection/garbagecollection.go @@ -0,0 +1,131 @@ +package garbagecollection + +import ( + "context" + "fmt" + "time" + + certmanagerversionedclient "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned" + certmanagerinformers "github.com/cert-manager/cert-manager/pkg/client/informers/externalversions" + v1beta1labels "github.com/kanopy-platform/gateway-certificate-controller/pkg/v1beta1/labels" + networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" + istioversionedclient "istio.io/client-go/pkg/clientset/versioned" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +type GarbageCollectionController struct { + name string + certmanagerClient certmanagerversionedclient.Interface + istioClient istioversionedclient.Interface + dryRun bool +} + +func NewGarbageCollectionController(istioClient istioversionedclient.Interface, certClient certmanagerversionedclient.Interface, opts ...OptionsFunc) *GarbageCollectionController { + gc := &GarbageCollectionController{ + name: "istio-garbage-collection-controller", + certmanagerClient: certClient, + istioClient: istioClient, + } + + for _, opt := range opts { + opt(gc) + } + + return gc +} + +func (c *GarbageCollectionController) SetupWithManager(ctx context.Context, mgr manager.Manager) error { + ctrl, err := controller.New(c.name, mgr, controller.Options{ + Reconciler: c, + RateLimiter: workqueue.NewItemExponentialFailureRateLimiter(time.Second, 1000*time.Second), // maxDelay is limited by the Informer defaultRsync interval + }) + if err != nil { + return err + } + + certmanagerInformerFactory := certmanagerinformers.NewSharedInformerFactoryWithOptions(c.certmanagerClient, time.Second*30, certmanagerinformers.WithTweakListOptions(func(listOptions *metav1.ListOptions) { + listOptions.LabelSelector = v1beta1labels.ManagedLabelSelector() + })) + + if err := ctrl.Watch(&source.Informer{Informer: certmanagerInformerFactory.Certmanager().V1().Certificates().Informer()}, + handler.Funcs{ + // only handle Update so that Deleting a certificate does not trigger another Reconcile + // Create will also trigger an Update + UpdateFunc: updateFunc, + }); err != nil { + return err + } + + certmanagerInformerFactory.Start(ctx.Done()) + + return nil +} + +func (c *GarbageCollectionController) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { + log := log.FromContext(ctx) + log.V(1).Info("Running Garbage Collection Reconcile", "request", request.String()) + + certIface := c.certmanagerClient.CertmanagerV1().Certificates(request.Namespace) + cert, err := certIface.Get(ctx, request.Name, metav1.GetOptions{}) + if err != nil { + log.Error(err, "failed to Get Certificate") + return reconcile.Result{ + Requeue: true, + }, err + } + + gatewayName, gatewayNamespace := v1beta1labels.ParseManagedLabel(cert.Labels[v1beta1labels.ManagedLabel]) + + deleteCert := false + deleteOptions := metav1.DeleteOptions{} + if c.dryRun { + deleteOptions.DryRun = []string{"All"} + } + + gateway, err := c.istioClient.NetworkingV1beta1().Gateways(gatewayNamespace).Get(ctx, gatewayName, metav1.GetOptions{}) + if k8serrors.IsNotFound(err) { + log.V(1).Info("Gateway not found, marking Certificate for deletion", "gateway-namespace", gatewayNamespace, "gateway", gatewayName) + deleteCert = true + } else if !isCertificateInGatewaySpec(request.Name, gateway) { + log.V(1).Info("Matching Tls.CredentialName not found, marking Certificate for deletion", "gateway-namespace", gatewayNamespace, "gateway", gatewayName) + deleteCert = true + } + + if deleteCert { + log.Info(fmt.Sprintf("Deleting Certificate %s", request), "dry-run", c.dryRun) + if err := certIface.Delete(ctx, request.Name, deleteOptions); err != nil { + log.Error(err, "failed to Delete Certificate") + return reconcile.Result{ + Requeue: true, + }, err + } + } + + return reconcile.Result{}, nil +} + +func updateFunc(e event.UpdateEvent, q workqueue.RateLimitingInterface) { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Name: e.ObjectNew.GetName(), + Namespace: e.ObjectNew.GetNamespace(), + }}) +} + +func isCertificateInGatewaySpec(certificate string, gateway *networkingv1beta1.Gateway) bool { + for _, s := range gateway.Spec.Servers { + if s.Tls.CredentialName == certificate { + return true + } + } + return false +} diff --git a/internal/controllers/v1beta1/garbagecollection/garbagecollection_test.go b/internal/controllers/v1beta1/garbagecollection/garbagecollection_test.go new file mode 100644 index 0000000..913a0fb --- /dev/null +++ b/internal/controllers/v1beta1/garbagecollection/garbagecollection_test.go @@ -0,0 +1,237 @@ +package garbagecollection + +import ( + "context" + "testing" + + v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + certmanagerfake "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/fake" + "github.com/stretchr/testify/assert" + "istio.io/api/networking/v1beta1" + networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" + istiofake "istio.io/client-go/pkg/clientset/versioned/fake" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +func TestNewGarbageCollectionController(t *testing.T) { + t.Parallel() + + certmanagerClient := certmanagerfake.NewSimpleClientset() + istioClient := istiofake.NewSimpleClientset() + dryRun := true + + want := &GarbageCollectionController{ + name: "istio-garbage-collection-controller", + certmanagerClient: certmanagerClient, + istioClient: istioClient, + dryRun: dryRun, + } + + gc := NewGarbageCollectionController(istioClient, certmanagerClient, WithDryRun(dryRun)) + assert.Equal(t, want, gc) +} + +func TestGarbageCollectionControllerReconcile(t *testing.T) { + t.Parallel() + + certificate := &v1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "devops-gateway-123-cert", + Namespace: "routing", + Labels: map[string]string{"v1beta1.kanopy-platform.github.io/istio-cert-controller-managed": "gateway-123.devops"}, + }, + } + + gatewayWithCert := &networkingv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-123", + Namespace: "devops", + }, + Spec: v1beta1.Gateway{ + Servers: []*v1beta1.Server{ + { + Tls: &v1beta1.ServerTLSSettings{ + CredentialName: "devops-gateway-123-diff-cert", + }, + }, + { + Tls: &v1beta1.ServerTLSSettings{ + CredentialName: "devops-gateway-123-cert", // should match certificate name + }, + }, + }, + }, + } + + gatewayWithoutCert := &networkingv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-123", + Namespace: "devops", + }, + Spec: v1beta1.Gateway{ + Servers: []*v1beta1.Server{ + { + Tls: &v1beta1.ServerTLSSettings{ + CredentialName: "devops-gateway-123-diff-cert", + }, + }, + }, + }, + } + + reconcileRequest := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: certificate.Namespace, + Name: certificate.Name, + }, + } + + tests := []struct { + description string + certs []*v1.Certificate + gateways []*networkingv1beta1.Gateway + wantError bool + wantNumCerts int + }{ + { + description: "Certificate points to existing Gateway, no-op", + certs: []*v1.Certificate{certificate}, + gateways: []*networkingv1beta1.Gateway{gatewayWithCert}, + wantError: false, + wantNumCerts: 1, + }, + { + description: "Certificate points to missing Gateway, delete Certificate", + certs: []*v1.Certificate{certificate}, + gateways: []*networkingv1beta1.Gateway{}, // no Gateway + wantError: false, + wantNumCerts: 0, + }, + { + description: "Reconcile called on a Certificate that doesn't exist anymore", + certs: []*v1.Certificate{}, // no Certificate + gateways: []*networkingv1beta1.Gateway{}, + wantError: true, + wantNumCerts: 0, + }, + { + description: "Gateway does not contain Certificate, delete Certificate", + certs: []*v1.Certificate{certificate}, + gateways: []*networkingv1beta1.Gateway{gatewayWithoutCert}, + wantError: false, + wantNumCerts: 0, + }, + } + + for _, test := range tests { + // setup + gc := NewGarbageCollectionController(istiofake.NewSimpleClientset(), certmanagerfake.NewSimpleClientset()) + + for _, cert := range test.certs { + _, err := gc.certmanagerClient.CertmanagerV1().Certificates(cert.Namespace).Create(context.TODO(), cert, metav1.CreateOptions{}) + assert.NoError(t, err, test.description) + } + for _, gateway := range test.gateways { + _, err := gc.istioClient.NetworkingV1beta1().Gateways(gateway.Namespace).Create(context.TODO(), gateway, metav1.CreateOptions{}) + assert.NoError(t, err, test.description) + } + + // test Reconcile + r, err := gc.Reconcile(context.TODO(), reconcileRequest) + + if test.wantError { + assert.Error(t, err, test.description) + assert.Equal(t, reconcile.Result{Requeue: true}, r) + } else { + assert.NoError(t, err, test.description) + assert.Equal(t, reconcile.Result{}, r) + } + + certs, err := gc.certmanagerClient.CertmanagerV1().Certificates(certificate.Namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err, test.description) + assert.Equal(t, test.wantNumCerts, len(certs.Items), test.description) + } +} + +func TestUpdateFunc(t *testing.T) { + t.Parallel() + + q := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()) + + event1 := event.UpdateEvent{ + ObjectNew: &v1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cert", + Namespace: "routing", + }, + }, + } + + event2 := event.UpdateEvent{ + ObjectNew: &v1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cert-2", + Namespace: "routing", + }, + }, + } + + updateFunc(event1, q) + updateFunc(event2, q) + + assert.Equal(t, 2, q.Len()) + + req, _ := q.Get() + assert.Equal(t, reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "routing", Name: "test-cert"}}, req) + req, _ = q.Get() + assert.Equal(t, reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "routing", Name: "test-cert-2"}}, req) +} + +func TestIsCertificateInGatewaySpec(t *testing.T) { + t.Parallel() + + gateway := &networkingv1beta1.Gateway{ + Spec: v1beta1.Gateway{ + Servers: []*v1beta1.Server{ + { + Tls: &v1beta1.ServerTLSSettings{ + CredentialName: "some-other-cred", + }, + }, + { + Tls: &v1beta1.ServerTLSSettings{ + CredentialName: "devops-gateway-123-https", + }, + }, + }, + }, + } + + tests := []struct { + description string + certificate string + gateway *networkingv1beta1.Gateway + want bool + }{ + { + description: "Certificate exists in Gateway spec", + certificate: "devops-gateway-123-https", + gateway: gateway, + want: true, + }, + { + description: "Certificate does not exist in Gateway spec", + certificate: "no-match", + gateway: gateway, + want: false, + }, + } + + for _, test := range tests { + assert.Equal(t, test.want, isCertificateInGatewaySpec(test.certificate, test.gateway), test.description) + } +} diff --git a/internal/controllers/v1beta1/garbagecollection/options.go b/internal/controllers/v1beta1/garbagecollection/options.go new file mode 100644 index 0000000..e22780c --- /dev/null +++ b/internal/controllers/v1beta1/garbagecollection/options.go @@ -0,0 +1,9 @@ +package garbagecollection + +type OptionsFunc func(*GarbageCollectionController) + +func WithDryRun(dryrun bool) OptionsFunc { + return func(gcc *GarbageCollectionController) { + gcc.dryRun = dryrun + } +} diff --git a/internal/controllers/v1beta1/gateway.go b/internal/controllers/v1beta1/gateway.go index 095e3d0..efd98a5 100644 --- a/internal/controllers/v1beta1/gateway.go +++ b/internal/controllers/v1beta1/gateway.go @@ -9,7 +9,6 @@ import ( "time" v1beta1labels "github.com/kanopy-platform/gateway-certificate-controller/pkg/v1beta1/labels" - "github.com/kanopy-platform/gateway-certificate-controller/pkg/v1beta1/version" certmanagerclient "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned" "istio.io/api/networking/v1beta1" @@ -34,11 +33,6 @@ const ( FieldManager = "isto-cert-controller" ) -var ( - IssuerAnnotation = fmt.Sprintf("%s/%s", version.String(), "istio-cert-controller-issuer") - ManagedLabel = fmt.Sprintf("%s/%s", version.String(), "istio-cert-controller-managed") -) - type certificateHandler interface { CreateCertificate(ctx context.Context, gateway *networkingv1beta1.Gateway, server *v1beta1.Server) error UpdateCertificate(ctx context.Context, cert *v1certmanager.Certificate, gateway *networkingv1beta1.Gateway, server *v1beta1.Server) error @@ -144,7 +138,7 @@ func (c *GatewayController) CreateCertificate(ctx context.Context, gateway *netw log := log.FromContext(ctx) issuer := c.clusterIssuer - if i, ok := gateway.Annotations[IssuerAnnotation]; ok { + if i, ok := gateway.Annotations[v1beta1labels.IssuerAnnotation]; ok { issuer = i } @@ -159,7 +153,7 @@ func (c *GatewayController) CreateCertificate(ctx context.Context, gateway *netw }, ObjectMeta: metav1.ObjectMeta{ Name: server.Tls.CredentialName, - Labels: map[string]string{ManagedLabel: fmt.Sprintf("%s.%s", gateway.Name, gateway.Namespace)}, + Labels: map[string]string{v1beta1labels.ManagedLabel: fmt.Sprintf("%s.%s", gateway.Name, gateway.Namespace)}, }, Spec: v1certmanager.CertificateSpec{ DNSNames: getSortedHostsWithoutNamespace(server.Hosts), @@ -219,7 +213,7 @@ func updateCertificateIssuer(ctx context.Context, cert *v1certmanager.Certificat log := log.FromContext(ctx) issuer := cert.Spec.IssuerRef.Name - if i, ok := gateway.Annotations[IssuerAnnotation]; ok { + if i, ok := gateway.Annotations[v1beta1labels.IssuerAnnotation]; ok { log.V(1).Info("got issuer from annotation", "issuer", i) issuer = i } diff --git a/internal/controllers/v1beta1/gateway_test.go b/internal/controllers/v1beta1/gateway_test.go index cb8d037..f4f0fa5 100644 --- a/internal/controllers/v1beta1/gateway_test.go +++ b/internal/controllers/v1beta1/gateway_test.go @@ -12,6 +12,7 @@ import ( certmanagerfake "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/fake" certmanagerv1fake "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/typed/certmanager/v1/fake" + v1beta1labels "github.com/kanopy-platform/gateway-certificate-controller/pkg/v1beta1/labels" networkingv1beta1 "istio.io/api/networking/v1beta1" "istio.io/client-go/pkg/apis/networking/v1beta1" istiofake "istio.io/client-go/pkg/clientset/versioned/fake" @@ -303,7 +304,7 @@ func TestGatewayReconcile_CreateCertificateLabeledAsManaged(t *testing.T) { cert, err := helper.CertClient.CertmanagerV1().Certificates(TestCertNamespace).Get(context.TODO(), TestCertificateName, metav1.GetOptions{}) assert.NoError(t, err) assert.NotNil(t, cert) - assert.Equal(t, fmt.Sprintf("%s.%s", TestGatewayName, TestNamespace), cert.Labels[ManagedLabel]) + assert.Equal(t, fmt.Sprintf("%s.%s", TestGatewayName, TestNamespace), cert.Labels[v1beta1labels.ManagedLabel]) } func TestGatewayReconcile_CreateCertificateWithHosts(t *testing.T) { @@ -328,7 +329,7 @@ func TestGatewayReconcile_CreatCertificateWithDefaultIssuer(t *testing.T) { func TestGatewayReconcile_CreatCertificateWithClusterIssuerFromGatewayAnnotation(t *testing.T) { t.Parallel() helper := NewTestHelperWithGateways(WithAnnotations(map[string]string{ - IssuerAnnotation: "testissuer", + v1beta1labels.IssuerAnnotation: "testissuer", })) assertCreateCertificateCalled(t, helper) cert, err := helper.CertClient.CertmanagerV1().Certificates(TestCertNamespace).Get(context.TODO(), TestCertificateName, metav1.GetOptions{}) @@ -380,7 +381,7 @@ func TestGatewayReconcile_UpdatesCertificateWithDeletedHost(t *testing.T) { func TestGatewayReconcile_UpdatesCertificateWithNewIssuer(t *testing.T) { t.Parallel() helper := NewTestHelperWithCertificates(WithAnnotations(map[string]string{ - IssuerAnnotation: "new", + v1beta1labels.IssuerAnnotation: "new", })) assertCertificateUpdated(t, helper) cert, err := helper.CertClient.CertmanagerV1().Certificates(TestCertNamespace).Get(context.TODO(), TestCertificateName, metav1.GetOptions{}) @@ -405,7 +406,7 @@ func TestGatewayReconcile_UpdateCertificateNoOp(t *testing.T) { func TestGatewayReconcile_UpdatesCertificateDryRunWillNotCommit(t *testing.T) { t.Parallel() helper := NewTestHelperWithCertificates(WithTestDryRun(), WithAnnotations(map[string]string{ - IssuerAnnotation: "achange", + v1beta1labels.IssuerAnnotation: "achange", })) updated := 0 diff --git a/pkg/v1beta1/labels/labels.go b/pkg/v1beta1/labels/labels.go index 9281074..d3c2519 100644 --- a/pkg/v1beta1/labels/labels.go +++ b/pkg/v1beta1/labels/labels.go @@ -2,15 +2,38 @@ package labels import ( "fmt" + "strings" "github.com/kanopy-platform/gateway-certificate-controller/pkg/v1beta1/version" apilabels "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" ) type Label string -const InjectSimpleCredentialNameLabel Label = "istio-cert-controller-inject-simple-credential-name" +var ( + InjectSimpleCredentialNameLabel = fmt.Sprintf("%s/%s", version.String(), "istio-cert-controller-inject-simple-credential-name") + IssuerAnnotation = fmt.Sprintf("%s/%s", version.String(), "istio-cert-controller-issuer") + ManagedLabel = fmt.Sprintf("%s/%s", version.String(), "istio-cert-controller-managed") +) func InjectSimpleCredentialNameLabelSelector() string { - return apilabels.Set(map[string]string{fmt.Sprintf("%s/%s", version.String(), string(InjectSimpleCredentialNameLabel)): "true"}).AsSelector().String() + return apilabels.Set(map[string]string{InjectSimpleCredentialNameLabel: "true"}).AsSelector().String() +} + +func ManagedLabelSelector() string { + managedReq, err := apilabels.NewRequirement(ManagedLabel, selection.Exists, []string{}) + utilruntime.Must(err) + + return apilabels.NewSelector().Add(*managedReq).String() +} + +func ParseManagedLabel(in string) (gateway string, namespace string) { + s := strings.Split(in, ".") + if len(s) < 2 { + return "", "" + } + + return s[0], s[1] } diff --git a/pkg/v1beta1/labels/labels_test.go b/pkg/v1beta1/labels/labels_test.go index b576f50..4e469ff 100644 --- a/pkg/v1beta1/labels/labels_test.go +++ b/pkg/v1beta1/labels/labels_test.go @@ -6,7 +6,64 @@ import ( "github.com/stretchr/testify/assert" ) -func TestBuildLabelSelector(t *testing.T) { +func TestInjectSimpleCredentialNameLabel(t *testing.T) { + t.Parallel() + assert.Equal(t, "v1beta1.kanopy-platform.github.io/istio-cert-controller-inject-simple-credential-name", InjectSimpleCredentialNameLabel) +} + +func TestInjectSimpleCredentialNameLabelSelector(t *testing.T) { t.Parallel() assert.Equal(t, "v1beta1.kanopy-platform.github.io/istio-cert-controller-inject-simple-credential-name=true", InjectSimpleCredentialNameLabelSelector()) } + +func TestIssuerAnnotation(t *testing.T) { + t.Parallel() + assert.Equal(t, "v1beta1.kanopy-platform.github.io/istio-cert-controller-issuer", IssuerAnnotation) +} + +func TestManagedLabel(t *testing.T) { + t.Parallel() + assert.Equal(t, "v1beta1.kanopy-platform.github.io/istio-cert-controller-managed", ManagedLabel) +} + +func TestManagedLabelSelector(t *testing.T) { + t.Parallel() + assert.Equal(t, "v1beta1.kanopy-platform.github.io/istio-cert-controller-managed", ManagedLabelSelector()) +} + +func TestParseManagedLabel(t *testing.T) { + t.Parallel() + + tests := []struct { + input string + wantGateway string + wantNamespace string + }{ + { + input: "", + wantGateway: "", + wantNamespace: "", + }, + { + input: ".", + wantGateway: "", + wantNamespace: "", + }, + { + input: ".test-ns", + wantGateway: "", + wantNamespace: "test-ns", + }, + { + input: "test-gateway.test-ns", + wantGateway: "test-gateway", + wantNamespace: "test-ns", + }, + } + + for _, test := range tests { + g, ns := ParseManagedLabel(test.input) + assert.Equal(t, test.wantGateway, g, test.input) + assert.Equal(t, test.wantNamespace, ns, test.input) + } +}