Fix for policy manager not setting labels in some cases

cli/command_policy_remove.go

@@ -11,6 +11,7 @@ var (
 	policyRemoveCommand = policyCommands.Command("remove", "Remove snapshot policy for a single directory, user@host or a global policy.").Alias("rm").Alias("delete")
 	policyRemoveTargets = policyRemoveCommand.Arg("target", "Target of a policy ('global','user@host','@host') or a path").Strings()
 	policyRemoveGlobal  = policyRemoveCommand.Flag("global", "Set global policy").Bool()
+	policyRemoveDryRun  = policyRemoveCommand.Flag("dry-run", "Do not remove").Short('n').Bool()
 )
 
 func init() {
@@ -26,6 +27,10 @@ func removePolicy(ctx context.Context, rep repo.Repository) error {
 	for _, target := range targets {
 		log(ctx).Infof("Removing policy on %q...", target)
 
+		if *policyRemoveDryRun {
+			continue
+		}
+
 		if err := policy.RemovePolicy(ctx, rep, target); err != nil {
 			return err
 		}

go.mod

@@ -17,6 +17,7 @@ require (
 	github.com/gofrs/flock v0.7.1
 	github.com/golang/protobuf v1.4.2
 	github.com/google/fswalker v0.2.1-0.20200214223026-f0e929ba4126
+	github.com/google/go-cmp v0.4.0
 	github.com/google/readahead v0.0.0-20161222183148-eaceba169032 // indirect
 	github.com/google/wire v0.4.0 // indirect
 	github.com/gorilla/mux v1.7.4

snapshot/policy/policy_manager.go

@@ -3,7 +3,6 @@ package policy
 
 import (
 	"context"
-	"path/filepath"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -36,7 +35,7 @@ func GetEffectivePolicy(ctx context.Context, rep repo.Repository, si snapshot.So
 
 		md = append(md, manifests...)
 
-		parentPath := filepath.Dir(tmp.Path)
+		parentPath := getParentPathOSIndependent(tmp.Path)
 		if parentPath == tmp.Path {
 			break
 		}
@@ -72,11 +71,10 @@ func GetEffectivePolicy(ctx context.Context, rep repo.Repository, si snapshot.So
 
 	for _, em := range md {
 		p := &Policy{}
-		if _, err := rep.GetManifest(ctx, em.ID, &p); err != nil {
+		if err := loadPolicyFromManifest(ctx, rep, em.ID, p); err != nil {
 			return nil, nil, errors.Wrapf(err, "got unexpected error when loading policy item %v", em.ID)
 		}
 
-		p.Labels = em.Labels
 		policies = append(policies, p)
 		log(ctx).Debugf("loaded parent policy for %v: %v", si, p.Target())
 	}
@@ -106,18 +104,10 @@ func GetDefinedPolicy(ctx context.Context, rep repo.Repository, si snapshot.Sour
 
 	p := &Policy{}
 
-	em, err := rep.GetManifest(ctx, manifestID, p)
-
-	if err != nil {
-		if err == manifest.ErrNotFound {
-			return nil, ErrPolicyNotFound
-		}
-
+	if err := loadPolicyFromManifest(ctx, rep, manifestID, p); err != nil {
 		return nil, err
 	}
 
-	p.Labels = em.Labels
-
 	return p, nil
 }
 
@@ -160,10 +150,8 @@ func RemovePolicy(ctx context.Context, rep repo.Repository, si snapshot.SourceIn
 // GetPolicyByID gets the policy for a given unique ID or ErrPolicyNotFound if not found.
 func GetPolicyByID(ctx context.Context, rep repo.Repository, id manifest.ID) (*Policy, error) {
 	p := &Policy{}
-	if _, err := rep.GetManifest(ctx, id, &p); err != nil {
-		if err == manifest.ErrNotFound {
-			return nil, ErrPolicyNotFound
-		}
+	if err := loadPolicyFromManifest(ctx, rep, id, p); err != nil {
+		return nil, err
 	}
 
 	return p, nil
@@ -183,13 +171,10 @@ func ListPolicies(ctx context.Context, rep repo.Repository) ([]*Policy, error) {
 	for _, id := range ids {
 		pol := &Policy{}
 
-		md, err := rep.GetManifest(ctx, id.ID, pol)
-		if err != nil {
+		if err := loadPolicyFromManifest(ctx, rep, id.ID, pol); err != nil {
 			return nil, err
 		}
 
-		pol.Labels = md.Labels
-		pol.Labels["id"] = string(id.ID)
 		policies = append(policies, pol)
 	}
 
@@ -206,6 +191,15 @@ func (m SubdirectoryPolicyMap) GetPolicyForPath(relativePath string) (*Policy, e
 
 // TreeForSource returns policy Tree for a given source.
 func TreeForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo) (*Tree, error) {
+	pols, err := applicablePoliciesForSource(ctx, rep, si)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to get policies")
+	}
+
+	return BuildTree(pols, DefaultPolicy), nil
+}
+
+func applicablePoliciesForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo) (map[string]*Policy, error) {
 	result := map[string]*Policy{}
 
 	pol, _, err := GetEffectivePolicy(ctx, rep, si)
@@ -226,34 +220,47 @@ func TreeForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceI
 		return nil, errors.Wrapf(err, "unable to find manifests for %v@%v", si.UserName, si.Host)
 	}
 
-	log(ctx).Debugf("found %v policies for %v@%v", si.UserName, si.Host)
+	log(ctx).Debugf("found %v policies for %v@%v", len(policies), si.UserName, si.Host)
 
 	for _, id := range policies {
 		pol := &Policy{}
 
-		em, err := rep.GetManifest(ctx, id.ID, pol)
+		err := loadPolicyFromManifest(ctx, rep, id.ID, pol)
 		if err != nil {
 			return nil, err
 		}
 
-		policyPath := em.Labels["path"]
+		policyPath := pol.Labels["path"]
 
-		if !strings.HasPrefix(policyPath, si.Path+"/") {
+		rel := nestedRelativePathNormalizedToSlashes(si.Path, policyPath)
+		if rel == "" {
+			log(ctx).Debugf("%v is not under %v", policyPath, si.Path)
 			continue
 		}
 
-		rel, err := filepath.Rel(si.Path, policyPath)
-		if err != nil {
-			return nil, errors.Wrap(err, "unable to determine relative path")
-		}
-
 		rel = "./" + rel
 		log(ctx).Debugf("loading policy for %v (%v)", policyPath, rel)
 
 		result[rel] = pol
 	}
 
-	return BuildTree(result, DefaultPolicy), nil
+	return result, nil
+}
+
+func loadPolicyFromManifest(ctx context.Context, rep repo.Repository, id manifest.ID, pol *Policy) error {
+	md, err := rep.GetManifest(ctx, id, pol)
+	if err != nil {
+		if errors.Is(err, manifest.ErrNotFound) {
+			return ErrPolicyNotFound
+		}
+
+		return err
+	}
+
+	pol.Labels = md.Labels
+	pol.Labels["id"] = string(md.ID)
+
+	return nil
 }
 
 func labelsForSource(si snapshot.SourceInfo) map[string]string {
@@ -286,3 +293,65 @@ func labelsForSource(si snapshot.SourceInfo) map[string]string {
 		}
 	}
 }
+
+func getParentPathOSIndependent(p string) string {
+	// split into volume (Windows only, e.g. X:) and path using either slash or backslash.
+	vol, pth := volumeAndPath(p)
+
+	last := strings.LastIndexAny(pth, "/\\")
+	if last == len(pth)-1 && last != 0 {
+		pth = pth[0:last]
+		last = strings.LastIndexAny(pth, "/\\")
+	}
+
+	if last < 0 {
+		return p
+	}
+
+	// special case for root, return root path itself (either slash or backslash-separated)
+	if last == 0 {
+		return vol + pth[0:1]
+	}
+
+	return vol + pth[0:last]
+}
+
+// volumeAndPath splits path 'p' into Windows-specific volume (e.g. "X:" and path after that starting with either slash or backslash)
+func volumeAndPath(p string) (vol, path string) {
+	if len(p) >= 3 && p[1] == ':' && isSlashOrBackslash(p[2]) {
+		// "X:\"
+		return p[0:2], p[2:]
+	}
+
+	return "", p
+}
+
+func isSlashOrBackslash(c uint8) bool {
+	return c == '/' || c == '\\'
+}
+
+func isWindowsStylePath(p string) bool {
+	v, _ := volumeAndPath(p)
+	return v != ""
+}
+
+func trimTrailingSlashOrBackslash(path string) string {
+	return strings.TrimSuffix(strings.TrimSuffix(path, "/"), "\\")
+}
+
+func nestedRelativePathNormalizedToSlashes(parent, child string) string {
+	isWin := isWindowsStylePath(parent)
+	parent = trimTrailingSlashOrBackslash(parent)
+
+	if !strings.HasPrefix(child, parent+"/") && !strings.HasPrefix(child, parent+"\\") {
+		return ""
+	}
+
+	p := strings.TrimPrefix(child, parent)[1:]
+
+	if isWin {
+		return strings.ReplaceAll(p, "\\", "/")
+	}
+
+	return p
+}