Implement required artifact resolution for cluster builder

integration/testdata/kaniko-microservices/base/Dockerfile

@@ -0,0 +1,5 @@
+FROM alpine:3.10
+# Define GOTRACEBACK to mark this container as using the Go language runtime
+# for `skaffold debug` (https://skaffold.dev/docs/workflows/debug/).
+ENV GOTRACEBACK=single
+CMD ["./app"]

integration/testdata/kaniko-microservices/leeroy-app/Dockerfile

@@ -1,9 +1,11 @@
+ARG BASE
 FROM golang:1.12.9-alpine3.10 as builder
 COPY app.go .
-RUN go build -o /app .
+# `skaffold debug` sets SKAFFOLD_GO_GCFLAGS to disable compiler optimizations
+ARG SKAFFOLD_GO_GCFLAGS
+RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app .
 
-FROM alpine:3.10 as target_stage
-CMD ["./app"]
+FROM $BASE as target_stage
 COPY --from=builder /app .
 
 FROM busybox

integration/testdata/kaniko-microservices/leeroy-web/Dockerfile

@@ -1,7 +1,9 @@
+ARG BASE
 FROM golang:1.12.9-alpine3.10 as builder
 COPY web.go .
-RUN go build -o /web .
+# `skaffold debug` sets SKAFFOLD_GO_GCFLAGS to disable compiler optimizations
+ARG SKAFFOLD_GO_GCFLAGS
+RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app .
 
-FROM alpine:3.10
-CMD ["./web"]
-COPY --from=builder /web .
+FROM $BASE
+COPY --from=builder /app .

integration/testdata/kaniko-microservices/skaffold.yaml

@@ -6,11 +6,21 @@ build:
       context: ./leeroy-web/
       kaniko:
         cache: {}
+      requires:
+      - image: base
+        alias: BASE
     - image: leeroy-app
       context: ./leeroy-app/
       kaniko:
         cache: {}
         target: target_stage
+      requires:
+      - image: base
+        alias: BASE
+    - image: base
+      context: ./base/
+      kaniko:
+        cache: {}
   cluster:
     pullSecretName: e2esecret
 deploy:

pkg/skaffold/build/cache/hash.go

@@ -163,9 +163,9 @@ func hashBuildArgs(artifact *latest.Artifact, mode config.RunMode) ([]string, er
 	var err error
 	switch {
 	case artifact.DockerArtifact != nil:
-		args, err = docker.EvalBuildArgs(mode, artifact.Workspace, artifact.DockerArtifact, nil)
+		args, err = docker.EvalBuildArgs(mode, artifact.Workspace, artifact.DockerArtifact.DockerfilePath, artifact.DockerArtifact.BuildArgs, nil)
 	case artifact.KanikoArtifact != nil:
-		args, err = util.EvaluateEnvTemplateMap(artifact.KanikoArtifact.BuildArgs)
+		args, err = docker.EvalBuildArgs(mode, artifact.Workspace, artifact.KanikoArtifact.DockerfilePath, artifact.KanikoArtifact.BuildArgs, nil)
 	case artifact.BuildpackArtifact != nil:
 		env, err = buildpacks.GetEnv(artifact, mode)
 	case artifact.CustomArtifact != nil && artifact.CustomArtifact.Dependencies.Dockerfile != nil:

pkg/skaffold/build/cache/hash_test.go

@@ -557,6 +557,12 @@ func TestHashBuildArgs(t *testing.T) {
 				a.Workspace = tmpDir.Path(".")
 				a.ArtifactType.DockerArtifact.DockerfilePath = Dockerfile
 			}
+			if test.artifactType.KanikoArtifact != nil {
+				tmpDir := t.NewTempDir()
+				tmpDir.Write("./Dockerfile", "ARG SKAFFOLD_GO_GCFLAGS\nFROM foo")
+				a.Workspace = tmpDir.Path(".")
+				a.ArtifactType.KanikoArtifact.DockerfilePath = Dockerfile
+			}
 			actual, err := hashBuildArgs(a, test.mode)
 			t.CheckNoError(err)
 			t.CheckDeepEqual(test.expected, actual)

pkg/skaffold/build/cache/retrieve_test.go

@@ -131,8 +131,8 @@ func TestCacheBuildLocal(t *testing.T) {
 		})
 
 		// Mock args builder
-		t.Override(&docker.EvalBuildArgs, func(mode config.RunMode, workspace string, a *latest.DockerArtifact, extra map[string]*string) (map[string]*string, error) {
-			return a.BuildArgs, nil
+		t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) {
+			return args, nil
 		})
 
 		// Create cache
@@ -227,8 +227,8 @@ func TestCacheBuildRemote(t *testing.T) {
 		})
 
 		// Mock args builder
-		t.Override(&docker.EvalBuildArgs, func(mode config.RunMode, workspace string, a *latest.DockerArtifact, extra map[string]*string) (map[string]*string, error) {
-			return a.BuildArgs, nil
+		t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) {
+			return args, nil
 		})
 
 		// Create cache
@@ -311,8 +311,8 @@ func TestCacheFindMissing(t *testing.T) {
 		})
 
 		// Mock args builder
-		t.Override(&docker.EvalBuildArgs, func(mode config.RunMode, workspace string, a *latest.DockerArtifact, extra map[string]*string) (map[string]*string, error) {
-			return a.BuildArgs, nil
+		t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) {
+			return args, nil
 		})
 
 		// Create cache

pkg/skaffold/build/cluster/cluster.go

@@ -26,7 +26,9 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
 // Build builds a list of artifacts with Kaniko.
@@ -60,12 +62,19 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *la
 }
 
 func (b *Builder) runBuildForArtifact(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) (string, error) {
+	// required artifacts as build-args
+	requiredImages := docker.ResolveDependencyImages(a.Dependencies, b.artifactStore, true)
 	switch {
 	case a.KanikoArtifact != nil:
+		buildArgs, err := docker.EvalBuildArgs(b.mode, a.Workspace, a.KanikoArtifact.DockerfilePath, a.KanikoArtifact.BuildArgs, requiredImages)
+		if err != nil {
+			return "", fmt.Errorf("unable to evaluate build args: %w", err)
+		}
+		a.KanikoArtifact.BuildArgs = buildArgs
 		return b.buildWithKaniko(ctx, out, a.Workspace, a.KanikoArtifact, tag)
 
 	case a.CustomArtifact != nil:
-		return custom.NewArtifactBuilder(nil, b.cfg, true, b.retrieveExtraEnv()).Build(ctx, out, a, tag)
+		return custom.NewArtifactBuilder(nil, b.cfg, true, append(b.retrieveExtraEnv(), util.EnvPtrMapToSlice(requiredImages, "=")...)).Build(ctx, out, a, tag)
 
 	default:
 		return "", fmt.Errorf("unexpected type %q for in-cluster artifact:\n%s", misc.ArtifactType(a), misc.FormatArtifact(a))

pkg/skaffold/build/cluster/types.go

@@ -35,6 +35,7 @@ type Builder struct {
 
 	cfg           Config
 	kubectlcli    *kubectl.CLI
+	mode          config.RunMode
 	timeout       time.Duration
 	artifactStore build.ArtifactStore
 }
@@ -46,6 +47,7 @@ type Config interface {
 	Pipeline() latest.Pipeline
 	GetKubeContext() string
 	Muted() config.Muted
+	Mode() config.RunMode
 }
 
 // NewBuilder creates a new Builder that builds artifacts on cluster.
@@ -59,6 +61,7 @@ func NewBuilder(cfg Config) (*Builder, error) {
 		ClusterDetails: cfg.Pipeline().Build.Cluster,
 		cfg:            cfg,
 		kubectlcli:     kubectl.NewCLI(cfg, ""),
+		mode:           cfg.Mode(),
 		timeout:        timeout,
 	}, nil
 }

pkg/skaffold/build/cluster/types_test.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
@@ -88,12 +89,14 @@ type mockConfig struct {
 	kubeContext           string
 	namespace             string
 	insecureRegistries    map[string]bool
+	runMode               config.RunMode
 	cluster               latest.ClusterDetails
 }
 
 func (c *mockConfig) GetKubeContext() string                 { return c.kubeContext }
 func (c *mockConfig) GetKubeNamespace() string               { return c.namespace }
 func (c *mockConfig) GetInsecureRegistries() map[string]bool { return c.insecureRegistries }
+func (c *mockConfig) Mode() config.RunMode                   { return c.runMode }
 func (c *mockConfig) Pipeline() latest.Pipeline {
 	var pipeline latest.Pipeline
 	pipeline.Build.BuildType.Cluster = &c.cluster

pkg/skaffold/build/dependencies.go

@@ -20,8 +20,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/sirupsen/logrus"
-
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/bazel"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/buildpacks"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/custom"
@@ -32,34 +30,33 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
-// ArtifactResolver provides an interface to resolve built artifact tags by image name.
-type ArtifactResolver interface {
-	GetImageTag(imageName string) (string, bool)
-}
-
 // DependenciesForArtifact returns the dependencies for a given artifact.
-func DependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker.Config, r ArtifactResolver) ([]string, error) {
+func DependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker.Config, r docker.ArtifactResolver) ([]string, error) {
 	var (
 		paths []string
 		err   error
 	)
 
 	switch {
 	case a.DockerArtifact != nil:
-		// Required artifacts cannot be resolved when `CreateBuildArgsFromArtifacts` runs prior to a completed build sequence (like `skaffold build` or the first iteration of `skaffold dev`).
+		// Required artifacts cannot be resolved when `ResolveDependencyImages` runs prior to a completed build sequence (like `skaffold build` or the first iteration of `skaffold dev`).
 		// However it only affects the behavior for Dockerfiles with ONBUILD instructions, and there's no functional change even for those scenarios.
 		// 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 := CreateBuildArgsFromArtifacts(a.Dependencies, r, false)
-
-		args, evalErr := docker.EvalBuildArgs(cfg.Mode(), a.Workspace, a.DockerArtifact, deps)
+		deps := docker.ResolveDependencyImages(a.Dependencies, r, false)
+		args, evalErr := docker.EvalBuildArgs(cfg.Mode(), a.Workspace, a.DockerArtifact.DockerfilePath, a.DockerArtifact.BuildArgs, deps)
 		if evalErr != nil {
 			return nil, fmt.Errorf("unable to evaluate build args: %w", evalErr)
 		}
 		paths, err = docker.GetDependencies(ctx, a.Workspace, a.DockerArtifact.DockerfilePath, args, cfg)
 
 	case a.KanikoArtifact != nil:
-		paths, err = docker.GetDependencies(ctx, a.Workspace, a.KanikoArtifact.DockerfilePath, a.KanikoArtifact.BuildArgs, cfg)
+		deps := docker.ResolveDependencyImages(a.Dependencies, r, false)
+		args, evalErr := docker.EvalBuildArgs(cfg.Mode(), a.Workspace, a.KanikoArtifact.DockerfilePath, a.KanikoArtifact.BuildArgs, deps)
+		if evalErr != nil {
+			return nil, fmt.Errorf("unable to evaluate build args: %w", evalErr)
+		}
+		paths, err = docker.GetDependencies(ctx, a.Workspace, a.KanikoArtifact.DockerfilePath, args, cfg)
 
 	case a.BazelArtifact != nil:
 		paths, err = bazel.GetDependencies(ctx, a.Workspace, a.BazelArtifact)
@@ -83,25 +80,3 @@ func DependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker
 
 	return util.AbsolutePaths(a.Workspace, paths), nil
 }
-
-// CreateBuildArgsFromArtifacts creates docker build args for an artifact from its required artifacts slice.
-// If `missingIsFatal` is false then it is permissive of missing entries in the ArtifactResolver and returns nil for those entries.
-func CreateBuildArgsFromArtifacts(deps []*latest.ArtifactDependency, r ArtifactResolver, missingIsFatal bool) map[string]*string {
-	if r == nil {
-		// `diagnose` is called without an artifact resolver. Return an empty map in this case.
-		return nil
-	}
-	m := make(map[string]*string)
-	for _, d := range deps {
-		t, found := r.GetImageTag(d.ImageName)
-		switch {
-		case found:
-			m[d.Alias] = &t
-		case missingIsFatal:
-			logrus.Fatalf("failed to resolve build result for required artifact %q", d.ImageName)
-		default:
-			m[d.Alias] = nil
-		}
-	}
-	return m
-}

pkg/skaffold/build/docker/docker.go

@@ -23,7 +23,6 @@ import (
 	"os"
 	"os/exec"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
@@ -43,7 +42,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact,
 	if err := b.pullCacheFromImages(ctx, out, a.ArtifactType.DockerArtifact); err != nil {
 		return "", fmt.Errorf("pulling cache-from images: %w", err)
 	}
-	opts := docker.BuildOptions{Tag: tag, Mode: b.mode, ExtraBuildArgs: build.CreateBuildArgsFromArtifacts(a.Dependencies, b.artifacts, true)}
+	opts := docker.BuildOptions{Tag: tag, Mode: b.mode, ExtraBuildArgs: docker.ResolveDependencyImages(a.Dependencies, b.artifacts, true)}
 
 	var imageID string
 
@@ -71,7 +70,7 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, workspace s
 	}
 
 	args := []string{"build", workspace, "--file", dockerfilePath, "-t", opts.Tag}
-	ba, err := docker.EvalBuildArgs(b.mode, workspace, a, opts.ExtraBuildArgs)
+	ba, err := docker.EvalBuildArgs(b.mode, workspace, a.DockerfilePath, a.BuildArgs, opts.ExtraBuildArgs)
 	if err != nil {
 		return "", fmt.Errorf("unable to evaluate build args: %w", err)
 	}

pkg/skaffold/build/docker/docker_test.go

@@ -73,8 +73,8 @@ func TestDockerCLIBuild(t *testing.T) {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.NewTempDir().Touch("Dockerfile").Chdir()
 			dockerfilePath, _ := filepath.Abs("Dockerfile")
-			t.Override(&docker.EvalBuildArgs, func(mode config.RunMode, workspace string, a *latest.DockerArtifact, extra map[string]*string) (map[string]*string, error) {
-				return a.BuildArgs, nil
+			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(&util.DefaultExecCommand, testutil.CmdRunEnv(
 				"docker build . --file "+dockerfilePath+" -t tag --force-rm",

pkg/skaffold/build/local/local_test.go

@@ -242,8 +242,8 @@ func TestLocalRun(t *testing.T) {
 			t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
 				return fakeLocalDaemon(test.api), nil
 			})
-			t.Override(&docker.EvalBuildArgs, func(mode config.RunMode, workspace string, a *latest.DockerArtifact, extra map[string]*string) (map[string]*string, error) {
-				return a.BuildArgs, nil
+			t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) {
+				return args, nil
 			})
 			event.InitializeState(latest.Pipeline{
 				Deploy: latest.DeployConfig{},
@@ -403,8 +403,8 @@ func TestGetArtifactBuilder(t *testing.T) {
 			t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
 				return fakeLocalDaemon(&testutil.FakeAPIClient{}), nil
 			})
-			t.Override(&docker.EvalBuildArgs, func(mode config.RunMode, workspace string, a *latest.DockerArtifact, extra map[string]*string) (map[string]*string, error) {
-				return a.BuildArgs, nil
+			t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) {
+				return args, nil
 			})
 
 			b, err := NewBuilder(&mockConfig{