From c67f23efd2af7837573d82f1c780dc08807c4796 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 09:42:29 +0200 Subject: [PATCH 1/9] pkg/k8sutil: add Getter helper struct Getter struct and NewGetter function is a replacement for helm.sh/helm/v3/pkg/kube.GetConfig(), which allows to create Helm action config from the content of kubeconfig file, instead of from the path of the file, which is part of #608. Signed-off-by: Mateusz Gozdek --- pkg/k8sutil/doc.go | 16 ++ pkg/k8sutil/getter.go | 80 ++++++ pkg/k8sutil/getter_test.go | 46 ++++ .../discovery/cached/memory/memcache.go | 243 ++++++++++++++++++ vendor/modules.txt | 1 + 5 files changed, 386 insertions(+) create mode 100644 pkg/k8sutil/doc.go create mode 100644 pkg/k8sutil/getter.go create mode 100644 pkg/k8sutil/getter_test.go create mode 100644 vendor/k8s.io/client-go/discovery/cached/memory/memcache.go diff --git a/pkg/k8sutil/doc.go b/pkg/k8sutil/doc.go new file mode 100644 index 000000000..73a6f4d5b --- /dev/null +++ b/pkg/k8sutil/doc.go @@ -0,0 +1,16 @@ +// 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 provides helper for interacting with Kubernetes API. +package k8sutil diff --git a/pkg/k8sutil/getter.go b/pkg/k8sutil/getter.go new file mode 100644 index 000000000..419f4b82d --- /dev/null +++ b/pkg/k8sutil/getter.go @@ -0,0 +1,80 @@ +// 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 + +import ( + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/discovery" + "k8s.io/client-go/discovery/cached/memory" + "k8s.io/client-go/rest" + "k8s.io/client-go/restmapper" + "k8s.io/client-go/tools/clientcmd" +) + +// Getter implements k8s.io/cli-runtime/pkg/genericclioptions.RESTClientGetter interface. +type Getter struct { + c clientcmd.ClientConfig +} + +// ToRESTMapper is part of k8s.io/cli-runtime/pkg/genericclioptions.RESTClientGetter interface. +func (c *Getter) ToRESTMapper() (meta.RESTMapper, error) { + d, err := c.ToDiscoveryClient() + if err != nil { + return nil, err + } + + mapper := restmapper.NewDeferredDiscoveryRESTMapper(d) + expander := restmapper.NewShortcutExpander(mapper, d) + + return expander, nil +} + +// ToDiscoveryClient is part of k8s.io/cli-runtime/pkg/genericclioptions.RESTClientGetter interface. +func (c *Getter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { + cc, err := c.ToRESTConfig() + if err != nil { + return nil, err + } + + d, err := discovery.NewDiscoveryClientForConfig(cc) + if err != nil { + return nil, err + } + + return memory.NewMemCacheClient(d), nil +} + +// ToRawKubeConfigLoader is part of k8s.io/cli-runtime/pkg/genericclioptions.RESTClientGetter interface. +func (c *Getter) ToRawKubeConfigLoader() clientcmd.ClientConfig { + return c.c +} + +// ToRESTConfig is part of k8s.io/cli-runtime/pkg/genericclioptions.RESTClientGetter interface. +func (c *Getter) ToRESTConfig() (*rest.Config, error) { + return c.c.ClientConfig() +} + +// NewGetter takes content of kubeconfig file as an argument and returns implementation of +// RESTClientGetter k8s interface. +func NewGetter(data []byte) (*Getter, error) { + c, err := clientcmd.NewClientConfigFromBytes(data) + if err != nil { + return nil, err + } + + return &Getter{ + c: c, + }, nil +} diff --git a/pkg/k8sutil/getter_test.go b/pkg/k8sutil/getter_test.go new file mode 100644 index 000000000..b4e3c94ef --- /dev/null +++ b/pkg/k8sutil/getter_test.go @@ -0,0 +1,46 @@ +// 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" +) + +func TestGetter(t *testing.T) { + g, err := k8sutil.NewGetter([]byte(validKubeconfig)) + if err != nil { + t.Fatalf("Creating getter from valid kubeconfig should succeed, got: %v", err) + } + + if _, err := g.ToDiscoveryClient(); err != nil { + t.Errorf("Turning getter into discovery client should succeed, got: %v", err) + } + + if _, err := g.ToRESTMapper(); err != nil { + t.Errorf("Turning getter into REST mapper should succeed, got: %v", err) + } + + if c := g.ToRawKubeConfigLoader(); c == nil { + t.Errorf("Turning getter into RawKubeConfigLoader should succeed") + } +} + +func TestGetterInvalidKubeconfig(t *testing.T) { + if _, err := k8sutil.NewGetter([]byte("foo")); err == nil { + t.Fatalf("Creating getter from invalid kubeconfig should fail") + } +} diff --git a/vendor/k8s.io/client-go/discovery/cached/memory/memcache.go b/vendor/k8s.io/client-go/discovery/cached/memory/memcache.go new file mode 100644 index 000000000..82a7cc470 --- /dev/null +++ b/vendor/k8s.io/client-go/discovery/cached/memory/memcache.go @@ -0,0 +1,243 @@ +/* +Copyright 2017 The Kubernetes 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 memory + +import ( + "errors" + "fmt" + "net" + "net/url" + "sync" + "syscall" + + "github.com/googleapis/gnostic/OpenAPIv2" + + errorsutil "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/version" + "k8s.io/client-go/discovery" + restclient "k8s.io/client-go/rest" +) + +type cacheEntry struct { + resourceList *metav1.APIResourceList + err error +} + +// memCacheClient can Invalidate() to stay up-to-date with discovery +// information. +// +// TODO: Switch to a watch interface. Right now it will poll after each +// Invalidate() call. +type memCacheClient struct { + delegate discovery.DiscoveryInterface + + lock sync.RWMutex + groupToServerResources map[string]*cacheEntry + groupList *metav1.APIGroupList + cacheValid bool +} + +// Error Constants +var ( + ErrCacheNotFound = errors.New("not found") +) + +var _ discovery.CachedDiscoveryInterface = &memCacheClient{} + +// isTransientConnectionError checks whether given error is "Connection refused" or +// "Connection reset" error which usually means that apiserver is temporarily +// unavailable. +func isTransientConnectionError(err error) bool { + urlError, ok := err.(*url.Error) + if !ok { + return false + } + opError, ok := urlError.Err.(*net.OpError) + if !ok { + return false + } + errno, ok := opError.Err.(syscall.Errno) + if !ok { + return false + } + return errno == syscall.ECONNREFUSED || errno == syscall.ECONNRESET +} + +func isTransientError(err error) bool { + if isTransientConnectionError(err) { + return true + } + + if t, ok := err.(errorsutil.APIStatus); ok && t.Status().Code >= 500 { + return true + } + + return errorsutil.IsTooManyRequests(err) +} + +// ServerResourcesForGroupVersion returns the supported resources for a group and version. +func (d *memCacheClient) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { + d.lock.Lock() + defer d.lock.Unlock() + if !d.cacheValid { + if err := d.refreshLocked(); err != nil { + return nil, err + } + } + cachedVal, ok := d.groupToServerResources[groupVersion] + if !ok { + return nil, ErrCacheNotFound + } + + if cachedVal.err != nil && isTransientError(cachedVal.err) { + r, err := d.serverResourcesForGroupVersion(groupVersion) + if err != nil { + utilruntime.HandleError(fmt.Errorf("couldn't get resource list for %v: %v", groupVersion, err)) + } + cachedVal = &cacheEntry{r, err} + d.groupToServerResources[groupVersion] = cachedVal + } + + return cachedVal.resourceList, cachedVal.err +} + +// ServerResources returns the supported resources for all groups and versions. +// Deprecated: use ServerGroupsAndResources instead. +func (d *memCacheClient) ServerResources() ([]*metav1.APIResourceList, error) { + return discovery.ServerResources(d) +} + +// ServerGroupsAndResources returns the groups and supported resources for all groups and versions. +func (d *memCacheClient) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { + return discovery.ServerGroupsAndResources(d) +} + +func (d *memCacheClient) ServerGroups() (*metav1.APIGroupList, error) { + d.lock.Lock() + defer d.lock.Unlock() + if !d.cacheValid { + if err := d.refreshLocked(); err != nil { + return nil, err + } + } + return d.groupList, nil +} + +func (d *memCacheClient) RESTClient() restclient.Interface { + return d.delegate.RESTClient() +} + +func (d *memCacheClient) ServerPreferredResources() ([]*metav1.APIResourceList, error) { + return discovery.ServerPreferredResources(d) +} + +func (d *memCacheClient) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) { + return discovery.ServerPreferredNamespacedResources(d) +} + +func (d *memCacheClient) ServerVersion() (*version.Info, error) { + return d.delegate.ServerVersion() +} + +func (d *memCacheClient) OpenAPISchema() (*openapi_v2.Document, error) { + return d.delegate.OpenAPISchema() +} + +func (d *memCacheClient) Fresh() bool { + d.lock.RLock() + defer d.lock.RUnlock() + // Return whether the cache is populated at all. It is still possible that + // a single entry is missing due to transient errors and the attempt to read + // that entry will trigger retry. + return d.cacheValid +} + +// Invalidate enforces that no cached data that is older than the current time +// is used. +func (d *memCacheClient) Invalidate() { + d.lock.Lock() + defer d.lock.Unlock() + d.cacheValid = false + d.groupToServerResources = nil + d.groupList = nil +} + +// refreshLocked refreshes the state of cache. The caller must hold d.lock for +// writing. +func (d *memCacheClient) refreshLocked() error { + // TODO: Could this multiplicative set of calls be replaced by a single call + // to ServerResources? If it's possible for more than one resulting + // APIResourceList to have the same GroupVersion, the lists would need merged. + gl, err := d.delegate.ServerGroups() + if err != nil || len(gl.Groups) == 0 { + utilruntime.HandleError(fmt.Errorf("couldn't get current server API group list: %v", err)) + return err + } + + wg := &sync.WaitGroup{} + resultLock := &sync.Mutex{} + rl := map[string]*cacheEntry{} + for _, g := range gl.Groups { + for _, v := range g.Versions { + gv := v.GroupVersion + wg.Add(1) + go func() { + defer wg.Done() + defer utilruntime.HandleCrash() + + r, err := d.serverResourcesForGroupVersion(gv) + if err != nil { + utilruntime.HandleError(fmt.Errorf("couldn't get resource list for %v: %v", gv, err)) + } + + resultLock.Lock() + defer resultLock.Unlock() + rl[gv] = &cacheEntry{r, err} + }() + } + } + wg.Wait() + + d.groupToServerResources, d.groupList = rl, gl + d.cacheValid = true + return nil +} + +func (d *memCacheClient) serverResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { + r, err := d.delegate.ServerResourcesForGroupVersion(groupVersion) + if err != nil { + return r, err + } + if len(r.APIResources) == 0 { + return r, fmt.Errorf("Got empty response for: %v", groupVersion) + } + return r, nil +} + +// NewMemCacheClient creates a new CachedDiscoveryInterface which caches +// discovery information in memory and will stay up-to-date if Invalidate is +// called with regularity. +// +// NOTE: The client will NOT resort to live lookups on cache misses. +func NewMemCacheClient(delegate discovery.DiscoveryInterface) discovery.CachedDiscoveryInterface { + return &memCacheClient{ + delegate: delegate, + groupToServerResources: map[string]*cacheEntry{}, + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index ea44dc7c9..8ccae1c53 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -615,6 +615,7 @@ k8s.io/cli-runtime/pkg/resource # k8s.io/client-go v0.18.0 k8s.io/client-go/discovery k8s.io/client-go/discovery/cached/disk +k8s.io/client-go/discovery/cached/memory k8s.io/client-go/dynamic k8s.io/client-go/kubernetes k8s.io/client-go/kubernetes/scheme From ecfd72702327185bf9d51642158e5c7fb25886e5 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 09:44:34 +0200 Subject: [PATCH 2/9] pkg/components/util: make HelmActionConfig use kubeconfig file content As part of #608, this commit changes the implementation of HelmActionConfig function to read the content of given kubeconfig file and then use the content to construct Helm's actionConfig. This will make easier changing the function signature to accept content of kubeconfig file in the future changes. Signed-off-by: Mateusz Gozdek --- pkg/components/util/install.go | 19 ++++++- pkg/components/util/install_test.go | 80 +++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 pkg/components/util/install_test.go diff --git a/pkg/components/util/install.go b/pkg/components/util/install.go index 11cd93cc8..c648772b0 100644 --- a/pkg/components/util/install.go +++ b/pkg/components/util/install.go @@ -151,14 +151,29 @@ func upgrade(helmAction *helmAction) error { func HelmActionConfig(ns string, kubeconfig string) (*action.Configuration, error) { actionConfig := &action.Configuration{} + kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 + if err != nil { + return nil, fmt.Errorf("failed to read kubeconfig file %q: %v", kubeconfig, err) + } + + getter, err := k8sutil.NewGetter(kubeconfigContent) + if err != nil { + return nil, fmt.Errorf("failed to create Kubernetes client getter: %v", err) + } + // TODO: Add some logging implementation? We currently just pass an empty function for logging. - kubeConfig := kube.GetConfig(kubeconfig, "", ns) logF := func(format string, v ...interface{}) {} - if err := actionConfig.Init(kubeConfig, ns, "secret", logF); err != nil { + if err := actionConfig.Init(getter, ns, "secret", logF); err != nil { return nil, fmt.Errorf("failed initializing helm: %w", err) } + kc := kube.New(getter) + kc.Log = logF + kc.Namespace = ns + + actionConfig.KubeClient = kc + return actionConfig, nil } diff --git a/pkg/components/util/install_test.go b/pkg/components/util/install_test.go new file mode 100644 index 000000000..15906c7e2 --- /dev/null +++ b/pkg/components/util/install_test.go @@ -0,0 +1,80 @@ +package util_test + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/kinvolk/lokomotive/pkg/components/util" +) + +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 TestHelmActionConfigFromValidKubeconfigFile(t *testing.T) { + tmpFile, err := ioutil.TempFile("", "lokoctl-tests-") + if err != nil { + t.Fatalf("creating tmp file should succeed, got: %v", err) + } + + defer func() { + if err := os.Remove(tmpFile.Name()); err != nil { + t.Logf("failed to remove tmp file %q: %v", tmpFile.Name(), err) + } + }() + + if _, err := tmpFile.Write([]byte(validKubeconfig)); err != nil { + t.Fatalf("writing to tmp file %q should succeed, got: %v", tmpFile.Name(), err) + } + + if err := tmpFile.Close(); err != nil { + t.Fatalf("closing tmp file %q should succeed, got: %v", tmpFile.Name(), err) + } + + if _, err := util.HelmActionConfig("foo", tmpFile.Name()); err != nil { + t.Fatalf("creating helm action config from valid kubeconfig file should succeed, got: %v", err) + } +} + +func TestHelmActionConfigFromInvalidKubeconfigFile(t *testing.T) { + tmpFile, err := ioutil.TempFile("", "lokoctl-tests-") + if err != nil { + t.Fatalf("creating tmp file should succeed, got: %v", err) + } + + defer func() { + if err := os.Remove(tmpFile.Name()); err != nil { + t.Logf("failed to remove tmp file %q: %v", tmpFile.Name(), err) + } + }() + + if _, err := tmpFile.Write([]byte("foo")); err != nil { + t.Fatalf("writing to tmp file %q should succeed, got: %v", tmpFile.Name(), err) + } + + if err := tmpFile.Close(); err != nil { + t.Fatalf("closing tmp file %q should succeed, got: %v", tmpFile.Name(), err) + } + + if _, err := util.HelmActionConfig("foo", tmpFile.Name()); err == nil { + t.Fatalf("creating helm action config from invalid kubeconfig file should fail") + } +} From 98b1d2fe1a4f348a6721b667f1938b6dcceec44a Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 10:33:29 +0200 Subject: [PATCH 3/9] cli/cmd: move deleteHelmRelease to pkg/components/util As component-oriented functionality should be stored in pkg/components and cli package should not go too much into helm details etc. Signed-off-by: Mateusz Gozdek --- cli/cmd/component-delete.go | 80 +--------------------------------- pkg/components/util/install.go | 72 ++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 79 deletions(-) diff --git a/cli/cmd/component-delete.go b/cli/cmd/component-delete.go index f8835b151..f2a1d59e8 100644 --- a/cli/cmd/component-delete.go +++ b/cli/cmd/component-delete.go @@ -15,20 +15,14 @@ package cmd import ( - "context" "fmt" - "io/ioutil" "strings" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "helm.sh/helm/v3/pkg/action" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/kinvolk/lokomotive/pkg/components" "github.com/kinvolk/lokomotive/pkg/components/util" - "github.com/kinvolk/lokomotive/pkg/k8sutil" ) var componentDeleteCmd = &cobra.Command{ @@ -106,7 +100,7 @@ func deleteComponents(kubeconfig string, componentObjects ...components.Componen for _, compObj := range componentObjects { fmt.Printf("Deleting component '%s'...\n", compObj.Metadata().Name) - if err := deleteHelmRelease(compObj, kubeconfig, deleteNamespace); err != nil { + if err := util.UninstallComponent(compObj, kubeconfig, deleteNamespace); err != nil { return err } @@ -118,75 +112,3 @@ func deleteComponents(kubeconfig string, componentObjects ...components.Componen return nil } - -// deleteComponent deletes a component. -func deleteHelmRelease(c components.Component, kubeconfig string, deleteNSBool bool) error { - name := c.Metadata().Name - if name == "" { - // This should never fail in real user usage, if this does that means the component was not - // created with all the needed information. - panic(fmt.Errorf("component name is empty")) - } - - ns := c.Metadata().Namespace - if ns == "" { - // This should never fail in real user usage, if this does that means the component was not - // created with all the needed information. - panic(fmt.Errorf("component %s namespace is empty", name)) - } - - cfg, err := util.HelmActionConfig(ns, kubeconfig) - if err != nil { - return fmt.Errorf("failed preparing helm client: %w", err) - } - - history := action.NewHistory(cfg) - // Check if the component's release exists. If it does only then proceed to delete. - // - // Note: It is assumed that this call will return error only when the release does not exist. - // The error check is ignored to make `lokoctl component delete ..` idempotent. - // We rely on the fact that the 'component name' == 'release name'. Since component's name is - // hardcoded and unlikely to change release name won't change as well. And they will be - // consistent if installed by lokoctl. So it is highly unlikely that following call will return - // any other error than "release not found". - if _, err := history.Run(name); err == nil { - uninstall := action.NewUninstall(cfg) - - // Ignore the err when we have deleted the release already or it does not exist for some reason. - if _, err := uninstall.Run(name); err != nil { - return err - } - } - - if deleteNSBool { - if err := deleteNS(ns, kubeconfig); err != nil { - return err - } - } - - return nil -} - -func deleteNS(ns string, kubeconfig string) error { - 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 - } - - // Delete the manually created namespace which was not created by helm. - if err = cs.CoreV1().Namespaces().Delete(context.TODO(), ns, metav1.DeleteOptions{}); err != nil { - // Ignore error when the namespace does not exist. - if errors.IsNotFound(err) { - return nil - } - - return err - } - - return nil -} diff --git a/pkg/components/util/install.go b/pkg/components/util/install.go index c648772b0..581e647a7 100644 --- a/pkg/components/util/install.go +++ b/pkg/components/util/install.go @@ -189,3 +189,75 @@ func ReleaseExists(actionConfig action.Configuration, name string) (bool, error) return err != driver.ErrReleaseNotFound, nil } + +// UninstallComponent uninstalls a component and optionally removes it's namespace. +func UninstallComponent(c components.Component, kubeconfig string, deleteNSBool bool) error { + name := c.Metadata().Name + if name == "" { + // This should never fail in real user usage, if this does that means the component was not + // created with all the needed information. + panic(fmt.Errorf("component name is empty")) + } + + ns := c.Metadata().Namespace + if ns == "" { + // This should never fail in real user usage, if this does that means the component was not + // created with all the needed information. + panic(fmt.Errorf("component %s namespace is empty", name)) + } + + cfg, err := HelmActionConfig(ns, kubeconfig) + if err != nil { + return fmt.Errorf("failed preparing helm client: %w", err) + } + + history := action.NewHistory(cfg) + // Check if the component's release exists. If it does only then proceed to delete. + // + // Note: It is assumed that this call will return error only when the release does not exist. + // The error check is ignored to make `lokoctl component delete ..` idempotent. + // We rely on the fact that the 'component name' == 'release name'. Since component's name is + // hardcoded and unlikely to change release name won't change as well. And they will be + // consistent if installed by lokoctl. So it is highly unlikely that following call will return + // any other error than "release not found". + if _, err := history.Run(name); err == nil { + uninstall := action.NewUninstall(cfg) + + // Ignore the err when we have deleted the release already or it does not exist for some reason. + if _, err := uninstall.Run(name); err != nil { + return err + } + } + + if deleteNSBool { + if err := deleteNS(ns, kubeconfig); err != nil { + return err + } + } + + return nil +} + +func deleteNS(ns string, kubeconfig string) error { + 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 + } + + // Delete the manually created namespace which was not created by helm. + if err = cs.CoreV1().Namespaces().Delete(context.TODO(), ns, metav1.DeleteOptions{}); err != nil { + // Ignore error when the namespace does not exist. + if errors.IsNotFound(err) { + return nil + } + + return err + } + + return nil +} From bf1ce3727932015afc1a1262d120b333ee9d5772 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 11:16:23 +0200 Subject: [PATCH 4/9] pkg/components/util: make HelmActionConfig take kubeconfig content Instead of kubeconfig file path. This simplifies the tests and moves us closer towards resolving #608. Signed-off-by: Mateusz Gozdek --- cli/cmd/cluster-apply.go | 13 ++++++--- cli/cmd/cluster.go | 10 +++---- pkg/components/util/install.go | 23 +++++++++------ pkg/components/util/install_test.go | 44 ++--------------------------- 4 files changed, 30 insertions(+), 60 deletions(-) diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go index f410cdb18..0c68ae51a 100644 --- a/cli/cmd/cluster-apply.go +++ b/cli/cmd/cluster-apply.go @@ -85,15 +85,20 @@ func runClusterApply(cmd *cobra.Command, args []string) { ctxLogger.Fatalf("Verify cluster: %v", err) } + kubeconfigContent, err := ioutil.ReadFile(kubeconfigPath) // #nosec G304 + if err != nil { + ctxLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfigPath, err) + } + // Do controlplane upgrades only if cluster already exists and it is not a managed platform. if exists && !p.Meta().Managed { fmt.Printf("\nEnsuring that cluster controlplane is up to date.\n") cu := controlplaneUpdater{ - kubeconfigPath: kubeconfigPath, - assetDir: assetDir, - ctxLogger: *ctxLogger, - ex: *ex, + kubeconfig: kubeconfigContent, + assetDir: assetDir, + ctxLogger: *ctxLogger, + ex: *ex, } releases := []string{"pod-checkpointer", "kube-apiserver", "kubernetes", "calico"} diff --git a/cli/cmd/cluster.go b/cli/cmd/cluster.go index bf12f9003..d8b261834 100644 --- a/cli/cmd/cluster.go +++ b/cli/cmd/cluster.go @@ -148,10 +148,10 @@ func clusterExists(ctxLogger *logrus.Entry, ex *terraform.Executor) bool { } type controlplaneUpdater struct { - kubeconfigPath string - assetDir string - ctxLogger logrus.Entry - ex terraform.Executor + kubeconfig []byte + assetDir string + ctxLogger logrus.Entry + ex terraform.Executor } func (c controlplaneUpdater) getControlplaneChart(name string) (*chart.Chart, error) { @@ -187,7 +187,7 @@ func (c controlplaneUpdater) upgradeComponent(component string) { "component": component, }) - actionConfig, err := util.HelmActionConfig("kube-system", c.kubeconfigPath) + actionConfig, err := util.HelmActionConfig("kube-system", c.kubeconfig) if err != nil { ctxLogger.Fatalf("Failed initializing helm: %v", err) } diff --git a/pkg/components/util/install.go b/pkg/components/util/install.go index 581e647a7..f418af37e 100644 --- a/pkg/components/util/install.go +++ b/pkg/components/util/install.go @@ -68,7 +68,12 @@ func InstallComponent(c components.Component, kubeconfig string) error { return fmt.Errorf("failed ensuring that namespace %q for component %q exists: %w", ns, name, err) } - actionConfig, err := HelmActionConfig(ns, kubeconfig) + kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 + if err != nil { + return fmt.Errorf("failed to read kubeconfig file %q: %v", kubeconfig, err) + } + + actionConfig, err := HelmActionConfig(ns, kubeconfigContent) if err != nil { return fmt.Errorf("failed preparing helm client: %w", err) } @@ -148,15 +153,10 @@ func upgrade(helmAction *helmAction) error { } // HelmActionConfig creates initialized Helm action configuration. -func HelmActionConfig(ns string, kubeconfig string) (*action.Configuration, error) { +func HelmActionConfig(ns string, kubeconfig []byte) (*action.Configuration, error) { actionConfig := &action.Configuration{} - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - return nil, fmt.Errorf("failed to read kubeconfig file %q: %v", kubeconfig, err) - } - - getter, err := k8sutil.NewGetter(kubeconfigContent) + getter, err := k8sutil.NewGetter(kubeconfig) if err != nil { return nil, fmt.Errorf("failed to create Kubernetes client getter: %v", err) } @@ -206,7 +206,12 @@ func UninstallComponent(c components.Component, kubeconfig string, deleteNSBool panic(fmt.Errorf("component %s namespace is empty", name)) } - cfg, err := HelmActionConfig(ns, kubeconfig) + kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 + if err != nil { + return fmt.Errorf("failed to read kubeconfig file %q: %v", kubeconfig, err) + } + + cfg, err := HelmActionConfig(ns, kubeconfigContent) if err != nil { return fmt.Errorf("failed preparing helm client: %w", err) } diff --git a/pkg/components/util/install_test.go b/pkg/components/util/install_test.go index 15906c7e2..9fefd78dc 100644 --- a/pkg/components/util/install_test.go +++ b/pkg/components/util/install_test.go @@ -1,8 +1,6 @@ package util_test import ( - "io/ioutil" - "os" "testing" "github.com/kinvolk/lokomotive/pkg/components/util" @@ -30,51 +28,13 @@ contexts: ) func TestHelmActionConfigFromValidKubeconfigFile(t *testing.T) { - tmpFile, err := ioutil.TempFile("", "lokoctl-tests-") - if err != nil { - t.Fatalf("creating tmp file should succeed, got: %v", err) - } - - defer func() { - if err := os.Remove(tmpFile.Name()); err != nil { - t.Logf("failed to remove tmp file %q: %v", tmpFile.Name(), err) - } - }() - - if _, err := tmpFile.Write([]byte(validKubeconfig)); err != nil { - t.Fatalf("writing to tmp file %q should succeed, got: %v", tmpFile.Name(), err) - } - - if err := tmpFile.Close(); err != nil { - t.Fatalf("closing tmp file %q should succeed, got: %v", tmpFile.Name(), err) - } - - if _, err := util.HelmActionConfig("foo", tmpFile.Name()); err != nil { + if _, err := util.HelmActionConfig("foo", []byte(validKubeconfig)); err != nil { t.Fatalf("creating helm action config from valid kubeconfig file should succeed, got: %v", err) } } func TestHelmActionConfigFromInvalidKubeconfigFile(t *testing.T) { - tmpFile, err := ioutil.TempFile("", "lokoctl-tests-") - if err != nil { - t.Fatalf("creating tmp file should succeed, got: %v", err) - } - - defer func() { - if err := os.Remove(tmpFile.Name()); err != nil { - t.Logf("failed to remove tmp file %q: %v", tmpFile.Name(), err) - } - }() - - if _, err := tmpFile.Write([]byte("foo")); err != nil { - t.Fatalf("writing to tmp file %q should succeed, got: %v", tmpFile.Name(), err) - } - - if err := tmpFile.Close(); err != nil { - t.Fatalf("closing tmp file %q should succeed, got: %v", tmpFile.Name(), err) - } - - if _, err := util.HelmActionConfig("foo", tmpFile.Name()); err == nil { + if _, err := util.HelmActionConfig("foo", []byte("foo")); err == nil { t.Fatalf("creating helm action config from invalid kubeconfig file should fail") } } From 4d59cbce6946aac15a3822edd24cbf4cbcb18935 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 11:18:53 +0200 Subject: [PATCH 5/9] cli/cmd: don't read kubeconfig file twice when running cluster apply Instead, change verifyCluster function to take content of kubeconfig file as an argument, which caller should already have. Signed-off-by: Mateusz Gozdek --- cli/cmd/cluster-apply.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go index 0c68ae51a..ea85b3505 100644 --- a/cli/cmd/cluster-apply.go +++ b/cli/cmd/cluster-apply.go @@ -81,15 +81,16 @@ func runClusterApply(cmd *cobra.Command, args []string) { fmt.Printf("\nYour configurations are stored in %s\n", assetDir) kubeconfigPath := assetsKubeconfig(assetDir) - if err := verifyCluster(kubeconfigPath, p.Meta().ExpectedNodes); err != nil { - ctxLogger.Fatalf("Verify cluster: %v", err) - } kubeconfigContent, err := ioutil.ReadFile(kubeconfigPath) // #nosec G304 if err != nil { ctxLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfigPath, err) } + if err := verifyCluster(kubeconfigContent, p.Meta().ExpectedNodes); err != nil { + ctxLogger.Fatalf("Verify cluster: %v", err) + } + // Do controlplane upgrades only if cluster already exists and it is not a managed platform. if exists && !p.Meta().Managed { fmt.Printf("\nEnsuring that cluster controlplane is up to date.\n") @@ -130,12 +131,7 @@ func runClusterApply(cmd *cobra.Command, args []string) { } } -func verifyCluster(kubeconfigPath string, expectedNodes int) error { - kubeconfig, err := ioutil.ReadFile(kubeconfigPath) // #nosec G304 - if err != nil { - return errors.Wrapf(err, "failed to read kubeconfig file") - } - +func verifyCluster(kubeconfig []byte, expectedNodes int) error { cs, err := k8sutil.NewClientset(kubeconfig) if err != nil { return errors.Wrapf(err, "failed to set up clientset") From 434b7bb983357707e054c1b19ffa139ceb95456f Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 11:23:05 +0200 Subject: [PATCH 6/9] pkg/components/util: make UninstallComponent take kubeconfig content So we don't necesserily need a file on file system to perfrom this action, which is needed to resolve #608. Signed-off-by: Mateusz Gozdek --- cli/cmd/component-delete.go | 10 ++++++++-- pkg/components/util/install.go | 18 ++++-------------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/cli/cmd/component-delete.go b/cli/cmd/component-delete.go index f2a1d59e8..b1587193e 100644 --- a/cli/cmd/component-delete.go +++ b/cli/cmd/component-delete.go @@ -16,6 +16,7 @@ package cmd import ( "fmt" + "io/ioutil" "strings" log "github.com/sirupsen/logrus" @@ -91,12 +92,17 @@ func runDelete(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - if err := deleteComponents(kubeconfig, componentsObjects...); err != nil { + kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 + if err != nil { + contextLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfig, err) + } + + if err := deleteComponents(kubeconfigContent, componentsObjects...); err != nil { contextLogger.Fatal(err) } } -func deleteComponents(kubeconfig string, componentObjects ...components.Component) error { +func deleteComponents(kubeconfig []byte, componentObjects ...components.Component) error { for _, compObj := range componentObjects { fmt.Printf("Deleting component '%s'...\n", compObj.Metadata().Name) diff --git a/pkg/components/util/install.go b/pkg/components/util/install.go index f418af37e..7a9d2f3b7 100644 --- a/pkg/components/util/install.go +++ b/pkg/components/util/install.go @@ -191,7 +191,7 @@ func ReleaseExists(actionConfig action.Configuration, name string) (bool, error) } // UninstallComponent uninstalls a component and optionally removes it's namespace. -func UninstallComponent(c components.Component, kubeconfig string, deleteNSBool bool) error { +func UninstallComponent(c components.Component, kubeconfig []byte, deleteNSBool bool) error { name := c.Metadata().Name if name == "" { // This should never fail in real user usage, if this does that means the component was not @@ -206,12 +206,7 @@ func UninstallComponent(c components.Component, kubeconfig string, deleteNSBool panic(fmt.Errorf("component %s namespace is empty", name)) } - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - return fmt.Errorf("failed to read kubeconfig file %q: %v", kubeconfig, err) - } - - cfg, err := HelmActionConfig(ns, kubeconfigContent) + cfg, err := HelmActionConfig(ns, kubeconfig) if err != nil { return fmt.Errorf("failed preparing helm client: %w", err) } @@ -243,13 +238,8 @@ func UninstallComponent(c components.Component, kubeconfig string, deleteNSBool return nil } -func deleteNS(ns string, kubeconfig string) error { - 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) +func deleteNS(ns string, kubeconfig []byte) error { + cs, err := k8sutil.NewClientset(kubeconfig) if err != nil { return err } From 88d87d3df4f5e74a15a7d1ff3e0952eea89b2376 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 11:28:01 +0200 Subject: [PATCH 7/9] pkg/components/util: make InstallComponent take kubeconfig content This commit changes InstallComponent and InstallAsRelease methods to accept kubeconfig content, rather than the file path, to allow loading it from places other than local filesystem. Part of #608. Signed-off-by: Mateusz Gozdek --- cli/cmd/cluster-apply.go | 2 +- cli/cmd/component-apply.go | 10 ++++++++-- pkg/components/util/install.go | 17 +++-------------- test/components/install_test.go | 3 ++- test/components/util/util.go | 14 ++++++++++++++ 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go index ea85b3505..2cb2065e5 100644 --- a/cli/cmd/cluster-apply.go +++ b/cli/cmd/cluster-apply.go @@ -125,7 +125,7 @@ func runClusterApply(cmd *cobra.Command, args []string) { ctxLogger.Println("Applying component configuration") if len(componentsToApply) > 0 { - if err := applyComponents(lokoConfig, kubeconfigPath, componentsToApply...); err != nil { + if err := applyComponents(lokoConfig, kubeconfigContent, componentsToApply...); err != nil { ctxLogger.Fatalf("Applying component configuration failed: %v", err) } } diff --git a/cli/cmd/component-apply.go b/cli/cmd/component-apply.go index 441a33fa4..fe28bf154 100644 --- a/cli/cmd/component-apply.go +++ b/cli/cmd/component-apply.go @@ -16,6 +16,7 @@ package cmd import ( "fmt" + "io/ioutil" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -63,12 +64,17 @@ func runApply(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - if err := applyComponents(lokoConfig, kubeconfig, componentsToApply...); err != nil { + kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 + if err != nil { + contextLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfig, err) + } + + if err := applyComponents(lokoConfig, kubeconfigContent, componentsToApply...); err != nil { contextLogger.Fatal(err) } } -func applyComponents(lokoConfig *config.Config, kubeconfig string, componentNames ...string) error { +func applyComponents(lokoConfig *config.Config, kubeconfig []byte, componentNames ...string) error { for _, componentName := range componentNames { fmt.Printf("Applying component '%s'...\n", componentName) diff --git a/pkg/components/util/install.go b/pkg/components/util/install.go index 7a9d2f3b7..2b6b5e82e 100644 --- a/pkg/components/util/install.go +++ b/pkg/components/util/install.go @@ -17,7 +17,6 @@ package util import ( "context" "fmt" - "io/ioutil" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart" @@ -31,12 +30,7 @@ import ( "github.com/kinvolk/lokomotive/pkg/k8sutil" ) -func ensureNamespaceExists(name string, kubeconfigPath string) error { - kubeconfig, err := ioutil.ReadFile(kubeconfigPath) // #nosec G304 - if err != nil { - return fmt.Errorf("reading kubeconfig file: %w", err) - } - +func ensureNamespaceExists(name string, kubeconfig []byte) error { cs, err := k8sutil.NewClientset(kubeconfig) if err != nil { return fmt.Errorf("creating clientset: %w", err) @@ -60,7 +54,7 @@ func ensureNamespaceExists(name string, kubeconfigPath string) error { } // InstallComponent installs given component using given kubeconfig as a Helm release using a Helm client. -func InstallComponent(c components.Component, kubeconfig string) error { +func InstallComponent(c components.Component, kubeconfig []byte) error { name := c.Metadata().Name ns := c.Metadata().Namespace @@ -68,12 +62,7 @@ func InstallComponent(c components.Component, kubeconfig string) error { return fmt.Errorf("failed ensuring that namespace %q for component %q exists: %w", ns, name, err) } - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - return fmt.Errorf("failed to read kubeconfig file %q: %v", kubeconfig, err) - } - - actionConfig, err := HelmActionConfig(ns, kubeconfigContent) + actionConfig, err := HelmActionConfig(ns, kubeconfig) if err != nil { return fmt.Errorf("failed preparing helm client: %w", err) } diff --git a/test/components/install_test.go b/test/components/install_test.go index 5014a9f90..50abb892b 100644 --- a/test/components/install_test.go +++ b/test/components/install_test.go @@ -50,7 +50,8 @@ component "flatcar-linux-update-operator" {} t.Fatalf("Valid config should not return error, got: %s", diagnostics) } - k := testutil.KubeconfigPath(t) + k := testutil.Kubeconfig(t) + if err := util.InstallComponent(c, k); err != nil { t.Fatalf("Installing component as release should succeed, got: %v", err) } diff --git a/test/components/util/util.go b/test/components/util/util.go index 49adc7fc2..402202e92 100644 --- a/test/components/util/util.go +++ b/test/components/util/util.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "fmt" + "io/ioutil" "net/http" "net/url" "os" @@ -47,6 +48,19 @@ func KubeconfigPath(t *testing.T) string { return kubeconfig } +// Kubeconfig returns content of kubeconfig file defined with KUBECONFIG +// environment variable. +func Kubeconfig(t *testing.T) []byte { + path := KubeconfigPath(t) + + k, err := ioutil.ReadFile(path) // #nosec:G304 + if err != nil { + t.Fatalf("reading KUBECONFIG file from %q failed: %v", path, err) + } + + return k +} + // buildKubeConfig reads the environment variable KUBECONFIG and then builds the rest client config // object which can be either used to create kube client to talk to apiserver or to just read the // kubeconfig data. From 8a5c4d845efcd3e72e12e05cfc29c38cedebde5e Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 11:33:11 +0200 Subject: [PATCH 8/9] cli/cmd: remove check if kubeconfig exists from health command The subcommand function will fail anyway while trying to read kubeconfig file, so there is no need to check it twice. Also part of #608. Signed-off-by: Mateusz Gozdek --- cli/cmd/health.go | 7 +++---- cli/cmd/utils.go | 14 -------------- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/cli/cmd/health.go b/cli/cmd/health.go index 913e5b9c2..a2c71ce72 100644 --- a/cli/cmd/health.go +++ b/cli/cmd/health.go @@ -28,10 +28,9 @@ import ( ) var healthCmd = &cobra.Command{ - Use: "health", - Short: "Get the health of a cluster", - Run: runHealth, - PersistentPreRunE: doesKubeconfigExist, + Use: "health", + Short: "Get the health of a cluster", + Run: runHealth, } func init() { diff --git a/cli/cmd/utils.go b/cli/cmd/utils.go index bf8dbdb50..aa9a3655e 100644 --- a/cli/cmd/utils.go +++ b/cli/cmd/utils.go @@ -21,7 +21,6 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/mitchellh/go-homedir" - "github.com/spf13/cobra" "github.com/spf13/viper" "github.com/kinvolk/lokomotive/pkg/backend" @@ -161,19 +160,6 @@ func assetsKubeconfig(assetDir string) string { return filepath.Join(assetDir, "cluster-assets", "auth", "kubeconfig") } -// doesKubeconfigExist checks if the kubeconfig provided by user exists -func doesKubeconfigExist(*cobra.Command, []string) error { - var err error - kubeconfig, err := getKubeconfig() - if err != nil { - return err - } - if _, err = os.Stat(kubeconfig); os.IsNotExist(err) { - return fmt.Errorf("Kubeconfig %q not found", kubeconfig) - } - return err -} - func getLokoConfig() (*config.Config, hcl.Diagnostics) { return config.LoadConfig(viper.GetString("lokocfg"), viper.GetString("lokocfg-vars")) } From b65cf5bea9ef0d9dbae0ee4691f8af05f775e9d1 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 16 Jun 2020 11:44:45 +0200 Subject: [PATCH 9/9] cli/cmd: refactor getKubeconfig() to return kubeconfig file content Rather than kubeconfig file path. This will allow changing this function to pull kubeconfig content from Terraform output rather than from assets directory, which will resolve #608. Signed-off-by: Mateusz Gozdek --- cli/cmd/cluster-apply.go | 13 +++--- cli/cmd/component-apply.go | 8 +--- cli/cmd/component-delete.go | 8 +--- cli/cmd/health.go | 8 +--- cli/cmd/utils.go | 31 +++++++------- cli/cmd/utils_internal_test.go | 76 ++++++++++++++++++++++++++++------ 6 files changed, 87 insertions(+), 57 deletions(-) diff --git a/cli/cmd/cluster-apply.go b/cli/cmd/cluster-apply.go index 2cb2065e5..287971086 100644 --- a/cli/cmd/cluster-apply.go +++ b/cli/cmd/cluster-apply.go @@ -16,7 +16,6 @@ package cmd import ( "fmt" - "io/ioutil" "github.com/pkg/errors" log "github.com/sirupsen/logrus" @@ -80,14 +79,12 @@ func runClusterApply(cmd *cobra.Command, args []string) { fmt.Printf("\nYour configurations are stored in %s\n", assetDir) - kubeconfigPath := assetsKubeconfig(assetDir) - - kubeconfigContent, err := ioutil.ReadFile(kubeconfigPath) // #nosec G304 + kubeconfig, err := getKubeconfig() if err != nil { - ctxLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfigPath, err) + ctxLogger.Fatalf("Failed to get kubeconfig: %v", err) } - if err := verifyCluster(kubeconfigContent, p.Meta().ExpectedNodes); err != nil { + if err := verifyCluster(kubeconfig, p.Meta().ExpectedNodes); err != nil { ctxLogger.Fatalf("Verify cluster: %v", err) } @@ -96,7 +93,7 @@ func runClusterApply(cmd *cobra.Command, args []string) { fmt.Printf("\nEnsuring that cluster controlplane is up to date.\n") cu := controlplaneUpdater{ - kubeconfig: kubeconfigContent, + kubeconfig: kubeconfig, assetDir: assetDir, ctxLogger: *ctxLogger, ex: *ex, @@ -125,7 +122,7 @@ func runClusterApply(cmd *cobra.Command, args []string) { ctxLogger.Println("Applying component configuration") if len(componentsToApply) > 0 { - if err := applyComponents(lokoConfig, kubeconfigContent, componentsToApply...); err != nil { + if err := applyComponents(lokoConfig, kubeconfig, componentsToApply...); err != nil { ctxLogger.Fatalf("Applying component configuration failed: %v", err) } } diff --git a/cli/cmd/component-apply.go b/cli/cmd/component-apply.go index fe28bf154..fb80a4f39 100644 --- a/cli/cmd/component-apply.go +++ b/cli/cmd/component-apply.go @@ -16,7 +16,6 @@ package cmd import ( "fmt" - "io/ioutil" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -64,12 +63,7 @@ func runApply(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - contextLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfig, err) - } - - if err := applyComponents(lokoConfig, kubeconfigContent, componentsToApply...); err != nil { + if err := applyComponents(lokoConfig, kubeconfig, componentsToApply...); err != nil { contextLogger.Fatal(err) } } diff --git a/cli/cmd/component-delete.go b/cli/cmd/component-delete.go index b1587193e..ab1fe0ebb 100644 --- a/cli/cmd/component-delete.go +++ b/cli/cmd/component-delete.go @@ -16,7 +16,6 @@ package cmd import ( "fmt" - "io/ioutil" "strings" log "github.com/sirupsen/logrus" @@ -92,12 +91,7 @@ func runDelete(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - contextLogger.Fatalf("Failed to read kubeconfig file: %q: %v", kubeconfig, err) - } - - if err := deleteComponents(kubeconfigContent, componentsObjects...); err != nil { + if err := deleteComponents(kubeconfig, componentsObjects...); err != nil { contextLogger.Fatal(err) } } diff --git a/cli/cmd/health.go b/cli/cmd/health.go index a2c71ce72..1779c07d0 100644 --- a/cli/cmd/health.go +++ b/cli/cmd/health.go @@ -16,7 +16,6 @@ package cmd import ( "fmt" - "io/ioutil" "os" "text/tabwriter" @@ -48,12 +47,7 @@ func runHealth(cmd *cobra.Command, args []string) { contextLogger.Fatalf("Error in finding kubeconfig file: %s", err) } - kubeconfigContent, err := ioutil.ReadFile(kubeconfig) // #nosec G304 - if err != nil { - contextLogger.Fatalf("Failed to read kubeconfig file: %v", err) - } - - cs, err := k8sutil.NewClientset(kubeconfigContent) + cs, err := k8sutil.NewClientset(kubeconfig) if err != nil { contextLogger.Fatalf("Error in creating setting up Kubernetes client: %q", err) } diff --git a/cli/cmd/utils.go b/cli/cmd/utils.go index aa9a3655e..1cd8b00e1 100644 --- a/cli/cmd/utils.go +++ b/cli/cmd/utils.go @@ -16,6 +16,7 @@ package cmd import ( "fmt" + "io/ioutil" "os" "path/filepath" @@ -92,17 +93,20 @@ func getAssetDir() (string, error) { return cfg.Meta().AssetDir, nil } -// expandKubeconfigPath tries to expand ~ in the given kubeconfig path. -// However, if that fails, it just returns original path as the best effort. -func expandKubeconfigPath(path string) string { +func getKubeconfig() ([]byte, error) { + path, err := getKubeconfigPath() + if err != nil { + return nil, fmt.Errorf("failed getting kubeconfig path: %w", err) + } + if expandedPath, err := homedir.Expand(path); err == nil { - return expandedPath + path = expandedPath } // homedir.Expand is too restrictive for the ~ prefix, // i.e., it errors on "~somepath" which is a valid path, - // so just return the original path. - return path + // so just read from the original path. + return ioutil.ReadFile(path) // #nosec G304 } // getKubeconfig finds the kubeconfig to be used. The precedence is the following: @@ -112,7 +116,7 @@ func expandKubeconfigPath(path string) string { // - Asset directory from cluster configuration. // - KUBECONFIG environment variable. // - ~/.kube/config path, which is the default for kubectl. -func getKubeconfig() (string, error) { +func getKubeconfigPath() (string, error) { assetKubeconfig, err := assetsKubeconfigPath() if err != nil { return "", fmt.Errorf("reading kubeconfig path from configuration failed: %w", err) @@ -125,18 +129,13 @@ func getKubeconfig() (string, error) { defaultKubeconfigPath, } - return expandKubeconfigPath(pickString(paths...)), nil -} - -// pickString returns first non-empty string. -func pickString(options ...string) string { - for _, option := range options { - if option != "" { - return option + for _, path := range paths { + if path != "" { + return path, nil } } - return "" + return "", nil } // assetsKubeconfigPath reads the lokocfg configuration and returns diff --git a/cli/cmd/utils_internal_test.go b/cli/cmd/utils_internal_test.go index 9982e7caa..541f67125 100644 --- a/cli/cmd/utils_internal_test.go +++ b/cli/cmd/utils_internal_test.go @@ -18,6 +18,7 @@ import ( "io/ioutil" "os" "path/filepath" + "reflect" "testing" "github.com/spf13/viper" @@ -29,6 +30,10 @@ type kubeconfigSources struct { configFile string } +const ( + tmpPattern = "lokoctl-tests-" +) + func prepareKubeconfigSource(t *testing.T, k *kubeconfigSources) { // Ensure viper flag is NOT empty. viper.Set(kubeconfigFlag, k.flag) @@ -48,7 +53,7 @@ func prepareKubeconfigSource(t *testing.T, k *kubeconfigSources) { } // Ensure there is no lokocfg configuration in working directory. - tmpDir, err := ioutil.TempDir("", "lokoctl-tests-") + tmpDir, err := ioutil.TempDir("", tmpPattern) if err != nil { t.Fatalf("creating tmp dir: %v", err) } @@ -71,7 +76,54 @@ func prepareKubeconfigSource(t *testing.T, k *kubeconfigSources) { } } -func TestGetKubeconfigFlag(t *testing.T) { +func TestGetKubeconfigBadConfig(t *testing.T) { + k := &kubeconfigSources{ + configFile: `cluster "packet" { + asset_dir = "/foo" +}`, + } + + prepareKubeconfigSource(t, k) + + kubeconfig, err := getKubeconfig() + if err == nil { + t.Errorf("getting kubeconfig with bad configuration should fail") + } + + if kubeconfig != nil { + t.Fatalf("if getting kubeconfig fails, empty content should be returned") + } +} + +func TestGetKubeconfig(t *testing.T) { + expectedContent := []byte("foo") + + f, err := ioutil.TempFile("", tmpPattern) + if err != nil { + t.Fatalf("creating temp file should succeed, got: %v", err) + } + + if err := ioutil.WriteFile(f.Name(), expectedContent, 0600); err != nil { + t.Fatalf("writing temp file %q should succeed, got: %v", f.Name(), err) + } + + k := &kubeconfigSources{ + env: f.Name(), + } + + prepareKubeconfigSource(t, k) + + kubeconfig, err := getKubeconfig() + if err != nil { + t.Fatalf("getting kubeconfig: %v", err) + } + + if !reflect.DeepEqual(kubeconfig, expectedContent) { + t.Fatalf("expected %q, got %q", expectedContent, kubeconfig) + } +} + +func TestGetKubeconfigPathFlag(t *testing.T) { expectedPath := "/foo" k := &kubeconfigSources{ @@ -99,7 +151,7 @@ func TestGetKubeconfigFlag(t *testing.T) { prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err != nil { t.Fatalf("getting kubeconfig: %v", err) } @@ -109,7 +161,7 @@ func TestGetKubeconfigFlag(t *testing.T) { } } -func TestGetKubeconfigConfigFile(t *testing.T) { +func TestGetKubeconfigPathConfigFile(t *testing.T) { expectedPath := assetsKubeconfig("/foo") k := &kubeconfigSources{ @@ -136,7 +188,7 @@ func TestGetKubeconfigConfigFile(t *testing.T) { prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err != nil { t.Fatalf("getting kubeconfig: %v", err) } @@ -146,7 +198,7 @@ func TestGetKubeconfigConfigFile(t *testing.T) { } } -func TestGetKubeconfigBadConfigFile(t *testing.T) { +func TestGetKubeconfigPathBadConfigFile(t *testing.T) { expectedPath := "" k := &kubeconfigSources{ @@ -157,7 +209,7 @@ func TestGetKubeconfigBadConfigFile(t *testing.T) { prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err == nil { t.Errorf("getting kubeconfig with bad configuration should fail") } @@ -167,7 +219,7 @@ func TestGetKubeconfigBadConfigFile(t *testing.T) { } } -func TestGetKubeconfigEnvVariable(t *testing.T) { +func TestGetKubeconfigPathEnvVariable(t *testing.T) { expectedPath := "/foo" k := &kubeconfigSources{ @@ -176,7 +228,7 @@ func TestGetKubeconfigEnvVariable(t *testing.T) { prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err != nil { t.Fatalf("getting kubeconfig: %v", err) } @@ -186,14 +238,14 @@ func TestGetKubeconfigEnvVariable(t *testing.T) { } } -func TestGetKubeconfigDefault(t *testing.T) { - expectedPath := expandKubeconfigPath(defaultKubeconfigPath) +func TestGetKubeconfigPathDefault(t *testing.T) { + expectedPath := defaultKubeconfigPath k := &kubeconfigSources{} prepareKubeconfigSource(t, k) - kubeconfig, err := getKubeconfig() + kubeconfig, err := getKubeconfigPath() if err != nil { t.Fatalf("getting kubeconfig: %v", err) }