Merge pull request #32063 from tonistiigi/named-contexts

Add support for named build stages

builder/dockerfile/dispatchers.go

@@ -187,20 +187,16 @@ func dispatchCopy(b *Builder, args []string, attributes map[string]bool, origina
 		return err
 	}
 
-	var contextID *int
+	var im *imageMount
 	if flFrom.IsUsed() {
 		var err error
-		context, err := strconv.Atoi(flFrom.Value)
+		im, err = b.imageContexts.get(flFrom.Value)
 		if err != nil {
-			return errors.Wrap(err, "from expects an integer value corresponding to the context number")
-		}
-		if err := b.imageContexts.validate(context); err != nil {
 			return err
 		}
-		contextID = &context
 	}
 
-	return b.runContextCommand(args, false, false, "COPY", contextID)
+	return b.runContextCommand(args, false, false, "COPY", im)
 }
 
 // FROM imagename
@@ -208,7 +204,13 @@ func dispatchCopy(b *Builder, args []string, attributes map[string]bool, origina
 // This sets the image the dockerfile will build on top of.
 //
 func from(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) != 1 {
+	ctxName := ""
+	if len(args) == 3 && strings.EqualFold(args[1], "as") {
+		ctxName = strings.ToLower(args[2])
+		if ok, _ := regexp.MatchString("^[a-z][a-z0-9-_\\.]*$", ctxName); !ok {
+			return errors.Errorf("invalid name for build stage: %q, name can't start with a number or contain symbols", ctxName)
+		}
+	} else if len(args) != 1 {
 		return errExactlyOneArgument("FROM")
 	}
 
@@ -221,8 +223,15 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
 	var image builder.Image
 
 	b.resetImageCache()
-	b.imageContexts.new()
+	if _, err := b.imageContexts.new(ctxName, true); err != nil {
+		return err
+	}
 
+	if im, ok := b.imageContexts.byName[name]; ok {
+		if len(im.ImageID()) > 0 {
+			image = im
+		}
+	} else {
 		// Windows cannot support a container with no base image.
 		if name == api.NoBaseImageSpecifier {
 			if runtime.GOOS == "windows" {
@@ -231,19 +240,15 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
 			b.image = ""
 			b.noBaseImage = true
 		} else {
-		// TODO: don't use `name`, instead resolve it to a digest
-		if !b.options.PullParent {
-			image, _ = b.docker.GetImageOnBuild(name)
-			// TODO: shouldn't we error out if error is different from "not found" ?
-		}
-		if image == nil {
 			var err error
-			image, err = b.docker.PullOnBuild(b.clientCtx, name, b.options.AuthConfigs, b.Output)
+			image, err = pullOrGetImage(b, name)
 			if err != nil {
 				return err
 			}
 		}
-		b.imageContexts.update(image.ImageID())
+	}
+	if image != nil {
+		b.imageContexts.update(image.ImageID(), image.RunConfig())
 	}
 	b.from = image
 
@@ -831,3 +836,33 @@ func getShell(c *container.Config) []string {
 	}
 	return append([]string{}, c.Shell[:]...)
 }
+
+// mountByRef creates an imageMount from a reference. pulling the image if needed.
+func mountByRef(b *Builder, name string) (*imageMount, error) {
+	image, err := pullOrGetImage(b, name)
+	if err != nil {
+		return nil, err
+	}
+	im, err := b.imageContexts.new("", false)
+	if err != nil {
+		return nil, err
+	}
+	im.id = image.ImageID()
+	return im, nil
+}
+
+func pullOrGetImage(b *Builder, name string) (builder.Image, error) {
+	var image builder.Image
+	if !b.options.PullParent {
+		image, _ = b.docker.GetImageOnBuild(name)
+		// TODO: shouldn't we error out if error is different from "not found" ?
+	}
+	if image == nil {
+		var err error
+		image, err = b.docker.PullOnBuild(b.clientCtx, name, b.options.AuthConfigs, b.Output)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return image, nil
+}

builder/dockerfile/imagecontext.go

@@ -1,9 +1,12 @@
 package dockerfile
 
 import (
+	"strconv"
+	"strings"
 	"sync"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/remotecontext"
 	"github.com/pkg/errors"
@@ -14,21 +17,30 @@ import (
 type imageContexts struct {
 	b      *Builder
 	list   []*imageMount
+	byName map[string]*imageMount
 	cache  *pathCache
 }
 
-type imageMount struct {
+func (ic *imageContexts) new(name string, increment bool) (*imageMount, error) {
-	id      string
+	im := &imageMount{ic: ic}
-	ctx     builder.Context
+	if len(name) > 0 {
-	release func() error
+		if ic.byName == nil {
+			ic.byName = make(map[string]*imageMount)
+		}
+		if _, ok := ic.byName[name]; ok {
+			return nil, errors.Errorf("duplicate name %s", name)
+		}
+		ic.byName[name] = im
+	}
+	if increment {
+		ic.list = append(ic.list, im)
+	}
+	return im, nil
 }
 
-func (ic *imageContexts) new() {
+func (ic *imageContexts) update(imageID string, runConfig *container.Config) {
-	ic.list = append(ic.list, &imageMount{})
-}
-
-func (ic *imageContexts) update(imageID string) {
 	ic.list[len(ic.list)-1].id = imageID
+	ic.list[len(ic.list)-1].runConfig = runConfig
 }
 
 func (ic *imageContexts) validate(i int) error {
@@ -42,16 +54,72 @@ func (ic *imageContexts) validate(i int) error {
 	return nil
 }
 
-func (ic *imageContexts) context(i int) (builder.Context, error) {
+func (ic *imageContexts) get(indexOrName string) (*imageMount, error) {
-	if err := ic.validate(i); err != nil {
+	index, err := strconv.Atoi(indexOrName)
+	if err == nil {
+		if err := ic.validate(index); err != nil {
 			return nil, err
 		}
-	im := ic.list[i]
+		return ic.list[index], nil
+	}
+	if im, ok := ic.byName[strings.ToLower(indexOrName)]; ok {
+		return im, nil
+	}
+	im, err := mountByRef(ic.b, indexOrName)
+	if err != nil {
+		return nil, errors.Wrapf(err, "invalid from flag value %s", indexOrName)
+	}
+	return im, nil
+}
+
+func (ic *imageContexts) unmount() (retErr error) {
+	for _, im := range ic.list {
+		if err := im.unmount(); err != nil {
+			logrus.Error(err)
+			retErr = err
+		}
+	}
+	for _, im := range ic.byName {
+		if err := im.unmount(); err != nil {
+			logrus.Error(err)
+			retErr = err
+		}
+	}
+	return
+}
+
+func (ic *imageContexts) getCache(id, path string) (interface{}, bool) {
+	if ic.cache != nil {
+		if id == "" {
+			return nil, false
+		}
+		return ic.cache.get(id + path)
+	}
+	return nil, false
+}
+
+func (ic *imageContexts) setCache(id, path string, v interface{}) {
+	if ic.cache != nil {
+		ic.cache.set(id+path, v)
+	}
+}
+
+// imageMount is a reference for getting access to a buildcontext that is backed
+// by an existing image
+type imageMount struct {
+	id        string
+	ctx       builder.Context
+	release   func() error
+	ic        *imageContexts
+	runConfig *container.Config
+}
+
+func (im *imageMount) context() (builder.Context, error) {
 	if im.ctx == nil {
 		if im.id == "" {
 			return nil, errors.Errorf("could not copy from empty context")
 		}
-		p, release, err := ic.b.docker.MountImage(im.id)
+		p, release, err := im.ic.b.docker.MountImage(im.id)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to mount %s", im.id)
 		}
@@ -59,40 +127,27 @@ func (ic *imageContexts) context(i int) (builder.Context, error) {
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to create lazycontext for %s", p)
 		}
-		logrus.Debugf("mounted image: %s %s", im.id, p)
 		im.release = release
 		im.ctx = ctx
 	}
 	return im.ctx, nil
 }
 
-func (ic *imageContexts) unmount() (retErr error) {
+func (im *imageMount) unmount() error {
-	for _, im := range ic.list {
 	if im.release != nil {
 		if err := im.release(); err != nil {
-				logrus.Error(errors.Wrapf(err, "failed to unmount previous build image"))
+			return errors.Wrapf(err, "failed to unmount previous build image %s", im.id)
-				retErr = err
 		}
+		im.release = nil
 	}
-	}
+	return nil
-	return
 }
 
-func (ic *imageContexts) getCache(i int, path string) (interface{}, bool) {
+func (im *imageMount) ImageID() string {
-	if ic.cache != nil {
+	return im.id
-		im := ic.list[i]
-		if im.id == "" {
-			return nil, false
-		}
-		return ic.cache.get(im.id + path)
-	}
-	return nil, false
-}
-
-func (ic *imageContexts) setCache(i int, path string, v interface{}) {
-	if ic.cache != nil {
-		ic.cache.set(ic.list[i].id+path, v)
 }
+func (im *imageMount) RunConfig() *container.Config {
+	return im.runConfig
 }
 
 type pathCache struct {

builder/dockerfile/internals.go

@@ -85,7 +85,7 @@ func (b *Builder) commit(id string, autoCmd strslice.StrSlice, comment string) e
 	}
 
 	b.image = imageID
-	b.imageContexts.update(imageID)
+	b.imageContexts.update(imageID, &autoConfig)
 	return nil
 }
 
@@ -94,7 +94,7 @@ type copyInfo struct {
 	decompress bool
 }
 
-func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalDecompression bool, cmdName string, contextID *int) error {
+func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalDecompression bool, cmdName string, imageSource *imageMount) error {
 	if len(args) < 2 {
 		return fmt.Errorf("Invalid %s format - at least two arguments required", cmdName)
 	}
@@ -128,7 +128,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
 			continue
 		}
 		// not a URL
-		subInfos, err := b.calcCopyInfo(cmdName, orig, allowLocalDecompression, true, contextID)
+		subInfos, err := b.calcCopyInfo(cmdName, orig, allowLocalDecompression, true, imageSource)
 		if err != nil {
 			return err
 		}
@@ -298,13 +298,13 @@ func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) {
 	return &builder.HashedFileInfo{FileInfo: builder.PathFileInfo{FileInfo: tmpFileSt, FilePath: tmpFileName}, FileHash: hash}, nil
 }
 
-func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression, allowWildcards bool, contextID *int) ([]copyInfo, error) {
+func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression, allowWildcards bool, imageSource *imageMount) ([]copyInfo, error) {
 
 	// Work in daemon-specific OS filepath semantics
 	origPath = filepath.FromSlash(origPath)
 
 	// validate windows paths from other images
-	if contextID != nil && runtime.GOOS == "windows" {
+	if imageSource != nil && runtime.GOOS == "windows" {
 		forbid := regexp.MustCompile("(?i)^" + string(os.PathSeparator) + "?(windows(" + string(os.PathSeparator) + ".+)?)?$")
 		if p := filepath.Clean(origPath); p == "." || forbid.MatchString(p) {
 			return nil, errors.Errorf("copy from %s is not allowed on windows", origPath)
@@ -318,8 +318,8 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 
 	context := b.context
 	var err error
-	if contextID != nil {
+	if imageSource != nil {
-		context, err = b.imageContexts.context(*contextID)
+		context, err = imageSource.context()
 		if err != nil {
 			return nil, err
 		}
@@ -346,7 +346,7 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 
 			// Note we set allowWildcards to false in case the name has
 			// a * in it
-			subInfos, err := b.calcCopyInfo(cmdName, path, allowLocalDecompression, false, contextID)
+			subInfos, err := b.calcCopyInfo(cmdName, path, allowLocalDecompression, false, imageSource)
 			if err != nil {
 				return err
 			}
@@ -370,9 +370,9 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 	if !handleHash {
 		return copyInfos, nil
 	}
-	if contextID != nil {
+	if imageSource != nil {
 		// fast-cache based on imageID
-		if h, ok := b.imageContexts.getCache(*contextID, origPath); ok {
+		if h, ok := b.imageContexts.getCache(imageSource.id, origPath); ok {
 			hfi.SetHash(h.(string))
 			return copyInfos, nil
 		}
@@ -401,8 +401,8 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 	hasher := sha256.New()
 	hasher.Write([]byte(strings.Join(subfiles, ",")))
 	hfi.SetHash("dir:" + hex.EncodeToString(hasher.Sum(nil)))
-	if contextID != nil {
+	if imageSource != nil {
-		b.imageContexts.setCache(*contextID, origPath, hfi.Hash())
+		b.imageContexts.setCache(imageSource.id, origPath, hfi.Hash())
 	}
 
 	return copyInfos, nil
@@ -497,7 +497,7 @@ func (b *Builder) probeCache() (bool, error) {
 	fmt.Fprint(b.Stdout, " ---> Using cache\n")
 	logrus.Debugf("[BUILDER] Use cached version: %s", b.runConfig.Cmd)
 	b.image = string(cache)
-	b.imageContexts.update(b.image)
+	b.imageContexts.update(b.image, b.runConfig)
 
 	return true, nil
 }

builder/dockerfile/parser/parser.go

@@ -106,7 +106,7 @@ func init() {
 		command.Entrypoint:  parseMaybeJSON,
 		command.Env:         parseEnv,
 		command.Expose:      parseStringsWhitespaceDelimited,
-		command.From:        parseString,
+		command.From:        parseStringsWhitespaceDelimited,
 		command.Healthcheck: parseHealthConfig,
 		command.Label:       parseLabel,
 		command.Maintainer:  parseString,

integration-cli/docker_cli_build_test.go

@@ -5752,7 +5752,7 @@ func (s *DockerSuite) TestBuildContChar(c *check.C) {
 
 func (s *DockerSuite) TestBuildCopyFromPreviousRootFS(c *check.C) {
 	dockerfile := `
-		FROM busybox
+		FROM busybox AS first
 		COPY foo bar
 		FROM busybox
     %s
@@ -5762,7 +5762,8 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFS(c *check.C) {
     COPY bar /
     COPY --from=1 baz sub/
     COPY --from=0 bar baz
-    COPY --from=0 bar bay`
+    COPY --from=first bar bay`
+
 	ctx := fakeContext(c, fmt.Sprintf(dockerfile, ""), map[string]string{
 		"Dockerfile": dockerfile,
 		"foo":        "abc",
@@ -5830,7 +5831,7 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFSErrors(c *check.C) {
 
 	buildImage("build1", withExternalBuildContext(ctx)).Assert(c, icmd.Expected{
 		ExitCode: 1,
-		Err:      "from expects an integer value corresponding to the context number",
+		Err:      "invalid from flag value foo",
 	})
 
 	dockerfile = `
@@ -5847,6 +5848,36 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFSErrors(c *check.C) {
 		ExitCode: 1,
 		Err:      "invalid from flag value 0 refers current build block",
 	})
+
+	dockerfile = `
+		FROM busybox AS foo
+		COPY --from=bar foo bar`
+
+	ctx = fakeContext(c, dockerfile, map[string]string{
+		"Dockerfile": dockerfile,
+		"foo":        "abc",
+	})
+	defer ctx.Close()
+
+	buildImage("build1", withExternalBuildContext(ctx)).Assert(c, icmd.Expected{
+		ExitCode: 1,
+		Err:      "invalid from flag value bar",
+	})
+
+	dockerfile = `
+		FROM busybox AS 1
+		COPY --from=1 foo bar`
+
+	ctx = fakeContext(c, dockerfile, map[string]string{
+		"Dockerfile": dockerfile,
+		"foo":        "abc",
+	})
+	defer ctx.Close()
+
+	buildImage("build1", withExternalBuildContext(ctx)).Assert(c, icmd.Expected{
+		ExitCode: 1,
+		Err:      "invalid name for build stage",
+	})
 }
 
 func (s *DockerSuite) TestBuildCopyFromPreviousFrom(c *check.C) {
@@ -5863,9 +5894,9 @@ func (s *DockerSuite) TestBuildCopyFromPreviousFrom(c *check.C) {
 	result.Assert(c, icmd.Success)
 
 	dockerfile = `
-		FROM build1:latest
+		FROM build1:latest AS foo
     FROM busybox
-		COPY --from=0 bar /
+		COPY --from=foo bar /
 		COPY foo /`
 	ctx = fakeContext(c, dockerfile, map[string]string{
 		"Dockerfile": dockerfile,
@@ -5882,6 +5913,117 @@ func (s *DockerSuite) TestBuildCopyFromPreviousFrom(c *check.C) {
 	c.Assert(strings.TrimSpace(out), check.Equals, "def")
 }
 
+func (s *DockerSuite) TestBuildCopyFromImplicitFrom(c *check.C) {
+	dockerfile := `
+		FROM busybox
+		COPY --from=busybox /etc/passwd /mypasswd
+		RUN cmp /etc/passwd /mypasswd`
+
+	if DaemonIsWindows() {
+		dockerfile = `
+			FROM busybox
+			COPY --from=busybox License.txt foo`
+	}
+
+	ctx := fakeContext(c, dockerfile, map[string]string{
+		"Dockerfile": dockerfile,
+	})
+	defer ctx.Close()
+
+	result := buildImage("build1", withExternalBuildContext(ctx))
+	result.Assert(c, icmd.Success)
+
+	if DaemonIsWindows() {
+		out, _ := dockerCmd(c, "run", "build1", "cat", "License.txt")
+		c.Assert(len(out), checker.GreaterThan, 10)
+		out2, _ := dockerCmd(c, "run", "build1", "cat", "foo")
+		c.Assert(out, check.Equals, out2)
+	}
+}
+
+func (s *DockerRegistrySuite) TestBuildCopyFromImplicitPullingFrom(c *check.C) {
+	repoName := fmt.Sprintf("%v/dockercli/testf", privateRegistryURL)
+
+	dockerfile := `
+		FROM busybox
+		COPY foo bar`
+	ctx := fakeContext(c, dockerfile, map[string]string{
+		"Dockerfile": dockerfile,
+		"foo":        "abc",
+	})
+	defer ctx.Close()
+
+	result := buildImage(repoName, withExternalBuildContext(ctx))
+	result.Assert(c, icmd.Success)
+
+	dockerCmd(c, "push", repoName)
+	dockerCmd(c, "rmi", repoName)
+
+	dockerfile = `
+		FROM busybox
+		COPY --from=%s bar baz`
+
+	ctx = fakeContext(c, fmt.Sprintf(dockerfile, repoName), map[string]string{
+		"Dockerfile": dockerfile,
+	})
+	defer ctx.Close()
+
+	result = buildImage("build1", withExternalBuildContext(ctx))
+	result.Assert(c, icmd.Success)
+
+	dockerCmdWithResult("run", "build1", "cat", "baz").Assert(c, icmd.Expected{Out: "abc"})
+}
+
+func (s *DockerSuite) TestBuildFromPreviousBlock(c *check.C) {
+	dockerfile := `
+		FROM busybox as foo
+		COPY foo /
+		FROM foo as foo1
+		RUN echo 1 >> foo
+		FROM foo as foO2
+		RUN echo 2 >> foo
+		FROM foo
+		COPY --from=foo1 foo f1
+		COPY --from=FOo2 foo f2
+		` // foo2 case also tests that names are canse insensitive
+	ctx := fakeContext(c, dockerfile, map[string]string{
+		"Dockerfile": dockerfile,
+		"foo":        "bar",
+	})
+	defer ctx.Close()
+
+	result := buildImage("build1", withExternalBuildContext(ctx))
+	result.Assert(c, icmd.Success)
+
+	dockerCmdWithResult("run", "build1", "cat", "foo").Assert(c, icmd.Expected{Out: "bar"})
+
+	dockerCmdWithResult("run", "build1", "cat", "f1").Assert(c, icmd.Expected{Out: "bar1"})
+
+	dockerCmdWithResult("run", "build1", "cat", "f2").Assert(c, icmd.Expected{Out: "bar2"})
+}
+
+func (s *DockerTrustSuite) TestCopyFromTrustedBuild(c *check.C) {
+	img1 := s.setupTrustedImage(c, "trusted-build1")
+	img2 := s.setupTrustedImage(c, "trusted-build2")
+	dockerFile := fmt.Sprintf(`
+	FROM %s AS build-base
+	RUN echo ok > /foo
+	FROM %s
+	COPY --from=build-base foo bar`, img1, img2)
+
+	name := "testcopyfromtrustedbuild"
+
+	r := buildImage(name, trustedBuild, build.WithDockerfile(dockerFile))
+	r.Assert(c, icmd.Expected{
+		Out: fmt.Sprintf("FROM %s@sha", img1[:len(img1)-7]),
+	})
+	r.Assert(c, icmd.Expected{
+		Out: fmt.Sprintf("FROM %s@sha", img2[:len(img2)-7]),
+	})
+
+	dockerCmdWithResult("run", name, "cat", "bar").Assert(c, icmd.Expected{Out: "ok"})
+}
+
 // TestBuildOpaqueDirectory tests that a build succeeds which
 // creates opaque directories.
 // See https://github.com/docker/docker/issues/25244