Merge pull request #37011 from arm64b/ReAdd-LABEL-command-4-target-option

Construct and add 'LABEL' command from 'label' option to last stage

builder/dockerfile/builder.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"sort"
 	"strings"
 	"time"
 
@@ -208,13 +209,26 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
 	return b
 }
 
+// Build 'LABEL' command(s) from '--label' options and add to the last stage
+func buildLabelOptions(labels map[string]string, stages []instructions.Stage) {
+	keys := []string{}
+	for key := range labels {
+		keys = append(keys, key)
+	}
+
+	// Sort the label to have a repeatable order
+	sort.Strings(keys)
+	for _, key := range keys {
+		value := labels[key]
+		stages[len(stages)-1].AddCommand(instructions.NewLabelCommand(key, value, true))
+	}
+}
+
 // Build runs the Dockerfile builder by parsing the Dockerfile and executing
 // the instructions from the file.
 func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*builder.Result, error) {
 	defer b.imageSources.Unmount()
 
-	addNodesForLabelOption(dockerfile.AST, b.options.Labels)
-
 	stages, metaArgs, err := instructions.Parse(dockerfile.AST)
 	if err != nil {
 		if instructions.IsUnknownInstruction(err) {
@@ -231,6 +245,9 @@ func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*buil
 		stages = stages[:targetIx+1]
 	}
 
+	// Add 'LABEL' command specified by '--label' option to the last stage
+	buildLabelOptions(b.options.Labels, stages)
+
 	dockerfile.PrintWarnings(b.Stderr)
 	dispatchState, err := b.dispatchDockerfileWithCancellation(stages, metaArgs, dockerfile.EscapeToken, source)
 	if err != nil {
@@ -333,15 +350,6 @@ func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions.
 	return dispatchRequest.state, nil
 }
 
-func addNodesForLabelOption(dockerfile *parser.Node, labels map[string]string) {
-	if len(labels) == 0 {
-		return
-	}
-
-	node := parser.NodeFromLabels(labels)
-	dockerfile.Children = append(dockerfile.Children, node)
-}
-
 // BuildFromConfig builds directly from `changes`, treating it as if it were the contents of a Dockerfile
 // It will:
 // - Call parse.Parse() to get an AST root for the concatenated Dockerfile entries.

builder/dockerfile/builder_test.go

@@ -1,35 +0,0 @@
-package dockerfile // import "github.com/docker/docker/builder/dockerfile"
-
-import (
-	"strings"
-	"testing"
-
-	"github.com/docker/docker/builder/dockerfile/parser"
-	"github.com/gotestyourself/gotestyourself/assert"
-	is "github.com/gotestyourself/gotestyourself/assert/cmp"
-)
-
-func TestAddNodesForLabelOption(t *testing.T) {
-	dockerfile := "FROM scratch"
-	result, err := parser.Parse(strings.NewReader(dockerfile))
-	assert.Check(t, err)
-
-	labels := map[string]string{
-		"org.e": "cli-e",
-		"org.d": "cli-d",
-		"org.c": "cli-c",
-		"org.b": "cli-b",
-		"org.a": "cli-a",
-	}
-	nodes := result.AST
-	addNodesForLabelOption(nodes, labels)
-
-	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'`,
-	}
-	assert.Check(t, is.Len(nodes.Children, 2))
-	for i, v := range nodes.Children {
-		assert.Check(t, is.Equal(expected[i], v.Original))
-	}
-}

builder/dockerfile/instructions/commands.go

@@ -110,17 +110,37 @@ type MaintainerCommand struct {
 	Maintainer string
 }
 
+// NewLabelCommand creates a new 'LABEL' command
+func NewLabelCommand(k string, v string, NoExp bool) *LabelCommand {
+	kvp := KeyValuePair{Key: k, Value: v}
+	c := "LABEL "
+	c += kvp.String()
+	nc := withNameAndCode{code: c, name: "label"}
+	cmd := &LabelCommand{
+		withNameAndCode: nc,
+		Labels: KeyValuePairs{
+			kvp,
+		},
+		noExpand: NoExp,
+	}
+	return cmd
+}
+
 // LabelCommand : LABEL some json data describing the image
 //
 // Sets the Label variable foo to bar,
 //
 type LabelCommand struct {
 	withNameAndCode
-	Labels KeyValuePairs // kvp slice instead of map to preserve ordering
+	Labels   KeyValuePairs // kvp slice instead of map to preserve ordering
+	noExpand bool
 }
 
 // Expand variables
 func (c *LabelCommand) Expand(expander SingleWordExpander) error {
+	if c.noExpand {
+		return nil
+	}
 	return expandKvpsInPlace(c.Labels, expander)
 }
 

builder/dockerfile/parser/line_parsers.go

@@ -10,12 +10,9 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"sort"
 	"strings"
 	"unicode"
 	"unicode/utf8"
-
-	"github.com/docker/docker/builder/dockerfile/command"
 )
 
 var (
@@ -205,34 +202,6 @@ func parseLabel(rest string, d *Directive) (*Node, map[string]bool, error) {
 	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))
-		// Value must be single quoted to prevent env variable expansion
-		// See https://github.com/docker/docker/issues/26027
-		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
 // value assignments, like `name1 name2= name3="" name4=value`.
 // Note that this is a stricter format than the old format of assignment,

builder/dockerfile/parser/line_parsers_test.go

@@ -42,32 +42,6 @@ func TestParseNameValNewFormat(t *testing.T) {
 	assert.DeepEqual(t, expected, node, cmpNodeOpt)
 }
 
-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, expected, node, cmpNodeOpt)
-}
-
 func TestParseNameValWithoutVal(t *testing.T) {
 	directive := Directive{}
 	// In Config.Env, a variable without `=` is removed from the environment. (#31634)

integration/build/build_test.go

@@ -173,6 +173,81 @@ func TestBuildMultiStageParentConfig(t *testing.T) {
 	assert.Check(t, is.Contains(image.Config.Env, "WHO=parent"))
 }
 
+// Test cases in #36996
+func TestBuildLabelWithTargets(t *testing.T) {
+	bldName := "build-a"
+	testLabels := map[string]string{
+		"foo":  "bar",
+		"dead": "beef",
+	}
+
+	dockerfile := `
+		FROM busybox AS target-a
+		CMD ["/dev"]
+		LABEL label-a=inline-a
+		FROM busybox AS target-b
+		CMD ["/dist"]
+		LABEL label-b=inline-b
+		`
+
+	ctx := context.Background()
+	source := fakecontext.New(t, "", fakecontext.WithDockerfile(dockerfile))
+	defer source.Close()
+
+	apiclient := testEnv.APIClient()
+	// For `target-a` build
+	resp, err := apiclient.ImageBuild(ctx,
+		source.AsTarReader(t),
+		types.ImageBuildOptions{
+			Remove:      true,
+			ForceRemove: true,
+			Tags:        []string{bldName},
+			Labels:      testLabels,
+			Target:      "target-a",
+		})
+	assert.NilError(t, err)
+	_, err = io.Copy(ioutil.Discard, resp.Body)
+	resp.Body.Close()
+	assert.NilError(t, err)
+
+	image, _, err := apiclient.ImageInspectWithRaw(ctx, bldName)
+	assert.NilError(t, err)
+
+	testLabels["label-a"] = "inline-a"
+	for k, v := range testLabels {
+		x, ok := image.Config.Labels[k]
+		assert.Assert(t, ok)
+		assert.Assert(t, x == v)
+	}
+
+	// For `target-b` build
+	bldName = "build-b"
+	delete(testLabels, "label-a")
+	resp, err = apiclient.ImageBuild(ctx,
+		source.AsTarReader(t),
+		types.ImageBuildOptions{
+			Remove:      true,
+			ForceRemove: true,
+			Tags:        []string{bldName},
+			Labels:      testLabels,
+			Target:      "target-b",
+		})
+	assert.NilError(t, err)
+	_, err = io.Copy(ioutil.Discard, resp.Body)
+	resp.Body.Close()
+	assert.NilError(t, err)
+
+	image, _, err = apiclient.ImageInspectWithRaw(ctx, bldName)
+	assert.NilError(t, err)
+
+	testLabels["label-b"] = "inline-b"
+	for k, v := range testLabels {
+		x, ok := image.Config.Labels[k]
+		assert.Assert(t, ok)
+		assert.Assert(t, x == v)
+	}
+}
+
 func TestBuildWithEmptyLayers(t *testing.T) {
 	dockerfile := `
 		FROM    busybox