Initialize the cluster client cache from the kubeconfig in secrets

controllers/toolchaincluster/healthchecker.go

@@ -15,13 +15,11 @@ const (
 
 // getClusterHealth gets the kubernetes cluster health status by requesting "/healthz"
 func getClusterHealthStatus(ctx context.Context, remoteClusterClientset *kubeclientset.Clientset) (bool, error) {
-
 	lgr := log.FromContext(ctx)
 	body, err := remoteClusterClientset.DiscoveryClient.RESTClient().Get().AbsPath("/healthz").Do(ctx).Raw()
 	if err != nil {
 		lgr.Error(err, "Failed to do cluster health check for a ToolchainCluster")
 		return false, err
 	}
 	return strings.EqualFold(string(body), "ok"), nil
-
 }

controllers/toolchaincluster/toolchaincluster_controller.go

@@ -60,7 +60,7 @@
 	cachedCluster, ok := cluster.GetCachedToolchainCluster(toolchainCluster.Name)
 	if !ok {
 		err := fmt.Errorf("cluster %s not found in cache", toolchainCluster.Name)
-		if err := r.updateStatus(ctx, toolchainCluster, clusterOfflineCondition(err.Error())); err != nil {
+		if err := r.updateStatus(ctx, toolchainCluster, nil, clusterOfflineCondition(err.Error())); err != nil {
 			reqLogger.Error(err, "unable to update cluster status of ToolchainCluster")
 		}
 		return reconcile.Result{}, err
@@ -73,7 +73,7 @@
 	clientSet, err := kubeclientset.NewForConfig(cachedCluster.RestConfig)
 	if err != nil {
 		reqLogger.Error(err, "cannot create ClientSet for the ToolchainCluster")
-		if err := r.updateStatus(ctx, toolchainCluster, clusterOfflineCondition(err.Error())); err != nil {
+		if err := r.updateStatus(ctx, toolchainCluster, cachedCluster, clusterOfflineCondition(err.Error())); err != nil {
 			reqLogger.Error(err, "unable to update cluster status of ToolchainCluster")
 		}
 		return reconcile.Result{}, err
@@ -83,15 +83,24 @@
 	healthCheckResult := r.getClusterHealthCondition(ctx, clientSet)
 
 	// update the status of the individual cluster.
-	if err := r.updateStatus(ctx, toolchainCluster, healthCheckResult); err != nil {
+	if err := r.updateStatus(ctx, toolchainCluster, cachedCluster, healthCheckResult); 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) updateStatus(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster, currentConditions ...toolchainv1alpha1.Condition) error {
+func (r *Reconciler) updateStatus(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster, cachedToolchainCluster *cluster.CachedToolchainCluster, currentConditions ...toolchainv1alpha1.Condition) error {
 	toolchainCluster.Status.Conditions = condition.AddOrUpdateStatusConditionsWithLastUpdatedTimestamp(toolchainCluster.Status.Conditions, currentConditions...)
+
+	if cachedToolchainCluster != nil {
+		toolchainCluster.Status.APIEndpoint = cachedToolchainCluster.APIEndpoint
+		toolchainCluster.Status.OperatorNamespace = cachedToolchainCluster.OperatorNamespace
+	} else {
+		toolchainCluster.Status.APIEndpoint = ""
+		toolchainCluster.Status.OperatorNamespace = ""
+	}
+
 	if err := r.Client.Status().Update(ctx, toolchainCluster); err != nil {
 		return fmt.Errorf("failed to update the status of cluster - %s: %w", toolchainCluster.Name, err)
 	}
@@ -107,7 +116,6 @@
 		return clusterNotReadyCondition()
 	}
 	return clusterReadyCondition()
-
 }
 
 func (r *Reconciler) getClusterHealth(ctx context.Context, remoteClusterClientset *kubeclientset.Clientset) (bool, error) {

pkg/cluster/service.go

@@ -160,16 +160,9 @@
 
 // NewClusterConfig generate a new cluster config by fetching the necessary info the given ToolchainCluster's associated Secret and taking all data from ToolchainCluster CR
 func NewClusterConfig(cl client.Client, toolchainCluster *toolchainv1alpha1.ToolchainCluster, timeout time.Duration) (*Config, error) {
-	clusterName := toolchainCluster.Name
-
-	apiEndpoint := toolchainCluster.Spec.APIEndpoint
-	if apiEndpoint == "" {
-		return nil, errors.Errorf("the api endpoint of cluster %s is empty", clusterName)
-	}
-
 	secretName := toolchainCluster.Spec.SecretRef.Name
 	if secretName == "" {
-		return nil, errors.Errorf("cluster %s does not have a secret name", clusterName)
+		return nil, errors.Errorf("cluster %s does not have a secret name", toolchainCluster.Name)
 	}
 	secret := &v1.Secret{}
 	name := types.NamespacedName{
@@ -178,7 +171,22 @@
 	}
 	err := cl.Get(context.TODO(), name, secret)
 	if err != nil {
-		return nil, errors.Wrapf(err, "unable to get secret %s for cluster %s", name, clusterName)
+		return nil, errors.Wrapf(err, "unable to get secret %s for cluster %s", name, toolchainCluster.Name)
+	}
+
+	if _, ok := secret.Data["kubeconfig"]; ok {
+		return loadConfigFromKubeConfig(toolchainCluster, secret, timeout)
+	} else {
+		return loadConfigFromLegacyToolchainCluster(toolchainCluster, secret, timeout)
+	}
+}
+
+func loadConfigFromLegacyToolchainCluster(toolchainCluster *toolchainv1alpha1.ToolchainCluster, secret *v1.Secret, timeout time.Duration) (*Config, error) {
+	clusterName := toolchainCluster.Name
+
+	apiEndpoint := toolchainCluster.Spec.APIEndpoint
+	if apiEndpoint == "" {
+		return nil, errors.Errorf("the api endpoint of cluster %s is empty", clusterName)
 	}
 
 	token, tokenFound := secret.Data[toolchainTokenKey]
@@ -201,15 +209,44 @@
 	restConfig.Timeout = timeout
 
 	return &Config{
-		Name:              toolchainCluster.Name,
-		APIEndpoint:       toolchainCluster.Spec.APIEndpoint,
+		Name:              clusterName,
+		APIEndpoint:       apiEndpoint,
 		RestConfig:        restConfig,
 		OperatorNamespace: toolchainCluster.Labels[labelNamespace],
 		OwnerClusterName:  toolchainCluster.Labels[labelOwnerClusterName],
 		Labels:            toolchainCluster.Labels,
 	}, nil
 }
 
+func loadConfigFromKubeConfig(toolchainCluster *toolchainv1alpha1.ToolchainCluster, secret *v1.Secret, timeout time.Duration) (*Config, error) {
+	cfg, err := clientcmd.Load(secret.Data["kubeconfig"])
+	if err != nil {
+		return nil, err
+	}
+	clientCfg := clientcmd.NewDefaultClientConfig(*cfg, &clientcmd.ConfigOverrides{})
+	restCfg, err := clientCfg.ClientConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	// This is questionable, but the timeout is currently configurable in the member configuration so let's keep it here...
+	restCfg.Timeout = timeout
+
+	operatorNamespace, _, err := clientCfg.Namespace()
+	if err != nil {
+		return nil, fmt.Errorf("Could not determine the operator namespace from the current context in the provided kubeconfig because of: %w", err)
+	}
+
+	return &Config{
+		Name:              toolchainCluster.Name,
+		APIEndpoint:       restCfg.Host,
+		RestConfig:        restCfg,
+		OperatorNamespace: operatorNamespace,
+		OwnerClusterName:  toolchainCluster.Labels[labelOwnerClusterName],
+		Labels:            toolchainCluster.Labels,
+	}, nil
+}
+
 func IsReady(clusterStatus *toolchainv1alpha1.ToolchainClusterStatus) bool {
 	for _, condition := range clusterStatus.Conditions {
 		if condition.Type == toolchainv1alpha1.ConditionReady {

pkg/cluster/service_test.go

@@ -6,13 +6,18 @@ import (
 	"testing"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	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/codeready-toolchain/toolchain-common/pkg/test/verify"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/tools/clientcmd"
+	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -22,7 +27,6 @@ func TestAddToolchainClusterAsMember(t *testing.T) {
 		// when
 		return service.AddOrUpdateToolchainCluster(toolchainCluster)
 	})
-
 }
 
 func TestAddToolchainClusterAsHost(t *testing.T) {
@@ -132,7 +136,7 @@ func TestListToolchainClusterConfigs(t *testing.T) {
 	})
 
 	t.Run("when list fails", func(t *testing.T) {
-		//given
+		// given
 		cl := test.NewFakeClient(t, m1, m2, host, noise, sec1, sec2, secHost, secNoise)
 		cl.MockList = func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
 			return fmt.Errorf("some error")
@@ -147,7 +151,7 @@ func TestListToolchainClusterConfigs(t *testing.T) {
 	})
 
 	t.Run("when get secret fails", func(t *testing.T) {
-		//given
+		// given
 		cl := test.NewFakeClient(t, m1, m2, host, noise, sec1, sec2, secHost, secNoise)
 		cl.MockGet = func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
 			return fmt.Errorf("some error")
@@ -161,3 +165,128 @@ func TestListToolchainClusterConfigs(t *testing.T) {
 		require.Empty(t, clusterConfigs)
 	})
 }
+
+func TestNewClusterConfig(t *testing.T) {
+	legacyTc := func() *toolchainv1alpha1.ToolchainCluster {
+		return &toolchainv1alpha1.ToolchainCluster{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "tc",
+				Namespace: "ns",
+				Labels: map[string]string{
+					"namespace": "operatorns",
+				},
+			},
+			Spec: toolchainv1alpha1.ToolchainClusterSpec{
+				APIEndpoint: "https://over.the.rainbow",
+				SecretRef: toolchainv1alpha1.LocalSecretReference{
+					Name: "secret",
+				},
+			},
+		}
+	}
+	newFormTc := func() *toolchainv1alpha1.ToolchainCluster {
+		return &toolchainv1alpha1.ToolchainCluster{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "tc",
+				Namespace: "ns",
+			},
+			Spec: toolchainv1alpha1.ToolchainClusterSpec{
+				SecretRef: toolchainv1alpha1.LocalSecretReference{
+					Name: "secret",
+				},
+			},
+		}
+	}
+
+	legacySecret := func() *corev1.Secret {
+		return &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "secret",
+				Namespace: "ns",
+			},
+			Data: map[string][]byte{
+				"token": []byte("token"),
+			},
+		}
+	}
+	kubeconfigSecret := func(t *testing.T) *corev1.Secret {
+		t.Helper()
+		kubeconfig := clientcmdapi.Config{
+			Clusters: map[string]*clientcmdapi.Cluster{
+				"cluster": {
+					Server: "https://over.the.rainbow",
+				},
+			},
+			Contexts: map[string]*clientcmdapi.Context{
+				"ctx": {
+					Cluster:   "cluster",
+					AuthInfo:  "auth",
+					Namespace: "operatorns",
+				},
+			},
+			AuthInfos: map[string]*clientcmdapi.AuthInfo{
+				"auth": {
+					Token: "token",
+				},
+			},
+			CurrentContext: "ctx",
+		}
+		kubeconfigContents, err := clientcmd.Write(kubeconfig)
+		require.NoError(t, err)
+
+		return &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "secret",
+				Namespace: "ns",
+			},
+			Data: map[string][]byte{
+				"kubeconfig": kubeconfigContents,
+			},
+		}
+	}
+
+	t.Run("using legacy fields in ToolchainCluster and token in secret", func(t *testing.T) {
+		tc := legacyTc()
+		secret := legacySecret()
+
+		cl := test.NewFakeClient(t, tc, secret)
+
+		cfg, err := cluster.NewClusterConfig(cl, tc, 1*time.Second)
+		require.NoError(t, err)
+
+		assert.Equal(t, "https://over.the.rainbow", cfg.APIEndpoint)
+		assert.Equal(t, "operatorns", cfg.OperatorNamespace)
+		assert.Equal(t, "token", cfg.RestConfig.BearerToken)
+	})
+
+	t.Run("using kubeconfig in secret", func(t *testing.T) {
+		tc := newFormTc()
+		secret := kubeconfigSecret(t)
+
+		cl := test.NewFakeClient(t, tc, secret)
+
+		cfg, err := cluster.NewClusterConfig(cl, tc, 1*time.Second)
+		require.NoError(t, err)
+
+		assert.Equal(t, "https://over.the.rainbow", cfg.APIEndpoint)
+		assert.Equal(t, "operatorns", cfg.OperatorNamespace)
+		assert.Equal(t, "token", cfg.RestConfig.BearerToken)
+	})
+
+	t.Run("uses kubeconfig in precedence over legacy fields", func(t *testing.T) {
+		tc := newFormTc()
+		// Combine the kubeconfig and the token in the same secret.
+		// We should see auth from the kubeconfig used...
+		secret := kubeconfigSecret(t)
+		secret.Data["token"] = []byte("not-the-token-we-want")
+
+		cl := test.NewFakeClient(t, tc, secret)
+
+		cfg, err := cluster.NewClusterConfig(cl, tc, 1*time.Second)
+		require.NoError(t, err)
+
+		assert.Equal(t, "https://over.the.rainbow", cfg.APIEndpoint)
+		assert.Equal(t, "operatorns", cfg.OperatorNamespace)
+		assert.Equal(t, "token", cfg.RestConfig.BearerToken)
+	})
+}