From 02a3bf0ac4a2f99f813f91ac9db3a679aa087c1a Mon Sep 17 00:00:00 2001 From: Edward Paget Date: Wed, 6 Oct 2021 13:23:37 -0500 Subject: [PATCH] bundle: Replace HasPrefix in erasePolicy (#3863) Using HasPrefix to compare a root path to the path of policy can result in unexpected behaviour when different bundles have similarly named roots. This replaces the HasPrefix comparison with a comparison that treats the paths as directories and checks if the policy path is the same path as the root or a subdirectory of the root path, using an existing helper. Signed-off-by: Edward Paget --- bundle/store.go | 3 +-- bundle/store_test.go | 12 +++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/bundle/store.go b/bundle/store.go index 1753ffa3eb..0dc7796457 100644 --- a/bundle/store.go +++ b/bundle/store.go @@ -9,7 +9,6 @@ import ( "encoding/base64" "encoding/json" "fmt" - "strings" "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/metrics" @@ -447,7 +446,7 @@ func erasePolicies(ctx context.Context, store storage.Store, txn storage.Transac } deleted := false for root := range roots { - if strings.HasPrefix(path, root) { + if RootPathsContain([]string{root}, path) { if err := store.DeletePolicy(ctx, txn, id); err != nil { return nil, err } diff --git a/bundle/store_test.go b/bundle/store_test.go index 9e9d09aee2..e578b515d7 100644 --- a/bundle/store_test.go +++ b/bundle/store_test.go @@ -509,6 +509,16 @@ func TestErasePolicies(t *testing.T) { expectErr: false, expectedRemaining: []string{"mod1", "mod2"}, }, + { + note: "erase correct paths", + initialPolicies: map[string][]byte{ + "mod1": []byte("package a.test\np = true"), + "mod2": []byte("package a.test_v2\np = true"), + }, + roots: []string{"a/test"}, + expectErr: false, + expectedRemaining: []string{"mod2"}, + }, { note: "erase some", initialPolicies: map[string][]byte{ @@ -556,7 +566,7 @@ func TestErasePolicies(t *testing.T) { if !tc.expectErr { if len(remaining) != len(tc.expectedRemaining) { - t.Fatalf("expected %d modules remaining, got %d", len(remaining), len(tc.expectedRemaining)) + t.Fatalf("expected %d modules remaining, got %d", len(tc.expectedRemaining), len(remaining)) } for _, name := range tc.expectedRemaining { if _, ok := remaining[name]; !ok {