Skip to content

Commit

Permalink
cmd: Respect --v0-compatible for opa eval partial eval support mo…
Browse files Browse the repository at this point in the history 
…dules (#7251)

Fixing issue in the `opa eval` command where `--v0-compatible` was ignored for PE support modules when
used in combination with the `pretty` output format; producing v1 support modules instead of v0.

Fixing: #7248

Signed-off-by: Johan Fylling <[email protected]>
  • Loading branch information
@johanfylling
johanfylling authored Jan 8, 2025
1 parent f9e1b32 commit 64ebe35
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 53 deletions.
171 changes: 125 additions & 46 deletions cmd/eval_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1872,17 +1872,18 @@ time.clock(input.y, time.clock(input.x))
}
}

func TestEvalPartialRegoVersionOutput(t *testing.T) {
func TestEvalPartialOutput_RegoVersion(t *testing.T) {
tests := []struct {
note string
regoV1ImportCapable bool
v1Compatible bool
v0Compatible bool
query string
module string
expected string
expected map[string]string
}{
{
note: "v0, no future keywords",
v0Compatible: true,
regoV1ImportCapable: true,
query: "data.test.p",
module: `package test
Expand All @@ -1891,9 +1892,9 @@ p[v] {
v := input.v
}
`,
expected: `# Query 1
data.partial.test.p = _term_0_0
_term_0_0
expected: map[string]string{
evalSourceOutput: `# Query 1
data.partial.test.p
# Module 1
package partial.test
Expand All @@ -1902,9 +1903,21 @@ import rego.v1
p contains __local0__1 if __local0__1 = input.v
`,
evalPrettyOutput: `+-----------+-------------------------------------------------+
| Query 1 | data.partial.test.p |
+-----------+-------------------------------------------------+
| Support 1 | package partial.test |
| | |
| | import rego.v1 |
| | |
| | p contains __local0__1 if __local0__1 = input.v |
+-----------+-------------------------------------------------+
`,
},
},
{
note: "v0, no future keywords, not rego.v1 import capable",
v0Compatible: true,
regoV1ImportCapable: false,
query: "data.test.p",
module: `package test
Expand All @@ -1913,9 +1926,9 @@ p[v] {
v := input.v
}
`,
expected: `# Query 1
data.partial.test.p = _term_0_0
_term_0_0
expected: map[string]string{
evalSourceOutput: `# Query 1
data.partial.test.p
# Module 1
package partial.test
Expand All @@ -1924,9 +1937,21 @@ p[__local0__1] {
__local0__1 = input.v
}
`,
evalPrettyOutput: `+-----------+-------------------------+
| Query 1 | data.partial.test.p |
+-----------+-------------------------+
| Support 1 | package partial.test |
| | |
| | p[__local0__1] { |
| | __local0__1 = input.v |
| | } |
+-----------+-------------------------+
`,
},
},
{
note: "v0, future keywords",
v0Compatible: true,
regoV1ImportCapable: true,
query: "data.test.p",
module: `package test
Expand All @@ -1937,9 +1962,9 @@ p contains v if {
v := input.v
}
`,
expected: `# Query 1
data.partial.test.p = _term_0_0
_term_0_0
expected: map[string]string{
evalSourceOutput: `# Query 1
data.partial.test.p
# Module 1
package partial.test
Expand All @@ -1948,64 +1973,118 @@ import rego.v1
p contains __local0__1 if __local0__1 = input.v
`,
evalPrettyOutput: `+-----------+-------------------------------------------------+
| Query 1 | data.partial.test.p |
+-----------+-------------------------------------------------+
| Support 1 | package partial.test |
| | |
| | import rego.v1 |
| | |
| | p contains __local0__1 if __local0__1 = input.v |
+-----------+-------------------------------------------------+
`,
},
},
{
note: "v1",
regoV1ImportCapable: true,
v1Compatible: true,
v0Compatible: false,
query: "data.test.p",
module: `package test
p contains v if {
v := input.v
}
`,
expected: `# Query 1
data.partial.test.p = _term_0_0
_term_0_0
expected: map[string]string{
evalSourceOutput: `# Query 1
data.partial.test.p
# Module 1
package partial.test
p contains __local0__1 if __local0__1 = input.v
`,
evalPrettyOutput: `+-----------+-------------------------------------------------+
| Query 1 | data.partial.test.p |
+-----------+-------------------------------------------------+
| Support 1 | package partial.test |
| | |
| | p contains __local0__1 if __local0__1 = input.v |
+-----------+-------------------------------------------------+
`,
},
},
{
note: "v1, rego.v1 import",
regoV1ImportCapable: true,
v0Compatible: false,
query: "data.test.p",
module: `package test
import rego.v1
p contains v if {
v := input.v
}
`,
expected: map[string]string{
evalSourceOutput: `# Query 1
data.partial.test.p
# Module 1
package partial.test
p contains __local0__1 if __local0__1 = input.v
`,
evalPrettyOutput: `+-----------+-------------------------------------------------+
| Query 1 | data.partial.test.p |
+-----------+-------------------------------------------------+
| Support 1 | package partial.test |
| | |
| | p contains __local0__1 if __local0__1 = input.v |
+-----------+-------------------------------------------------+
`,
},
},
}

for _, tc := range tests {
t.Run(tc.note, func(t *testing.T) {
files := map[string]string{
"test.rego": tc.module,
}

test.WithTempFS(files, func(path string) {
params := newEvalCommandParams()
_ = params.dataPaths.Set(filepath.Join(path, "test.rego"))
params.partial = true
params.shallowInlining = true
params.v0Compatible = !tc.v1Compatible
params.v1Compatible = tc.v1Compatible
_ = params.outputFormat.Set(evalSourceOutput)

if !tc.regoV1ImportCapable {
caps := newcapabilitiesFlag()
caps.C = ast.CapabilitiesForThisVersion()
caps.C.Features = []string{
ast.FeatureRefHeadStringPrefixes,
ast.FeatureRefHeads,
for format, expected := range tc.expected {
t.Run(format, func(t *testing.T) {
files := map[string]string{
"test.rego": tc.module,
}
params.capabilities = caps
}

buf := new(bytes.Buffer)
_, err := eval([]string{tc.query}, params, buf)
if err != nil {
t.Fatal("unexpected error:", err)
}
if actual := buf.String(); actual != tc.expected {
t.Errorf("expected output %q\ngot %q", tc.expected, actual)
}
})
test.WithTempFS(files, func(path string) {
params := newEvalCommandParams()
_ = params.dataPaths.Set(filepath.Join(path, "test.rego"))
params.partial = true
params.v0Compatible = tc.v0Compatible
_ = params.outputFormat.Set(format)

if !tc.regoV1ImportCapable {
caps := newcapabilitiesFlag()
caps.C = ast.CapabilitiesForThisVersion()
caps.C.Features = []string{
ast.FeatureRefHeadStringPrefixes,
ast.FeatureRefHeads,
}
params.capabilities = caps
}

buf := new(bytes.Buffer)
_, err := eval([]string{tc.query}, params, buf)
if err != nil {
t.Fatal("unexpected error:", err)
}
if actual := buf.String(); actual != expected {
t.Errorf("expected output:\n\n%s\n\ngot:\n\n%s", expected, actual)
}
})
})
}
})
}
}
Expand Down
14 changes: 7 additions & 7 deletions internal/presentation/presentation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,25 +457,25 @@ func prettyPartial(w io.Writer, pq *rego.PartialQueries) error {
var maxWidth int

for i := range pq.Queries {
s, width, err := prettyASTNode(pq.Queries[i])
f, width, err := prettyASTNode(pq.Queries[i], ast.DefaultRegoVersion)
if err != nil {
return err
}
if width > maxWidth {
maxWidth = width
}
table.Append([]string{fmt.Sprintf("Query %d", i+1), s})
table.Append([]string{fmt.Sprintf("Query %d", i+1), f})
}

for i := range pq.Support {
s, width, err := prettyASTNode(pq.Support[i])
for i, s := range pq.Support {
f, width, err := prettyASTNode(s, s.RegoVersion())
if err != nil {
return err
}
if width > maxWidth {
maxWidth = width
}
table.Append([]string{fmt.Sprintf("Support %d", i+1), s})
table.Append([]string{fmt.Sprintf("Support %d", i+1), f})
}

table.SetColMinWidth(1, maxWidth)
Expand All @@ -485,8 +485,8 @@ func prettyPartial(w io.Writer, pq *rego.PartialQueries) error {
}

// prettyASTNode is used for pretty-printing the result of partial eval
func prettyASTNode(x interface{}) (string, int, error) {
bs, err := format.AstWithOpts(x, format.Opts{IgnoreLocations: true})
func prettyASTNode(x interface{}, regoVersion ast.RegoVersion) (string, int, error) {
bs, err := format.AstWithOpts(x, format.Opts{IgnoreLocations: true, RegoVersion: regoVersion})
if err != nil {
return "", 0, fmt.Errorf("format error: %w", err)
}
Expand Down

0 comments on commit 64ebe35

Please sign in to comment.