Fix --label on docker build when a single quote is used in the value

Signed-off-by: Daniel Nephin <dnephin@docker.com>

builder/dockerfile/builder.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"sort"
 	"strings"
 
 	"github.com/Sirupsen/logrus"
@@ -209,26 +208,13 @@ func sanitizeRepoAndTags(names []string) ([]reference.Named, error) {
 	return repoAndTags, nil
 }
 
-func (b *Builder) processLabels() error {
+func (b *Builder) processLabels() {
 	if len(b.options.Labels) == 0 {
-		return nil
+		return
 	}
 
-	var labels []string
-	for k, v := range b.options.Labels {
-		labels = append(labels, fmt.Sprintf("%q='%s'", k, v))
-	}
-	// Sort the label to have a repeatable order
-	sort.Strings(labels)
-
-	line := "LABEL " + strings.Join(labels, " ")
-	_, node, err := parser.ParseLine(line, &b.directive, false)
-	if err != nil {
-		return err
-	}
+	node := parser.NodeFromLabels(b.options.Labels)
 	b.dockerfile.Children = append(b.dockerfile.Children, node)
-
-	return nil
 }
 
 // build runs the Dockerfile builder from a context and a docker object that allows to make calls
@@ -263,9 +249,7 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
 		return "", err
 	}
 
-	if err := b.processLabels(); err != nil {
-		return "", err
-	}
+	b.processLabels()
 
 	var shortImgID string
 	total := len(b.dockerfile.Children)

builder/dockerfile/builder_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder/dockerfile/parser"
+	"github.com/docker/docker/pkg/testutil/assert"
 )
 
 func TestBuildProcessLabels(t *testing.T) {
@@ -15,9 +16,7 @@ func TestBuildProcessLabels(t *testing.T) {
 	d := parser.Directive{}
 	parser.SetEscapeToken(parser.DefaultEscapeToken, &d)
 	n, err := parser.Parse(strings.NewReader(dockerfile), &d)
-	if err != nil {
-		t.Fatalf("Error when parsing Dockerfile: %s", err)
-	}
+	assert.NilError(t, err)
 
 	options := &types.ImageBuildOptions{
 		Labels: map[string]string{
@@ -34,21 +33,14 @@ func TestBuildProcessLabels(t *testing.T) {
 		directive:  d,
 		dockerfile: n,
 	}
-	err = b.processLabels()
-	if err != nil {
-		t.Fatalf("Error when processing labels: %s", err)
-	}
+	b.processLabels()
 
 	expected := []string{
 		"FROM scratch",
 		`LABEL "org.a"='cli-a' "org.b"='cli-b' "org.c"='cli-c' "org.d"='cli-d' "org.e"='cli-e'`,
 	}
-	if len(b.dockerfile.Children) != 2 {
-		t.Fatalf("Expect 2, got %d", len(b.dockerfile.Children))
-	}
+	assert.Equal(t, len(b.dockerfile.Children), 2)
 	for i, v := range b.dockerfile.Children {
-		if v.Original != expected[i] {
-			t.Fatalf("Expect '%s' for %dth children, got, '%s'", expected[i], i, v.Original)
-		}
+		assert.Equal(t, v.Original, expected[i])
 	}
 }

builder/dockerfile/parser/line_parsers.go

@@ -10,15 +10,22 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"sort"
 	"strings"
 	"unicode"
 	"unicode/utf8"
+
+	"github.com/docker/docker/builder/dockerfile/command"
 )
 
 var (
 	errDockerfileNotStringArray = errors.New("When using JSON array syntax, arrays must be comprised of strings only.")
 )
 
+const (
+	commandLabel = "LABEL"
+)
+
 // ignore the current argument. This will still leave a command parsed, but
 // will not incorporate the arguments into the ast.
 func parseIgnore(rest string, d *Directive) (*Node, map[string]bool, error) {
@@ -133,7 +140,7 @@ func parseWords(rest string, d *Directive) []string {
 
 // parse environment like statements. Note that this does *not* handle
 // variable interpolation, which will be handled in the evaluator.
-func parseNameVal(rest string, key string, d *Directive) (*Node, map[string]bool, error) {
+func parseNameVal(rest string, key string, d *Directive) (*Node, error) {
 	// This is kind of tricky because we need to support the old
 	// variant:   KEY name value
 	// as well as the new one:    KEY name=value ...
@@ -142,57 +149,86 @@ func parseNameVal(rest string, key string, d *Directive) (*Node, map[string]bool
 
 	words := parseWords(rest, d)
 	if len(words) == 0 {
-		return nil, nil, nil
+		return nil, nil
 	}
 
-	var rootnode *Node
-
 	// Old format (KEY name value)
 	if !strings.Contains(words[0], "=") {
-		node := &Node{}
-		rootnode = node
-		strs := tokenWhitespace.Split(rest, 2)
-
-		if len(strs) < 2 {
-			return nil, nil, fmt.Errorf(key + " must have two arguments")
-		}
-
-		node.Value = strs[0]
-		node.Next = &Node{}
-		node.Next.Value = strs[1]
-	} else {
-		var prevNode *Node
-		for i, word := range words {
-			if !strings.Contains(word, "=") {
-				return nil, nil, fmt.Errorf("Syntax error - can't find = in %q. Must be of the form: name=value", word)
-			}
-			parts := strings.SplitN(word, "=", 2)
-
-			name := &Node{}
-			value := &Node{}
-
-			name.Next = value
-			name.Value = parts[0]
-			value.Value = parts[1]
-
-			if i == 0 {
-				rootnode = name
-			} else {
-				prevNode.Next = name
-			}
-			prevNode = value
+		parts := tokenWhitespace.Split(rest, 2)
+		if len(parts) < 2 {
+			return nil, fmt.Errorf(key + " must have two arguments")
 		}
+		return newKeyValueNode(parts[0], parts[1]), nil
 	}
 
-	return rootnode, nil, nil
+	var rootNode *Node
+	var prevNode *Node
+	for _, word := range words {
+		if !strings.Contains(word, "=") {
+			return nil, fmt.Errorf("Syntax error - can't find = in %q. Must be of the form: name=value", word)
+		}
+
+		parts := strings.SplitN(word, "=", 2)
+		node := newKeyValueNode(parts[0], parts[1])
+		rootNode, prevNode = appendKeyValueNode(node, rootNode, prevNode)
+	}
+
+	return rootNode, nil
+}
+
+func newKeyValueNode(key, value string) *Node {
+	return &Node{
+		Value: key,
+		Next:  &Node{Value: value},
+	}
+}
+
+func appendKeyValueNode(node, rootNode, prevNode *Node) (*Node, *Node) {
+	if rootNode == nil {
+		rootNode = node
+	}
+	if prevNode != nil {
+		prevNode.Next = node
+	}
+
+	prevNode = node.Next
+	return rootNode, prevNode
 }
 
 func parseEnv(rest string, d *Directive) (*Node, map[string]bool, error) {
-	return parseNameVal(rest, "ENV", d)
+	node, err := parseNameVal(rest, "ENV", d)
+	return node, nil, err
 }
 
 func parseLabel(rest string, d *Directive) (*Node, map[string]bool, error) {
-	return parseNameVal(rest, "LABEL", d)
+	node, err := parseNameVal(rest, commandLabel, d)
+	return node, nil, err
+}
+
+// NodeFromLabels returns a Node for the injected labels
+func NodeFromLabels(labels map[string]string) *Node {
+	keys := []string{}
+	for key := range labels {
+		keys = append(keys, key)
+	}
+	// Sort the label to have a repeatable order
+	sort.Strings(keys)
+
+	labelPairs := []string{}
+	var rootNode *Node
+	var prevNode *Node
+	for _, key := range keys {
+		value := labels[key]
+		labelPairs = append(labelPairs, fmt.Sprintf("%q='%s'", key, value))
+		node := newKeyValueNode(key, value)
+		rootNode, prevNode = appendKeyValueNode(node, rootNode, prevNode)
+	}
+
+	return &Node{
+		Value:    command.Label,
+		Original: commandLabel + " " + strings.Join(labelPairs, " "),
+		Next:     rootNode,
+	}
 }
 
 // parses a statement containing one or more keyword definition(s) and/or

builder/dockerfile/parser/line_parsers_test.go

@@ -0,0 +1,65 @@
+package parser
+
+import (
+	"github.com/docker/docker/pkg/testutil/assert"
+	"testing"
+)
+
+func TestParseNameValOldFormat(t *testing.T) {
+	directive := Directive{}
+	node, err := parseNameVal("foo bar", "LABEL", &directive)
+	assert.NilError(t, err)
+
+	expected := &Node{
+		Value: "foo",
+		Next:  &Node{Value: "bar"},
+	}
+	assert.DeepEqual(t, node, expected)
+}
+
+func TestParseNameValNewFormat(t *testing.T) {
+	directive := Directive{}
+	node, err := parseNameVal("foo=bar thing=star", "LABEL", &directive)
+	assert.NilError(t, err)
+
+	expected := &Node{
+		Value: "foo",
+		Next: &Node{
+			Value: "bar",
+			Next: &Node{
+				Value: "thing",
+				Next: &Node{
+					Value: "star",
+				},
+			},
+		},
+	}
+	assert.DeepEqual(t, node, expected)
+}
+
+func TestNodeFromLabels(t *testing.T) {
+	labels := map[string]string{
+		"foo":   "bar",
+		"weird": "'first second'",
+	}
+	expected := &Node{
+		Value:    "label",
+		Original: `LABEL "foo"='bar' "weird"=''first second''`,
+		Next: &Node{
+			Value: "foo",
+			Next: &Node{
+				Value: "bar",
+				Next: &Node{
+					Value: "weird",
+					Next: &Node{
+						Value: "'first second'",
+					},
+				},
+			},
+		},
+	}
+
+	node := NodeFromLabels(labels)
+	assert.DeepEqual(t, node, expected)
+
+}