Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ require (
github.com/spf13/pflag v1.0.10
github.com/stretchr/testify v1.11.1
github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399
github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0
github.com/xeipuuv/gojsonschema v1.2.0
go.lsp.dev/jsonrpc2 v0.10.0
go.lsp.dev/protocol v0.12.0
Expand Down Expand Up @@ -334,7 +335,6 @@ require (
github.com/subosito/gotenv v1.6.0 // indirect
github.com/theupdateframework/go-tuf v0.7.0 // indirect
github.com/theupdateframework/go-tuf/v2 v2.2.0 // indirect
github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0 // indirect
github.com/transparency-dev/formats v0.0.0-20251013100031-c085883e704b // indirect
github.com/transparency-dev/merkle v0.0.2 // indirect
github.com/transparency-dev/tessera v1.0.0 // indirect
Expand Down
127 changes: 116 additions & 11 deletions pkg/skaffold/build/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import (
"io"
"os"
"os/exec"
"strings"

v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/tonistiigi/go-csvvalue"

"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/instrumentation"
Expand All @@ -33,7 +35,6 @@ import (
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/platform"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util/stringslice"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/warnings"
)

Expand Down Expand Up @@ -149,44 +150,148 @@ func (b *Builder) pullCacheFromImages(ctx context.Context, out io.Writer, a *lat
return nil
}

for _, image := range a.CacheFrom {
imageID, err := b.localDocker.ImageID(ctx, image)
for _, cache := range a.CacheFrom {
imageRef, err := extractImageReference(cache)
if err != nil {
return fmt.Errorf("getting imageID for %q: %w", image, err)
return fmt.Errorf("parsing cache reference %q: %w", cache, err)
}
if imageRef == "" {
// Non-registry cache types (e.g., "type=local,src=...") are handled directly by buildx
continue
}

imageID, err := b.localDocker.ImageID(ctx, imageRef)
if err != nil {
return fmt.Errorf("getting imageID for %q: %w", imageRef, err)
}
if imageID != "" {
// already pulled
continue
}

if err := b.localDocker.Pull(ctx, out, image, pl); err != nil {
warnings.Printf("cacheFrom image %q couldn't be pulled for platform %q\n", image, pl)
if err := b.localDocker.Pull(ctx, out, imageRef, pl); err != nil {
warnings.Printf("cacheFrom image %q couldn't be pulled for platform %q\n", imageRef, pl)
}
}

return nil
}

// extractImageReference extracts an image reference from a cache specification.
// It handles both simple image references (e.g., "myimage:latest") and buildx cache format
// (e.g., "type=registry,ref=myimage:latest"). Returns the image reference if it's a registry
// cache type, or an empty string for other cache types.
func extractImageReference(cache string) (string, error) {
fields, err := csvvalue.Fields(cache, nil)
if err != nil {
return "", err
}

if len(fields) == 1 && !strings.Contains(fields[0], "=") {
return fields[0], nil
}

var cacheType, cacheRef string

for _, field := range fields {
parts := strings.SplitN(field, "=", 2)
if len(parts) != 2 {
return "", fmt.Errorf("invalid cache format: field %q is not in key=value format", field)
}
key := strings.ToLower(parts[0])
value := parts[1]
switch key {
case "type":
cacheType = value
case "ref":
cacheRef = value
}
}

if cacheType != "registry" {
return "", nil
}
if cacheRef == "" {
return "", fmt.Errorf("cache type is registry but ref is empty")
}

return cacheRef, nil
}

// adjustCacheFrom returns an artifact where any cache references from the artifactImage is changed to the tagged built image name instead.
func adjustCacheFrom(a *latest.Artifact, artifactTag string) *latest.Artifact {
if os.Getenv("SKAFFOLD_DISABLE_DOCKER_CACHE_ADJUSTMENT") != "" {
// allow this behaviour to be disabled
return a
}

if !stringslice.Contains(a.DockerArtifact.CacheFrom, a.ImageName) {
needsAdjustment := false
for _, cache := range a.DockerArtifact.CacheFrom {
imageRef, _ := extractImageReference(cache)
if imageRef == a.ImageName {
needsAdjustment = true
break
}
}

if !needsAdjustment {
return a
}

cf := make([]string, 0, len(a.DockerArtifact.CacheFrom))
for _, image := range a.DockerArtifact.CacheFrom {
if image == a.ImageName {
cf = append(cf, artifactTag)
for _, cache := range a.DockerArtifact.CacheFrom {
adjusted, err := adjustCacheEntry(cache, a.ImageName, artifactTag)
if err != nil {
// If we can't parse the cache entry, keep it as is
cf = append(cf, cache)
} else {
cf = append(cf, image)
cf = append(cf, adjusted)
}
}
copy := *a
copy.DockerArtifact.CacheFrom = cf
return &copy
}

// adjustCacheEntry adjusts a single cache entry, replacing the image reference if it matches imageName.
// For buildx-style format (e.g., "type=registry,ref=..."), it replaces only the ref= value.
// For simple format (e.g., "myimage:latest"), it replaces the entire string.
func adjustCacheEntry(cache, imageName, artifactTag string) (string, error) {
imageRef, err := extractImageReference(cache)
if err != nil {
return "", err
}

// If the image reference doesn't match, no adjustment needed
if imageRef != imageName {
return cache, nil
}

// Parse the cache entry to determine if it's buildx-style or simple format
// Note: csvvalue.Fields should not fail here since extractImageReference already validated the format
fields, err := csvvalue.Fields(cache, nil)
if err != nil {
return "", err
}

// Simple format: just the image reference
if len(fields) == 1 && !strings.Contains(fields[0], "=") {
return artifactTag, nil
}

// Buildx-style format: reconstruct with updated ref
// Note: All fields are guaranteed to be in key=value format since extractImageReference already validated them
adjustedFields := make([]string, 0, len(fields))
for _, field := range fields {
parts := strings.SplitN(field, "=", 2)
// len(parts) is guaranteed to be 2 here since extractImageReference already validated the format
key := strings.ToLower(parts[0])
if key == "ref" {
adjustedFields = append(adjustedFields, "ref="+artifactTag)
} else {
adjustedFields = append(adjustedFields, field)
}
}

return strings.Join(adjustedFields, ","), nil
}
132 changes: 132 additions & 0 deletions pkg/skaffold/build/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,32 @@ func TestDockerCLICheckCacheFromArgs(t *testing.T) {
tag: "gcr.io/k8s-skaffold/test:tagged",
expectedCacheFrom: []string{"gcr.io/k8s-skaffold/test:tagged"},
},
{
description: "buildx-style cache-from with type=registry",
artifact: &latest.Artifact{
ImageName: "gcr.io/k8s-skaffold/test",
ArtifactType: latest.ArtifactType{
DockerArtifact: &latest.DockerArtifact{
CacheFrom: []string{"type=registry,ref=gcr.io/k8s-skaffold/test,mode=max"},
},
},
},
tag: "gcr.io/k8s-skaffold/test:tagged",
expectedCacheFrom: []string{"type=registry,ref=gcr.io/k8s-skaffold/test:tagged,mode=max"},
},
{
description: "buildx-style cache-from with type=local should not be adjusted",
artifact: &latest.Artifact{
ImageName: "gcr.io/k8s-skaffold/test",
ArtifactType: latest.ArtifactType{
DockerArtifact: &latest.DockerArtifact{
CacheFrom: []string{"type=local,src=/tmp/cache"},
},
},
},
tag: "gcr.io/k8s-skaffold/test:tagged",
expectedCacheFrom: []string{"type=local,src=/tmp/cache"},
},
}

for _, test := range tests {
Expand Down Expand Up @@ -333,3 +359,109 @@ func (m mockConfig) Prune() bool {
func (m mockConfig) ContainerDebugging() bool {
return false
}

func TestExtractImageReference(t *testing.T) {
tests := []struct {
description string
cache string
expected string
shouldError bool
}{
{
description: "simple image reference",
cache: "myimage:latest",
expected: "myimage:latest",
},
{
description: "buildx-style type=registry",
cache: "type=registry,ref=myimage:latest",
expected: "myimage:latest",
},
{
description: "buildx-style type=local",
cache: "type=local,src=/tmp/cache",
expected: "",
},
{
description: "buildx-style type=registry without ref",
cache: "type=registry",
shouldError: true,
},
{
description: "invalid format",
cache: "type=registry,invalid",
shouldError: true,
},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
result, err := extractImageReference(test.cache)
if test.shouldError {
t.CheckError(true, err)
} else {
t.CheckNoError(err)
t.CheckDeepEqual(test.expected, result)
}
})
}
}

func TestAdjustCacheEntry(t *testing.T) {
tests := []struct {
description string
cache string
imageName string
artifactTag string
expected string
shouldError bool
}{
{
description: "simple format matches imageName",
cache: "myimage",
imageName: "myimage",
artifactTag: "myimage:tagged",
expected: "myimage:tagged",
},
{
description: "simple format doesn't match imageName",
cache: "otherimage",
imageName: "myimage",
artifactTag: "myimage:tagged",
expected: "otherimage",
},
{
description: "buildx-style type=registry matches imageName",
cache: "type=registry,ref=myimage",
imageName: "myimage",
artifactTag: "myimage:tagged",
expected: "type=registry,ref=myimage:tagged",
},
{
description: "buildx-style type=registry doesn't match imageName",
cache: "type=registry,ref=otherimage,mode=max",
imageName: "myimage",
artifactTag: "myimage:tagged",
expected: "type=registry,ref=otherimage,mode=max",
},
{
description: "buildx-style type=local should not be adjusted",
cache: "type=local,src=/tmp/cache",
imageName: "myimage",
artifactTag: "myimage:tagged",
expected: "type=local,src=/tmp/cache",
},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
result, err := adjustCacheEntry(test.cache, test.imageName, test.artifactTag)
if test.shouldError {
t.CheckError(true, err)
} else {
t.CheckNoError(err)
t.CheckDeepEqual(test.expected, result)
}
})
}
}
4 changes: 4 additions & 0 deletions pkg/skaffold/docker/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,10 @@ func ToCLIBuildArgs(a *latest.DockerArtifact, evaluatedArgs map[string]*string,
args = append(args, "--cache-from", from)
}

for _, to := range a.CacheTo {
args = append(args, "--cache-to", to)
}

for _, cliFlag := range a.CliFlags {
cliFlag, err := util.ExpandEnvTemplate(cliFlag, env)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions pkg/skaffold/docker/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,13 @@ func TestGetBuildArgs(t *testing.T) {
},
want: []string{"--cache-from", "gcr.io/foo/bar", "--cache-from", "baz:latest"},
},
{
description: "cache to",
artifact: &latest.DockerArtifact{
CacheTo: []string{"type=registry,ref=gcr.io/foo/cache", "type=local,dest=/tmp/cache"},
},
want: []string{"--cache-to", "type=registry,ref=gcr.io/foo/cache", "--cache-to", "type=local,dest=/tmp/cache"},
},
{
description: "additional CLI flags",
artifact: &latest.DockerArtifact{
Expand Down
10 changes: 8 additions & 2 deletions pkg/skaffold/schema/latest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1600,10 +1600,16 @@ type DockerArtifact struct {
// For example: `["host1:ip1", "host2:ip2"]`.
AddHost []string `yaml:"addHost,omitempty"`

// CacheFrom lists the Docker images used as cache sources.
// For example: `["golang:1.10.1-alpine3.7", "alpine:3.7"]`.
// CacheFrom is used to pass in --cache-from to docker build to use images or external cache sources.
// Supports both simple image references and buildx cache importer formats.
// For example: `["golang:1.10.1-alpine3.7", "type=registry,ref=alpine:3.7", "type=local,src=/tmp/cache"]`.
CacheFrom []string `yaml:"cacheFrom,omitempty"`

// CacheTo is used to pass in --cache-to to docker build to export build cache for future builds.
// Supports buildx cache exporter formats.
// For example: `["type=registry,mode=max,ref=myregistry.com/myapp/cache", "type=local,dest=/tmp/cache"]`.
CacheTo []string `yaml:"cacheTo,omitempty"`

// CliFlags are any additional flags to pass to the local daemon during a build.
// These flags are only used during a build through the Docker CLI.
CliFlags []string `yaml:"cliFlags,omitempty"`
Expand Down