diff --git a/pkg/executor/build.go b/pkg/executor/build.go index dd7021a77..c0add7e86 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -72,7 +72,7 @@ type stageBuilder struct { stageIdxToDigest map[string]string snapshotter snapShotter layerCache cache.LayerCache - pushCache cachePusher + pushLayerToCache cachePusher } // newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage @@ -115,7 +115,7 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, cross layerCache: &cache.RegistryCache{ Opts: opts, }, - pushCache: pushLayerToCache, + pushLayerToCache: pushLayerToCache, } for _, cmd := range s.stage.Commands { @@ -146,9 +146,15 @@ func initializeConfig(img partial.WithConfigFile) (*v1.ConfigFile, error) { return imageConfig, nil } -func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string, compositeKey CompositeCache) (CompositeCache, error) { +func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string, compositeKey CompositeCache, args *dockerfile.BuildArgs, env []string) (CompositeCache, error) { + // First replace all the environment variables or args in the command + replacementEnvs := args.ReplacementEnvs(env) + resolvedCmd, err := util.ResolveEnvironmentReplacement(command.String(), replacementEnvs, false) + if err != nil { + return compositeKey, err + } // Add the next command to the cache key. - compositeKey.AddKey(command.String()) + compositeKey.AddKey(resolvedCmd) switch v := command.(type) { case *commands.CopyCommand: compositeKey = s.populateCopyCmdCompositeKey(command, v.From(), compositeKey) @@ -201,7 +207,7 @@ func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) erro return errors.Wrap(err, "failed to get files used from context") } - compositeKey, err = s.populateCompositeKey(command, files, compositeKey) + compositeKey, err = s.populateCompositeKey(command, files, compositeKey, s.args, cfg.Env) if err != nil { return err } @@ -251,8 +257,6 @@ func (s *stageBuilder) build() error { compositeKey = NewCompositeCache(s.baseImageDigest) } - compositeKey.AddKey(s.opts.BuildArgs...) - // Apply optimizations to the instructions. if err := s.optimize(*compositeKey, s.cf.Config); err != nil { return errors.Wrap(err, "failed to optimize instructions") @@ -309,7 +313,7 @@ func (s *stageBuilder) build() error { return errors.Wrap(err, "failed to get files used from context") } - *compositeKey, err = s.populateCompositeKey(command, files, *compositeKey) + *compositeKey, err = s.populateCompositeKey(command, files, *compositeKey, s.args, s.cf.Config.Env) if err != nil { return err } @@ -358,7 +362,7 @@ func (s *stageBuilder) build() error { // Push layer to cache (in parallel) now along with new config file if s.opts.Cache && command.ShouldCacheOutput() { cacheGroup.Go(func() error { - return s.pushCache(s.opts, ck, tarPath, command.String()) + return s.pushLayerToCache(s.opts, ck, tarPath, command.String()) }) } if err := s.saveSnapshotToImage(command.String(), tarPath); err != nil { diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 64046bc29..ef09de5f7 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -517,6 +517,130 @@ func Test_stageBuilder_optimize(t *testing.T) { } } +type stageContext struct { + command fmt.Stringer + args *dockerfile.BuildArgs + env []string +} + +func newStageContext(command string, args map[string]string, env []string) stageContext { + dockerArgs := dockerfile.NewBuildArgs([]string{}) + for k, v := range args { + dockerArgs.AddArg(k, &v) + } + return stageContext{MockDockerCommand{command: command}, dockerArgs, env} +} + +func Test_stageBuilder_populateCompositeKey(t *testing.T) { + testCases := []struct { + description string + cmd1 stageContext + cmd2 stageContext + shdEqual bool + }{ + { + description: "cache key for same command, different buildargs, args not used in command", + cmd1: newStageContext( + "RUN echo const > test", + map[string]string{"ARG": "foo"}, + []string{"ENV=foo1"}, + ), + cmd2: newStageContext( + "RUN echo const > test", + map[string]string{"ARG": "bar"}, + []string{"ENV=bar1"}, + ), + shdEqual: true, + }, + { + description: "cache key for same command with same build args", + cmd1: newStageContext( + "RUN echo $ARG > test", + map[string]string{"ARG": "foo"}, + []string{}, + ), + cmd2: newStageContext( + "RUN echo $ARG > test", + map[string]string{"ARG": "foo"}, + []string{}, + ), + shdEqual: true, + }, + { + description: "cache key for same command with same env", + cmd1: newStageContext( + "RUN echo $ENV > test", + map[string]string{"ARG": "foo"}, + []string{"ENV=same"}, + ), + cmd2: newStageContext( + "RUN echo $ENV > test", + map[string]string{"ARG": "bar"}, + []string{"ENV=same"}, + ), + shdEqual: true, + }, + { + description: "cache key for same command with a build arg values", + cmd1: newStageContext( + "RUN echo $ARG > test", + map[string]string{"ARG": "foo"}, + []string{}, + ), + cmd2: newStageContext( + "RUN echo $ARG > test", + map[string]string{"ARG": "bar"}, + []string{}, + ), + }, + { + description: "cache key for same command with different env values", + cmd1: newStageContext( + "RUN echo $ENV > test", + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + cmd2: newStageContext( + "RUN echo $ENV > test", + map[string]string{"ARG": "foo"}, + []string{"ENV=2"}, + ), + }, + { + description: "cache key for different command same context", + cmd1: newStageContext( + "RUN echo other > test", + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + cmd2: newStageContext( + "RUN echo another > test", + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + }, + } + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + sb := &stageBuilder{opts: &config.KanikoOptions{SrcContext: "workspace"}} + ck := CompositeCache{} + + ck1, err := sb.populateCompositeKey(tc.cmd1.command, []string{}, ck, tc.cmd1.args, tc.cmd1.env) + if err != nil { + t.Errorf("Expected error to be nil but was %v", err) + } + ck2, err := sb.populateCompositeKey(tc.cmd2.command, []string{}, ck, tc.cmd2.args, tc.cmd2.env) + if err != nil { + t.Errorf("Expected error to be nil but was %v", err) + } + key1, key2 := hashCompositeKeys(t, ck1, ck2) + if b := key1 == key2; b != tc.shdEqual { + t.Errorf("expected keys to be equal as %t but found %t", tc.shdEqual, !tc.shdEqual) + } + }) + } +} + func Test_stageBuilder_build(t *testing.T) { type testcase struct { description string @@ -881,7 +1005,7 @@ COPY %s bar.txt cf: cf, snapshotter: snap, layerCache: lc, - pushCache: func(_ *config.KanikoOptions, cacheKey, _, _ string) error { + pushLayerToCache: func(_ *config.KanikoOptions, cacheKey, _, _ string) error { keys = append(keys, cacheKey) return nil }, @@ -993,3 +1117,15 @@ func generateTar(t *testing.T, dir string, fileNames ...string) []byte { } return buf.Bytes() } + +func hashCompositeKeys(t *testing.T, ck1 CompositeCache, ck2 CompositeCache) (string, string) { + key1, err := ck1.Hash() + if err != nil { + t.Errorf("could not hash composite key due to %s", err) + } + key2, err := ck2.Hash() + if err != nil { + t.Errorf("could not hash composite key due to %s", err) + } + return key1, key2 +} diff --git a/pkg/executor/fakes.go b/pkg/executor/fakes.go index e9cfdc694..e89d5ac47 100644 --- a/pkg/executor/fakes.go +++ b/pkg/executor/fakes.go @@ -43,13 +43,14 @@ func (f fakeSnapShotter) TakeSnapshot(_ []string) (string, error) { } type MockDockerCommand struct { + command string contextFiles []string cacheCommand commands.DockerCommand } func (m MockDockerCommand) ExecuteCommand(c *v1.Config, args *dockerfile.BuildArgs) error { return nil } func (m MockDockerCommand) String() string { - return "meow" + return m.command } func (m MockDockerCommand) FilesToSnapshot() []string { return []string{"meow-snapshot-no-cache"}