Dedup secret handling

Makefile

@@ -175,19 +175,19 @@ $(LOCALBIN):
 
 ## Tool Binaries
 KUBECTL ?= kubectl
-KUSTOMIZE ?= $(LOCALBIN)/kustomize-$(KUSTOMIZE_VERSION)
-CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen-$(CONTROLLER_TOOLS_VERSION)
-ENVTEST ?= $(LOCALBIN)/setup-envtest-$(ENVTEST_VERSION)
-GOLANGCI_LINT = $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION)
-KIND = $(LOCALBIN)/kind-$(KIND_VERSION)
-GRPCURL = $(LOCALBIN)/grpcurl-$(GRPCURL_VERSION)
+KUSTOMIZE ?= $(LOCALBIN)/kustomize
+CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
+ENVTEST ?= $(LOCALBIN)/setup-envtest
+GOLANGCI_LINT = $(LOCALBIN)/golangci-lint
+KIND = $(LOCALBIN)/kind
+GRPCURL = $(LOCALBIN)/grpcurl
 
 ## Tool Versions
 KUSTOMIZE_VERSION ?= v5.4.1
 CONTROLLER_TOOLS_VERSION ?= v0.16.3
 ENVTEST_VERSION ?= release-0.18
 GOLANGCI_LINT_VERSION ?= v1.61.0
-KIND_VERSION ?= v0.23.0
+KIND_VERSION ?= v0.27.0
 GRPCURL_VERSION ?= v1.9.2
 
 .PHONY: kustomize
@@ -231,15 +231,17 @@ clean: $(KIND)
 
 
 # go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist
-# $1 - target path with name of binary (ideally with version)
+# $1 - target path with name of binary
 # $2 - package url which can be installed
 # $3 - specific version of package
 define go-install-tool
-@[ -f $(1) ] || { \
+@[ -f "$(1)-$(3)" ] || { \
 set -e; \
 package=$(2)@$(3) ;\
 echo "Downloading $${package}" ;\
+rm -f $(1) || true ;\
 GOBIN=$(LOCALBIN) go install $${package} ;\
-mv "$$(echo "$(1)" | sed "s/-$(3)$$//")" $(1) ;\
-}
+mv $(1) $(1)-$(3) ;\
+} ;\
+ln -sf $(1)-$(3) $(1)
 endef

hack/deploy_with_helm.sh

@@ -2,8 +2,8 @@
 set -eo pipefail
 SCRIPT_DIR="$(dirname "$(readlink -f "$0")")"
 
-KIND=${KIND:-bin/kind-v0.23.0}
-GRPCURL=${GRPCURL:-bin/grpcurl-v1.9.2}
+KIND=${KIND:-bin/kind}
+GRPCURL=${GRPCURL:-bin/grpcurl}
 IMG=${IMG:-quay.io/jumpstarter-dev/jumpstarter-controller:latest}
 INGRESS_ENABLED=${INGRESS_ENABLED:-false}
 

internal/controller/client_controller.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -73,65 +72,24 @@ func (r *ClientReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	return ctrl.Result{}, nil
 }
 
-func (r *ClientReconciler) clientSecretExists(
-	ctx context.Context,
-	client *jumpstarterdevv1alpha1.Client,
-) (bool, error) {
-	logger := log.FromContext(ctx)
-
-	if client.Status.Credential == nil {
-		return false, nil
-	}
-	// NOTE: this could deserve some level of optimization/caching in the future
-	secret := &corev1.Secret{}
-	err := r.Get(ctx, kclient.ObjectKey{
-		Namespace: client.Namespace,
-		Name:      client.Status.Credential.Name,
-	}, secret)
-	if err != nil {
-		return false, kclient.IgnoreNotFound(err)
-	}
-
-	token, ok := secret.Data["token"]
-	if !ok || r.Signer.UnsafeValidate(string(token)) != nil {
-		logger.Info("reconcileStatusCredential: the client secret is invalid", "client", client.Name)
-		return false, r.Delete(ctx, secret)
-	}
-
-	return true, nil
-}
-
 func (r *ClientReconciler) reconcileStatusCredential(
 	ctx context.Context,
 	client *jumpstarterdevv1alpha1.Client,
 ) error {
-	logger := log.FromContext(ctx)
-
-	exists, err := r.clientSecretExists(ctx, client)
+	secret, err := ensureSecret(ctx, kclient.ObjectKey{
+		Name:      client.Name + "-client",
+		Namespace: client.Namespace,
+	}, r.Client, r.Signer, client.Username(r.Signer.Prefix()))
 	if err != nil {
-		logger.Info("reconcileStatusCredential: the client secret's existence cannot be checked", "client", client.Name)
-		return err
+		return fmt.Errorf("reconcileStatusCredential: failed to prepare credential for client: %w", err)
 	}
-
-	if !exists {
-		if client.Status.Credential != nil {
-			// TODO: Send an alert notification to cluster
-			logger.Info("reconcileStatusCredential: the client secret has ceased to exist, will be recreated", "client", client.Name)
-		} else {
-			logger.Info("reconcileStatusCredential: creating credential for client")
-		}
-		secret, err := r.secretForClient(client)
-		if err != nil {
-			return fmt.Errorf("reconcileStatusCredential: failed to prepare credential for client: %w", err)
-		}
-		if err := r.Create(ctx, secret); err != nil {
-			return fmt.Errorf("reconcileStatusCredential: failed to create credential for client: %w", err)
-		}
-		client.Status.Credential = &corev1.LocalObjectReference{
-			Name: secret.Name,
-		}
+	// enable garbage collection on the created resource
+	if err := controllerutil.SetControllerReference(client, secret, r.Scheme); err != nil {
+		return fmt.Errorf("reconcileStatusCredential: error setting owner reference: %w", err)
+	}
+	client.Status.Credential = &corev1.LocalObjectReference{
+		Name: secret.Name,
 	}
-
 	return nil
 }
 
@@ -151,29 +109,6 @@ func (r *ClientReconciler) reconcileStatusEndpoint(
 	return nil
 }
 
-func (r *ClientReconciler) secretForClient(client *jumpstarterdevv1alpha1.Client) (*corev1.Secret, error) {
-	token, err := r.Signer.Token(client.Username(r.Signer.Prefix()))
-	if err != nil {
-		return nil, err
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      client.Name + "-client",
-			Namespace: client.Namespace,
-		},
-		Type: corev1.SecretTypeOpaque,
-		StringData: map[string]string{
-			"token": token,
-		},
-	}
-	// enable garbage collection on the created resource
-	if err := controllerutil.SetControllerReference(client, secret, r.Scheme); err != nil {
-		return nil, fmt.Errorf("secretForClient, error setting owner reference: %w", err)
-	}
-	return secret, nil
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *ClientReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).

internal/controller/client_controller_test.go

@@ -126,5 +126,44 @@ var _ = Describe("Identity Controller", func() {
 				Name:      resourceName + "-client",
 			}, secret)).To(Succeed())
 		})
+
+		It("should reconcile an invalid token secret", func() {
+			By("recreating the secret")
+			signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy", "dummy:")
+			Expect(err).NotTo(HaveOccurred())
+
+			controllerReconciler := &ClientReconciler{
+				Client: k8sClient,
+				Scheme: k8sClient.Scheme(),
+				Signer: signer,
+			}
+
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: typeNamespacedName,
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// modify the secret to something invalid
+			secret := &corev1.Secret{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{
+				Namespace: "default",
+				Name:      resourceName + "-client",
+			}, secret)).To(Succeed())
+			secret.Data[TokenKey] = []byte("invalid")
+			Expect(k8sClient.Update(ctx, secret)).To(Succeed())
+
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: typeNamespacedName,
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("verifying the secret was updated")
+			secret = &corev1.Secret{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{
+				Namespace: "default",
+				Name:      resourceName + "-client",
+			}, secret)).To(Succeed())
+			Expect(secret.Data[TokenKey]).NotTo(Equal([]byte("invalid")))
+		})
 	})
 })

internal/controller/exporter_controller.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -85,64 +84,23 @@ func (r *ExporterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	return ctrl.Result{}, nil
 }
 
-func (r *ExporterReconciler) exporterSecretExists(
-	ctx context.Context,
-	exporter *jumpstarterdevv1alpha1.Exporter,
-) (bool, error) {
-	logger := log.FromContext(ctx)
-
-	if exporter.Status.Credential == nil {
-		return false, nil
-	}
-	// NOTE: this could deserve some level of optimization/caching in the future
-	secret := &corev1.Secret{}
-	err := r.Get(ctx, client.ObjectKey{
-		Namespace: exporter.Namespace,
-		Name:      exporter.Status.Credential.Name,
-	}, secret)
-	if err != nil {
-		return false, client.IgnoreNotFound(err)
-	}
-
-	token, ok := secret.Data["token"]
-
-	if !ok || r.Signer.UnsafeValidate(string(token)) != nil {
-		logger.Info("reconcileStatusCredential: the exporter secret is invalid", "exporter", exporter.Name)
-		return false, r.Delete(ctx, secret)
-	}
-
-	return true, nil
-}
-
 func (r *ExporterReconciler) reconcileStatusCredential(
 	ctx context.Context,
 	exporter *jumpstarterdevv1alpha1.Exporter,
 ) error {
-	logger := log.FromContext(ctx)
-
-	exists, err := r.exporterSecretExists(ctx, exporter)
+	secret, err := ensureSecret(ctx, client.ObjectKey{
+		Name:      exporter.Name + "-exporter",
+		Namespace: exporter.Namespace,
+	}, r.Client, r.Signer, exporter.Username(r.Signer.Prefix()))
 	if err != nil {
-		logger.Info("reconcileStatusCredential: the exporter secret's existence cannot be checked", "exporter", exporter.Name)
-		return err
+		return fmt.Errorf("reconcileStatusCredential: failed to prepare credential for exporter: %w", err)
 	}
-
-	if !exists {
-		if exporter.Status.Credential != nil {
-			// TODO: Send an alert notification to cluster
-			logger.Info("reconcileStatusCredential: the exporter secret has ceased to exist, will be recreated", "exporter", exporter.Name)
-		} else {
-			logger.Info("reconcileStatusCredential: creating credential for exporter")
-		}
-		secret, err := r.secretForExporter(exporter)
-		if err != nil {
-			return fmt.Errorf("reconcileStatusCredential: failed to prepare credential for exporter: %w", err)
-		}
-		if err := r.Create(ctx, secret); err != nil {
-			return fmt.Errorf("reconcileStatusCredential: failed to create credential for exporter: %w", err)
-		}
-		exporter.Status.Credential = &corev1.LocalObjectReference{
-			Name: secret.Name,
-		}
+	// enable garbage collection on the created resource
+	if err := controllerutil.SetControllerReference(exporter, secret, r.Scheme); err != nil {
+		return fmt.Errorf("reconcileStatusCredential: error setting owner reference: %w", err)
+	}
+	exporter.Status.Credential = &corev1.LocalObjectReference{
+		Name: secret.Name,
 	}
 	return nil
 }
@@ -191,29 +149,6 @@ func (r *ExporterReconciler) reconcileStatusEndpoint(
 	return nil
 }
 
-func (r *ExporterReconciler) secretForExporter(exporter *jumpstarterdevv1alpha1.Exporter) (*corev1.Secret, error) {
-	token, err := r.Signer.Token(exporter.Username(r.Signer.Prefix()))
-	if err != nil {
-		return nil, err
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      exporter.Name + "-exporter",
-			Namespace: exporter.Namespace,
-		},
-		Type: corev1.SecretTypeOpaque,
-		StringData: map[string]string{
-			"token": token,
-		},
-	}
-	// enable garbage collection on the created resource
-	if err := controllerutil.SetControllerReference(exporter, secret, r.Scheme); err != nil {
-		return nil, fmt.Errorf("secretForExporter, error setting owner reference: %w", err)
-	}
-	return secret, nil
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *ExporterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).