Repository: kinvolk/lokomotive
Pull request #628 (2020-06-15 15:26:53)

pkg/k8sutil: change NewClientset to load kubeconfig from file content, not from path
As a first step towards resolving #608, this PR changes `NewClientset` to create Kubernetes Clientset from `kubeconfig` file **content**, instead of from file **path**. The clients are changes to manually load the file content, which should simplify changing it to also accept the file content as an argument in the upcoming patches.

invidian: Rebased to resolve conflicts.
invidian: I applied your suggestions, thanks for reviewing @iaguis :pray: 

Patch:
diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go
index 0a75d155..f410cdb1 100644
--- a/cli/cmd/cluster-apply.go
+++ b/cli/cmd/cluster-apply.go
@@ -16,6 +16,7 @@ package cmd
 
 import (
 	"fmt"
+	"io/ioutil"
 
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
@@ -125,12 +126,17 @@ func runClusterApply(cmd *cobra.Command, args []string) {
 }
 
 func verifyCluster(kubeconfigPath string, expectedNodes int) error {
-	client, err := k8sutil.NewClientset(kubeconfigPath)
+	kubeconfig, err := ioutil.ReadFile(kubeconfigPath) // #nosec G304
+	if err != nil {
+		return errors.Wrapf(err, "failed to read kubeconfig file")
+	}
+
+	cs, err := k8sutil.NewClientset(kubeconfig)
 	if err != nil {
 		return errors.Wrapf(err, "failed to set up clientset")
 	}
 
-	cluster, err := lokomotive.NewCluster(client, expectedNodes)
+	cluster, err := lokomotive.NewCluster(cs, expectedNodes)
 	if err != nil {
 		return errors.Wrapf(err, "failed to set up cluster client")
 	}
diff --git a/cli/cmd/component-delete.go b/cli/cmd/component-delete.go
index 6fccc1cf..f8835b15 100644
--- a/cli/cmd/component-delete.go
+++ b/cli/cmd/component-delete.go
@@ -17,6 +17,7 @@ package cmd
 import (
 	"context"
 	"fmt"
+	"io/ioutil"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -167,7 +168,12 @@ func deleteHelmRelease(c components.Component, kubeconfig string, deleteNSBool b
 }
 
 func deleteNS(ns string, kubeconfig string) error {
-	cs, err := k8sutil.NewClientset(kubeconfig)
+	kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304
+	if err != nil {
+		return fmt.Errorf("failed to read kubeconfig file: %v", err)
+	}
+
+	cs, err := k8sutil.NewClientset(kubeconfigContent)
 	if err != nil {
 		return err
 	}
diff --git a/cli/cmd/health.go b/cli/cmd/health.go
index 898215e4..913e5b9c 100644
--- a/cli/cmd/health.go
+++ b/cli/cmd/health.go
@@ -16,6 +16,7 @@ package cmd
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"text/tabwriter"
 
@@ -47,7 +48,13 @@ func runHealth(cmd *cobra.Command, args []string) {
 	if err != nil {
 		contextLogger.Fatalf("Error in finding kubeconfig file: %s", err)
 	}
-	client, err := k8sutil.NewClientset(kubeconfig)
+
+	kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304
+	if err != nil {
+		contextLogger.Fatalf("Failed to read kubeconfig file: %v", err)
+	}
+
+	cs, err := k8sutil.NewClientset(kubeconfigContent)
 	if err != nil {
 		contextLogger.Fatalf("Error in creating setting up Kubernetes client: %q", err)
 	}
@@ -64,7 +71,7 @@ func runHealth(cmd *cobra.Command, args []string) {
 		contextLogger.Fatal("No cluster configured")
 	}
 
-	cluster, err := lokomotive.NewCluster(client, p.Meta().ExpectedNodes)
+	cluster, err := lokomotive.NewCluster(cs, p.Meta().ExpectedNodes)
 	if err != nil {
 		contextLogger.Fatalf("Error in creating new Lokomotive cluster: %q", err)
 	}
diff --git a/pkg/components/util/install.go b/pkg/components/util/install.go
index 95f69867..11cd93cc 100644
--- a/pkg/components/util/install.go
+++ b/pkg/components/util/install.go
@@ -17,6 +17,7 @@ package util
 import (
 	"context"
 	"fmt"
+	"io/ioutil"
 
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart"
@@ -31,7 +32,12 @@ import (
 )
 
 func ensureNamespaceExists(name string, kubeconfigPath string) error {
-	cs, err := k8sutil.NewClientset(kubeconfigPath)
+	kubeconfig, err := ioutil.ReadFile(kubeconfigPath) // #nosec G304
+	if err != nil {
+		return fmt.Errorf("reading kubeconfig file: %w", err)
+	}
+
+	cs, err := k8sutil.NewClientset(kubeconfig)
 	if err != nil {
 		return fmt.Errorf("creating clientset: %w", err)
 	}
diff --git a/pkg/k8sutil/client.go b/pkg/k8sutil/client.go
index a6dcffdc..4cd495e5 100644
--- a/pkg/k8sutil/client.go
+++ b/pkg/k8sutil/client.go
@@ -15,21 +15,25 @@
 package k8sutil
 
 import (
+	"fmt"
+
 	"k8s.io/client-go/kubernetes"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/oidc"
 	"k8s.io/client-go/tools/clientcmd"
 )
 
-func NewClientset(kubeconfigPath string) (*kubernetes.Clientset, error) {
-	c, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath)
+// NewClientset creates new Kubernetes Client set object from the contents
+// of the given kubeconfig file.
+func NewClientset(data []byte) (*kubernetes.Clientset, error) {
+	c, err := clientcmd.NewClientConfigFromBytes(data)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("creating client config failed: %w", err)
 	}
 
-	apiclientset, err := kubernetes.NewForConfig(c)
+	restConfig, err := c.ClientConfig()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("converting client config to rest client config failed: %w", err)
 	}
 
-	return apiclientset, nil
+	return kubernetes.NewForConfig(restConfig)
 }


Test patch:
diff --git a/pkg/k8sutil/client_test.go b/pkg/k8sutil/client_test.go
new file mode 100644
index 00000000..973006a4
--- /dev/null
+++ b/pkg/k8sutil/client_test.go
@@ -0,0 +1,54 @@
+// Copyright 2020 The Lokomotive Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package k8sutil_test
+
+import (
+	"testing"
+
+	"github.com/kinvolk/lokomotive/pkg/k8sutil"
+)
+
+const (
+	validKubeconfig = `
+apiVersion: v1
+kind: Config
+clusters:
+- name: admin
+  cluster:
+    server: https://nonexistent:6443
+users:
+- name: admin
+  user:
+    token: "foo.bar"
+current-context: admin
+contexts:
+- name: admin
+  context:
+    cluster: admin
+    user: admin
+`
+)
+
+func TestNewClientset(t *testing.T) {
+	if _, err := k8sutil.NewClientset([]byte(validKubeconfig)); err != nil {
+		t.Fatalf("Creating clientset from valid kubeconfig should succeed, got: %v", err)
+	}
+}
+
+func TestNewClientsetInvalidKubeconfig(t *testing.T) {
+	if _, err := k8sutil.NewClientset([]byte("foo")); err == nil {
+		t.Fatalf("creating clientset from invalid kubeconfig should fail")
+	}
+}