Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make IMAGE_TAG available in buildArgs when used in docker FROM #9664

Merged
merged 46 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
1657f18
provide a docker.ImageReference in dependency resolution step
abe-winter Jun 19, 2024
147d21b
pass nils
abe-winter Jun 19, 2024
35a37c1
other plumbing
abe-winter Jun 19, 2024
f6411bf
fix: add back out io.Writer field
alphanota Jan 14, 2025
354b03c
Merge branch 'GoogleContainerTools:main' into abe-winter-changes
alphanota Jan 14, 2025
a61996c
fix bugs
alphanota Jan 14, 2025
13849cf
Merge branch 'GoogleContainerTools:main' into abe-winter-changes
alphanota Jan 15, 2025
d506f62
fix lookup and hash tests
alphanota Jan 15, 2025
c3bcf4d
fix tag test
alphanota Jan 15, 2025
67014b3
fix skaffold/runner
alphanota Jan 15, 2025
5076965
fix skaffold/graph
alphanota Jan 15, 2025
5324000
remove the tags field where its not needed
alphanota Jan 15, 2025
2784556
make fixes to test
alphanota Jan 16, 2025
630979a
fix tests
alphanota Jan 16, 2025
65af3c9
fix another typo
alphanota Jan 16, 2025
7e67078
add license boilerplate
alphanota Jan 16, 2025
ae699c0
add 2025 as a valid year
alphanota Jan 16, 2025
0b19012
chore:add integration test
alphanota Jan 16, 2025
da11b02
format go file
alphanota Jan 16, 2025
0787e5a
fix: lint and test
alphanota Jan 16, 2025
10ed9ec
fix test
alphanota Jan 16, 2025
104f908
fix:lint
alphanota Jan 16, 2025
0469ddd
add debug output
alphanota Jan 17, 2025
20c9e2a
add test action for debugging
alphanota Jan 17, 2025
6090a40
add test
alphanota Jan 17, 2025
a212113
add test
alphanota Jan 17, 2025
d52f744
add test
alphanota Jan 17, 2025
8f7960a
add test
alphanota Jan 17, 2025
a0569f3
add test
alphanota Jan 17, 2025
09b820a
use the default context when building the image
alphanota Jan 17, 2025
19cc68e
fix imports
alphanota Jan 17, 2025
6ea296a
fix: flaky helm deploy unit test by making the order of helm command …
alphanota Jan 17, 2025
fd7d94d
fix lint
alphanota Jan 18, 2025
954da5c
add unit tests and fix existing tests
alphanota Jan 22, 2025
d3556f7
remove xtra file
alphanota Jan 22, 2025
b042e30
fix lint
alphanota Jan 22, 2025
c052c83
test that the build arg used as part of a filename is also replaced
alphanota Jan 22, 2025
004c943
use stub instead of mock for test objects
alphanota Jan 22, 2025
8c4e2e4
use empty string in place of tag because hash_test unit tests mock Si…
alphanota Jan 22, 2025
4a345a2
add integration test to ensure that the build int BUILD args can be u…
alphanota Jan 22, 2025
6ff04ca
fix imports, remove github action workflow used for debugging
alphanota Jan 22, 2025
c4f74ff
fix lint
alphanota Jan 22, 2025
0ddf270
use one function everywhere in the codebase to generate the image inf…
alphanota Jan 22, 2025
7ca8dbc
Replace use of EvalBuildArgs with EvalBuildArgsWithEnv since they inv…
alphanota Jan 22, 2025
9fd1b34
add unit test for hash package
alphanota Jan 22, 2025
c6298fe
add test for TestCheckArtifacts
alphanota Jan 22, 2025
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
13 changes: 11 additions & 2 deletions cmd/skaffold/app/cmd/diagnose.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion hack/boilerplate/boilerplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 49 additions & 0 deletions integration/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
}
}
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module github.com/GoogleContainerTools/skaffold/examples/docker-deploy-with-build-args/base

go 1.23
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ARG BASE
FROM $BASE as parent
FROM alpine
COPY --from=parent /app .
CMD ["./app"]
Original file line number Diff line number Diff line change
@@ -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]
Original file line number Diff line number Diff line change
@@ -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"]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
echo "HELLO WORLD"
Original file line number Diff line number Diff line change
@@ -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]
2 changes: 1 addition & 1 deletion pkg/skaffold/build/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 18 additions & 12 deletions pkg/skaffold/build/cache/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading