[buildpacks] Refactor code to simplify #3395

pkg/skaffold/build/buildpacks/build.go

@@ -27,8 +27,8 @@ import (
 
 // Build builds an artifact with Cloud Native Buildpacks:
 // https://buildpacks.io/
-func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) (string, error) {
-	built, err := b.build(ctx, out, a.Workspace, a.BuildpackArtifact, tag)
+func (b *Builder) Build(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
+	built, err := b.build(ctx, out, artifact, tag)
 	if err != nil {
 		return "", err
 	}

pkg/skaffold/build/buildpacks/lifecycle.go

@@ -33,7 +33,10 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
-func (b *Builder) build(ctx context.Context, out io.Writer, workspace string, artifact *latest.BuildpackArtifact, tag string) (string, error) {
+func (b *Builder) build(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) (string, error) {
+	artifact := a.BuildpackArtifact
+	workspace := a.Workspace
+
 	// To improve caching, we always build the image with [:latest] tag
 	// This way, the lifecycle is able to "bootstrap" from the previously built image.
 	// The image will then be tagged as usual with the tag provided by the tag policy.

pkg/skaffold/docker/image_util.go

@@ -27,14 +27,14 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 )
 
-func RetrieveWorkingDir(tagged string, insecureRegistries map[string]bool) (string, error) {
-	var cf *v1.ConfigFile
-	var err error
-
+func RetrieveConfigFile(tagged string, insecureRegistries map[string]bool) (*v1.ConfigFile, error) {
 	if strings.ToLower(tagged) == "scratch" {
-		return "/", nil
+		return nil, nil
 	}
 
+	var cf *v1.ConfigFile
+	var err error
+
 	// TODO: use the proper RunContext
 	localDocker, err := NewAPIClient(&runcontext.RunContext{})
 	if err == nil {
@@ -45,12 +45,23 @@ func RetrieveWorkingDir(tagged string, insecureRegistries map[string]bool) (stri
 		cf, err = RetrieveRemoteConfig(tagged, insecureRegistries)
 	}
 	if err != nil {
-		return "", errors.Wrap(err, "retrieving image config")
+		return nil, errors.Wrap(err, "retrieving image config")
 	}
 
-	if cf.Config.WorkingDir == "" {
+	return cf, err
+}
+
+func RetrieveWorkingDir(tagged string, insecureRegistries map[string]bool) (string, error) {
+	cf, err := RetrieveConfigFile(tagged, insecureRegistries)
+	switch {
+	case err != nil:
+		return "", err
+	case cf == nil:
+		return "/", nil
+	case cf.Config.WorkingDir == "":
 		logrus.Debugf("Using default workdir '/' for %s", tagged)
 		return "/", nil
+	default:
+		return cf.Config.WorkingDir, nil
 	}
-	return cf.Config.WorkingDir, nil
 }

pkg/skaffold/runner/dev_test.go

@@ -342,10 +342,10 @@ func TestDevSync(t *testing.T) {
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			var actualFileSyncEventCalls fileSyncEventCalls
+			t.SetupFakeKubernetesContext(api.Config{CurrentContext: "cluster1"})
 			t.Override(&fileSyncInProgress, func(int, string) { actualFileSyncEventCalls.InProgress++ })
 			t.Override(&fileSyncFailed, func(int, string, error) { actualFileSyncEventCalls.Failed++ })
 			t.Override(&fileSyncSucceeded, func(int, string) { actualFileSyncEventCalls.Succeeded++ })
-			t.SetupFakeKubernetesContext(api.Config{CurrentContext: "cluster1"})
 			t.Override(&sync.WorkingDir, func(string, map[string]bool) (string, error) { return "/", nil })
 			test.testBench.cycles = len(test.watchEvents)
 

pkg/skaffold/sync/sync.go

@@ -45,19 +45,19 @@ var (
 )
 
 func NewItem(a *latest.Artifact, e filemon.Events, builds []build.Artifact, insecureRegistries map[string]bool, destProvider DestinationProvider) (*Item, error) {
-	if !e.HasChanged() || a.Sync == nil {
+	switch {
+	case !e.HasChanged():
 		return nil, nil
-	}
 
-	if len(a.Sync.Manual) > 0 {
+	case a.Sync != nil && len(a.Sync.Manual) > 0:
 		return manualSyncItem(a, e, builds, insecureRegistries)
-	}
 
-	if len(a.Sync.Infer) > 0 {
+	case a.Sync != nil && len(a.Sync.Infer) > 0:
 		return inferredSyncItem(a, e, builds, destProvider)
-	}
 
-	return nil, nil
+	default:
+		return nil, nil
+	}
 }
 
 func manualSyncItem(a *latest.Artifact, e filemon.Events, builds []build.Artifact, insecureRegistries map[string]bool) (*Item, error) {

pkg/skaffold/sync/sync_test.go

@@ -637,9 +637,7 @@ func TestNewSyncItem(t *testing.T) {
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
-			t.Override(&WorkingDir, func(string, map[string]bool) (string, error) {
-				return test.workingDir, nil
-			})
+			t.Override(&WorkingDir, func(string, map[string]bool) (string, error) { return test.workingDir, nil })
 
 			provider := func() (map[string][]string, error) { return test.dependencies, nil }
 			actual, err := NewItem(test.artifact, test.evt, test.builds, nil, provider)