Remove the migration step to kubeconfig

controllers/toolchaincluster/healthchecker_test.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"testing"
 
-	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/require"
@@ -14,21 +13,20 @@ import (
 )
 
 func TestClusterHealthChecks(t *testing.T) {
-
 	// given
 	defer gock.Off()
 	tcNs := "test-namespace"
-	gock.New("http://cluster.com").
+	gock.New("https://cluster.com").
 		Get("healthz").
 		Persist().
 		Reply(200).
 		BodyString("ok")
-	gock.New("http://unstable.com").
+	gock.New("https://unstable.com").
 		Get("healthz").
 		Persist().
 		Reply(200).
 		BodyString("unstable")
-	gock.New("http://not-found.com").
+	gock.New("https://not-found.com").
 		Get("healthz").
 		Persist().
 		Reply(404)
@@ -41,25 +39,25 @@ func TestClusterHealthChecks(t *testing.T) {
 	}{
 		"HealthOkay": {
 			tcType:      "stable",
-			apiEndPoint: "http://cluster.com",
+			apiEndPoint: "https://cluster.com",
 			healthCheck: true,
 		},
 		"HealthNotOkayButNoError": {
 			tcType:      "unstable",
-			apiEndPoint: "http://unstable.com",
+			apiEndPoint: "https://unstable.com",
 			healthCheck: false,
 		},
 		"ErrorWhileDoingHealth": {
 			tcType:      "Notfound",
-			apiEndPoint: "http://not-found.com",
+			apiEndPoint: "https://not-found.com",
 			healthCheck: false,
 			err:         fmt.Errorf("the server could not find the requested resource"),
 		},
 	}
 	for k, tc := range tests {
 		t.Run(k, func(t *testing.T) {
-			//given
-			tcType, sec := newToolchainCluster(tc.tcType, tcNs, tc.apiEndPoint, toolchainv1alpha1.ToolchainClusterStatus{})
+			// given
+			tcType, sec := newToolchainCluster(t, tc.tcType, tcNs, tc.apiEndPoint)
 			cl := test.NewFakeClient(t, tcType, sec)
 			reset := setupCachedClusters(t, cl, tcType)
 			defer reset()
@@ -68,17 +66,16 @@ func TestClusterHealthChecks(t *testing.T) {
 			cacheClient, err := kubeclientset.NewForConfig(cachedTC.RestConfig)
 			require.NoError(t, err)
 
-			//when
+			// when
 			healthCheck, err := getClusterHealthStatus(context.TODO(), cacheClient)
 
-			//then
+			// then
 			require.Equal(t, tc.healthCheck, healthCheck)
 			if tc.err != nil {
 				require.EqualError(t, err, tc.err.Error())
 			} else {
 				require.NoError(t, err)
 			}
-
 		})
 	}
 }

controllers/toolchaincluster/toolchaincluster_controller.go

@@ -1,7 +1,6 @@
 package toolchaincluster
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"time"
@@ -13,7 +12,6 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	kubeclientset "k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -66,10 +64,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 		return reconcile.Result{}, err
 	}
 
-	if err = r.migrateSecretToKubeConfig(ctx, toolchainCluster); err != nil {
-		return reconcile.Result{}, err
-	}
-
 	clientSet, err := kubeclientset.NewForConfig(cachedCluster.RestConfig)
 	if err != nil {
 		reqLogger.Error(err, "cannot create ClientSet for the ToolchainCluster")
@@ -152,41 +146,6 @@ func clusterNotReadyCondition() toolchainv1alpha1.Condition {
 	}
 }
 
-func (r *Reconciler) migrateSecretToKubeConfig(ctx context.Context, tc *toolchainv1alpha1.ToolchainCluster) error {
-	if len(tc.Spec.SecretRef.Name) == 0 {
-		return nil
-	}
-
-	secret := &corev1.Secret{}
-	if err := r.Client.Get(ctx, client.ObjectKey{Name: tc.Spec.SecretRef.Name, Namespace: tc.Namespace}, secret); err != nil {
-		return err
-	}
-
-	token := secret.Data["token"]
-	apiEndpoint := tc.Spec.APIEndpoint
-	operatorNamespace := tc.Labels["namespace"]
-	insecureTls := len(tc.Spec.DisabledTLSValidations) == 1 && tc.Spec.DisabledTLSValidations[0] == "*"
-	// we ignore the Spec.CABundle here because we don't want it migrated. The new configurations are free
-	// to use the certificate data for the connections but we don't want to migrate the existing certificates.
-	kubeConfig := composeKubeConfigFromData(token, apiEndpoint, operatorNamespace, insecureTls)
-
-	data, err := clientcmd.Write(kubeConfig)
-	if err != nil {
-		return err
-	}
-
-	origKubeConfigData := secret.Data["kubeconfig"]
-	secret.Data["kubeconfig"] = data
-
-	if !bytes.Equal(origKubeConfigData, data) {
-		if err = r.Client.Update(ctx, secret); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 func composeKubeConfigFromData(token []byte, apiEndpoint, operatorNamespace string, insecureTls bool) clientcmdapi.Config {
 	return clientcmdapi.Config{
 		Contexts: map[string]*clientcmdapi.Context{

controllers/toolchaincluster/toolchaincluster_controller_test.go

@@ -9,17 +9,13 @@ 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/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/h2non/gock.v1"
-	"gotest.tools/assert/cmp"
 	corev1 "k8s.io/api/core/v1"
 	kubeclientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/clientcmd"
-	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -32,24 +28,24 @@ func TestClusterControllerChecks(t *testing.T) {
 
 	defer gock.Off()
 	tcNs := "test-namespace"
-	gock.New("http://cluster.com").
+	gock.New("https://cluster.com").
 		Get("healthz").
 		Persist().
 		Reply(200).
 		BodyString("ok")
-	gock.New("http://unstable.com").
+	gock.New("https://unstable.com").
 		Get("healthz").
 		Persist().
 		Reply(200).
 		BodyString("unstable")
-	gock.New("http://not-found.com").
+	gock.New("https://not-found.com").
 		Get("healthz").
 		Persist().
 		Reply(404)
 
 	t.Run("ToolchainCluster not found", func(t *testing.T) {
 		// given
-		NotFound, sec := newToolchainCluster("notfound", tcNs, "http://not-found.com", toolchainv1alpha1.ToolchainClusterStatus{})
+		NotFound, sec := newToolchainCluster(t, "notfound", tcNs, "http://not-found.com")
 
 		cl := test.NewFakeClient(t, sec)
 		reset := setupCachedClusters(t, cl, NotFound)
@@ -66,7 +62,7 @@ func TestClusterControllerChecks(t *testing.T) {
 
 	t.Run("Error while getting ToolchainCluster", func(t *testing.T) {
 		// given
-		tc, sec := newToolchainCluster("tc", tcNs, "http://tc.com", toolchainv1alpha1.ToolchainClusterStatus{})
+		tc, sec := newToolchainCluster(t, "tc", tcNs, "http://tc.com")
 
 		cl := test.NewFakeClient(t, sec)
 		cl.MockGet = func(ctx context.Context, key runtimeclient.ObjectKey, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
@@ -87,7 +83,7 @@ func TestClusterControllerChecks(t *testing.T) {
 
 	t.Run("reconcile successful and requeued", func(t *testing.T) {
 		// given
-		stable, sec := newToolchainCluster("stable", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})
+		stable, sec := newToolchainCluster(t, "stable", tcNs, "https://cluster.com")
 
 		cl := test.NewFakeClient(t, stable, sec)
 		reset := setupCachedClusters(t, cl, stable)
@@ -106,7 +102,7 @@ func TestClusterControllerChecks(t *testing.T) {
 
 	t.Run("toolchain cluster cache not found", func(t *testing.T) {
 		// given
-		unstable, _ := newToolchainCluster("unstable", tcNs, "http://unstable.com", toolchainv1alpha1.ToolchainClusterStatus{})
+		unstable, _ := newToolchainCluster(t, "unstable", tcNs, "http://unstable.com")
 
 		cl := test.NewFakeClient(t, unstable)
 		controller, req := prepareReconcile(unstable, cl, requeAfter)
@@ -121,7 +117,7 @@ func TestClusterControllerChecks(t *testing.T) {
 
 	t.Run("error while updating a toolchain cluster status on cache not found", func(t *testing.T) {
 		// given
-		stable, _ := newToolchainCluster("stable", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})
+		stable, _ := newToolchainCluster(t, "stable", tcNs, "https://cluster.com")
 
 		cl := test.NewFakeClient(t, stable)
 		cl.MockStatusUpdate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.UpdateOption) error {
@@ -141,7 +137,7 @@ func TestClusterControllerChecks(t *testing.T) {
 
 	t.Run("error while updating a toolchain cluster status when health-check failed", func(t *testing.T) {
 		// given
-		stable, sec := newToolchainCluster("stable", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})
+		stable, sec := newToolchainCluster(t, "stable", tcNs, "https://cluster.com")
 		expectedErr := fmt.Errorf("my test error")
 		cl := test.NewFakeClient(t, stable, sec)
 		cl.MockStatusUpdate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.UpdateOption) error {
@@ -162,46 +158,12 @@ func TestClusterControllerChecks(t *testing.T) {
 		require.Equal(t, reconcile.Result{}, recResult)
 		assertClusterStatus(t, cl, "stable")
 	})
-
-	t.Run("migrates connection settings to kubeconfig in secret", 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)
-		expectedKubeConfig := composeKubeConfigFromData([]byte("mycooltoken"), "http://cluster.com", "test-namespace", true)
-
-		// when
-		_, err := controller.Reconcile(context.TODO(), req)
-		secretAfterReconcile := &corev1.Secret{}
-		require.NoError(t, cl.Get(context.TODO(), runtimeclient.ObjectKeyFromObject(secret), secretAfterReconcile))
-		actualKubeConfig, loadErr := clientcmd.Load(secretAfterReconcile.Data["kubeconfig"])
-
-		// then
-		require.NoError(t, err)
-		require.NoError(t, loadErr)
-		assert.Contains(t, secretAfterReconcile.Data, "kubeconfig")
-
-		// we need to use this more complex equals, because we don't initialize the Extension fields (i.e. they're nil)
-		// while they're initialized and empty after deserialization, which causes the "normal" deep equals to fail.
-		result := cmp.DeepEqual(expectedKubeConfig, *actualKubeConfig,
-			cmpopts.IgnoreFields(clientcmdapi.Config{}, "Extensions"),
-			cmpopts.IgnoreFields(clientcmdapi.Preferences{}, "Extensions"),
-			cmpopts.IgnoreFields(clientcmdapi.Cluster{}, "Extensions"),
-			cmpopts.IgnoreFields(clientcmdapi.AuthInfo{}, "Extensions"),
-			cmpopts.IgnoreFields(clientcmdapi.Context{}, "Extensions"),
-		)()
-
-		assert.True(t, result.Success())
-	})
 }
 
 func TestGetClusterHealth(t *testing.T) {
 	t.Run("Check health default", func(t *testing.T) {
 		// given
-		stable, sec := newToolchainCluster("stable", "test-namespace", "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})
+		stable, sec := newToolchainCluster(t, "stable", "test-namespace", "https://cluster.com")
 
 		cl := test.NewFakeClient(t, stable, sec)
 		reset := setupCachedClusters(t, cl, stable)
@@ -222,7 +184,7 @@ func TestGetClusterHealth(t *testing.T) {
 	})
 	t.Run("get health condition when health obtained is false ", func(t *testing.T) {
 		// given
-		stable, sec := newToolchainCluster("stable", "test-namespace", "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})
+		stable, sec := newToolchainCluster(t, "stable", "test-namespace", "https://cluster.com")
 
 		cl := test.NewFakeClient(t, stable, sec)
 		reset := setupCachedClusters(t, cl, stable)
@@ -242,6 +204,7 @@ func TestGetClusterHealth(t *testing.T) {
 		assertClusterStatus(t, cl, "stable", clusterNotReadyCondition())
 	})
 }
+
 func TestComposeKubeConfig(t *testing.T) {
 	// when
 	kubeConfig := composeKubeConfigFromData([]byte("token"), "http://over.the.rainbow", "the-namespace", false)
@@ -275,8 +238,8 @@ func setupCachedClusters(t *testing.T, cl *test.FakeClient, clusters ...*toolcha
 	}
 }
 
-func newToolchainCluster(name, tcNs string, apiEndpoint string, status toolchainv1alpha1.ToolchainClusterStatus) (*toolchainv1alpha1.ToolchainCluster, *corev1.Secret) {
-	toolchainCluster, secret := test.NewToolchainClusterWithEndpoint(name, tcNs, "secret", apiEndpoint, status, map[string]string{"namespace": "test-namespace"})
+func newToolchainCluster(t *testing.T, name, tcNs string, apiEndpoint string) (*toolchainv1alpha1.ToolchainCluster, *corev1.Secret) {
+	toolchainCluster, secret := test.NewToolchainClusterWithEndpoint(t, name, tcNs, "test-namespace", "secret", apiEndpoint, toolchainv1alpha1.ToolchainClusterStatus{}, false)
 	return toolchainCluster, secret
 }
 

pkg/cluster/service.go

@@ -25,9 +25,10 @@ const (
 	// labelClusterRolePrefix is the prefix that defines the cluster role as label key
 	labelClusterRolePrefix = "cluster-role"
 
-	toolchainAPIQPS   = 20.0
-	toolchainAPIBurst = 30
-	toolchainTokenKey = "token"
+	// These are not used
+	// toolchainAPIQPS   = 20.0
+	// toolchainAPIBurst = 30
+	// toolchainTokenKey = "token"
 )
 
 // ToolchainClusterService manages cached cluster kube clients and related ToolchainCluster CRDs
@@ -174,48 +175,7 @@ func NewClusterConfig(cl client.Client, toolchainCluster *toolchainv1alpha1.Tool
 		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]
-	if !tokenFound || len(token) == 0 {
-		return nil, errors.Errorf("the secret for cluster %s is missing a non-empty value for %q", clusterName, toolchainTokenKey)
-	}
-
-	restConfig, err := clientcmd.BuildConfigFromFlags(apiEndpoint, "")
-	if err != nil {
-		return nil, err
-	}
-
-	if len(toolchainCluster.Spec.DisabledTLSValidations) == 1 &&
-		toolchainCluster.Spec.DisabledTLSValidations[0] == toolchainv1alpha1.TLSAll {
-		restConfig.Insecure = true
-	}
-	restConfig.BearerToken = string(token)
-	restConfig.QPS = toolchainAPIQPS
-	restConfig.Burst = toolchainAPIBurst
-	restConfig.Timeout = timeout
-
-	return &Config{
-		Name:              clusterName,
-		APIEndpoint:       apiEndpoint,
-		RestConfig:        restConfig,
-		OperatorNamespace: toolchainCluster.Labels[labelNamespace],
-		OwnerClusterName:  toolchainCluster.Labels[labelOwnerClusterName],
-		Labels:            toolchainCluster.Labels,
-	}, nil
+	return loadConfigFromKubeConfig(toolchainCluster, secret, timeout)
 }
 
 func loadConfigFromKubeConfig(toolchainCluster *toolchainv1alpha1.ToolchainCluster, secret *v1.Secret, timeout time.Duration) (*Config, error) {