diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 21bfc1a0b4..6ac76e877e 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -212,12 +212,20 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri return "", err } + if len(b.options.Labels) > 0 { + line := "LABEL " + for k, v := range b.options.Labels { + line += fmt.Sprintf("%q=%q ", k, v) + } + _, node, err := parser.ParseLine(line) + if err != nil { + return "", err + } + b.dockerfile.Children = append(b.dockerfile.Children, node) + } + var shortImgID string for i, n := range b.dockerfile.Children { - // we only want to add labels to the last layer - if i == len(b.dockerfile.Children)-1 { - b.addLabels() - } select { case <-b.clientCtx.Done(): logrus.Debug("Builder: build cancelled!") @@ -233,16 +241,6 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri return "", err } - // Commit the layer when there are only one children in - // the dockerfile, this is only the `FROM` tag, and - // build labels. Otherwise, the new image won't be - // labeled properly. - // Commit here, so the ID of the final image is reported - // properly. - if len(b.dockerfile.Children) == 1 && len(b.options.Labels) > 0 { - b.commit("", b.runConfig.Cmd, "") - } - shortImgID = stringid.TruncateID(b.image) fmt.Fprintf(b.Stdout, " ---> %s\n", shortImgID) if b.options.Remove { diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 6a937707f8..c3d727a891 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -40,19 +40,6 @@ import ( "github.com/docker/engine-api/types/strslice" ) -func (b *Builder) addLabels() { - // merge labels - if len(b.options.Labels) > 0 { - logrus.Debugf("[BUILDER] setting labels %v", b.options.Labels) - if b.runConfig.Labels == nil { - b.runConfig.Labels = make(map[string]string) - } - for kL, vL := range b.options.Labels { - b.runConfig.Labels[kL] = vL - } - } -} - func (b *Builder) commit(id string, autoCmd strslice.StrSlice, comment string) error { if b.disableCommit { return nil @@ -418,20 +405,7 @@ func (b *Builder) processImageFrom(img builder.Image) error { b.image = img.ImageID() if img.RunConfig() != nil { - imgConfig := *img.RunConfig() - // inherit runConfig labels from the current - // state if they've been set already. - // Ensures that images with only a FROM - // get the labels populated properly. - if b.runConfig.Labels != nil { - if imgConfig.Labels == nil { - imgConfig.Labels = make(map[string]string) - } - for k, v := range b.runConfig.Labels { - imgConfig.Labels[k] = v - } - } - b.runConfig = &imgConfig + b.runConfig = img.RunConfig() } } diff --git a/builder/dockerfile/parser/line_parsers.go b/builder/dockerfile/parser/line_parsers.go index 8cfd39bb2f..1d7ece43dc 100644 --- a/builder/dockerfile/parser/line_parsers.go +++ b/builder/dockerfile/parser/line_parsers.go @@ -34,7 +34,7 @@ func parseSubCommand(rest string) (*Node, map[string]bool, error) { return nil, nil, nil } - _, child, err := parseLine(rest) + _, child, err := ParseLine(rest) if err != nil { return nil, nil, err } diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index d16683a756..ece601a957 100644 --- a/builder/dockerfile/parser/parser.go +++ b/builder/dockerfile/parser/parser.go @@ -68,8 +68,8 @@ func init() { } } -// parse a line and return the remainder. -func parseLine(line string) (string, *Node, error) { +// ParseLine parse a line and return the remainder. +func ParseLine(line string) (string, *Node, error) { if line = stripComments(line); line == "" { return "", nil, nil } @@ -111,7 +111,7 @@ func Parse(rwc io.Reader) (*Node, error) { for scanner.Scan() { scannedLine := strings.TrimLeftFunc(scanner.Text(), unicode.IsSpace) currentLine++ - line, child, err := parseLine(scannedLine) + line, child, err := ParseLine(scannedLine) if err != nil { return nil, err } @@ -126,7 +126,7 @@ func Parse(rwc io.Reader) (*Node, error) { continue } - line, child, err = parseLine(line + newline) + line, child, err = ParseLine(line + newline) if err != nil { return nil, err } @@ -136,7 +136,7 @@ func Parse(rwc io.Reader) (*Node, error) { } } if child == nil && line != "" { - _, child, err = parseLine(line) + _, child, err = ParseLine(line) if err != nil { return nil, err } diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index b1347284d6..392a0efccc 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -6894,3 +6894,94 @@ func (s *DockerRegistryAuthHtpasswdSuite) TestBuildWithExternalAuth(c *check.C) out, _, err := runCommandWithOutput(buildCmd) c.Assert(err, check.IsNil, check.Commentf(out)) } + +// Test cases in #22036 +func (s *DockerSuite) TestBuildLabelsOverride(c *check.C) { + testRequires(c, DaemonIsLinux) + + // Command line option labels will always override + name := "scratchy" + expected := `{"bar":"from-flag","foo":"from-flag"}` + _, err := buildImage(name, + `FROM scratch + LABEL foo=from-dockerfile`, + true, "--label", "foo=from-flag", "--label", "bar=from-flag") + c.Assert(err, check.IsNil) + + res := inspectFieldJSON(c, name, "Config.Labels") + if res != expected { + c.Fatalf("Labels %s, expected %s", res, expected) + } + + name = "from" + expected = `{"foo":"from-dockerfile"}` + _, err = buildImage(name, + `FROM scratch + LABEL foo from-dockerfile`, + true) + c.Assert(err, check.IsNil) + + res = inspectFieldJSON(c, name, "Config.Labels") + if res != expected { + c.Fatalf("Labels %s, expected %s", res, expected) + } + + // Command line option label will override even via `FROM` + name = "new" + expected = `{"bar":"from-dockerfile2","foo":"new"}` + _, err = buildImage(name, + `FROM from + LABEL bar from-dockerfile2`, + true, "--label", "foo=new") + c.Assert(err, check.IsNil) + + res = inspectFieldJSON(c, name, "Config.Labels") + if res != expected { + c.Fatalf("Labels %s, expected %s", res, expected) + } + + // Command line option without a value set (--label foo, --label bar=) + // will be treated as --label foo="", --label bar="" + name = "scratchy2" + expected = `{"bar":"","foo":""}` + _, err = buildImage(name, + `FROM scratch + LABEL foo=from-dockerfile`, + true, "--label", "foo", "--label", "bar=") + c.Assert(err, check.IsNil) + + res = inspectFieldJSON(c, name, "Config.Labels") + if res != expected { + c.Fatalf("Labels %s, expected %s", res, expected) + } + + // Command line option without a value set (--label foo, --label bar=) + // will be treated as --label foo="", --label bar="" + // This time is for inherited images + name = "new2" + expected = `{"bar":"","foo":""}` + _, err = buildImage(name, + `FROM from + LABEL bar from-dockerfile2`, + true, "--label", "foo=", "--label", "bar") + c.Assert(err, check.IsNil) + + res = inspectFieldJSON(c, name, "Config.Labels") + if res != expected { + c.Fatalf("Labels %s, expected %s", res, expected) + } + + // Command line option labels with only `FROM` + name = "scratchy" + expected = `{"bar":"from-flag","foo":"from-flag"}` + _, err = buildImage(name, + `FROM scratch`, + true, "--label", "foo=from-flag", "--label", "bar=from-flag") + c.Assert(err, check.IsNil) + + res = inspectFieldJSON(c, name, "Config.Labels") + if res != expected { + c.Fatalf("Labels %s, expected %s", res, expected) + } + +}