1. docker
  2. Commit d32efdbf8baba891a35c740454624206f41be94a
Browse code

Sort the build labels passed from `build --labels`

This fix tries to fix the issue in 29619 where
labels passed from `build --labels` are not sorted.
As a result, if multiple labels have been passed,
each `docker build --labels A=A --labels B=B --labels C=C`
will generate different layers.

This fix fixes the issue by sort the Labels before
they are concatenated to `LABEL ...`.

A unit test has been added to cover the changes

This fix fixes 29619.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Yong Tang authored on 2016/12/21 15:18:55
Showing 2 changed files
builder/dockerfile/builder.go
History View file @ d32efdb
... ... 
@@ -7,6 +7,7 @@ import (
7 7 
 	"io"
8 8 
 	"io/ioutil"
9 9 
 	"os"
10 
+	"sort"
10 11 
 	"strings"
11 12
12 13 
 	"github.com/Sirupsen/logrus"
... ... 
@@ -203,6 +204,28 @@ func sanitizeRepoAndTags(names []string) ([]reference.Named, error) {
203 203 
 	return repoAndTags, nil
204 204 
 }
205 205
206 
+func (b *Builder) processLabels() error {
207 
+	if len(b.options.Labels) == 0 {
208 
+		return nil
209 
+	}
210 
+
211 
+	var labels []string
212 
+	for k, v := range b.options.Labels {
213 
+		labels = append(labels, fmt.Sprintf("%q='%s'", k, v))
214 
+	}
215 
+	// Sort the label to have a repeatable order
216 
+	sort.Strings(labels)
217 
+
218 
+	line := "LABEL " + strings.Join(labels, " ")
219 
+	_, node, err := parser.ParseLine(line, &b.directive, false)
220 
+	if err != nil {
221 
+		return err
222 
+	}
223 
+	b.dockerfile.Children = append(b.dockerfile.Children, node)
224 
+
225 
+	return nil
226 
+}
227 
+
206 228 
 // build runs the Dockerfile builder from a context and a docker object that allows to make calls
207 229 
 // to Docker.
208 230 
 //
... ... 
@@ -233,16 +256,8 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
233 233 
 		return "", err
234 234 
 	}
235 235
236 
-	if len(b.options.Labels) > 0 {
237 
-		line := "LABEL "
238 
-		for k, v := range b.options.Labels {
239 
-			line += fmt.Sprintf("%q='%s' ", k, v)
240 
-		}
241 
-		_, node, err := parser.ParseLine(line, &b.directive, false)
242 
-		if err != nil {
243 
-			return "", err
244 
-		}
245 
-		b.dockerfile.Children = append(b.dockerfile.Children, node)
236 
+	if err := b.processLabels(); err != nil {
237 
+		return "", err
246 238 
 	}
247 239
248 240 
 	var shortImgID string
builder/dockerfile/builder_test.go
History View file @ d32efdb
249 241 
new file mode 100644
... ... 
@@ -0,0 +1,54 @@
0 
+package dockerfile
1 
+
2 
+import (
3 
+	"strings"
4 
+
5 
+	"testing"
6 
+
7 
+	"github.com/docker/docker/api/types"
8 
+	"github.com/docker/docker/api/types/container"
9 
+	"github.com/docker/docker/builder/dockerfile/parser"
10 
+)
11 
+
12 
+func TestBuildProcessLabels(t *testing.T) {
13 
+	dockerfile := "FROM scratch"
14 
+	d := parser.Directive{}
15 
+	parser.SetEscapeToken(parser.DefaultEscapeToken, &d)
16 
+	n, err := parser.Parse(strings.NewReader(dockerfile), &d)
17 
+	if err != nil {
18 
+		t.Fatalf("Error when parsing Dockerfile: %s", err)
19 
+	}
20 
+
21 
+	options := &types.ImageBuildOptions{
22 
+		Labels: map[string]string{
23 
+			"org.e": "cli-e",
24 
+			"org.d": "cli-d",
25 
+			"org.c": "cli-c",
26 
+			"org.b": "cli-b",
27 
+			"org.a": "cli-a",
28 
+		},
29 
+	}
30 
+	b := &Builder{
31 
+		runConfig:  &container.Config{},
32 
+		options:    options,
33 
+		directive:  d,
34 
+		dockerfile: n,
35 
+	}
36 
+	err = b.processLabels()
37 
+	if err != nil {
38 
+		t.Fatalf("Error when processing labels: %s", err)
39 
+	}
40 
+
41 
+	expected := []string{
42 
+		"FROM scratch",
43 
+		`LABEL "org.a"='cli-a' "org.b"='cli-b' "org.c"='cli-c' "org.d"='cli-d' "org.e"='cli-e'`,
44 
+	}
45 
+	if len(b.dockerfile.Children) != 2 {
46 
+		t.Fatalf("Expect 2, got %d", len(b.dockerfile.Children))
47 
+	}
48 
+	for i, v := range b.dockerfile.Children {
49 
+		if v.Original != expected[i] {
50 
+			t.Fatalf("Expect '%s' for %dth children, got, '%s'", expected[i], i, v.Original)
51 
+		}
52 
+	}
53 
+}