Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse arg commands at the top of dockerfiles #404

Merged
merged 12 commits into from
Nov 6, 2018
Merged
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

# Bump these on release
VERSION_MAJOR ?= 0
VERSION_MINOR ?= 3
VERSION_MINOR ?= 5
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this!! Must have missed it...many times... 😅

VERSION_BUILD ?= 0

VERSION ?= v$(VERSION_MAJOR).$(VERSION_MINOR).$(VERSION_BUILD)
Expand Down
17 changes: 17 additions & 0 deletions integration/dockerfiles/Dockerfile_test_meta_arg
Original file line number Diff line number Diff line change
@@ -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}
6 changes: 6 additions & 0 deletions pkg/commands/arg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions pkg/config/stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ type KanikoStage struct {
Final bool
BaseImageStoredLocally bool
SaveStage bool
MetaArgs []instructions.ArgCommand
}
9 changes: 9 additions & 0 deletions pkg/dockerfile/buildargs.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"

d "github.com/docker/docker/builder/dockerfile"
"github.com/moby/buildkit/frontend/dockerfile/instructions"
)

type BuildArgs struct {
Expand Down Expand Up @@ -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)
}
}
14 changes: 8 additions & 6 deletions pkg/dockerfile/dockerfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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
}

Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/executor/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/executor/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
9 changes: 8 additions & 1 deletion pkg/util/image_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package util

import (
"crypto/tls"
"fmt"
"net/http"
"path/filepath"
"strconv"
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/util/image_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
}
Expand All @@ -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)
}

Expand Down