fix: copy dependencies to a new slice to prevent mutation affecting the cache value

[sberss]

Description

There is a bad interaction in dependency building which exposes itself only under a specific set of conditions, the result of which is that skaffold dev hashes the wrong set of files for an artefact.

The set of files that make up an artefact are stored in a sync.Map, and when it is loaded the underlying array will be mutated if the following conditions are met:

• The artefact you are building depends on another artefact.
• The files of the child artefact appear before the files of the parent artefact when sorted alphabetically. E.g. parent: /path/to/project/artefact-foo, child: /path/to/project/artefact-bar.
• The remaining capacity of the slice containing the files for the parents is large enough that appending the files for the child does not create a new underlying array.

When the above conditions are met, this sort will mutate the underlying array in such a way that future loads from the store for the parent artefact will return a slice with the child artefacts files at the top, and some of the parent artefacts files will be pushed out (depending on the number of files that make up the child artefact).

This can be trivially reproduced outside of skaffold:

package main

import (
	"fmt"
	"reflect"
	"sort"
	"unsafe"
)

func main() {
	foo := make(map[string][]string)
	arr := make([]string, 0, 92)
	arr = append(arr, "c", "d")
	foo["x"] = arr

	var pulled []string
	pulled = foo["x"]
	pulled = append(pulled, []string{"a", "b"}...)

	var pulled2 []string
	pulled2 = foo["x"]

	hdr := (*reflect.SliceHeader)(unsafe.Pointer(&pulled2))
	data := (*[4]string)(unsafe.Pointer(hdr.Data))

	fmt.Printf("foo[\"x\"] before sorting: %v. Underlying array: %v\n", foo["x"], data)

	sort.Strings(pulled)

	fmt.Printf("foo[\"x\"] after sorting: %v. Underlying array: %v\n", foo["x"], data)
}

This PR creates a copy of the deps slice before returning it to be mutated. I'm not sure if this is the right fix, but it does do the right thing.

[conventional-commit-lint-gcf[bot]]

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot https://conventionalcommits.org/

[codecov[bot]]

Codecov Report

Merging #7875 (bafab27) into main (290280e) will decrease coverage by 3.85%. The diff coverage is 53.94%.

@@            Coverage Diff             @@
##             main    #7875      +/-   ##
==========================================
- Coverage   70.48%   66.63%   -3.86%     
==========================================
  Files         515      593      +78     
  Lines       23150    28718    +5568     
==========================================
+ Hits        16317    19135    +2818     
- Misses       5776     8176    +2400     
- Partials     1057     1407     +350     

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/credits/export.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go	40.90% <0.00%> (-12.94%)	:arrow_down:
cmd/skaffold/app/cmd/test.go	44.44% <0.00%> (ø)
cmd/skaffold/skaffold.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go	62.50% <14.28%> (-1.14%)	:arrow_down:
cmd/skaffold/app/cmd/render.go	35.48% <18.18%> (-5.90%)	:arrow_down:
cmd/skaffold/app/cmd/lsp.go	28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/run.go	64.28% <33.33%> (-9.63%)	:arrow_down:
cmd/skaffold/app/cmd/fix.go	56.41% <37.50%> (-20.07%)	:arrow_down:
cmd/skaffold/app/cmd/verify.go	41.17% <41.17%> (ø)
... and 367 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more