Move key from dockerfilePath to artifact name

pkg/skaffold/build/cache/retrieve_test.go

@@ -66,7 +66,7 @@ func (b *mockBuilder) BuildAndTest(ctx context.Context, out io.Writer, tags tag.
 		b.built = append(b.built, artifact)
 		tag := tags[artifact.ImageName]
 		opts := docker.BuildOptions{Tag: tag, Mode: config.RunModes.Dev}
-		_, err := b.dockerDaemon.Build(ctx, out, artifact.Workspace, artifact.DockerArtifact, opts)
+		_, err := b.dockerDaemon.Build(ctx, out, artifact.Workspace, artifact.ImageName, artifact.DockerArtifact, opts)
 		if err != nil {
 			return nil, err
 		}

pkg/skaffold/build/cluster/kaniko.go

@@ -104,10 +104,8 @@ func (b *Builder) copyKanikoBuildContext(ctx context.Context, workspace string,
 
 	buildCtx, buildCtxWriter := io.Pipe()
 	go func() {
-		err := docker.CreateDockerTarContext(ctx, buildCtxWriter, workspace, &latest.DockerArtifact{
-			BuildArgs:      artifact.BuildArgs,
-			DockerfilePath: artifact.DockerfilePath,
-		}, b.cfg)
+		err := docker.CreateDockerTarContext(ctx, buildCtxWriter, docker.NewBuildConfig(
+			workspace, artifact.Image, artifact.DockerfilePath, artifact.BuildArgs), b.cfg)
 		if err != nil {
 			buildCtxWriter.CloseWithError(fmt.Errorf("creating docker context: %w", err))
 			return

pkg/skaffold/build/custom/dependencies.go

@@ -30,11 +30,10 @@ import (
 )
 
 // GetDependencies returns dependencies listed for a custom artifact
-func GetDependencies(ctx context.Context, workspace string, a *latest.CustomArtifact, cfg docker.Config) ([]string, error) {
+func GetDependencies(ctx context.Context, workspace string, artifactName string, a *latest.CustomArtifact, cfg docker.Config) ([]string, error) {
 	switch {
 	case a.Dependencies.Dockerfile != nil:
-		dockerfile := a.Dependencies.Dockerfile
-		return docker.GetDependencies(ctx, workspace, dockerfile.Path, dockerfile.BuildArgs, cfg)
+		return docker.GetDependencies(ctx, getDockerBuildConfig(workspace, artifactName, a), cfg)
 
 	case a.Dependencies.Command != "":
 		split := strings.Split(a.Dependencies.Command, " ")
@@ -53,3 +52,8 @@ func GetDependencies(ctx context.Context, workspace string, a *latest.CustomArti
 		return list.Files(workspace, a.Dependencies.Paths, a.Dependencies.Ignore)
 	}
 }
+
+func getDockerBuildConfig(ws string, artifact string, a *latest.CustomArtifact) docker.BuildConfig {
+	dockerfile := a.Dependencies.Dockerfile
+	return docker.NewBuildConfig(ws, artifact, dockerfile.Path, dockerfile.BuildArgs)
+}

pkg/skaffold/build/custom/dependencies_test.go

@@ -50,7 +50,7 @@ func TestGetDependenciesDockerfile(t *testing.T) {
 	}
 
 	expected := []string{"Dockerfile", filepath.FromSlash("baz/file"), "foo"}
-	deps, err := GetDependencies(context.Background(), tmpDir.Root(), customArtifact, nil)
+	deps, err := GetDependencies(context.Background(), tmpDir.Root(), "test", customArtifact, nil)
 
 	testutil.CheckErrorAndDeepEqual(t, false, err, expected, deps)
 }
@@ -69,7 +69,7 @@ func TestGetDependenciesCommand(t *testing.T) {
 		}
 
 		expected := []string{"file1", "file2", "file3"}
-		deps, err := GetDependencies(context.Background(), "", customArtifact, nil)
+		deps, err := GetDependencies(context.Background(), "", "test", customArtifact, nil)
 
 		t.CheckNoError(err)
 		t.CheckDeepEqual(expected, deps)
@@ -119,7 +119,7 @@ func TestGetDependenciesPaths(t *testing.T) {
 			tmpDir := t.NewTempDir().
 				Touch("foo", "bar", "baz/file")
 
-			deps, err := GetDependencies(context.Background(), tmpDir.Root(), &latest.CustomArtifact{
+			deps, err := GetDependencies(context.Background(), tmpDir.Root(), "test", &latest.CustomArtifact{
 				Dependencies: &latest.CustomDependencies{
 					Paths:  test.paths,
 					Ignore: test.ignore,

pkg/skaffold/build/dependencies.go

@@ -48,15 +48,15 @@ func DependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker
 		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)
+		paths, err = docker.GetDependencies(ctx, docker.NewBuildConfig(a.Workspace, a.ImageName, a.DockerArtifact.DockerfilePath, args), cfg)
 
 	case a.KanikoArtifact != nil:
 		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)
+		paths, err = docker.GetDependencies(ctx, docker.NewBuildConfig(a.Workspace, a.ImageName, a.KanikoArtifact.DockerfilePath, args), cfg)
 
 	case a.BazelArtifact != nil:
 		paths, err = bazel.GetDependencies(ctx, a.Workspace, a.BazelArtifact)
@@ -65,7 +65,7 @@ func DependenciesForArtifact(ctx context.Context, a *latest.Artifact, cfg docker
 		paths, err = jib.GetDependencies(ctx, a.Workspace, a.JibArtifact)
 
 	case a.CustomArtifact != nil:
-		paths, err = custom.GetDependencies(ctx, a.Workspace, a.CustomArtifact, cfg)
+		paths, err = custom.GetDependencies(ctx, a.Workspace, a.ImageName, a.CustomArtifact, cfg)
 
 	case a.BuildpackArtifact != nil:
 		paths, err = buildpacks.GetDependencies(ctx, a.Workspace, a.BuildpackArtifact)

pkg/skaffold/build/docker/docker.go

@@ -50,7 +50,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact,
 	if b.useCLI {
 		imageID, err = b.dockerCLIBuild(ctx, color.GetWriter(out), a.Workspace, dockerfile, a.ArtifactType.DockerArtifact, opts)
 	} else {
-		imageID, err = b.localDocker.Build(ctx, out, a.Workspace, a.ArtifactType.DockerArtifact, opts)
+		imageID, err = b.localDocker.Build(ctx, out, a.Workspace, a.ImageName, a.ArtifactType.DockerArtifact, opts)
 	}
 
 	if err != nil {

pkg/skaffold/diagnose/diagnose.go

@@ -136,7 +136,8 @@ func timeToComputeMTimes(deps []string) (time.Duration, error) {
 func sizeOfDockerContext(ctx context.Context, a *latest.Artifact, cfg docker.Config) (int64, error) {
 	buildCtx, buildCtxWriter := io.Pipe()
 	go func() {
-		err := docker.CreateDockerTarContext(ctx, buildCtxWriter, a.Workspace, a.DockerArtifact, cfg)
+		err := docker.CreateDockerTarContext(ctx, buildCtxWriter, docker.NewBuildConfig(
+			a.Workspace, a.ImageName, a.DockerArtifact.DockerfilePath, nil), cfg)
 		if err != nil {
 			buildCtxWriter.CloseWithError(fmt.Errorf("creating docker context: %w", err))
 			return

pkg/skaffold/docker/context.go

@@ -22,22 +22,21 @@ import (
 	"io"
 	"path/filepath"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
-func CreateDockerTarContext(ctx context.Context, w io.Writer, workspace string, a *latest.DockerArtifact, cfg Config) error {
-	paths, err := GetDependenciesCached(ctx, workspace, a.DockerfilePath, a.BuildArgs, cfg)
+func CreateDockerTarContext(ctx context.Context, w io.Writer, buildCfg BuildConfig, cfg Config) error {
+	paths, err := GetDependenciesCached(ctx, buildCfg, cfg)
 	if err != nil {
 		return fmt.Errorf("getting relative tar paths: %w", err)
 	}
 
 	var p []string
 	for _, path := range paths {
-		p = append(p, filepath.Join(workspace, path))
+		p = append(p, filepath.Join(buildCfg.workspace, path))
 	}
 
-	if err := util.CreateTar(w, workspace, p); err != nil {
+	if err := util.CreateTar(w, buildCfg.workspace, p); err != nil {
 		return fmt.Errorf("creating tar gz: %w", err)
 	}
 

pkg/skaffold/docker/context_test.go

@@ -46,7 +46,7 @@ func TestDockerContext(t *testing.T) {
 
 			reader, writer := io.Pipe()
 			go func() {
-				err := CreateDockerTarContext(context.Background(), writer, dir, artifact, nil)
+				err := CreateDockerTarContext(context.Background(), writer, NewBuildConfig(dir, "test", artifact.DockerfilePath, artifact.BuildArgs), nil)
 				if err != nil {
 					writer.CloseWithError(err)
 				} else {

pkg/skaffold/docker/dependencies.go

@@ -33,7 +33,25 @@ var (
 	dependencyCache = util.NewSyncStore()
 )
 
-// NormalizeDockerfilePath returns the absolute path to the dockerfile.
+// BuildConfig encapsulates all the build configuration required for performing a dockerbuild.
+type BuildConfig struct {
+	workspace      string
+	artifact       string
+	dockerfilePath string
+	args           map[string]*string
+}
+
+// NewBuildConfig returns a `BuildConfig` for a dockerfilePath build.
+func NewBuildConfig(ws string, a string, path string, args map[string]*string) BuildConfig {
+	return BuildConfig{
+		workspace:      ws,
+		artifact:       a,
+		dockerfilePath: path,
+		args:           args,
+	}
+}
+
+// NormalizeDockerfilePath returns the absolute path to the dockerfilePath.
 func NormalizeDockerfilePath(context, dockerfile string) (string, error) {
 	// Expected case: should be found relative to the context directory.
 	// If it does not exist, check if it's found relative to the current directory in case it's shared.
@@ -50,26 +68,26 @@ func NormalizeDockerfilePath(context, dockerfile string) (string, error) {
 // GetDependencies finds the sources dependency for the given docker artifact.
 // it caches the results for the computed dependency which can be used by `GetDependenciesCached`
 // All paths are relative to the workspace.
-func GetDependencies(ctx context.Context, workspace string, dockerfilePath string, buildArgs map[string]*string, cfg Config) ([]string, error) {
-	absDockerfilePath, err := NormalizeDockerfilePath(workspace, dockerfilePath)
+func GetDependencies(ctx context.Context, buildCfg BuildConfig, cfg Config) ([]string, error) {
+	absDockerfilePath, err := NormalizeDockerfilePath(buildCfg.workspace, buildCfg.dockerfilePath)
 	if err != nil {
-		return nil, fmt.Errorf("normalizing dockerfile path: %w", err)
+		return nil, fmt.Errorf("normalizing dockerfilePath path: %w", err)
 	}
-	result := getDependencies(workspace, dockerfilePath, absDockerfilePath, buildArgs, cfg)
-	dependencyCache.Store(absDockerfilePath, result)
+	result := getDependencies(buildCfg.workspace, buildCfg.dockerfilePath, absDockerfilePath, buildCfg.args, cfg)
+	dependencyCache.Store(buildCfg.artifact, result)
 	return resultPair(result)
 }
 
 // GetDependenciesCached reads from cache finds the sources dependency for the given docker artifact.
 // All paths are relative to the workspace.
-func GetDependenciesCached(ctx context.Context, workspace string, dockerfilePath string, buildArgs map[string]*string, cfg Config) ([]string, error) {
-	absDockerfilePath, err := NormalizeDockerfilePath(workspace, dockerfilePath)
+func GetDependenciesCached(ctx context.Context, buildCfg BuildConfig, cfg Config) ([]string, error) {
+	absDockerfilePath, err := NormalizeDockerfilePath(buildCfg.workspace, buildCfg.dockerfilePath)
 	if err != nil {
-		return nil, fmt.Errorf("normalizing dockerfile path: %w", err)
+		return nil, fmt.Errorf("normalizing dockerfilePath path: %w", err)
 	}
 
-	deps := dependencyCache.Exec(absDockerfilePath, func() interface{} {
-		return getDependencies(workspace, dockerfilePath, absDockerfilePath, buildArgs, cfg)
+	deps := dependencyCache.Exec(buildCfg.artifact, func() interface{} {
+		return getDependencies(buildCfg.workspace, buildCfg.dockerfilePath, absDockerfilePath, buildCfg.args, cfg)
 	})
 	return resultPair(deps)
 }

pkg/skaffold/docker/dependencies_test.go

@@ -577,7 +577,7 @@ func TestGetDependencies(t *testing.T) {
 			}
 
 			workspace := tmpDir.Path(test.workspace)
-			deps, err := GetDependencies(context.Background(), workspace, "Dockerfile", test.buildArgs, nil)
+			deps, err := GetDependencies(context.Background(), NewBuildConfig(workspace, "test", "Dockerfile", test.buildArgs), nil)
 
 			t.CheckError(test.shouldErr, err)
 			t.CheckDeepEqual(test.expected, deps)
@@ -674,23 +674,22 @@ func TestGetDependenciesCached(t *testing.T) {
 			retrieveImgMock: func(_ string, _ Config) (*v1.ConfigFile, error) {
 				return nil, fmt.Errorf("unexpected call")
 			},
-			dependencyCache: map[string]interface{}{"Dockerfile": []string{"random.go"}},
+			dependencyCache: map[string]interface{}{"dummy": []string{"random.go"}},
 			expected:        []string{"random.go"},
 		},
 		{
 			description: "with cached results is error getDependencies should read from cache",
 			retrieveImgMock: func(_ string, _ Config) (*v1.ConfigFile, error) {
 				return &v1.ConfigFile{}, nil
 			},
-			dependencyCache: map[string]interface{}{"Dockerfile": fmt.Errorf("remote manifest fetch")},
+			dependencyCache: map[string]interface{}{"dummy": fmt.Errorf("remote manifest fetch")},
 			shouldErr:       true,
 		},
 		{
 			description:     "with cached results for dockerfile in another app",
 			retrieveImgMock: imageFetcher.fetch,
-			dependencyCache: map[string]interface{}{
-				filepath.Join("app", "Dockerfile"): []string{"random.go"}},
-			expected: []string{"Dockerfile", "server.go"},
+			dependencyCache: map[string]interface{}{"another": []string{"random.go"}},
+			expected:        []string{"Dockerfile", "server.go"},
 		},
 	}
 
@@ -704,11 +703,11 @@ func TestGetDependenciesCached(t *testing.T) {
 			tmpDir.Write("Dockerfile", copyServerGo)
 
 			for k, v := range test.dependencyCache {
-				dependencyCache.Exec(tmpDir.Path(k), func() interface{} {
+				dependencyCache.Exec(k, func() interface{} {
 					return v
 				})
 			}
-			deps, err := GetDependenciesCached(context.Background(), tmpDir.Root(), "Dockerfile", map[string]*string{}, nil)
+			deps, err := GetDependenciesCached(context.Background(), NewBuildConfig(tmpDir.Root(), "dummy", "Dockerfile", map[string]*string{}), nil)
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, deps)
 		})
 	}

pkg/skaffold/docker/image.go

@@ -64,7 +64,7 @@ type LocalDaemon interface {
 	ExtraEnv() []string
 	ServerVersion(ctx context.Context) (types.Version, error)
 	ConfigFile(ctx context.Context, image string) (*v1.ConfigFile, error)
-	Build(ctx context.Context, out io.Writer, workspace string, a *latest.DockerArtifact, opts BuildOptions) (string, error)
+	Build(ctx context.Context, out io.Writer, workspace string, artifact string, a *latest.DockerArtifact, opts BuildOptions) (string, error)
 	Push(ctx context.Context, out io.Writer, ref string) (string, error)
 	Pull(ctx context.Context, out io.Writer, ref string) error
 	Load(ctx context.Context, out io.Writer, input io.Reader, ref string) (string, error)
@@ -174,7 +174,7 @@ func (l *localDaemon) CheckCompatible(a *latest.DockerArtifact) error {
 }
 
 // Build performs a docker build and returns the imageID.
-func (l *localDaemon) Build(ctx context.Context, out io.Writer, workspace string, a *latest.DockerArtifact, opts BuildOptions) (string, error) {
+func (l *localDaemon) Build(ctx context.Context, out io.Writer, workspace string, artifact string, a *latest.DockerArtifact, opts BuildOptions) (string, error) {
 	logrus.Debugf("Running docker build: context: %s, dockerfile: %s", workspace, a.DockerfilePath)
 
 	if err := l.CheckCompatible(a); err != nil {
@@ -191,10 +191,8 @@ func (l *localDaemon) Build(ctx context.Context, out io.Writer, workspace string
 
 	buildCtx, buildCtxWriter := io.Pipe()
 	go func() {
-		err := CreateDockerTarContext(ctx, buildCtxWriter, workspace, &latest.DockerArtifact{
-			DockerfilePath: a.DockerfilePath,
-			BuildArgs:      buildArgs,
-		}, l.cfg)
+		err := CreateDockerTarContext(ctx, buildCtxWriter,
+			NewBuildConfig(workspace, artifact, a.DockerfilePath, buildArgs), l.cfg)
 		if err != nil {
 			buildCtxWriter.CloseWithError(fmt.Errorf("creating docker context: %w", err))
 			return

pkg/skaffold/docker/image_test.go

@@ -200,7 +200,7 @@ func TestBuild(t *testing.T) {
 
 			localDocker := NewLocalDaemon(test.api, nil, false, nil)
 			opts := BuildOptions{Tag: "finalimage", Mode: test.mode}
-			_, err := localDocker.Build(context.Background(), ioutil.Discard, test.workspace, test.artifact, opts)
+			_, err := localDocker.Build(context.Background(), ioutil.Discard, test.workspace, "final-image", test.artifact, opts)
 
 			if test.shouldErr {
 				t.CheckErrorContains(test.expectedError, err)

pkg/skaffold/util/store.go

@@ -56,7 +56,7 @@ func (o *SyncStore) Exec(key string, f func() interface{}) interface{} {
 
 // Store will store the results for a key in a cache
 // This function is not safe to use if multiple subroutines store the
-// result for the same artifact.
+// result for the same key.
 func (o *SyncStore) Store(key string, r interface{}) {
 	o.results.Store(key, r)
 }