Adding --rego-v1 flag to check cmd (#6430)

When enabled, checked module(s) must be compliant with OPA 1.0 Rego.

Fixes: #6429
Signed-off-by: Johan Fylling <johan.dev@fylling.se>

Author: johanfylling

ast/parser.go

@@ -106,6 +106,7 @@ type ParserOptions struct {
 	SkipRules          bool
 	JSONOptions        *astJSON.Options
 	unreleasedKeywords bool // TODO(sr): cleanup
+	RegoV1Compatible   bool
 }
 
 // NewParser creates and initializes a Parser.

ast/parser_ext.go

@@ -477,7 +477,7 @@ func ParseModuleWithOpts(filename, input string, popts ParserOptions) (*Module,
 	if err != nil {
 		return nil, err
 	}
-	return parseModule(filename, stmts, comments)
+	return parseModule(filename, stmts, comments, popts.RegoV1Compatible)
 }
 
 // ParseBody returns exactly one body.
@@ -637,7 +637,7 @@ func ParseStatementsWithOpts(filename, input string, popts ParserOptions) ([]Sta
 	return stmts, comments, nil
 }
 
-func parseModule(filename string, stmts []Statement, comments []*Comment) (*Module, error) {
+func parseModule(filename string, stmts []Statement, comments []*Comment, regoV1Compatible bool) (*Module, error) {
 
 	if len(stmts) == 0 {
 		return nil, NewError(ParseErr, &Location{File: filename}, "empty module")
@@ -658,6 +658,7 @@ func parseModule(filename string, stmts []Statement, comments []*Comment) (*Modu
 
 	// The comments slice only holds comments that were not their own statements.
 	mod.Comments = append(mod.Comments, comments...)
+	mod.regoV1Compatible = regoV1Compatible
 
 	for i, stmt := range stmts[1:] {
 		switch stmt := stmt.(type) {

cmd/check.go

@@ -25,6 +25,7 @@ type checkParams struct {
 	capabilities *capabilitiesFlag
 	schema       *schemaFlags
 	strict       bool
+	regoV1       bool
 }
 
 func newCheckParams() checkParams {
@@ -64,6 +65,7 @@ func checkModules(params checkParams, args []string) error {
 	if params.bundleMode {
 		for _, path := range args {
 			b, err := loader.NewFileLoader().
+				WithRegoV1Compatible(params.regoV1).
 				WithSkipBundleVerification(true).
 				WithProcessAnnotation(true).
 				WithCapabilities(capabilities).
@@ -82,6 +84,7 @@ func checkModules(params checkParams, args []string) error {
 		}
 
 		result, err := loader.NewFileLoader().
+			WithRegoV1Compatible(params.regoV1).
 			WithProcessAnnotation(true).
 			WithCapabilities(capabilities).
 			Filtered(args, f.Apply)
@@ -165,5 +168,6 @@ func init() {
 	addCapabilitiesFlag(checkCommand.Flags(), checkParams.capabilities)
 	addSchemaFlags(checkCommand.Flags(), checkParams.schema)
 	addStrictFlag(checkCommand.Flags(), &checkParams.strict, false)
+	checkCommand.Flags().BoolVar(&checkParams.regoV1, "rego-v1", false, "check for Rego v1 compatibility (policies must also be compatible with current OPA version)")
 	RootCommand.AddCommand(checkCommand)
 }

cmd/check_test.go

@@ -236,3 +236,134 @@ p {
 		t.Fatalf("unexpected error from eval with inlined schema, got: %v", err)
 	}
 }
+
+func TestCheckRegoV1(t *testing.T) {
+	cases := []struct {
+		note    string
+		policy  string
+		expErrs []string
+	}{
+		{
+			note: "rego.v1 imported, v1 compliant",
+			policy: `package test
+import rego.v1
+p contains x if {
+	x := [1,2,3]
+}`,
+		},
+		{
+			note: "rego.v1 imported, NOT v1 compliant (parser)",
+			policy: `package test
+import rego.v1
+p contains x {
+	x := [1,2,3]
+}
+
+q.r`,
+			expErrs: []string{
+				"test.rego:3: rego_parse_error: `if` keyword is required before rule body",
+				"test.rego:7: rego_parse_error: `contains` keyword is required for partial set rules",
+			},
+		},
+		{
+			note: "rego.v1 imported, NOT v1 compliant (compiler)",
+			policy: `package test
+import rego.v1
+
+import data.foo
+import data.bar as foo
+`,
+			expErrs: []string{
+				"test.rego:5: rego_compile_error: import must not shadow import data.foo",
+			},
+		},
+		{
+			note: "keywords imported, v1 compliant",
+			policy: `package test
+import future.keywords.if
+import future.keywords.contains
+p contains x if {
+	x := [1,2,3]
+}`,
+		},
+		{
+			note: "keywords imported, NOT v1 compliant",
+			policy: `package test
+import future.keywords.contains
+p contains x {
+	x := [1,2,3]
+}
+
+q.r`,
+			expErrs: []string{
+				"test.rego:3: rego_parse_error: `if` keyword is required before rule body",
+				"test.rego:7: rego_parse_error: `contains` keyword is required for partial set rules",
+			},
+		},
+		{
+			note: "keywords imported, NOT v1 compliant (compiler)",
+			policy: `package test
+import future.keywords.if
+
+input := 1 if {
+	1 == 2
+}`,
+			expErrs: []string{
+				"test.rego:4: rego_compile_error: rules must not shadow input (use a different rule name)",
+			},
+		},
+		{
+			note: "no imports, v1 compliant",
+			policy: `package test
+p := 1
+`,
+		},
+		{
+			note: "no imports, NOT v1 compliant but v0 compliant (compiler)",
+			policy: `package test
+p.x`,
+			expErrs: []string{
+				"test.rego:2: rego_parse_error: `contains` keyword is required for partial set rules",
+			},
+		},
+		{
+			note: "no imports, v1 compliant but NOT v0 compliant",
+			policy: `package test
+p contains x if {
+	x := [1,2,3]
+}`,
+			expErrs: []string{
+				"test.rego:2: rego_parse_error: var cannot be used for rule name", // This error actually appears three times: once for 'p'; once for 'contains'; and once for 'x'. All are interpreted as [invalid] rule declarations with no value and body.
+				"test.rego:2: rego_parse_error: `if` keyword is required before rule body",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.note, func(t *testing.T) {
+			files := map[string]string{
+				"test.rego": tc.policy,
+			}
+
+			test.WithTempFS(files, func(root string) {
+				params := newCheckParams()
+				params.regoV1 = true
+
+				err := checkModules(params, []string{root})
+				switch {
+				case err != nil && len(tc.expErrs) > 0:
+					for _, expErr := range tc.expErrs {
+						if !strings.Contains(err.Error(), expErr) {
+							t.Fatalf("expected err:\n\n%v\n\ngot:\n\n%v", expErr, err)
+						}
+					}
+					return // don't read back bundle below
+				case err != nil && len(tc.expErrs) == 0:
+					t.Fatalf("unexpected error: %v", err)
+				case err == nil && len(tc.expErrs) > 0:
+					t.Fatalf("expected error:\n\n%v\n\ngot: none", tc.expErrs)
+				}
+			})
+		})
+	}
+}

loader/loader.go

@@ -102,6 +102,8 @@ type FileLoader interface {
 	WithProcessAnnotation(bool) FileLoader
 	WithCapabilities(*ast.Capabilities) FileLoader
 	WithJSONOptions(*astJSON.Options) FileLoader
+
+	WithRegoV1Compatible(bool) FileLoader
 }
 
 // NewFileLoader returns a new FileLoader instance.
@@ -181,6 +183,11 @@ func (fl *fileLoader) WithJSONOptions(opts *astJSON.Options) FileLoader {
 	return fl
 }
 
+func (fl *fileLoader) WithRegoV1Compatible(compatible bool) FileLoader {
+	fl.opts.RegoV1Compatible = compatible
+	return fl
+}
+
 // All returns a Result object loaded (recursively) from the specified paths.
 func (fl fileLoader) All(paths []string) (*Result, error) {
 	return fl.Filtered(paths, nil)