From 98dd1fdca1f5b82cbc7066c4a48f9ddd8f135095 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 3 Oct 2017 10:20:41 -0700 Subject: [PATCH 1/7] Builder - parser - remove OS Signed-off-by: John Howard --- builder/dockerfile/parser/parser.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index 277176ee1c..1ff6fba560 100644 --- a/builder/dockerfile/parser/parser.go +++ b/builder/dockerfile/parser/parser.go @@ -237,10 +237,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 - // This next field will be removed in a future update for LCOW support. - OS string - Warnings []string + Warnings []string } // PrintWarnings to the writer @@ -320,7 +317,6 @@ func Parse(rwc io.Reader) (*Result, error) { AST: root, Warnings: warnings, EscapeToken: d.escapeToken, - OS: d.platformToken, }, handleScannerError(scanner.Err()) } From 735e5d22b7ca208acc9ad7373bb8f93167ee3f85 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 3 Oct 2017 11:32:54 -0700 Subject: [PATCH 2/7] Builder - dockerfile - just use API for now, and unit test fix Signed-off-by: John Howard --- builder/dockerfile/builder.go | 12 +++--------- builder/dockerfile/dispatchers_test.go | 1 + 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index d20bc0403a..2a41c1f0c3 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -105,17 +105,11 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) ( } os := runtime.GOOS - optionsPlatform := system.ParsePlatform(config.Options.Platform) - if dockerfile.OS != "" { - if optionsPlatform.OS != "" && optionsPlatform.OS != dockerfile.OS { - return nil, fmt.Errorf("invalid platform") - } - os = dockerfile.OS - } else if optionsPlatform.OS != "" { - os = optionsPlatform.OS + apiPlatform := system.ParsePlatform(config.Options.Platform) + if apiPlatform.OS != "" { + os = apiPlatform.OS } config.Options.Platform = os - dockerfile.OS = os builderOptions := builderOptions{ Options: config.Options, diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index 6d52e7e619..7ad0d8d3c2 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/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" From 9cae03900fc27ff39e913978ca8f084691954881 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 3 Oct 2017 11:38:37 -0700 Subject: [PATCH 3/7] Builder - Parser. Remove platform parser directive Signed-off-by: John Howard --- builder/dockerfile/parser/parser.go | 51 ++++------------------------- 1 file changed, 7 insertions(+), 44 deletions(-) diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index 1ff6fba560..b065b8a4ea 100644 --- a/builder/dockerfile/parser/parser.go +++ b/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) - tokenWhitespace = regexp.MustCompile(`[\t\v\f\r ]+`) - tokenEscapeCommand = regexp.MustCompile(`^#[ \t]*escape[ \t]*=[ \t]*(?P.).*$`) - tokenPlatformCommand = regexp.MustCompile(`^#[ \t]*platform[ \t]*=[ \t]*(?P.*)$`) - tokenComment = regexp.MustCompile(`^#.*$`) + dispatch map[string]func(string, *Directive) (*Node, map[string]bool, error) + tokenWhitespace = regexp.MustCompile(`[\t\v\f\r ]+`) + tokenEscapeCommand = regexp.MustCompile(`^#[ \t]*escape[ \t]*=[ \t]*(?P.).*$`) + 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. -func (d *Directive) setPlatformToken(s string) error { - s = strings.ToLower(s) - 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=' and -// '# platform='. Parser directives must precede any builder instruction -// or other comments, and cannot be repeated. +// possibleParserDirective looks for parser directives, eg '# escapeToken='. +// 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 } From 7f0c2d23e11485c7f026dd8c111c60c2e1e03375 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 3 Oct 2017 15:34:33 -0700 Subject: [PATCH 4/7] Builder - add --platform to FROM statement Signed-off-by: John Howard --- builder/dockerfile/dispatchers.go | 2 +- builder/dockerfile/instructions/commands.go | 9 ++++--- builder/dockerfile/instructions/parse.go | 27 +++++++++++++++---- builder/dockerfile/instructions/parse_test.go | 13 ++++++++- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 99d4aa627e..38e6f768c4 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -148,7 +148,7 @@ func (d *dispatchRequest) getImageMount(imageRefOrID string) (*imageMount, error return d.builder.imageSources.Get(imageRefOrID, localOnly) } -// 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() diff --git a/builder/dockerfile/instructions/commands.go b/builder/dockerfile/instructions/commands.go index d4f55ceb43..44382dc61e 100644 --- a/builder/dockerfile/instructions/commands.go +++ b/builder/dockerfile/instructions/commands.go @@ -357,10 +357,11 @@ type ShellCommand struct { // Stage represents a single stage in a multi-stage build type Stage struct { - Name string - Commands []Command - BaseName string - SourceCode string + Name string + Commands []Command + BaseName string + SourceCode string + OperatingSystem string } // AddCommand to the stage diff --git a/builder/dockerfile/instructions/parse.go b/builder/dockerfile/instructions/parse.go index 9226f4d46e..9b94ee3660 100644 --- a/builder/dockerfile/instructions/parse.go +++ b/builder/dockerfile/instructions/parse.go @@ -3,6 +3,7 @@ package instructions // import "github.com/docker/docker/builder/dockerfile/inst import ( "fmt" "regexp" + "runtime" "sort" "strconv" "strings" @@ -12,6 +13,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 +273,31 @@ func parseFrom(req parseRequest) (*Stage, error) { return nil, err } + flPlatform := req.flags.AddString("platform", "") if err := req.flags.Parse(); err != nil { return nil, err } + specPlatform := system.ParsePlatform(flPlatform.Value) + if specPlatform.OS == "" { + specPlatform.OS = runtime.GOOS + } + if err := system.ValidatePlatform(specPlatform); err != nil { + return nil, fmt.Errorf("invalid platform %q on FROM", flPlatform.Value) + } + if !system.IsOSSupported(specPlatform.OS) { + return nil, fmt.Errorf("unsupported platform %q on FROM", flPlatform.Value) + } + if err != nil { + return nil, err + } code := strings.TrimSpace(req.original) - + fmt.Println("JJH", specPlatform.OS) return &Stage{ - BaseName: req.args[0], - Name: stageName, - SourceCode: code, - Commands: []Command{}, + BaseName: req.args[0], + Name: stageName, + SourceCode: code, + Commands: []Command{}, + OperatingSystem: specPlatform.OS, }, nil } diff --git a/builder/dockerfile/instructions/parse_test.go b/builder/dockerfile/instructions/parse_test.go index ffd6d4f45c..0c27f203d0 100644 --- a/builder/dockerfile/instructions/parse_test.go +++ b/builder/dockerfile/instructions/parse_test.go @@ -1,6 +1,8 @@ package instructions // import "github.com/docker/docker/builder/dockerfile/instructions" import ( + "fmt" + "runtime" "strings" "testing" @@ -184,6 +186,16 @@ func TestErrorCases(t *testing.T) { dockerfile: `foo bar`, expectedError: "unknown instruction: FOO", }, + { + name: "Invalid platform", + dockerfile: `FROM --platform=invalid busybox`, + expectedError: `invalid platform "invalid"`, + }, + { + name: "Only OS", + dockerfile: fmt.Sprintf(`FROM --platform=%s/%s busybox`, runtime.GOOS, runtime.GOARCH), + expectedError: `invalid platform`, + }, } for _, c := range cases { r := strings.NewReader(c.dockerfile) @@ -196,5 +208,4 @@ func TestErrorCases(t *testing.T) { _, err = ParseInstruction(n) testutil.ErrorContains(t, err, c.expectedError) } - } From 69fa84bc3d57dafd19800642c5ba196bc6d45f90 Mon Sep 17 00:00:00 2001 From: John Howard Date: Wed, 4 Oct 2017 14:26:56 -0700 Subject: [PATCH 5/7] Builder: Plumbing through platform in `FROM` statement Signed-off-by: John Howard --- builder/dockerfile/builder.go | 3 +- builder/dockerfile/dispatchers.go | 55 ++++++++++++++++-------- builder/dockerfile/dispatchers_test.go | 3 ++ builder/dockerfile/evaluator.go | 3 +- builder/dockerfile/imagecontext.go | 12 +++--- builder/dockerfile/instructions/parse.go | 7 +-- builder/dockerfile/internals.go | 10 ++--- 7 files changed, 53 insertions(+), 40 deletions(-) diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 2a41c1f0c3..d328235a14 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "io/ioutil" - "runtime" "strings" "time" @@ -104,7 +103,7 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) ( source = src } - os := runtime.GOOS + os := "" apiPlatform := system.ParsePlatform(config.Options.Platform) if apiPlatform.OS != "" { os = apiPlatform.OS diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 38e6f768c4..33e6bd4c23 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -145,14 +145,14 @@ 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.baseImage.OperatingSystem()) } // 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) + image, err := d.getFromImage(d.shlex, cmd.BaseName, cmd.OperatingSystem) if err != nil { return err } @@ -210,20 +210,44 @@ 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=...`). +func (d *dispatchRequest) getOsFromFlagsAndStage(stagePlatform string) string { + osForPull := "" + // First, take the API platform if nothing provided on FROM + if stagePlatform == "" && d.builder.options.Platform != "" { + osForPull = d.builder.options.Platform + } + // Next, use the FROM flag if that was provided + if osForPull == "" && stagePlatform != "" { + osForPull = stagePlatform + } + // Finally, assume the host OS + if osForPull == "" { + osForPull = runtime.GOOS + } + return osForPull +} + +func (d *dispatchRequest) getImageOrStage(name string, stagePlatform string) (builder.Image, error) { var localOnly bool if im, ok := d.stages.getByName(name); ok { name = im.Image localOnly = true } + os := d.getOsFromFlagsAndStage(stagePlatform) + // 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 optionsOS { + switch os { 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, stagePlatform string) (builder.Image, error) { name, err := d.getExpandedImageName(shlex, name) if err != nil { return nil, err } - return d.getImageOrStage(name) + return d.getImageOrStage(name, stagePlatform) } 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(baseImageOS, runConfig.WorkingDir, c.Path) + runConfig.WorkingDir, err = normalizeWorkdir(d.state.baseImage.OperatingSystem(), 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.baseImage.OperatingSystem())) containerID, err := d.builder.probeAndCreate(d.state, runConfigWithCommentCmd) if err != nil || containerID == "" { return err @@ -316,7 +339,7 @@ func dispatchRun(d dispatchRequest, c *instructions.RunCommand) error { return system.ErrNotSupportedOperatingSystem } stateRunConfig := d.state.runConfig - cmdFromArgs := resolveCmdLine(c.ShellDependantCmdLine, stateRunConfig, d.state.operatingSystem) + cmdFromArgs := resolveCmdLine(c.ShellDependantCmdLine, stateRunConfig, d.state.baseImage.OperatingSystem()) buildArgs := d.state.buildArgs.FilterAllowed(stateRunConfig.Env) saveCmd := cmdFromArgs @@ -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, optionsOS) + cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, d.state.baseImage.OperatingSystem()) 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, optionsOS) + cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, d.state.baseImage.OperatingSystem()) runConfig.Entrypoint = cmd if !d.state.cmdSet { runConfig.Cmd = nil diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index 7ad0d8d3c2..d65caf8745 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/builder/dockerfile/dispatchers_test.go @@ -225,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{ @@ -282,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{ @@ -357,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{ diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index 74264faf2e..9f747001c7 100644 --- a/builder/dockerfile/evaluator.go +++ b/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(optionsOS) + err := c.CheckPlatform(d.state.baseImage.OperatingSystem()) if err != nil { return errdefs.InvalidParameter(err) } diff --git a/builder/dockerfile/imagecontext.go b/builder/dockerfile/imagecontext.go index 0d4af384af..fd2b942393 100644 --- a/builder/dockerfile/imagecontext.go +++ b/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 } diff --git a/builder/dockerfile/instructions/parse.go b/builder/dockerfile/instructions/parse.go index 9b94ee3660..6d310eac08 100644 --- a/builder/dockerfile/instructions/parse.go +++ b/builder/dockerfile/instructions/parse.go @@ -3,7 +3,6 @@ package instructions // import "github.com/docker/docker/builder/dockerfile/inst import ( "fmt" "regexp" - "runtime" "sort" "strconv" "strings" @@ -278,20 +277,16 @@ func parseFrom(req parseRequest) (*Stage, error) { return nil, err } specPlatform := system.ParsePlatform(flPlatform.Value) - if specPlatform.OS == "" { - specPlatform.OS = runtime.GOOS - } if err := system.ValidatePlatform(specPlatform); err != nil { return nil, fmt.Errorf("invalid platform %q on FROM", flPlatform.Value) } - if !system.IsOSSupported(specPlatform.OS) { + if specPlatform.OS != "" && !system.IsOSSupported(specPlatform.OS) { return nil, fmt.Errorf("unsupported platform %q on FROM", flPlatform.Value) } if err != nil { return nil, err } code := strings.TrimSpace(req.original) - fmt.Println("JJH", specPlatform.OS) return &Stage{ BaseName: req.args[0], Name: stageName, diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index c8b34f8f65..e141a78465 100644 --- a/builder/dockerfile/internals.go +++ b/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, optionsPlatform.OS)) + runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment, dispatchState.baseImage.OperatingSystem())) 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.baseImage.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.baseImage.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.baseImage.OperatingSystem()) if err != nil { return err } From 317513d6984c0ba5df41dc578b22eb32fec55b55 Mon Sep 17 00:00:00 2001 From: John Howard Date: Mon, 5 Feb 2018 14:41:45 -0800 Subject: [PATCH 6/7] Builder: Fix CI issues Signed-off-by: John Howard --- builder/dockerfile/dispatchers.go | 12 ++++++------ builder/dockerfile/evaluator.go | 2 +- builder/dockerfile/internals.go | 8 ++++---- daemon/images/image_history.go | 5 ++++- daemon/images/images.go | 4 +++- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 33e6bd4c23..be1183684c 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -145,7 +145,7 @@ func (d *dispatchRequest) getImageMount(imageRefOrID string) (*imageMount, error imageRefOrID = stage.Image localOnly = true } - return d.builder.imageSources.Get(imageRefOrID, localOnly, d.state.baseImage.OperatingSystem()) + return d.builder.imageSources.Get(imageRefOrID, localOnly, d.state.operatingSystem) } // FROM [--platform=platform] imagename[:tag | @digest] [AS build-stage-name] @@ -288,7 +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 - runConfig.WorkingDir, err = normalizeWorkdir(d.state.baseImage.OperatingSystem(), runConfig.WorkingDir, c.Path) + runConfig.WorkingDir, err = normalizeWorkdir(d.state.operatingSystem, runConfig.WorkingDir, c.Path) if err != nil { return err } @@ -304,7 +304,7 @@ func dispatchWorkdir(d dispatchRequest, c *instructions.WorkdirCommand) error { } comment := "WORKDIR " + runConfig.WorkingDir - runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment, d.state.baseImage.OperatingSystem())) + runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment, d.state.operatingSystem)) containerID, err := d.builder.probeAndCreate(d.state, runConfigWithCommentCmd) if err != nil || containerID == "" { return err @@ -339,7 +339,7 @@ func dispatchRun(d dispatchRequest, c *instructions.RunCommand) error { return system.ErrNotSupportedOperatingSystem } stateRunConfig := d.state.runConfig - cmdFromArgs := resolveCmdLine(c.ShellDependantCmdLine, stateRunConfig, d.state.baseImage.OperatingSystem()) + cmdFromArgs := resolveCmdLine(c.ShellDependantCmdLine, stateRunConfig, d.state.operatingSystem) buildArgs := d.state.buildArgs.FilterAllowed(stateRunConfig.Env) saveCmd := cmdFromArgs @@ -420,7 +420,7 @@ func prependEnvOnCmd(buildArgs *buildArgs, buildArgVars []string, cmd strslice.S // func dispatchCmd(d dispatchRequest, c *instructions.CmdCommand) error { runConfig := d.state.runConfig - cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, d.state.baseImage.OperatingSystem()) + cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, d.state.operatingSystem) runConfig.Cmd = cmd // set config as already being escaped, this prevents double escaping on windows runConfig.ArgsEscaped = true @@ -463,7 +463,7 @@ func dispatchHealthcheck(d dispatchRequest, c *instructions.HealthCheckCommand) // func dispatchEntrypoint(d dispatchRequest, c *instructions.EntrypointCommand) error { runConfig := d.state.runConfig - cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, d.state.baseImage.OperatingSystem()) + cmd := resolveCmdLine(c.ShellDependantCmdLine, runConfig, d.state.operatingSystem) runConfig.Entrypoint = cmd if !d.state.cmdSet { runConfig.Cmd = nil diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index 9f747001c7..0f76845086 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -37,7 +37,7 @@ import ( func dispatch(d dispatchRequest, cmd instructions.Command) (err error) { if c, ok := cmd.(instructions.PlatformSpecific); ok { - err := c.CheckPlatform(d.state.baseImage.OperatingSystem()) + err := c.CheckPlatform(d.state.operatingSystem) if err != nil { return errdefs.InvalidParameter(err) } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index e141a78465..53748f0619 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -83,7 +83,7 @@ func (b *Builder) commit(dispatchState *dispatchState, comment string) error { return errors.New("Please provide a source image with `from` prior to commit") } - runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment, dispatchState.baseImage.OperatingSystem())) + runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment, dispatchState.operatingSystem)) hit, err := b.probeCache(dispatchState, runConfigWithCommentCmd) if err != nil || hit { return err @@ -165,13 +165,13 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error // TODO: should this have been using origPaths instead of srcHash in the comment? runConfigWithCommentCmd := copyRunConfig( state.runConfig, - withCmdCommentString(commentStr, state.baseImage.OperatingSystem())) + withCmdCommentString(commentStr, state.operatingSystem)) hit, err := b.probeCache(state, runConfigWithCommentCmd) if err != nil || hit { return err } - imageMount, err := b.imageSources.Get(state.imageID, true, state.baseImage.OperatingSystem()) + 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) } @@ -182,7 +182,7 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error } defer rwLayer.Release() - destInfo, err := createDestInfo(state.runConfig.WorkingDir, inst, rwLayer, state.baseImage.OperatingSystem()) + destInfo, err := createDestInfo(state.runConfig.WorkingDir, inst, rwLayer, state.operatingSystem) if err != nil { return err } diff --git a/daemon/images/image_history.go b/daemon/images/image_history.go index 2b92292631..b4ca25b1b6 100644 --- a/daemon/images/image_history.go +++ b/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 { diff --git a/daemon/images/images.go b/daemon/images/images.go index 46056f15b5..49212341c5 100644 --- a/daemon/images/images.go +++ b/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") From 14429056d3745ca052fba448d879788d16bbb01b Mon Sep 17 00:00:00 2001 From: John Howard Date: Fri, 23 Feb 2018 13:03:49 -0800 Subject: [PATCH 7/7] Builder: Review feedback Signed-off-by: John Howard --- builder/dockerfile/dispatchers.go | 40 +++++++++---------- builder/dockerfile/instructions/commands.go | 11 ++--- builder/dockerfile/instructions/parse.go | 20 +++------- builder/dockerfile/instructions/parse_test.go | 12 ------ 4 files changed, 31 insertions(+), 52 deletions(-) diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index be1183684c..991c433b2b 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -152,7 +152,10 @@ func (d *dispatchRequest) getImageMount(imageRefOrID string) (*imageMount, error // func initializeStage(d dispatchRequest, cmd *instructions.Stage) error { d.builder.imageProber.Reset() - image, err := d.getFromImage(d.shlex, cmd.BaseName, cmd.OperatingSystem) + 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 } @@ -215,32 +218,29 @@ func (d *dispatchRequest) getExpandedImageName(shlex *shell.Lex, name string) (s // 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=...`). -func (d *dispatchRequest) getOsFromFlagsAndStage(stagePlatform string) string { - osForPull := "" - // First, take the API platform if nothing provided on FROM - if stagePlatform == "" && d.builder.options.Platform != "" { - osForPull = d.builder.options.Platform +// 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 } - // Next, use the FROM flag if that was provided - if osForPull == "" && stagePlatform != "" { - osForPull = stagePlatform - } - // Finally, assume the host OS - if osForPull == "" { - osForPull = runtime.GOOS - } - return osForPull } -func (d *dispatchRequest) getImageOrStage(name string, stagePlatform string) (builder.Image, error) { +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(stagePlatform) + os := d.getOsFromFlagsAndStage(stageOS) // Windows cannot support a container with no base image unless it is LCOW. if name == api.NoBaseImageSpecifier { @@ -267,12 +267,12 @@ func (d *dispatchRequest) getImageOrStage(name string, stagePlatform string) (bu } return imageMount.Image(), nil } -func (d *dispatchRequest) getFromImage(shlex *shell.Lex, name string, stagePlatform 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, stagePlatform) + return d.getImageOrStage(name, stageOS) } func dispatchOnbuild(d dispatchRequest, c *instructions.OnbuildCommand) error { diff --git a/builder/dockerfile/instructions/commands.go b/builder/dockerfile/instructions/commands.go index 44382dc61e..a10140cf04 100644 --- a/builder/dockerfile/instructions/commands.go +++ b/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) @@ -357,11 +358,11 @@ type ShellCommand struct { // Stage represents a single stage in a multi-stage build type Stage struct { - Name string - Commands []Command - BaseName string - SourceCode string - OperatingSystem string + Name string + Commands []Command + BaseName string + SourceCode string + Platform specs.Platform } // AddCommand to the stage diff --git a/builder/dockerfile/instructions/parse.go b/builder/dockerfile/instructions/parse.go index 6d310eac08..e2d69a4887 100644 --- a/builder/dockerfile/instructions/parse.go +++ b/builder/dockerfile/instructions/parse.go @@ -276,23 +276,13 @@ func parseFrom(req parseRequest) (*Stage, error) { if err := req.flags.Parse(); err != nil { return nil, err } - specPlatform := system.ParsePlatform(flPlatform.Value) - if err := system.ValidatePlatform(specPlatform); err != nil { - return nil, fmt.Errorf("invalid platform %q on FROM", flPlatform.Value) - } - if specPlatform.OS != "" && !system.IsOSSupported(specPlatform.OS) { - return nil, fmt.Errorf("unsupported platform %q on FROM", flPlatform.Value) - } - if err != nil { - return nil, err - } code := strings.TrimSpace(req.original) return &Stage{ - BaseName: req.args[0], - Name: stageName, - SourceCode: code, - Commands: []Command{}, - OperatingSystem: specPlatform.OS, + BaseName: req.args[0], + Name: stageName, + SourceCode: code, + Commands: []Command{}, + Platform: *system.ParsePlatform(flPlatform.Value), }, nil } diff --git a/builder/dockerfile/instructions/parse_test.go b/builder/dockerfile/instructions/parse_test.go index 0c27f203d0..b78114d306 100644 --- a/builder/dockerfile/instructions/parse_test.go +++ b/builder/dockerfile/instructions/parse_test.go @@ -1,8 +1,6 @@ package instructions // import "github.com/docker/docker/builder/dockerfile/instructions" import ( - "fmt" - "runtime" "strings" "testing" @@ -186,16 +184,6 @@ func TestErrorCases(t *testing.T) { dockerfile: `foo bar`, expectedError: "unknown instruction: FOO", }, - { - name: "Invalid platform", - dockerfile: `FROM --platform=invalid busybox`, - expectedError: `invalid platform "invalid"`, - }, - { - name: "Only OS", - dockerfile: fmt.Sprintf(`FROM --platform=%s/%s busybox`, runtime.GOOS, runtime.GOARCH), - expectedError: `invalid platform`, - }, } for _, c := range cases { r := strings.NewReader(c.dockerfile)