From 46f5c63ccdea1fccaac27efeac09716d46297887 Mon Sep 17 00:00:00 2001 From: Ian Howell Date: Mon, 17 Feb 2020 12:04:58 -0600 Subject: [PATCH] [#48] Make output from `config get-*` uniform This changes the output of the get-cluster, get-context, and get-credentials subcommands to be uniform across multiple runs of the program. This allows for predictable output for users and a controlled environment for testing. Change-Id: If87ec50b9d65750565a204514d6b647be64bb30d --- pkg/config/cmds.go | 37 ++++++---------- pkg/config/config.go | 88 +++++++++++++++++++-------------------- pkg/config/config_test.go | 13 +++--- 3 files changed, 60 insertions(+), 78 deletions(-) diff --git a/pkg/config/cmds.go b/pkg/config/cmds.go index 83c5b674a..26c1e0ec8 100644 --- a/pkg/config/cmds.go +++ b/pkg/config/cmds.go @@ -92,14 +92,14 @@ func (o *AuthInfoOptions) Validate() error { // runGetAuthInfo performs the execution of 'config get-credentials' sub command func RunGetAuthInfo(o *AuthInfoOptions, out io.Writer, airconfig *Config) error { if o.Name == "" { - return getAuthInfos(out, airconfig) + getAuthInfos(out, airconfig) + return nil } return getAuthInfo(o, out, airconfig) } func getAuthInfo(o *AuthInfoOptions, out io.Writer, airconfig *Config) error { - cName := o.Name - authinfo, err := airconfig.GetAuthInfo(cName) + authinfo, err := airconfig.GetAuthInfo(o.Name) if err != nil { return err } @@ -107,24 +107,21 @@ func getAuthInfo(o *AuthInfoOptions, out io.Writer, airconfig *Config) error { return nil } -func getAuthInfos(out io.Writer, airconfig *Config) error { - authinfos, err := airconfig.GetAuthInfos() - if err != nil { - return err - } +func getAuthInfos(out io.Writer, airconfig *Config) { + authinfos := airconfig.GetAuthInfos() if len(authinfos) == 0 { fmt.Fprintln(out, "No User credentials found in the configuration.") } for _, authinfo := range authinfos { fmt.Fprintln(out, authinfo) } - return nil } // runGetCluster performs the execution of 'config get-cluster' sub command func RunGetCluster(o *ClusterOptions, out io.Writer, airconfig *Config) error { if o.Name == "" { - return getClusters(out, airconfig) + getClusters(out, airconfig) + return nil } return getCluster(o.Name, o.ClusterType, out, airconfig) } @@ -139,26 +136,22 @@ func getCluster(cName, cType string, return nil } -func getClusters(out io.Writer, airconfig *Config) error { - clusters, err := airconfig.GetClusters() - if err != nil { - return err - } +func getClusters(out io.Writer, airconfig *Config) { + clusters := airconfig.GetClusters() if len(clusters) == 0 { fmt.Fprintln(out, "No clusters found in the configuration.") - return nil } for _, cluster := range clusters { fmt.Fprintf(out, "%s\n", cluster.PrettyString()) } - return nil } // runGetContext performs the execution of 'config get-Context' sub command func RunGetContext(o *ContextOptions, out io.Writer, airconfig *Config) error { if o.Name == "" && !o.CurrentContext { - return getContexts(out, airconfig) + getContexts(out, airconfig) + return nil } return getContext(o, out, airconfig) } @@ -176,18 +169,14 @@ func getContext(o *ContextOptions, out io.Writer, airconfig *Config) error { return nil } -func getContexts(out io.Writer, airconfig *Config) error { - contexts, err := airconfig.GetContexts() - if err != nil { - return err - } +func getContexts(out io.Writer, airconfig *Config) { + contexts := airconfig.GetContexts() if len(contexts) == 0 { fmt.Fprintln(out, "No Contexts found in the configuration.") } for _, context := range contexts { fmt.Fprintf(out, "%s", context.PrettyString()) } - return nil } func RunSetAuthInfo(o *AuthInfoOptions, airconfig *Config, writeToStorage bool) (bool, error) { diff --git a/pkg/config/config.go b/pkg/config/config.go index 6d9285895..18a2e72b9 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -398,24 +398,6 @@ func (c *Config) KubeConfig() *clientcmdapi.Config { return c.kubeConfig } -func (c *Config) ClusterNames() []string { - names := make([]string, 0, len(c.Clusters)) - for c := range c.Clusters { - names = append(names, c) - } - sort.Strings(names) - return names -} - -func (c *Config) ContextNames() []string { - names := make([]string, 0, len(c.Contexts)) - for c := range c.Contexts { - names = append(names, c) - } - sort.Strings(names) - return names -} - // Get A Cluster func (c *Config) GetCluster(cName, cType string) (*Cluster, error) { _, exists := c.Clusters[cName] @@ -498,20 +480,27 @@ func (c *Config) ModifyCluster(cluster *Cluster, theCluster *ClusterOptions) (*C return cluster, nil } -func (c *Config) GetClusters() ([]*Cluster, error) { - clusters := make([]*Cluster, 0, len(c.ClusterNames())) - for _, cName := range c.ClusterNames() { - for _, ctName := range AllClusterTypes { - cluster, err := c.GetCluster(cName, ctName) - // Err simple means something that does not exists - // Which is possible since I am iterating thorugh both possible - // cluster types - if err == nil { +// GetClusters returns all of the clusters associated with the Config sorted by name +func (c *Config) GetClusters() []*Cluster { + keys := make([]string, 0, len(c.Clusters)) + for name := range c.Clusters { + keys = append(keys, name) + } + sort.Strings(keys) + + clusters := make([]*Cluster, 0, len(c.Clusters)) + for _, name := range keys { + for _, clusterType := range AllClusterTypes { + cluster, exists := c.Clusters[name].ClusterTypes[clusterType] + if exists { + // If it doesn't exist, then there must not be + // a cluster with this name/type combination. + // This is expected behavior clusters = append(clusters, cluster) } } } - return clusters, nil + return clusters } // Context Operations from Config point of view @@ -524,17 +513,19 @@ func (c *Config) GetContext(cName string) (*Context, error) { return context, nil } -func (c *Config) GetContexts() ([]*Context, error) { - contexts := make([]*Context, 0, len(c.ContextNames())) - // Given that we change the testing metholdogy - // The ordered names are no longer required - for _, cName := range c.ContextNames() { - context, err := c.GetContext(cName) - if err == nil { - contexts = append(contexts, context) - } +// GetContexts returns all of the contexts associated with the Config sorted by name +func (c *Config) GetContexts() []*Context { + keys := make([]string, 0, len(c.Contexts)) + for name := range c.Contexts { + keys = append(keys, name) } - return contexts, nil + sort.Strings(keys) + + contexts := make([]*Context, 0, len(c.Contexts)) + for _, name := range keys { + contexts = append(contexts, c.Contexts[name]) + } + return contexts } func (c *Config) AddContext(theContext *ContextOptions) *Context { @@ -630,15 +621,20 @@ func (c *Config) GetAuthInfo(aiName string) (*AuthInfo, error) { return authinfo, nil } -func (c *Config) GetAuthInfos() ([]*AuthInfo, error) { - authinfos := make([]*AuthInfo, 0, len(c.AuthInfos)) - for cName := range c.AuthInfos { - authinfo, err := c.GetAuthInfo(cName) - if err == nil { - authinfos = append(authinfos, authinfo) - } +// GetAuthInfos returns a slice containing all the AuthInfos associated with +// the Config sorted by name +func (c *Config) GetAuthInfos() []*AuthInfo { + keys := make([]string, 0, len(c.AuthInfos)) + for name := range c.AuthInfos { + keys = append(keys, name) } - return authinfos, nil + sort.Strings(keys) + + authInfos := make([]*AuthInfo, 0, len(c.AuthInfos)) + for _, name := range keys { + authInfos = append(authInfos, c.AuthInfos[name]) + } + return authInfos } func (c *Config) AddAuthInfo(theAuthInfo *AuthInfoOptions) *AuthInfo { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 9b9d5687e..b9237457f 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -255,10 +255,10 @@ func TestPersistConfig(t *testing.T) { assert.Contains(t, cfg.KubeConfig().Clusters, "invalidName_target") // Check that the missing cluster was added to the airshipconfig - assert.Contains(t, cfg.ClusterNames(), "onlyinkubeconf") + assert.Contains(t, cfg.Clusters, "onlyinkubeconf") // Check that the "stragglers" were removed from the airshipconfig - assert.NotContains(t, cfg.ClusterNames(), "straggler") + assert.NotContains(t, cfg.Clusters, "straggler") } func TestEnsureComplete(t *testing.T) { @@ -497,15 +497,13 @@ func TestModifyCluster(t *testing.T) { func TestGetClusters(t *testing.T) { conf := config.InitConfig(t) - clusters, err := conf.GetClusters() - require.NoError(t, err) + clusters := conf.GetClusters() assert.Len(t, clusters, 5) } func TestGetContexts(t *testing.T) { conf := config.InitConfig(t) - contexts, err := conf.GetContexts() - require.NoError(t, err) + contexts := conf.GetContexts() assert.Len(t, contexts, 3) } @@ -551,8 +549,7 @@ func TestModifyContext(t *testing.T) { func TestGetAuthInfos(t *testing.T) { conf := config.InitConfig(t) - authinfos, err := conf.GetAuthInfos() - require.NoError(t, err) + authinfos := conf.GetAuthInfos() assert.Len(t, authinfos, 3) }