[dockerfile] Allow ARG in FROM

builder/dockerfile/buildargs.go

@@ -0,0 +1,124 @@
+package dockerfile
+
+// builtinAllowedBuildArgs is list of built-in allowed build args
+// these args are considered transparent and are excluded from the image history.
+// Filtering from history is implemented in dispatchers.go
+var builtinAllowedBuildArgs = map[string]bool{
+	"HTTP_PROXY":  true,
+	"http_proxy":  true,
+	"HTTPS_PROXY": true,
+	"https_proxy": true,
+	"FTP_PROXY":   true,
+	"ftp_proxy":   true,
+	"NO_PROXY":    true,
+	"no_proxy":    true,
+}
+
+// buildArgs manages arguments used by the builder
+type buildArgs struct {
+	// args that are allowed for expansion/substitution and passing to commands in 'run'.
+	allowedBuildArgs map[string]*string
+	// args defined before the first `FROM` in a Dockerfile
+	allowedMetaArgs map[string]*string
+	// args referenced by the Dockerfile
+	referencedArgs map[string]struct{}
+	// args provided by the user on the command line
+	argsFromOptions map[string]*string
+}
+
+func newBuildArgs(argsFromOptions map[string]*string) *buildArgs {
+	return &buildArgs{
+		allowedBuildArgs: make(map[string]*string),
+		allowedMetaArgs:  make(map[string]*string),
+		referencedArgs:   make(map[string]struct{}),
+		argsFromOptions:  argsFromOptions,
+	}
+}
+
+// UnreferencedOptionArgs returns the list of args that were set from options but
+// were never referenced from the Dockerfile
+func (b *buildArgs) UnreferencedOptionArgs() []string {
+	leftoverArgs := []string{}
+	for arg := range b.argsFromOptions {
+		if _, ok := b.referencedArgs[arg]; !ok {
+			leftoverArgs = append(leftoverArgs, arg)
+		}
+	}
+	return leftoverArgs
+}
+
+// ResetAllowed clears the list of args that are allowed to be used by a
+// directive
+func (b *buildArgs) ResetAllowed() {
+	b.allowedBuildArgs = make(map[string]*string)
+}
+
+// AddMetaArg adds a new meta arg that can be used by FROM directives
+func (b *buildArgs) AddMetaArg(key string, value *string) {
+	b.allowedMetaArgs[key] = value
+}
+
+// AddArg adds a new arg that can be used by directives
+func (b *buildArgs) AddArg(key string, value *string) {
+	b.allowedBuildArgs[key] = value
+	b.referencedArgs[key] = struct{}{}
+}
+
+// IsUnreferencedBuiltin checks if the key is a built-in arg, or if it has been
+// referenced by the Dockerfile. Returns true if the arg is a builtin that has
+// not been referenced in the Dockerfile.
+func (b *buildArgs) IsUnreferencedBuiltin(key string) bool {
+	_, isBuiltin := builtinAllowedBuildArgs[key]
+	_, isAllowed := b.allowedBuildArgs[key]
+	return isBuiltin && !isAllowed
+}
+
+// GetAllAllowed returns a mapping with all the allowed args
+func (b *buildArgs) GetAllAllowed() map[string]string {
+	return b.getAllFromMapping(b.allowedBuildArgs)
+}
+
+// GetAllMeta returns a mapping with all the meta meta args
+func (b *buildArgs) GetAllMeta() map[string]string {
+	return b.getAllFromMapping(b.allowedMetaArgs)
+}
+
+func (b *buildArgs) getAllFromMapping(source map[string]*string) map[string]string {
+	m := make(map[string]string)
+
+	keys := keysFromMaps(source, builtinAllowedBuildArgs)
+	for _, key := range keys {
+		v, ok := b.getBuildArg(key, source)
+		if ok {
+			m[key] = v
+		}
+	}
+	return m
+}
+
+func (b *buildArgs) getBuildArg(key string, mapping map[string]*string) (string, bool) {
+	defaultValue, exists := mapping[key]
+	// Return override from options if one is defined
+	if v, ok := b.argsFromOptions[key]; ok && v != nil {
+		return *v, ok
+	}
+
+	if defaultValue == nil {
+		if v, ok := b.allowedMetaArgs[key]; ok && v != nil {
+			return *v, ok
+		}
+		return "", false
+	}
+	return *defaultValue, exists
+}
+
+func keysFromMaps(source map[string]*string, builtin map[string]bool) []string {
+	keys := []string{}
+	for key := range source {
+		keys = append(keys, key)
+	}
+	for key := range builtin {
+		keys = append(keys, key)
+	}
+	return keys
+}

builder/dockerfile/buildargs_test.go

@@ -0,0 +1,63 @@
+package dockerfile
+
+import (
+	"github.com/docker/docker/pkg/testutil/assert"
+	"testing"
+)
+
+func strPtr(source string) *string {
+	return &source
+}
+
+func TestGetAllAllowed(t *testing.T) {
+	buildArgs := newBuildArgs(map[string]*string{
+		"ArgNotUsedInDockerfile":              strPtr("fromopt1"),
+		"ArgOverriddenByOptions":              strPtr("fromopt2"),
+		"ArgNoDefaultInDockerfileFromOptions": strPtr("fromopt3"),
+		"HTTP_PROXY":                          strPtr("theproxy"),
+	})
+
+	buildArgs.AddMetaArg("ArgFromMeta", strPtr("frommeta1"))
+	buildArgs.AddMetaArg("ArgFromMetaOverriden", strPtr("frommeta2"))
+	buildArgs.AddMetaArg("ArgFromMetaNotUsed", strPtr("frommeta3"))
+
+	buildArgs.AddArg("ArgOverriddenByOptions", strPtr("fromdockerfile2"))
+	buildArgs.AddArg("ArgWithDefaultInDockerfile", strPtr("fromdockerfile1"))
+	buildArgs.AddArg("ArgNoDefaultInDockerfile", nil)
+	buildArgs.AddArg("ArgNoDefaultInDockerfileFromOptions", nil)
+	buildArgs.AddArg("ArgFromMeta", nil)
+	buildArgs.AddArg("ArgFromMetaOverriden", strPtr("fromdockerfile3"))
+
+	all := buildArgs.GetAllAllowed()
+	expected := map[string]string{
+		"HTTP_PROXY":                          "theproxy",
+		"ArgOverriddenByOptions":              "fromopt2",
+		"ArgWithDefaultInDockerfile":          "fromdockerfile1",
+		"ArgNoDefaultInDockerfileFromOptions": "fromopt3",
+		"ArgFromMeta":                         "frommeta1",
+		"ArgFromMetaOverriden":                "fromdockerfile3",
+	}
+	assert.DeepEqual(t, all, expected)
+}
+
+func TestGetAllMeta(t *testing.T) {
+	buildArgs := newBuildArgs(map[string]*string{
+		"ArgNotUsedInDockerfile":        strPtr("fromopt1"),
+		"ArgOverriddenByOptions":        strPtr("fromopt2"),
+		"ArgNoDefaultInMetaFromOptions": strPtr("fromopt3"),
+		"HTTP_PROXY":                    strPtr("theproxy"),
+	})
+
+	buildArgs.AddMetaArg("ArgFromMeta", strPtr("frommeta1"))
+	buildArgs.AddMetaArg("ArgOverriddenByOptions", strPtr("frommeta2"))
+	buildArgs.AddMetaArg("ArgNoDefaultInMetaFromOptions", nil)
+
+	all := buildArgs.GetAllMeta()
+	expected := map[string]string{
+		"HTTP_PROXY":                    "theproxy",
+		"ArgFromMeta":                   "frommeta1",
+		"ArgOverriddenByOptions":        "fromopt2",
+		"ArgNoDefaultInMetaFromOptions": "fromopt3",
+	}
+	assert.DeepEqual(t, all, expected)
+}

builder/dockerfile/builder.go

@@ -36,20 +36,6 @@ var validCommitCommands = map[string]bool{
 	"workdir":     true,
 }
 
-// BuiltinAllowedBuildArgs is list of built-in allowed build args
-// these args are considered transparent and are excluded from the image history.
-// Filtering from history is implemented in dispatchers.go
-var BuiltinAllowedBuildArgs = map[string]bool{
-	"HTTP_PROXY":  true,
-	"http_proxy":  true,
-	"HTTPS_PROXY": true,
-	"https_proxy": true,
-	"FTP_PROXY":   true,
-	"ftp_proxy":   true,
-	"NO_PROXY":    true,
-	"no_proxy":    true,
-}
-
 var defaultLogConfig = container.LogConfig{Type: "none"}
 
 // Builder is a Dockerfile builder
@@ -66,20 +52,19 @@ type Builder struct {
 	clientCtx context.Context
 	cancel    context.CancelFunc
 
-	dockerfile       *parser.Node
-	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
-	maintainer       string
-	cmdSet           bool
-	disableCommit    bool
-	cacheBusted      bool
-	allowedBuildArgs map[string]*string  // list of build-time args that are allowed for expansion/substitution and passing to commands in 'run'.
-	allBuildArgs     map[string]struct{} // list of all build-time args found during parsing of the Dockerfile
-	directive        parser.Directive
+	dockerfile    *parser.Node
+	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
+	directive     parser.Directive
 
 	// TODO: remove once docker.Commit can receive a tag
 	id string
@@ -134,18 +119,17 @@ func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, back
 	}
 	ctx, cancel := context.WithCancel(clientCtx)
 	b = &Builder{
-		clientCtx:        ctx,
-		cancel:           cancel,
-		options:          config,
-		Stdout:           os.Stdout,
-		Stderr:           os.Stderr,
-		docker:           backend,
-		context:          buildContext,
-		runConfig:        new(container.Config),
-		tmpContainers:    map[string]struct{}{},
-		id:               stringid.GenerateNonCryptoID(),
-		allowedBuildArgs: make(map[string]*string),
-		allBuildArgs:     make(map[string]struct{}),
+		clientCtx:     ctx,
+		cancel:        cancel,
+		options:       config,
+		Stdout:        os.Stdout,
+		Stderr:        os.Stderr,
+		docker:        backend,
+		context:       buildContext,
+		runConfig:     new(container.Config),
+		tmpContainers: map[string]struct{}{},
+		id:            stringid.GenerateNonCryptoID(),
+		buildArgs:     newBuildArgs(config.BuildArgs),
 		directive: parser.Directive{
 			EscapeSeen:           false,
 			LookingForDirectives: true,
@@ -316,18 +300,17 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
 // check if there are any leftover build-args that were passed but not
 // consumed during build. Print a warning, if there are any.
 func (b *Builder) warnOnUnusedBuildArgs() {
-	leftoverArgs := []string{}
-	for arg := range b.options.BuildArgs {
-		if _, ok := b.allBuildArgs[arg]; !ok {
-			leftoverArgs = append(leftoverArgs, arg)
-		}
-	}
-
+	leftoverArgs := b.buildArgs.UnreferencedOptionArgs()
 	if len(leftoverArgs) > 0 {
 		fmt.Fprintf(b.Stderr, "[Warning] One or more build-args %v were not consumed\n", leftoverArgs)
 	}
 }
 
+// hasFromImage returns true if the builder has processed a `FROM <image>` line
+func (b *Builder) hasFromImage() bool {
+	return b.image != "" || b.noBaseImage
+}
+
 // Cancel cancels an ongoing Dockerfile build.
 func (b *Builder) Cancel() {
 	b.cancel()

builder/dockerfile/dispatchers.go

@@ -218,7 +218,15 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
 		return err
 	}
 
-	name := args[0]
+	substituionArgs := []string{}
+	for key, value := range b.buildArgs.GetAllMeta() {
+		substituionArgs = append(substituionArgs, key+"="+value)
+	}
+
+	name, err := ProcessWord(args[0], substituionArgs, b.directive.EscapeToken)
+	if err != nil {
+		return err
+	}
 
 	var image builder.Image
 
@@ -252,8 +260,7 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
 	}
 	b.from = image
 
-	b.allowedBuildArgs = make(map[string]*string)
-
+	b.buildArgs.ResetAllowed()
 	return b.processImageFrom(image)
 }
 
@@ -360,7 +367,7 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
 // RUN [ "echo", "hi" ] # echo hi
 //
 func run(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if b.image == "" && !b.noBaseImage {
+	if !b.hasFromImage() {
 		return errors.New("Please provide a source image with `from` prior to run")
 	}
 
@@ -438,19 +445,16 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 	// properly match it.
 	b.runConfig.Env = env
 
-	// remove BuiltinAllowedBuildArgs (see: builder.go)  from the saveCmd
+	// remove builtinAllowedBuildArgs (see: builder.go)  from the saveCmd
 	// these args are transparent so resulting image should be the same regardless of the value
 	if len(cmdBuildEnv) > 0 {
 		saveCmd = config.Cmd
 		tmpBuildEnv := make([]string, len(cmdBuildEnv))
 		copy(tmpBuildEnv, cmdBuildEnv)
 		for i, env := range tmpBuildEnv {
 			key := strings.SplitN(env, "=", 2)[0]
-			if _, ok := BuiltinAllowedBuildArgs[key]; ok {
-				// If an built-in arg is explicitly added in the Dockerfile, don't prune it
-				if _, ok := b.allowedBuildArgs[key]; !ok {
-					tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...)
-				}
+			if b.buildArgs.IsUnreferencedBuiltin(key) {
+				tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...)
 			}
 		}
 		sort.Strings(tmpBuildEnv)
@@ -781,15 +785,18 @@ func arg(b *Builder, args []string, attributes map[string]bool, original string)
 		name = arg
 		hasDefault = false
 	}
-	// add the arg to allowed list of build-time args from this step on.
-	b.allBuildArgs[name] = struct{}{}
 
 	var value *string
 	if hasDefault {
 		value = &newValue
 	}
-	b.allowedBuildArgs[name] = value
+	b.buildArgs.AddArg(name, value)
 
+	// Arg before FROM doesn't add a layer
+	if !b.hasFromImage() {
+		b.buildArgs.AddMetaArg(name, value)
+		return nil
+	}
 	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("ARG %s", arg))
 }