Skip to content

Commit

Permalink
throw a warning if value of a config is of the wrong type (#4700)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jk1484 authored Jan 10, 2024
1 parent 2b2c441 commit 5b68fc6
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 26 deletions.
48 changes: 27 additions & 21 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,15 @@ func (c *Config) GetStepConfig(flagValues map[string]interface{}, paramJSON stri
stepConfig.mixInStepDefaults(parameters)

// merge parameters provided by Piper environment
stepConfig.mixIn(envParameters, filters.All)
stepConfig.mixIn(envParameters, ReportingParameters.getReportingFilter())
stepConfig.mixIn(envParameters, filters.All, metadata)
stepConfig.mixIn(envParameters, ReportingParameters.getReportingFilter(), metadata)

// read defaults & merge general -> steps (-> general -> steps ...)
for _, def := range c.defaults.Defaults {
def.ApplyAliasConfig(parameters, secrets, filters, stageName, stepName, stepAliases)
stepConfig.mixIn(def.General, filters.General)
stepConfig.mixIn(def.Steps[stepName], filters.Steps)
stepConfig.mixIn(def.Stages[stageName], filters.Steps)
stepConfig.mixIn(def.General, filters.General, metadata)
stepConfig.mixIn(def.Steps[stepName], filters.Steps, metadata)
stepConfig.mixIn(def.Stages[stageName], filters.Steps, metadata)
stepConfig.mixinVaultConfig(parameters, def.General, def.Steps[stepName], def.Stages[stageName])
reportingConfig, err := cloneConfig(&def)
if err != nil {
Expand All @@ -204,16 +204,16 @@ func (c *Config) GetStepConfig(flagValues map[string]interface{}, paramJSON stri
reportingConfig.ApplyAliasConfig(ReportingParameters.Parameters, []StepSecrets{}, ReportingParameters.getStepFilters(), stageName, stepName, []Alias{})
stepConfig.mixinReportingConfig(reportingConfig.General, reportingConfig.Steps[stepName], reportingConfig.Stages[stageName])

stepConfig.mixInHookConfig(def.Hooks)
stepConfig.mixInHookConfig(def.Hooks, metadata)
}

// read config & merge - general -> steps -> stages
stepConfig.mixIn(c.General, filters.General)
stepConfig.mixIn(c.Steps[stepName], filters.Steps)
stepConfig.mixIn(c.Stages[stageName], filters.Stages)
stepConfig.mixIn(c.General, filters.General, metadata)
stepConfig.mixIn(c.Steps[stepName], filters.Steps, metadata)
stepConfig.mixIn(c.Stages[stageName], filters.Stages, metadata)

// merge parameters provided via env vars
stepConfig.mixIn(envValues(filters.All), filters.All)
stepConfig.mixIn(envValues(filters.All), filters.All, metadata)

// if parameters are provided in JSON format merge them
if len(paramJSON) != 0 {
Expand All @@ -230,13 +230,13 @@ func (c *Config) GetStepConfig(flagValues map[string]interface{}, paramJSON stri
params = setParamValueFromAlias(stepName, params, filters.Parameters, s.Name, s.Aliases)
}

stepConfig.mixIn(params, filters.Parameters)
stepConfig.mixIn(params, filters.Parameters, metadata)
}
}

// merge command line flags
if flagValues != nil {
stepConfig.mixIn(flagValues, filters.Parameters)
stepConfig.mixIn(flagValues, filters.Parameters, metadata)
}

if verbose, ok := stepConfig.Config["verbose"].(bool); ok && verbose {
Expand Down Expand Up @@ -314,12 +314,12 @@ func GetStepConfigWithJSON(flagValues map[string]interface{}, stepConfigJSON str
log.Entry().Warnf("invalid stepConfig JSON: %v", err)
}

stepConfig.mixIn(stepConfigMap, filters.All)
stepConfig.mixIn(stepConfigMap, filters.All, StepData{})

// ToDo: mix in parametersJSON

if flagValues != nil {
stepConfig.mixIn(flagValues, filters.Parameters)
stepConfig.mixIn(flagValues, filters.Parameters, StepData{})
}
return stepConfig
}
Expand Down Expand Up @@ -402,22 +402,22 @@ func envValues(filter []string) map[string]interface{} {
return vals
}

func (s *StepConfig) mixIn(mergeData map[string]interface{}, filter []string) {
func (s *StepConfig) mixIn(mergeData map[string]interface{}, filter []string, metadata StepData) {

if s.Config == nil {
s.Config = map[string]interface{}{}
}

s.Config = merge(s.Config, filterMap(mergeData, filter))
s.Config = merge(s.Config, filterMap(mergeData, filter), metadata)
}

func (s *StepConfig) mixInHookConfig(mergeData map[string]interface{}) {
func (s *StepConfig) mixInHookConfig(mergeData map[string]interface{}, metadata StepData) {

if s.HookConfig == nil {
s.HookConfig = map[string]interface{}{}
}

s.HookConfig = merge(s.HookConfig, mergeData)
s.HookConfig = merge(s.HookConfig, mergeData, metadata)
}

func (s *StepConfig) mixInStepDefaults(stepParams []StepParameters) {
Expand Down Expand Up @@ -480,7 +480,7 @@ func filterMap(data map[string]interface{}, filter []string) map[string]interfac
return result
}

func merge(base, overlay map[string]interface{}) map[string]interface{} {
func merge(base, overlay map[string]interface{}, metadata StepData) map[string]interface{} {

result := map[string]interface{}{}

Expand All @@ -495,12 +495,18 @@ func merge(base, overlay map[string]interface{}) map[string]interface{} {
for key, value := range overlay {
if val, ok := value.(map[string]interface{}); ok {
if valBaseKey, ok := base[key].(map[string]interface{}); !ok {
result[key] = merge(map[string]interface{}{}, val)
result[key] = merge(map[string]interface{}{}, val, metadata)
} else {
result[key] = merge(valBaseKey, val)
result[key] = merge(valBaseKey, val, metadata)
}
} else {
result[key] = value
for _, v := range metadata.Spec.Inputs.Parameters {
tVal := reflect.TypeOf(value).String()
if v.Name == key && tVal != v.Type {
log.Entry().Warn("config value provided for", v.Name, " is of wrong type", tVal, "should be of type", v.Type)
}
}
}
}
return result
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ func TestMerge(t *testing.T) {
for _, row := range testTable {
t.Run(fmt.Sprintf("Merging %v into %v", row.MergeData, row.Source), func(t *testing.T) {
stepConfig := StepConfig{Config: row.Source}
stepConfig.mixIn(row.MergeData, row.Filter)
stepConfig.mixIn(row.MergeData, row.Filter, StepData{})
assert.Equal(t, row.ExpectedOutput, stepConfig.Config, "Mixin was incorrect")
})
}
Expand Down Expand Up @@ -897,7 +897,7 @@ func TestStepConfig_mixInHookConfig(t *testing.T) {
Config: tt.fields.Config,
HookConfig: tt.fields.HookConfig,
}
s.mixInHookConfig(tt.args.mergeData)
s.mixInHookConfig(tt.args.mergeData, StepData{})
if !reflect.DeepEqual(s.HookConfig, tt.want) {
t.Errorf("mixInHookConfig() = %v, want %v", s.HookConfig, tt.want)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/reporting.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,6 @@ func (r ReportingParams) getReportingFilter() []string {
func (s *StepConfig) mixinReportingConfig(configs ...map[string]interface{}) {
reportingFilter := ReportingParameters.getReportingFilter()
for _, config := range configs {
s.mixIn(config, reportingFilter)
s.mixIn(config, reportingFilter, StepData{})
}
}
4 changes: 2 additions & 2 deletions pkg/config/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ type vaultClient interface {

func (s *StepConfig) mixinVaultConfig(parameters []StepParameters, configs ...map[string]interface{}) {
for _, config := range configs {
s.mixIn(config, vaultFilter)
s.mixIn(config, vaultFilter, StepData{})
// when an empty filter is returned we skip the mixin call since an empty filter will allow everything
if referencesFilter := getFilterForResourceReferences(parameters); len(referencesFilter) > 0 {
s.mixIn(config, referencesFilter)
s.mixIn(config, referencesFilter, StepData{})
}
}
}
Expand Down

0 comments on commit 5b68fc6

Please sign in to comment.