diff --git a/cmd/skaffold/app/cmd/diagnose.go b/cmd/skaffold/app/cmd/diagnose.go index 05437b1622c..c99dc23c536 100644 --- a/cmd/skaffold/app/cmd/diagnose.go +++ b/cmd/skaffold/app/cmd/diagnose.go @@ -24,12 +24,14 @@ import ( "github.com/spf13/cobra" + deployutil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/deploy/util" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/diagnose" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/parser" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" schemaUtil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/util" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/tag" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/tags" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/version" @@ -106,13 +108,20 @@ func printArtifactDiagnostics(ctx context.Context, out io.Writer, configs []sche if err != nil { return fmt.Errorf("getting run context: %w", err) } + tagger, err := tag.NewTaggerMux(runCtx) + if err != nil { + return fmt.Errorf("getting tagger: %w", err) + } + imageTags, err := deployutil.ImageTags(ctx, runCtx, tagger, out, runCtx.Artifacts()) + if err != nil { + return fmt.Errorf("getting tags: %w", err) + } for _, c := range configs { config := c.(*latest.SkaffoldConfig) fmt.Fprintln(out, "Skaffold version:", version.Get().GitCommit) fmt.Fprintln(out, "Configuration version:", config.APIVersion) fmt.Fprintln(out, "Number of artifacts:", len(config.Build.Artifacts)) - - if err := diagnose.CheckArtifacts(ctx, runCtx, out); err != nil { + if err := diagnose.CheckArtifacts(ctx, runCtx, imageTags, out); err != nil { return fmt.Errorf("running diagnostic on artifacts: %w", err) } diff --git a/hack/boilerplate/boilerplate.py b/hack/boilerplate/boilerplate.py index 22af9f0ce06..a949ea6c975 100644 --- a/hack/boilerplate/boilerplate.py +++ b/hack/boilerplate/boilerplate.py @@ -145,7 +145,7 @@ def get_regexs(): # Search for "YEAR" which exists in the boilerplate, but shouldn't in the real thing regexs["year"] = re.compile( 'YEAR' ) # dates can be 2018, company holder names can be anything - regexs["date"] = re.compile( '(2019|2020|2021|2022|2023|2024)' ) + regexs["date"] = re.compile( '(2019|2020|2021|2022|2023|2024|2025)' ) # strip // +build \n\n build constraints regexs["go_build_constraints"] = re.compile(r"^(// \+build.*\n)+\n", re.MULTILINE) # strip //go:build \n build constraints (for go1.17 and higher) diff --git a/integration/build_test.go b/integration/build_test.go index eb46951049f..461454fec80 100644 --- a/integration/build_test.go +++ b/integration/build_test.go @@ -28,6 +28,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/opencontainers/image-spec/specs-go/v1" + "k8s.io/apimachinery/pkg/util/wait" "github.com/GoogleContainerTools/skaffold/v2/cmd/skaffold/app/flags" "github.com/GoogleContainerTools/skaffold/v2/integration/skaffold" @@ -331,3 +332,51 @@ func failNowIfError(t Fataler, err error) { t.Fatal(err) } } + +func TestRunWithDockerAndBuildArgs(t *testing.T) { + tests := []struct { + description string + projectDir string + skaffoldArgs []string + dockerRunArgs []string + wantOutput string + }{ + { + description: "IMAGE_REPO, IMAGE_TAG, and IMAGE_NAME are passed to Docker build as build args", + projectDir: "testdata/docker-run-with-build-args/artifact-with-dependency", + skaffoldArgs: []string{"--kube-context", "default"}, + dockerRunArgs: []string{"run", "child:latest"}, + wantOutput: "IMAGE_REPO: gcr.io/k8s-skaffold, IMAGE_NAME: skaffold, IMAGE_TAG:latest", + }, + { + description: "IMAGE_TAG can be used as a part of a filename in the Dockerfile", + projectDir: "testdata/docker-run-with-build-args/single-artifact", + skaffoldArgs: []string{"--kube-context", "default"}, + dockerRunArgs: []string{"run", "example:latest"}, + wantOutput: "HELLO WORLD", + }, + } + + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + defer skaffold.Delete().InDir(test.projectDir).Run(t.T) + if err := skaffold.Build(test.skaffoldArgs...).InDir(test.projectDir).Run(t.T); err != nil { + t.Errorf("skaffold build args: %v working directory:%s returned unexpected error: %v", test.skaffoldArgs, test.projectDir, err) + } + + got := "" + + err := wait.PollImmediate(time.Millisecond*500, 1*time.Minute, func() (bool, error) { + out, _ := exec.Command("docker", test.dockerRunArgs...).Output() + t.Logf("Output:[%s]\n", out) + got = strings.Trim(string(out), " \n") + return got == test.wantOutput, nil + }) + + if err != nil { + t.Errorf("docker run produced incorrect output, got:[%s], want:[%s], err: %v", got, test.wantOutput, err) + } + failNowIfError(t, err) + }) + } +} diff --git a/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/Dockerfile b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/Dockerfile new file mode 100644 index 00000000000..24eb993a214 --- /dev/null +++ b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/Dockerfile @@ -0,0 +1,11 @@ +FROM golang:1.23 as builder +WORKDIR /code +COPY main.go . +COPY go.mod . +# `skaffold debug` sets SKAFFOLD_GO_GCFLAGS to disable compiler optimizations +ARG SKAFFOLD_GO_GCFLAGS +ARG IMAGE_REPO +ARG IMAGE_NAME +ARG IMAGE_TAG +RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -ldflags="-X main.ImageRepo=${IMAGE_REPO} -X main.ImageName=${IMAGE_NAME} -X main.ImageTag=${IMAGE_TAG}" -trimpath -o /app main.go + diff --git a/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/go.mod b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/go.mod new file mode 100644 index 00000000000..447daad7f1f --- /dev/null +++ b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/go.mod @@ -0,0 +1,3 @@ +module github.com/GoogleContainerTools/skaffold/examples/docker-deploy-with-build-args/base + +go 1.23 diff --git a/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/main.go b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/main.go new file mode 100644 index 00000000000..6cd9c0ba221 --- /dev/null +++ b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/base/main.go @@ -0,0 +1,14 @@ +package main + +import ( + "fmt" +) + +var ImageRepo = "unknown" +var ImageTag = "unknown" +var ImageName = "unknown" + +func main() { + output := fmt.Sprintf("IMAGE_REPO: %s, IMAGE_NAME: %s, IMAGE_TAG:%s\n", ImageRepo, ImageName, ImageTag) + fmt.Println(output) +} diff --git a/integration/testdata/docker-run-with-build-args/artifact-with-dependency/child/Dockerfile b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/child/Dockerfile new file mode 100644 index 00000000000..0c931200956 --- /dev/null +++ b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/child/Dockerfile @@ -0,0 +1,5 @@ +ARG BASE +FROM $BASE as parent +FROM alpine +COPY --from=parent /app . +CMD ["./app"] diff --git a/integration/testdata/docker-run-with-build-args/artifact-with-dependency/skaffold.yaml b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/skaffold.yaml new file mode 100644 index 00000000000..a9a79bb4ec5 --- /dev/null +++ b/integration/testdata/docker-run-with-build-args/artifact-with-dependency/skaffold.yaml @@ -0,0 +1,29 @@ +apiVersion: skaffold/v4beta12 +kind: Config +build: + tagPolicy: + sha256: {} + local: + push: false + useDockerCLI: true + artifacts: + - image: gcr.io/k8s-skaffold/skaffold + context: base + docker: + dockerfile: Dockerfile + noCache: true + buildArgs: + IMAGE_REPO: '{{.IMAGE_REPO}}' + IMAGE_NAME: '{{.IMAGE_NAME}}' + IMAGE_TAG: '{{.IMAGE_TAG}}' + - image: child + requires: + - image: gcr.io/k8s-skaffold/skaffold + alias: BASE + context: child + docker: + dockerfile: Dockerfile + noCache: true +deploy: + docker: + images: [child] diff --git a/integration/testdata/docker-run-with-build-args/single-artifact/Dockerfile b/integration/testdata/docker-run-with-build-args/single-artifact/Dockerfile new file mode 100644 index 00000000000..7daf2952870 --- /dev/null +++ b/integration/testdata/docker-run-with-build-args/single-artifact/Dockerfile @@ -0,0 +1,5 @@ +FROM busybox +ARG IMAGE_TAG +RUN echo $IMAGE_TAG +COPY "script-${IMAGE_TAG}.sh" script.sh +CMD ["/bin/sh","script.sh"] diff --git a/integration/testdata/docker-run-with-build-args/single-artifact/script-latest.sh b/integration/testdata/docker-run-with-build-args/single-artifact/script-latest.sh new file mode 100755 index 00000000000..dfc0eb82378 --- /dev/null +++ b/integration/testdata/docker-run-with-build-args/single-artifact/script-latest.sh @@ -0,0 +1 @@ +echo "HELLO WORLD" \ No newline at end of file diff --git a/integration/testdata/docker-run-with-build-args/single-artifact/skaffold.yaml b/integration/testdata/docker-run-with-build-args/single-artifact/skaffold.yaml new file mode 100644 index 00000000000..20ee43ce74a --- /dev/null +++ b/integration/testdata/docker-run-with-build-args/single-artifact/skaffold.yaml @@ -0,0 +1,18 @@ +apiVersion: skaffold/v4beta12 +kind: Config +build: + tagPolicy: + sha256: {} + local: + push: false + useDockerCLI: true + artifacts: + - image: example + docker: + dockerfile: Dockerfile + noCache: true + buildArgs: + IMAGE_TAG: '{{.IMAGE_TAG}}' +deploy: + docker: + images: [example] diff --git a/pkg/skaffold/build/cache/cache.go b/pkg/skaffold/build/cache/cache.go index 371ffb104f2..ca32e87ccbc 100644 --- a/pkg/skaffold/build/cache/cache.go +++ b/pkg/skaffold/build/cache/cache.go @@ -61,7 +61,7 @@ type cache struct { } // DependencyLister fetches a list of dependencies for an artifact -type DependencyLister func(ctx context.Context, artifact *latest.Artifact) ([]string, error) +type DependencyLister func(ctx context.Context, artifact *latest.Artifact, tag string) ([]string, error) type Config interface { docker.Config diff --git a/pkg/skaffold/build/cache/hash.go b/pkg/skaffold/build/cache/hash.go index 02d80202ab9..e103fdd2463 100644 --- a/pkg/skaffold/build/cache/hash.go +++ b/pkg/skaffold/build/cache/hash.go @@ -47,7 +47,7 @@ var ( ) type artifactHasher interface { - hash(context.Context, io.Writer, *latest.Artifact, platform.Resolver) (string, error) + hash(context.Context, io.Writer, *latest.Artifact, platform.Resolver, string) (string, error) } type artifactHasherImpl struct { @@ -67,20 +67,20 @@ func newArtifactHasher(artifacts graph.ArtifactGraph, lister DependencyLister, m } } -func (h *artifactHasherImpl) hash(ctx context.Context, out io.Writer, a *latest.Artifact, platforms platform.Resolver) (string, error) { +func (h *artifactHasherImpl) hash(ctx context.Context, out io.Writer, a *latest.Artifact, platforms platform.Resolver, tag string) (string, error) { ctx, endTrace := instrumentation.StartTrace(ctx, "hash_GenerateHashOneArtifact", map[string]string{ "ImageName": instrumentation.PII(a.ImageName), }) defer endTrace() - hash, err := h.safeHash(ctx, out, a, platforms.GetPlatforms(a.ImageName)) + hash, err := h.safeHash(ctx, out, a, platforms.GetPlatforms(a.ImageName), tag) if err != nil { endTrace(instrumentation.TraceEndError(err)) return "", err } hashes := []string{hash} for _, dep := range sortedDependencies(a, h.artifacts) { - depHash, err := h.hash(ctx, out, dep, platforms) + depHash, err := h.hash(ctx, out, dep, platforms, tag) if err != nil { endTrace(instrumentation.TraceEndError(err)) return "", err @@ -94,15 +94,15 @@ func (h *artifactHasherImpl) hash(ctx context.Context, out io.Writer, a *latest. return encode(hashes) } -func (h *artifactHasherImpl) safeHash(ctx context.Context, out io.Writer, a *latest.Artifact, platforms platform.Matcher) (string, error) { +func (h *artifactHasherImpl) safeHash(ctx context.Context, out io.Writer, a *latest.Artifact, platforms platform.Matcher, tag string) (string, error) { return h.syncStore.Exec(a.ImageName, func() (string, error) { - return singleArtifactHash(ctx, out, h.lister, a, h.mode, platforms) + return singleArtifactHash(ctx, out, h.lister, a, h.mode, platforms, tag) }) } // singleArtifactHash calculates the hash for a single artifact, and ignores its required artifacts. -func singleArtifactHash(ctx context.Context, out io.Writer, depLister DependencyLister, a *latest.Artifact, mode config.RunMode, m platform.Matcher) (string, error) { +func singleArtifactHash(ctx context.Context, out io.Writer, depLister DependencyLister, a *latest.Artifact, mode config.RunMode, m platform.Matcher, tag string) (string, error) { var inputs []string // Append the artifact's configuration @@ -113,7 +113,7 @@ func singleArtifactHash(ctx context.Context, out io.Writer, depLister Dependency inputs = append(inputs, config) // Append the digest of each input file - deps, err := depLister(ctx, a) + deps, err := depLister(ctx, a, tag) if err != nil { return "", fmt.Errorf("getting dependencies for %q: %w", a.ImageName, err) } @@ -133,7 +133,7 @@ func singleArtifactHash(ctx context.Context, out io.Writer, depLister Dependency } // add build args for the artifact if specified - args, err := hashBuildArgs(out, a, mode) + args, err := hashBuildArgs(out, a, tag, mode) if err != nil { return "", fmt.Errorf("hashing build args: %w", err) } @@ -171,16 +171,22 @@ func artifactConfig(a *latest.Artifact) (string, error) { return string(buf), nil } -func hashBuildArgs(out io.Writer, artifact *latest.Artifact, mode config.RunMode) ([]string, error) { +func hashBuildArgs(out io.Writer, artifact *latest.Artifact, tag string, mode config.RunMode) ([]string, error) { // only one of args or env is ever populated var args map[string]*string var env map[string]string var err error + + envTags, evalErr := docker.EnvTags(tag) + if evalErr != nil { + return nil, fmt.Errorf("unable to create build args: %w", err) + } + switch { case artifact.DockerArtifact != nil: - args, err = docker.EvalBuildArgs(mode, artifact.Workspace, artifact.DockerArtifact.DockerfilePath, artifact.DockerArtifact.BuildArgs, nil) + args, err = docker.EvalBuildArgsWithEnv(mode, artifact.Workspace, artifact.DockerArtifact.DockerfilePath, artifact.DockerArtifact.BuildArgs, nil, envTags) case artifact.KanikoArtifact != nil: - args, err = docker.EvalBuildArgs(mode, kaniko.GetContext(artifact.KanikoArtifact, artifact.Workspace), artifact.KanikoArtifact.DockerfilePath, artifact.KanikoArtifact.BuildArgs, nil) + args, err = docker.EvalBuildArgsWithEnv(mode, kaniko.GetContext(artifact.KanikoArtifact, artifact.Workspace), artifact.KanikoArtifact.DockerfilePath, artifact.KanikoArtifact.BuildArgs, nil, envTags) case artifact.BuildpackArtifact != nil: env, err = buildpacks.GetEnv(out, artifact, mode) case artifact.CustomArtifact != nil && artifact.CustomArtifact.Dependencies.Dockerfile != nil: diff --git a/pkg/skaffold/build/cache/hash_test.go b/pkg/skaffold/build/cache/hash_test.go index f4c7099f522..75c3b6ad654 100644 --- a/pkg/skaffold/build/cache/hash_test.go +++ b/pkg/skaffold/build/cache/hash_test.go @@ -31,7 +31,7 @@ import ( ) func stubDependencyLister(dependencies []string) DependencyLister { - return func(context.Context, *latest.Artifact) ([]string, error) { + return func(context.Context, *latest.Artifact, string) ([]string, error) { return dependencies, nil } } @@ -50,6 +50,8 @@ var fakeArtifactConfig = func(a *latest.Artifact) (string, error) { return "", nil } +var testTag = "gcr.io/k8s-skaffold/skaffold:latest" + const Dockerfile = "Dockerfile" func TestGetHashForArtifact(t *testing.T) { @@ -162,7 +164,7 @@ func TestGetHashForArtifact(t *testing.T) { } depLister := stubDependencyLister(test.dependencies) - actual, err := newArtifactHasher(nil, depLister, test.mode).hash(context.Background(), io.Discard, test.artifact, test.platforms) + actual, err := newArtifactHasher(nil, depLister, test.mode).hash(context.Background(), io.Discard, test.artifact, test.platforms, testTag) t.CheckNoError(err) t.CheckDeepEqual(test.expected, actual) @@ -242,11 +244,11 @@ func TestGetHashForArtifactWithDependencies(t *testing.T) { } } - depLister := func(_ context.Context, a *latest.Artifact) ([]string, error) { + depLister := func(_ context.Context, a *latest.Artifact, tag string) ([]string, error) { return test.fileDeps[a.ImageName], nil } - actual, err := newArtifactHasher(g, depLister, test.mode).hash(context.Background(), io.Discard, test.artifacts[0], platform.Resolver{}) + actual, err := newArtifactHasher(g, depLister, test.mode).hash(context.Background(), io.Discard, test.artifacts[0], platform.Resolver{}, testTag) t.CheckNoError(err) t.CheckDeepEqual(test.expected, actual) @@ -309,21 +311,21 @@ func TestBuildArgs(t *testing.T) { } t.Override(&fileHasherFunc, mockCacheHasher) t.Override(&artifactConfigFunc, fakeArtifactConfig) - actual, err := newArtifactHasher(nil, stubDependencyLister(nil), test.mode).hash(context.Background(), io.Discard, artifact, platform.Resolver{}) + actual, err := newArtifactHasher(nil, stubDependencyLister(nil), test.mode).hash(context.Background(), io.Discard, artifact, platform.Resolver{}, testTag) t.CheckNoError(err) t.CheckDeepEqual(test.expected, actual) // Change order of buildargs artifact.ArtifactType.DockerArtifact.BuildArgs = map[string]*string{"two": util.Ptr("2"), "one": util.Ptr("1")} - actual, err = newArtifactHasher(nil, stubDependencyLister(nil), test.mode).hash(context.Background(), io.Discard, artifact, platform.Resolver{}) + actual, err = newArtifactHasher(nil, stubDependencyLister(nil), test.mode).hash(context.Background(), io.Discard, artifact, platform.Resolver{}, testTag) t.CheckNoError(err) t.CheckDeepEqual(test.expected, actual) // Change build args, get different hash artifact.ArtifactType.DockerArtifact.BuildArgs = map[string]*string{"one": util.Ptr("1")} - actual, err = newArtifactHasher(nil, stubDependencyLister(nil), test.mode).hash(context.Background(), io.Discard, artifact, platform.Resolver{}) + actual, err = newArtifactHasher(nil, stubDependencyLister(nil), test.mode).hash(context.Background(), io.Discard, artifact, platform.Resolver{}, testTag) t.CheckNoError(err) if actual == test.expected { @@ -356,7 +358,7 @@ func TestBuildArgsEnvSubstitution(t *testing.T) { t.Override(&artifactConfigFunc, fakeArtifactConfig) depLister := stubDependencyLister([]string{"graph"}) - hash1, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, artifact, platform.Resolver{}) + hash1, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, artifact, platform.Resolver{}, testTag) t.CheckNoError(err) @@ -366,7 +368,7 @@ func TestBuildArgsEnvSubstitution(t *testing.T) { return []string{"FOO=baz"} } - hash2, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, artifact, platform.Resolver{}) + hash2, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, artifact, platform.Resolver{}, testTag) t.CheckNoError(err) if hash1 == hash2 { @@ -375,6 +377,78 @@ func TestBuildArgsEnvSubstitution(t *testing.T) { }) } +func TestBuildArgsImageInfoSubstitution(t *testing.T) { + tests := []struct { + description string + artifactType *latest.Artifact + originalTag string + newTag string + }{ + { + description: "hash differs if image repo changes", + artifactType: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + BuildArgs: map[string]*string{"IMAGE_REPO": util.Ptr("${{.IMAGE_REPO}}")}, + DockerfilePath: Dockerfile, + }, + }, + }, + originalTag: "gcr.io/k8s-skaffold/skaffold:latest", + newTag: "us.gcr.io/k8s-skaffold/skaffold:latest", + }, + { + description: "hash differs if image name changes", + artifactType: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + BuildArgs: map[string]*string{"IMAGE_NAME": util.Ptr("${{.IMAGE_NAME}}")}, + DockerfilePath: Dockerfile, + }, + }, + }, + originalTag: "gcr.io/k8s-skaffold/skaffold:latest", + newTag: "gcr.io/k8s-skaffold/new-skaffold:latest", + }, + { + description: "hash differs if image tag changes", + artifactType: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + BuildArgs: map[string]*string{"IMAGE_TAG": util.Ptr("${{.IMAGE_TAG}}")}, + DockerfilePath: Dockerfile, + }, + }, + }, + originalTag: "gcr.io/k8s-skaffold/skaffold:v1", + newTag: "gcr.io/k8s-skaffold/skaffold:v2", + }, + } + + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + tmpDir := t.NewTempDir() + tmpDir.Write("./Dockerfile", "ARG SKAFFOLD_GO_GCFLAGS\nFROM foo") + test.artifactType.Workspace = tmpDir.Path(".") + + t.Override(&fileHasherFunc, mockCacheHasher) + t.Override(&artifactConfigFunc, fakeArtifactConfig) + + depLister := stubDependencyLister([]string{"graph"}) + hash1, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, test.artifactType, platform.Resolver{}, test.originalTag) + + t.CheckNoError(err) + + hash2, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, test.artifactType, platform.Resolver{}, test.newTag) + + t.CheckNoError(err) + if hash1 == hash2 { + t.Fatal("hashes are the same even though build arg changed") + } + }) + } +} + func TestCacheHasher(t *testing.T) { tests := []struct { description string @@ -423,7 +497,7 @@ func TestCacheHasher(t *testing.T) { path := originalFile depLister := stubDependencyLister([]string{tmpDir.Path(originalFile)}) - oldHash, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, &latest.Artifact{}, platform.Resolver{}) + oldHash, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, &latest.Artifact{}, platform.Resolver{}, testTag) t.CheckNoError(err) test.update(originalFile, tmpDir) @@ -432,7 +506,7 @@ func TestCacheHasher(t *testing.T) { } depLister = stubDependencyLister([]string{tmpDir.Path(path)}) - newHash, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, &latest.Artifact{}, platform.Resolver{}) + newHash, err := newArtifactHasher(nil, depLister, config.RunModes.Build).hash(context.Background(), io.Discard, &latest.Artifact{}, platform.Resolver{}, testTag) t.CheckNoError(err) t.CheckFalse(test.differentHash && oldHash == newHash) @@ -445,6 +519,7 @@ func TestHashBuildArgs(t *testing.T) { tests := []struct { description string artifactType latest.ArtifactType + tag string expected []string mode config.RunMode }{ @@ -547,6 +622,33 @@ func TestHashBuildArgs(t *testing.T) { }, }, }, + { + description: "docker artifact with image info build args", + artifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + BuildArgs: map[string]*string{ + "IMAGE_REPO": util.Ptr("{{.IMAGE_REPO}}"), + "IMAGE_NAME": util.Ptr("{{.IMAGE_NAME}}"), + "IMAGE_TAG": util.Ptr("{{.IMAGE_TAG}}"), + }, + }, + }, + mode: config.RunModes.Debug, + expected: []string{"SKAFFOLD_GO_GCFLAGS=all=-N -l", "IMAGE_REPO=gcr.io/k8s-skaffold", "IMAGE_NAME=skaffold", "IMAGE_TAG=latest"}, + }, + { + description: "the tag segment has 'latest' as default if no tag segment was provided", + tag: "busybox", + artifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + BuildArgs: map[string]*string{ + "IMAGE_TAG": util.Ptr("{{.IMAGE_TAG}}"), + }, + }, + }, + mode: config.RunModes.Debug, + expected: []string{"SKAFFOLD_GO_GCFLAGS=all=-N -l", "IMAGE_TAG=latest"}, + }, } for _, test := range tests { @@ -566,9 +668,13 @@ func TestHashBuildArgs(t *testing.T) { a.Workspace = tmpDir.Path(".") a.ArtifactType.KanikoArtifact.DockerfilePath = Dockerfile } - actual, err := hashBuildArgs(io.Discard, a, test.mode) + + if test.tag == "" { + test.tag = testTag + } + actual, err := hashBuildArgs(io.Discard, a, testTag, test.mode) t.CheckNoError(err) - t.CheckDeepEqual(test.expected, actual) + t.CheckElementsMatch(test.expected, actual) }) } } diff --git a/pkg/skaffold/build/cache/lookup.go b/pkg/skaffold/build/cache/lookup.go index f9634dcfda7..aa84c43a70d 100644 --- a/pkg/skaffold/build/cache/lookup.go +++ b/pkg/skaffold/build/cache/lookup.go @@ -48,7 +48,7 @@ func (c *cache) lookupArtifacts(ctx context.Context, out io.Writer, tags tag.Ima i := i go func() { - details[i] = c.lookup(ctx, out, artifacts[i], tags[artifacts[i].ImageName], platforms, h) + details[i] = c.lookup(ctx, out, artifacts[i], tags, platforms, h) wg.Done() }() } @@ -57,13 +57,14 @@ func (c *cache) lookupArtifacts(ctx context.Context, out io.Writer, tags tag.Ima return details } -func (c *cache) lookup(ctx context.Context, out io.Writer, a *latest.Artifact, tag string, platforms platform.Resolver, h artifactHasher) cacheDetails { +func (c *cache) lookup(ctx context.Context, out io.Writer, a *latest.Artifact, tags map[string]string, platforms platform.Resolver, h artifactHasher) cacheDetails { + tag := tags[a.ImageName] ctx, endTrace := instrumentation.StartTrace(ctx, "lookup_CacheLookupOneArtifact", map[string]string{ "ImageName": instrumentation.PII(a.ImageName), }) defer endTrace() - hash, err := h.hash(ctx, out, a, platforms) + hash, err := h.hash(ctx, out, a, platforms, tag) if err != nil { return failed{err: fmt.Errorf("getting hash for artifact %q: %s", a.ImageName, err)} } diff --git a/pkg/skaffold/build/cache/lookup_test.go b/pkg/skaffold/build/cache/lookup_test.go index 455f08bb218..3e01c90d69b 100644 --- a/pkg/skaffold/build/cache/lookup_test.go +++ b/pkg/skaffold/build/cache/lookup_test.go @@ -233,7 +233,7 @@ type mockHasher struct { val string } -func (m mockHasher) hash(context.Context, io.Writer, *latest.Artifact, platform.Resolver) (string, error) { +func (m mockHasher) hash(context.Context, io.Writer, *latest.Artifact, platform.Resolver, string) (string, error) { return m.val, nil } @@ -241,7 +241,7 @@ type failingHasher struct { err error } -func (f failingHasher) hash(context.Context, io.Writer, *latest.Artifact, platform.Resolver) (string, error) { +func (f failingHasher) hash(context.Context, io.Writer, *latest.Artifact, platform.Resolver, string) (string, error) { return "", f.err } diff --git a/pkg/skaffold/build/cache/retrieve_test.go b/pkg/skaffold/build/cache/retrieve_test.go index 9a21409ba5a..7abbf80b97c 100644 --- a/pkg/skaffold/build/cache/retrieve_test.go +++ b/pkg/skaffold/build/cache/retrieve_test.go @@ -37,7 +37,7 @@ import ( ) func depLister(files map[string][]string) DependencyLister { - return func(_ context.Context, artifact *latest.Artifact) ([]string, error) { + return func(_ context.Context, artifact *latest.Artifact, tag string) ([]string, error) { list, found := files[artifact.ImageName] if !found { return nil, errors.New("unknown artifact") @@ -138,10 +138,6 @@ func TestCacheBuildLocal(t *testing.T) { return dockerDaemon, nil }) - // Mock args builder - t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) { - return args, nil - }) t.Override(&docker.EvalBuildArgsWithEnv, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string, _ map[string]string) (map[string]*string, error) { return args, nil }) @@ -238,10 +234,6 @@ func TestCacheBuildRemote(t *testing.T) { } }) - // Mock args builder - t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) { - return args, nil - }) t.Override(&docker.EvalBuildArgsWithEnv, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string, _ map[string]string) (map[string]*string, error) { return args, nil }) @@ -326,10 +318,6 @@ func TestCacheFindMissing(t *testing.T) { } }) - // Mock args builder - t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) { - return args, nil - }) t.Override(&docker.EvalBuildArgsWithEnv, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string, _ map[string]string) (map[string]*string, error) { return args, nil }) diff --git a/pkg/skaffold/build/cluster/kaniko.go b/pkg/skaffold/build/cluster/kaniko.go index 80f3690907e..703befbd979 100644 --- a/pkg/skaffold/build/cluster/kaniko.go +++ b/pkg/skaffold/build/cluster/kaniko.go @@ -269,17 +269,14 @@ func envMapFromVars(env []v1.EnvVar) map[string]string { } func generateEnvFromImage(imageStr string) ([]v1.EnvVar, error) { - imgRef, err := docker.ParseReference(imageStr) + envMap, err := docker.EnvTags(imageStr) if err != nil { - return nil, err - } - if imgRef.Tag == "" { - imgRef.Tag = "latest" + return nil, fmt.Errorf("unable to get image tags: %w", err) } var generatedEnvs []v1.EnvVar - generatedEnvs = append(generatedEnvs, v1.EnvVar{Name: "IMAGE_REPO", Value: imgRef.Repo}) - generatedEnvs = append(generatedEnvs, v1.EnvVar{Name: "IMAGE_NAME", Value: imgRef.Name}) - generatedEnvs = append(generatedEnvs, v1.EnvVar{Name: "IMAGE_TAG", Value: imgRef.Tag}) + for k, v := range envMap { + generatedEnvs = append(generatedEnvs, v1.EnvVar{Name: k, Value: v}) + } return generatedEnvs, nil } diff --git a/pkg/skaffold/build/docker/docker.go b/pkg/skaffold/build/docker/docker.go index 53166e303a3..9e9f826435d 100644 --- a/pkg/skaffold/build/docker/docker.go +++ b/pkg/skaffold/build/docker/docker.go @@ -93,15 +93,10 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact, func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string, workspace string, dockerfilePath string, a *latest.DockerArtifact, opts docker.BuildOptions, pl v1.Platform) (string, error) { args := []string{"build", workspace, "--file", dockerfilePath, "-t", opts.Tag} - imgRef, err := docker.ParseReference(opts.Tag) + imageInfoEnv, err := docker.EnvTags(opts.Tag) if err != nil { return "", fmt.Errorf("couldn't parse image tag: %w", err) } - imageInfoEnv := map[string]string{ - "IMAGE_REPO": imgRef.Repo, - "IMAGE_NAME": imgRef.Name, - "IMAGE_TAG": imgRef.Tag, - } ba, err := docker.EvalBuildArgsWithEnv(b.cfg.Mode(), workspace, a.DockerfilePath, a.BuildArgs, opts.ExtraBuildArgs, imageInfoEnv) if err != nil { return "", fmt.Errorf("unable to evaluate build args: %w", err) diff --git a/pkg/skaffold/build/gcb/cloud_build.go b/pkg/skaffold/build/gcb/cloud_build.go index 920ba600dec..2ec89efc4ab 100644 --- a/pkg/skaffold/build/gcb/cloud_build.go +++ b/pkg/skaffold/build/gcb/cloud_build.go @@ -127,7 +127,7 @@ func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer }) } - dependencies, err := b.sourceDependencies.SingleArtifactDependencies(ctx, artifact) + dependencies, err := b.sourceDependencies.SingleArtifactDependencies(ctx, artifact, tag) if err != nil { return "", sErrors.NewErrorWithStatusCode(&proto.ActionableErr{ ErrCode: proto.StatusCode_BUILD_GCB_GET_DEPENDENCY_ERR, diff --git a/pkg/skaffold/build/gcb/docker.go b/pkg/skaffold/build/gcb/docker.go index b23431cac4b..01c64634d71 100644 --- a/pkg/skaffold/build/gcb/docker.go +++ b/pkg/skaffold/build/gcb/docker.go @@ -103,7 +103,7 @@ func (b *Builder) dockerBuildArgs(a *latest.Artifact, tag string, deps []*latest return nil, errors.New("docker build options, secrets and ssh, are not currently supported in GCB builds") } requiredImages := docker.ResolveDependencyImages(deps, b.artifactStore, true) - buildArgs, err := docker.EvalBuildArgs(b.cfg.Mode(), a.Workspace, d.DockerfilePath, d.BuildArgs, requiredImages) + buildArgs, err := docker.EvalBuildArgsWithEnv(b.cfg.Mode(), a.Workspace, d.DockerfilePath, d.BuildArgs, requiredImages, nil) if err != nil { return nil, fmt.Errorf("unable to evaluate build args: %w", err) } diff --git a/pkg/skaffold/build/gcb/docker_test.go b/pkg/skaffold/build/gcb/docker_test.go index c27aab2da16..6c8d0e36049 100644 --- a/pkg/skaffold/build/gcb/docker_test.go +++ b/pkg/skaffold/build/gcb/docker_test.go @@ -175,7 +175,7 @@ func TestDockerBuildSpec(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, extra map[string]*string) (map[string]*string, error) { + t.Override(&docker.EvalBuildArgsWithEnv, func(_ config.RunMode, _ string, _ string, args map[string]*string, extra map[string]*string, _ map[string]string) (map[string]*string, error) { m := make(map[string]*string) for k, v := range args { m[k] = v @@ -290,7 +290,7 @@ func TestPullCacheFrom(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) { + t.Override(&docker.EvalBuildArgsWithEnv, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string, _ map[string]string) (map[string]*string, error) { return args, nil }) builder := NewBuilder(&mockBuilderContext{}, &latest.GoogleCloudBuild{ diff --git a/pkg/skaffold/build/gcb/kaniko.go b/pkg/skaffold/build/gcb/kaniko.go index 2d64f03266c..5cbe0b39b30 100644 --- a/pkg/skaffold/build/gcb/kaniko.go +++ b/pkg/skaffold/build/gcb/kaniko.go @@ -32,7 +32,11 @@ func (b *Builder) kanikoBuildSpec(a *latest.Artifact, tag string) (cloudbuild.Bu k := a.KanikoArtifact requiredImages := docker.ResolveDependencyImages(a.Dependencies, b.artifactStore, true) // add required artifacts as build args - buildArgs, err := docker.EvalBuildArgs(b.cfg.Mode(), kaniko.GetContext(a.KanikoArtifact, a.Workspace), k.DockerfilePath, k.BuildArgs, requiredImages) + envTags, err := docker.EnvTags(tag) + if err != nil { + return cloudbuild.Build{}, fmt.Errorf("unable to create build args: %w", err) + } + buildArgs, err := docker.EvalBuildArgsWithEnv(b.cfg.Mode(), kaniko.GetContext(a.KanikoArtifact, a.Workspace), k.DockerfilePath, k.BuildArgs, requiredImages, envTags) if err != nil { return cloudbuild.Build{}, fmt.Errorf("unable to evaluate build args: %w", err) } diff --git a/pkg/skaffold/build/gcb/kaniko_test.go b/pkg/skaffold/build/gcb/kaniko_test.go index 6a464d6f615..fbb927ccfed 100644 --- a/pkg/skaffold/build/gcb/kaniko_test.go +++ b/pkg/skaffold/build/gcb/kaniko_test.go @@ -467,7 +467,7 @@ func TestKanikoBuildSpec(t *testing.T) { imageArgs := []string{kaniko.BuildArgsFlag, "IMG2=img2:tag", kaniko.BuildArgsFlag, "IMG3=img3:tag"} - t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, extra map[string]*string) (map[string]*string, error) { + t.Override(&docker.EvalBuildArgsWithEnv, func(_ config.RunMode, _ string, _ string, args map[string]*string, extra map[string]*string, _ map[string]string) (map[string]*string, error) { m := make(map[string]*string) for k, v := range args { m[k] = v diff --git a/pkg/skaffold/deploy/helm/helm.go b/pkg/skaffold/deploy/helm/helm.go index 4f2f44136ce..3102fd8d952 100644 --- a/pkg/skaffold/deploy/helm/helm.go +++ b/pkg/skaffold/deploy/helm/helm.go @@ -312,6 +312,10 @@ func (h *Deployer) Deploy(ctx context.Context, out io.Writer, builds []graph.Art olog.Entry(ctx).Infof("Installing releases (%d releases)", len(releases)) } + // sort releases in the same level, this is merely to ensure that the series of helm commands are in order for + // consistency in unit testing. + sort.Strings(releases) + // Deploy releases in current level for _, releaseName := range releases { release := releaseNameToRelease[releaseName] diff --git a/pkg/skaffold/deploy/util/util.go b/pkg/skaffold/deploy/util/util.go index 72e5f26dfe5..12a89db14da 100644 --- a/pkg/skaffold/deploy/util/util.go +++ b/pkg/skaffold/deploy/util/util.go @@ -22,8 +22,11 @@ import ( "io" "os" "path/filepath" + "runtime" + "time" "github.com/buildpacks/lifecycle/cmd" + "golang.org/x/sync/semaphore" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" k8s "k8s.io/client-go/kubernetes" @@ -36,8 +39,13 @@ import ( "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/initializer/prompt" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/manifest" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output/log" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/runner/runcontext" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/tag" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util/stringset" + timeutil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util/time" ) var ( @@ -184,3 +192,83 @@ func GetManifestsFromHydratedManifests(ctx context.Context, hydratedManifests [] return manifests, nil } + +type tagErr struct { + tag string + err error +} + +// ImageTags generates tags for a list of artifacts +func ImageTags(ctx context.Context, runCtx *runcontext.RunContext, tagger tag.Tagger, out io.Writer, artifacts []*latest.Artifact) (tag.ImageTags, error) { + start := time.Now() + maxWorkers := runtime.GOMAXPROCS(0) + + if len(artifacts) > 0 { + output.Default.Fprintln(out, "Generating tags...") + } else { + output.Default.Fprintln(out, "No tags generated") + } + + tagErrs := make([]chan tagErr, len(artifacts)) + + // Use a weighted semaphore as a counting semaphore to limit the number of simultaneous taggers. + sem := semaphore.NewWeighted(int64(maxWorkers)) + for i := range artifacts { + tagErrs[i] = make(chan tagErr, 1) + + if err := sem.Acquire(ctx, 1); err != nil { + return nil, err + } + + i := i + go func() { + defer sem.Release(1) + _tag, err := tag.GenerateFullyQualifiedImageName(ctx, tagger, *artifacts[i]) + tagErrs[i] <- tagErr{tag: _tag, err: err} + }() + } + + imageTags := make(tag.ImageTags, len(artifacts)) + showWarning := false + + for i, artifact := range artifacts { + imageName := artifact.ImageName + out, ctx := output.WithEventContext(ctx, out, constants.Build, imageName) + output.Default.Fprintf(out, " - %s -> ", imageName) + + select { + case <-ctx.Done(): + return nil, context.Canceled + + case t := <-tagErrs[i]: + if t.err != nil { + log.Entry(ctx).Debug(t.err) + log.Entry(ctx).Debug("Using a fall-back tagger") + + fallbackTag, err := tag.GenerateFullyQualifiedImageName(ctx, &tag.ChecksumTagger{}, *artifact) + if err != nil { + return nil, fmt.Errorf("generating checksum as fall-back tag for %q: %w", imageName, err) + } + + t.tag = fallbackTag + showWarning = true + } + + _tag, err := ApplyDefaultRepo(runCtx.GlobalConfig(), runCtx.DefaultRepo(), t.tag) + + if err != nil { + return nil, err + } + + fmt.Fprintln(out, _tag) + imageTags[imageName] = _tag + } + } + + if showWarning { + output.Yellow.Fprintln(out, "Some taggers failed. Rerun with -vdebug for errors.") + } + + log.Entry(ctx).Infoln("Tags generated in", timeutil.Humanize(time.Since(start))) + return imageTags, nil +} diff --git a/pkg/skaffold/diagnose/diagnose.go b/pkg/skaffold/diagnose/diagnose.go index 5db4f2df9fe..dcab9463f1e 100644 --- a/pkg/skaffold/diagnose/diagnose.go +++ b/pkg/skaffold/diagnose/diagnose.go @@ -29,6 +29,7 @@ import ( "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/sync" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/tag" timeutil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util/time" ) @@ -39,7 +40,7 @@ type Config interface { Artifacts() []*latest.Artifact } -func CheckArtifacts(ctx context.Context, cfg Config, out io.Writer) error { +func CheckArtifacts(ctx context.Context, cfg Config, tags tag.ImageTags, out io.Writer) error { for _, p := range cfg.GetPipelines() { for _, artifact := range p.Build.Artifacts { output.Default.Fprintf(out, "\n%s: %s\n", typeOfArtifact(artifact), artifact.ImageName) @@ -53,11 +54,16 @@ func CheckArtifacts(ctx context.Context, cfg Config, out io.Writer) error { fmt.Fprintf(out, " - Size of the context: %vbytes\n", size) } - timeDeps1, deps, err := timeToListDependencies(ctx, artifact, cfg) + tag, ok := tags[artifact.ImageName] + if !ok { + return fmt.Errorf("getting tag for image %s", artifact.ImageName) + } + + timeDeps1, deps, err := timeToListDependencies(ctx, artifact, tag, cfg) if err != nil { return fmt.Errorf("listing artifact dependencies: %w", err) } - timeDeps2, _, err := timeToListDependencies(ctx, artifact, cfg) + timeDeps2, _, err := timeToListDependencies(ctx, artifact, tag, cfg) if err != nil { return fmt.Errorf("listing artifact dependencies: %w", err) } @@ -119,11 +125,11 @@ func typeOfArtifact(a *latest.Artifact) string { } } -func timeToListDependencies(ctx context.Context, a *latest.Artifact, cfg Config) (string, []string, error) { +func timeToListDependencies(ctx context.Context, a *latest.Artifact, tag string, cfg Config) (string, []string, error) { start := time.Now() g := graph.ToArtifactGraph(cfg.Artifacts()) sourceDependencies := graph.NewSourceDependenciesCache(cfg, nil, g) - paths, err := sourceDependencies.SingleArtifactDependencies(ctx, a) + paths, err := sourceDependencies.SingleArtifactDependencies(ctx, a, tag) return timeutil.Humanize(time.Since(start)), paths, err } diff --git a/pkg/skaffold/diagnose/diagnose_test.go b/pkg/skaffold/diagnose/diagnose_test.go index e79b0f2e350..29d5e12f4e1 100644 --- a/pkg/skaffold/diagnose/diagnose_test.go +++ b/pkg/skaffold/diagnose/diagnose_test.go @@ -80,9 +80,10 @@ func TestSizeOfDockerContext(t *testing.T) { func TestCheckArtifacts(t *testing.T) { testutil.Run(t, "", func(t *testutil.T) { tmpDir := t.NewTempDir().Write("Dockerfile", "FROM busybox") - + tags := map[string]string{"base": "gcr.io/library/busybox:latest"} err := CheckArtifacts(context.Background(), &mockConfig{ artifacts: []*latest.Artifact{{ + ImageName: "base", Workspace: tmpDir.Root(), ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{ @@ -90,12 +91,29 @@ func TestCheckArtifacts(t *testing.T) { }, }, }}, - }, io.Discard) - + }, tags, io.Discard) t.CheckNoError(err) }) } +func TestCheckArtifactsFailure(t *testing.T) { + testutil.Run(t, "", func(t *testutil.T) { + tmpDir := t.NewTempDir().Write("Dockerfile", "FROM busybox") + err := CheckArtifacts(context.Background(), &mockConfig{ + artifacts: []*latest.Artifact{{ + ImageName: "base", + Workspace: tmpDir.Root(), + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + DockerfilePath: "Dockerfile", + }, + }, + }}, + }, nil, io.Discard) + t.CheckError(true, err) + }) +} + type mockConfig struct { runcontext.RunContext // Embedded to provide the default values. artifacts []*latest.Artifact diff --git a/pkg/skaffold/docker/build_args.go b/pkg/skaffold/docker/build_args.go index fa45af54520..73c0cf32d0a 100644 --- a/pkg/skaffold/docker/build_args.go +++ b/pkg/skaffold/docker/build_args.go @@ -114,3 +114,19 @@ func ResolveDependencyImages(deps []*latest.ArtifactDependency, r ArtifactResolv } return m } + +// EnvTags generate a set of build tags from the docker image name. +func EnvTags(tag string) (map[string]string, error) { + imgRef, err := ParseReference(tag) + if err != nil { + return nil, fmt.Errorf("couldn't parse image tag %s %w", tag, err) + } + if imgRef.Tag == "" { + imgRef.Tag = "latest" + } + return map[string]string{ + "IMAGE_REPO": imgRef.Repo, + "IMAGE_NAME": imgRef.Name, + "IMAGE_TAG": imgRef.Tag, + }, nil +} diff --git a/pkg/skaffold/docker/build_args_test.go b/pkg/skaffold/docker/build_args_test.go index 9ba27ab767f..f5002b8458a 100644 --- a/pkg/skaffold/docker/build_args_test.go +++ b/pkg/skaffold/docker/build_args_test.go @@ -210,7 +210,7 @@ FROM bar1`, tmpDir.Write("./Dockerfile", test.dockerfile) workspace := tmpDir.Path(".") - actual, err := EvalBuildArgs(test.mode, workspace, artifact.DockerfilePath, artifact.BuildArgs, test.extra) + actual, err := EvalBuildArgsWithEnv(test.mode, workspace, artifact.DockerfilePath, artifact.BuildArgs, test.extra, nil) t.CheckNoError(err) t.CheckDeepEqual(test.expected, actual) }) @@ -226,13 +226,13 @@ func TestCreateBuildArgsFromArtifacts(t *testing.T) { }{ { description: "can resolve artifacts", - r: mockArtifactResolver{m: map[string]string{"img1": "tag1", "img2": "tag2", "img3": "tag3", "img4": "tag4"}}, + r: NewStubArtifactResolver(map[string]string{"img1": "tag1", "img2": "tag2", "img3": "tag3", "img4": "tag4"}), deps: []*latest.ArtifactDependency{{ImageName: "img3", Alias: "alias3"}, {ImageName: "img4", Alias: "alias4"}}, args: map[string]*string{"alias3": util.Ptr("tag3"), "alias4": util.Ptr("tag4")}, }, { description: "cannot resolve artifacts", - r: mockArtifactResolver{m: make(map[string]string)}, + r: NewStubArtifactResolver(map[string]string{}), args: map[string]*string{"alias3": nil, "alias4": nil}, deps: []*latest.ArtifactDependency{{ImageName: "img3", Alias: "alias3"}, {ImageName: "img4", Alias: "alias4"}}, }, @@ -274,19 +274,10 @@ func TestBuildArgTemplating(t *testing.T) { tmpDir.Write("./Dockerfile", dockerFile) workspace := tmpDir.Path(".") - filledMap, err := EvalBuildArgs(config.RunModes.Dev, workspace, artifact.DockerfilePath, artifact.BuildArgs, nil) + filledMap, err := EvalBuildArgsWithEnv(config.RunModes.Dev, workspace, artifact.DockerfilePath, artifact.BuildArgs, nil, nil) t.CheckNil(err) t.CheckMatches(*filledMap["MY_KEY"], "abc123") t.CheckMatches(*filledMap["SO_SECRET"], "do not say it") }) } - -type mockArtifactResolver struct { - m map[string]string -} - -func (r mockArtifactResolver) GetImageTag(imageName string) (string, bool) { - val, found := r.m[imageName] - return val, found -} diff --git a/pkg/skaffold/docker/dependencies_test.go b/pkg/skaffold/docker/dependencies_test.go index a63dafc0575..939adbdfc62 100644 --- a/pkg/skaffold/docker/dependencies_test.go +++ b/pkg/skaffold/docker/dependencies_test.go @@ -254,27 +254,6 @@ FROM library/ruby:2.3.0 ADD ./file /etc/file ` -type fakeImageFetcher struct{} - -func (f *fakeImageFetcher) fetch(_ context.Context, image string, _ Config) (*v1.ConfigFile, error) { - switch image { - case "ubuntu:14.04", "busybox", "nginx", "golang:1.9.2", "jboss/wildfly:14.0.1.Final", "gcr.io/distroless/base": - return &v1.ConfigFile{}, nil - case "golang:onbuild": - return &v1.ConfigFile{ - Config: v1.Config{ - OnBuild: []string{ - "COPY . /onbuild", - }, - }, - }, nil - case "library/ruby:2.3.0": - return nil, fmt.Errorf("retrieving image \"library/ruby:2.3.0\": unsupported MediaType: \"application/vnd.docker.distribution.manifest.v1+prettyjws\", see https://github.com/google/go-containerregistry/issues/377") - } - - return nil, fmt.Errorf("no image found for %s", image) -} - func TestGetDependencies(t *testing.T) { tests := []struct { description string diff --git a/pkg/skaffold/docker/dockertest.go b/pkg/skaffold/docker/dockertest.go new file mode 100644 index 00000000000..833c51b713b --- /dev/null +++ b/pkg/skaffold/docker/dockertest.go @@ -0,0 +1,121 @@ +/* +Copyright 2025 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package docker + +import ( + "context" + "fmt" + + v1 "github.com/google/go-containerregistry/pkg/v1" + + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/config" +) + +// MockArtifactResolver mocks docker.ArtifactResolver interface. +type stubArtifactResolver struct { + m map[string]string +} + +// NewStubArtifactResolver returns a mock ArtifactResolver for testing. +func NewStubArtifactResolver(m map[string]string) *stubArtifactResolver { + return &stubArtifactResolver{m} +} + +func (r stubArtifactResolver) GetImageTag(imageName string) (string, bool) { + val, found := r.m[imageName] + return val, found +} + +// simpleStubArtifactResolver is an implementation of docker.ArtifactResolver +// that returns the same value for any key +type simpleStubArtifactResolver struct{} + +// GetImageTag is an implementation of docker.ArtifactResolver that +// always returns the same tag. +func (s *simpleStubArtifactResolver) GetImageTag(_ string) (string, bool) { + return "image:latest", true +} + +func NewSimpleStubArtifactResolver() ArtifactResolver { + return &simpleStubArtifactResolver{} +} + +// configStub is a mock implementation of the Config interface. +type configStub struct { + runMode config.RunMode + prune bool +} + +func (m configStub) GetKubeContext() string { + return "" +} + +func (m configStub) GlobalConfig() string { + return "" +} + +func (m configStub) MinikubeProfile() string { + return "" +} + +func (m configStub) GetInsecureRegistries() map[string]bool { + return map[string]bool{} +} + +func (m configStub) Mode() config.RunMode { + return m.runMode +} + +func (m configStub) Prune() bool { + return m.prune +} + +func (m configStub) ContainerDebugging() bool { + return false +} + +func NewConfigStub(mode config.RunMode, prune bool) Config { + return &configStub{runMode: mode, prune: prune} +} + +type fakeImageFetcher struct{} + +func (f *fakeImageFetcher) fetch(_ context.Context, image string, _ Config) (*v1.ConfigFile, error) { + switch image { + case "ubuntu:14.04", "busybox", "nginx", "golang:1.9.2", "jboss/wildfly:14.0.1.Final", "gcr.io/distroless/base", "gcr.io/distroless/base:latest": + return &v1.ConfigFile{}, nil + case "golang:onbuild": + return &v1.ConfigFile{ + Config: v1.Config{ + OnBuild: []string{ + "COPY . /onbuild", + }, + }, + }, nil + case "library/ruby:2.3.0": + return nil, fmt.Errorf("retrieving image \"library/ruby:2.3.0\": unsupported MediaType: \"application/vnd.docker.distribution.manifest.v1+prettyjws\", see https://github.com/google/go-containerregistry/issues/377") + } + + return nil, fmt.Errorf("no image found for %s", image) +} + +type FetchImage func(context.Context, string, Config) (*v1.ConfigFile, error) + +func NewFakeImageFetcher() FetchImage { + fetcher := fakeImageFetcher{} + return fetcher.fetch +} diff --git a/pkg/skaffold/docker/image.go b/pkg/skaffold/docker/image.go index 02455ea3495..1ac35b86b5d 100644 --- a/pkg/skaffold/docker/image.go +++ b/pkg/skaffold/docker/image.go @@ -323,15 +323,10 @@ func (l *localDaemon) Build(ctx context.Context, out io.Writer, workspace string if err := l.CheckCompatible(a); err != nil { return "", err } - imgRef, err := ParseReference(opts.Tag) + imageInfoEnv, err := EnvTags(opts.Tag) if err != nil { return "", fmt.Errorf("couldn't parse image tag: %w", err) } - imageInfoEnv := map[string]string{ - "IMAGE_REPO": imgRef.Repo, - "IMAGE_NAME": imgRef.Name, - "IMAGE_TAG": imgRef.Tag, - } buildArgs, err := EvalBuildArgsWithEnv(opts.Mode, workspace, a.DockerfilePath, a.BuildArgs, opts.ExtraBuildArgs, imageInfoEnv) if err != nil { return "", fmt.Errorf("unable to evaluate build args: %w", err) diff --git a/pkg/skaffold/graph/dependencies.go b/pkg/skaffold/graph/dependencies.go index d3a2a9adcd9..1854d19f2a8 100644 --- a/pkg/skaffold/graph/dependencies.go +++ b/pkg/skaffold/graph/dependencies.go @@ -43,7 +43,7 @@ type SourceDependenciesCache interface { TransitiveArtifactDependencies(ctx context.Context, a *latest.Artifact) ([]string, error) // SingleArtifactDependencies returns the source dependencies for only the target artifact. // The result (even if an error) is cached so that the function is evaluated only once for every artifact. The cache is reset before the start of the next devloop. - SingleArtifactDependencies(ctx context.Context, a *latest.Artifact) ([]string, error) + SingleArtifactDependencies(ctx context.Context, a *latest.Artifact, tag string) ([]string, error) // Reset removes the cached source dependencies for all artifacts Reset() } @@ -65,7 +65,12 @@ func (r *dependencyResolverImpl) TransitiveArtifactDependencies(ctx context.Cont }) defer endTrace() - deps, err := r.SingleArtifactDependencies(ctx, a) + tag, ok := r.artifactResolver.GetImageTag(a.ImageName) + if !ok { + return nil, fmt.Errorf("unable to resolve tag for image: %s", a.ImageName) + } + + deps, err := r.SingleArtifactDependencies(ctx, a, tag) if err != nil { endTrace(instrumentation.TraceEndError(err)) return nil, err @@ -81,14 +86,14 @@ func (r *dependencyResolverImpl) TransitiveArtifactDependencies(ctx context.Cont return deps, nil } -func (r *dependencyResolverImpl) SingleArtifactDependencies(ctx context.Context, a *latest.Artifact) ([]string, error) { +func (r *dependencyResolverImpl) SingleArtifactDependencies(ctx context.Context, a *latest.Artifact, tag string) ([]string, error) { ctx, endTrace := instrumentation.StartTrace(ctx, "SingleArtifactDependencies", map[string]string{ "ArtifactName": instrumentation.PII(a.ImageName), }) defer endTrace() res, err := r.cache.Exec(a.ImageName, func() ([]string, error) { - return getDependenciesFunc(ctx, a, r.cfg, r.artifactResolver) + return getDependenciesFunc(ctx, a, r.cfg, r.artifactResolver, tag) }) if err != nil { endTrace(instrumentation.TraceEndError(err)) @@ -105,12 +110,17 @@ func (r *dependencyResolverImpl) Reset() { } // sourceDependenciesForArtifact returns the build dependencies for the current artifact. -func sourceDependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker.Config, r docker.ArtifactResolver) ([]string, error) { +func sourceDependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker.Config, r docker.ArtifactResolver, tag string) ([]string, error) { var ( paths []string err error ) + envTags, evalErr := docker.EnvTags(tag) + if evalErr != nil { + return nil, fmt.Errorf("unable to create build args: %w", err) + } + switch { case a.DockerArtifact != nil: // Required artifacts cannot be resolved when `ResolveDependencyImages` runs prior to a completed build sequence (like `skaffold build` or the first iteration of `skaffold dev`). @@ -118,7 +128,8 @@ func sourceDependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg // For single build scenarios like `build` and `run`, it is called for the cache hash calculations which are already handled in `artifactHasher`. // For `dev` it will succeed on the first dev loop and list any additional dependencies found from the base artifact's ONBUILD instructions as a file added instead of modified (see `filemon.Events`) deps := docker.ResolveDependencyImages(a.Dependencies, r, false) - args, evalErr := docker.EvalBuildArgs(cfg.Mode(), a.Workspace, a.DockerArtifact.DockerfilePath, a.DockerArtifact.BuildArgs, deps) + + args, evalErr := docker.EvalBuildArgsWithEnv(cfg.Mode(), a.Workspace, a.DockerArtifact.DockerfilePath, a.DockerArtifact.BuildArgs, deps, envTags) if evalErr != nil { return nil, fmt.Errorf("unable to evaluate build args: %w", evalErr) } @@ -126,7 +137,7 @@ func sourceDependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg case a.KanikoArtifact != nil: deps := docker.ResolveDependencyImages(a.Dependencies, r, false) - args, evalErr := docker.EvalBuildArgs(cfg.Mode(), kaniko.GetContext(a.KanikoArtifact, a.Workspace), a.KanikoArtifact.DockerfilePath, a.KanikoArtifact.BuildArgs, deps) + args, evalErr := docker.EvalBuildArgsWithEnv(cfg.Mode(), kaniko.GetContext(a.KanikoArtifact, a.Workspace), a.KanikoArtifact.DockerfilePath, a.KanikoArtifact.BuildArgs, deps, envTags) if evalErr != nil { return nil, fmt.Errorf("unable to evaluate build args: %w", evalErr) } diff --git a/pkg/skaffold/graph/dependencies_test.go b/pkg/skaffold/graph/dependencies_test.go index 37993f3f7aa..0137f8ff7a1 100644 --- a/pkg/skaffold/graph/dependencies_test.go +++ b/pkg/skaffold/graph/dependencies_test.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/config" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/docker" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/v2/testutil" @@ -43,12 +44,12 @@ func TestSourceDependenciesCache(t *testing.T) { "img4": {"file41", "file42"}, } counts := map[string]int{"img1": 0, "img2": 0, "img3": 0, "img4": 0} - t.Override(&getDependenciesFunc, func(_ context.Context, a *latest.Artifact, _ docker.Config, _ docker.ArtifactResolver) ([]string, error) { + t.Override(&getDependenciesFunc, func(_ context.Context, a *latest.Artifact, _ docker.Config, _ docker.ArtifactResolver, _ string) ([]string, error) { counts[a.ImageName]++ return deps[a.ImageName], nil }) - r := NewSourceDependenciesCache(nil, nil, g) + r := NewSourceDependenciesCache(nil, docker.NewSimpleStubArtifactResolver(), g) d, err := r.TransitiveArtifactDependencies(context.Background(), g["img1"]) t.CheckNoError(err) expectedDeps := []string{"file11", "file12", "file21", "file22", "file31", "file32", "file41", "file42", "file41", "file42"} @@ -63,15 +64,18 @@ func TestSourceDependenciesForArtifact(t *testing.T) { tmpDir := testutil.NewTempDir(t).Touch( "foo.java", "bar.go", + "latest.go", "dir1/baz.java", "dir2/frob.go", ) tests := []struct { - description string - artifact *latest.Artifact - dockerConfig docker.Config - dockerArtifactResolver docker.ArtifactResolver - expectedPaths []string + description string + artifact *latest.Artifact + tag string + dockerConfig docker.Config + dockerBuildArgs map[string]string + dockerFileContents string + expectedPaths []string }{ { description: "ko default dependencies", @@ -81,15 +85,59 @@ func TestSourceDependenciesForArtifact(t *testing.T) { }, Workspace: tmpDir.Root(), }, + tag: "gcr.io/distroless/base:latest", expectedPaths: []string{ filepath.Join(tmpDir.Root(), "dir2/frob.go"), filepath.Join(tmpDir.Root(), "bar.go"), + filepath.Join(tmpDir.Root(), "latest.go"), }, }, + { + description: "docker default dependencies", + artifact: &latest.Artifact{ + ImageName: "img1", + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + DockerfilePath: "Dockerfile", + }, + }, + Workspace: tmpDir.Root(), + Dependencies: []*latest.ArtifactDependency{{ImageName: "img2", Alias: "BASE"}}, + }, + dockerBuildArgs: map[string]string{ + "IMAGE_REPO": "{{.IMAGE_REPO}}", + "IMAGE_NAME": "{{.IMAGE_NAME}}", + "IMAGE_TAG": "{{.IMAGE_TAG}}", + }, + dockerConfig: docker.NewConfigStub(config.RunModes.Build, false), + dockerFileContents: `ARG IMAGE_REPO +ARG IMAGE_NAME +ARG IMAGE_TAG +FROM $IMAGE_REPO/$IMAGE_NAME:$IMAGE_TAG +COPY bar.go . +COPY $IMAGE_TAG.go . +`, + expectedPaths: []string{ + filepath.Join(tmpDir.Root(), "Dockerfile"), + filepath.Join(tmpDir.Root(), "bar.go"), + filepath.Join(tmpDir.Root(), "latest.go"), + }, + tag: "gcr.io/distroless/base:latest", + }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - paths, err := sourceDependenciesForArtifact(context.Background(), test.artifact, test.dockerConfig, test.dockerArtifactResolver) + t.Override(&docker.RetrieveImage, docker.NewFakeImageFetcher()) + d := docker.NewSimpleStubArtifactResolver() + tmpDir.Write("Dockerfile", test.dockerFileContents) + if test.dockerBuildArgs != nil { + args := map[string]*string{} + for k, v := range test.dockerBuildArgs { + args[k] = &v + } + test.artifact.DockerArtifact.BuildArgs = args + } + paths, err := sourceDependenciesForArtifact(context.Background(), test.artifact, test.dockerConfig, d, test.tag) t.CheckNoError(err) t.CheckDeepEqual(test.expectedPaths, paths, cmpopts.SortSlices(func(x, y string) bool { return x < y })) diff --git a/pkg/skaffold/runner/build.go b/pkg/skaffold/runner/build.go index c3acd4ea547..b4ed64a7c03 100644 --- a/pkg/skaffold/runner/build.go +++ b/pkg/skaffold/runner/build.go @@ -21,10 +21,6 @@ import ( "fmt" "io" "os" - "runtime" - "time" - - "golang.org/x/sync/semaphore" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/build/cache" @@ -33,12 +29,10 @@ import ( eventV2 "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/event/v2" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/graph" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output" - "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output/log" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/platform" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/tag" - timeutil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util/time" ) func NewBuilder(builder build.Builder, tagger tag.Tagger, platforms platform.Resolver, cache cache.Cache, runCtx *runcontext.RunContext) *Builder { @@ -82,7 +76,7 @@ func (r *Builder) Build(ctx context.Context, out io.Writer, artifacts []*latest. return nil, err } - tags, err := r.imageTags(ctx, out, artifacts) + tags, err := deployutil.ImageTags(ctx, r.runCtx, r.tagger, out, artifacts) if err != nil { eventV2.TaskFailed(constants.Build, err) return nil, err @@ -130,6 +124,11 @@ func (r *Builder) Build(ctx context.Context, out io.Writer, artifacts []*latest. return bRes, nil } +// ApplyDefaultRepo applies the default repo to a given image tag. +func (r *Builder) ApplyDefaultRepo(tag string) (string, error) { + return deployutil.ApplyDefaultRepo(r.runCtx.GlobalConfig(), r.runCtx.DefaultRepo(), tag) +} + // HasBuilt returns true if this runner has built something. func (r *Builder) HasBuilt() bool { return r.hasBuilt @@ -148,90 +147,6 @@ func artifactsWithTags(tags tag.ImageTags, artifacts []*latest.Artifact) []graph return bRes } -type tagErr struct { - tag string - err error -} - -// ApplyDefaultRepo applies the default repo to a given image tag. -func (r *Builder) ApplyDefaultRepo(tag string) (string, error) { - return deployutil.ApplyDefaultRepo(r.runCtx.GlobalConfig(), r.runCtx.DefaultRepo(), tag) -} - -// imageTags generates tags for a list of artifacts -func (r *Builder) imageTags(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) (tag.ImageTags, error) { - start := time.Now() - maxWorkers := runtime.GOMAXPROCS(0) - - if len(artifacts) > 0 { - output.Default.Fprintln(out, "Generating tags...") - } else { - output.Default.Fprintln(out, "No tags generated") - } - - tagErrs := make([]chan tagErr, len(artifacts)) - - // Use a weighted semaphore as a counting semaphore to limit the number of simultaneous taggers. - sem := semaphore.NewWeighted(int64(maxWorkers)) - for i := range artifacts { - tagErrs[i] = make(chan tagErr, 1) - - if err := sem.Acquire(ctx, 1); err != nil { - return nil, err - } - - i := i - go func() { - defer sem.Release(1) - _tag, err := tag.GenerateFullyQualifiedImageName(ctx, r.tagger, *artifacts[i]) - tagErrs[i] <- tagErr{tag: _tag, err: err} - }() - } - - imageTags := make(tag.ImageTags, len(artifacts)) - showWarning := false - - for i, artifact := range artifacts { - imageName := artifact.ImageName - out, ctx := output.WithEventContext(ctx, out, constants.Build, imageName) - output.Default.Fprintf(out, " - %s -> ", imageName) - - select { - case <-ctx.Done(): - return nil, context.Canceled - - case t := <-tagErrs[i]: - if t.err != nil { - log.Entry(ctx).Debug(t.err) - log.Entry(ctx).Debug("Using a fall-back tagger") - - fallbackTag, err := tag.GenerateFullyQualifiedImageName(ctx, &tag.ChecksumTagger{}, *artifact) - if err != nil { - return nil, fmt.Errorf("generating checksum as fall-back tag for %q: %w", imageName, err) - } - - t.tag = fallbackTag - showWarning = true - } - - _tag, err := r.ApplyDefaultRepo(t.tag) - if err != nil { - return nil, err - } - - fmt.Fprintln(out, _tag) - imageTags[imageName] = _tag - } - } - - if showWarning { - output.Yellow.Fprintln(out, "Some taggers failed. Rerun with -vdebug for errors.") - } - - log.Entry(ctx).Infoln("Tags generated in", timeutil.Humanize(time.Since(start))) - return imageTags, nil -} - func CheckWorkspaces(artifacts []*latest.Artifact) error { for _, a := range artifacts { if a.Workspace != "" { diff --git a/pkg/skaffold/runner/listen_test.go b/pkg/skaffold/runner/listen_test.go index 59dcf853c88..f2fde4f733d 100644 --- a/pkg/skaffold/runner/listen_test.go +++ b/pkg/skaffold/runner/listen_test.go @@ -59,7 +59,7 @@ func (f *fakeDepsResolver) TransitiveArtifactDependencies(context.Context, *late return nil, nil } -func (f *fakeDepsResolver) SingleArtifactDependencies(context.Context, *latest.Artifact) ([]string, error) { +func (f *fakeDepsResolver) SingleArtifactDependencies(context.Context, *latest.Artifact, string) ([]string, error) { return nil, nil } diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index 4adc8518f97..f73dd9d8aab 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -117,11 +117,11 @@ func NewForConfig(ctx context.Context, runCtx *runcontext.RunContext) (*Skaffold return nil, fmt.Errorf("creating actiosn runner: %w", err) } - depLister := func(ctx context.Context, artifact *latest.Artifact) ([]string, error) { + depLister := func(ctx context.Context, artifact *latest.Artifact, tag string) ([]string, error) { ctx, endTrace := instrumentation.StartTrace(ctx, "NewForConfig_depLister") defer endTrace() - buildDependencies, err := sourceDependencies.SingleArtifactDependencies(ctx, artifact) + buildDependencies, err := sourceDependencies.SingleArtifactDependencies(ctx, artifact, tag) if err != nil { endTrace(instrumentation.TraceEndError(err)) return nil, err diff --git a/pkg/skaffold/tag/custom_template_test.go b/pkg/skaffold/tag/custom_template_test.go index 7530809d9c5..aa84c4a7d31 100644 --- a/pkg/skaffold/tag/custom_template_test.go +++ b/pkg/skaffold/tag/custom_template_test.go @@ -35,7 +35,7 @@ func (r *dependencyResolverImpl) TransitiveArtifactDependencies(ctx context.Cont return []string{}, nil } -func (r *dependencyResolverImpl) SingleArtifactDependencies(ctx context.Context, a *latest.Artifact) ([]string, error) { +func (r *dependencyResolverImpl) SingleArtifactDependencies(ctx context.Context, a *latest.Artifact, tag string) ([]string, error) { return []string{}, nil } diff --git a/pkg/skaffold/tag/input_digest.go b/pkg/skaffold/tag/input_digest.go index a0d7d7acf02..d144326aa5f 100644 --- a/pkg/skaffold/tag/input_digest.go +++ b/pkg/skaffold/tag/input_digest.go @@ -40,7 +40,7 @@ type inputDigestTagger struct { } func NewInputDigestTagger(cfg docker.Config, ag graph.ArtifactGraph) (Tagger, error) { - return NewInputDigestTaggerWithSourceCache(cfg, graph.NewSourceDependenciesCache(cfg, nil, ag)) + return NewInputDigestTaggerWithSourceCache(cfg, graph.NewSourceDependenciesCache(cfg, docker.NewSimpleStubArtifactResolver(), ag)) } func NewInputDigestTaggerWithSourceCache(cfg docker.Config, cache graph.SourceDependenciesCache) (Tagger, error) {