Fix bug in sync

integration/examples/hot-reload/python/src/app.py

@@ -3,4 +3,4 @@
 
 @app.route('/')
 def hello_world():
-    return 'Hello, World from Flask!'
+    return 'Hello, World from Flask!\n'

integration/examples/test-file-sync/skaffold.yaml

@@ -1,4 +1,4 @@
-apiVersion: skaffold/v1beta1
+apiVersion: skaffold/v1beta6
 kind: Config
 build:
   tagPolicy:
@@ -7,7 +7,7 @@ build:
   - image: gcr.io/k8s-skaffold/test-file-sync
     context: .
     sync:
-      '**/foo*' : .
+      '**/foo*' : /test
 deploy:
  kubectl:
    manifests:

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, tagged 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,21 +21,33 @@ import (
 	"fmt"
 	"os/exec"
 	"path/filepath"
-	"strings"
 
 	"github.com/bmatcuk/doublestar"
 	"github.com/sirupsen/logrus"
 
+	"strings"
+
 	"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"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/watch"
+	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+const (
+	lenDigest = 71
+)
+
+var (
+	// WorkingDir is here for testing
+	WorkingDir = retrieveWorkingDir
+)
+
 type Syncer interface {
 	Sync(context.Context, *Item) error
 }
@@ -52,12 +64,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(a.ImageName, 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 +89,42 @@ 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, tagged string) (string, error) {
+	fullyQualifedImage := stripTagIfDigestPresent(image, tagged)
+	cf, err := docker.RetrieveRemoteConfig(fullyQualifedImage)
+	if err != nil {
+		return "", errors.Wrap(err, "retrieving remote config")
+
+	}
+	if cf.Config.WorkingDir == "" {
+		return "/", nil
+	}
+	return cf.Config.WorkingDir, nil
+}
+
+// stripTagIfDigestPresent removes the tag from the image if there is a tag and a digest
+func stripTagIfDigestPresent(image, tagged string) string {
+	// try to parse the reference, return image if it works
+	_, err := name.ParseReference(tagged, name.WeakValidation)
+	if err == nil {
+		return tagged
+	}
+	// strip out the tag
+	digestIndex := strings.Index(tagged, "sha256:")
+	if digestIndex == -1 {
+		return tagged
+	}
+	digest := tagged[digestIndex : digestIndex+lenDigest]
+	return fmt.Sprintf("%s@%s", image, digest)
+}
+
 func latestTag(image string, builds []build.Artifact) string {
 	for _, build := range builds {
 		if build.ImageName == image {
@@ -88,12 +134,11 @@ 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 {
 		relPath, err := filepath.Rel(context, f)
-
 		if err != nil {
 			return nil, errors.Wrapf(err, "changed file %s can't be found relative to context %s", f, context)
 		}
@@ -103,21 +148,15 @@ func intersect(context string, syncMap map[string]string, files []string) (map[s
 			if err != nil {
 				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, 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/app/server/server.js",
+					filepath.Join("node", "src/app/server/server.js"): "/src/server.js",
 				},
 				Delete: map[string]string{},
 			},
@@ -193,6 +195,11 @@ func TestNewSyncItem(t *testing.T) {
 				Added:   []string{"main.go"},
 				Deleted: []string{"index.html"},
 			},
+			builds: []build.Artifact{
+				{
+					Tag: "placeholder",
+				},
+			},
 		},
 		{
 			description: "not delete syncable",
@@ -206,6 +213,11 @@ func TestNewSyncItem(t *testing.T) {
 				Added:   []string{"index.html"},
 				Deleted: []string{"some/other/file"},
 			},
+			builds: []build.Artifact{
+				{
+					Tag: "placeholder",
+				},
+			},
 		},
 		{
 			description: "err bad pattern",
@@ -245,6 +257,7 @@ func TestNewSyncItem(t *testing.T) {
 				},
 				Workspace: ".",
 			},
+			workingDir: "/some/dir",
 			builds: []build.Artifact{
 				{
 					ImageName: "test",
@@ -257,7 +270,7 @@ func TestNewSyncItem(t *testing.T) {
 			expected: &Item{
 				Image: "test:123",
 				Copy: map[string]string{
-					filepath.Join("dir1", "dir2/node.js"): "dir1/dir2/node.js",
+					filepath.Join("dir1", "dir2/node.js"): "/some/dir/node.js",
 				},
 				Delete: map[string]string{},
 			},
@@ -266,6 +279,13 @@ func TestNewSyncItem(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
+			originalWorkingDir := WorkingDir
+			WorkingDir = func(image, tagged 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)
 		})
@@ -279,6 +299,7 @@ func TestIntersect(t *testing.T) {
 		syncPatterns map[string]string
 		files        []string
 		context      string
+		workingDir   string
 		expected     map[string]string
 		shouldErr    bool
 	}{
@@ -321,7 +342,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)
 		})
 	}
@@ -434,3 +455,38 @@ func TestPerform(t *testing.T) {
 		})
 	}
 }
+
+func TestStripTagIfDigestPresent(t *testing.T) {
+	tests := []struct {
+		name     string
+		image    string
+		tagged   string
+		expected string
+	}{
+		{
+			name:     "strip out tag",
+			image:    "gcr.io/test/test",
+			tagged:   "gcr.io/test/test:sometag@sha256:13e30612be9aa5dae72fb06619ff0dabf80ca600e8f949da683abe73d2f0113b",
+			expected: "gcr.io/test/test@sha256:13e30612be9aa5dae72fb06619ff0dabf80ca600e8f949da683abe73d2f0113b",
+		},
+		{
+			name:     "only tag provided",
+			image:    "gcr.io/test/test",
+			tagged:   "gcr.io/test/test:sometag",
+			expected: "gcr.io/test/test:sometag",
+		},
+		{
+			name:     "only digest provided",
+			image:    "gcr.io/test/test",
+			tagged:   "gcr.io/test/test@sha256:13e30612be9aa5dae72fb06619ff0dabf80ca600e8f949da683abe73d2f0113b",
+			expected: "gcr.io/test/test@sha256:13e30612be9aa5dae72fb06619ff0dabf80ca600e8f949da683abe73d2f0113b",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual := stripTagIfDigestPresent(test.image, test.tagged)
+			testutil.CheckErrorAndDeepEqual(t, false, nil, test.expected, actual)
+		})
+	}
+}