From 4564be537ec85c434f1396dd46ffc98e167b5223 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 15 Jun 2020 14:31:03 +0200 Subject: [PATCH 1/6] pkg/k8sutil: rename NewClientset to NewClientsetFromFile This commit renames NewClientset function into NewClientsetFromFile to better describe it's functionality, as function takes file path as an argument to build the clientset object. It also makes room for new NewClientset function, which will take content of kubeconfig file instead of file path, to make it decoupled from dependency on local filesystem. NewClientsetFromFile is only a temporary and should be removed once we switch back to use NewClientset function everywhere. This commit also adds some small documentation to NewClientsetFromFile function to avoid triggering the linter. Signed-off-by: Mateusz Gozdek --- cli/cmd/cluster-apply.go | 2 +- cli/cmd/component-delete.go | 2 +- cli/cmd/health.go | 3 ++- pkg/components/util/install.go | 2 +- pkg/k8sutil/client.go | 4 +++- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go index 0a75d1557..d72ededbf 100644 --- a/cli/cmd/cluster-apply.go +++ b/cli/cmd/cluster-apply.go @@ -125,7 +125,7 @@ func runClusterApply(cmd *cobra.Command, args []string) { } func verifyCluster(kubeconfigPath string, expectedNodes int) error { - client, err := k8sutil.NewClientset(kubeconfigPath) + client, err := k8sutil.NewClientsetFromFile(kubeconfigPath) if err != nil { return errors.Wrapf(err, "failed to set up clientset") } diff --git a/cli/cmd/component-delete.go b/cli/cmd/component-delete.go index 6fccc1cf3..96c4781a9 100644 --- a/cli/cmd/component-delete.go +++ b/cli/cmd/component-delete.go @@ -167,7 +167,7 @@ func deleteHelmRelease(c components.Component, kubeconfig string, deleteNSBool b } func deleteNS(ns string, kubeconfig string) error { - cs, err := k8sutil.NewClientset(kubeconfig) + cs, err := k8sutil.NewClientsetFromFile(kubeconfig) if err != nil { return err } diff --git a/cli/cmd/health.go b/cli/cmd/health.go index 898215e45..077d6b65d 100644 --- a/cli/cmd/health.go +++ b/cli/cmd/health.go @@ -47,7 +47,8 @@ func runHealth(cmd *cobra.Command, args []string) { if err != nil { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - client, err := k8sutil.NewClientset(kubeconfig) + + client, err := k8sutil.NewClientsetFromFile(kubeconfig) if err != nil { contextLogger.Fatalf("Error in creating setting up Kubernetes client: %q", err) } diff --git a/pkg/components/util/install.go b/pkg/components/util/install.go index 95f69867f..cd9d2afbc 100644 --- a/pkg/components/util/install.go +++ b/pkg/components/util/install.go @@ -31,7 +31,7 @@ import ( ) func ensureNamespaceExists(name string, kubeconfigPath string) error { - cs, err := k8sutil.NewClientset(kubeconfigPath) + cs, err := k8sutil.NewClientsetFromFile(kubeconfigPath) if err != nil { return fmt.Errorf("creating clientset: %w", err) } diff --git a/pkg/k8sutil/client.go b/pkg/k8sutil/client.go index a6dcffdc6..4f6fe1226 100644 --- a/pkg/k8sutil/client.go +++ b/pkg/k8sutil/client.go @@ -20,7 +20,9 @@ import ( "k8s.io/client-go/tools/clientcmd" ) -func NewClientset(kubeconfigPath string) (*kubernetes.Clientset, error) { +// NewClientsetFromFile creates a new Kubernetes Client set object from the given +// kubeconfig file path. +func NewClientsetFromFile(kubeconfigPath string) (*kubernetes.Clientset, error) { c, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath) if err != nil { return nil, err From c2652d6e156a6134815c2dbcfcaa15f295433f6e Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 15 Jun 2020 15:04:09 +0200 Subject: [PATCH 2/6] pkg/k8sutil: add new NewClientset function This commit adds NewClientset exported function, which allows creating Kubernetes Clientset from content of kubeconfig file, which allows to create it from sources different than file on local file system. This is required for being able to execute 'lokoctl health' without having kubeconfig file present in local assets directory, as it can be pulled from other places, e.g. Terraform state. See #608 for more details. Signed-off-by: Mateusz Gozdek --- pkg/k8sutil/client.go | 18 +++++++++++++ pkg/k8sutil/client_test.go | 54 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 pkg/k8sutil/client_test.go diff --git a/pkg/k8sutil/client.go b/pkg/k8sutil/client.go index 4f6fe1226..991b346d4 100644 --- a/pkg/k8sutil/client.go +++ b/pkg/k8sutil/client.go @@ -15,6 +15,8 @@ package k8sutil import ( + "fmt" + "k8s.io/client-go/kubernetes" _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" "k8s.io/client-go/tools/clientcmd" @@ -35,3 +37,19 @@ func NewClientsetFromFile(kubeconfigPath string) (*kubernetes.Clientset, error) return apiclientset, nil } + +// 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, fmt.Errorf("creating client config failed: %w", err) + } + + restConfig, err := c.ClientConfig() + if err != nil { + return nil, fmt.Errorf("converting client config to rest client config failed: %w", err) + } + + return kubernetes.NewForConfig(restConfig) +} diff --git a/pkg/k8sutil/client_test.go b/pkg/k8sutil/client_test.go new file mode 100644 index 000000000..973006a4b --- /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") + } +} From d2e160b471b8569262122f545fe07ff1109d98f0 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 15 Jun 2020 15:30:18 +0200 Subject: [PATCH 3/6] cli/cmd: consistently name Kubernetes clientset object In some places we use 'client' to refer to it, in other 'cs', we should be consistent in it. This commit fixes that. The consistency is particulary useful when you grep for code calling the function. Signed-off-by: Mateusz Gozdek --- cli/cmd/cluster-apply.go | 4 ++-- cli/cmd/health.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go index d72ededbf..cd1dbdb62 100644 --- a/cli/cmd/cluster-apply.go +++ b/cli/cmd/cluster-apply.go @@ -125,12 +125,12 @@ func runClusterApply(cmd *cobra.Command, args []string) { } func verifyCluster(kubeconfigPath string, expectedNodes int) error { - client, err := k8sutil.NewClientsetFromFile(kubeconfigPath) + cs, err := k8sutil.NewClientsetFromFile(kubeconfigPath) 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/health.go b/cli/cmd/health.go index 077d6b65d..f28fb48d6 100644 --- a/cli/cmd/health.go +++ b/cli/cmd/health.go @@ -48,7 +48,7 @@ func runHealth(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - client, err := k8sutil.NewClientsetFromFile(kubeconfig) + cs, err := k8sutil.NewClientsetFromFile(kubeconfig) if err != nil { contextLogger.Fatalf("Error in creating setting up Kubernetes client: %q", err) } @@ -65,7 +65,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) } From 6241dcf0a510a4d0337a9d37c5c079b666bc7dd6 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 15 Jun 2020 16:49:28 +0200 Subject: [PATCH 4/6] cli/cmd: use NewClientset to construct Kubernetes clientset NewClientsetFromFile is now deprecated, as we want to gradually switch to loading kubeconfig file from the file content rather than from the file path. This commit preserves existing functionality, but uses new function, which should simplify the change later for loading from the file content. The code duplication here is intentional, to put calls to NewClientset function in place. In next commits, functions parameter types will be changed and calls to iotuil.ReadFile will be removed. This allows to do more step-by-step transition to different data flow. Signed-off-by: Mateusz Gozdek --- cli/cmd/cluster-apply.go | 8 +++++++- cli/cmd/component-delete.go | 8 +++++++- cli/cmd/health.go | 8 +++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go index cd1dbdb62..f410cdb18 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,7 +126,12 @@ func runClusterApply(cmd *cobra.Command, args []string) { } func verifyCluster(kubeconfigPath string, expectedNodes int) error { - cs, err := k8sutil.NewClientsetFromFile(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") } diff --git a/cli/cmd/component-delete.go b/cli/cmd/component-delete.go index 96c4781a9..f8835b151 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.NewClientsetFromFile(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 f28fb48d6..913e5b9c2 100644 --- a/cli/cmd/health.go +++ b/cli/cmd/health.go @@ -16,6 +16,7 @@ package cmd import ( "fmt" + "io/ioutil" "os" "text/tabwriter" @@ -48,7 +49,12 @@ func runHealth(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - cs, err := k8sutil.NewClientsetFromFile(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) } From 09cdba10c4eee74799f223d152d715da7414c454 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Wed, 1 Jul 2020 09:59:42 +0200 Subject: [PATCH 5/6] pkg/components/util: use NewClientset to construct Kubernetes clientset NewClientsetFromFile is now deprecated, as we want to gradually switch to loading kubeconfig file from the file content rather than from the file path. This commit preserves existing functionality, but uses new function, which should simplify the change later for loading from the file content. Signed-off-by: Mateusz Gozdek --- pkg/components/util/install.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/components/util/install.go b/pkg/components/util/install.go index cd9d2afbc..11cd93cc8 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.NewClientsetFromFile(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) } From 90f3c1a011d45e54a66b4a1d5056829b8f6944fe Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Wed, 1 Jul 2020 13:22:44 +0200 Subject: [PATCH 6/6] pkg/k8sutil: remove unused NewClientsetFromFile function All uses of this function are now replaced by NewClientset function, so this function is unused and safe to remove. Signed-off-by: Mateusz Gozdek --- pkg/k8sutil/client.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/pkg/k8sutil/client.go b/pkg/k8sutil/client.go index 991b346d4..4cd495e50 100644 --- a/pkg/k8sutil/client.go +++ b/pkg/k8sutil/client.go @@ -22,22 +22,6 @@ import ( "k8s.io/client-go/tools/clientcmd" ) -// NewClientsetFromFile creates a new Kubernetes Client set object from the given -// kubeconfig file path. -func NewClientsetFromFile(kubeconfigPath string) (*kubernetes.Clientset, error) { - c, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath) - if err != nil { - return nil, err - } - - apiclientset, err := kubernetes.NewForConfig(c) - if err != nil { - return nil, err - } - - return apiclientset, nil -} - // NewClientset creates new Kubernetes Client set object from the contents // of the given kubeconfig file. func NewClientset(data []byte) (*kubernetes.Clientset, error) {