Get working dir from image and use as relative directory if necessary

Author: Priya Wadhwa

pkg/skaffold/docker/image.go

@@ -110,7 +110,7 @@ func (l *localDaemon) ConfigFile(ctx context.Context, image string) (*v1.ConfigF
 			return nil, err
 		}
 	} else {
-		cfg, err = retrieveRemoteConfig(image)
+		cfg, err = RetrieveRemoteConfig(image)
 		if err != nil {
 			return nil, errors.Wrap(err, "getting remote config")
 		}

pkg/skaffold/docker/remote.go

@@ -61,7 +61,8 @@ func RemoteDigest(identifier string) (string, error) {
 	return h.String(), nil
 }
 
-func retrieveRemoteConfig(identifier string) (*v1.ConfigFile, error) {
+// RetrieveRemoteConfig retrieves the remote config file for an image
+func RetrieveRemoteConfig(identifier string) (*v1.ConfigFile, error) {
 	img, err := remoteImage(identifier)
 	if err != nil {
 		return nil, errors.Wrap(err, "getting image")

pkg/skaffold/runner/dev_test.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/sync"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/watch"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
@@ -327,6 +328,13 @@ func TestDevSync(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
+			originalWorkingDir := sync.WorkingDir
+			sync.WorkingDir = func(image string) (string, error) {
+				return "/", nil
+			}
+			defer func() {
+				sync.WorkingDir = originalWorkingDir
+			}()
 			runner := createRunner(t, test.testBench)
 			runner.Watcher = &TestWatcher{
 				events:    test.watchEvents,

pkg/skaffold/sync/kubectl/kubectl.go

@@ -74,7 +74,7 @@ func copyFileFn(ctx context.Context, pod v1.Pod, container v1.Container, files m
 	// Use "m" flag to touch the files as they are copied.
 	reader, writer := io.Pipe()
 	copy := exec.CommandContext(ctx, "kubectl", "exec", pod.Name, "--namespace", pod.Namespace, "-c", container.Name, "-i",
-		"--", "tar", "xmf", "-", "-C", "/", "--no-same-owner")
+		"--", "tar", "xmf", "-", "--no-same-owner")
 	copy.Stdin = reader
 	go func() {
 		defer writer.Close()

pkg/skaffold/sync/sync.go

@@ -21,12 +21,12 @@ import (
 	"fmt"
 	"os/exec"
 	"path/filepath"
-	"strings"
 
 	"github.com/bmatcuk/doublestar"
 	"github.com/sirupsen/logrus"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
@@ -36,6 +36,11 @@ import (
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var (
+	// For testing
+	WorkingDir = retrieveWorkingDir
+)
+
 type Syncer interface {
 	Sync(context.Context, *Item) error
 }
@@ -52,12 +57,22 @@ func NewItem(a *latest.Artifact, e watch.Events, builds []build.Artifact) (*Item
 		return nil, nil
 	}
 
-	toCopy, err := intersect(a.Workspace, a.Sync, append(e.Added, e.Modified...))
+	tag := latestTag(a.ImageName, builds)
+	if tag == "" {
+		return nil, fmt.Errorf("could not find latest tag for image %s in builds: %v", a.ImageName, builds)
+	}
+
+	wd, err := WorkingDir(tag)
+	if err != nil {
+		return nil, errors.Wrapf(err, "retrieving working dir for %s", tag)
+	}
+
+	toCopy, err := intersect(a.Workspace, a.Sync, append(e.Added, e.Modified...), wd)
 	if err != nil {
 		return nil, errors.Wrap(err, "intersecting sync map and added, modified files")
 	}
 
-	toDelete, err := intersect(a.Workspace, a.Sync, e.Deleted)
+	toDelete, err := intersect(a.Workspace, a.Sync, e.Deleted, wd)
 	if err != nil {
 		return nil, errors.Wrap(err, "intersecting sync map and deleted files")
 	}
@@ -67,18 +82,24 @@ func NewItem(a *latest.Artifact, e watch.Events, builds []build.Artifact) (*Item
 		return nil, nil
 	}
 
-	tag := latestTag(a.ImageName, builds)
-	if tag == "" {
-		return nil, fmt.Errorf("could not find latest tag for image %s in builds: %v", a.ImageName, builds)
-	}
-
 	return &Item{
 		Image:  tag,
 		Copy:   toCopy,
 		Delete: toDelete,
 	}, nil
 }
 
+func retrieveWorkingDir(image string) (string, error) {
+	cf, err := docker.RetrieveRemoteConfig(image)
+	if err != nil {
+		return "", errors.Wrap(err, "retrieving remote config")
+	}
+	if cf.Config.WorkingDir == "" {
+		return "/", nil
+	}
+	return cf.Config.WorkingDir, nil
+}
+
 func latestTag(image string, builds []build.Artifact) string {
 	for _, build := range builds {
 		if build.ImageName == image {
@@ -88,7 +109,7 @@ func latestTag(image string, builds []build.Artifact) string {
 	return ""
 }
 
-func intersect(context string, syncMap map[string]string, files []string) (map[string]string, error) {
+func intersect(context string, syncMap map[string]string, files []string, workingDir string) (map[string]string, error) {
 	ret := map[string]string{}
 
 	for _, f := range files {
@@ -103,18 +124,14 @@ func intersect(context string, syncMap map[string]string, files []string) (map[s
 				return nil, errors.Wrapf(err, "pattern error for %s", relPath)
 			}
 			if match {
-				staticPath := strings.Split(filepath.FromSlash(p), "*")[0]
+				if filepath.IsAbs(dst) {
+					dst = filepath.ToSlash(filepath.Join(dst, filepath.Base(relPath)))
+				} else {
+					dst = filepath.ToSlash(filepath.Join(workingDir, dst, filepath.Base(relPath)))
+				}
 				// Every file must match at least one sync pattern, if not we'll have to
 				// skip the entire sync
 				matches = true
-				// If the source has special match characters,
-				// the destination must be a directory
-				// The path package must be used here to enforce slashes,
-				// since the destination is always a linux filesystem.
-				if util.HasMeta(p) {
-					relPathDynamic := strings.TrimPrefix(relPath, staticPath)
-					dst = filepath.ToSlash(filepath.Join(dst, filepath.Base(relPathDynamic)))
-				}
 				ret[f] = dst
 			}
 		}

pkg/skaffold/sync/sync_test.go

@@ -44,6 +44,7 @@ func TestNewSyncItem(t *testing.T) {
 		builds      []build.Artifact
 		shouldErr   bool
 		expected    *Item
+		workingDir  string
 	}{
 		{
 			description: "match copy",
@@ -142,10 +143,11 @@ func TestNewSyncItem(t *testing.T) {
 			evt: watch.Events{
 				Modified: []string{filepath.Join("node", "src/app/server/server.js")},
 			},
+			workingDir: "/",
 			expected: &Item{
 				Image: "test:123",
 				Copy: map[string]string{
-					filepath.Join("node", "src/app/server/server.js"): "src/server.js",
+					filepath.Join("node", "src/app/server/server.js"): "/src/server.js",
 				},
 				Delete: map[string]string{},
 			},
@@ -193,6 +195,12 @@ func TestNewSyncItem(t *testing.T) {
 				Added:   []string{"main.go"},
 				Deleted: []string{"index.html"},
 			},
+			builds: []build.Artifact{
+				{
+					ImageName: "",
+					Tag:       "placeholder",
+				},
+			},
 		},
 		{
 			description: "not delete syncable",
@@ -206,6 +214,12 @@ func TestNewSyncItem(t *testing.T) {
 				Added:   []string{"index.html"},
 				Deleted: []string{"some/other/file"},
 			},
+			builds: []build.Artifact{
+				{
+					ImageName: "",
+					Tag:       "placeholder",
+				},
+			},
 		},
 		{
 			description: "err bad pattern",
@@ -245,6 +259,7 @@ func TestNewSyncItem(t *testing.T) {
 				},
 				Workspace: ".",
 			},
+			workingDir: "/some/dir",
 			builds: []build.Artifact{
 				{
 					ImageName: "test",
@@ -257,33 +272,7 @@ func TestNewSyncItem(t *testing.T) {
 			expected: &Item{
 				Image: "test:123",
 				Copy: map[string]string{
-					filepath.Join("dir1", "dir2/node.js"): "node.js",
-				},
-				Delete: map[string]string{},
-			},
-		},
-		{
-			description: "copy subdirectory",
-			artifact: &latest.Artifact{
-				ImageName: "test",
-				Sync: map[string]string{
-					"**/*.py": ".",
-				},
-				Workspace: "python",
-			},
-			builds: []build.Artifact{
-				{
-					ImageName: "test",
-					Tag:       "test:123",
-				},
-			},
-			evt: watch.Events{
-				Modified: []string{filepath.Join("src", "app.py")},
-			},
-			expected: &Item{
-				Image: "test:123",
-				Copy: map[string]string{
-					filepath.Join("src", "app.py"): "app.py",
+					filepath.Join("dir1", "dir2/node.js"): "/some/dir/node.js",
 				},
 				Delete: map[string]string{},
 			},
@@ -292,6 +281,13 @@ func TestNewSyncItem(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
+			originalWorkingDir := WorkingDir
+			WorkingDir = func(image string) (string, error) {
+				return test.workingDir, nil
+			}
+			defer func() {
+				WorkingDir = originalWorkingDir
+			}()
 			actual, err := NewItem(test.artifact, test.evt, test.builds)
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, actual)
 		})
@@ -305,6 +301,7 @@ func TestIntersect(t *testing.T) {
 		syncPatterns map[string]string
 		files        []string
 		context      string
+		workingDir   string
 		expected     map[string]string
 		shouldErr    bool
 	}{
@@ -347,7 +344,7 @@ func TestIntersect(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			actual, err := intersect(test.context, test.syncPatterns, test.files)
+			actual, err := intersect(test.context, test.syncPatterns, test.files, test.workingDir)
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, actual)
 		})
 	}