From a3ce1cebf3291eeb4b3da720825b71f101c29c58 Mon Sep 17 00:00:00 2001 From: Dani Raznikov Date: Fri, 3 Apr 2020 17:39:59 +0300 Subject: [PATCH 1/3] optimize: don't parse Dockerfile twice and just reuse stages --- pkg/executor/build.go | 11 +++++------ pkg/executor/build_test.go | 8 ++++++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 224992b3db..8db06a0c9d 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -487,11 +487,7 @@ func (s *stageBuilder) saveLayerToImage(layer v1.Layer, createdBy string) error return err } -func CalculateDependencies(opts *config.KanikoOptions) (map[int][]string, error) { - stages, err := dockerfile.Stages(opts) - if err != nil { - return nil, err - } +func CalculateDependencies(stages []config.KanikoStage, opts *config.KanikoOptions) (map[int][]string, error) { images := []v1.Image{} depGraph := map[int][]string{} for _, s := range stages { @@ -559,15 +555,18 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { if err != nil { return nil, err } + + // TODO is this even used? if err := util.GetExcludedFiles(opts.DockerfilePath, opts.SrcContext); err != nil { return nil, err } + // Some stages may refer to other random images, not previous stages if err := fetchExtraStages(stages, opts); err != nil { return nil, err } - crossStageDependencies, err := CalculateDependencies(opts) + crossStageDependencies, err := CalculateDependencies(stages, opts) if err != nil { return nil, err } diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index e52961e8c9..121d01bd71 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -322,8 +322,12 @@ COPY --from=stage2 /bar /bat opts := &config.KanikoOptions{ DockerfilePath: f.Name(), } - - got, err := CalculateDependencies(opts) + testStages, err := dockerfile.Stages(opts) + if err != nil { + t.Errorf("Failed to parse test dockerfile to stages: %s", err) + } + + got, err := CalculateDependencies(testStages, opts) if err != nil { t.Errorf("got error: %s,", err) } From 3ab6524fe580464e98817a3a59505f10b82baf23 Mon Sep 17 00:00:00 2001 From: Dani Raznikov Date: Fri, 3 Apr 2020 17:39:59 +0300 Subject: [PATCH 2/3] optimize: don't parse Dockerfile twice and just reuse stages --- pkg/executor/build.go | 2 -- pkg/executor/build_test.go | 1 - 2 files changed, 3 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 8db06a0c9d..5479658989 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -556,7 +556,6 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { return nil, err } - // TODO is this even used? if err := util.GetExcludedFiles(opts.DockerfilePath, opts.SrcContext); err != nil { return nil, err } @@ -565,7 +564,6 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { if err := fetchExtraStages(stages, opts); err != nil { return nil, err } - crossStageDependencies, err := CalculateDependencies(stages, opts) if err != nil { return nil, err diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 121d01bd71..344e2611b0 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -326,7 +326,6 @@ COPY --from=stage2 /bar /bat if err != nil { t.Errorf("Failed to parse test dockerfile to stages: %s", err) } - got, err := CalculateDependencies(testStages, opts) if err != nil { t.Errorf("got error: %s,", err) From 6b44ed4477f6a5a6fbdbc706d0b6bfe16d983b74 Mon Sep 17 00:00:00 2001 From: Dani Raznikov Date: Sat, 4 Apr 2020 21:08:03 +0300 Subject: [PATCH 3/3] calculateDepdencies on a copy of envvars --- pkg/util/command_util.go | 5 ++- pkg/util/command_util_test.go | 69 +++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index d2a604c1ed..77de804915 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -290,8 +290,9 @@ func IsSrcRemoteFileURL(rawurl string) bool { return err == nil } -func UpdateConfigEnv(newEnvs []instructions.KeyValuePair, config *v1.Config, replacementEnvs []string) error { - for index, pair := range newEnvs { +func UpdateConfigEnv(envVars []instructions.KeyValuePair, config *v1.Config, replacementEnvs []string) error { + newEnvs := make([]instructions.KeyValuePair, len(envVars)) + for index, pair := range envVars { expandedKey, err := ResolveEnvironmentReplacement(pair.Key, replacementEnvs, false) if err != nil { return err diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index 46cb34ca50..018d9637b2 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -25,6 +25,8 @@ import ( "testing" "github.com/GoogleContainerTools/kaniko/testutil" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/moby/buildkit/frontend/dockerfile/instructions" ) var testURL = "https://github.com/GoogleContainerTools/runtimes-common/blob/master/LICENSE" @@ -275,6 +277,73 @@ func Test_MatchSources(t *testing.T) { } } +var updateConfigEnvTests = []struct { + name string + envVars []instructions.KeyValuePair + config *v1.Config + replacementEnvs []string + expectedEnv []string +}{ + { + name: "test env config update", + envVars: []instructions.KeyValuePair{ + { + Key: "key", + Value: "var", + }, + { + Key: "foo", + Value: "baz", + }}, + config: &v1.Config{}, + replacementEnvs: []string{}, + expectedEnv: []string{"key=var", "foo=baz"}, + }, { + name: "test env config update with replacmenets", + envVars: []instructions.KeyValuePair{ + { + Key: "key", + Value: "/var/run", + }, + { + Key: "env", + Value: "$var", + }, + { + Key: "foo", + Value: "$argarg", + }}, + config: &v1.Config{}, + replacementEnvs: []string{"var=/test/with'chars'/", "not=used", "argarg=\"a\"b\""}, + expectedEnv: []string{"key=/var/run", "env=/test/with'chars'/", "foo=\"a\"b\""}, + }, { + name: "test env config update replacing existing variable", + envVars: []instructions.KeyValuePair{ + { + Key: "alice", + Value: "nice", + }, + { + Key: "bob", + Value: "cool", + }}, + config: &v1.Config{Env: []string{"bob=used", "more=test"}}, + replacementEnvs: []string{}, + expectedEnv: []string{"bob=cool", "more=test", "alice=nice"}, + }, +} + +func Test_UpdateConfigEnvTests(t *testing.T) { + for _, test := range updateConfigEnvTests { + t.Run(test.name, func(t *testing.T) { + if err := UpdateConfigEnv(test.envVars, test.config, test.replacementEnvs); err != nil { + t.Fatalf("error updating config with env vars: %s", err) + } + testutil.CheckDeepEqual(t, test.expectedEnv, test.config.Env) + }) + } +} + var isSrcValidTests = []struct { name string srcsAndDest []string