feat(controllers): implement finalizer

This fixes a TODO from 2020.

We now also rely on Kubernetes garbage collection of the referent
secrets as these should have proper owner references now.

cmd/jwker/main.go

@@ -34,6 +34,7 @@ func init() {
 	metrics.Registry.MustRegister(
 		jwkermetrics.JwkersTotal,
 		jwkermetrics.JwkersProcessedCount,
+		jwkermetrics.JwkersFinalizedCount,
 		jwkermetrics.JwkerSecretsTotal,
 		jwkermetrics.JwkersProcessingFailedCount,
 	)

controllers/jwker_controller.go

@@ -13,7 +13,6 @@ import (
 	"github.com/nais/liberator/pkg/events"
 	libernetes "github.com/nais/liberator/pkg/kubernetes"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
@@ -31,6 +30,7 @@ import (
 )
 
 const (
+	finalizer       = "jwker.nais.io/finalizer"
 	requeueInterval = 10 * time.Second
 )
 
@@ -53,27 +53,29 @@ func (r *JwkerReconciler) appClientID(req ctrl.Request) tokendings.ClientId {
 	}
 }
 
-// delete all associated objects
-// TODO: needs finalizer
-func (r *JwkerReconciler) purge(ctx context.Context, req ctrl.Request) error {
-	aid := r.appClientID(req)
-
-	r.logger.Info(fmt.Sprintf("Jwker resource %s in namespace: %s has been deleted. Cleaning up resources", req.Name, req.Namespace))
+// finalize purges relevant resources from external systems (i.e. the tokendings instances)
+func (r *JwkerReconciler) finalize(ctx context.Context, clientId tokendings.ClientId, jwker *jwkerv1.Jwker) error {
+	if !controllerutil.ContainsFinalizer(jwker, finalizer) {
+		return nil
+	}
 
-	r.logger.Info(fmt.Sprintf("Deleting resource %s in namespace %s from %d tokendings instances", req.Name, req.Namespace, len(r.Config.TokendingsInstances)))
 	for _, instance := range r.Config.TokendingsInstances {
-		r.logger.Info(fmt.Sprintf("Deleting client from tokendings instance %s", instance.BaseURL))
-		if err := instance.DeleteClient(ctx, aid); err != nil {
-			return fmt.Errorf("deleting resource from Tokendings instance '%s': %w", instance.BaseURL, err)
+		if err := instance.DeleteClient(ctx, clientId); err != nil {
+			return fmt.Errorf("deleting resource from tokendings at %q: %w", instance.BaseURL, err)
 		}
+		r.logger.Info(fmt.Sprintf("Finalizer: deleted client from tokendings at %q", instance.BaseURL))
 	}
 
-	r.logger.Info(fmt.Sprintf("Deleting application %s jwker secrets in namespace %s from cluster", req.Name, req.Namespace))
-	if err := secret.DeleteClusterSecrets(r.Client, ctx, aid, ""); err != nil {
-		return fmt.Errorf("deleting secrets from cluster: %s", err)
+	r.logger.Info(fmt.Sprintf("Finalizer: client deleted from %d tokendings instances", len(r.Config.TokendingsInstances)))
+
+	// finally (heh, get it?), remove finalizer from Jwker resource to allow for garbage collection
+	controllerutil.RemoveFinalizer(jwker, finalizer)
+	err := r.Client.Update(ctx, jwker)
+	if err != nil {
+		return fmt.Errorf("removing finalizer: %w", err)
 	}
-	jwkermetrics.JwkerSecretsTotal.Dec()
 
+	jwkermetrics.JwkersFinalizedCount.Inc()
 	return nil
 }
 
@@ -220,24 +222,26 @@ func (r *JwkerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		"correlation_id", correlationId,
 	)
 
-	// purge other systems if resource was deleted
 	err := r.Get(ctx, req.NamespacedName, &jwker)
-	switch {
-	case errors.IsNotFound(err):
-		err := r.purge(ctx, req)
-		if err == nil {
-			return ctrl.Result{}, nil
+	if err != nil {
+		// we ignore not found errors as cleanup should already be handled by the finalizer
+		return ctrl.Result{}, client.IgnoreNotFound(err)
+	}
+
+	// object is marked for deletion
+	if !jwker.ObjectMeta.DeletionTimestamp.IsZero() {
+		if err := r.finalize(ctx, r.appClientID(req), &jwker); err != nil {
+			r.reportError(err, "failed purge")
+			return ctrl.Result{}, fmt.Errorf("finalize: %w", err)
 		}
-		r.reportError(err, "failed purge")
-		return ctrl.Result{
-			RequeueAfter: requeueInterval,
-		}, err
+		return ctrl.Result{}, nil
+	}
 
-	case err != nil:
-		r.reportError(err, "unable to get jwker resource from cluster")
-		return ctrl.Result{
-			RequeueAfter: requeueInterval,
-		}, nil
+	if !controllerutil.ContainsFinalizer(&jwker, finalizer) {
+		controllerutil.AddFinalizer(&jwker, finalizer)
+		if err := r.Client.Update(ctx, &jwker); err != nil {
+			return ctrl.Result{}, fmt.Errorf("registering finalizer: %w", err)
+		}
 	}
 
 	hash, err = jwker.Spec.Hash()

controllers/suite_test.go

@@ -306,14 +306,14 @@ func TestReconciler(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, hash, jwker.Status.SynchronizationHash)
 	assert.Equal(t, events.RolloutComplete, jwker.Status.SynchronizationState)
+	assert.Equal(t, []string{
+		"jwker.nais.io/finalizer",
+	}, jwker.GetFinalizers())
 
 	// remove the jwker resource; usually done when naiserator syncs
 	err = cli.Delete(ctx, jwker)
 	assert.NoError(t, err)
-
-	// test that deleting the jwker resource purges associated secrets
-	assert.NoError(t, waitForDeletedSecret(ctx, cli, namespace, secretName))
-	assert.NoError(t, waitForDeletedSecret(ctx, cli, namespace, alreadyInUseSecret))
+	assert.NoError(t, waitForDeletedJwker(ctx, cli, namespace, jwker.Name))
 
 	cancel()
 	err = testEnv.Stop()
@@ -355,21 +355,21 @@ func getSecretWithTimeout(ctx context.Context, cli client.Client, namespace, nam
 	}
 }
 
-func waitForDeletedSecret(ctx context.Context, cli client.Client, namespace, name string) error {
+func waitForDeletedJwker(ctx context.Context, cli client.Client, namespace, name string) error {
 	key := client.ObjectKey{
 		Namespace: namespace,
 		Name:      name,
 	}
-	sec := &corev1.Secret{}
+	jwker := &naisiov1.Jwker{}
 	timeout := time.NewTimer(5 * time.Second)
 	ticker := time.NewTicker(250 * time.Millisecond)
 
 	for {
 		select {
 		case <-timeout.C:
-			return fmt.Errorf("secret still exists")
+			return fmt.Errorf("jwker still exists")
 		case <-ticker.C:
-			err := cli.Get(ctx, key, sec)
+			err := cli.Get(ctx, key, jwker)
 			if errors.IsNotFound(err) {
 				return nil
 			} else if err != nil {

pkg/metrics/jwkermetrics.go

@@ -13,7 +13,6 @@ import (
 )
 
 var (
-	// metrics
 	JwkersTotal = prometheus.NewGauge(
 		prometheus.GaugeOpts{
 			Name: "jwker_total",
@@ -24,6 +23,12 @@ var (
 			Help: "Number of jwkers processed",
 		},
 	)
+	JwkersFinalizedCount = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "jwker_finalized_count",
+			Help: "Number of jwkers finalized",
+		},
+	)
 	JwkerSecretsTotal = prometheus.NewGauge(
 		prometheus.GaugeOpts{
 			Name: "jwker_secrets_total",

pkg/secret/secrets.go

@@ -91,21 +91,6 @@ func CreateSecretSpec(secretName string, data PodSecretData) (*corev1.Secret, er
 	}, nil
 }
 
-func DeleteClusterSecrets(cli client.Client, ctx context.Context, app tokendings.ClientId, secretName string) error {
-	secretList, err := ClusterSecrets(ctx, app, cli)
-	if err != nil {
-		return err
-	}
-	for _, clusterSecret := range secretList.Items {
-		if clusterSecret.Name != secretName {
-			if err := cli.Delete(ctx, &clusterSecret); err != nil {
-				return fmt.Errorf("unable to delete clusterSecret: %w", err)
-			}
-		}
-	}
-	return nil
-}
-
 func ClusterSecrets(ctx context.Context, app tokendings.ClientId, cli client.Client) (corev1.SecretList, error) {
 	var secrets corev1.SecretList
 	mLabels := client.MatchingLabels{}