diff --git a/Makefile b/Makefile index cd353239..c25ec3ad 100644 --- a/Makefile +++ b/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 diff --git a/hack/deploy_with_helm.sh b/hack/deploy_with_helm.sh index f8b0a821..db54bc67 100755 --- a/hack/deploy_with_helm.sh +++ b/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} diff --git a/internal/controller/client_controller.go b/internal/controller/client_controller.go index 7b26cf30..47f987a7 100644 --- a/internal/controller/client_controller.go +++ b/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). diff --git a/internal/controller/client_controller_test.go b/internal/controller/client_controller_test.go index 4eda0569..21aac28d 100644 --- a/internal/controller/client_controller_test.go +++ b/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"))) + }) }) }) diff --git a/internal/controller/exporter_controller.go b/internal/controller/exporter_controller.go index d8d8a3a9..bed30a24 100644 --- a/internal/controller/exporter_controller.go +++ b/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). diff --git a/internal/controller/secret_helpers.go b/internal/controller/secret_helpers.go new file mode 100644 index 00000000..25050814 --- /dev/null +++ b/internal/controller/secret_helpers.go @@ -0,0 +1,76 @@ +package controller + +import ( + "context" + + "github.com/jumpstarter-dev/jumpstarter-controller/internal/oidc" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +const TokenKey string = "token" + +func ensureSecret( + ctx context.Context, + key client.ObjectKey, + kclient client.Client, + signer *oidc.Signer, + username string, +) (*corev1.Secret, error) { + logger := log.FromContext(ctx).WithName("ensureSecret") + var secret corev1.Secret + if err := kclient.Get(ctx, key, &secret); err != nil { + if !errors.IsNotFound(err) { + logger.Error(err, "failed to get secret") + return nil, err + } + // Secret not present + logger.Info("secret not present, creating") + token, err := signer.Token(username) + if err != nil { + logger.Error(err, "failed to sign token") + return nil, err + } + secret = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: key.Namespace, + Name: key.Name, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + TokenKey: []byte(token), + }, + } + if err = kclient.Create(ctx, &secret); err != nil { + logger.Error(err, "failed to create secret") + return nil, err + } + return &secret, nil + } else { + token, ok := secret.Data[TokenKey] + if !ok || signer.UnsafeValidate(string(token)) != nil { + // Secret present but invalid + logger.Info("secret present but invalid, updating") + original := client.MergeFrom(secret.DeepCopy()) + token, err := signer.Token(username) + if err != nil { + logger.Error(err, "failed to sign token") + return nil, err + } + secret.Data = map[string][]byte{ + TokenKey: []byte(token), + } + if err = kclient.Patch(ctx, &secret, original); err != nil { + logger.Error(err, "failed to update secret") + return nil, err + } + return &secret, nil + } else { + // Secret present and valid + return &secret, nil + } + } +}