KUBESAW-16: Label the ToolchainCluster token secrets (#396)

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,11 +81,43 @@ 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
 	}
 
 	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
+}

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() {

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

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=