From bb3daf17959ca100db40587dcd5dcffa3c40ab08 Mon Sep 17 00:00:00 2001 From: Vladislav Kuzmin Date: Wed, 22 Apr 2020 22:52:12 +0400 Subject: [PATCH] Improve config package organization pt.1 This is a series of patches that refactor config.go into several smaller modules, each relating specifically to one component of the configuration structure. This particular patch split auth info and cluster related part in separate modules. Relates-To: #35 Change-Id: Ib2abebc87c80549544a8b7775969df9db55aa8be --- pkg/config/authinfo.go | 44 ++++++++++++ pkg/config/authinfo_test.go | 79 +++++++++++++++++++++ pkg/config/cluster.go | 137 ++++++++++++++++++++++++++++++++++++ pkg/config/cluster_test.go | 72 +++++++++++++++++++ pkg/config/config.go | 57 --------------- pkg/config/config_test.go | 100 -------------------------- pkg/config/types.go | 33 --------- pkg/config/utils.go | 47 ------------- 8 files changed, 332 insertions(+), 237 deletions(-) create mode 100644 pkg/config/authinfo.go create mode 100644 pkg/config/authinfo_test.go create mode 100644 pkg/config/cluster.go create mode 100644 pkg/config/cluster_test.go diff --git a/pkg/config/authinfo.go b/pkg/config/authinfo.go new file mode 100644 index 000000000..c28088e93 --- /dev/null +++ b/pkg/config/authinfo.go @@ -0,0 +1,44 @@ +/* +Copyright 2014 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 config + +import ( + "k8s.io/client-go/tools/clientcmd/api" + "sigs.k8s.io/yaml" +) + +type AuthInfo struct { + // KubeConfig AuthInfo Object + authInfo *api.AuthInfo +} + +// AuthInfo functions +func (c *AuthInfo) String() string { + kauthinfo := c.KubeAuthInfo() + kyaml, err := yaml.Marshal(&kauthinfo) + if err != nil { + return "" + } + return string(kyaml) +} + +func (c *AuthInfo) KubeAuthInfo() *api.AuthInfo { + return c.authInfo +} +func (c *AuthInfo) SetKubeAuthInfo(kc *api.AuthInfo) { + c.authInfo = kc +} diff --git a/pkg/config/authinfo_test.go b/pkg/config/authinfo_test.go new file mode 100644 index 000000000..d3df3b317 --- /dev/null +++ b/pkg/config/authinfo_test.go @@ -0,0 +1,79 @@ +/* +Copyright 2014 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 config_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "opendev.org/airship/airshipctl/testutil" +) + +func TestGetAuthInfos(t *testing.T) { + conf, cleanup := testutil.InitConfig(t) + defer cleanup(t) + + authinfos := conf.GetAuthInfos() + assert.Len(t, authinfos, 3) +} + +func TestGetAuthInfo(t *testing.T) { + conf, cleanup := testutil.InitConfig(t) + defer cleanup(t) + + authinfo, err := conf.GetAuthInfo("def-user") + require.NoError(t, err) + + // Test Positives + assert.EqualValues(t, authinfo.KubeAuthInfo().Username, "dummy_username") + + // Test Wrong Cluster + _, err = conf.GetAuthInfo("unknown") + assert.Error(t, err) +} + +func TestAddAuthInfo(t *testing.T) { + conf, cleanup := testutil.InitConfig(t) + defer cleanup(t) + + co := testutil.DummyAuthInfoOptions() + authinfo := conf.AddAuthInfo(co) + assert.EqualValues(t, conf.AuthInfos[co.Name], authinfo) +} + +func TestModifyAuthInfo(t *testing.T) { + conf, cleanup := testutil.InitConfig(t) + defer cleanup(t) + + co := testutil.DummyAuthInfoOptions() + authinfo := conf.AddAuthInfo(co) + + co.Username += stringDelta + co.Password += stringDelta + co.ClientCertificate += stringDelta + co.ClientKey += stringDelta + co.Token += stringDelta + conf.ModifyAuthInfo(authinfo, co) + assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().Username, co.Username) + assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().Password, co.Password) + assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().ClientCertificate, co.ClientCertificate) + assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().ClientKey, co.ClientKey) + assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().Token, co.Token) + assert.EqualValues(t, conf.AuthInfos[co.Name], authinfo) +} diff --git a/pkg/config/cluster.go b/pkg/config/cluster.go new file mode 100644 index 000000000..045cdb9fe --- /dev/null +++ b/pkg/config/cluster.go @@ -0,0 +1,137 @@ +/* +Copyright 2014 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 config + +import ( + "fmt" + "strings" + + "k8s.io/client-go/tools/clientcmd/api" + "sigs.k8s.io/yaml" +) + +// Cluster contains information about how to communicate with a kubernetes cluster +type Cluster struct { + // Complex cluster name defined by the using _) + NameInKubeconf string `json:"clusterKubeconf"` + + // KubeConfig Cluster Object + cluster *api.Cluster + + // Management configuration which will be used for all hosts in the cluster + ManagementConfiguration string `json:"managementConfiguration"` + + // Bootstrap configuration this clusters ephemeral hosts will rely on + Bootstrap string `json:"bootstrapInfo"` +} + +// ClusterPurpose encapsulates the Cluster Type as an enumeration +type ClusterPurpose struct { + // Cluster map of referenceable names to cluster configs + ClusterTypes map[string]*Cluster `json:"clusterType"` +} + +// ClusterComplexName holds the complex cluster name information +// Encapsulates the different operations around using it. +type ClusterComplexName struct { + Name string + Type string +} + +func (c *Cluster) String() string { + cyaml, err := yaml.Marshal(&c) + if err != nil { + return "" + } + kcluster := c.KubeCluster() + kyaml, err := yaml.Marshal(&kcluster) + if err != nil { + return string(cyaml) + } + + return fmt.Sprintf("%s\n%s", string(cyaml), string(kyaml)) +} + +func (c *Cluster) PrettyString() string { + clusterName := NewClusterComplexNameFromKubeClusterName(c.NameInKubeconf) + return fmt.Sprintf("Cluster: %s\n%s:\n%s", clusterName.Name, clusterName.Type, c) +} + +func (c *Cluster) KubeCluster() *api.Cluster { + return c.cluster +} +func (c *Cluster) SetKubeCluster(kc *api.Cluster) { + c.cluster = kc +} + +func (c *ClusterComplexName) String() string { + return strings.Join([]string{c.Name, c.Type}, AirshipClusterNameSeparator) +} + +func ValidClusterType(clusterType string) error { + for _, validType := range AllClusterTypes { + if clusterType == validType { + return nil + } + } + return fmt.Errorf("cluster type must be one of %v", AllClusterTypes) +} + +// NewClusterPurpose is a convenience function that returns a new ClusterPurpose +func NewClusterPurpose() *ClusterPurpose { + return &ClusterPurpose{ + ClusterTypes: make(map[string]*Cluster), + } +} + +// NewClusterComplexName returns a ClusterComplexName with the given name and type. +func NewClusterComplexName(clusterName, clusterType string) ClusterComplexName { + return ClusterComplexName{ + Name: clusterName, + Type: clusterType, + } +} + +// NewClusterComplexNameFromKubeClusterName takes the name of a cluster in a +// format which might be found in a kubeconfig file. This may be a simple +// string (e.g. myCluster), or it may be prepended with the type of the cluster +// (e.g. myCluster_target) +// +// If a valid cluster type was appended, the returned ClusterComplexName will +// have that type. If no cluster type is provided, the +// AirshipDefaultClusterType will be used. +func NewClusterComplexNameFromKubeClusterName(kubeClusterName string) ClusterComplexName { + parts := strings.Split(kubeClusterName, AirshipClusterNameSeparator) + + if len(parts) == 1 { + return NewClusterComplexName(kubeClusterName, AirshipDefaultClusterType) + } + + // kubeClusterName matches the format myCluster_something. + // Let's check if "something" is a clusterType. + potentialType := parts[len(parts)-1] + for _, ct := range AllClusterTypes { + if potentialType == ct { + // Rejoin the parts in the case of "my_cluster_etc_etc_" + name := strings.Join(parts[:len(parts)-1], AirshipClusterNameSeparator) + return NewClusterComplexName(name, potentialType) + } + } + + // "something" is not a valid clusterType, so just use the default + return NewClusterComplexName(kubeClusterName, AirshipDefaultClusterType) +} diff --git a/pkg/config/cluster_test.go b/pkg/config/cluster_test.go new file mode 100644 index 000000000..805532b47 --- /dev/null +++ b/pkg/config/cluster_test.go @@ -0,0 +1,72 @@ +/* +Copyright 2014 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 config_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "opendev.org/airship/airshipctl/pkg/config" + "opendev.org/airship/airshipctl/testutil" +) + +func TestPrettyString(t *testing.T) { + fSys := testutil.SetupTestFs(t, "testdata") + data, err := fSys.ReadFile("/prettycluster-string.yaml") + require.NoError(t, err) + + cluster := testutil.DummyCluster() + assert.EqualValues(t, cluster.PrettyString(), string(data)) +} + +func TestValidClusterTypeFail(t *testing.T) { + err := config.ValidClusterType("Fake") + assert.Error(t, err) +} + +func TestGetCluster(t *testing.T) { + conf, cleanup := testutil.InitConfig(t) + defer cleanup(t) + + cluster, err := conf.GetCluster("def", config.Ephemeral) + require.NoError(t, err) + + // Test Positives + assert.EqualValues(t, cluster.NameInKubeconf, "def_ephemeral") + assert.EqualValues(t, cluster.KubeCluster().Server, "http://5.6.7.8") + + // Test Wrong Cluster + _, err = conf.GetCluster("unknown", config.Ephemeral) + assert.Error(t, err) + + // Test Wrong Cluster Type + _, err = conf.GetCluster("def", "Unknown") + assert.Error(t, err) +} + +func TestAddCluster(t *testing.T) { + conf, cleanup := testutil.InitConfig(t) + defer cleanup(t) + + co := testutil.DummyClusterOptions() + cluster, err := conf.AddCluster(co) + require.NoError(t, err) + + assert.EqualValues(t, conf.Clusters[co.Name].ClusterTypes[co.ClusterType], cluster) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 1e5fb2ba3..493c1fffe 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -24,7 +24,6 @@ import ( "path" "path/filepath" "sort" - "strings" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -769,32 +768,6 @@ func (c *Config) Purge() error { return os.Remove(c.loadedConfigPath) } -func (c *Cluster) String() string { - cyaml, err := yaml.Marshal(&c) - if err != nil { - return "" - } - kcluster := c.KubeCluster() - kyaml, err := yaml.Marshal(&kcluster) - if err != nil { - return string(cyaml) - } - - return fmt.Sprintf("%s\n%s", string(cyaml), string(kyaml)) -} - -func (c *Cluster) PrettyString() string { - clusterName := NewClusterComplexNameFromKubeClusterName(c.NameInKubeconf) - return fmt.Sprintf("Cluster: %s\n%s:\n%s", clusterName.Name, clusterName.Type, c) -} - -func (c *Cluster) KubeCluster() *clientcmdapi.Cluster { - return c.cluster -} -func (c *Cluster) SetKubeCluster(kc *clientcmdapi.Cluster) { - c.cluster = kc -} - // Context functions func (c *Context) String() string { cyaml, err := yaml.Marshal(&c) @@ -830,23 +803,6 @@ func (c *Context) ClusterName() string { return NewClusterComplexNameFromKubeClusterName(c.NameInKubeconf).Name } -// AuthInfo functions -func (c *AuthInfo) String() string { - kauthinfo := c.KubeAuthInfo() - kyaml, err := yaml.Marshal(&kauthinfo) - if err != nil { - return "" - } - return string(kyaml) -} - -func (c *AuthInfo) KubeAuthInfo() *clientcmdapi.AuthInfo { - return c.authInfo -} -func (c *AuthInfo) SetKubeAuthInfo(kc *clientcmdapi.AuthInfo) { - c.authInfo = kc -} - // Manifest functions func (m *Manifest) String() string { yamlData, err := yaml.Marshal(&m) @@ -891,16 +847,3 @@ func (b *Builder) String() string { } return string(yamlData) } - -func (c *ClusterComplexName) String() string { - return strings.Join([]string{c.Name, c.Type}, AirshipClusterNameSeparator) -} - -func ValidClusterType(clusterType string) error { - for _, validType := range AllClusterTypes { - if clusterType == validType { - return nil - } - } - return fmt.Errorf("cluster type must be one of %v", AllClusterTypes) -} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 536bfc803..b4241dbe9 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -111,15 +111,6 @@ func TestString(t *testing.T) { } } -func TestPrettyString(t *testing.T) { - fSys := testutil.SetupTestFs(t, "testdata") - data, err := fSys.ReadFile("/prettycluster-string.yaml") - require.NoError(t, err) - - cluster := testutil.DummyCluster() - assert.EqualValues(t, cluster.PrettyString(), string(data)) -} - func TestLoadConfig(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) @@ -303,11 +294,6 @@ func TestPurge(t *testing.T) { assert.Falsef(t, os.IsExist(err), "Purge failed to remove file at %v", conf.LoadedConfigPath()) } -func TestValidClusterTypeFail(t *testing.T) { - err := config.ValidClusterType("Fake") - assert.Error(t, err) -} - func TestSetLoadedConfigPath(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) @@ -330,37 +316,6 @@ func TestSetKubeConfigPath(t *testing.T) { assert.Equal(t, testPath, conf.KubeConfigPath()) } -func TestGetCluster(t *testing.T) { - conf, cleanup := testutil.InitConfig(t) - defer cleanup(t) - - cluster, err := conf.GetCluster("def", config.Ephemeral) - require.NoError(t, err) - - // Test Positives - assert.EqualValues(t, cluster.NameInKubeconf, "def_ephemeral") - assert.EqualValues(t, cluster.KubeCluster().Server, "http://5.6.7.8") - - // Test Wrong Cluster - _, err = conf.GetCluster("unknown", config.Ephemeral) - assert.Error(t, err) - - // Test Wrong Cluster Type - _, err = conf.GetCluster("def", "Unknown") - assert.Error(t, err) -} - -func TestAddCluster(t *testing.T) { - conf, cleanup := testutil.InitConfig(t) - defer cleanup(t) - - co := testutil.DummyClusterOptions() - cluster, err := conf.AddCluster(co) - require.NoError(t, err) - - assert.EqualValues(t, conf.Clusters[co.Name].ClusterTypes[co.ClusterType], cluster) -} - func TestModifyCluster(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) @@ -587,61 +542,6 @@ func TestCurrentContextClusterName(t *testing.T) { assert.Equal(t, expectedClusterName, actualClusterName) } -// AuthInfo Related - -func TestGetAuthInfos(t *testing.T) { - conf, cleanup := testutil.InitConfig(t) - defer cleanup(t) - - authinfos := conf.GetAuthInfos() - assert.Len(t, authinfos, 3) -} - -func TestGetAuthInfo(t *testing.T) { - conf, cleanup := testutil.InitConfig(t) - defer cleanup(t) - - authinfo, err := conf.GetAuthInfo("def-user") - require.NoError(t, err) - - // Test Positives - assert.EqualValues(t, authinfo.KubeAuthInfo().Username, "dummy_username") - - // Test Wrong Cluster - _, err = conf.GetAuthInfo("unknown") - assert.Error(t, err) -} - -func TestAddAuthInfo(t *testing.T) { - conf, cleanup := testutil.InitConfig(t) - defer cleanup(t) - - co := testutil.DummyAuthInfoOptions() - authinfo := conf.AddAuthInfo(co) - assert.EqualValues(t, conf.AuthInfos[co.Name], authinfo) -} - -func TestModifyAuthInfo(t *testing.T) { - conf, cleanup := testutil.InitConfig(t) - defer cleanup(t) - - co := testutil.DummyAuthInfoOptions() - authinfo := conf.AddAuthInfo(co) - - co.Username += stringDelta - co.Password += stringDelta - co.ClientCertificate += stringDelta - co.ClientKey += stringDelta - co.Token += stringDelta - conf.ModifyAuthInfo(authinfo, co) - assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().Username, co.Username) - assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().Password, co.Password) - assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().ClientCertificate, co.ClientCertificate) - assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().ClientKey, co.ClientKey) - assert.EqualValues(t, conf.AuthInfos[co.Name].KubeAuthInfo().Token, co.Token) - assert.EqualValues(t, conf.AuthInfos[co.Name], authinfo) -} - func TestNewClusterComplexNameFromKubeClusterName(t *testing.T) { tests := []struct { name string diff --git a/pkg/config/types.go b/pkg/config/types.go index 587ba7eed..aff3d1470 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -69,27 +69,6 @@ type Config struct { kubeConfig *kubeconfig.Config } -// ClusterPurpose encapsulates the Cluster Type as an enumeration -type ClusterPurpose struct { - // Cluster map of referenceable names to cluster configs - ClusterTypes map[string]*Cluster `json:"clusterType"` -} - -// Cluster contains information about how to communicate with a kubernetes cluster -type Cluster struct { - // Complex cluster name defined by the using _) - NameInKubeconf string `json:"clusterKubeconf"` - - // KubeConfig Cluster Object - cluster *kubeconfig.Cluster - - // Management configuration which will be used for all hosts in the cluster - ManagementConfiguration string `json:"managementConfiguration"` - - // Bootstrap configuration this clusters ephemeral hosts will rely on - Bootstrap string `json:"bootstrapInfo"` -} - // Context is a tuple of references to a cluster (how do I communicate with a kubernetes context), // a user (how do I identify myself), and a namespace (what subset of resources do I want to work with) type Context struct { @@ -104,11 +83,6 @@ type Context struct { context *kubeconfig.Context } -type AuthInfo struct { - // KubeConfig AuthInfo Object - authInfo *kubeconfig.AuthInfo -} - // Manifest is a tuple of references to a Manifest (how do Identify, collect , // find the yaml manifests that airship uses to perform its operations) type Manifest struct { @@ -175,13 +149,6 @@ type RepoCheckout struct { ForceCheckout bool `json:"force"` } -// ClusterComplexName holds the complex cluster name information -// Encapsulates the different operations around using it. -type ClusterComplexName struct { - Name string - Type string -} - // ManagementConfiguration defines configuration data for all remote systems within a context. type ManagementConfiguration struct { // Insecure indicates whether the SSL certificate should be checked on remote management requests. diff --git a/pkg/config/utils.go b/pkg/config/utils.go index 545afa5f1..ed69b9a28 100644 --- a/pkg/config/utils.go +++ b/pkg/config/utils.go @@ -17,8 +17,6 @@ limitations under the License. package config import ( - "strings" - "opendev.org/airship/airshipctl/pkg/remote/redfish" ) @@ -109,48 +107,3 @@ func NewRepository() *Repository { func NewAuthInfo() *AuthInfo { return &AuthInfo{} } - -// NewClusterPurpose is a convenience function that returns a new ClusterPurpose -func NewClusterPurpose() *ClusterPurpose { - return &ClusterPurpose{ - ClusterTypes: make(map[string]*Cluster), - } -} - -// NewClusterComplexName returns a ClusterComplexName with the given name and type. -func NewClusterComplexName(clusterName, clusterType string) ClusterComplexName { - return ClusterComplexName{ - Name: clusterName, - Type: clusterType, - } -} - -// NewClusterComplexNameFromKubeClusterName takes the name of a cluster in a -// format which might be found in a kubeconfig file. This may be a simple -// string (e.g. myCluster), or it may be prepended with the type of the cluster -// (e.g. myCluster_target) -// -// If a valid cluster type was appended, the returned ClusterComplexName will -// have that type. If no cluster type is provided, the -// AirshipDefaultClusterType will be used. -func NewClusterComplexNameFromKubeClusterName(kubeClusterName string) ClusterComplexName { - parts := strings.Split(kubeClusterName, AirshipClusterNameSeparator) - - if len(parts) == 1 { - return NewClusterComplexName(kubeClusterName, AirshipDefaultClusterType) - } - - // kubeClusterName matches the format myCluster_something. - // Let's check if "something" is a clusterType. - potentialType := parts[len(parts)-1] - for _, ct := range AllClusterTypes { - if potentialType == ct { - // Rejoin the parts in the case of "my_cluster_etc_etc_" - name := strings.Join(parts[:len(parts)-1], AirshipClusterNameSeparator) - return NewClusterComplexName(name, potentialType) - } - } - - // "something" is not a valid clusterType, so just use the default - return NewClusterComplexName(kubeClusterName, AirshipDefaultClusterType) -}