From 1ceadb6ea36be9856e60e25e0d53ff2aa20288da Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Tue, 14 May 2024 12:17:49 +0200 Subject: [PATCH] KUBESAW-16: Label the ToolchainCluster token secrets (#396) --- .../toolchaincluster_controller.go | 43 +++++++++++++++- .../toolchaincluster_controller_test.go | 49 +++++++++++++++++-- go.mod | 2 +- go.sum | 4 +- 4 files changed, 91 insertions(+), 7 deletions(-) diff --git a/controllers/toolchaincluster/toolchaincluster_controller.go b/controllers/toolchaincluster/toolchaincluster_controller.go index fb564cc1..64d867a2 100644 --- a/controllers/toolchaincluster/toolchaincluster_controller.go +++ b/controllers/toolchaincluster/toolchaincluster_controller.go @@ -7,6 +7,7 @@ import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/toolchain-common/pkg/cluster" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" kubeclientset "k8s.io/client-go/kubernetes" @@ -51,6 +52,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. return reconcile.Result{}, err } + // this is a migration step to make sure that we are forwards-compatible with + // the secrets labeled by the toolchainCluster name, which are going to be the basis + // for *creating* toolchain clusters in the future. + if err := r.labelTokenSecret(ctx, toolchainCluster); err != nil { + reqLogger.Error(err, "unable to check the labels in the associated secret") + return reconcile.Result{}, err + } + cachedCluster, ok := cluster.GetCachedToolchainCluster(toolchainCluster.Name) if !ok { err := fmt.Errorf("cluster %s not found in cache", toolchainCluster.Name) @@ -72,7 +81,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. remoteClusterClientset: clientSet, logger: reqLogger, } - //update the status of the individual cluster. + // update the status of the individual cluster. if err := healthChecker.updateIndividualClusterStatus(ctx, toolchainCluster); err != nil { reqLogger.Error(err, "unable to update cluster status of ToolchainCluster") return reconcile.Result{}, err @@ -80,3 +89,35 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. return reconcile.Result{RequeueAfter: r.RequeAfter}, nil } + +func (r *Reconciler) labelTokenSecret(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster) error { + if toolchainCluster.Spec.SecretRef.Name == "" { + return nil + } + + secret := &corev1.Secret{} + if err := r.Client.Get(ctx, client.ObjectKey{Name: toolchainCluster.Spec.SecretRef.Name, Namespace: toolchainCluster.Namespace}, secret); err != nil { + if errors.IsNotFound(err) { + // The referenced secret does not exist yet, so we can't really label it. + // Because the reconciler runs periodically (not just on ToolchainCluster change), we will + // recover from this condition once the secret appears in the cluster. + log.FromContext(ctx).Info("failed to find the referenced secret. Cluster cache might be broken until it is created.", "expectedSecretName", toolchainCluster.Spec.SecretRef.Name) + return nil + } + return err + } + + if secret.Labels[toolchainv1alpha1.ToolchainClusterLabel] != toolchainCluster.Name { + if secret.Labels == nil { + secret.Labels = map[string]string{} + } + + secret.Labels[toolchainv1alpha1.ToolchainClusterLabel] = toolchainCluster.Name + + if err := r.Client.Update(ctx, secret); err != nil { + return err + } + } + + return nil +} diff --git a/controllers/toolchaincluster/toolchaincluster_controller_test.go b/controllers/toolchaincluster/toolchaincluster_controller_test.go index 22727789..9b64206e 100644 --- a/controllers/toolchaincluster/toolchaincluster_controller_test.go +++ b/controllers/toolchaincluster/toolchaincluster_controller_test.go @@ -9,6 +9,7 @@ import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/toolchain-common/pkg/cluster" "github.com/codeready-toolchain/toolchain-common/pkg/test" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/h2non/gock.v1" corev1 "k8s.io/api/core/v1" @@ -58,7 +59,6 @@ func TestClusterControllerChecks(t *testing.T) { // then require.Equal(t, err, nil) require.Equal(t, reconcile.Result{Requeue: false, RequeueAfter: 0}, recresult) - }) t.Run("Error while getting ToolchainCluster", func(t *testing.T) { @@ -82,7 +82,6 @@ func TestClusterControllerChecks(t *testing.T) { // then require.EqualError(t, err, "mock error") require.Equal(t, reconcile.Result{Requeue: false, RequeueAfter: 0}, recresult) - }) t.Run("reconcile successful and requeued", func(t *testing.T) { @@ -101,7 +100,6 @@ func TestClusterControllerChecks(t *testing.T) { require.Equal(t, err, nil) require.Equal(t, reconcile.Result{RequeueAfter: requeAfter}, recresult) assertClusterStatus(t, cl, "stable", healthy()) - }) t.Run("toolchain cluster cache not found", func(t *testing.T) { @@ -121,9 +119,54 @@ func TestClusterControllerChecks(t *testing.T) { err = cl.Client.Get(context.TODO(), types.NamespacedName{Name: "stable", Namespace: tcNs}, actualtoolchaincluster) require.NoError(t, err) assertClusterStatus(t, cl, "stable", offline()) + }) + t.Run("pre-existing secret is updated with the label linking it to the toolchaincluster resource", func(t *testing.T) { + // given + tc, secret := newToolchainCluster("tc", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{}) + + cl := test.NewFakeClient(t, tc, secret) + reset := setupCachedClusters(t, cl, tc) + defer reset() + controller, req := prepareReconcile(tc, cl, requeAfter) + + // just make sure that there is label on the secret yet... + require.Empty(t, secret.Labels[toolchainv1alpha1.ToolchainClusterLabel]) + + // when + _, err := controller.Reconcile(context.TODO(), req) + + // then + require.NoError(t, err) + linkedSecret := &corev1.Secret{} + err = cl.Client.Get(context.TODO(), types.NamespacedName{Name: tc.Spec.SecretRef.Name, Namespace: tcNs}, linkedSecret) + require.NoError(t, err) + assert.Equal(t, "tc", linkedSecret.Labels[toolchainv1alpha1.ToolchainClusterLabel]) }) + t.Run("secret labeling does not break on missing secret even though the missing secret breaks the tc cache", func(t *testing.T) { + // given + stable, secret := newToolchainCluster("stable", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{}) + + // we need the secret to be able to initialize the cluster cache + cl := test.NewFakeClient(t, stable, secret) + + controller, req := prepareReconcile(stable, cl, requeAfter) + // initialize the cluster cache at the point in time we still have the secret + reset := setupCachedClusters(t, cl, stable) + defer reset() + + // now enter the invalid state - delete the secret before the actual reconcile and check that we don't get an error. + // we don't care here that the cluster is essentially in an invalid state because all we test here is that the labeling + // doesn't introduce a new failure mode. + require.NoError(t, cl.Delete(context.TODO(), secret)) + + // when + _, err := controller.Reconcile(context.TODO(), req) + + // then + require.NoError(t, err) + }) } func setupCachedClusters(t *testing.T, cl *test.FakeClient, clusters ...*toolchainv1alpha1.ToolchainCluster) func() { diff --git a/go.mod b/go.mod index 865c995c..e89f4d91 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/codeready-toolchain/toolchain-common go 1.20 require ( - github.com/codeready-toolchain/api v0.0.0-20240502171347-8db815b922bd + github.com/codeready-toolchain/api v0.0.0-20240514085958-3b5237399fe5 github.com/go-logr/logr v1.2.3 github.com/golang-jwt/jwt/v5 v5.2.0 github.com/lestrrat-go/jwx v1.2.29 diff --git a/go.sum b/go.sum index c3bc4148..40375772 100644 --- a/go.sum +++ b/go.sum @@ -115,8 +115,8 @@ github.com/cncf/xds/go v0.0.0-20210312221358-fbca930ec8ed/go.mod h1:eXthEFrGJvWH github.com/cockroachdb/datadriven v0.0.0-20200714090401-bf6692d28da5/go.mod h1:h6jFvWxBdQXxjopDMZyH2UVceIRfR84bdzbkoKrsWNo= github.com/cockroachdb/errors v1.2.4/go.mod h1:rQD95gz6FARkaKkQXUksEje/d9a6wBJoCr5oaCLELYA= github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI= -github.com/codeready-toolchain/api v0.0.0-20240502171347-8db815b922bd h1:znIdWMiUgIJ/ypSQ17NemR+29V688DUjH6xrG3eBEMo= -github.com/codeready-toolchain/api v0.0.0-20240502171347-8db815b922bd/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E= +github.com/codeready-toolchain/api v0.0.0-20240514085958-3b5237399fe5 h1:lfW7VPmj70Tt75VzgOfAdVEGKMCR5/zACugWl1BDw34= +github.com/codeready-toolchain/api v0.0.0-20240514085958-3b5237399fe5/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=