fix: consider excluede file in COPY --from cache keys #2615

GoogleContainerTools · Jul 10, 2023 · ce24f5d · ce24f5d
1 parent 63be499
commit ce24f5d
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 14 deletions.
diff --git a/pkg/commands/base_command.go b/pkg/commands/base_command.go
@@ -28,6 +28,10 @@ func (b *BaseCommand) IsArgsEnvsRequiredInCache() bool {
 	return false
 }
 
+func (b *BaseCommand) IsConsiderExcludedFilesInCache() bool {
+	return false
+}
+
 func (b *BaseCommand) CacheCommand(v1.Image) DockerCommand {
 	return nil
 }

diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go
@@ -62,6 +62,10 @@ type DockerCommand interface {
 	// True if need add ARGs and EVNs to composite cache string with resolved command
 	// need only for RUN instruction
 	IsArgsEnvsRequiredInCache() bool
+
+	// True if need add consider excluded files to composite cache string with resolved command
+	// need only for COPY --from instruction
+	IsConsiderExcludedFilesInCache() bool
 }
 
 func GetCommand(cmd instructions.Command, fileContext util.FileContext, useNewRun bool, cacheCopy bool, cacheRun bool) (DockerCommand, error) {

diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go
@@ -152,6 +152,10 @@ func (c *CopyCommand) ShouldCacheOutput() bool {
 	return c.shdCache
 }
 
+func (c *CopyCommand) IsConsiderExcludedFilesInCache() bool {
+	return c.cmd.From != ""
+}
+
 // CacheCommand returns true since this command should be cached
 func (c *CopyCommand) CacheCommand(img v1.Image) DockerCommand {
 	return &CachingCopyCommand{

diff --git a/pkg/executor/build.go b/pkg/executor/build.go
@@ -223,8 +223,9 @@ func (s *stageBuilder) populateCompositeKey(command commands.DockerCommand, file
 	// Add the next command to the cache key.
 	compositeKey.AddKey(resolvedCmd)
 
+	considerExcludedFiles := command.IsConsiderExcludedFilesInCache()
 	for _, f := range files {
-		if err := compositeKey.AddPath(f, s.fileContext); err != nil {
+		if err := compositeKey.AddPath(f, s.fileContext, considerExcludedFiles); err != nil {
 			return compositeKey, err
 		}
 	}

diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go
@@ -905,7 +905,7 @@ func Test_stageBuilder_build(t *testing.T) {
 			filePath := filepath.Join(dir, file)
 			ch := NewCompositeCache("", "meow")
 
-			ch.AddPath(filePath, util.FileContext{})
+			ch.AddPath(filePath, util.FileContext{}, false)
 			hash, err := ch.Hash()
 			if err != nil {
 				t.Errorf("couldn't create hash %v", err)
@@ -935,7 +935,7 @@ func Test_stageBuilder_build(t *testing.T) {
 			filePath := filepath.Join(dir, file)
 			ch := NewCompositeCache("", "meow")
 
-			ch.AddPath(filePath, util.FileContext{})
+			ch.AddPath(filePath, util.FileContext{}, false)
 			hash, err := ch.Hash()
 			if err != nil {
 				t.Errorf("couldn't create hash %v", err)
@@ -968,7 +968,7 @@ func Test_stageBuilder_build(t *testing.T) {
 			filePath := filepath.Join(dir, file)
 			ch := NewCompositeCache("", "meow")
 
-			ch.AddPath(filePath, util.FileContext{})
+			ch.AddPath(filePath, util.FileContext{}, false)
 			hash, err := ch.Hash()
 			if err != nil {
 				t.Errorf("couldn't create hash %v", err)
@@ -1014,7 +1014,7 @@ func Test_stageBuilder_build(t *testing.T) {
 			tarContent := generateTar(t, dir, filename)
 
 			ch := NewCompositeCache("", fmt.Sprintf("COPY %s foo.txt", filename))
-			ch.AddPath(filepath, util.FileContext{})
+			ch.AddPath(filepath, util.FileContext{}, false)
 
 			hash, err := ch.Hash()
 			if err != nil {
@@ -1081,7 +1081,7 @@ func Test_stageBuilder_build(t *testing.T) {
 			destDir := t.TempDir()
 			filePath := filepath.Join(dir, filename)
 			ch := NewCompositeCache("", fmt.Sprintf("COPY %s foo.txt", filename))
-			ch.AddPath(filePath, util.FileContext{})
+			ch.AddPath(filePath, util.FileContext{}, false)
 
 			hash, err := ch.Hash()
 			if err != nil {
@@ -1147,15 +1147,15 @@ COPY %s foo.txt
 			}
 
 			ch.AddKey(fmt.Sprintf("COPY %s bar.txt", filename))
-			ch.AddPath(filePath, util.FileContext{})
+			ch.AddPath(filePath, util.FileContext{}, false)
 
 			hash2, err := ch.Hash()
 			if err != nil {
 				t.Errorf("couldn't create hash %v", err)
 			}
 			ch = NewCompositeCache("", fmt.Sprintf("COPY %s foo.txt", filename))
 			ch.AddKey(fmt.Sprintf("COPY %s bar.txt", filename))
-			ch.AddPath(filePath, util.FileContext{})
+			ch.AddPath(filePath, util.FileContext{}, false)
 
 			image := fakeImage{
 				ImageLayers: []v1.Layer{
@@ -1215,7 +1215,7 @@ COPY %s bar.txt
 			filePath := filepath.Join(dir, filename)
 
 			ch := NewCompositeCache("", fmt.Sprintf("COPY %s bar.txt", filename))
-			ch.AddPath(filePath, util.FileContext{})
+			ch.AddPath(filePath, util.FileContext{}, false)
 
 			// copy hash
 			_, err := ch.Hash()

diff --git a/pkg/executor/composite_cache.go b/pkg/executor/composite_cache.go
@@ -55,7 +55,7 @@ func (s *CompositeCache) Hash() (string, error) {
 	return util.SHA256(strings.NewReader(s.Key()))
 }
 
-func (s *CompositeCache) AddPath(p string, context util.FileContext) error {
+func (s *CompositeCache) AddPath(p string, context util.FileContext, considerExcludedFiles bool) error {
 	sha := sha256.New()
 	fi, err := os.Lstat(p)
 	if err != nil {
@@ -70,7 +70,7 @@ func (s *CompositeCache) AddPath(p string, context util.FileContext) error {
 
 		// Only add the hash of this directory to the key
 		// if there is any ignored content.
-		if !empty || !context.ExcludesFile(p) {
+		if !empty || considerExcludedFiles || !context.ExcludesFile(p) {
 			s.keys = append(s.keys, k)
 		}
 		return nil

diff --git a/pkg/executor/composite_cache_test.go b/pkg/executor/composite_cache_test.go
@@ -77,7 +77,7 @@ func Test_CompositeCache_AddPath_dir(t *testing.T) {
 
 	fn := func() string {
 		r := NewCompositeCache()
-		if err := r.AddPath(tmpDir, util.FileContext{}); err != nil {
+		if err := r.AddPath(tmpDir, util.FileContext{}, false); err != nil {
 			t.Errorf("expected error to be nil but was %v", err)
 		}
 
@@ -115,7 +115,7 @@ func Test_CompositeCache_AddPath_file(t *testing.T) {
 	p := tmpfile.Name()
 	fn := func() string {
 		r := NewCompositeCache()
-		if err := r.AddPath(p, util.FileContext{}); err != nil {
+		if err := r.AddPath(p, util.FileContext{}, false); err != nil {
 			t.Errorf("expected error to be nil but was %v", err)
 		}
 
@@ -167,7 +167,7 @@ func setIgnoreContext(t *testing.T, content string) (util.FileContext, error) {
 
 func hashDirectory(dirpath string, fileContext util.FileContext) (string, error) {
 	cache1 := NewCompositeCache()
-	err := cache1.AddPath(dirpath, fileContext)
+	err := cache1.AddPath(dirpath, fileContext, false)
 	if err != nil {
 		return "", err
 	}

diff --git a/pkg/executor/fakes.go b/pkg/executor/fakes.go
@@ -47,6 +47,7 @@ type MockDockerCommand struct {
 	contextFiles        []string
 	cacheCommand        commands.DockerCommand
 	argToCompositeCache bool
+	considerExcludedFiles bool
 }
 
 func (m MockDockerCommand) ExecuteCommand(c *v1.Config, args *dockerfile.BuildArgs) error { return nil }
@@ -80,10 +81,14 @@ func (m MockDockerCommand) ShouldDetectDeletedFiles() bool {
 func (m MockDockerCommand) IsArgsEnvsRequiredInCache() bool {
 	return m.argToCompositeCache
 }
+func (m MockDockerCommand) IsConsiderExcludedFilesInCache() bool {
+	return m.argToCompositeCache
+}
 
 type MockCachedDockerCommand struct {
 	contextFiles        []string
 	argToCompositeCache bool
+	considerExcludedFiles bool
 }
 
 func (m MockCachedDockerCommand) ExecuteCommand(c *v1.Config, args *dockerfile.BuildArgs) error {
@@ -119,6 +124,9 @@ func (m MockCachedDockerCommand) ShouldCacheOutput() bool {
 func (m MockCachedDockerCommand) IsArgsEnvsRequiredInCache() bool {
 	return m.argToCompositeCache
 }
+func (m MockCachedDockerCommand) IsConsiderExcludedFilesInCache() bool {
+	return m.considerExcludedFiles
+}
 
 type fakeLayerCache struct {
 	retrieve     bool