From ea43c33330ec9a1e9a9b8a85348c1757fdae65c4 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 7 Feb 2017 09:44:47 +0000 Subject: [PATCH] compose: fix environment interpolation from the client For an environment variable defined in the yaml without value, the value needs to be propagated from the client, as in Docker Compose. Signed-off-by: Akihiro Suda --- cli/command/stack/deploy_composefile.go | 10 +++ cli/compose/loader/example2.env | 3 + cli/compose/loader/loader.go | 73 ++++++++++++++-------- cli/compose/loader/loader_test.go | 82 +++++++++++++------------ 4 files changed, 104 insertions(+), 64 deletions(-) diff --git a/cli/command/stack/deploy_composefile.go b/cli/command/stack/deploy_composefile.go index 3e62494325..b176b47e0b 100644 --- a/cli/command/stack/deploy_composefile.go +++ b/cli/command/stack/deploy_composefile.go @@ -115,6 +115,16 @@ func getConfigDetails(opts deployOptions) (composetypes.ConfigDetails, error) { } // TODO: support multiple files details.ConfigFiles = []composetypes.ConfigFile{*configFile} + env := os.Environ() + details.Environment = make(map[string]string, len(env)) + for _, s := range env { + // if value is empty, s is like "K=", not "K". + if !strings.Contains(s, "=") { + return details, fmt.Errorf("unexpected environment %q", s) + } + kv := strings.SplitN(s, "=", 2) + details.Environment[kv[0]] = kv[1] + } return details, nil } diff --git a/cli/compose/loader/example2.env b/cli/compose/loader/example2.env index 0920d5ab05..642334e9fd 100644 --- a/cli/compose/loader/example2.env +++ b/cli/compose/loader/example2.env @@ -1 +1,4 @@ BAR=2 + +# overridden in configDetails.Environment +QUX=1 diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index 995047e8c9..7c8bfa0a2a 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -2,15 +2,16 @@ package loader import ( "fmt" - "os" "path" "reflect" "regexp" "sort" "strings" + "github.com/Sirupsen/logrus" "github.com/docker/docker/cli/compose/interpolation" "github.com/docker/docker/cli/compose/schema" + "github.com/docker/docker/cli/compose/template" "github.com/docker/docker/cli/compose/types" "github.com/docker/docker/opts" runconfigopts "github.com/docker/docker/runconfig/opts" @@ -69,13 +70,17 @@ func Load(configDetails types.ConfigDetails) (*types.Config, error) { } cfg := types.Config{} + lookupEnv := func(k string) (string, bool) { + v, ok := configDetails.Environment[k] + return v, ok + } if services, ok := configDict["services"]; ok { - servicesConfig, err := interpolation.Interpolate(services.(types.Dict), "service", os.LookupEnv) + servicesConfig, err := interpolation.Interpolate(services.(types.Dict), "service", lookupEnv) if err != nil { return nil, err } - servicesList, err := LoadServices(servicesConfig, configDetails.WorkingDir) + servicesList, err := LoadServices(servicesConfig, configDetails.WorkingDir, lookupEnv) if err != nil { return nil, err } @@ -84,7 +89,7 @@ func Load(configDetails types.ConfigDetails) (*types.Config, error) { } if networks, ok := configDict["networks"]; ok { - networksConfig, err := interpolation.Interpolate(networks.(types.Dict), "network", os.LookupEnv) + networksConfig, err := interpolation.Interpolate(networks.(types.Dict), "network", lookupEnv) if err != nil { return nil, err } @@ -98,7 +103,7 @@ func Load(configDetails types.ConfigDetails) (*types.Config, error) { } if volumes, ok := configDict["volumes"]; ok { - volumesConfig, err := interpolation.Interpolate(volumes.(types.Dict), "volume", os.LookupEnv) + volumesConfig, err := interpolation.Interpolate(volumes.(types.Dict), "volume", lookupEnv) if err != nil { return nil, err } @@ -112,7 +117,7 @@ func Load(configDetails types.ConfigDetails) (*types.Config, error) { } if secrets, ok := configDict["secrets"]; ok { - secretsConfig, err := interpolation.Interpolate(secrets.(types.Dict), "secret", os.LookupEnv) + secretsConfig, err := interpolation.Interpolate(secrets.(types.Dict), "secret", lookupEnv) if err != nil { return nil, err } @@ -308,11 +313,11 @@ func formatInvalidKeyError(keyPrefix string, key interface{}) error { // LoadServices produces a ServiceConfig map from a compose file Dict // the servicesDict is not validated if directly used. Use Load() to enable validation -func LoadServices(servicesDict types.Dict, workingDir string) ([]types.ServiceConfig, error) { +func LoadServices(servicesDict types.Dict, workingDir string, lookupEnv template.Mapping) ([]types.ServiceConfig, error) { var services []types.ServiceConfig for name, serviceDef := range servicesDict { - serviceConfig, err := LoadService(name, serviceDef.(types.Dict), workingDir) + serviceConfig, err := loadService(name, serviceDef.(types.Dict), workingDir, lookupEnv) if err != nil { return nil, err } @@ -324,22 +329,39 @@ func LoadServices(servicesDict types.Dict, workingDir string) ([]types.ServiceCo // LoadService produces a single ServiceConfig from a compose file Dict // the serviceDict is not validated if directly used. Use Load() to enable validation -func LoadService(name string, serviceDict types.Dict, workingDir string) (*types.ServiceConfig, error) { +func LoadService(name string, serviceDict types.Dict, workingDir string, lookupEnv template.Mapping) (*types.ServiceConfig, error) { serviceConfig := &types.ServiceConfig{} if err := transform(serviceDict, serviceConfig); err != nil { return nil, err } serviceConfig.Name = name - if err := resolveEnvironment(serviceConfig, workingDir); err != nil { + if err := resolveEnvironment(serviceConfig, workingDir, lookupEnv); err != nil { return nil, err } - resolveVolumePaths(serviceConfig.Volumes, workingDir) + resolveVolumePaths(serviceConfig.Volumes, workingDir, lookupEnv) return serviceConfig, nil } -func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string) error { +func updateEnvironment(environment map[string]string, vars map[string]string, lookupEnv template.Mapping) map[string]string { + result := make(map[string]string, len(environment)) + for k, v := range environment { + result[k]=v + } + for k, v := range vars { + interpolatedV, ok := lookupEnv(k) + if ok { + // lookupEnv is prioritized over vars + result[k] = interpolatedV + } else { + result[k] = v + } + } + return result +} + +func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string, lookupEnv template.Mapping) error { environment := make(map[string]string) if len(serviceConfig.EnvFile) > 0 { @@ -353,36 +375,35 @@ func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string) e } envVars = append(envVars, fileVars...) } - - for k, v := range runconfigopts.ConvertKVStringsToMap(envVars) { - environment[k] = v - } + environment = updateEnvironment(environment, + runconfigopts.ConvertKVStringsToMap(envVars), lookupEnv) } - for k, v := range serviceConfig.Environment { - environment[k] = v - } - - serviceConfig.Environment = environment - + serviceConfig.Environment = updateEnvironment(environment, + serviceConfig.Environment, lookupEnv) return nil } -func resolveVolumePaths(volumes []types.ServiceVolumeConfig, workingDir string) { +func resolveVolumePaths(volumes []types.ServiceVolumeConfig, workingDir string, lookupEnv template.Mapping) { for i, volume := range volumes { if volume.Type != "bind" { continue } - volume.Source = absPath(workingDir, expandUser(volume.Source)) + volume.Source = absPath(workingDir, expandUser(volume.Source, lookupEnv)) volumes[i] = volume } } // TODO: make this more robust -func expandUser(path string) string { +func expandUser(path string, lookupEnv template.Mapping) string { if strings.HasPrefix(path, "~") { - return strings.Replace(path, "~", os.Getenv("HOME"), 1) + home, ok := lookupEnv("HOME") + if !ok { + logrus.Warn("cannot expand '~', because the environment lacks HOME") + return path + } + return strings.Replace(path, "~", home, 1) } return path } diff --git a/cli/compose/loader/loader_test.go b/cli/compose/loader/loader_test.go index b9fb10f227..4f424d6126 100644 --- a/cli/compose/loader/loader_test.go +++ b/cli/compose/loader/loader_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/assert" ) -func buildConfigDetails(source types.Dict) types.ConfigDetails { +func buildConfigDetails(source types.Dict, env map[string]string) types.ConfigDetails { workingDir, err := os.Getwd() if err != nil { panic(err) @@ -23,7 +23,7 @@ func buildConfigDetails(source types.Dict) types.ConfigDetails { ConfigFiles: []types.ConfigFile{ {Filename: "filename.yml", Config: source}, }, - Environment: nil, + Environment: env, } } @@ -154,7 +154,7 @@ func TestParseYAML(t *testing.T) { } func TestLoad(t *testing.T) { - actual, err := Load(buildConfigDetails(sampleDict)) + actual, err := Load(buildConfigDetails(sampleDict, nil)) if !assert.NoError(t, err) { return } @@ -173,7 +173,7 @@ services: secrets: super: external: true -`) +`, nil) if !assert.NoError(t, err) { return } @@ -182,7 +182,7 @@ secrets: } func TestParseAndLoad(t *testing.T) { - actual, err := loadYAML(sampleYAML) + actual, err := loadYAML(sampleYAML, nil) if !assert.NoError(t, err) { return } @@ -192,15 +192,15 @@ func TestParseAndLoad(t *testing.T) { } func TestInvalidTopLevelObjectType(t *testing.T) { - _, err := loadYAML("1") + _, err := loadYAML("1", nil) assert.Error(t, err) assert.Contains(t, err.Error(), "Top-level object must be a mapping") - _, err = loadYAML("\"hello\"") + _, err = loadYAML("\"hello\"", nil) assert.Error(t, err) assert.Contains(t, err.Error(), "Top-level object must be a mapping") - _, err = loadYAML("[\"hello\"]") + _, err = loadYAML("[\"hello\"]", nil) assert.Error(t, err) assert.Contains(t, err.Error(), "Top-level object must be a mapping") } @@ -211,7 +211,7 @@ version: "3" 123: foo: image: busybox -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "Non-string key at top level: 123") @@ -222,7 +222,7 @@ services: image: busybox 123: image: busybox -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "Non-string key in services: 123") @@ -236,7 +236,7 @@ networks: ipam: config: - 123: oh dear -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "Non-string key in networks.default.ipam.config[0]: 123") @@ -247,7 +247,7 @@ services: image: busybox environment: 1: FOO -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "Non-string key in services.dict-env.environment: 1") } @@ -258,7 +258,7 @@ version: "3" services: foo: image: busybox -`) +`, nil) assert.NoError(t, err) _, err = loadYAML(` @@ -266,7 +266,7 @@ version: "3.0" services: foo: image: busybox -`) +`, nil) assert.NoError(t, err) } @@ -276,7 +276,7 @@ version: "2" services: foo: image: busybox -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "version") @@ -285,7 +285,7 @@ version: "2.0" services: foo: image: busybox -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "version") } @@ -296,7 +296,7 @@ version: 3 services: foo: image: busybox -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "version must be a string") } @@ -305,7 +305,7 @@ func TestV1Unsupported(t *testing.T) { _, err := loadYAML(` foo: image: busybox -`) +`, nil) assert.Error(t, err) } @@ -315,7 +315,7 @@ version: "3" services: - foo: image: busybox -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "services must be a mapping") @@ -323,7 +323,7 @@ services: version: "3" services: foo: busybox -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "services.foo must be a mapping") @@ -332,7 +332,7 @@ version: "3" networks: - default: driver: bridge -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "networks must be a mapping") @@ -340,7 +340,7 @@ networks: version: "3" networks: default: bridge -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "networks.default must be a mapping") @@ -349,7 +349,7 @@ version: "3" volumes: - data: driver: local -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "volumes must be a mapping") @@ -357,7 +357,7 @@ volumes: version: "3" volumes: data: local -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "volumes.data must be a mapping") } @@ -368,7 +368,7 @@ version: "3" services: foo: image: ["busybox", "latest"] -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "services.foo.image must be a string") } @@ -383,6 +383,7 @@ services: FOO: "1" BAR: 2 BAZ: 2.5 + QUX: QUUX: list-env: image: busybox @@ -390,14 +391,16 @@ services: - FOO=1 - BAR=2 - BAZ=2.5 + - QUX - QUUX= -`) +`, map[string]string{"QUX": "qux"}) assert.NoError(t, err) expected := types.MappingWithEquals{ "FOO": "1", "BAR": "2", "BAZ": "2.5", + "QUX": "qux", "QUUX": "", } @@ -416,7 +419,7 @@ services: image: busybox environment: FOO: ["1"] -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "services.dict-env.environment.FOO must be a string, number or null") } @@ -428,12 +431,13 @@ services: dict-env: image: busybox environment: "FOO=1" -`) +`, nil) assert.Error(t, err) assert.Contains(t, err.Error(), "services.dict-env.environment must be a mapping") } func TestEnvironmentInterpolation(t *testing.T) { + home := "/home/foo" config, err := loadYAML(` version: "3" services: @@ -450,12 +454,13 @@ networks: volumes: test: driver: $HOME -`) +`, map[string]string{ + "HOME": home, + "FOO": "foo", + }) assert.NoError(t, err) - home := os.Getenv("HOME") - expectedLabels := types.MappingWithEquals{ "home1": home, "home2": home, @@ -483,7 +488,7 @@ services: `)) assert.NoError(t, err) - configDetails := buildConfigDetails(dict) + configDetails := buildConfigDetails(dict, nil) _, err = Load(configDetails) assert.NoError(t, err) @@ -506,7 +511,7 @@ services: `)) assert.NoError(t, err) - configDetails := buildConfigDetails(dict) + configDetails := buildConfigDetails(dict, nil) _, err = Load(configDetails) assert.NoError(t, err) @@ -529,7 +534,7 @@ services: bar: extends: service: foo -`) +`, nil) assert.Error(t, err) assert.IsType(t, &ForbiddenPropertiesError{}, err) @@ -601,7 +606,8 @@ func TestFullExample(t *testing.T) { bytes, err := ioutil.ReadFile("full-example.yml") assert.NoError(t, err) - config, err := loadYAML(string(bytes)) + homeDir := "/home/foo" + config, err := loadYAML(string(bytes), map[string]string{"HOME": homeDir, "QUX": "2"}) if !assert.NoError(t, err) { return } @@ -609,7 +615,6 @@ func TestFullExample(t *testing.T) { workingDir, err := os.Getwd() assert.NoError(t, err) - homeDir := os.Getenv("HOME") stopGracePeriod := time.Duration(20 * time.Second) expectedServiceConfig := types.ServiceConfig{ @@ -664,6 +669,7 @@ func TestFullExample(t *testing.T) { "FOO": "1", "BAR": "2", "BAZ": "3", + "QUX": "2", }, EnvFile: []string{ "./example1.env", @@ -955,13 +961,13 @@ func TestFullExample(t *testing.T) { assert.Equal(t, expectedVolumeConfig, config.Volumes) } -func loadYAML(yaml string) (*types.Config, error) { +func loadYAML(yaml string, env map[string]string) (*types.Config, error) { dict, err := ParseYAML([]byte(yaml)) if err != nil { return nil, err } - return Load(buildConfigDetails(dict)) + return Load(buildConfigDetails(dict, env)) } func serviceSort(services []types.ServiceConfig) []types.ServiceConfig {