parse arg commands at the top of dockerfiles (GoogleContainerTools#404)

* parse arg commands at the top of dockerfiles

* fix pointer reference bug and remove debugging

* fixing tests

* account for meta args with no value

* don't take fs snapshot if / is the only changed path

* move metaArgs inside KanikoStage

* removing unused property

* check for any directory instead of just /

* remove unnecessary check

Author: sharifelgamal

Makefile

@@ -14,7 +14,7 @@
 
 # Bump these on release
 VERSION_MAJOR ?= 0
-VERSION_MINOR ?= 3
+VERSION_MINOR ?= 5
 VERSION_BUILD ?= 0
 
 VERSION ?= v$(VERSION_MAJOR).$(VERSION_MINOR).$(VERSION_BUILD)

integration/dockerfiles/Dockerfile_test_meta_arg

@@ -0,0 +1,17 @@
+ARG REGISTRY=gcr.io
+ARG REPO=google-appengine
+ARG WORD=hello
+ARG W0RD2=hey
+
+FROM ${REGISTRY}/${REPO}/debian9 as stage1
+
+# Should evaluate WORD and create /tmp/hello
+ARG WORD
+RUN touch /${WORD}
+
+FROM ${REGISTRY}/${REPO}/debian9
+
+COPY --from=stage1 /hello /tmp
+
+# /tmp/hey should not get created without the ARG statement
+RUN touch /tmp/${WORD2}

pkg/commands/arg.go

@@ -42,7 +42,13 @@ func (r *ArgCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 			return err
 		}
 		resolvedValue = &value
+	} else {
+		meta := buildArgs.GetAllMeta()
+		if value, ok := meta[resolvedKey]; ok {
+			resolvedValue = &value
+		}
 	}
+
 	buildArgs.AddArg(resolvedKey, resolvedValue)
 	return nil
 }

pkg/config/stage.go

@@ -25,4 +25,5 @@ type KanikoStage struct {
 	Final                  bool
 	BaseImageStoredLocally bool
 	SaveStage              bool
+	MetaArgs               []instructions.ArgCommand
 }

pkg/dockerfile/buildargs.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	d "github.com/docker/docker/builder/dockerfile"
+	"github.com/moby/buildkit/frontend/dockerfile/instructions"
 )
 
 type BuildArgs struct {
@@ -53,3 +54,11 @@ func (b *BuildArgs) ReplacementEnvs(envs []string) []string {
 	filtered := b.FilterAllowed(envs)
 	return append(envs, filtered...)
 }
+
+// AddMetaArgs adds the supplied args map to b's allowedMetaArgs
+func (b *BuildArgs) AddMetaArgs(metaArgs []instructions.ArgCommand) {
+	for _, arg := range metaArgs {
+		v := arg.Value
+		b.AddMetaArg(arg.Key, v)
+	}
+}

pkg/dockerfile/dockerfile.go

@@ -38,7 +38,7 @@ func Stages(opts *config.KanikoOptions) ([]config.KanikoStage, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, fmt.Sprintf("reading dockerfile at path %s", opts.DockerfilePath))
 	}
-	stages, err := Parse(d)
+	stages, metaArgs, err := Parse(d)
 	if err != nil {
 		return nil, errors.Wrap(err, "parsing dockerfile")
 	}
@@ -60,11 +60,13 @@ func Stages(opts *config.KanikoOptions) ([]config.KanikoStage, error) {
 			BaseImageStoredLocally: (baseImageIndex(index, stages) != -1),
 			SaveStage:              saveStage(index, stages),
 			Final:                  index == targetStage,
+			MetaArgs:               metaArgs,
 		})
 		if index == targetStage {
 			break
 		}
 	}
+
 	return kanikoStages, nil
 }
 
@@ -83,16 +85,16 @@ func baseImageIndex(currentStage int, stages []instructions.Stage) int {
 }
 
 // Parse parses the contents of a Dockerfile and returns a list of commands
-func Parse(b []byte) ([]instructions.Stage, error) {
+func Parse(b []byte) ([]instructions.Stage, []instructions.ArgCommand, error) {
 	p, err := parser.Parse(bytes.NewReader(b))
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	stages, _, err := instructions.Parse(p.AST)
+	stages, metaArgs, err := instructions.Parse(p.AST)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	return stages, err
+	return stages, metaArgs, err
 }
 
 // targetStage returns the index of the target stage kaniko is trying to build

pkg/dockerfile/dockerfile_test.go

@@ -35,7 +35,7 @@ func Test_resolveStages(t *testing.T) {
 	FROM scratch
 	COPY --from=second /hi2 /hi3
 	`
-	stages, err := Parse([]byte(dockerfile))
+	stages, _, err := Parse([]byte(dockerfile))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -63,7 +63,7 @@ func Test_targetStage(t *testing.T) {
 	FROM scratch
 	COPY --from=second /hi2 /hi3
 	`
-	stages, err := Parse([]byte(dockerfile))
+	stages, _, err := Parse([]byte(dockerfile))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -142,7 +142,7 @@ func Test_SaveStage(t *testing.T) {
 			expected: false,
 		},
 	}
-	stages, err := Parse([]byte(testutil.Dockerfile))
+	stages, _, err := Parse([]byte(testutil.Dockerfile))
 	if err != nil {
 		t.Fatalf("couldn't retrieve stages from Dockerfile: %v", err)
 	}
@@ -177,7 +177,7 @@ func Test_baseImageIndex(t *testing.T) {
 		},
 	}
 
-	stages, err := Parse([]byte(testutil.Dockerfile))
+	stages, _, err := Parse([]byte(testutil.Dockerfile))
 	if err != nil {
 		t.Fatalf("couldn't retrieve stages from Dockerfile: %v", err)
 	}

pkg/executor/build.go

@@ -54,7 +54,7 @@ type stageBuilder struct {
 
 // newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage
 func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*stageBuilder, error) {
-	sourceImage, err := util.RetrieveSourceImage(stage, opts.BuildArgs, opts)
+	sourceImage, err := util.RetrieveSourceImage(stage, opts)
 	if err != nil {
 		return nil, err
 	}
@@ -136,6 +136,7 @@ func (s *stageBuilder) build() error {
 	}
 
 	args := dockerfile.NewBuildArgs(s.opts.BuildArgs)
+	args.AddMetaArgs(s.stage.MetaArgs)
 	for index, command := range cmds {
 		if command == nil {
 			continue

pkg/executor/build_test.go

@@ -66,7 +66,7 @@ func Test_reviewConfig(t *testing.T) {
 }
 
 func stage(t *testing.T, d string) config.KanikoStage {
-	stages, err := dockerfile.Parse([]byte(d))
+	stages, _, err := dockerfile.Parse([]byte(d))
 	if err != nil {
 		t.Fatalf("error parsing dockerfile: %v", err)
 	}

pkg/util/image_util.go

@@ -18,6 +18,7 @@ package util
 
 import (
 	"crypto/tls"
+	"fmt"
 	"net/http"
 	"path/filepath"
 	"strconv"
@@ -44,7 +45,13 @@ var (
 )
 
 // RetrieveSourceImage returns the base image of the stage at index
-func RetrieveSourceImage(stage config.KanikoStage, buildArgs []string, opts *config.KanikoOptions) (v1.Image, error) {
+func RetrieveSourceImage(stage config.KanikoStage, opts *config.KanikoOptions) (v1.Image, error) {
+	buildArgs := opts.BuildArgs
+	var metaArgsString []string
+	for _, arg := range stage.MetaArgs {
+		metaArgsString = append(metaArgsString, fmt.Sprintf("%s=%s", arg.Key, arg.ValueString()))
+	}
+	buildArgs = append(buildArgs, metaArgsString...)
 	currentBaseName, err := ResolveEnvironmentReplacement(stage.BaseName, buildArgs, false)
 	if err != nil {
 		return nil, err

pkg/util/image_util_test.go

@@ -57,7 +57,7 @@ func Test_StandardImage(t *testing.T) {
 	retrieveRemoteImage = mock
 	actual, err := RetrieveSourceImage(config.KanikoStage{
 		Stage: stages[0],
-	}, nil, &config.KanikoOptions{})
+	}, &config.KanikoOptions{})
 	testutil.CheckErrorAndDeepEqual(t, false, err, nil, actual)
 }
 func Test_ScratchImage(t *testing.T) {
@@ -67,7 +67,7 @@ func Test_ScratchImage(t *testing.T) {
 	}
 	actual, err := RetrieveSourceImage(config.KanikoStage{
 		Stage: stages[1],
-	}, nil, &config.KanikoOptions{})
+	}, &config.KanikoOptions{})
 	expected := empty.Image
 	testutil.CheckErrorAndDeepEqual(t, false, err, expected, actual)
 }
@@ -89,7 +89,7 @@ func Test_TarImage(t *testing.T) {
 		BaseImageStoredLocally: true,
 		BaseImageIndex:         0,
 		Stage:                  stages[2],
-	}, nil, &config.KanikoOptions{})
+	}, &config.KanikoOptions{})
 	testutil.CheckErrorAndDeepEqual(t, false, err, nil, actual)
 }