From 550d532ae3061c7d3aed6000ed762932919702dc Mon Sep 17 00:00:00 2001 From: Josh Curl Date: Tue, 6 Dec 2016 10:26:50 -0800 Subject: [PATCH] Interpolate under volumes and networks Signed-off-by: Josh Curl Signed-off-by: Vincent Demeester --- config/interpolation.go | 48 +++++++++--------------- config/interpolation_test.go | 7 ++-- config/merge.go | 73 +++++++++++++++++++++++++++++------- config/merge_test.go | 17 --------- config/merge_v1.go | 17 +-------- config/merge_v2.go | 17 +-------- config/types.go | 10 ++--- lookup/composable.go | 4 +- lookup/composable_test.go | 8 ++-- lookup/envfile.go | 2 +- lookup/envfile_test.go | 12 +++--- lookup/simple_env.go | 2 +- lookup/simple_env_test.go | 10 ++--- project/project.go | 6 +-- project/project_test.go | 2 +- 15 files changed, 109 insertions(+), 126 deletions(-) diff --git a/config/interpolation.go b/config/interpolation.go index fc420de9a..66c987a71 100644 --- a/config/interpolation.go +++ b/config/interpolation.go @@ -102,7 +102,7 @@ func parseLine(line string, mapping func(string) string) (string, bool) { return buffer.String(), true } -func parseConfig(option, service string, data *interface{}, mapping func(string) string) error { +func parseConfig(key string, data *interface{}, mapping func(string) string) error { switch typedData := (*data).(type) { case string: var success bool @@ -110,11 +110,11 @@ func parseConfig(option, service string, data *interface{}, mapping func(string) *data, success = parseLine(typedData, mapping) if !success { - return fmt.Errorf("Invalid interpolation format for \"%s\" option in service \"%s\": \"%s\"", option, service, typedData) + return fmt.Errorf("Invalid interpolation format for key \"%s\": \"%s\"", key, typedData) } case []interface{}: for k, v := range typedData { - err := parseConfig(option, service, &v, mapping) + err := parseConfig(key, &v, mapping) if err != nil { return err @@ -124,7 +124,7 @@ func parseConfig(option, service string, data *interface{}, mapping func(string) } case map[interface{}]interface{}: for k, v := range typedData { - err := parseConfig(option, service, &v, mapping) + err := parseConfig(key, &v, mapping) if err != nil { return err @@ -137,33 +137,21 @@ func parseConfig(option, service string, data *interface{}, mapping func(string) return nil } -// Interpolate replaces variables in the raw map representation of the project file -func Interpolate(environmentLookup EnvironmentLookup, config *RawServiceMap) error { - for k, v := range *config { - for k2, v2 := range v { - err := parseConfig(k2, k, &v2, func(s string) string { - values := environmentLookup.Lookup(s, k, nil) +// Interpolate replaces variables in a map entry +func Interpolate(key string, data *interface{}, environmentLookup EnvironmentLookup) error { + return parseConfig(key, data, func(s string) string { + values := environmentLookup.Lookup(s, nil) - if len(values) == 0 { - logrus.Warnf("The %s variable is not set. Substituting a blank string.", s) - return "" - } - - // Use first result if many are given - value := values[0] - - // Environment variables come in key=value format - // Return everything past first '=' - return strings.SplitN(value, "=", 2)[1] - }) - - if err != nil { - return err - } - - (*config)[k][k2] = v2 + if len(values) == 0 { + logrus.Warnf("The %s variable is not set. Substituting a blank string.", s) + return "" } - } - return nil + // Use first result if many are given + value := values[0] + + // Environment variables come in key=value format + // Return everything past first '=' + return strings.SplitN(value, "=", 2)[1] + }) } diff --git a/config/interpolation_test.go b/config/interpolation_test.go index 6d56385da..d5afeb68e 100644 --- a/config/interpolation_test.go +++ b/config/interpolation_test.go @@ -81,7 +81,7 @@ type MockEnvironmentLookup struct { Variables map[string]string } -func (m MockEnvironmentLookup) Lookup(key, serviceName string, config *ServiceConfig) []string { +func (m MockEnvironmentLookup) Lookup(key string, config *ServiceConfig) []string { return []string{fmt.Sprintf("%s=%s", key, m.Variables[key])} } @@ -99,7 +99,7 @@ func testInterpolatedConfig(t *testing.T, expectedConfig, interpolatedConfig str yaml.Unmarshal(expectedConfigBytes, &expectedData) yaml.Unmarshal(interpolatedConfigBytes, &interpolatedData) - _ = Interpolate(MockEnvironmentLookup{envVariables}, &interpolatedData) + _ = InterpolateRawServiceMap(&interpolatedData, MockEnvironmentLookup{envVariables}) for k := range envVariables { os.Unsetenv(k) @@ -113,8 +113,7 @@ func testInvalidInterpolatedConfig(t *testing.T, interpolatedConfig string) { interpolatedData := make(RawServiceMap) yaml.Unmarshal(interpolatedConfigBytes, &interpolatedData) - err := Interpolate(new(MockEnvironmentLookup), &interpolatedData) - + err := InterpolateRawServiceMap(&interpolatedData, new(MockEnvironmentLookup)) assert.NotNil(t, err) } diff --git a/config/merge.go b/config/merge.go index 1c56d7be0..f40619d4c 100644 --- a/config/merge.go +++ b/config/merge.go @@ -29,18 +29,8 @@ func CreateConfig(bytes []byte) (*Config, error) { if err := yaml.Unmarshal(bytes, &config); err != nil { return nil, err } - if config.Version == "2" { - for key, value := range config.Networks { - if value == nil { - config.Networks[key] = &NetworkConfig{} - } - } - for key, value := range config.Volumes { - if value == nil { - config.Volumes[key] = &VolumeConfig{} - } - } - } else { + + if config.Version != "2" { var baseRawServices RawServiceMap if err := yaml.Unmarshal(bytes, &baseRawServices); err != nil { return nil, err @@ -48,6 +38,13 @@ func CreateConfig(bytes []byte) (*Config, error) { config.Services = baseRawServices } + if config.Volumes == nil { + config.Volumes = make(map[string]interface{}) + } + if config.Networks == nil { + config.Networks = make(map[string]interface{}) + } + return &config, nil } @@ -63,6 +60,34 @@ func Merge(existingServices *ServiceConfigs, environmentLookup EnvironmentLookup } baseRawServices := config.Services + if options.Interpolate { + if err := InterpolateRawServiceMap(&baseRawServices, environmentLookup); err != nil { + return "", nil, nil, nil, err + } + + for k, v := range config.Volumes { + if err := Interpolate(k, &v, environmentLookup); err != nil { + return "", nil, nil, nil, err + } + config.Volumes[k] = v + } + + for k, v := range config.Networks { + if err := Interpolate(k, &v, environmentLookup); err != nil { + return "", nil, nil, nil, err + } + config.Networks[k] = v + } + } + + if options.Preprocess != nil { + var err error + baseRawServices, err = options.Preprocess(baseRawServices) + if err != nil { + return "", nil, nil, nil, err + } + } + var serviceConfigs map[string]*ServiceConfig if config.Version == "2" { var err error @@ -91,7 +116,29 @@ func Merge(existingServices *ServiceConfigs, environmentLookup EnvironmentLookup } } - return config.Version, serviceConfigs, config.Volumes, config.Networks, nil + var volumes map[string]*VolumeConfig + var networks map[string]*NetworkConfig + if err := utils.Convert(config.Volumes, &volumes); err != nil { + return "", nil, nil, nil, err + } + if err := utils.Convert(config.Networks, &networks); err != nil { + return "", nil, nil, nil, err + } + + return config.Version, serviceConfigs, volumes, networks, nil +} + +// InterpolateRawServiceMap replaces varialbse in raw service map struct based on environment lookup +func InterpolateRawServiceMap(baseRawServices *RawServiceMap, environmentLookup EnvironmentLookup) error { + for k, v := range *baseRawServices { + for k2, v2 := range v { + if err := Interpolate(k2, &v2, environmentLookup); err != nil { + return err + } + (*baseRawServices)[k][k2] = v2 + } + } + return nil } func adjustValues(configs map[string]*ServiceConfig) { diff --git a/config/merge_test.go b/config/merge_test.go index c8fc06bdb..0cba787d0 100644 --- a/config/merge_test.go +++ b/config/merge_test.go @@ -405,20 +405,3 @@ services: } } } - -func TestNilNetworks(t *testing.T) { - composeFile := []byte(` -version: '2' -networks: - public:`) - - config, err := CreateConfig(composeFile) - if err != nil { - t.Fatal(err) - } - for key, networkConfig := range config.Networks { - if networkConfig == nil { - t.Fatalf("networkConfig %s was nil, shouldn't be", key) - } - } -} diff --git a/config/merge_v1.go b/config/merge_v1.go index c1eeeae73..dab39144f 100644 --- a/config/merge_v1.go +++ b/config/merge_v1.go @@ -10,20 +10,6 @@ import ( // MergeServicesV1 merges a v1 compose file into an existing set of service configs func MergeServicesV1(existingServices *ServiceConfigs, environmentLookup EnvironmentLookup, resourceLookup ResourceLookup, file string, datas RawServiceMap, options *ParseOptions) (map[string]*ServiceConfigV1, error) { - if options.Interpolate { - if err := Interpolate(environmentLookup, &datas); err != nil { - return nil, err - } - } - - if options.Preprocess != nil { - var err error - datas, err = options.Preprocess(datas) - if err != nil { - return nil, err - } - } - if options.Validate { if err := validate(datas); err != nil { return nil, err @@ -117,8 +103,7 @@ func parseV1(resourceLookup ResourceLookup, environmentLookup EnvironmentLookup, baseRawServices := config.Services if options.Interpolate { - err = Interpolate(environmentLookup, &baseRawServices) - if err != nil { + if err = InterpolateRawServiceMap(&baseRawServices, environmentLookup); err != nil { return nil, err } } diff --git a/config/merge_v2.go b/config/merge_v2.go index e9e5678ba..085829e41 100644 --- a/config/merge_v2.go +++ b/config/merge_v2.go @@ -10,20 +10,6 @@ import ( // MergeServicesV2 merges a v2 compose file into an existing set of service configs func MergeServicesV2(existingServices *ServiceConfigs, environmentLookup EnvironmentLookup, resourceLookup ResourceLookup, file string, datas RawServiceMap, options *ParseOptions) (map[string]*ServiceConfig, error) { - if options.Interpolate { - if err := Interpolate(environmentLookup, &datas); err != nil { - return nil, err - } - } - - if options.Preprocess != nil { - var err error - datas, err = options.Preprocess(datas) - if err != nil { - return nil, err - } - } - if options.Validate { if err := validateV2(datas); err != nil { return nil, err @@ -108,8 +94,7 @@ func parseV2(resourceLookup ResourceLookup, environmentLookup EnvironmentLookup, baseRawServices := config.Services if options.Interpolate { - err = Interpolate(environmentLookup, &baseRawServices) - if err != nil { + if err = InterpolateRawServiceMap(&baseRawServices, environmentLookup); err != nil { return nil, err } } diff --git a/config/types.go b/config/types.go index 501adfcdd..7b0f10916 100644 --- a/config/types.go +++ b/config/types.go @@ -8,7 +8,7 @@ import ( // EnvironmentLookup defines methods to provides environment variable loading. type EnvironmentLookup interface { - Lookup(key, serviceName string, config *ServiceConfig) []string + Lookup(key string, config *ServiceConfig) []string } // ResourceLookup defines methods to provides file loading. @@ -172,10 +172,10 @@ type NetworkConfig struct { // Config holds libcompose top level configuration type Config struct { - Version string `yaml:"version,omitempty"` - Services RawServiceMap `yaml:"services,omitempty"` - Volumes map[string]*VolumeConfig `yaml:"volumes,omitempty"` - Networks map[string]*NetworkConfig `yaml:"networks,omitempty"` + Version string `yaml:"version,omitempty"` + Services RawServiceMap `yaml:"services,omitempty"` + Volumes map[string]interface{} `yaml:"volumes,omitempty"` + Networks map[string]interface{} `yaml:"networks,omitempty"` } // NewServiceConfigs initializes a new Configs struct diff --git a/lookup/composable.go b/lookup/composable.go index c48987fef..ff76480b3 100644 --- a/lookup/composable.go +++ b/lookup/composable.go @@ -13,10 +13,10 @@ type ComposableEnvLookup struct { // Lookup creates a string slice of string containing a "docker-friendly" environment string // in the form of 'key=value'. It loop through the lookups and returns the latest value if // more than one lookup return a result. -func (l *ComposableEnvLookup) Lookup(key, serviceName string, config *config.ServiceConfig) []string { +func (l *ComposableEnvLookup) Lookup(key string, config *config.ServiceConfig) []string { result := []string{} for _, lookup := range l.Lookups { - env := lookup.Lookup(key, serviceName, config) + env := lookup.Lookup(key, config) if len(env) == 1 { result = env } diff --git a/lookup/composable_test.go b/lookup/composable_test.go index c7d04cbbf..a371f6fda 100644 --- a/lookup/composable_test.go +++ b/lookup/composable_test.go @@ -10,13 +10,13 @@ type simpleEnvLookup struct { value []string } -func (l *simpleEnvLookup) Lookup(key, serviceName string, config *config.ServiceConfig) []string { +func (l *simpleEnvLookup) Lookup(key string, config *config.ServiceConfig) []string { return l.value } func TestComposableLookupWithoutAnyLookup(t *testing.T) { envLookup := &ComposableEnvLookup{} - actuals := envLookup.Lookup("any", "", nil) + actuals := envLookup.Lookup("any", nil) if len(actuals) != 0 { t.Fatalf("expected an empty slice, got %v", actuals) } @@ -35,7 +35,7 @@ func TestComposableLookupReturnsTheLastValue(t *testing.T) { envLookup2, }, } - validateLookup(t, "value=2", envLookup.Lookup("value", "", nil)) + validateLookup(t, "value=2", envLookup.Lookup("value", nil)) envLookup = &ComposableEnvLookup{ []config.EnvironmentLookup{ @@ -43,5 +43,5 @@ func TestComposableLookupReturnsTheLastValue(t *testing.T) { envLookup1, }, } - validateLookup(t, "value=1", envLookup.Lookup("value", "", nil)) + validateLookup(t, "value=1", envLookup.Lookup("value", nil)) } diff --git a/lookup/envfile.go b/lookup/envfile.go index 65bd98674..62241255c 100644 --- a/lookup/envfile.go +++ b/lookup/envfile.go @@ -16,7 +16,7 @@ type EnvfileLookup struct { // Lookup creates a string slice of string containing a "docker-friendly" environment string // in the form of 'key=value'. It gets environment values using a '.env' file in the specified // path. -func (l *EnvfileLookup) Lookup(key, serviceName string, config *config.ServiceConfig) []string { +func (l *EnvfileLookup) Lookup(key string, config *config.ServiceConfig) []string { envs, err := opts.ParseEnvFile(l.Path) if err != nil { return []string{} diff --git a/lookup/envfile_test.go b/lookup/envfile_test.go index 812c42e01..c869bce05 100644 --- a/lookup/envfile_test.go +++ b/lookup/envfile_test.go @@ -11,7 +11,7 @@ func TestEnvfileLookupReturnsEmptyIfError(t *testing.T) { envfileLookup := &EnvfileLookup{ Path: "anything/file.env", } - actuals := envfileLookup.Lookup("any", "", nil) + actuals := envfileLookup.Lookup("any", nil) if len(actuals) != 0 { t.Fatalf("expected an empty slice, got %v", actuals) } @@ -40,11 +40,11 @@ and_underscore=working too Path: envfile, } - validateLookup(t, "baz=quux", envfileLookup.Lookup("baz", "", nil)) - validateLookup(t, "foo=bar", envfileLookup.Lookup("foo", "", nil)) - validateLookup(t, "_foobar=foobaz", envfileLookup.Lookup("_foobar", "", nil)) - validateLookup(t, "with.dots=working", envfileLookup.Lookup("with.dots", "", nil)) - validateLookup(t, "and_underscore=working too", envfileLookup.Lookup("and_underscore", "", nil)) + validateLookup(t, "baz=quux", envfileLookup.Lookup("baz", nil)) + validateLookup(t, "foo=bar", envfileLookup.Lookup("foo", nil)) + validateLookup(t, "_foobar=foobaz", envfileLookup.Lookup("_foobar", nil)) + validateLookup(t, "with.dots=working", envfileLookup.Lookup("with.dots", nil)) + validateLookup(t, "and_underscore=working too", envfileLookup.Lookup("and_underscore", nil)) } func validateLookup(t *testing.T, expected string, actuals []string) { diff --git a/lookup/simple_env.go b/lookup/simple_env.go index 6f8c25bf2..40ce12843 100644 --- a/lookup/simple_env.go +++ b/lookup/simple_env.go @@ -15,7 +15,7 @@ type OsEnvLookup struct { // in the form of 'key=value'. It gets environment values using os.Getenv. // If the os environment variable does not exists, the slice is empty. serviceName and config // are not used at all in this implementation. -func (o *OsEnvLookup) Lookup(key, serviceName string, config *config.ServiceConfig) []string { +func (o *OsEnvLookup) Lookup(key string, config *config.ServiceConfig) []string { ret := os.Getenv(key) if ret == "" { return []string{} diff --git a/lookup/simple_env_test.go b/lookup/simple_env_test.go index c42624e64..1597915d7 100644 --- a/lookup/simple_env_test.go +++ b/lookup/simple_env_test.go @@ -5,23 +5,19 @@ import ( ) func TestOsEnvLookup(t *testing.T) { - // Putting bare minimun value for serviceName and config as there are - // not important on this test. - serviceName := "anything" - osEnvLookup := &OsEnvLookup{} - envs := osEnvLookup.Lookup("PATH", serviceName, nil) + envs := osEnvLookup.Lookup("PATH", nil) if len(envs) != 1 { t.Fatalf("Expected envs to contains one element, but was %v", envs) } - envs = osEnvLookup.Lookup("path", serviceName, nil) + envs = osEnvLookup.Lookup("path", nil) if len(envs) != 0 { t.Fatalf("Expected envs to be empty, but was %v", envs) } - envs = osEnvLookup.Lookup("DOES_NOT_EXIST", serviceName, nil) + envs = osEnvLookup.Lookup("DOES_NOT_EXIST", nil) if len(envs) != 0 { t.Fatalf("Expected envs to be empty, but was %v", envs) } diff --git a/project/project.go b/project/project.go index d74398dd3..f2ea3c58e 100644 --- a/project/project.go +++ b/project/project.go @@ -141,7 +141,7 @@ func (p *Project) CreateService(name string) (Service, error) { env = parts[0] } - for _, value := range p.context.EnvironmentLookup.Lookup(env, name, &config) { + for _, value := range p.context.EnvironmentLookup.Lookup(env, &config) { parsedEnv = append(parsedEnv, value) } } @@ -151,7 +151,7 @@ func (p *Project) CreateService(name string) (Service, error) { // check the environment for extra build Args that are set but not given a value in the compose file for arg, value := range config.Build.Args { if value == "\x00" { - envValue := p.context.EnvironmentLookup.Lookup(arg, name, &config) + envValue := p.context.EnvironmentLookup.Lookup(arg, &config) // depending on what we get back we do different things switch l := len(envValue); l { case 0: @@ -160,7 +160,7 @@ func (p *Project) CreateService(name string) (Service, error) { parts := strings.SplitN(envValue[0], "=", 2) config.Build.Args[parts[0]] = parts[1] default: - return nil, fmt.Errorf("Tried to set Build Arg %#v to multi-value %#v.", arg, envValue) + return nil, fmt.Errorf("tried to set Build Arg %#v to multi-value %#v", arg, envValue) } } } diff --git a/project/project_test.go b/project/project_test.go index c30ada375..73b356520 100644 --- a/project/project_test.go +++ b/project/project_test.go @@ -152,7 +152,7 @@ func TestParseWithDefaultEnvironmentLookup(t *testing.T) { type TestEnvironmentLookup struct { } -func (t *TestEnvironmentLookup) Lookup(key, serviceName string, config *config.ServiceConfig) []string { +func (t *TestEnvironmentLookup) Lookup(key string, config *config.ServiceConfig) []string { return []string{fmt.Sprintf("%s=X", key)} }