From ec3daea0214b9600062e13413ce3062c9d3690cb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 25 Sep 2018 18:35:39 +0200 Subject: [PATCH] Fix substitution with non-empty env-var Due to a typo, substitution would not work if the given environment-variable was set. Given the following docker compose file; ```yaml version: "3.7" services: app: image: nginx:${version:-latest} ``` Deploying a stack with `$version` set would ignore the `$version` environment variable, and use the default value instead; ```bash version=alpine docker stack deploy -c docker-compose.yml foobar Creating network foobar_default Creating service foobar_app docker service ls ID NAME MODE REPLICAS IMAGE PORTS rskkjxe6sm0w foobar_app replicated 1/1 nginx:latest ``` This patch also fixes "soft default" not detecting empty environment variables, only non-set environment variables. Signed-off-by: Sebastiaan van Stijn --- cli/compose/template/template.go | 16 +++++++++++----- cli/compose/template/template_test.go | 6 ++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/cli/compose/template/template.go b/cli/compose/template/template.go index 1762ab11a9d8..b958bde4015c 100644 --- a/cli/compose/template/template.go +++ b/cli/compose/template/template.go @@ -176,15 +176,21 @@ func extractVariable(value interface{}, pattern *regexp.Regexp) ([]extractedValu // Soft default (fall back if unset or empty) func softDefault(substitution string, mapping Mapping) (string, bool, error) { - return withDefault(substitution, mapping, "-:") + sep := ":-" + if !strings.Contains(substitution, sep) { + return "", false, nil + } + name, defaultValue := partition(substitution, sep) + value, ok := mapping(name) + if !ok || value == "" { + return defaultValue, true, nil + } + return value, true, nil } // Hard default (fall back if-and-only-if empty) func hardDefault(substitution string, mapping Mapping) (string, bool, error) { - return withDefault(substitution, mapping, "-") -} - -func withDefault(substitution string, mapping Mapping, sep string) (string, bool, error) { + sep := "-" if !strings.Contains(substitution, sep) { return "", false, nil } diff --git a/cli/compose/template/template_test.go b/cli/compose/template/template_test.go index 672a7e75d95b..ce3690410ffb 100644 --- a/cli/compose/template/template_test.go +++ b/cli/compose/template/template_test.go @@ -78,6 +78,12 @@ func TestEmptyValueWithSoftDefault(t *testing.T) { assert.Check(t, is.Equal("ok def", result)) } +func TestValueWithSoftDefault(t *testing.T) { + result, err := Substitute("ok ${FOO:-def}", defaultMapping) + assert.NilError(t, err) + assert.Check(t, is.Equal("ok first", result)) +} + func TestEmptyValueWithHardDefault(t *testing.T) { result, err := Substitute("ok ${BAR-def}", defaultMapping) assert.NilError(t, err)