diff --git a/cli/command_policy_remove.go b/cli/command_policy_remove.go index 2f9994ce3..daf32b40d 100644 --- a/cli/command_policy_remove.go +++ b/cli/command_policy_remove.go @@ -11,6 +11,7 @@ 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 } diff --git a/go.mod b/go.mod index 1d45a48ec..739bc6e4c 100644 --- a/go.mod +++ b/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 diff --git a/snapshot/policy/policy_manager.go b/snapshot/policy/policy_manager.go index b8b4cd991..b66d783c4 100644 --- a/snapshot/policy/policy_manager.go +++ b/snapshot/policy/policy_manager.go @@ -3,7 +3,6 @@ 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 +} diff --git a/snapshot/policy/policy_manager_test.go b/snapshot/policy/policy_manager_test.go index c5e617c0b..29d70265e 100644 --- a/snapshot/policy/policy_manager_test.go +++ b/snapshot/policy/policy_manager_test.go @@ -2,12 +2,170 @@ import ( "context" + "encoding/json" + "fmt" + "sort" "testing" + "github.com/google/go-cmp/cmp" + "github.com/kopia/kopia/internal/repotesting" + "github.com/kopia/kopia/internal/testlogging" + "github.com/kopia/kopia/snapshot" ) -func TestPolicyManager(t *testing.T) { +func TestPolicyManagerInheritanceTest(t *testing.T) { + ctx := context.Background() + + var env repotesting.Environment + + defer env.Setup(t).Close(ctx, t) + + defaultPolicyWithLabels := policyWithLabels(DefaultPolicy, map[string]string{ + "policyType": "global", + "type": "policy", + }) + + must(t, SetPolicy(ctx, env.Repository, snapshot.SourceInfo{ + Host: "host-a", + }, &Policy{ + RetentionPolicy: RetentionPolicy{ + KeepDaily: intPtr(44), + }, + })) + + must(t, SetPolicy(ctx, env.Repository, snapshot.SourceInfo{ + Host: "host-a", + UserName: "myuser", + Path: "/some/path2", + }, &Policy{ + RetentionPolicy: RetentionPolicy{ + KeepDaily: intPtr(66), + }, + })) + + must(t, SetPolicy(ctx, env.Repository, snapshot.SourceInfo{ + Host: "host-b", + }, &Policy{ + RetentionPolicy: RetentionPolicy{ + KeepDaily: intPtr(55), + }, + })) + + cases := []struct { + sourceInfo snapshot.SourceInfo + wantEffective *Policy + wantSources []snapshot.SourceInfo + }{ + { + sourceInfo: GlobalPolicySourceInfo, + wantEffective: defaultPolicyWithLabels, + }, + { + sourceInfo: snapshot.SourceInfo{ + UserName: "myuser", + Host: "host-c", + Path: "/some/path", + }, + wantEffective: policyWithLabels(DefaultPolicy, map[string]string{ + "type": "policy", + "policyType": "path", + "hostname": "host-c", + "path": "/some/path", + "username": "myuser", + }), + }, + { + sourceInfo: snapshot.SourceInfo{ + UserName: "myuser", + Host: "host-a", + Path: "/some/path", + }, + wantEffective: policyWithLabels(defaultPolicyWithKeepDaily(t, 44), map[string]string{ + "type": "policy", + "policyType": "path", + "hostname": "host-a", + "path": "/some/path", + "username": "myuser", + }), + wantSources: []snapshot.SourceInfo{ + {Host: "host-a"}, + }, + }, + { + sourceInfo: snapshot.SourceInfo{ + UserName: "myuser", + Host: "host-a", + Path: "/some/path2/nested", + }, + wantEffective: policyWithLabels(defaultPolicyWithKeepDaily(t, 66), map[string]string{ + "type": "policy", + "policyType": "path", + "hostname": "host-a", + "path": "/some/path2/nested", + "username": "myuser", + }), + wantSources: []snapshot.SourceInfo{ + {UserName: "myuser", Path: "/some/path2", Host: "host-a"}, + {Host: "host-a"}, + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(fmt.Sprintf("%v", tc.sourceInfo), func(t *testing.T) { + pol, src, err := GetEffectivePolicy(ctx, env.Repository, tc.sourceInfo) + if err != nil { + t.Fatalf("err: %v", err) + } + if diff := cmp.Diff(pol, tc.wantEffective); diff != "" { + t.Errorf("got: %v", pol) + t.Errorf("want: %v", tc.wantEffective) + t.Errorf("invalid effective policy: %v", diff) + } + + var sources []snapshot.SourceInfo + for _, s := range src { + sources = append(sources, s.Target()) + } + + if diff := cmp.Diff(sources, tc.wantSources); diff != "" { + t.Errorf("got: %v", sources) + t.Errorf("want: %v", tc.wantSources) + t.Errorf("invalid sources: %v", diff) + } + }) + } +} + +func clonePolicy(t *testing.T, p *Policy) *Policy { + j, err := json.Marshal(p) + if err != nil { + t.Fatalf("unable to marshal JSON: %v", err.Error()) + } + + p2 := &Policy{} + json.Unmarshal(j, &p2) + + return p2 +} + +func policyWithLabels(p *Policy, l map[string]string) *Policy { + p2 := *p + p2.Labels = l + + return &p2 +} + +func defaultPolicyWithKeepDaily(t *testing.T, keepDaily int) *Policy { + p := clonePolicy(t, DefaultPolicy) + p.RetentionPolicy.KeepDaily = &keepDaily + + return p +} + +func TestPolicyManagerResolvesConflicts(t *testing.T) { ctx := context.Background() var env repotesting.Environment @@ -36,13 +194,226 @@ func TestPolicyManager(t *testing.T) { r3 := env.MustOpenAnother(t) pi, err := GetDefinedPolicy(ctx, r3, sourceInfo) + must(t, err) + if got, want := pi.Target(), sourceInfo; got != want { + t.Errorf("invalid policy target %v, want %v", got, want) + } + if got := *pi.RetentionPolicy.KeepDaily; got != 33 && got != 44 { t.Errorf("unexpected policy returned") } } +func TestParentPathOSIndependent(t *testing.T) { + cases := []struct { + input, want string + }{ + {"/", "/"}, + {"/x", "/"}, + {"/x/", "/"}, + {"/x/a", "/x"}, + {"/x/a/", "/x"}, + {"x:\\", "x:\\"}, + {"X:\\Program Files", "X:\\"}, + {"X:\\Program Files\\Blah", "X:\\Program Files"}, + {"X:/Program Files", "X:/"}, + {"X:/Program Files/", "X:/"}, + {"X:/Program Files\\", "X:/"}, + {"X:/Program Files/Blah", "X:/Program Files"}, + {"X:/Program Files/Blah/", "X:/Program Files"}, + {"X:/Program Files/Blah/xxx", "X:/Program Files/Blah"}, + } + + for _, tc := range cases { + if got, want := getParentPathOSIndependent(tc.input), tc.want; got != want { + t.Errorf("invalid value of getParentPathOSIndependent(%q): %q, want %q", tc.input, got, want) + } + } +} + +// TestApplicablePoliciesForSource verifies that when we build a policy tree, we pick the appropriate policies +// defined for the subtree regardless of path style (Unix or Windows). +func TestApplicablePoliciesForSource(t *testing.T) { + ctx := testlogging.Context(t) + + var env repotesting.Environment + + defer env.Setup(t).Close(ctx, t) + + setPols := map[snapshot.SourceInfo]*Policy{ + // unix-style path names + {Host: "host-a"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(0)}, + }, + {Host: "host-a", UserName: "myuser", Path: "/home"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(1)}, + }, + {Host: "host-a", UserName: "myuser", Path: "/home/users"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(2)}, + }, + // on Unix \ a regular character so the directory name is 'user-with\\backslash' + {Host: "host-a", UserName: "myuser", Path: "/home/users/user-with\\backslash"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(2)}, + }, + {Host: "host-a", UserName: "myuser", Path: "/home/users/user-with\\backslash/x"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(2)}, + }, + {Host: "host-a", UserName: "myuser", Path: "/home/users/myuser"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(3)}, + }, + {Host: "host-a", UserName: "myuser", Path: "/home/users/myuser/dir1"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(4)}, + }, + {Host: "host-a", UserName: "myuser", Path: "/home/users/myuser/dir2"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(5)}, + }, + {Host: "host-a", UserName: "myuser", Path: "/home/users/myuser/dir2/a"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(6)}, + }, + {Host: "host-a", UserName: "myuser", Path: "/home/users/myuser2"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(7)}, + }, + + // windows-style path names with backslash + {Host: "host-b"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(0)}, + }, + {Host: "host-b", UserName: "myuser", Path: "C:\\"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(1)}, + }, + {Host: "host-b", UserName: "myuser", Path: "C:\\Users"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(2)}, + }, + {Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(3)}, + }, + {Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser\\dir1"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(4)}, + }, + {Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser\\dir2"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(5)}, + }, + {Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser\\dir2\\a"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(6)}, + }, + {Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser2"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(7)}, + }, + + // windows-style path names with slashes + {Host: "host-c"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(0)}, + }, + {Host: "host-c", UserName: "myuser", Path: "C:/Users"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(1)}, + }, + {Host: "host-c", UserName: "myuser", Path: "C:/Users"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(2)}, + }, + {Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(3)}, + }, + {Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser/dir1"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(4)}, + }, + {Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser/dir2"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(5)}, + }, + {Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser/dir2/a"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(6)}, + }, + {Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser2"}: { + RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(7)}, + }, + } + + for si, pol := range setPols { + must(t, SetPolicy(ctx, env.Repository, si, pol)) + } + + cases := []struct { + si snapshot.SourceInfo + wantPaths []string + }{ + {snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/tmp"}, []string{"."}}, + {snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser"}, + []string{".", "./dir1", "./dir2", "./dir2/a"}, + }, + {snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser2"}, + []string{"."}, + }, + {snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/user-with\\backslash"}, + []string{".", "./x"}, + }, + {snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home"}, + []string{ + ".", + "./users", + "./users/myuser", + "./users/myuser/dir1", + "./users/myuser/dir2", + "./users/myuser/dir2/a", + "./users/myuser2", + `./users/user-with\backslash`, + `./users/user-with\backslash/x`, + }, + }, + {snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/"}, + []string{ + ".", + "./home", + "./home/users", + "./home/users/myuser", + "./home/users/myuser/dir1", + "./home/users/myuser/dir2", + "./home/users/myuser/dir2/a", + "./home/users/myuser2", + `./home/users/user-with\backslash`, + `./home/users/user-with\backslash/x`, + }, + }, + + {snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Temp"}, []string{"."}}, + {snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser"}, + []string{".", "./dir1", "./dir2", "./dir2/a"}, + }, + {snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\"}, + []string{ + ".", + "./Users", + "./Users/myuser", + "./Users/myuser/dir1", + "./Users/myuser/dir2", + "./Users/myuser/dir2/a", + "./Users/myuser2", + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(fmt.Sprintf("%v", tc.si), func(t *testing.T) { + res, err := applicablePoliciesForSource(ctx, env.Repository, tc.si) + if err != nil { + t.Fatalf("error in applicablePoliciesForSource(%v): %v", tc.si, err) + } + + var relPaths []string + for k := range res { + relPaths = append(relPaths, k) + } + + sort.Strings(relPaths) + + if diff := cmp.Diff(relPaths, tc.wantPaths); diff != "" { + t.Errorf("invalid sub-policies %v", diff) + } + }) + } +} + func must(t *testing.T, err error) { t.Helper() diff --git a/snapshot/policy/policy_tree_test.go b/snapshot/policy/policy_tree_test.go index a06ec9741..2b14dc4b3 100644 --- a/snapshot/policy/policy_tree_test.go +++ b/snapshot/policy/policy_tree_test.go @@ -7,7 +7,7 @@ ) var ( - defaultPolicy = &Policy{ + defPolicy = &Policy{ FilesPolicy: FilesPolicy{ IgnoreRules: []string{"default"}, }, @@ -84,7 +84,7 @@ func TestBuildTree(t *testing.T) { ".": policyA, "./foo": policyB, "./bar/baz/bleh": policyC, - }, defaultPolicy) + }, defPolicy) dumpTree(n, "root")