From e48b436680858ac315d7efc1473c3e45bb4579f3 Mon Sep 17 00:00:00 2001 From: Ian Howell Date: Thu, 7 Nov 2019 10:28:59 -0600 Subject: [PATCH] This commit prevents NewConfig from returning errors Prior to this commit, NewConfig was setting up a new Config object and filling it with a new kubeconfig object. The process for creating a kubeconfig object has the potential to return an error. This commit removes the creation of that object from NewConfig, delegating creation of kubeconfig objects to more appropriate functions, such as LoadConfig. Change-Id: I57a040f2e76bbc003eb82171f382e80425b37870 --- pkg/config/config.go | 4 +--- pkg/config/test_utils.go | 37 +++++++++++++++++++++++++------------ pkg/config/utils.go | 31 +++++++++---------------------- pkg/environment/settings.go | 11 ++++++----- 4 files changed, 41 insertions(+), 42 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 1a1b88599..845eb22fe 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -37,7 +37,6 @@ import ( // Called from root to Load the initial configuration func (c *Config) LoadConfig(configFileArg string, kPathOptions *clientcmd.PathOptions) error { - err := c.loadFromAirConfig(configFileArg) if err != nil { return err @@ -49,9 +48,8 @@ func (c *Config) LoadConfig(configFileArg string, kPathOptions *clientcmd.PathOp return err } - // Lets navigate through the kConfig to populate the references in airship config + // Lets navigate through the kubeconfig to populate the references in airship config return c.reconcileConfig() - } func (c *Config) loadFromAirConfig(configFileArg string) error { diff --git a/pkg/config/test_utils.go b/pkg/config/test_utils.go index 27635b031..8b48bce70 100644 --- a/pkg/config/test_utils.go +++ b/pkg/config/test_utils.go @@ -37,18 +37,28 @@ const ( // DummyConfig used by tests, to initialize min set of data func DummyConfig() *Config { - conf := NewConfig() - // Make sure the .airship directory is created - //conf.ConfigFilePath() - conf.Clusters["dummy_cluster"] = DummyClusterPurpose() - conf.KubeConfig().Clusters["dummycluster_target"] = conf.Clusters["dummy_cluster"].ClusterTypes[Target].KubeCluster() - conf.KubeConfig().Clusters["dummycluster_ephemeral"] = - conf.Clusters["dummy_cluster"].ClusterTypes[Ephemeral].KubeCluster() - conf.AuthInfos["dummy_user"] = DummyAuthInfo() - conf.Contexts["dummy_context"] = DummyContext() - conf.Manifests["dummy_manifest"] = DummyManifest() - conf.ModulesConfig = DummyModules() - conf.CurrentContext = "dummy_context" + conf := &Config{ + Kind: AirshipConfigKind, + APIVersion: AirshipConfigApiVersion, + Clusters: map[string]*ClusterPurpose{ + "dummy_cluster": DummyClusterPurpose(), + }, + AuthInfos: map[string]*AuthInfo{ + "dummy_user": DummyAuthInfo(), + }, + Contexts: map[string]*Context{ + "dummy_context": DummyContext(), + }, + Manifests: map[string]*Manifest{ + "dummy_manifest": DummyManifest(), + }, + ModulesConfig: DummyModules(), + CurrentContext: "dummy_context", + kubeConfig: kubeconfig.NewConfig(), + } + dummyCluster := conf.Clusters["dummy_cluster"] + conf.KubeConfig().Clusters["dummycluster_target"] = dummyCluster.ClusterTypes[Target].KubeCluster() + conf.KubeConfig().Clusters["dummycluster_ephemeral"] = dummyCluster.ClusterTypes[Ephemeral].KubeCluster() return conf } @@ -111,10 +121,13 @@ func DummyClusterPurpose() *ClusterPurpose { func InitConfigAt(t *testing.T, airConfigFile, kConfigFile string) *Config { conf := NewConfig() + kubePathOptions := clientcmd.NewDefaultPathOptions() kubePathOptions.GlobalFile = kConfigFile + err := conf.LoadConfig(airConfigFile, kubePathOptions) require.NoError(t, err) + return conf } func InitConfig(t *testing.T) *Config { diff --git a/pkg/config/utils.go b/pkg/config/utils.go index 626ce8a00..5b886bcec 100644 --- a/pkg/config/utils.go +++ b/pkg/config/utils.go @@ -16,30 +16,17 @@ limitations under the License. package config -import ( - "path/filepath" - - "k8s.io/client-go/tools/clientcmd" -) - -// NewConfig is a convenience function that returns a new Config -// object with non-nill maps +// NewConfig returns a newly initialized Config object func NewConfig() *Config { - conf := &Config{ - Clusters: make(map[string]*ClusterPurpose), - Contexts: make(map[string]*Context), - AuthInfos: make(map[string]*AuthInfo), - Manifests: make(map[string]*Manifest), + return &Config{ + Kind: AirshipConfigKind, + APIVersion: AirshipConfigApiVersion, + Clusters: make(map[string]*ClusterPurpose), + Contexts: make(map[string]*Context), + AuthInfos: make(map[string]*AuthInfo), + Manifests: make(map[string]*Manifest), + ModulesConfig: NewModules(), } - conf.ModulesConfig = NewModules() - conf.Kind = AirshipConfigKind - conf.APIVersion = AirshipConfigApiVersion - - conf.loadedConfigPath = filepath.Join(AirshipConfigDir, AirshipConfig) - conf.loadedPathOptions = clientcmd.NewDefaultPathOptions() - conf.kubeConfig, _ = conf.loadedPathOptions.GetStartingConfig() - return conf - } // NewContext is a convenience function that returns a new Context diff --git a/pkg/environment/settings.go b/pkg/environment/settings.go index 5db9872b2..eafcd00ab 100644 --- a/pkg/environment/settings.go +++ b/pkg/environment/settings.go @@ -34,12 +34,12 @@ func (a *AirshipCTLSettings) InitFlags(cmd *cobra.Command) { flags.StringVar(&a.kubeConfigPath, clientcmd.RecommendedConfigPathFlag, filepath.Join(HomePlaceholder, config.AirshipConfigDir, config.AirshipKubeConfig), "Path to kubeconfig associated with airshipctl configuration.") - } func (a *AirshipCTLSettings) Config() *config.Config { return a.config } + func (a *AirshipCTLSettings) SetConfig(conf *config.Config) { a.config = conf } @@ -47,30 +47,31 @@ func (a *AirshipCTLSettings) SetConfig(conf *config.Config) { func (a *AirshipCTLSettings) AirshipConfigPath() string { return a.airshipConfigPath } + func (a *AirshipCTLSettings) SetAirshipConfigPath(acp string) { a.airshipConfigPath = acp } + func (a *AirshipCTLSettings) KubeConfigPath() string { return a.kubeConfigPath } + func (a *AirshipCTLSettings) SetKubeConfigPath(kcp string) { a.kubeConfigPath = kcp } // InitConfig - Initializes and loads Config it exists. func (a *AirshipCTLSettings) InitConfig() { - - // Raw - Empty Config object a.SetConfig(config.NewConfig()) a.setAirshipConfigPath() + //Pass the airshipConfigPath and kubeConfig object err := a.Config().LoadConfig(a.AirshipConfigPath(), a.setKubePathOptions()) if err != nil { // Should stop airshipctl log.Fatal(err) } - } func (a *AirshipCTLSettings) setAirshipConfigPath() { @@ -113,8 +114,8 @@ func (a *AirshipCTLSettings) setKubePathOptions() *clientcmd.PathOptions { kubePathOptions.GlobalFileSubpath = a.kubeConfigPath return kubePathOptions - } + func (a *AirshipCTLSettings) replaceHomePlaceholder(configPath string) (string, string) { home, err := os.UserHomeDir() if err != nil {