airship/airshipctl
Code Issues Proposed changes

Remove defaults from Config object at runtime

Browse Source 
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
This commit is contained in:
Matthew Fuller 2021-03-18 20:01:40 +00:00
parent 7f95156110
commit 585af5a516
6 changed files with 41 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
cmd/config/set_management_configuration_test.go
View File

@ -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
}

2
cmd/config/set_manifest_test.go
View File

@ -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)
}

2
pkg/config/config.go
View File

@ -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 {

19
pkg/config/config_test.go
View File

@ -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)

10
pkg/config/utils.go
View File

@ -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{}

7
testutil/testconfig.go
View File

@ -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
}