Fix composite cache key for multi-stage copy command (#1735)

* chore: add workflows for pr tests

* fix unit tests

* fix formatting

* chore: fix gobuild

* change minikube script

* chore: fix lint install script

* chore: ignore and fix tests

* fix lint and run gofmt

* lint fixes

* k8s executor image only

* fix Makefile

* fix travis env variables

* more info on k8s tests

* fix travis run

* fix

* fix

* fix

* fix log

* some more changes

* increase timeout

* delete travis.yml and fix multiple copy tests

* fix registry mirror

* fix lint

* add concurency

* last attemot to fix k8 integrations

* diff id for diff workflows

* Fix composite cache key for multi-stage copy command (#1706)

PR #1518 reintroduced COPY layers caching using the `--cache-copy-layers`
flag. Unfortunately, this PR also introduced a bug by not including the
stage digest into the caching key of the COPY command when the
`--cache-copy-layers` flag was not set. As a result, kaniko would use
any previous (possibly stalled) layer from the cache because the digest
of the "COPY --from" command would never change.

PR author probably expected Go to fallthrough in the switch just like C
does. However, this is not the case. Go does not fallthrough in
switch-statements by default and requires the fallthrough keyword to be
used. Note that this keyword is not available in type-switches though,
because it wouldn't work properly with typings.

* refactor: add an abstract copy command interface to avoid code duplication

* fix typo in error message

Co-authored-by: Tejal Desai <[email protected]>

integration/integration_test.go

@@ -483,7 +483,7 @@ func TestCache(t *testing.T) {
 			}
 			// Build the second image which should pull from the cache
 			if err := imageBuilder.buildCachedImages(config, cache, dockerfilesPath, 1, args); err != nil {
-				t.Fatalf("error building cached image for the first time: %v", err)
+				t.Fatalf("error building cached image for the second time: %v", err)
 			}
 			// Make sure both images are the same
 			kanikoVersion0 := GetVersionedKanikoImage(config.imageRepo, dockerfile, 0)

pkg/commands/copy.go

@@ -293,3 +293,20 @@ func copyCmdFilesUsedFromContext(
 
 	return files, nil
 }
+
+// AbstractCopyCommand can either be a CopyCommand or a CachingCopyCommand.
+type AbstractCopyCommand interface {
+	From() string
+}
+
+// CastAbstractCopyCommand tries to convert a command to an AbstractCopyCommand.
+func CastAbstractCopyCommand(cmd interface{}) (AbstractCopyCommand, bool) {
+	switch v := cmd.(type) {
+	case *CopyCommand:
+		return v, true
+	case *CachingCopyCommand:
+		return v, true
+	}
+
+	return nil, false
+}

pkg/executor/build.go

@@ -187,10 +187,8 @@ func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string
 	}
 	// Add the next command to the cache key.
 	compositeKey.AddKey(resolvedCmd)
-	switch v := command.(type) {
-	case *commands.CopyCommand:
-	case *commands.CachingCopyCommand:
-		compositeKey = s.populateCopyCmdCompositeKey(command, v.From(), compositeKey)
+	if copyCmd, ok := commands.CastAbstractCopyCommand(command); ok == true {
+		compositeKey = s.populateCopyCmdCompositeKey(command, copyCmd.From(), compositeKey)
 	}
 
 	for _, f := range files {

pkg/executor/build_test.go

@@ -1361,6 +1361,81 @@ func hashCompositeKeys(t *testing.T, ck1 CompositeCache, ck2 CompositeCache) (st
 	return key1, key2
 }
 
+func Test_stageBuild_populateCompositeKeyForCopyCommand(t *testing.T) {
+	// See https://github.com/GoogleContainerTools/kaniko/issues/589
+
+	for _, tc := range []struct {
+		description      string
+		command          string
+		expectedCacheKey string
+	}{
+		{
+			description:      "multi-stage copy command",
+			command:          "COPY --from=0 foo.txt bar.txt",
+			expectedCacheKey: "COPY --from=0 foo.txt bar.txt-some-cache-key",
+		},
+		{
+			description:      "copy command",
+			command:          "COPY foo.txt bar.txt",
+			expectedCacheKey: "COPY foo.txt bar.txt",
+		},
+	} {
+		t.Run(tc.description, func(t *testing.T) {
+			instructions, err := dockerfile.ParseCommands([]string{tc.command})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			fc := util.FileContext{Root: "workspace"}
+			copyCommand, err := commands.GetCommand(instructions[0], fc, false, true)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			for _, useCacheCommand := range []bool{false, true} {
+				t.Run(fmt.Sprintf("CacheCommand=%t", useCacheCommand), func(t *testing.T) {
+					var cmd fmt.Stringer = copyCommand
+					if useCacheCommand {
+						cmd = copyCommand.(*commands.CopyCommand).CacheCommand(nil)
+					}
+
+					sb := &stageBuilder{
+						fileContext: fc,
+						stageIdxToDigest: map[string]string{
+							"0": "some-digest",
+						},
+						digestToCacheKey: map[string]string{
+							"some-digest": "some-cache-key",
+						},
+					}
+
+					ck := CompositeCache{}
+					ck, err = sb.populateCompositeKey(
+						cmd,
+						[]string{},
+						ck,
+						dockerfile.NewBuildArgs([]string{}),
+						[]string{},
+					)
+					if err != nil {
+						t.Fatal(err)
+					}
+
+					actualCacheKey := ck.Key()
+					if tc.expectedCacheKey != actualCacheKey {
+						t.Errorf(
+							"Expected cache key to be %s, was %s",
+							tc.expectedCacheKey,
+							actualCacheKey,
+						)
+					}
+
+				})
+			}
+		})
+	}
+}
+
 func Test_ResolveCrossStageInstructions(t *testing.T) {
 	df := `
 	FROM scratch