Merge pull request #1174 from dani29/onbuild-copy

optimize: don't parse Dockerfile twice, reusing stages

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,16 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	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
 	}

pkg/executor/build_test.go

@@ -322,8 +322,11 @@ 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)
 			}

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

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