Garbage Collection Controller (#3)

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)

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
+}

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)
+	}
+}