From 585af5a51641c411a3a19123109e2f89a6d92093 Mon Sep 17 00:00:00 2001 From: Matthew Fuller Date: Thu, 18 Mar 2021 20:01:40 +0000 Subject: [PATCH] Remove defaults from Config object at runtime This change fixes a bug where the default values that are populated during an `airshipctl config init` were getting mixed in with user specified values at runtime. For example, a user with two contexts defined in their airship config file would actually see a third phantom context with default values in the output of `airshipctl config get-context`. Also fixes a number of the config cmd and pkg unit tests that were affected by this bug. Change-Id: Iac591d4c2a616090028eb730576478edefb82544 Closes: #447 --- .../set_management_configuration_test.go | 8 ++++++++ cmd/config/set_manifest_test.go | 2 +- pkg/config/config.go | 2 +- pkg/config/config_test.go | 19 ++++++++++++++++--- pkg/config/utils.go | 10 ++++++++++ testutil/testconfig.go | 7 +++++-- 6 files changed, 41 insertions(+), 7 deletions(-) diff --git a/cmd/config/set_management_configuration_test.go b/cmd/config/set_management_configuration_test.go index 4e208f4e0..071a2e85e 100644 --- a/cmd/config/set_management_configuration_test.go +++ b/cmd/config/set_management_configuration_test.go @@ -72,6 +72,8 @@ func TestConfigSetManagementConfigurationInsecure(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.ManagementConfiguration[config.AirshipDefaultManagementConfiguration] = config.NewManagementConfiguration() + settings := func() (*config.Config, error) { return conf, nil } @@ -96,6 +98,8 @@ func TestConfigSetManagementConfigurationType(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.ManagementConfiguration[config.AirshipDefaultManagementConfiguration] = config.NewManagementConfiguration() + settings := func() (*config.Config, error) { return conf, nil } @@ -130,6 +134,8 @@ func TestConfigSetManagementConfigurationUseProxy(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.ManagementConfiguration[config.AirshipDefaultManagementConfiguration] = config.NewManagementConfiguration() + settings := func() (*config.Config, error) { return conf, nil } @@ -149,6 +155,8 @@ func TestConfigSetManagementConfigurationMultipleOptions(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.ManagementConfiguration[config.AirshipDefaultManagementConfiguration] = config.NewManagementConfiguration() + settings := func() (*config.Config, error) { return conf, nil } diff --git a/cmd/config/set_manifest_test.go b/cmd/config/set_manifest_test.go index 2e1725b7b..3d61a09d7 100644 --- a/cmd/config/set_manifest_test.go +++ b/cmd/config/set_manifest_test.go @@ -139,7 +139,7 @@ func (test setManifestTest) run(t *testing.T) { afterRunConf := test.inputConfig // Find the Manifest Created or Modified - afterRunManifest, _ := afterRunConf.Manifests[test.manifestName] + afterRunManifest := afterRunConf.Manifests[test.manifestName] require.NotNil(t, afterRunManifest) assert.EqualValues(t, afterRunManifest.TargetPath, test.manifestTargetPath) } diff --git a/pkg/config/config.go b/pkg/config/config.go index f53c0cc24..425741564 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -74,7 +74,7 @@ type Factory func() (*Config, error) // CreateFactory returns function which creates ready to use Config object func CreateFactory(airshipConfigPath *string) Factory { return func() (*Config, error) { - cfg := NewConfig() + cfg := NewEmptyConfig() var acp string if airshipConfigPath != nil { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 3d4daf991..03add579c 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -90,7 +90,7 @@ func TestLoadConfig(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) - assert.Len(t, conf.Contexts, 4) + assert.Len(t, conf.Contexts, 3) } func TestPersistConfig(t *testing.T) { @@ -183,13 +183,14 @@ func TestCurrentContextManagementConfig(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.ManagementConfiguration[defaultString] = testutil.DummyManagementConfiguration() + managementConfig, err := conf.CurrentContextManagementConfig() require.Error(t, err) assert.Nil(t, managementConfig) conf.CurrentContext = currentContextName conf.Contexts[currentContextName].ManagementConfiguration = defaultString - conf.Contexts[currentContextName].Manifest = defaultString managementConfig, err = conf.CurrentContextManagementConfig() require.NoError(t, err) @@ -234,7 +235,7 @@ func TestGetContexts(t *testing.T) { defer cleanup(t) contexts := conf.GetContexts() - assert.Len(t, contexts, 4) + assert.Len(t, contexts, 3) } func TestGetContext(t *testing.T) { @@ -292,6 +293,8 @@ func TestCurrentContextManifest(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.Manifests[defaultString] = testutil.DummyManifest() + conf.CurrentContext = currentContextName conf.Contexts[currentContextName].Manifest = defaultString @@ -304,6 +307,8 @@ func TestCurrentTargetPath(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.Manifests[defaultString] = testutil.DummyManifest() + conf.CurrentContext = currentContextName conf.Contexts[currentContextName].Manifest = defaultString @@ -316,6 +321,8 @@ func TestCurrentPhaseRepositoryDir(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.Manifests[defaultString] = testutil.DummyManifest() + conf.CurrentContext = currentContextName conf.Contexts[currentContextName].Manifest = defaultString @@ -336,6 +343,8 @@ func TestCurrentInventoryRepositoryDir(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.Manifests[defaultString] = testutil.DummyManifest() + conf.CurrentContext = currentContextName conf.Contexts[currentContextName].Manifest = defaultString @@ -454,6 +463,8 @@ func TestManagementConfigurationByName(t *testing.T) { conf, cleanupConfig := testutil.InitConfig(t) defer cleanupConfig(t) + conf.ManagementConfiguration[defaultString] = testutil.DummyManagementConfiguration() + mgmtCfg, err := conf.GetManagementConfiguration(config.AirshipDefaultContext) require.NoError(t, err) assert.Equal(t, conf.ManagementConfiguration[config.AirshipDefaultContext], mgmtCfg) @@ -471,6 +482,8 @@ func TestGetManifests(t *testing.T) { conf, cleanup := testutil.InitConfig(t) defer cleanup(t) + conf.Manifests["dummy_manifest"] = testutil.DummyManifest() + manifests := conf.GetManifests() require.NotNil(t, manifests) diff --git a/pkg/config/utils.go b/pkg/config/utils.go index b2e135db9..98b38ccfb 100644 --- a/pkg/config/utils.go +++ b/pkg/config/utils.go @@ -59,6 +59,16 @@ func NewConfig() *Config { } } +// NewEmptyConfig returns an initialized Config object with no default values +func NewEmptyConfig() *Config { + return &Config{ + ManagementConfiguration: map[string]*ManagementConfiguration{}, + Manifests: map[string]*Manifest{}, + Contexts: map[string]*Context{}, + fileSystem: fs.NewDocumentFs(), + } +} + // NewContext is a convenience function that returns a new Context func NewContext() *Context { return &Context{} diff --git a/testutil/testconfig.go b/testutil/testconfig.go index 2af1f8086..893d3b2e0 100644 --- a/testutil/testconfig.go +++ b/testutil/testconfig.go @@ -115,11 +115,14 @@ func InitConfig(t *testing.T) (conf *config.Config, cleanup func(*testing.T)) { err := ioutil.WriteFile(configPath, []byte(testConfigYAML), 0600) require.NoError(t, err) - conf = config.NewConfig() - cfg, err := config.CreateFactory(&configPath)() require.NoError(t, err) + cfg.Permissions = config.Permissions{ + DirectoryPermission: config.AirshipDefaultDirectoryPermission, + FilePermission: config.AirshipDefaultFilePermission, + } + return cfg, cleanup }