Merge pull request #35089 from Microsoft/jjh/fromplatformbuilder

LCOW - Change platform parser directive to FROM statement flag

builder/dockerfile/builder.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"runtime"
 	"strings"
 	"time"
 
@@ -104,18 +103,12 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) (
 		source = src
 	}
 
-	os := runtime.GOOS
+	os := ""
-	optionsPlatform := system.ParsePlatform(config.Options.Platform)
+	apiPlatform := system.ParsePlatform(config.Options.Platform)
-	if dockerfile.OS != "" {
+	if apiPlatform.OS != "" {
-		if optionsPlatform.OS != "" && optionsPlatform.OS != dockerfile.OS {
+		os = apiPlatform.OS
-			return nil, fmt.Errorf("invalid platform")
-		}
-		os = dockerfile.OS
-	} else if optionsPlatform.OS != "" {
-		os = optionsPlatform.OS
 	}
 	config.Options.Platform = os
-	dockerfile.OS = os
 
 	builderOptions := builderOptions{
 		Options:        config.Options,

builder/dockerfile/dispatchers.go

@@ -145,14 +145,17 @@ func (d *dispatchRequest) getImageMount(imageRefOrID string) (*imageMount, error
 		imageRefOrID = stage.Image
 		localOnly = true
 	}
-	return d.builder.imageSources.Get(imageRefOrID, localOnly)
+	return d.builder.imageSources.Get(imageRefOrID, localOnly, d.state.operatingSystem)
 }
 
-// FROM imagename[:tag | @digest] [AS build-stage-name]
+// FROM [--platform=platform] imagename[:tag | @digest] [AS build-stage-name]
 //
 func initializeStage(d dispatchRequest, cmd *instructions.Stage) error {
 	d.builder.imageProber.Reset()
-	image, err := d.getFromImage(d.shlex, cmd.BaseName)
+	if err := system.ValidatePlatform(&cmd.Platform); err != nil {
+		return err
+	}
+	image, err := d.getFromImage(d.shlex, cmd.BaseName, cmd.Platform.OS)
 	if err != nil {
 		return err
 	}
@@ -210,20 +213,41 @@ func (d *dispatchRequest) getExpandedImageName(shlex *shell.Lex, name string) (s
 	}
 	return name, nil
 }
-func (d *dispatchRequest) getImageOrStage(name string) (builder.Image, error) {
+
+// getOsFromFlagsAndStage calculates the operating system if we need to pull an image.
+// stagePlatform contains the value supplied by optional `--platform=` on
+// a current FROM statement. b.builder.options.Platform contains the operating
+// system part of the optional flag passed in the API call (or CLI flag
+// through `docker build --platform=...`). Precedence is for an explicit
+// platform indication in the FROM statement.
+func (d *dispatchRequest) getOsFromFlagsAndStage(stageOS string) string {
+	switch {
+	case stageOS != "":
+		return stageOS
+	case d.builder.options.Platform != "":
+		// Note this is API "platform", but by this point, as the daemon is not
+		// multi-arch aware yet, it is guaranteed to only hold the OS part here.
+		return d.builder.options.Platform
+	default:
+		return runtime.GOOS
+	}
+}
+
+func (d *dispatchRequest) getImageOrStage(name string, stageOS string) (builder.Image, error) {
 	var localOnly bool
 	if im, ok := d.stages.getByName(name); ok {
 		name = im.Image
 		localOnly = true
 	}
 
+	os := d.getOsFromFlagsAndStage(stageOS)
+
 	// Windows cannot support a container with no base image unless it is LCOW.
 	if name == api.NoBaseImageSpecifier {
 		imageImage := &image.Image{}
 		imageImage.OS = runtime.GOOS
 		if runtime.GOOS == "windows" {
-			optionsOS := system.ParsePlatform(d.builder.options.Platform).OS
+			switch os {
-			switch optionsOS {
 			case "windows", "":
 				return nil, errors.New("Windows does not support FROM scratch")
 			case "linux":
@@ -232,23 +256,23 @@ func (d *dispatchRequest) getImageOrStage(name string) (builder.Image, error) {
 				}
 				imageImage.OS = "linux"
 			default:
-				return nil, errors.Errorf("operating system %q is not supported", optionsOS)
+				return nil, errors.Errorf("operating system %q is not supported", os)
 			}
 		}
 		return builder.Image(imageImage), nil
 	}
-	imageMount, err := d.builder.imageSources.Get(name, localOnly)
+	imageMount, err := d.builder.imageSources.Get(name, localOnly, os)
 	if err != nil {
 		return nil, err
 	}
 	return imageMount.Image(), nil
 }
-func (d *dispatchRequest) getFromImage(shlex *shell.Lex, name string) (builder.Image, error) {
+func (d *dispatchRequest) getFromImage(shlex *shell.Lex, name string, stageOS string) (builder.Image, error) {
 	name, err := d.getExpandedImageName(shlex, name)
 	if err != nil {
 		return nil, err
 	}
-	return d.getImageOrStage(name)
+	return d.getImageOrStage(name, stageOS)
 }
 
 func dispatchOnbuild(d dispatchRequest, c *instructions.OnbuildCommand) error {
@@ -264,8 +288,7 @@ func dispatchOnbuild(d dispatchRequest, c *instructions.OnbuildCommand) error {
 func dispatchWorkdir(d dispatchRequest, c *instructions.WorkdirCommand) error {
 	runConfig := d.state.runConfig
 	var err error
-	baseImageOS := system.ParsePlatform(d.state.operatingSystem).OS
+	runConfig.WorkingDir, err = normalizeWorkdir(d.state.operatingSystem, runConfig.WorkingDir, c.Path)
-	runConfig.WorkingDir, err = normalizeWorkdir(baseImageOS, runConfig.WorkingDir, c.Path)
 	if err != nil {
 		return err
 	}
@@ -281,7 +304,7 @@ func dispatchWorkdir(d dispatchRequest, c *instructions.WorkdirCommand) error {
 	}
 
 	comment := "WORKDIR " + runConfig.WorkingDir
-	runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment, baseImageOS))
+	runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment, d.state.operatingSystem))
 	containerID, err := d.builder.probeAndCreate(d.state, runConfigWithCommentCmd)
 	if err != nil || containerID == "" {
 		return err
@@ -397,8 +420,7 @@ func prependEnvOnCmd(buildArgs *buildArgs, buildArgVars []string, cmd strslice.S
 //
 func dispatchCmd(d dispatchRequest, c *instructions.CmdCommand) error {
 	runConfig := d.state.runConfig
-	optionsOS := system.ParsePlatform(d.builder.options.Platform).OS
+	cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, d.state.operatingSystem)
-	cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, optionsOS)
 	runConfig.Cmd = cmd
 	// set config as already being escaped, this prevents double escaping on windows
 	runConfig.ArgsEscaped = true
@@ -441,8 +463,7 @@ func dispatchHealthcheck(d dispatchRequest, c *instructions.HealthCheckCommand)
 //
 func dispatchEntrypoint(d dispatchRequest, c *instructions.EntrypointCommand) error {
 	runConfig := d.state.runConfig
-	optionsOS := system.ParsePlatform(d.builder.options.Platform).OS
+	cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, d.state.operatingSystem)
-	cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, optionsOS)
 	runConfig.Entrypoint = cmd
 	if !d.state.cmdSet {
 		runConfig.Cmd = nil

builder/dockerfile/dispatchers_test.go

@@ -208,6 +208,7 @@ func TestOnbuild(t *testing.T) {
 func TestWorkdir(t *testing.T) {
 	b := newBuilderWithMockBackend()
 	sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults())
+	sb.state.baseImage = &mockImage{}
 	workingDir := "/app"
 	if runtime.GOOS == "windows" {
 		workingDir = "C:\\app"
@@ -224,6 +225,7 @@ func TestWorkdir(t *testing.T) {
 func TestCmd(t *testing.T) {
 	b := newBuilderWithMockBackend()
 	sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults())
+	sb.state.baseImage = &mockImage{}
 	command := "./executable"
 
 	cmd := &instructions.CmdCommand{
@@ -281,6 +283,7 @@ func TestHealthcheckCmd(t *testing.T) {
 func TestEntrypoint(t *testing.T) {
 	b := newBuilderWithMockBackend()
 	sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults())
+	sb.state.baseImage = &mockImage{}
 	entrypointCmd := "/usr/sbin/nginx"
 
 	cmd := &instructions.EntrypointCommand{
@@ -356,6 +359,7 @@ func TestStopSignal(t *testing.T) {
 	}
 	b := newBuilderWithMockBackend()
 	sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults())
+	sb.state.baseImage = &mockImage{}
 	signal := "SIGKILL"
 
 	cmd := &instructions.StopSignalCommand{

builder/dockerfile/evaluator.go

@@ -37,8 +37,7 @@ import (
 
 func dispatch(d dispatchRequest, cmd instructions.Command) (err error) {
 	if c, ok := cmd.(instructions.PlatformSpecific); ok {
-		optionsOS := system.ParsePlatform(d.builder.options.Platform).OS
+		err := c.CheckPlatform(d.state.operatingSystem)
-		err := c.CheckPlatform(optionsOS)
 		if err != nil {
 			return errdefs.InvalidParameter(err)
 		}

builder/dockerfile/imagecontext.go

@@ -6,13 +6,12 @@ import (
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/builder"
 	dockerimage "github.com/docker/docker/image"
-	"github.com/docker/docker/pkg/system"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 )
 
-type getAndMountFunc func(string, bool) (builder.Image, builder.ROLayer, error)
+type getAndMountFunc func(string, bool, string) (builder.Image, builder.ROLayer, error)
 
 // imageSources mounts images and provides a cache for mounted images. It tracks
 // all images so they can be unmounted at the end of the build.
@@ -23,7 +22,7 @@ type imageSources struct {
 }
 
 func newImageSources(ctx context.Context, options builderOptions) *imageSources {
-	getAndMount := func(idOrRef string, localOnly bool) (builder.Image, builder.ROLayer, error) {
+	getAndMount := func(idOrRef string, localOnly bool, osForPull string) (builder.Image, builder.ROLayer, error) {
 		pullOption := backend.PullOptionNoPull
 		if !localOnly {
 			if options.Options.PullParent {
@@ -32,12 +31,11 @@ func newImageSources(ctx context.Context, options builderOptions) *imageSources
 				pullOption = backend.PullOptionPreferLocal
 			}
 		}
-		optionsPlatform := system.ParsePlatform(options.Options.Platform)
 		return options.Backend.GetImageAndReleasableLayer(ctx, idOrRef, backend.GetImageAndLayerOptions{
 			PullOption: pullOption,
 			AuthConfig: options.Options.AuthConfigs,
 			Output:     options.ProgressWriter.Output,
-			OS:         optionsPlatform.OS,
+			OS:         osForPull,
 		})
 	}
 
@@ -47,12 +45,12 @@ func newImageSources(ctx context.Context, options builderOptions) *imageSources
 	}
 }
 
-func (m *imageSources) Get(idOrRef string, localOnly bool) (*imageMount, error) {
+func (m *imageSources) Get(idOrRef string, localOnly bool, osForPull string) (*imageMount, error) {
 	if im, ok := m.byImageID[idOrRef]; ok {
 		return im, nil
 	}
 
-	image, layer, err := m.getImage(idOrRef, localOnly)
+	image, layer, err := m.getImage(idOrRef, localOnly, osForPull)
 	if err != nil {
 		return nil, err
 	}

builder/dockerfile/instructions/commands.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // KeyValuePair represent an arbitrary named value (useful in slice instead of map[string] string to preserve ordering)
@@ -361,6 +362,7 @@ type Stage struct {
 	Commands   []Command
 	BaseName   string
 	SourceCode string
+	Platform   specs.Platform
 }
 
 // AddCommand to the stage

builder/dockerfile/instructions/parse.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/docker/builder/dockerfile/command"
 	"github.com/docker/docker/builder/dockerfile/parser"
+	"github.com/docker/docker/pkg/system"
 	"github.com/pkg/errors"
 )
 
@@ -271,16 +272,17 @@ func parseFrom(req parseRequest) (*Stage, error) {
 		return nil, err
 	}
 
+	flPlatform := req.flags.AddString("platform", "")
 	if err := req.flags.Parse(); err != nil {
 		return nil, err
 	}
 	code := strings.TrimSpace(req.original)
-
 	return &Stage{
 		BaseName:   req.args[0],
 		Name:       stageName,
 		SourceCode: code,
 		Commands:   []Command{},
+		Platform:   *system.ParsePlatform(flPlatform.Value),
 	}, nil
 
 }

builder/dockerfile/instructions/parse_test.go

@@ -196,5 +196,4 @@ func TestErrorCases(t *testing.T) {
 		_, err = ParseInstruction(n)
 		testutil.ErrorContains(t, err, c.expectedError)
 	}
-
 }

builder/dockerfile/internals.go

@@ -83,8 +83,7 @@ func (b *Builder) commit(dispatchState *dispatchState, comment string) error {
 		return errors.New("Please provide a source image with `from` prior to commit")
 	}
 
-	optionsPlatform := system.ParsePlatform(b.options.Platform)
+	runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment, dispatchState.operatingSystem))
-	runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment, optionsPlatform.OS))
 	hit, err := b.probeCache(dispatchState, runConfigWithCommentCmd)
 	if err != nil || hit {
 		return err
@@ -164,16 +163,15 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error
 	commentStr := fmt.Sprintf("%s %s%s in %s ", inst.cmdName, chownComment, srcHash, inst.dest)
 
 	// TODO: should this have been using origPaths instead of srcHash in the comment?
-	optionsPlatform := system.ParsePlatform(b.options.Platform)
 	runConfigWithCommentCmd := copyRunConfig(
 		state.runConfig,
-		withCmdCommentString(commentStr, optionsPlatform.OS))
+		withCmdCommentString(commentStr, state.operatingSystem))
 	hit, err := b.probeCache(state, runConfigWithCommentCmd)
 	if err != nil || hit {
 		return err
 	}
 
-	imageMount, err := b.imageSources.Get(state.imageID, true)
+	imageMount, err := b.imageSources.Get(state.imageID, true, state.operatingSystem)
 	if err != nil {
 		return errors.Wrapf(err, "failed to get destination image %q", state.imageID)
 	}
@@ -184,7 +182,7 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error
 	}
 	defer rwLayer.Release()
 
-	destInfo, err := createDestInfo(state.runConfig.WorkingDir, inst, rwLayer, b.options.Platform)
+	destInfo, err := createDestInfo(state.runConfig.WorkingDir, inst, rwLayer, state.operatingSystem)
 	if err != nil {
 		return err
 	}

builder/dockerfile/parser/parser.go

@@ -7,13 +7,11 @@ import (
 	"fmt"
 	"io"
 	"regexp"
-	"runtime"
 	"strconv"
 	"strings"
 	"unicode"
 
 	"github.com/docker/docker/builder/dockerfile/command"
-	"github.com/docker/docker/pkg/system"
 	"github.com/pkg/errors"
 )
 
@@ -81,11 +79,10 @@ func (node *Node) AddChild(child *Node, startLine, endLine int) {
 }
 
 var (
-	dispatch             map[string]func(string, *Directive) (*Node, map[string]bool, error)
+	dispatch           map[string]func(string, *Directive) (*Node, map[string]bool, error)
-	tokenWhitespace      = regexp.MustCompile(`[\t\v\f\r ]+`)
+	tokenWhitespace    = regexp.MustCompile(`[\t\v\f\r ]+`)
-	tokenEscapeCommand   = regexp.MustCompile(`^#[ \t]*escape[ \t]*=[ \t]*(?P<escapechar>.).*$`)
+	tokenEscapeCommand = regexp.MustCompile(`^#[ \t]*escape[ \t]*=[ \t]*(?P<escapechar>.).*$`)
-	tokenPlatformCommand = regexp.MustCompile(`^#[ \t]*platform[ \t]*=[ \t]*(?P<platform>.*)$`)
+	tokenComment       = regexp.MustCompile(`^#.*$`)
-	tokenComment         = regexp.MustCompile(`^#.*$`)
 )
 
 // DefaultEscapeToken is the default escape token
@@ -95,11 +92,9 @@ const DefaultEscapeToken = '\\'
 // parsing directives.
 type Directive struct {
 	escapeToken           rune           // Current escape token
-	platformToken         string         // Current platform token
 	lineContinuationRegex *regexp.Regexp // Current line continuation regex
 	processingComplete    bool           // Whether we are done looking for directives
 	escapeSeen            bool           // Whether the escape directive has been seen
-	platformSeen          bool           // Whether the platform directive has been seen
 }
 
 // setEscapeToken sets the default token for escaping characters in a Dockerfile.
@@ -112,25 +107,9 @@ func (d *Directive) setEscapeToken(s string) error {
 	return nil
 }
 
-// setPlatformToken sets the default platform for pulling images in a Dockerfile.
+// possibleParserDirective looks for parser directives, eg '# escapeToken=<char>'.
-func (d *Directive) setPlatformToken(s string) error {
+// Parser directives must precede any builder instruction or other comments,
-	s = strings.ToLower(s)
+// and cannot be repeated.
-	valid := []string{runtime.GOOS}
-	if system.LCOWSupported() {
-		valid = append(valid, "linux")
-	}
-	for _, item := range valid {
-		if s == item {
-			d.platformToken = s
-			return nil
-		}
-	}
-	return fmt.Errorf("invalid PLATFORM '%s'. Must be one of %v", s, valid)
-}
-
-// possibleParserDirective looks for one or more parser directives '# escapeToken=<char>' and
-// '# platform=<string>'. Parser directives must precede any builder instruction
-// or other comments, and cannot be repeated.
 func (d *Directive) possibleParserDirective(line string) error {
 	if d.processingComplete {
 		return nil
@@ -149,22 +128,6 @@ func (d *Directive) possibleParserDirective(line string) error {
 		}
 	}
 
-	// Only recognise a platform token if LCOW is supported
-	if system.LCOWSupported() {
-		tpcMatch := tokenPlatformCommand.FindStringSubmatch(strings.ToLower(line))
-		if len(tpcMatch) != 0 {
-			for i, n := range tokenPlatformCommand.SubexpNames() {
-				if n == "platform" {
-					if d.platformSeen {
-						return errors.New("only one platform parser directive can be used")
-					}
-					d.platformSeen = true
-					return d.setPlatformToken(tpcMatch[i])
-				}
-			}
-		}
-	}
-
 	d.processingComplete = true
 	return nil
 }
@@ -237,10 +200,7 @@ func newNodeFromLine(line string, directive *Directive) (*Node, error) {
 type Result struct {
 	AST         *Node
 	EscapeToken rune
-	// TODO @jhowardmsft - see https://github.com/moby/moby/issues/34617
+	Warnings    []string
-	// This next field will be removed in a future update for LCOW support.
-	OS       string
-	Warnings []string
 }
 
 // PrintWarnings to the writer
@@ -320,7 +280,6 @@ func Parse(rwc io.Reader) (*Result, error) {
 		AST:         root,
 		Warnings:    warnings,
 		EscapeToken: d.escapeToken,
-		OS:          d.platformToken,
 	}, handleScannerError(scanner.Err())
 }
 

daemon/images/image_history.go

@@ -7,6 +7,7 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types/image"
 	"github.com/docker/docker/layer"
+	"github.com/docker/docker/pkg/system"
 )
 
 // ImageHistory returns a slice of ImageHistory structures for the specified image
@@ -31,7 +32,9 @@ func (i *ImageService) ImageHistory(name string) ([]*image.HistoryResponseItem,
 			if len(img.RootFS.DiffIDs) <= layerCounter {
 				return nil, fmt.Errorf("too many non-empty layers in History section")
 			}
-
+			if !system.IsOSSupported(img.OperatingSystem()) {
+				return nil, system.ErrNotSupportedOperatingSystem
+			}
 			rootFS.Append(img.RootFS.DiffIDs[layerCounter])
 			l, err := i.layerStores[img.OperatingSystem()].Get(rootFS.ChainID())
 			if err != nil {

daemon/images/images.go

@@ -271,7 +271,9 @@ func (i *ImageService) SquashImage(id, parent string) (string, error) {
 		rootFS := image.NewRootFS()
 		parentImg = &image.Image{RootFS: rootFS}
 	}
-
+	if !system.IsOSSupported(img.OperatingSystem()) {
+		return "", errors.Wrap(err, system.ErrNotSupportedOperatingSystem.Error())
+	}
 	l, err := i.layerStores[img.OperatingSystem()].Get(img.RootFS.ChainID())
 	if err != nil {
 		return "", errors.Wrap(err, "error getting image layer")