From 4d62f67117ca515671fcc882807bfc4862067ea6 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 27 Mar 2017 17:51:42 -0700 Subject: [PATCH 1/2] Extract two functions from the FROM dispatcher This helps clarify the difference cases for parsing ctxName, and for getting an image. Signed-off-by: Daniel Nephin --- builder/dockerfile/dispatchers.go | 97 +++++++++++++++++-------------- 1 file changed, 54 insertions(+), 43 deletions(-) diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 3df8070b93..2234bdb3d7 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -176,61 +176,26 @@ func dispatchCopy(b *Builder, args []string, attributes map[string]bool, origina return b.runContextCommand(args, false, false, "COPY", im) } -// FROM imagename -// -// This sets the image the dockerfile will build on top of. +// FROM imagename[:tag | @digest] [AS build-stage-name] // +// from sets the base image func from(b *Builder, args []string, attributes map[string]bool, original string) error { - 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 errors.New("FROM requires either one or three arguments") + ctxName, err := parseBuildStageName(args) + if err != nil { + return err } if err := b.flags.Parse(); err != nil { return err } - - substituionArgs := []string{} - for key, value := range b.buildArgs.GetAllMeta() { - substituionArgs = append(substituionArgs, key+"="+value) - } - - name, err := ProcessWord(args[0], substituionArgs, b.escapeToken) - if err != nil { - return err - } - - var image builder.Image - b.resetImageCache() 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" { - return errors.New("Windows does not support FROM scratch") - } - b.image = "" - b.noBaseImage = true - } else { - var err error - image, err = pullOrGetImage(b, name) - if err != nil { - return err - } - } + image, err := b.getFromImage(args[0]) + if err != nil { + return err } if image != nil { b.imageContexts.update(image.ImageID(), image.RunConfig()) @@ -241,6 +206,52 @@ func from(b *Builder, args []string, attributes map[string]bool, original string return b.processImageFrom(image) } +func parseBuildStageName(args []string) (string, error) { + stageName := "" + switch { + case len(args) == 3 && strings.EqualFold(args[1], "as"): + stageName = strings.ToLower(args[2]) + if ok, _ := regexp.MatchString("^[a-z][a-z0-9-_\\.]*$", stageName); !ok { + return "", errors.Errorf("invalid name for build stage: %q, name can't start with a number or contain symbols", stageName) + } + case len(args) != 1: + return "", errors.New("FROM requires either one or three arguments") + } + + return stageName, nil +} + +func (b *Builder) getFromImage(name string) (builder.Image, error) { + substitutionArgs := []string{} + for key, value := range b.buildArgs.GetAllMeta() { + substitutionArgs = append(substitutionArgs, key+"="+value) + } + + name, err := ProcessWord(name, substitutionArgs, b.escapeToken) + if err != nil { + return nil, err + } + + if im, ok := b.imageContexts.byName[name]; ok { + if len(im.ImageID()) > 0 { + return im, nil + } + // FROM scratch does not have an ImageID + return nil, nil + } + + // Windows cannot support a container with no base image. + if name == api.NoBaseImageSpecifier { + if runtime.GOOS == "windows" { + return nil, errors.New("Windows does not support FROM scratch") + } + b.image = "" + b.noBaseImage = true + return nil, nil + } + return pullOrGetImage(b, name) +} + // ONBUILD RUN echo yo // // ONBUILD triggers run when the image is used in a FROM statement. From aafd7fa96909f48e43fd1b785c3c3a1f8b656ede Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 27 Mar 2017 17:52:01 -0700 Subject: [PATCH 2/2] remove increment flag on imageContexts.new() Rename the methods to match my understanding of the behaviour. Signed-off-by: Daniel Nephin --- builder/dockerfile/dispatchers.go | 8 ++------ builder/dockerfile/imagecontext.go | 10 ++++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 2234bdb3d7..06b218f23b 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -189,7 +189,7 @@ func from(b *Builder, args []string, attributes map[string]bool, original string return err } b.resetImageCache() - if _, err := b.imageContexts.new(ctxName, true); err != nil { + if _, err := b.imageContexts.add(ctxName); err != nil { return err } @@ -846,11 +846,7 @@ func mountByRef(b *Builder, name string) (*imageMount, error) { if err != nil { return nil, err } - im, err := b.imageContexts.new("", false) - if err != nil { - return nil, err - } - im.id = image.ImageID() + im := b.imageContexts.newImageMount(image.ImageID()) return im, nil } diff --git a/builder/dockerfile/imagecontext.go b/builder/dockerfile/imagecontext.go index 1b92ced179..9e35d28821 100644 --- a/builder/dockerfile/imagecontext.go +++ b/builder/dockerfile/imagecontext.go @@ -22,7 +22,11 @@ type imageContexts struct { currentName string } -func (ic *imageContexts) new(name string, increment bool) (*imageMount, error) { +func (ic *imageContexts) newImageMount(id string) *imageMount { + return &imageMount{ic: ic, id: id} +} + +func (ic *imageContexts) add(name string) (*imageMount, error) { im := &imageMount{ic: ic} if len(name) > 0 { if ic.byName == nil { @@ -33,10 +37,8 @@ func (ic *imageContexts) new(name string, increment bool) (*imageMount, error) { } ic.byName[name] = im } - if increment { - ic.list = append(ic.list, im) - } ic.currentName = name + ic.list = append(ic.list, im) return im, nil }