Merge pull request #838 from vdemeester/fix-label-file-behavior

Fix `--label-file` weird behavior

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
 	}

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
 	}

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)
 }

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 {

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()
+}

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"}