Skip to content

Commit

Permalink
improved defensive coding per review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
bwhaley committed Oct 30, 2020
1 parent 8b94b42 commit 6e977ec
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 16 deletions.
10 changes: 9 additions & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,15 @@ func ParseBoilerplateConfig(configContents []byte) (*BoilerplateConfig, error) {
return nil, errors.WithStackTrace(err)
}

boilerplateConfig = variables.Convert(boilerplateConfig).(*BoilerplateConfig)
converted, err := variables.ConvertYAMLToStringMap(boilerplateConfig)
if err != nil {
return boilerplateConfig, err
}

boilerplateConfig, ok := converted.(*BoilerplateConfig)
if !ok {
return nil, variables.YAMLConversionErr{Key: converted}
}

return boilerplateConfig, nil
}
Expand Down
6 changes: 5 additions & 1 deletion variables/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,11 @@ func ConvertType(value interface{}, variable Variable) (interface{}, error) {
}
case Map:
if reflect.TypeOf(value).Kind() == reflect.Map {
return Convert(value), nil
value, err := ConvertYAMLToStringMap(value)
if err != nil {
return nil, err
}
return value, nil
}
if isString {
return parseStringAsMap(asString)
Expand Down
61 changes: 48 additions & 13 deletions variables/yaml_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,12 @@ func ParseYamlString(str string) (interface{}, error) {
return nil, errors.WithStackTrace(err)
}

return Convert(parsedValue), nil
parsedValue, err = ConvertYAMLToStringMap(parsedValue)
if err != nil {
return nil, errors.WithStackTrace(err)
}

return parsedValue, nil
}

// Parse a list of YAML files that define variables into a map from variable name to variable value. Along the way,
Expand Down Expand Up @@ -320,7 +325,17 @@ func parseVariablesFromVarFileContents(varFileContents []byte) (map[string]inter
return vars, err
}

return Convert(vars).(map[string]interface{}), nil
converted, err := ConvertYAMLToStringMap(vars)
if err != nil {
return nil, errors.WithStackTrace(err)
}

vars, ok := converted.(map[string]interface{})
if !ok {
return nil, YAMLConversionErr{converted}
}

return vars, nil
}

// Parse variables passed in via command line options, either as a list of NAME=VALUE variable pairs in varsList, or a
Expand All @@ -342,27 +357,47 @@ func ParseVars(varsList []string, varFileList []string) (map[string]interface{},
return util.MergeMaps(varsFromVarsList, varsFromVarFiles), nil
}

// Convert updates i of type map[interface{}]interface{} to a map[string]interface{} so that it may be properly
// marshalled in to JSON.
// convertYAMLToStringMap modifies an input with type map[interface{}]interface{} to map[string]interface{} so that it may be
// properly marshalled in to JSON.
// See: https://github.com/go-yaml/yaml/issues/139
func Convert(i interface{}) interface{} {
switch x := i.(type) {
func ConvertYAMLToStringMap(yamlMapOrList interface{}) (interface{}, error) {
switch mapOrList := yamlMapOrList.(type) {
case map[interface{}]interface{}:
m2 := map[string]interface{}{}
for k, v := range x {
m2[k.(string)] = Convert(v)
outputMap := map[string]interface{}{}
for k, v := range mapOrList {
strK, ok := k.(string)
if !ok {
return nil, YAMLConversionErr{k}
}
res, err := ConvertYAMLToStringMap(v)
if err != nil {
return nil, err
}
outputMap[strK] = res
}
return m2
return outputMap, nil
case []interface{}:
for i, v := range x {
x[i] = Convert(v)
for index, value := range mapOrList {
res, err := ConvertYAMLToStringMap(value)
if err != nil {
return nil, err
}
mapOrList[index] = res
}
}
return i
return yamlMapOrList, nil
}

// Custom error types

type YAMLConversionErr struct {
Key interface{}
}

func (err YAMLConversionErr) Error() string {
return fmt.Sprintf("YAML value has type %s and cannot be cast to to the correct type.", reflect.TypeOf(err.Key))
}

type OptionsMissing string

func (err OptionsMissing) Error() string {
Expand Down
12 changes: 11 additions & 1 deletion variables/yaml_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,20 @@ func TestConvert(t *testing.T) {
},
expectedType: []interface{}{},
},
{
input: map[string]interface{}{
"key3": 42,
"key1": map[string]interface{}{
"key2": "value2",
},
},
expectedType: map[string]interface{}{},
},
}

for _, testCase := range testCases {
actual := Convert(testCase.input)
actual, err := ConvertYAMLToStringMap(testCase.input)
assert.NoError(t, err)
assert.IsType(t, testCase.expectedType, actual)
}
}

0 comments on commit 6e977ec

Please sign in to comment.