From 2b17f4c8a8caad552025edb05a73db683fb8a5c6 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Fri, 26 Jan 2018 09:45:29 -0800 Subject: [PATCH] Fix `--label-file` weird behavior `--label-file` has the exact same behavior as `--env-file`, meaning any placeholder (i.e. a simple key, no `=` sign, no value), it will get the value from the environment variable. For `--label-file` it should just add an empty label. Signed-off-by: Vincent Demeester --- cli/command/container/opts.go | 4 +- cli/command/service/opts.go | 2 +- opts/envfile.go | 61 +----------------------------- opts/envfile_test.go | 12 +++--- opts/file.go | 71 +++++++++++++++++++++++++++++++++++ opts/parse.go | 22 ++++++++--- 6 files changed, 98 insertions(+), 74 deletions(-) create mode 100644 opts/file.go diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index 017a43657a..0adecca759 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -145,7 +145,7 @@ func addFlags(flags *pflag.FlagSet) *containerOptions { expose: opts.NewListOpts(nil), extraHosts: opts.NewListOpts(opts.ValidateExtraHost), groupAdd: opts.NewListOpts(nil), - labels: opts.NewListOpts(opts.ValidateEnv), + labels: opts.NewListOpts(opts.ValidateLabel), labelsFile: opts.NewListOpts(nil), linkLocalIPs: opts.NewListOpts(nil), links: opts.NewListOpts(opts.ValidateLink), @@ -410,7 +410,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*containerConfig, err } // collect all the environment variables for the container - envVariables, err := opts.ReadKVStrings(copts.envFile.GetAll(), copts.env.GetAll()) + envVariables, err := opts.ReadKVEnvStrings(copts.envFile.GetAll(), copts.env.GetAll()) if err != nil { return nil, err } diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index 19915050b9..e8f9718c7c 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -560,7 +560,7 @@ func (options *serviceOptions) ToStopGracePeriod(flags *pflag.FlagSet) *time.Dur func (options *serviceOptions) ToService(ctx context.Context, apiClient client.NetworkAPIClient, flags *pflag.FlagSet) (swarm.ServiceSpec, error) { var service swarm.ServiceSpec - envVariables, err := opts.ReadKVStrings(options.envFile.GetAll(), options.env.GetAll()) + envVariables, err := opts.ReadKVEnvStrings(options.envFile.GetAll(), options.env.GetAll()) if err != nil { return service, err } diff --git a/opts/envfile.go b/opts/envfile.go index f723799215..10054c896c 100644 --- a/opts/envfile.go +++ b/opts/envfile.go @@ -1,13 +1,7 @@ package opts import ( - "bufio" - "bytes" - "fmt" "os" - "strings" - "unicode" - "unicode/utf8" ) // ParseEnvFile reads a file with environment variables enumerated by lines @@ -24,58 +18,5 @@ import ( // environment variables, that's why we just strip leading whitespace and // nothing more. func ParseEnvFile(filename string) ([]string, error) { - fh, err := os.Open(filename) - if err != nil { - return []string{}, err - } - defer fh.Close() - - lines := []string{} - scanner := bufio.NewScanner(fh) - currentLine := 0 - utf8bom := []byte{0xEF, 0xBB, 0xBF} - for scanner.Scan() { - scannedBytes := scanner.Bytes() - if !utf8.Valid(scannedBytes) { - return []string{}, fmt.Errorf("env file %s contains invalid utf8 bytes at line %d: %v", filename, currentLine+1, scannedBytes) - } - // We trim UTF8 BOM - if currentLine == 0 { - scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom) - } - // trim the line from all leading whitespace first - line := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace) - currentLine++ - // line is not empty, and not starting with '#' - if len(line) > 0 && !strings.HasPrefix(line, "#") { - data := strings.SplitN(line, "=", 2) - - // trim the front of a variable, but nothing else - variable := strings.TrimLeft(data[0], whiteSpaces) - if strings.ContainsAny(variable, whiteSpaces) { - return []string{}, ErrBadEnvVariable{fmt.Sprintf("variable '%s' has white spaces", variable)} - } - - if len(data) > 1 { - - // pass the value through, no trimming - lines = append(lines, fmt.Sprintf("%s=%s", variable, data[1])) - } else { - // if only a pass-through variable is given, clean it up. - lines = append(lines, fmt.Sprintf("%s=%s", strings.TrimSpace(line), os.Getenv(line))) - } - } - } - return lines, scanner.Err() -} - -var whiteSpaces = " \t" - -// ErrBadEnvVariable typed error for bad environment variable -type ErrBadEnvVariable struct { - msg string -} - -func (e ErrBadEnvVariable) Error() string { - return fmt.Sprintf("poorly formatted environment: %s", e.msg) + return parseKeyValueFile(filename, os.Getenv) } diff --git a/opts/envfile_test.go b/opts/envfile_test.go index f3faabe3c0..6302263848 100644 --- a/opts/envfile_test.go +++ b/opts/envfile_test.go @@ -95,10 +95,10 @@ func TestParseEnvFileBadlyFormattedFile(t *testing.T) { _, err := ParseEnvFile(tmpFile) if err == nil { - t.Fatalf("Expected an ErrBadEnvVariable, got nothing") + t.Fatalf("Expected an ErrBadKey, got nothing") } - if _, ok := err.(ErrBadEnvVariable); !ok { - t.Fatalf("Expected an ErrBadEnvVariable, got [%v]", err) + if _, ok := err.(ErrBadKey); !ok { + t.Fatalf("Expected an ErrBadKey, got [%v]", err) } expectedMessage := "poorly formatted environment: variable 'f ' has white spaces" if err.Error() != expectedMessage { @@ -129,10 +129,10 @@ another invalid line` _, err := ParseEnvFile(tmpFile) if err == nil { - t.Fatalf("Expected an ErrBadEnvVariable, got nothing") + t.Fatalf("Expected an ErrBadKey, got nothing") } - if _, ok := err.(ErrBadEnvVariable); !ok { - t.Fatalf("Expected an ErrBadEnvVariable, got [%v]", err) + if _, ok := err.(ErrBadKey); !ok { + t.Fatalf("Expected an ErrBadKey, got [%v]", err) } expectedMessage := "poorly formatted environment: variable 'first line' has white spaces" if err.Error() != expectedMessage { diff --git a/opts/file.go b/opts/file.go new file mode 100644 index 0000000000..281905949b --- /dev/null +++ b/opts/file.go @@ -0,0 +1,71 @@ +package opts + +import ( + "bufio" + "bytes" + "fmt" + "os" + "strings" + "unicode" + "unicode/utf8" +) + +var whiteSpaces = " \t" + +// ErrBadKey typed error for bad environment variable +type ErrBadKey struct { + msg string +} + +func (e ErrBadKey) Error() string { + return fmt.Sprintf("poorly formatted environment: %s", e.msg) +} + +func parseKeyValueFile(filename string, emptyFn func(string) string) ([]string, error) { + fh, err := os.Open(filename) + if err != nil { + return []string{}, err + } + defer fh.Close() + + lines := []string{} + scanner := bufio.NewScanner(fh) + currentLine := 0 + utf8bom := []byte{0xEF, 0xBB, 0xBF} + for scanner.Scan() { + scannedBytes := scanner.Bytes() + if !utf8.Valid(scannedBytes) { + return []string{}, fmt.Errorf("env file %s contains invalid utf8 bytes at line %d: %v", filename, currentLine+1, scannedBytes) + } + // We trim UTF8 BOM + if currentLine == 0 { + scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom) + } + // trim the line from all leading whitespace first + line := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace) + currentLine++ + // line is not empty, and not starting with '#' + if len(line) > 0 && !strings.HasPrefix(line, "#") { + data := strings.SplitN(line, "=", 2) + + // trim the front of a variable, but nothing else + variable := strings.TrimLeft(data[0], whiteSpaces) + if strings.ContainsAny(variable, whiteSpaces) { + return []string{}, ErrBadKey{fmt.Sprintf("variable '%s' has white spaces", variable)} + } + + if len(data) > 1 { + // pass the value through, no trimming + lines = append(lines, fmt.Sprintf("%s=%s", variable, data[1])) + } else { + var value string + if emptyFn != nil { + value = emptyFn(line) + } + // if only a pass-through variable is given, clean it up. + lines = append(lines, fmt.Sprintf("%s=%s", strings.TrimSpace(line), value)) + } + } + } + return lines, scanner.Err() +} diff --git a/opts/parse.go b/opts/parse.go index a88ea1385a..679759deda 100644 --- a/opts/parse.go +++ b/opts/parse.go @@ -2,6 +2,7 @@ package opts import ( "fmt" + "os" "strconv" "strings" @@ -11,18 +12,29 @@ import ( // ReadKVStrings reads a file of line terminated key=value pairs, and overrides any keys // present in the file with additional pairs specified in the override parameter func ReadKVStrings(files []string, override []string) ([]string, error) { - envVariables := []string{} + return readKVStrings(files, override, nil) +} + +// ReadKVEnvStrings reads a file of line terminated key=value pairs, and overrides any keys +// present in the file with additional pairs specified in the override parameter. +// If a key has no value, it will get the value from the environment. +func ReadKVEnvStrings(files []string, override []string) ([]string, error) { + return readKVStrings(files, override, os.Getenv) +} + +func readKVStrings(files []string, override []string, emptyFn func(string) string) ([]string, error) { + variables := []string{} for _, ef := range files { - parsedVars, err := ParseEnvFile(ef) + parsedVars, err := parseKeyValueFile(ef, emptyFn) if err != nil { return nil, err } - envVariables = append(envVariables, parsedVars...) + variables = append(variables, parsedVars...) } // parse the '-e' and '--env' after, to allow override - envVariables = append(envVariables, override...) + variables = append(variables, override...) - return envVariables, nil + return variables, nil } // ConvertKVStringsToMap converts ["key=value"] to {"key":"value"}