diff --git a/builder/dockerfile/bflag.go b/builder/dockerfile/bflag.go index 34b3399ff6..d849661620 100644 --- a/builder/dockerfile/bflag.go +++ b/builder/dockerfile/bflag.go @@ -37,6 +37,13 @@ func NewBFlags() *BFlags { } } +// NewBFlagsWithArgs returns the new BFlags struct with Args set to args +func NewBFlagsWithArgs(args []string) *BFlags { + flags := NewBFlags() + flags.Args = args + return flags +} + // AddBool adds a bool flag to BFlags // Note, any error will be generated when Parse() is called (see Parse). func (bf *BFlags) AddBool(name string, def bool) *Flag { diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 744f503dd3..3215a25e15 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -52,20 +52,20 @@ type Builder struct { clientCtx context.Context runConfig *container.Config // runconfig for cmd, run, entrypoint etc. - flags *BFlags tmpContainers map[string]struct{} - image string // imageID imageContexts *imageContexts // helper for storing contexts from builds - noBaseImage bool // A flag to track the use of `scratch` as the base image - maintainer string - cmdSet bool disableCommit bool cacheBusted bool buildArgs *buildArgs - escapeToken rune + imageCache builder.ImageCache - imageCache builder.ImageCache - from builder.Image + // TODO: these move to DispatchState + escapeToken rune + maintainer string + cmdSet bool + noBaseImage bool // A flag to track the use of `scratch` as the base image + image string // imageID + from builder.Image } // BuildManager implements builder.Backend and is shared across all Builder objects. @@ -304,6 +304,7 @@ func (b *Builder) tagImages(repoAndTags []reference.Named) error { } // hasFromImage returns true if the builder has processed a `FROM ` line +// TODO: move to DispatchState func (b *Builder) hasFromImage() bool { return b.image != "" || b.noBaseImage } diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 06b218f23b..6ca11f2a8a 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -33,103 +33,104 @@ import ( // Sets the environment variable foo to bar, also makes interpolation // in the dockerfile available from the next statement on via ${foo}. // -func env(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) == 0 { +func env(req dispatchRequest) error { + if len(req.args) == 0 { return errAtLeastOneArgument("ENV") } - if len(args)%2 != 0 { + if len(req.args)%2 != 0 { // should never get here, but just in case return errTooManyArguments("ENV") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } commitMessage := bytes.NewBufferString("ENV") - for j := 0; j < len(args); j += 2 { - if len(args[j]) == 0 { + for j := 0; j < len(req.args); j += 2 { + if len(req.args[j]) == 0 { return errBlankCommandNames("ENV") } - name := args[j] - value := args[j+1] + name := req.args[j] + value := req.args[j+1] newVar := name + "=" + value commitMessage.WriteString(" " + newVar) gotOne := false - for i, envVar := range b.runConfig.Env { + for i, envVar := range req.runConfig.Env { envParts := strings.SplitN(envVar, "=", 2) compareFrom := envParts[0] if equalEnvKeys(compareFrom, name) { - b.runConfig.Env[i] = newVar + req.runConfig.Env[i] = newVar gotOne = true break } } if !gotOne { - b.runConfig.Env = append(b.runConfig.Env, newVar) + req.runConfig.Env = append(req.runConfig.Env, newVar) } } - return b.commit("", b.runConfig.Cmd, commitMessage.String()) + return req.builder.commit("", req.runConfig.Cmd, commitMessage.String()) } // MAINTAINER some text // // Sets the maintainer metadata. -func maintainer(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) != 1 { +func maintainer(req dispatchRequest) error { + if len(req.args) != 1 { return errExactlyOneArgument("MAINTAINER") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } - b.maintainer = args[0] - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("MAINTAINER %s", b.maintainer)) + maintainer := req.args[0] + req.builder.maintainer = maintainer + return req.builder.commit("", req.runConfig.Cmd, "MAINTAINER "+maintainer) } // LABEL some json data describing the image // // Sets the Label variable foo to bar, // -func label(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) == 0 { +func label(req dispatchRequest) error { + if len(req.args) == 0 { return errAtLeastOneArgument("LABEL") } - if len(args)%2 != 0 { + if len(req.args)%2 != 0 { // should never get here, but just in case return errTooManyArguments("LABEL") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } commitStr := "LABEL" - if b.runConfig.Labels == nil { - b.runConfig.Labels = map[string]string{} + if req.runConfig.Labels == nil { + req.runConfig.Labels = map[string]string{} } - for j := 0; j < len(args); j++ { - // name ==> args[j] - // value ==> args[j+1] + for j := 0; j < len(req.args); j++ { + // name ==> req.args[j] + // value ==> req.args[j+1] - if len(args[j]) == 0 { + if len(req.args[j]) == 0 { return errBlankCommandNames("LABEL") } - newVar := args[j] + "=" + args[j+1] + "" + newVar := req.args[j] + "=" + req.args[j+1] + "" commitStr += " " + newVar - b.runConfig.Labels[args[j]] = args[j+1] + req.runConfig.Labels[req.args[j]] = req.args[j+1] j++ } - return b.commit("", b.runConfig.Cmd, commitStr) + return req.builder.commit("", req.runConfig.Cmd, commitStr) } // ADD foo /path @@ -137,73 +138,73 @@ func label(b *Builder, args []string, attributes map[string]bool, original strin // Add the file 'foo' to '/path'. Tarball and Remote URL (git, http) handling // exist here. If you do not wish to have this automatic handling, use COPY. // -func add(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) < 2 { +func add(req dispatchRequest) error { + if len(req.args) < 2 { return errAtLeastTwoArguments("ADD") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } - return b.runContextCommand(args, true, true, "ADD", nil) + return req.builder.runContextCommand(req.args, true, true, "ADD", nil) } // COPY foo /path // // Same as 'ADD' but without the tar and remote url handling. // -func dispatchCopy(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) < 2 { +func dispatchCopy(req dispatchRequest) error { + if len(req.args) < 2 { return errAtLeastTwoArguments("COPY") } - flFrom := b.flags.AddString("from", "") + flFrom := req.flags.AddString("from", "") - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } var im *imageMount if flFrom.IsUsed() { var err error - im, err = b.imageContexts.get(flFrom.Value) + im, err = req.builder.imageContexts.get(flFrom.Value) if err != nil { return err } } - return b.runContextCommand(args, false, false, "COPY", im) + return req.builder.runContextCommand(req.args, false, false, "COPY", im) } // 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, err := parseBuildStageName(args) +func from(req dispatchRequest) error { + ctxName, err := parseBuildStageName(req.args) if err != nil { return err } - if err := b.flags.Parse(); err != nil { - return err - } - b.resetImageCache() - if _, err := b.imageContexts.add(ctxName); err != nil { + if err := req.flags.Parse(); err != nil { return err } - image, err := b.getFromImage(args[0]) + req.builder.resetImageCache() + if _, err := req.builder.imageContexts.add(ctxName); err != nil { + return err + } + + image, err := req.builder.getFromImage(req.args[0]) if err != nil { return err } if image != nil { - b.imageContexts.update(image.ImageID(), image.RunConfig()) + req.builder.imageContexts.update(image.ImageID(), image.RunConfig()) } - b.from = image + req.builder.from = image - b.buildArgs.ResetAllowed() - return b.processImageFrom(image) + req.builder.buildArgs.ResetAllowed() + return req.builder.processImageFrom(image) } func parseBuildStageName(args []string) (string, error) { @@ -261,16 +262,16 @@ func (b *Builder) getFromImage(name string) (builder.Image, error) { // special cases. search for 'OnBuild' in internals.go for additional special // cases. // -func onbuild(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) == 0 { +func onbuild(req dispatchRequest) error { + if len(req.args) == 0 { return errAtLeastOneArgument("ONBUILD") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } - triggerInstruction := strings.ToUpper(strings.TrimSpace(args[0])) + triggerInstruction := strings.ToUpper(strings.TrimSpace(req.args[0])) switch triggerInstruction { case "ONBUILD": return errors.New("Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed") @@ -278,29 +279,28 @@ func onbuild(b *Builder, args []string, attributes map[string]bool, original str return fmt.Errorf("%s isn't allowed as an ONBUILD trigger", triggerInstruction) } - original = regexp.MustCompile(`(?i)^\s*ONBUILD\s*`).ReplaceAllString(original, "") - - b.runConfig.OnBuild = append(b.runConfig.OnBuild, original) - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("ONBUILD %s", original)) + original := regexp.MustCompile(`(?i)^\s*ONBUILD\s*`).ReplaceAllString(req.original, "") + req.runConfig.OnBuild = append(req.runConfig.OnBuild, original) + return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("ONBUILD %s", original)) } // WORKDIR /tmp // // Set the working directory for future RUN/CMD/etc statements. // -func workdir(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) != 1 { +func workdir(req dispatchRequest) error { + if len(req.args) != 1 { return errExactlyOneArgument("WORKDIR") } - err := b.flags.Parse() + err := req.flags.Parse() if err != nil { return err } // This is from the Dockerfile and will not necessarily be in platform // specific semantics, hence ensure it is converted. - b.runConfig.WorkingDir, err = normaliseWorkdir(b.runConfig.WorkingDir, args[0]) + req.runConfig.WorkingDir, err = normaliseWorkdir(req.runConfig.WorkingDir, req.args[0]) if err != nil { return err } @@ -309,39 +309,40 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str // This avoids having an unnecessary expensive mount/unmount calls // (on Windows in particular) during each container create. // Prior to 1.13, the mkdir was deferred and not executed at this step. - if b.disableCommit { + if req.builder.disableCommit { // Don't call back into the daemon if we're going through docker commit --change "WORKDIR /foo". // We've already updated the runConfig and that's enough. return nil } - b.runConfig.Image = b.image + req.runConfig.Image = req.builder.image - cmd := b.runConfig.Cmd - comment := "WORKDIR " + b.runConfig.WorkingDir + cmd := req.runConfig.Cmd + comment := "WORKDIR " + req.runConfig.WorkingDir // reset the command for cache detection - b.runConfig.Cmd = strslice.StrSlice(append(getShell(b.runConfig), "#(nop) "+comment)) - defer func(cmd strslice.StrSlice) { b.runConfig.Cmd = cmd }(cmd) + req.runConfig.Cmd = strslice.StrSlice(append(getShell(req.runConfig), "#(nop) "+comment)) + defer func(cmd strslice.StrSlice) { req.runConfig.Cmd = cmd }(cmd) - if hit, err := b.probeCache(); err != nil { + if hit, err := req.builder.probeCache(); err != nil { return err } else if hit { return nil } - container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{ - Config: b.runConfig, + req.runConfig.Image = req.builder.image + container, err := req.builder.docker.ContainerCreate(types.ContainerCreateConfig{ + Config: req.runConfig, // Set a log config to override any default value set on the daemon HostConfig: &container.HostConfig{LogConfig: defaultLogConfig}, }) if err != nil { return err } - b.tmpContainers[container.ID] = struct{}{} - if err := b.docker.ContainerCreateWorkdir(container.ID); err != nil { + req.builder.tmpContainers[container.ID] = struct{}{} + if err := req.builder.docker.ContainerCreateWorkdir(container.ID); err != nil { return err } - return b.commit(container.ID, cmd, comment) + return req.builder.commit(container.ID, cmd, comment) } // RUN some command yo @@ -354,38 +355,38 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str // RUN echo hi # cmd /S /C echo hi (Windows) // RUN [ "echo", "hi" ] # echo hi // -func run(b *Builder, args []string, attributes map[string]bool, original string) error { - if !b.hasFromImage() { +func run(req dispatchRequest) error { + if !req.builder.hasFromImage() { return errors.New("Please provide a source image with `from` prior to run") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } - args = handleJSONArgs(args, attributes) + args := handleJSONArgs(req.args, req.attributes) - if !attributes["json"] { - args = append(getShell(b.runConfig), args...) + if !req.attributes["json"] { + args = append(getShell(req.runConfig), args...) } config := &container.Config{ Cmd: strslice.StrSlice(args), - Image: b.image, + Image: req.builder.image, } // stash the cmd - cmd := b.runConfig.Cmd - if len(b.runConfig.Entrypoint) == 0 && len(b.runConfig.Cmd) == 0 { - b.runConfig.Cmd = config.Cmd + cmd := req.runConfig.Cmd + if len(req.runConfig.Entrypoint) == 0 && len(req.runConfig.Cmd) == 0 { + req.runConfig.Cmd = config.Cmd } // stash the config environment - env := b.runConfig.Env + env := req.runConfig.Env - defer func(cmd strslice.StrSlice) { b.runConfig.Cmd = cmd }(cmd) - defer func(env []string) { b.runConfig.Env = env }(env) + defer func(cmd strslice.StrSlice) { req.runConfig.Cmd = cmd }(cmd) + defer func(env []string) { req.runConfig.Env = env }(env) - cmdBuildEnv := b.buildArgsWithoutConfigEnv() + cmdBuildEnv := req.builder.buildArgsWithoutConfigEnv() // derive the command to use for probeCache() and to commit in this container. // Note that we only do this if there are any build-time env vars. Also, we @@ -401,8 +402,8 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...)) } - b.runConfig.Cmd = saveCmd - hit, err := b.probeCache() + req.runConfig.Cmd = saveCmd + hit, err := req.builder.probeCache() if err != nil { return err } @@ -411,20 +412,20 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) } // set Cmd manually, this is special case only for Dockerfiles - b.runConfig.Cmd = config.Cmd + req.runConfig.Cmd = config.Cmd // set build-time environment for 'run'. - b.runConfig.Env = append(b.runConfig.Env, cmdBuildEnv...) + req.runConfig.Env = append(req.runConfig.Env, cmdBuildEnv...) // set config as already being escaped, this prevents double escaping on windows - b.runConfig.ArgsEscaped = true + req.runConfig.ArgsEscaped = true - logrus.Debugf("[BUILDER] Command to be executed: %v", b.runConfig.Cmd) + logrus.Debugf("[BUILDER] Command to be executed: %v", req.runConfig.Cmd) - cID, err := b.create() + cID, err := req.builder.create() if err != nil { return err } - if err := b.run(cID); err != nil { + if err := req.builder.run(cID); err != nil { return err } @@ -432,7 +433,7 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) // revert to original config environment and set the command string to // have the build-time env vars in it (if any) so that future cache look-ups // properly match it. - b.runConfig.Env = env + req.runConfig.Env = env // remove builtinAllowedBuildArgs (see: builder.go) from the saveCmd // these args are transparent so resulting image should be the same regardless of the value @@ -442,7 +443,7 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) copy(tmpBuildEnv, cmdBuildEnv) for i, env := range tmpBuildEnv { key := strings.SplitN(env, "=", 2)[0] - if b.buildArgs.IsUnreferencedBuiltin(key) { + if req.builder.buildArgs.IsUnreferencedBuiltin(key) { tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...) } } @@ -450,8 +451,8 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) tmpEnv := append([]string{fmt.Sprintf("|%d", len(tmpBuildEnv))}, tmpBuildEnv...) saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...)) } - b.runConfig.Cmd = saveCmd - return b.commit(cID, cmd, "run") + req.runConfig.Cmd = saveCmd + return req.builder.commit(cID, cmd, "run") } // CMD foo @@ -459,27 +460,27 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) // Set the default command to run in the container (which may be empty). // Argument handling is the same as RUN. // -func cmd(b *Builder, args []string, attributes map[string]bool, original string) error { - if err := b.flags.Parse(); err != nil { +func cmd(req dispatchRequest) error { + if err := req.flags.Parse(); err != nil { return err } - cmdSlice := handleJSONArgs(args, attributes) + cmdSlice := handleJSONArgs(req.args, req.attributes) - if !attributes["json"] { - cmdSlice = append(getShell(b.runConfig), cmdSlice...) + if !req.attributes["json"] { + cmdSlice = append(getShell(req.runConfig), cmdSlice...) } - b.runConfig.Cmd = strslice.StrSlice(cmdSlice) + req.runConfig.Cmd = strslice.StrSlice(cmdSlice) // set config as already being escaped, this prevents double escaping on windows - b.runConfig.ArgsEscaped = true + req.runConfig.ArgsEscaped = true - if err := b.commit("", b.runConfig.Cmd, fmt.Sprintf("CMD %q", cmdSlice)); err != nil { + if err := req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("CMD %q", cmdSlice)); err != nil { return err } - if len(args) != 0 { - b.cmdSet = true + if len(req.args) != 0 { + req.builder.cmdSet = true } return nil @@ -507,47 +508,47 @@ func parseOptInterval(f *Flag) (time.Duration, error) { // Set the default healthcheck command to run in the container (which may be empty). // Argument handling is the same as RUN. // -func healthcheck(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) == 0 { +func healthcheck(req dispatchRequest) error { + if len(req.args) == 0 { return errAtLeastOneArgument("HEALTHCHECK") } - typ := strings.ToUpper(args[0]) - args = args[1:] + typ := strings.ToUpper(req.args[0]) + args := req.args[1:] if typ == "NONE" { if len(args) != 0 { return errors.New("HEALTHCHECK NONE takes no arguments") } test := strslice.StrSlice{typ} - b.runConfig.Healthcheck = &container.HealthConfig{ + req.runConfig.Healthcheck = &container.HealthConfig{ Test: test, } } else { - if b.runConfig.Healthcheck != nil { - oldCmd := b.runConfig.Healthcheck.Test + if req.runConfig.Healthcheck != nil { + oldCmd := req.runConfig.Healthcheck.Test if len(oldCmd) > 0 && oldCmd[0] != "NONE" { - fmt.Fprintf(b.Stdout, "Note: overriding previous HEALTHCHECK: %v\n", oldCmd) + fmt.Fprintf(req.builder.Stdout, "Note: overriding previous HEALTHCHECK: %v\n", oldCmd) } } healthcheck := container.HealthConfig{} - flInterval := b.flags.AddString("interval", "") - flTimeout := b.flags.AddString("timeout", "") - flStartPeriod := b.flags.AddString("start-period", "") - flRetries := b.flags.AddString("retries", "") + flInterval := req.flags.AddString("interval", "") + flTimeout := req.flags.AddString("timeout", "") + flStartPeriod := req.flags.AddString("start-period", "") + flRetries := req.flags.AddString("retries", "") - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } switch typ { case "CMD": - cmdSlice := handleJSONArgs(args, attributes) + cmdSlice := handleJSONArgs(args, req.attributes) if len(cmdSlice) == 0 { return errors.New("Missing command after HEALTHCHECK CMD") } - if !attributes["json"] { + if !req.attributes["json"] { typ = "CMD-SHELL" } @@ -587,10 +588,10 @@ func healthcheck(b *Builder, args []string, attributes map[string]bool, original healthcheck.Retries = 0 } - b.runConfig.Healthcheck = &healthcheck + req.runConfig.Healthcheck = &healthcheck } - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("HEALTHCHECK %q", b.runConfig.Healthcheck)) + return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("HEALTHCHECK %q", req.runConfig.Healthcheck)) } // ENTRYPOINT /usr/sbin/nginx @@ -598,35 +599,35 @@ func healthcheck(b *Builder, args []string, attributes map[string]bool, original // Set the entrypoint to /usr/sbin/nginx. Will accept the CMD as the arguments // to /usr/sbin/nginx. Uses the default shell if not in JSON format. // -// Handles command processing similar to CMD and RUN, only b.runConfig.Entrypoint +// Handles command processing similar to CMD and RUN, only req.runConfig.Entrypoint // is initialized at NewBuilder time instead of through argument parsing. // -func entrypoint(b *Builder, args []string, attributes map[string]bool, original string) error { - if err := b.flags.Parse(); err != nil { +func entrypoint(req dispatchRequest) error { + if err := req.flags.Parse(); err != nil { return err } - parsed := handleJSONArgs(args, attributes) + parsed := handleJSONArgs(req.args, req.attributes) switch { - case attributes["json"]: + case req.attributes["json"]: // ENTRYPOINT ["echo", "hi"] - b.runConfig.Entrypoint = strslice.StrSlice(parsed) + req.runConfig.Entrypoint = strslice.StrSlice(parsed) case len(parsed) == 0: // ENTRYPOINT [] - b.runConfig.Entrypoint = nil + req.runConfig.Entrypoint = nil default: // ENTRYPOINT echo hi - b.runConfig.Entrypoint = strslice.StrSlice(append(getShell(b.runConfig), parsed[0])) + req.runConfig.Entrypoint = strslice.StrSlice(append(getShell(req.runConfig), parsed[0])) } // when setting the entrypoint if a CMD was not explicitly set then // set the command to nil - if !b.cmdSet { - b.runConfig.Cmd = nil + if !req.builder.cmdSet { + req.runConfig.Cmd = nil } - if err := b.commit("", b.runConfig.Cmd, fmt.Sprintf("ENTRYPOINT %q", b.runConfig.Entrypoint)); err != nil { + if err := req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("ENTRYPOINT %q", req.runConfig.Entrypoint)); err != nil { return err } @@ -636,21 +637,21 @@ func entrypoint(b *Builder, args []string, attributes map[string]bool, original // EXPOSE 6667/tcp 7000/tcp // // Expose ports for links and port mappings. This all ends up in -// b.runConfig.ExposedPorts for runconfig. +// req.runConfig.ExposedPorts for runconfig. // -func expose(b *Builder, args []string, attributes map[string]bool, original string) error { - portsTab := args +func expose(req dispatchRequest) error { + portsTab := req.args - if len(args) == 0 { + if len(req.args) == 0 { return errAtLeastOneArgument("EXPOSE") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } - if b.runConfig.ExposedPorts == nil { - b.runConfig.ExposedPorts = make(nat.PortSet) + if req.runConfig.ExposedPorts == nil { + req.runConfig.ExposedPorts = make(nat.PortSet) } ports, _, err := nat.ParsePortSpecs(portsTab) @@ -664,14 +665,14 @@ func expose(b *Builder, args []string, attributes map[string]bool, original stri portList := make([]string, len(ports)) var i int for port := range ports { - if _, exists := b.runConfig.ExposedPorts[port]; !exists { - b.runConfig.ExposedPorts[port] = struct{}{} + if _, exists := req.runConfig.ExposedPorts[port]; !exists { + req.runConfig.ExposedPorts[port] = struct{}{} } portList[i] = string(port) i++ } sort.Strings(portList) - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("EXPOSE %s", strings.Join(portList, " "))) + return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("EXPOSE %s", strings.Join(portList, " "))) } // USER foo @@ -679,43 +680,43 @@ func expose(b *Builder, args []string, attributes map[string]bool, original stri // Set the user to 'foo' for future commands and when running the // ENTRYPOINT/CMD at container run time. // -func user(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) != 1 { +func user(req dispatchRequest) error { + if len(req.args) != 1 { return errExactlyOneArgument("USER") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } - b.runConfig.User = args[0] - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("USER %v", args)) + req.runConfig.User = req.args[0] + return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("USER %v", req.args)) } // VOLUME /foo // // Expose the volume /foo for use. Will also accept the JSON array form. // -func volume(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) == 0 { +func volume(req dispatchRequest) error { + if len(req.args) == 0 { return errAtLeastOneArgument("VOLUME") } - if err := b.flags.Parse(); err != nil { + if err := req.flags.Parse(); err != nil { return err } - if b.runConfig.Volumes == nil { - b.runConfig.Volumes = map[string]struct{}{} + if req.runConfig.Volumes == nil { + req.runConfig.Volumes = map[string]struct{}{} } - for _, v := range args { + for _, v := range req.args { v = strings.TrimSpace(v) if v == "" { return errors.New("VOLUME specified can not be an empty string") } - b.runConfig.Volumes[v] = struct{}{} + req.runConfig.Volumes[v] = struct{}{} } - if err := b.commit("", b.runConfig.Cmd, fmt.Sprintf("VOLUME %v", args)); err != nil { + if err := req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("VOLUME %v", req.args)); err != nil { return err } return nil @@ -724,19 +725,19 @@ func volume(b *Builder, args []string, attributes map[string]bool, original stri // STOPSIGNAL signal // // Set the signal that will be used to kill the container. -func stopSignal(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) != 1 { +func stopSignal(req dispatchRequest) error { + if len(req.args) != 1 { return errExactlyOneArgument("STOPSIGNAL") } - sig := args[0] + sig := req.args[0] _, err := signal.ParseSignal(sig) if err != nil { return err } - b.runConfig.StopSignal = sig - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("STOPSIGNAL %v", args)) + req.runConfig.StopSignal = sig + return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("STOPSIGNAL %v", req.args)) } // ARG name[=value] @@ -744,8 +745,8 @@ func stopSignal(b *Builder, args []string, attributes map[string]bool, original // Adds the variable foo to the trusted list of variables that can be passed // to builder using the --build-arg flag for expansion/substitution or passing to 'run'. // Dockerfile author may optionally set a default value of this variable. -func arg(b *Builder, args []string, attributes map[string]bool, original string) error { - if len(args) != 1 { +func arg(req dispatchRequest) error { + if len(req.args) != 1 { return errExactlyOneArgument("ARG") } @@ -755,7 +756,7 @@ func arg(b *Builder, args []string, attributes map[string]bool, original string) hasDefault bool ) - arg := args[0] + arg := req.args[0] // 'arg' can just be a name or name-value pair. Note that this is different // from 'env' that handles the split of name and value at the parser level. // The reason for doing it differently for 'arg' is that we support just @@ -779,36 +780,36 @@ func arg(b *Builder, args []string, attributes map[string]bool, original string) if hasDefault { value = &newValue } - b.buildArgs.AddArg(name, value) + req.builder.buildArgs.AddArg(name, value) // Arg before FROM doesn't add a layer - if !b.hasFromImage() { - b.buildArgs.AddMetaArg(name, value) + if !req.builder.hasFromImage() { + req.builder.buildArgs.AddMetaArg(name, value) return nil } - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("ARG %s", arg)) + return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("ARG %s", arg)) } // SHELL powershell -command // // Set the non-default shell to use. -func shell(b *Builder, args []string, attributes map[string]bool, original string) error { - if err := b.flags.Parse(); err != nil { +func shell(req dispatchRequest) error { + if err := req.flags.Parse(); err != nil { return err } - shellSlice := handleJSONArgs(args, attributes) + shellSlice := handleJSONArgs(req.args, req.attributes) switch { case len(shellSlice) == 0: // SHELL [] return errAtLeastOneArgument("SHELL") - case attributes["json"]: + case req.attributes["json"]: // SHELL ["powershell", "-command"] - b.runConfig.Shell = strslice.StrSlice(shellSlice) + req.runConfig.Shell = strslice.StrSlice(shellSlice) default: // SHELL powershell -command - not JSON - return errNotJSON("SHELL", original) + return errNotJSON("SHELL", req.original) } - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("SHELL %v", shellSlice)) + return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("SHELL %v", shellSlice)) } func errAtLeastOneArgument(command string) error { diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index a504540e3d..c1d4ed0dff 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/builder/dockerfile/dispatchers_test.go @@ -3,7 +3,6 @@ package dockerfile import ( "fmt" "runtime" - "strings" "testing" "github.com/docker/docker/api/types" @@ -12,6 +11,8 @@ import ( "github.com/docker/docker/builder" "github.com/docker/go-connections/nat" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/docker/docker/pkg/testutil" ) type commandWithFunction struct { @@ -19,161 +20,142 @@ type commandWithFunction struct { function func(args []string) error } +func withArgs(f dispatcher) func([]string) error { + return func(args []string) error { + return f(dispatchRequest{args: args, runConfig: &container.Config{}}) + } +} + +func withBuilderAndArgs(builder *Builder, f dispatcher) func([]string) error { + return func(args []string) error { + return f(defaultDispatchReq(builder, args...)) + } +} + +func defaultDispatchReq(builder *Builder, args ...string) dispatchRequest { + return dispatchRequest{ + builder: builder, + args: args, + flags: NewBFlags(), + runConfig: &container.Config{}, + } +} + +func newBuilderWithMockBackend() *Builder { + b := &Builder{ + runConfig: &container.Config{}, + options: &types.ImageBuildOptions{}, + docker: &MockBackend{}, + buildArgs: newBuildArgs(make(map[string]*string)), + disableCommit: true, + } + b.imageContexts = &imageContexts{b: b} + return b +} + func TestCommandsExactlyOneArgument(t *testing.T) { commands := []commandWithFunction{ - {"MAINTAINER", func(args []string) error { return maintainer(nil, args, nil, "") }}, - {"WORKDIR", func(args []string) error { return workdir(nil, args, nil, "") }}, - {"USER", func(args []string) error { return user(nil, args, nil, "") }}, - {"STOPSIGNAL", func(args []string) error { return stopSignal(nil, args, nil, "") }}} + {"MAINTAINER", withArgs(maintainer)}, + {"WORKDIR", withArgs(workdir)}, + {"USER", withArgs(user)}, + {"STOPSIGNAL", withArgs(stopSignal)}, + } for _, command := range commands { err := command.function([]string{}) - - if err == nil { - t.Fatalf("Error should be present for %s command", command.name) - } - - expectedError := errExactlyOneArgument(command.name) - - if err.Error() != expectedError.Error() { - t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) - } + assert.EqualError(t, err, errExactlyOneArgument(command.name).Error()) } } func TestCommandsAtLeastOneArgument(t *testing.T) { commands := []commandWithFunction{ - {"ENV", func(args []string) error { return env(nil, args, nil, "") }}, - {"LABEL", func(args []string) error { return label(nil, args, nil, "") }}, - {"ONBUILD", func(args []string) error { return onbuild(nil, args, nil, "") }}, - {"HEALTHCHECK", func(args []string) error { return healthcheck(nil, args, nil, "") }}, - {"EXPOSE", func(args []string) error { return expose(nil, args, nil, "") }}, - {"VOLUME", func(args []string) error { return volume(nil, args, nil, "") }}} + {"ENV", withArgs(env)}, + {"LABEL", withArgs(label)}, + {"ONBUILD", withArgs(onbuild)}, + {"HEALTHCHECK", withArgs(healthcheck)}, + {"EXPOSE", withArgs(expose)}, + {"VOLUME", withArgs(volume)}, + } for _, command := range commands { err := command.function([]string{}) - - if err == nil { - t.Fatalf("Error should be present for %s command", command.name) - } - - expectedError := errAtLeastOneArgument(command.name) - - if err.Error() != expectedError.Error() { - t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) - } + assert.EqualError(t, err, errAtLeastOneArgument(command.name).Error()) } } func TestCommandsAtLeastTwoArguments(t *testing.T) { commands := []commandWithFunction{ - {"ADD", func(args []string) error { return add(nil, args, nil, "") }}, - {"COPY", func(args []string) error { return dispatchCopy(nil, args, nil, "") }}} + {"ADD", withArgs(add)}, + {"COPY", withArgs(dispatchCopy)}} for _, command := range commands { err := command.function([]string{"arg1"}) - - if err == nil { - t.Fatalf("Error should be present for %s command", command.name) - } - - expectedError := errAtLeastTwoArguments(command.name) - - if err.Error() != expectedError.Error() { - t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) - } + assert.EqualError(t, err, errAtLeastTwoArguments(command.name).Error()) } } func TestCommandsTooManyArguments(t *testing.T) { commands := []commandWithFunction{ - {"ENV", func(args []string) error { return env(nil, args, nil, "") }}, - {"LABEL", func(args []string) error { return label(nil, args, nil, "") }}} + {"ENV", withArgs(env)}, + {"LABEL", withArgs(label)}} for _, command := range commands { err := command.function([]string{"arg1", "arg2", "arg3"}) - - if err == nil { - t.Fatalf("Error should be present for %s command", command.name) - } - - expectedError := errTooManyArguments(command.name) - - if err.Error() != expectedError.Error() { - t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) - } + assert.EqualError(t, err, errTooManyArguments(command.name).Error()) } } -func TestCommandseBlankNames(t *testing.T) { - bflags := &BFlags{} - config := &container.Config{} - - b := &Builder{flags: bflags, runConfig: config, disableCommit: true} - +func TestCommandsBlankNames(t *testing.T) { + builder := newBuilderWithMockBackend() commands := []commandWithFunction{ - {"ENV", func(args []string) error { return env(b, args, nil, "") }}, - {"LABEL", func(args []string) error { return label(b, args, nil, "") }}, + {"ENV", withBuilderAndArgs(builder, env)}, + {"LABEL", withBuilderAndArgs(builder, label)}, } for _, command := range commands { err := command.function([]string{"", ""}) - - if err == nil { - t.Fatalf("Error should be present for %s command", command.name) - } - - expectedError := errBlankCommandNames(command.name) - - if err.Error() != expectedError.Error() { - t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) - } + assert.EqualError(t, err, errBlankCommandNames(command.name).Error()) } } func TestEnv2Variables(t *testing.T) { b := newBuilderWithMockBackend() - b.disableCommit = true args := []string{"var1", "val1", "var2", "val2"} - err := env(b, args, nil, "") - assert.NoError(t, err) + req := defaultDispatchReq(b, args...) + err := env(req) + require.NoError(t, err) expected := []string{ fmt.Sprintf("%s=%s", args[0], args[1]), fmt.Sprintf("%s=%s", args[2], args[3]), } - assert.Equal(t, expected, b.runConfig.Env) + assert.Equal(t, expected, req.runConfig.Env) } func TestEnvValueWithExistingRunConfigEnv(t *testing.T) { b := newBuilderWithMockBackend() - b.disableCommit = true - b.runConfig.Env = []string{"var1=old", "var2=fromenv"} args := []string{"var1", "val1"} - err := env(b, args, nil, "") - assert.NoError(t, err) + req := defaultDispatchReq(b, args...) + req.runConfig.Env = []string{"var1=old", "var2=fromenv"} + err := env(req) + require.NoError(t, err) expected := []string{ fmt.Sprintf("%s=%s", args[0], args[1]), "var2=fromenv", } - assert.Equal(t, expected, b.runConfig.Env) + assert.Equal(t, expected, req.runConfig.Env) } func TestMaintainer(t *testing.T) { maintainerEntry := "Some Maintainer " - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} - - if err := maintainer(b, []string{maintainerEntry}, nil, ""); err != nil { - t.Fatalf("Error when executing maintainer: %s", err.Error()) - } - - if b.maintainer != maintainerEntry { - t.Fatalf("Maintainer in builder should be set to %s. Got: %s", maintainerEntry, b.maintainer) - } + b := newBuilderWithMockBackend() + err := maintainer(defaultDispatchReq(b, maintainerEntry)) + require.NoError(t, err) + assert.Equal(t, maintainerEntry, b.maintainer) } func TestLabel(t *testing.T) { @@ -181,45 +163,25 @@ func TestLabel(t *testing.T) { labelValue := "value" labelEntry := []string{labelName, labelValue} + b := newBuilderWithMockBackend() + req := defaultDispatchReq(b, labelEntry...) + err := label(req) + require.NoError(t, err) - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} - - if err := label(b, labelEntry, nil, ""); err != nil { - t.Fatalf("Error when executing label: %s", err.Error()) - } - - if val, ok := b.runConfig.Labels[labelName]; ok { - if val != labelValue { - t.Fatalf("Label %s should have value %s, had %s instead", labelName, labelValue, val) - } - } else { - t.Fatalf("Label %s should be present but it is not", labelName) - } -} - -func newBuilderWithMockBackend() *Builder { - b := &Builder{ - flags: &BFlags{}, - runConfig: &container.Config{}, - options: &types.ImageBuildOptions{}, - docker: &MockBackend{}, - buildArgs: newBuildArgs(make(map[string]*string)), - } - b.imageContexts = &imageContexts{b: b} - return b + require.Contains(t, req.runConfig.Labels, labelName) + assert.Equal(t, req.runConfig.Labels[labelName], labelValue) } func TestFromScratch(t *testing.T) { b := newBuilderWithMockBackend() - - err := from(b, []string{"scratch"}, nil, "") + err := from(defaultDispatchReq(b, "scratch")) if runtime.GOOS == "windows" { assert.EqualError(t, err, "Windows does not support FROM scratch") return } - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, "", b.image) assert.Equal(t, true, b.noBaseImage) } @@ -234,10 +196,10 @@ func TestFromWithArg(t *testing.T) { b := newBuilderWithMockBackend() b.docker.(*MockBackend).getImageOnBuildFunc = getImage - assert.NoError(t, arg(b, []string{"THETAG=" + tag}, nil, "")) - err := from(b, []string{"alpine${THETAG}"}, nil, "") + require.NoError(t, arg(defaultDispatchReq(b, "THETAG="+tag))) + err := from(defaultDispatchReq(b, "alpine${THETAG}")) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, expected, b.image) assert.Equal(t, expected, b.from.ImageID()) assert.Len(t, b.buildArgs.GetAllAllowed(), 0) @@ -255,8 +217,8 @@ func TestFromWithUndefinedArg(t *testing.T) { b.docker.(*MockBackend).getImageOnBuildFunc = getImage b.options.BuildArgs = map[string]*string{"THETAG": &tag} - err := from(b, []string{"alpine${THETAG}"}, nil, "") - assert.NoError(t, err) + err := from(defaultDispatchReq(b, "alpine${THETAG}")) + require.NoError(t, err) assert.Equal(t, expected, b.image) } @@ -267,237 +229,147 @@ func TestOnbuildIllegalTriggers(t *testing.T) { {"FROM", "FROM isn't allowed as an ONBUILD trigger"}} for _, trigger := range triggers { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} + b := newBuilderWithMockBackend() - err := onbuild(b, []string{trigger.command}, nil, "") - - if err == nil { - t.Fatal("Error should not be nil") - } - - if !strings.Contains(err.Error(), trigger.expectedError) { - t.Fatalf("Error message not correct. Should be: %s, got: %s", trigger.expectedError, err.Error()) - } + err := onbuild(defaultDispatchReq(b, trigger.command)) + testutil.ErrorContains(t, err, trigger.expectedError) } } func TestOnbuild(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} + b := newBuilderWithMockBackend() - err := onbuild(b, []string{"ADD", ".", "/app/src"}, nil, "ONBUILD ADD . /app/src") + req := defaultDispatchReq(b, "ADD", ".", "/app/src") + req.original = "ONBUILD ADD . /app/src" + req.runConfig = &container.Config{} - if err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } - - expectedOnbuild := "ADD . /app/src" - - if b.runConfig.OnBuild[0] != expectedOnbuild { - t.Fatalf("Wrong ONBUILD command. Expected: %s, got: %s", expectedOnbuild, b.runConfig.OnBuild[0]) - } + err := onbuild(req) + require.NoError(t, err) + assert.Equal(t, "ADD . /app/src", req.runConfig.OnBuild[0]) } func TestWorkdir(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} - + b := newBuilderWithMockBackend() workingDir := "/app" - if runtime.GOOS == "windows" { workingDir = "C:\app" } - err := workdir(b, []string{workingDir}, nil, "") - - if err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } - - if b.runConfig.WorkingDir != workingDir { - t.Fatalf("WorkingDir should be set to %s, got %s", workingDir, b.runConfig.WorkingDir) - } - + req := defaultDispatchReq(b, workingDir) + err := workdir(req) + require.NoError(t, err) + assert.Equal(t, workingDir, req.runConfig.WorkingDir) } func TestCmd(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} - + b := newBuilderWithMockBackend() command := "./executable" - err := cmd(b, []string{command}, nil, "") - - if err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } + req := defaultDispatchReq(b, command) + err := cmd(req) + require.NoError(t, err) var expectedCommand strslice.StrSlice - if runtime.GOOS == "windows" { expectedCommand = strslice.StrSlice(append([]string{"cmd"}, "/S", "/C", command)) } else { expectedCommand = strslice.StrSlice(append([]string{"/bin/sh"}, "-c", command)) } - if !compareStrSlice(b.runConfig.Cmd, expectedCommand) { - t.Fatalf("Command should be set to %s, got %s", command, b.runConfig.Cmd) - } - - if !b.cmdSet { - t.Fatal("Command should be marked as set") - } -} - -func compareStrSlice(slice1, slice2 strslice.StrSlice) bool { - if len(slice1) != len(slice2) { - return false - } - - for i := range slice1 { - if slice1[i] != slice2[i] { - return false - } - } - - return true + assert.Equal(t, expectedCommand, req.runConfig.Cmd) + assert.True(t, b.cmdSet) } func TestHealthcheckNone(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} + b := newBuilderWithMockBackend() - if err := healthcheck(b, []string{"NONE"}, nil, ""); err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } + req := defaultDispatchReq(b, "NONE") + err := healthcheck(req) + require.NoError(t, err) - if b.runConfig.Healthcheck == nil { - t.Fatal("Healthcheck should be set, got nil") - } - - expectedTest := strslice.StrSlice(append([]string{"NONE"})) - - if !compareStrSlice(expectedTest, b.runConfig.Healthcheck.Test) { - t.Fatalf("Command should be set to %s, got %s", expectedTest, b.runConfig.Healthcheck.Test) - } + require.NotNil(t, req.runConfig.Healthcheck) + assert.Equal(t, []string{"NONE"}, req.runConfig.Healthcheck.Test) } func TestHealthcheckCmd(t *testing.T) { - b := &Builder{flags: &BFlags{flags: make(map[string]*Flag)}, runConfig: &container.Config{}, disableCommit: true} + b := newBuilderWithMockBackend() - if err := healthcheck(b, []string{"CMD", "curl", "-f", "http://localhost/", "||", "exit", "1"}, nil, ""); err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } + args := []string{"CMD", "curl", "-f", "http://localhost/", "||", "exit", "1"} + req := defaultDispatchReq(b, args...) + err := healthcheck(req) + require.NoError(t, err) - if b.runConfig.Healthcheck == nil { - t.Fatal("Healthcheck should be set, got nil") - } - - expectedTest := strslice.StrSlice(append([]string{"CMD-SHELL"}, "curl -f http://localhost/ || exit 1")) - - if !compareStrSlice(expectedTest, b.runConfig.Healthcheck.Test) { - t.Fatalf("Command should be set to %s, got %s", expectedTest, b.runConfig.Healthcheck.Test) - } + require.NotNil(t, req.runConfig.Healthcheck) + expectedTest := []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"} + assert.Equal(t, expectedTest, req.runConfig.Healthcheck.Test) } func TestEntrypoint(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} - + b := newBuilderWithMockBackend() entrypointCmd := "/usr/sbin/nginx" - if err := entrypoint(b, []string{entrypointCmd}, nil, ""); err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } - - if b.runConfig.Entrypoint == nil { - t.Fatal("Entrypoint should be set") - } + req := defaultDispatchReq(b, entrypointCmd) + err := entrypoint(req) + require.NoError(t, err) + require.NotNil(t, req.runConfig.Entrypoint) var expectedEntrypoint strslice.StrSlice - if runtime.GOOS == "windows" { expectedEntrypoint = strslice.StrSlice(append([]string{"cmd"}, "/S", "/C", entrypointCmd)) } else { expectedEntrypoint = strslice.StrSlice(append([]string{"/bin/sh"}, "-c", entrypointCmd)) } - - if !compareStrSlice(expectedEntrypoint, b.runConfig.Entrypoint) { - t.Fatalf("Entrypoint command should be set to %s, got %s", expectedEntrypoint, b.runConfig.Entrypoint) - } + assert.Equal(t, expectedEntrypoint, req.runConfig.Entrypoint) } func TestExpose(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} + b := newBuilderWithMockBackend() exposedPort := "80" + req := defaultDispatchReq(b, exposedPort) + err := expose(req) + require.NoError(t, err) - if err := expose(b, []string{exposedPort}, nil, ""); err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } - - if b.runConfig.ExposedPorts == nil { - t.Fatal("ExposedPorts should be set") - } - - if len(b.runConfig.ExposedPorts) != 1 { - t.Fatalf("ExposedPorts should contain only 1 element. Got %s", b.runConfig.ExposedPorts) - } + require.NotNil(t, req.runConfig.ExposedPorts) + require.Len(t, req.runConfig.ExposedPorts, 1) portsMapping, err := nat.ParsePortSpec(exposedPort) - - if err != nil { - t.Fatalf("Error when parsing port spec: %s", err.Error()) - } - - if _, ok := b.runConfig.ExposedPorts[portsMapping[0].Port]; !ok { - t.Fatalf("Port %s should be present. Got %s", exposedPort, b.runConfig.ExposedPorts) - } + require.NoError(t, err) + assert.Contains(t, req.runConfig.ExposedPorts, portsMapping[0].Port) } func TestUser(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} - + b := newBuilderWithMockBackend() userCommand := "foo" - if err := user(b, []string{userCommand}, nil, ""); err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } - - if b.runConfig.User != userCommand { - t.Fatalf("User should be set to %s, got %s", userCommand, b.runConfig.User) - } + req := defaultDispatchReq(b, userCommand) + err := user(req) + require.NoError(t, err) + assert.Equal(t, userCommand, req.runConfig.User) } func TestVolume(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} + b := newBuilderWithMockBackend() exposedVolume := "/foo" - if err := volume(b, []string{exposedVolume}, nil, ""); err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } + req := defaultDispatchReq(b, exposedVolume) + err := volume(req) + require.NoError(t, err) - if b.runConfig.Volumes == nil { - t.Fatal("Volumes should be set") - } - - if len(b.runConfig.Volumes) != 1 { - t.Fatalf("Volumes should contain only 1 element. Got %s", b.runConfig.Volumes) - } - - if _, ok := b.runConfig.Volumes[exposedVolume]; !ok { - t.Fatalf("Volume %s should be present. Got %s", exposedVolume, b.runConfig.Volumes) - } + require.NotNil(t, req.runConfig.Volumes) + assert.Len(t, req.runConfig.Volumes, 1) + assert.Contains(t, req.runConfig.Volumes, exposedVolume) } func TestStopSignal(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} - + b := newBuilderWithMockBackend() signal := "SIGKILL" - if err := stopSignal(b, []string{signal}, nil, ""); err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } - - if b.runConfig.StopSignal != signal { - t.Fatalf("StopSignal should be set to %s, got %s", signal, b.runConfig.StopSignal) - } + req := defaultDispatchReq(b, signal) + err := stopSignal(req) + require.NoError(t, err) + assert.Equal(t, signal, req.runConfig.StopSignal) } func TestArg(t *testing.T) { @@ -507,33 +379,23 @@ func TestArg(t *testing.T) { argVal := "bar" argDef := fmt.Sprintf("%s=%s", argName, argVal) - err := arg(b, []string{argDef}, nil, "") - assert.NoError(t, err) + err := arg(defaultDispatchReq(b, argDef)) + require.NoError(t, err) expected := map[string]string{argName: argVal} - allowed := b.buildArgs.GetAllAllowed() - assert.Equal(t, expected, allowed) + assert.Equal(t, expected, b.buildArgs.GetAllAllowed()) } func TestShell(t *testing.T) { - b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true} + b := newBuilderWithMockBackend() shellCmd := "powershell" + req := defaultDispatchReq(b, shellCmd) + req.attributes = map[string]bool{"json": true} - attrs := make(map[string]bool) - attrs["json"] = true - - if err := shell(b, []string{shellCmd}, attrs, ""); err != nil { - t.Fatalf("Error should be empty, got: %s", err.Error()) - } - - if b.runConfig.Shell == nil { - t.Fatal("Shell should be set") - } + err := shell(req) + require.NoError(t, err) expectedShell := strslice.StrSlice([]string{shellCmd}) - - if !compareStrSlice(expectedShell, b.runConfig.Shell) { - t.Fatalf("Shell should be set to %s, got %s", expectedShell, b.runConfig.Shell) - } + assert.Equal(t, expectedShell, req.runConfig.Shell) } diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index 472e9ae09a..f6776be3fb 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -22,7 +22,9 @@ package dockerfile import ( "fmt" "strings" + "bytes" + "github.com/docker/docker/api/types/container" "github.com/docker/docker/builder/dockerfile/command" "github.com/docker/docker/builder/dockerfile/parser" "github.com/docker/docker/runconfig/opts" @@ -56,10 +58,32 @@ var allowWordExpansion = map[string]bool{ command.Expose: true, } -var evaluateTable map[string]func(*Builder, []string, map[string]bool, string) error +type dispatchRequest struct { + builder *Builder // TODO: replace this with a smaller interface + args []string + attributes map[string]bool + flags *BFlags + original string + runConfig *container.Config +} + +func newDispatchRequestFromNode(node *parser.Node, builder *Builder, args []string) dispatchRequest { + return dispatchRequest{ + builder: builder, + args: args, + attributes: node.Attributes, + original: node.Original, + flags: NewBFlagsWithArgs(node.Flags), + runConfig: builder.runConfig, + } +} + +type dispatcher func(dispatchRequest) error + +var evaluateTable map[string]dispatcher func init() { - evaluateTable = map[string]func(*Builder, []string, map[string]bool, string) error{ + evaluateTable = map[string]dispatcher{ command.Add: add, command.Arg: arg, command.Cmd: cmd, @@ -95,8 +119,8 @@ func init() { // such as `RUN` in ONBUILD RUN foo. There is special case logic in here to // deal with that, at least until it becomes more of a general concern with new // features. -func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error { - cmd := ast.Value +func (b *Builder) dispatch(stepN int, stepTotal int, node *parser.Node) error { + cmd := node.Value upperCasedCmd := strings.ToUpper(cmd) // To ensure the user is given a decent error message if the platform @@ -105,28 +129,25 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error { return err } - attrs := ast.Attributes - original := ast.Original - flags := ast.Flags strList := []string{} - msg := fmt.Sprintf("Step %d/%d : %s", stepN+1, stepTotal, upperCasedCmd) + msg := bytes.NewBufferString(fmt.Sprintf("Step %d/%d : %s", stepN+1, stepTotal, upperCasedCmd)) - if len(ast.Flags) > 0 { - msg += " " + strings.Join(ast.Flags, " ") + if len(node.Flags) > 0 { + msg.WriteString(strings.Join(node.Flags, " ")) } + ast := node if cmd == "onbuild" { if ast.Next == nil { return errors.New("ONBUILD requires at least one argument") } ast = ast.Next.Children[0] strList = append(strList, ast.Value) - msg += " " + ast.Value + msg.WriteString(" " + ast.Value) if len(ast.Flags) > 0 { - msg += " " + strings.Join(ast.Flags, " ") + msg.WriteString(" " + strings.Join(ast.Flags, " ")) } - } msgList := initMsgList(ast) @@ -143,15 +164,13 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error { msgList[i] = ast.Value } - msg += " " + strings.Join(msgList, " ") - fmt.Fprintln(b.Stdout, msg) + msg.WriteString(" " + strings.Join(msgList, " ")) + fmt.Fprintln(b.Stdout, msg.String()) // XXX yes, we skip any cmds that are not valid; the parser should have // picked these out already. if f, ok := evaluateTable[cmd]; ok { - b.flags = NewBFlags() - b.flags.Args = flags - return f(b, strList, attrs, original) + return f(newDispatchRequestFromNode(node, b, strList)) } return fmt.Errorf("Unknown instruction: %s", upperCasedCmd) diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 10633ede86..cd996750cd 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -505,7 +505,7 @@ func (s *DockerSuite) TestBuildAddSingleFileToWorkdir(c *check.C) { func (s *DockerSuite) TestBuildAddSingleFileToExistDir(c *check.C) { testRequires(c, DaemonIsLinux) // Linux specific test - buildImageSuccessfully(c, "testaddsinglefiletoexistdir", build.WithBuildContext(c, + cli.BuildCmd(c, "testaddsinglefiletoexistdir", build.WithBuildContext(c, build.WithFile("Dockerfile", `FROM busybox RUN echo 'dockerio:x:1001:1001::/bin:/bin/false' >> /etc/passwd RUN echo 'dockerio:x:1001:' >> /etc/group @@ -561,13 +561,13 @@ func (s *DockerSuite) TestBuildUsernamespaceValidateRemappedRoot(c *check.C) { } name := "testbuildusernamespacevalidateremappedroot" for _, tc := range testCases { - buildImageSuccessfully(c, name, build.WithBuildContext(c, + cli.BuildCmd(c, name, build.WithBuildContext(c, build.WithFile("Dockerfile", fmt.Sprintf(`FROM busybox %s RUN [ $(ls -l / | grep new_dir | awk '{print $3":"$4}') = 'root:root' ]`, tc)), build.WithFile("test_dir/test_file", "test file"))) - dockerCmd(c, "rmi", name) + cli.DockerCmd(c, "rmi", name) } } @@ -576,7 +576,7 @@ func (s *DockerSuite) TestBuildAddAndCopyFileWithWhitespace(c *check.C) { name := "testaddfilewithwhitespace" for _, command := range []string{"ADD", "COPY"} { - buildImageSuccessfully(c, name, build.WithBuildContext(c, + cli.BuildCmd(c, name, build.WithBuildContext(c, build.WithFile("Dockerfile", fmt.Sprintf(`FROM busybox RUN mkdir "/test dir" RUN mkdir "/test_dir" @@ -600,7 +600,7 @@ RUN [ $(cat "/test dir/test_file6") = 'test6' ]`, command, command, command, com build.WithFile("test dir/test_file6", "test6"), )) - dockerCmd(c, "rmi", name) + cli.DockerCmd(c, "rmi", name) } } @@ -623,7 +623,7 @@ RUN find "test5" "C:/test dir/test_file5" RUN find "test6" "C:/test dir/test_file6"` name := "testcopyfilewithwhitespace" - buildImageSuccessfully(c, name, build.WithBuildContext(c, + cli.BuildCmd(c, name, build.WithBuildContext(c, build.WithFile("Dockerfile", dockerfile), build.WithFile("test file1", "test1"), build.WithFile("test_file2", "test2"),