Skip to content

Commit

Permalink
Add fix for showconfig command
Browse files Browse the repository at this point in the history
This commit adds a fix where we create a copy of the top-level config map, instead
of using it directly. Previously this situation caused an infinite loop in the
map structure, so creating a copy prevents that from happening.
  • Loading branch information
LandonTClipp committed Nov 5, 2023
1 parent 77064ad commit 0310201
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 9 deletions.
10 changes: 6 additions & 4 deletions cmd/showconfig.go
Expand Up @@ -37,6 +37,11 @@ func showConfig(
if err != nil {
return stackerr.NewStackErrf(err, "failed to unmarshal config")
}
log, err := logging.GetLogger(config.LogLevel)
if err != nil {
return fmt.Errorf("getting logger: %w", err)
}
ctx = log.WithContext(ctx)
if err := config.Initialize(ctx); err != nil {
return err
}
Expand All @@ -48,10 +53,7 @@ func showConfig(
if err != nil {
return stackerr.NewStackErrf(err, "failed to marshal yaml")
}
log, err := logging.GetLogger(config.LogLevel)
if err != nil {
panic(err)
}

log.Info().Msgf("Using config: %s", config.Config)

fmt.Fprintf(outputter, "%s", string(out))
Expand Down
39 changes: 35 additions & 4 deletions pkg/config/config.go
Expand Up @@ -210,7 +210,11 @@ func (c *Config) getPackageConfigMap(ctx context.Context, packageName string) (m
return configAsMap, nil
}

return map[string]any{}, nil
// Package is something other than map, so set its value to an
// empty map.
emptyMap := map[string]any{}
packageSection[packageName] = emptyMap
return emptyMap, nil

}
func (c *Config) GetPackageConfig(ctx context.Context, packageName string) (*Config, error) {
Expand Down Expand Up @@ -659,6 +663,17 @@ func contains[T comparable](slice []T, elem T) bool {
return false
}

func deepCopyConfigMap(src map[string]any) map[string]any {
new := map[string]any{}

Check failure on line 667 in pkg/config/config.go

View workflow job for this annotation

GitHub Actions / lint

variable new has same name as predeclared identifier (predeclared)

Check failure on line 667 in pkg/config/config.go

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest, 1.20)

variable new has same name as predeclared identifier (predeclared)
for key, val := range src {
if contains([]string{"packages", "config", "interfaces"}, key) {
continue
}
new[key] = val
}
return new
}

// mergeInConfig takes care of merging inheritable configuration
// in the config map. For example, it merges default config, then
// package-level config, then interface-level config.
Expand All @@ -684,15 +699,28 @@ func (c *Config) mergeInConfig(ctx context.Context) error {
pkgLog.Err(err).Msg("failed to get package config")
return fmt.Errorf("failed to get package config: %w", err)
}
pkgLog.Trace().Msg("got package config map")

configSectionUntyped, configExists := packageConfig["config"]
if !configExists {
packageConfig["config"] = defaultCfg
continue
pkgLog.Trace().Msg("config section doesn't exist, setting it to a deepcopy of the top-level config")
packageConfig["config"] = deepCopyConfigMap(defaultCfg)
}

pkgLog.Trace().Msg("got config section for package")
// Sometimes the config section may be provided, but it's nil.
// We need to account for this fact.
if configSectionUntyped == nil {
configSectionUntyped = map[string]any{}
pkgLog.Trace().Msg("config section is nil, converting to empty map")
emptyMap := map[string]any{}

// We need to add this to the "global" config mapping so the change
// gets persisted, and also into configSectionUntyped for the logic
// further down.
packageConfig["config"] = emptyMap
configSectionUntyped = emptyMap
} else {
pkgLog.Trace().Msg("config section is not nil")
}

configSectionTyped := configSectionUntyped.(map[string]any)
Expand All @@ -701,8 +729,11 @@ func (c *Config) mergeInConfig(ctx context.Context) error {
if contains([]string{"packages", "config"}, key) {
continue
}
keyValLog := pkgLog.With().Str("key", key).Str("value", fmt.Sprintf("%v", value)).Logger()

_, keyExists := configSectionTyped[key]
if !keyExists {
keyValLog.Trace().Msg("setting key to value")
configSectionTyped[key] = value
}
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/config/config_test.go
Expand Up @@ -1204,6 +1204,7 @@ with-expecter: false
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
tmpdir := pathlib.NewPath(t.TempDir())
cfg := tmpdir.Join("config.yaml")
require.NoError(t, cfg.WriteFile([]byte(tt.cfgYaml)))
Expand All @@ -1218,7 +1219,10 @@ with-expecter: false
t.Errorf("Config.Initialize() error = %v, wantErr %v", err, tt.wantErr)
}

cfgAsStr, err := yaml.Marshal(c._cfgAsMap)
cfgAsMap, err := c.CfgAsMap(ctx)
require.NoError(t, err)

cfgAsStr, err := yaml.Marshal(cfgAsMap)
require.NoError(t, err)

if tt.wantCfgMap != "" && !reflect.DeepEqual(string(cfgAsStr), tt.wantCfgMap) {
Expand Down

0 comments on commit 0310201

Please sign in to comment.