Skip to content

Commit

Permalink
Fix bug with sub-package inheritance
Browse files Browse the repository at this point in the history
Subpackages were not correctly inheriting their parent package
configuration.
  • Loading branch information
LandonTClipp committed Nov 6, 2023
1 parent e86d230 commit d3515d1
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 12 deletions.
41 changes: 29 additions & 12 deletions pkg/config/config.go
Expand Up @@ -195,7 +195,14 @@ func (c *Config) GetPackages(ctx context.Context) ([]string, error) {
return packageList, nil
}

// getPackageConfigMap returns the map for the particular package, which includes
// (but is not limited to) both the `configs` section and the `interfaces` section.
// Note this does NOT return the `configs` section for the package. It returns the
// entire mapping for the package.
func (c *Config) getPackageConfigMap(ctx context.Context, packageName string) (map[string]any, error) {
log := zerolog.Ctx(ctx)
log.Trace().Msg("getting package config map")

cfgMap, err := c.CfgAsMap(ctx)
if err != nil {
return nil, err
Expand All @@ -207,8 +214,10 @@ func (c *Config) getPackageConfigMap(ctx context.Context, packageName string) (m
}
configAsMap, isMap := configUnmerged.(map[string]any)
if isMap {
log.Trace().Msg("package's value is a map, returning")
return configAsMap, nil
}
log.Trace().Msg("package's value is not a map")

// Package is something other than map, so set its value to an
// empty map.
Expand Down Expand Up @@ -248,7 +257,7 @@ func (c *Config) GetPackageConfig(ctx context.Context, packageName string) (*Con
configSection, ok := configMap["config"]
if !ok {
log.Debug().Msg("config section not provided for package")
configMap["config"] = deepCopyConfigMap(c._cfgAsMap)
configMap["config"] = map[string]any{}
c.pkgConfigCache[packageName] = pkgConfig
return pkgConfig, nil
}
Expand Down Expand Up @@ -445,6 +454,7 @@ func (c *Config) addSubPkgConfig(ctx context.Context, subPkgPath string, parentP
log := zerolog.Ctx(ctx).With().
Str("parent-package", parentPkgPath).
Str("sub-package", subPkgPath).Logger()
ctx = log.WithContext(ctx)

log.Debug().Msg("adding sub-package to config map")
parentPkgConfig, err := c.getPackageConfigMap(ctx, parentPkgPath)
Expand All @@ -463,13 +473,14 @@ func (c *Config) addSubPkgConfig(ctx context.Context, subPkgPath string, parentP
log.Debug().Msg("getting packages section")
packagesSection := topLevelConfig["packages"].(map[string]any)

// Don't overwrite any config that already exists
_, pkgExists := packagesSection[subPkgPath]
if !pkgExists {
log.Trace().Msg("sub-package doesn't exist in config")

// Copy the parent package directly into the subpackage config section
packagesSection[subPkgPath] = map[string]any{}
newPkgSection := packagesSection[subPkgPath].(map[string]any)
newPkgSection["config"] = parentPkgConfig["config"]
newPkgSection["config"] = deepCopyConfigMap(parentPkgConfig["config"].(map[string]any))
} else {
log.Trace().Msg("sub-package exists in config")
// The sub-package exists in config. Check if it has its
Expand All @@ -481,10 +492,15 @@ func (c *Config) addSubPkgConfig(ctx context.Context, subPkgPath string, parentP
log.Err(err).Msg("could not get child package config")
return fmt.Errorf("failed to get sub-package config: %w", err)
}

for key, val := range parentPkgConfig {
if _, keyInSubPkg := subPkgConfig[key]; !keyInSubPkg {
subPkgConfig[key] = val
log.Trace().Msgf("sub-package config: %v", subPkgConfig)
log.Trace().Msgf("parent-package config: %v", parentPkgConfig)

// Merge the parent config with the sub-package config.
parentConfigSection := parentPkgConfig["config"].(map[string]any)
subPkgConfigSection := subPkgConfig["config"].(map[string]any)
for key, val := range parentConfigSection {
if _, keyInSubPkg := subPkgConfigSection[key]; !keyInSubPkg {
subPkgConfigSection[key] = val
}

}
Expand Down Expand Up @@ -629,7 +645,6 @@ func (c *Config) discoverRecursivePackages(ctx context.Context) error {
recursivePackages[pkg] = pkgConfig
} else {
pkgLog.Trace().Msg("package not marked as recursive")
pkgLog.Trace().Msg(fmt.Sprintf("%+v", pkgConfig))
}
}
if len(recursivePackages) == 0 {
Expand Down Expand Up @@ -711,13 +726,15 @@ func (c *Config) mergeInConfig(ctx context.Context) error {
}
for _, pkgPath := range pkgs {
pkgLog := log.With().Str("package-path", pkgPath).Logger()
pkgCtx := pkgLog.WithContext(ctx)

pkgLog.Trace().Msg("merging for package")
packageConfig, err := c.getPackageConfigMap(ctx, pkgPath)
packageConfig, err := c.getPackageConfigMap(pkgCtx, pkgPath)
if err != nil {
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")
pkgLog.Trace().Msgf("got package config map: %v", packageConfig)

configSectionUntyped, configExists := packageConfig["config"]
if !configExists {
Expand Down Expand Up @@ -759,12 +776,12 @@ func (c *Config) mergeInConfig(ctx context.Context) error {
configSectionTyped[key] = value
}
}
interfaces, err := c.getInterfacesForPackage(ctx, pkgPath)
interfaces, err := c.getInterfacesForPackage(pkgCtx, pkgPath)
if err != nil {
return fmt.Errorf("failed to get interfaces for package: %w", err)
}
for _, interfaceName := range interfaces {
interfacesSection, err := c.getInterfacesSection(ctx, pkgPath)
interfacesSection, err := c.getInterfacesSection(pkgCtx, pkgPath)
if err != nil {
return err
}
Expand Down
1 change: 1 addition & 0 deletions pkg/config/config_test.go
Expand Up @@ -1206,6 +1206,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")
Expand Down

0 comments on commit d3515d1

Please sign in to comment.