Skip to content

Commit

Permalink
bundle: Replace HasPrefix in erasePolicy (#3863)
Browse files Browse the repository at this point in the history
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 <edward.paget@chime.com>
  • Loading branch information
edpaget authored Oct 6, 2021
1 parent 0871e16 commit 02a3bf0
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
3 changes: 1 addition & 2 deletions bundle/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 11 additions & 1 deletion bundle/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 02a3bf0

Please sign in to comment.