Skip to content

Commit ad79316

Browse files
ulyssessouzaulyssessouza
authored
Merge pull request #219 from compose-spec/fix-vars-default-values
Fix default values on variables interpolation
2 parents cc56874 + b6e6cdc commit ad79316

File tree

3 files changed

+184
-9
lines changed

3 files changed

+184
-9
lines changed

‎interpolation/interpolation_test.go‎

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717
package interpolation
1818

1919
import (
20-
"testing"
21-
20+
"fmt"
2221
"strconv"
22+
"testing"
2323

2424
"gotest.tools/v3/assert"
2525
is "gotest.tools/v3/assert/cmp"
@@ -95,6 +95,96 @@ func TestInterpolateWithDefaults(t *testing.T) {
9595
assert.Check(t, is.DeepEqual(expected, result))
9696
}
9797

98+
func TestValidUnexistentInterpolation(t *testing.T) {
99+
var testcases = []struct {
100+
test string
101+
expected string
102+
errMsg string
103+
}{
104+
{test: "{{{ ${FOO:-foo_} }}}", expected: "{{{ foo_ }}}"},
105+
{test: "{{{ ${FOO:-foo-bar-value} }}}", expected: "{{{ foo-bar-value }}}"},
106+
{test: "{{{ ${FOO:-foo?bar?value} }}}", expected: "{{{ foo?bar?value }}}"},
107+
{test: "{{{ ${FOO:-foo:?bar:?value} }}}", expected: "{{{ foo:?bar:?value }}}"},
108+
{test: "{{{ ${FOO:-foo} ${BAR:-DEFAULT_VALUE} }}}", expected: "{{{ foo DEFAULT_VALUE }}}"},
109+
{test: "{{{ ${BAR} }}}", expected: "{{{ }}}"},
110+
{test: "${FOO:-baz} }}}", expected: "baz }}}"},
111+
{test: "${FOO-baz} }}}", expected: "baz }}}"},
112+
113+
{test: "{{{ ${FOO:?foo_} }}}", errMsg: "foo_"},
114+
{test: "{{{ ${FOO:?foo-bar-value} }}}", errMsg: "foo-bar-value"},
115+
{test: "{{{ ${FOO:?foo} ${BAR:-DEFAULT_VALUE} }}}", errMsg: "foo"},
116+
{test: "{{{ ${BAR} }}}", expected: "{{{ }}}"},
117+
{test: "${FOO:?baz} }}}", errMsg: "baz"},
118+
{test: "${FOO?baz} }}}", errMsg: "baz"},
119+
}
120+
121+
getServiceConfig := func(val string) map[string]interface{} {
122+
if val == "" {
123+
return map[string]interface{}{}
124+
}
125+
return map[string]interface{}{
126+
"myservice": map[string]interface{}{
127+
"environment": map[string]interface{}{
128+
"TESTVAR": val,
129+
},
130+
},
131+
}
132+
}
133+
134+
getFullErrorMsg := func(msg string) string {
135+
return fmt.Sprintf("invalid interpolation format for myservice.environment.TESTVAR: "+
136+
"\"required variable FOO is missing a value: %s\". You may need to escape any $ with another $.", msg)
137+
}
138+
139+
for _, testcase := range testcases {
140+
result, err := Interpolate(getServiceConfig(testcase.test), Options{})
141+
if testcase.errMsg != "" {
142+
assert.Assert(t, err != nil, fmt.Sprintf("This should result in an error %q", testcase.errMsg))
143+
assert.Equal(t, getFullErrorMsg(testcase.errMsg), err.Error())
144+
}
145+
assert.Check(t, is.DeepEqual(getServiceConfig(testcase.expected), result))
146+
}
147+
}
148+
149+
func TestValidExistentInterpolation(t *testing.T) {
150+
var testcases = []struct {
151+
test string
152+
expected string
153+
}{
154+
// Only FOO is set
155+
{test: "{{{ ${FOO:-foo_} }}}", expected: "{{{ bar }}}"},
156+
{test: "{{{ ${FOO:-foo-bar-value} }}}", expected: "{{{ bar }}}"},
157+
{test: "{{{ ${FOO:-foo} ${BAR:-DEFAULT_VALUE} }}}", expected: "{{{ bar DEFAULT_VALUE }}}"},
158+
{test: "{{{ ${BAR} }}}", expected: "{{{ }}}"},
159+
{test: "${FOO:-baz} }}}", expected: "bar }}}"},
160+
{test: "${FOO-baz} }}}", expected: "bar }}}"},
161+
162+
// Both FOO and USER are set
163+
{test: "{{{ ${FOO:-foo_} }}}", expected: "{{{ bar }}}"},
164+
{test: "{{{ ${FOO:-foo-bar-value} }}}", expected: "{{{ bar }}}"},
165+
{test: "{{{ ${FOO:-foo} ${USER:-bar} }}}", expected: "{{{ bar jenny }}}"},
166+
{test: "{{{ ${USER} }}}", expected: "{{{ jenny }}}"},
167+
{test: "${FOO:-baz} }}}", expected: "bar }}}"},
168+
{test: "${FOO-baz} }}}", expected: "bar }}}"},
169+
}
170+
171+
getServiceConfig := func(val string) map[string]interface{} {
172+
return map[string]interface{}{
173+
"myservice": map[string]interface{}{
174+
"environment": map[string]interface{}{
175+
"TESTVAR": val,
176+
},
177+
},
178+
}
179+
}
180+
181+
for _, testcase := range testcases {
182+
result, err := Interpolate(getServiceConfig(testcase.test), Options{LookupValue: defaultMapping})
183+
assert.NilError(t, err)
184+
assert.Check(t, is.DeepEqual(getServiceConfig(testcase.expected), result))
185+
}
186+
}
187+
98188
func TestInterpolateWithCast(t *testing.T) {
99189
config := map[string]interface{}{
100190
"foo": map[string]interface{}{

‎template/template.go‎

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
var delimiter = "\\$"
2828
var substitutionNamed = "[_a-z][_a-z0-9]*"
29+
2930
var substitutionBraced = "[_a-z][_a-z0-9]*(?::?[-?](.*}|[^}]*))?"
3031

3132
var patternString = fmt.Sprintf(
@@ -60,15 +61,17 @@ type SubstituteFunc func(string, Mapping) (string, bool, error)
6061
// It accepts additional substitute function.
6162
func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, subsFuncs ...SubstituteFunc) (string, error) {
6263
if len(subsFuncs) == 0 {
63-
subsFuncs = []SubstituteFunc{
64-
softDefault,
65-
hardDefault,
66-
requiredNonEmpty,
67-
required,
68-
}
64+
subsFuncs = getDefaultSortedSubstitutionFunctions(template)
6965
}
7066
var err error
7167
result := pattern.ReplaceAllStringFunc(template, func(substring string) string {
68+
closingBraceIndex := getFirstBraceClosingIndex(substring)
69+
rest := ""
70+
if closingBraceIndex > -1 {
71+
rest = substring[closingBraceIndex+1:]
72+
substring = substring[0 : closingBraceIndex+1]
73+
}
74+
7275
matches := pattern.FindStringSubmatch(substring)
7376
groups := matchGroups(matches, pattern)
7477
if escaped := groups["escaped"]; escaped != "" {
@@ -100,7 +103,11 @@ func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, su
100103
if !applied {
101104
continue
102105
}
103-
return value
106+
interpolatedNested, err := SubstituteWith(rest, mapping, pattern, subsFuncs...)
107+
if err != nil {
108+
return ""
109+
}
110+
return value + interpolatedNested
104111
}
105112
}
106113

@@ -114,6 +121,42 @@ func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, su
114121
return result, err
115122
}
116123

124+
func getDefaultSortedSubstitutionFunctions(template string, fns ...SubstituteFunc) []SubstituteFunc {
125+
hyphenIndex := strings.IndexByte(template, '-')
126+
questionIndex := strings.IndexByte(template, '?')
127+
if hyphenIndex < 0 || hyphenIndex > questionIndex {
128+
return []SubstituteFunc{
129+
requiredNonEmpty,
130+
required,
131+
softDefault,
132+
hardDefault,
133+
}
134+
}
135+
return []SubstituteFunc{
136+
softDefault,
137+
hardDefault,
138+
requiredNonEmpty,
139+
required,
140+
}
141+
}
142+
143+
func getFirstBraceClosingIndex(s string) int {
144+
openVariableBraces := 0
145+
for i := 0; i < len(s); i++ {
146+
if s[i] == '}' {
147+
openVariableBraces--
148+
if openVariableBraces == 0 {
149+
return i
150+
}
151+
}
152+
if strings.HasPrefix(s[i:], "${") {
153+
openVariableBraces++
154+
i++
155+
}
156+
}
157+
return -1
158+
}
159+
117160
// Substitute variables in the string with their values
118161
func Substitute(template string, mapping Mapping) (string, error) {
119162
return SubstituteWith(template, mapping, defaultPattern)

‎template/template_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,48 @@ func TestSubstituteWithCustomFunc(t *testing.T) {
262262
assert.Check(t, is.ErrorContains(err, "required variable"))
263263
}
264264

265+
// TestPrecedence tests is the precedence on '-' and '?' is of the first match
266+
func TestPrecedence(t *testing.T) {
267+
268+
testCases := []struct {
269+
template string
270+
expected string
271+
err error
272+
}{
273+
{
274+
template: "${UNSET_VAR?bar-baz}", // Unexistent variable
275+
expected: "",
276+
err: &InvalidTemplateError{
277+
Template: "required variable UNSET_VAR is missing a value: bar-baz",
278+
},
279+
},
280+
{
281+
template: "${UNSET_VAR-myerror?msg}", // Unexistent variable
282+
expected: "myerror?msg",
283+
err: nil,
284+
},
285+
286+
{
287+
template: "${FOO?bar-baz}", // Existent variable
288+
expected: "first",
289+
},
290+
{
291+
template: "${BAR:-default_value_for_empty_var}", // Existent empty variable
292+
expected: "default_value_for_empty_var",
293+
},
294+
{
295+
template: "${UNSET_VAR-default_value_for_unset_var}", // Unset variable
296+
expected: "default_value_for_unset_var",
297+
},
298+
}
299+
300+
for _, tc := range testCases {
301+
result, err := Substitute(tc.template, defaultMapping)
302+
assert.Check(t, is.DeepEqual(tc.err, err))
303+
assert.Check(t, is.Equal(tc.expected, result))
304+
}
305+
}
306+
265307
func TestExtractVariables(t *testing.T) {
266308
testCases := []struct {
267309
name string

0 commit comments

Comments
 (0)