diff --git a/v1/bundle/bundle.go b/v1/bundle/bundle.go index 80bad2090bb..1feb20c2a0c 100644 --- a/v1/bundle/bundle.go +++ b/v1/bundle/bundle.go @@ -1259,12 +1259,16 @@ func (m *Manifest) numericRegoVersionForFile(path string) (*int, error) { if len(m.FileRegoVersions) != len(m.compiledFileRegoVersions) { m.compiledFileRegoVersions = make([]fileRegoVersion, 0, len(m.FileRegoVersions)) - for pattern, v := range m.FileRegoVersions { + // Compile patterns in sorted key order. The behaviour for overlapping + // patterns is documented as undefined, however we ensure the ordering + // of which patterns are used will be deterministic by sorting the + // keys before iterating over the patterns. + for _, pattern := range util.KeysSorted(m.FileRegoVersions) { compiled, err := glob.Compile(pattern) if err != nil { return nil, fmt.Errorf("failed to compile glob pattern %s: %s", pattern, err) } - m.compiledFileRegoVersions = append(m.compiledFileRegoVersions, fileRegoVersion{compiled, v}) + m.compiledFileRegoVersions = append(m.compiledFileRegoVersions, fileRegoVersion{compiled, m.FileRegoVersions[pattern]}) } } diff --git a/v1/bundle/bundle_test.go b/v1/bundle/bundle_test.go index 7b9fb5ca2e0..91d5ba0bacd 100644 --- a/v1/bundle/bundle_test.go +++ b/v1/bundle/bundle_test.go @@ -13,8 +13,10 @@ import ( "errors" "fmt" "io" + "maps" "path/filepath" "reflect" + "slices" "strings" "testing" "testing/fstest" @@ -158,6 +160,58 @@ func TestBundleRegoVersion(t *testing.T) { } } +// TestManifestNumericRegoVersionForFileDeterministic is a regression test +// that ensures that resolving a file's rego-version is stable across +// invocations when multiple patterns match the same path. +// +// The resolution for overlapping patterns is documented as undefined, but +// it should not vary from run to run for the same manifest. +// +// We test this by loading up several overlapping patterns, and resolving +// the file rego-versions multiple times. If there is any non-determinism +// in how patterns are selected, this test should surface it. +func TestManifestNumericRegoVersionForFileDeterministic(t *testing.T) { + const path = "/example/policy.rego" + + // Every pattern matches path; the version value identifies which pattern + // the resolver selected. (glob.Compile is called without separators, so + // '*' crosses '/'.) numericRegoVersionForFile does not validate the + // version range, so distinct ints are fine for probing which pattern won. + patterns := map[string]int{ + "**": 0, + "*": 1, + "/**": 2, + "/example/**": 3, + "/example/*": 4, + "/example/*.rego": 5, + "**/policy.rego": 6, + "/example/policy.*": 7, + } + + var first *int + for i := range 64 { + // Fresh manifest each iteration so the resolver runs afresh. + m := Manifest{FileRegoVersions: maps.Clone(patterns)} + got, err := m.numericRegoVersionForFile(path) + if err != nil { + t.Fatal(err) + } + if got == nil { + t.Fatalf("iteration %d: expected a match, got nil", i) + } + if first == nil { + first = got + } else if *got != *first { + t.Fatalf("rego-version for %q is not stable across runs: got %d and %d (overlapping-pattern resolution must be deterministic)", path, *first, *got) + } + } + + // Check and make sure lexical sorting is in place for patterns. + if want := patterns[slices.Min(slices.Collect(maps.Keys(patterns)))]; first == nil || *first != want { + t.Fatalf("expected lexically-first matching pattern to win (version %d), got %v", want, first) + } +} + func TestRead(t *testing.T) { for _, useMemoryFS := range []bool{false, true} { testReadBundle(t, "", useMemoryFS)