From 6f991b6436777cc7dcd1d80dfcc20abca072e3dc Mon Sep 17 00:00:00 2001 From: Dean Chen <862469039@qq.com> Date: Tue, 29 Sep 2026 07:30:51 +0500 Subject: [PATCH] cli/compose: keep hyphens inside :? error text docker stack was treating the first "-" as a hard default, so ${VAR:?must be set - reason} succeeded when VAR was unset. Signed-off-by: Dean Chen <862469039@qq.com> --- cli/compose/template/template.go | 39 ++++++++++++++++++--------- cli/compose/template/template_test.go | 36 +++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/cli/compose/template/template.go b/cli/compose/template/template.go index 14ba09de3599..9e4a8db35b2f 100644 --- a/cli/compose/template/template.go +++ b/cli/compose/template/template.go @@ -183,25 +183,40 @@ func extractVariable(value any, pattern regexper) ([]extractedValue, bool) { } name := val var defaultValue string - switch { - case strings.Contains(val, ":?"): - name, _ = partition(val, ":?") - case strings.Contains(val, "?"): - name, _ = partition(val, "?") - case strings.Contains(val, ":-"): - name, defaultValue = partition(val, ":-") - case strings.Contains(val, "-"): - name, defaultValue = partition(val, "-") + switch sep := operatorSep(val); sep { + case ":?", "?": + name, _ = partition(val, sep) + case ":-", "-": + name, defaultValue = partition(val, sep) } values = append(values, extractedValue{name: name, value: defaultValue}) } return values, len(values) > 0 } +// operatorSep is the interpolation operator that applies to substitution. +// The earliest of :?, :-, ? and - wins, so a hyphen or question mark later +// in an error message or default is not treated as another operator. +func operatorSep(substitution string) string { + bestAt := -1 + best := "" + for _, sep := range []string{":?", ":-", "?", "-"} { + at := strings.Index(substitution, sep) + if at < 0 { + continue + } + if bestAt < 0 || at < bestAt || (at == bestAt && len(sep) > len(best)) { + bestAt = at + best = sep + } + } + return best +} + // Soft default (fall back if unset or empty) func softDefault(substitution string, mapping Mapping) (string, bool, error) { sep := ":-" - if !strings.Contains(substitution, sep) { + if operatorSep(substitution) != sep { return "", false, nil } name, defaultValue := partition(substitution, sep) @@ -215,7 +230,7 @@ func softDefault(substitution string, mapping Mapping) (string, bool, error) { // Hard default (fall back if-and-only-if empty) func hardDefault(substitution string, mapping Mapping) (string, bool, error) { sep := "-" - if !strings.Contains(substitution, sep) { + if operatorSep(substitution) != sep { return "", false, nil } name, defaultValue := partition(substitution, sep) @@ -235,7 +250,7 @@ func required(substitution string, mapping Mapping) (string, bool, error) { } func withRequired(substitution string, mapping Mapping, sep string, valid func(string) bool) (string, bool, error) { - if !strings.Contains(substitution, sep) { + if operatorSep(substitution) != sep { return "", false, nil } name, errorMessage := partition(substitution, sep) diff --git a/cli/compose/template/template_test.go b/cli/compose/template/template_test.go index 0b2d463ebbe8..e617ac4d6c38 100644 --- a/cli/compose/template/template_test.go +++ b/cli/compose/template/template_test.go @@ -132,6 +132,42 @@ func TestMandatoryVariableErrors(t *testing.T) { } } +func TestRequiredMessageMayContainHyphen(t *testing.T) { + testCases := []struct { + template string + expectedError string + }{ + { + template: "not ok ${UNSET_VAR:?must be set - hyphen in this message}", + expectedError: "required variable UNSET_VAR is missing a value: must be set - hyphen in this message", + }, + { + template: "not ok ${UNSET_VAR?must be set - hyphen in this message}", + expectedError: "required variable UNSET_VAR is missing a value: must be set - hyphen in this message", + }, + { + template: "not ok ${BAR:?must be set - hyphen in this message}", + expectedError: "required variable BAR is missing a value: must be set - hyphen in this message", + }, + } + for _, tc := range testCases { + _, err := Substitute(tc.template, defaultMapping) + assert.Check(t, is.ErrorContains(err, tc.expectedError)) + } + + result, err := Substitute("ok ${FOO:?must be set - hyphen in this message}", defaultMapping) + assert.NilError(t, err) + assert.Check(t, is.Equal("ok first", result)) + + result, err = Substitute("ok ${missing-foo?bar}", defaultMapping) + assert.NilError(t, err) + assert.Check(t, is.Equal("ok foo?bar", result)) + + result, err = Substitute("ok ${missing:-foo?bar}", defaultMapping) + assert.NilError(t, err) + assert.Check(t, is.Equal("ok foo?bar", result)) +} + func TestDefaultsForMandatoryVariables(t *testing.T) { testCases := []struct { template string