From fa93f3e09c27ed5daffb096711961f69f06388c5 Mon Sep 17 00:00:00 2001 From: Ryo Kitagawa Date: Sat, 23 Nov 2024 17:28:14 +0900 Subject: [PATCH] feat: support TemplateField for build.artifacts.docker.cliFlags --- pkg/skaffold/build/docker/docker.go | 2 +- pkg/skaffold/build/docker/docker_test.go | 38 ++++++++++++++++-------- pkg/skaffold/build/gcb/docker.go | 2 +- pkg/skaffold/docker/image.go | 10 +++++-- pkg/skaffold/docker/image_test.go | 10 ++++++- 5 files changed, 45 insertions(+), 17 deletions(-) diff --git a/pkg/skaffold/build/docker/docker.go b/pkg/skaffold/build/docker/docker.go index 8eaa38858c5..53166e303a3 100644 --- a/pkg/skaffold/build/docker/docker.go +++ b/pkg/skaffold/build/docker/docker.go @@ -106,7 +106,7 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string if err != nil { return "", fmt.Errorf("unable to evaluate build args: %w", err) } - cliArgs, err := docker.ToCLIBuildArgs(a, ba) + cliArgs, err := docker.ToCLIBuildArgs(a, ba, imageInfoEnv) if err != nil { return "", fmt.Errorf("getting docker build args: %w", err) } diff --git a/pkg/skaffold/build/docker/docker_test.go b/pkg/skaffold/build/docker/docker_test.go index 11364c43d95..509e3a1804f 100644 --- a/pkg/skaffold/build/docker/docker_test.go +++ b/pkg/skaffold/build/docker/docker_test.go @@ -45,6 +45,7 @@ func TestDockerCLIBuild(t *testing.T) { cliFlags []string // CLI flags to pass through to command. cfg mockConfig extraEnv []string + imageName string err error expectedEnv []string expectedCLIFlags []string // CLI flags expected to be autogenerated. @@ -79,11 +80,20 @@ func TestDockerCLIBuild(t *testing.T) { expectedEnv: []string{"KEY=VALUE", "DOCKER_BUILDKIT=1"}, }, { - description: "cliFlags", - cliFlags: []string{"--platform", "linux/amd64"}, - localBuild: latest.LocalBuild{}, - wantDockerCLI: true, - expectedEnv: []string{"KEY=VALUE"}, + description: "cliFlags", + cliFlags: []string{"--platform", "linux/amd64"}, + localBuild: latest.LocalBuild{}, + wantDockerCLI: true, + expectedCLIFlags: []string{"--platform", "linux/amd64"}, + expectedEnv: []string{"KEY=VALUE"}, + }, + { + description: "cliFlags replace template", + imageName: "docker.io/library/image:tag", + cliFlags: []string{"--cache-to=type=registry,ref={{ .IMAGE_REPO }}/cache-image:cache"}, + localBuild: latest.LocalBuild{}, + wantDockerCLI: true, + expectedCLIFlags: []string{"--cache-to=type=registry,ref=docker.io/library/cache-image:cache"}, }, { description: "buildkit and extra env", @@ -151,23 +161,26 @@ func TestDockerCLIBuild(t *testing.T) { t.Override(&docker.DefaultAuthHelper, stubAuth{}) var mockCmd *testutil.FakeCmd + imageName := "tag" + if test.imageName != "" { + imageName = test.imageName + } + if test.err != nil { var pruneFlag string if test.cfg.Prune() { pruneFlag = " --force-rm" } mockCmd = testutil.CmdRunErr( - "docker build . --file "+dockerfilePath+" -t tag"+pruneFlag, + "docker build . --file "+dockerfilePath+" -t "+imageName+pruneFlag, test.err, ) t.Override(&util.DefaultExecCommand, mockCmd) } if test.wantDockerCLI { - cmdLine := "docker build . --file " + dockerfilePath + " -t tag" - if len(test.cliFlags) > 0 || len(test.expectedCLIFlags) > 0 { - flags := append(test.cliFlags, test.expectedCLIFlags...) - - cmdLine += " " + strings.Join(flags, " ") + cmdLine := "docker build . --file " + dockerfilePath + " -t " + imageName + if len(test.expectedCLIFlags) > 0 { + cmdLine += " " + strings.Join(test.expectedCLIFlags, " ") } mockCmd = testutil.CmdRunEnv(cmdLine, test.expectedEnv) t.Override(&util.DefaultExecCommand, mockCmd) @@ -186,7 +199,7 @@ func TestDockerCLIBuild(t *testing.T) { }, } - _, err := builder.Build(context.Background(), io.Discard, artifact, "tag", platform.Matcher{}) + _, err := builder.Build(context.Background(), io.Discard, artifact, imageName, platform.Matcher{}) t.CheckError(test.err != nil, err) if mockCmd != nil { t.CheckDeepEqual(1, mockCmd.TimesCalled()) @@ -283,6 +296,7 @@ type stubAuth struct{} func (t stubAuth) GetAuthConfig(context.Context, string) (registry.AuthConfig, error) { return registry.AuthConfig{}, nil } + func (t stubAuth) GetAllAuthConfigs(context.Context) (map[string]registry.AuthConfig, error) { return nil, nil } diff --git a/pkg/skaffold/build/gcb/docker.go b/pkg/skaffold/build/gcb/docker.go index 3f41563299e..b23431cac4b 100644 --- a/pkg/skaffold/build/gcb/docker.go +++ b/pkg/skaffold/build/gcb/docker.go @@ -108,7 +108,7 @@ func (b *Builder) dockerBuildArgs(a *latest.Artifact, tag string, deps []*latest return nil, fmt.Errorf("unable to evaluate build args: %w", err) } - ba, err := docker.ToCLIBuildArgs(d, buildArgs) + ba, err := docker.ToCLIBuildArgs(d, buildArgs, nil) if err != nil { return nil, fmt.Errorf("getting docker build args: %w", err) } diff --git a/pkg/skaffold/docker/image.go b/pkg/skaffold/docker/image.go index 1d68f41996d..02455ea3495 100644 --- a/pkg/skaffold/docker/image.go +++ b/pkg/skaffold/docker/image.go @@ -626,7 +626,7 @@ func (l *localDaemon) DiskUsage(ctx context.Context) (uint64, error) { return uint64(usage.LayersSize), nil } -func ToCLIBuildArgs(a *latest.DockerArtifact, evaluatedArgs map[string]*string) ([]string, error) { +func ToCLIBuildArgs(a *latest.DockerArtifact, evaluatedArgs map[string]*string, env map[string]string) ([]string, error) { var args []string var keys []string for k := range evaluatedArgs { @@ -653,7 +653,13 @@ func ToCLIBuildArgs(a *latest.DockerArtifact, evaluatedArgs map[string]*string) args = append(args, "--cache-from", from) } - args = append(args, a.CliFlags...) + for _, cliFlag := range a.CliFlags { + cliFlag, err := util.ExpandEnvTemplate(cliFlag, env) + if err != nil { + return nil, fmt.Errorf("unable to evaluate cli flags: %w", err) + } + args = append(args, cliFlag) + } if a.Target != "" { args = append(args, "--target", a.Target) diff --git a/pkg/skaffold/docker/image_test.go b/pkg/skaffold/docker/image_test.go index 5c84e9d1131..322e9fb419d 100644 --- a/pkg/skaffold/docker/image_test.go +++ b/pkg/skaffold/docker/image_test.go @@ -318,6 +318,14 @@ func TestGetBuildArgs(t *testing.T) { }, want: []string{"--foo", "--bar"}, }, + { + description: "expand env for CLI flags", + artifact: &latest.DockerArtifact{ + CliFlags: []string{"--cache-to=type=registry,ref={{ .IMAGE_REPO }}/cache-image:cache"}, + }, + env: []string{"IMAGE_REPO=docker.io/library"}, + want: []string{"--cache-to=type=registry,ref=docker.io/library/cache-image:cache"}, + }, { description: "target", artifact: &latest.DockerArtifact{ @@ -430,7 +438,7 @@ func TestGetBuildArgs(t *testing.T) { return } - result, err := ToCLIBuildArgs(test.artifact, args) + result, err := ToCLIBuildArgs(test.artifact, args, nil) t.CheckError(test.shouldErr, err) if !test.shouldErr {