ast: Allowing packages to be declared within the dynamic extent of a …

…rule (open-policy-agent#6388) Fixes: open-policy-agent#6387 Signed-off-by: Johan Fylling <[email protected]>
johanfylling · Nov 7, 2023 · 7f17c36 · 7f17c36
1 parent c12463c
commit 7f17c36
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 4 deletions.
diff --git a/ast/compile.go b/ast/compile.go
@@ -1120,6 +1120,12 @@ func (c *Compiler) checkRuleConflicts() {
 		for _, mod := range node.Modules {
 			for _, rule := range mod.Rules {
 				ref := rule.Head.Ref().GroundPrefix()
+				// Rules with a dynamic portion in their ref are exempted, as a conflict within the dynamic portion
+				// can only be detected at eval-time.
+				if len(ref) < len(rule.Head.Ref()) {
+					continue
+				}
+
 				childNode, tail := node.find(ref)
 				if childNode != nil && len(tail) == 0 {
 					for _, childMod := range childNode.Modules {

diff --git a/ast/compile_test.go b/ast/compile_test.go
@@ -1769,9 +1769,23 @@ p[x] = y { x = y; x = "a" }
 q[1] { true }
 q = {1, 2, 3} { true }
 r[x] = y { x = y; x = "a" }
-r[x] = y { x = y; x = "a" }`,
+r[x] = y { x = y; x = "a" }
+s[x] { x = "a" }
+s[x] { x = "b" }
+t := x { x = "a"}`,
+
+		// valid extension of r in mod1.rego
+		"mod2a.rego": `package badrules.r
+
+q[1] { true }`,
+
+		// invalid override of s in mod1.rego
+		"mod2b.rego": `package badrules.s
+
+q[1] { true }`,
 
-		"mod2.rego": `package badrules.r
+		// invalid override of t in mod1.rego
+		"mod2c.rego": `package badrules.t
 
 q[1] { true }`,
 
@@ -1837,8 +1851,11 @@ p[r] := 2 { r := "foo" }`,
 		"rego_type_error: conflicting rules data.badrules.q found",
 		"rego_type_error: multiple default rules data.badrules.defkw.foo found",
 		"rego_type_error: multiple default rules data.badrules.defkw.p.q.bar found",
-		"rego_type_error: package badrules.r conflicts with rule r[x] defined at mod1.rego:7",
-		"rego_type_error: package badrules.r conflicts with rule r[x] defined at mod1.rego:8",
+		"rego_type_error: package badrules.s conflicts with rule s defined at mod1.rego:10",
+		"rego_type_error: package badrules.s conflicts with rule s defined at mod1.rego:9",
+		"rego_type_error: package badrules.t conflicts with rule t defined at mod1.rego:11",
+		"rego_type_error: rule data.badrules.s conflicts with [data.badrules.s.q]",
+		"rego_type_error: rule data.badrules.t conflicts with [data.badrules.t.q]",
 	}
 
 	assertCompilerErrorStrings(t, c, expected)
@@ -2125,6 +2142,72 @@ func TestCompilerCheckRuleConflictsDotsInRuleHeads(t *testing.T) {
 	}
 }
 
+func TestCompilerCheckRulePkgConflicts(t *testing.T) {
+	tests := []struct {
+		note    string
+		modules []*Module
+		err     []string
+	}{
+		{
+			note: "Package can be declared within dynamic extent of rule (#6387 regression test)",
+			modules: modules(
+				`package test
+					p[x] := y { x := "a"; y := "b" }`,
+				`package test.p
+					q := 1`),
+		},
+		{
+			note: "Package can be declared deep within dynamic extent of rule (#6387 regression test)",
+			modules: modules(
+				`package test
+					p[x] := y { x := "a"; y := "b" }`,
+				`package test.p.q.r.s
+					t := 1`),
+		},
+		{
+			note: "Package cannot be declared within extent of single-value rule (ground ref)",
+			modules: modules(
+				`package test
+					p := x { x := "a" }`,
+				`package test.p
+					q := 1`),
+			err: []string{
+				"rego_type_error: package test.p conflicts with rule p defined at mod0.rego:2",
+				"rego_type_error: rule data.test.p conflicts with [data.test.p.q]",
+			},
+		},
+		{
+			note: "Package cannot be declared within extent of multi-value rule",
+			modules: modules(
+				`package test
+					p[x] { x := "a" }`,
+				`package test.p
+					q := 1`),
+			err: []string{
+				"rego_type_error: package test.p conflicts with rule p defined at mod0.rego:2",
+				"rego_type_error: rule data.test.p conflicts with [data.test.p.q]",
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.note, func(t *testing.T) {
+			mods := make(map[string]*Module, len(tc.modules))
+			for i, m := range tc.modules {
+				mods[fmt.Sprint(i)] = m
+			}
+			c := NewCompiler()
+			c.Modules = mods
+			compileStages(c, c.checkRuleConflicts)
+			if len(tc.err) > 0 {
+				assertCompilerErrorStrings(t, c, tc.err)
+			} else {
+				assertCompilerErrorStrings(t, c, []string{})
+			}
+		})
+	}
+}
+
 func TestCompilerCheckUndefinedFuncs(t *testing.T) {
 
 	module := `
@@ -9462,6 +9545,7 @@ func runStrictnessQueryTestCase(t *testing.T, cases []strictnessQueryTestCase) {
 }
 
 func assertCompilerErrorStrings(t *testing.T, compiler *Compiler, expected []string) {
+	t.Helper()
 	result := compilerErrsToStringSlice(compiler.Errors)
 
 	if len(result) != len(expected) {