diff --git a/ast/parser.go b/ast/parser.go index 3037e1e855..97386a3391 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -2696,18 +2696,19 @@ func (p *Parser) regoV1Import(imp *Import) { return } - if p.po.RegoVersion == RegoV1 { - // We're parsing for Rego v1, where the 'rego.v1' import is a no-op. - return - } - path := imp.Path.Value.(Ref) + // v1 is only valid option if len(path) == 1 || !path[1].Equal(RegoV1CompatibleRef[1]) || len(path) > 2 { p.errorf(imp.Path.Location, "invalid import `%s`, must be `%s`", path, RegoV1CompatibleRef) return } + if p.po.RegoVersion == RegoV1 { + // We're parsing for Rego v1, where the 'rego.v1' import is a no-op. + return + } + if imp.Alias != "" { p.errorf(imp.Path.Location, "`rego` imports cannot be aliased") return diff --git a/cmd/bench.go b/cmd/bench.go index b2e6b84ac7..0a9315a451 100644 --- a/cmd/bench.go +++ b/cmd/bench.go @@ -124,6 +124,7 @@ The optional "gobench" output format conforms to the Go Benchmark Data Format. addIgnoreFlag(benchCommand.Flags(), ¶ms.ignore) addSchemaFlags(benchCommand.Flags(), params.schema) addTargetFlag(benchCommand.Flags(), params.target) + addV0CompatibleFlag(benchCommand.Flags(), ¶ms.v0Compatible, false) addV1CompatibleFlag(benchCommand.Flags(), ¶ms.v1Compatible, false) // Shared benchmark flags @@ -304,6 +305,7 @@ func benchE2E(ctx context.Context, args []string, params benchmarkCommandParams, GracefulShutdownPeriod: params.gracefulShutdownPeriod, ShutdownWaitPeriod: params.shutdownWaitPeriod, ConfigFile: params.configFile, + V0Compatible: params.v0Compatible, V1Compatible: params.v1Compatible, } diff --git a/cmd/bench_test.go b/cmd/bench_test.go index bb347964ab..2b22683d49 100644 --- a/cmd/bench_test.go +++ b/cmd/bench_test.go @@ -683,8 +683,9 @@ func TestBenchMainWithDataE2E(t *testing.T) { params.e2e = true mod := `package a.b + import rego.v1 - x { + x if { data.a.b.c == 42 } ` @@ -731,9 +732,10 @@ func TestBenchMainBadQueryE2E(t *testing.T) { } } -func TestBenchMainV1Compatible(t *testing.T) { +func TestBenchMainCompatibleFlags(t *testing.T) { tests := []struct { note string + v0Compatible bool v1Compatible bool module string query string @@ -741,7 +743,8 @@ func TestBenchMainV1Compatible(t *testing.T) { }{ // These tests are slow, so we're not being completely exhaustive here. { - note: "v0.x, keywords not used", + note: "v0, keywords not used", + v0Compatible: true, module: `package test a[4] { 1 == 1 @@ -749,7 +752,8 @@ a[4] { query: `data.test.a`, }, { - note: "v0.x, no keywords imported", + note: "v0, no keywords imported", + v0Compatible: true, module: `package test a contains 4 if { 1 == 1 @@ -761,7 +765,7 @@ a contains 4 if { }, }, { - note: "v1.0, keywords not used", + note: "v1, keywords not used", v1Compatible: true, module: `package test a[4] { @@ -774,11 +778,21 @@ a[4] { }, }, { - note: "v1.0, no keywords imported", + note: "v1, no keywords imported", v1Compatible: true, module: `package test a contains 4 if { 1 == 1 +}`, + query: `data.test.a`, + }, + { + note: "v0+v1, keywords not used (v0 takes precedence)", + v0Compatible: true, + v1Compatible: true, + module: `package test +a[4] { + 1 == 1 }`, query: `data.test.a`, }, @@ -807,6 +821,7 @@ a contains 4 if { test.WithTempFS(files, func(path string) { params := testBenchParams() _ = params.outputFormat.Set(evalPrettyOutput) + params.v0Compatible = tc.v0Compatible params.v1Compatible = tc.v1Compatible params.e2e = mode.e2e @@ -1303,8 +1318,9 @@ func fakeBenchResults() testing.BenchmarkResult { func testBundle() bundle.Bundle { mod := `package a.b + import rego.v1 - x { + x if { data.a.b.c == 42 } ` diff --git a/cmd/build.go b/cmd/build.go index 7c0b450684..7e33eefdf2 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -44,6 +44,7 @@ type buildParams struct { excludeVerifyFiles []string plugin string ns string + v0Compatible bool v1Compatible bool followSymlinks bool } @@ -257,6 +258,7 @@ against OPA v0.22.0: addSigningPluginFlag(buildCommand.Flags(), &buildParams.plugin) addClaimsFileFlag(buildCommand.Flags(), &buildParams.claimsFile) + addV0CompatibleFlag(buildCommand.Flags(), &buildParams.v0Compatible, false) addV1CompatibleFlag(buildCommand.Flags(), &buildParams.v1Compatible, false) RootCommand.AddCommand(buildCommand) @@ -307,9 +309,14 @@ func dobuild(params buildParams, args []string) error { WithPartialNamespace(params.ns). WithFollowSymlinks(params.followSymlinks) - if params.v1Compatible { - compiler = compiler.WithRegoVersion(ast.RegoV1) + regoVersion := ast.DefaultRegoVersion + if params.v0Compatible { + // v0 takes precedence over v1 + regoVersion = ast.RegoV0 + } else if params.v1Compatible { + regoVersion = ast.RegoV1 } + compiler = compiler.WithRegoVersion(regoVersion) if params.revision.isSet { compiler = compiler.WithRevision(*params.revision.v) diff --git a/cmd/build_test.go b/cmd/build_test.go index 0b8178acde..9eaf359078 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -162,6 +162,8 @@ p if "opa" in input.tools`, params.outputFile = path.Join(root, "bundle.tar.gz") params.capabilities = caps params.bundleMode = tc.bundleMode + // Test capabilities are all pre-v1 + params.v0Compatible = true err := dobuild(params, []string{root}) switch { @@ -223,7 +225,8 @@ func TestBuildErrorDoesNotWriteFile(t *testing.T) { files := map[string]string{ "test.rego": ` package test - p { p } + import rego.v1 + p if { p } `, } @@ -232,7 +235,7 @@ func TestBuildErrorDoesNotWriteFile(t *testing.T) { params.outputFile = path.Join(root, "bundle.tar.gz") err := dobuild(params, []string{root}) - exp := fmt.Sprintf("1 error occurred: %s/test.rego:3: rego_recursion_error: rule data.test.p is recursive: data.test.p -> data.test.p", + exp := fmt.Sprintf("1 error occurred: %s/test.rego:4: rego_recursion_error: rule data.test.p is recursive: data.test.p -> data.test.p", root) if err == nil || err.Error() != exp { t.Fatalf("expected recursion error %q but got: %q", exp, err) @@ -249,7 +252,8 @@ func TestBuildErrorVerifyNonBundle(t *testing.T) { files := map[string]string{ "test.rego": ` package test - p { p } + import rego.v1 + p if { p } `, } @@ -331,10 +335,11 @@ func TestBuildPlanWithPruneUnused(t *testing.T) { files := map[string]string{ "test.rego": ` package test + import rego.v1 - p[1] + p contains 1 - f(x) { p[x] } + f(x) if { p[x] } `, } @@ -399,8 +404,9 @@ func TestBuildPlanWithPrintStatements(t *testing.T) { files := map[string]string{ "test.rego": ` package test + import rego.v1 - p { print("hello") } + p if { print("hello") } `, } @@ -464,9 +470,10 @@ func TestBuildPlanWithPrintStatements(t *testing.T) { func TestBuildPlanWithRegoEntrypointAnnotations(t *testing.T) { tests := []struct { - note string - files map[string]string - err error + note string + files map[string]string + err error + v0Compatible bool }{ { note: "annotated entrypoint", @@ -475,16 +482,18 @@ func TestBuildPlanWithRegoEntrypointAnnotations(t *testing.T) { # METADATA # entrypoint: true package test +import rego.v1 -p[1] +p contains 1 -f(x) { p[x] } +f(x) if { p[x] } `, }, err: nil, }, { - note: "set generation with annotated entrypoint", + note: "set generation with annotated entrypoint (v0)", + v0Compatible: true, files: map[string]string{ "test.rego": ` package test @@ -516,7 +525,8 @@ p contains x if { err: nil, }, { - note: "object generation with annotated entrypoint", + note: "object generation with annotated entrypoint (v0)", + v0Compatible: true, files: map[string]string{ "test.rego": ` package test @@ -567,10 +577,11 @@ p.a.b if { files: map[string]string{ "test.rego": ` package test +import rego.v1 # METADATA # entrypoint: true -p.a.b[i] := x { +p.a.b[i] := x if { x := ["a", "b"][i] } `, @@ -581,12 +592,12 @@ p.a.b[i] := x { note: "no annotated entrypoint", files: map[string]string{ "test.rego": ` - package test +import rego.v1 -p[1] +p contains 1 -f(x) { p[x] } +f(x) if { p[x] } `, }, err: fmt.Errorf("plan compilation requires at least one entrypoint"), @@ -601,6 +612,7 @@ f(x) { p[x] } } params.pruneUnused = true params.outputFile = path.Join(root, "bundle.tar.gz") + params.v0Compatible = tc.v0Compatible // Build should fail if entrypoint is not discovered from annotations. err := dobuild(params, []string{root}) @@ -647,7 +659,7 @@ p2 := 2 manifest: ` { "revision":"", - "rego_version": 0, + "rego_version": %REGO_VERSION%, "roots":[""], "wasm":[{ "entrypoint":"test/p2", @@ -680,7 +692,7 @@ p2 := 2 manifest: ` { "revision":"", - "rego_version": 0, + "rego_version": %REGO_VERSION%, "roots":[""], "wasm":[{ "entrypoint":"test/p2", @@ -732,7 +744,7 @@ bar := "baz" manifest: ` { "revision":"", - "rego_version": 0, + "rego_version": %REGO_VERSION%, "roots":[""], "wasm":[{ "entrypoint":"test/p3", @@ -777,7 +789,7 @@ p := 1 manifest: ` { "revision":"", - "rego_version": 0, + "rego_version": %REGO_VERSION%, "roots":[""], "wasm":[{ "entrypoint":"test/p", @@ -817,7 +829,7 @@ p2 := 2 manifest: ` { "revision":"", - "rego_version": 0, + "rego_version": %REGO_VERSION%, "roots":[""], "wasm":[{ "entrypoint":"test", @@ -845,7 +857,7 @@ p2 := 2 manifest: ` { "revision":"", - "rego_version": 0, + "rego_version": %REGO_VERSION%, "roots":[""], "wasm":[{ "entrypoint":"test", @@ -892,6 +904,9 @@ p2 := 2 tr := tar.NewReader(gr) + expManifest := strings.ReplaceAll(tc.manifest, "%REGO_VERSION%", + fmt.Sprintf("%d", ast.DefaultRegoVersion.Int())) + found := false for { f, err := tr.Next() @@ -907,8 +922,8 @@ p2 := 2 t.Fatal(err) } manifest := util.MustUnmarshalJSON(data) - if !reflect.DeepEqual(manifest, util.MustUnmarshalJSON([]byte(tc.manifest))) { - t.Fatalf("expected manifest\n\n%v\n\nbut got\n\n%v", tc.manifest, string(util.MustMarshalJSON(manifest))) + if !reflect.DeepEqual(manifest, util.MustUnmarshalJSON([]byte(expManifest))) { + t.Fatalf("expected manifest\n\n%v\n\nbut got\n\n%v", expManifest, string(util.MustMarshalJSON(manifest))) } break } @@ -987,11 +1002,13 @@ func TestBuildBundleModeIgnoreFlag(t *testing.T) { func TestBuildBundleModeWithManifestRegoVersion(t *testing.T) { tests := []struct { - note string - roots []string - files map[string]string - expManifest string - expErrs []string + note string + roots []string + files map[string]string + expManifest string + expErrs []string + v0Compatible bool + v1Compatible bool }{ { note: "v0 bundle rego-version", @@ -1090,8 +1107,9 @@ p contains 2 if { }, }, { - note: "multiple bundles with different rego-versions", - roots: []string{"bundle1", "bundle2"}, + note: "multiple bundles with different rego-versions (v0-compatible)", + v0Compatible: true, + roots: []string{"bundle1", "bundle2"}, files: map[string]string{ "bundle1/.manifest": `{ "roots": ["test1"], @@ -1126,6 +1144,44 @@ p[4] { }, expManifest: `{"revision":"","roots":["test1","test2"],"rego_version":0,"file_rego_versions":{"%ROOT%/bundle1/test2.rego":1,"%ROOT%/bundle2/test3.rego":1}}`, }, + { + note: "multiple bundles with different rego-versions (v1-compatible)", + v1Compatible: true, + roots: []string{"bundle1", "bundle2"}, + files: map[string]string{ + "bundle1/.manifest": `{ + "roots": ["test1"], + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "bundle1/test1.rego": `package test1 +p[1] { + input.x == 1 +}`, + "bundle1/test2.rego": `package test1 +p contains 2 if { + input.x == 1 +}`, + "bundle2/.manifest": `{ + "roots": ["test2"], + "rego_version": 1, + "file_rego_versions": { + "*/test4.rego": 0 + } +}`, + "bundle2/test3.rego": `package test2 +p contains 3 if { + input.x == 1 +}`, + "bundle2/test4.rego": `package test2 +p[4] { + input.x == 1 +}`, + }, + expManifest: `{"revision":"","roots":["test1","test2"],"rego_version":1,"file_rego_versions":{"%ROOT%/bundle1/test1.rego":0,"%ROOT%/bundle2/test4.rego":0}}`, + }, } for _, tc := range tests { @@ -1134,6 +1190,8 @@ p[4] { params := newBuildParams() params.outputFile = path.Join(root, "bundle.tar.gz") params.bundleMode = true + params.v0Compatible = tc.v0Compatible + params.v1Compatible = tc.v1Compatible var roots []string if len(tc.roots) == 0 { @@ -1208,6 +1266,7 @@ func TestBuildBundleFromOtherBundles(t *testing.T) { tests := []struct { note string + v0Compatible bool v1Compatible bool bundles map[string]bundleInfo expBundle bundleInfo @@ -1218,21 +1277,19 @@ func TestBuildBundleFromOtherBundles(t *testing.T) { bundles: map[string]bundleInfo{ "bundle.tar.gz": { "policy.rego": `package test -p { - input.x == 1 -}`, + +p := input.x == 1 +`, }, }, expBundle: bundleInfo{ "/data.json": `{} `, - "/.manifest": `{"revision":"","roots":[""],"rego_version":0} + "/.manifest": `{"revision":"","roots":[""],"rego_version":%DEFAULT_REGO_VERSION%} `, "%ROOT%/bundle.tar.gz/policy.rego": `package test -p { - input.x == 1 -} +p := input.x == 1 `, }, }, @@ -1307,6 +1364,32 @@ p { p { input.x == 1 } +`, + }, + }, + { + note: "single v1 bundle, --v0-compatible", + v1Compatible: true, + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +p if { + input.x == 1 +}`, + }, + }, + // We don't expect parse/compile errors, as the bundle rego-version is 0, which overrides the --v1-compatible flag. + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":1} +`, + "%ROOT%/bundle.tar.gz/policy.rego": `package test + +p if { + input.x == 1 +} `, }, }, @@ -1390,7 +1473,87 @@ q contains 1 if { }, }, { - note: "v0 bundle + v1 bundle", + note: "single v1 bundle, v0 per-file override", + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy_0.rego": 0 + } +}`, + "policy_0.rego": `package test +p { + input.x == 1 +}`, + "policy_1.rego": `package test +q contains 1 if { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":1,"file_rego_versions":{"%ROOT%/bundle.tar.gz/policy_0.rego":0}} +`, + "%ROOT%/bundle.tar.gz/policy_0.rego": `package test + +p { + input.x == 1 +} +`, + "%ROOT%/bundle.tar.gz/policy_1.rego": `package test + +q contains 1 if { + input.x == 1 +} +`, + }, + }, + { + note: "single v1 bundle, v0 per-file override, --v0-compatible", + v0Compatible: true, + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy_0.rego": 0 + } +}`, + "policy_0.rego": `package test +p { + input.x == 1 +}`, + "policy_1.rego": `package test +q contains 1 if { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":1,"file_rego_versions":{"%ROOT%/bundle.tar.gz/policy_0.rego":0}} +`, + "%ROOT%/bundle.tar.gz/policy_0.rego": `package test + +p { + input.x == 1 +} +`, + "%ROOT%/bundle.tar.gz/policy_1.rego": `package test + +q contains 1 if { + input.x == 1 +} +`, + }, + }, + { + note: "v0 bundle + v1 bundle, --v0-compatible", + v0Compatible: true, bundles: map[string]bundleInfo{ "bundle_v0.tar.gz": { ".manifest": `{"roots": ["test1"], "rego_version": 0}`, @@ -1492,6 +1655,7 @@ q contains 1 if { params := newBuildParams() params.outputFile = path.Join(root, "bundle.tar.gz") params.bundleMode = true + params.v0Compatible = tc.v0Compatible params.v1Compatible = tc.v1Compatible err := dobuild(params, roots) @@ -1547,6 +1711,8 @@ q contains 1 if { t.Fatal(err) } expVal = strings.ReplaceAll(expVal, "%ROOT%", root) + expVal = strings.ReplaceAll(expVal, "%DEFAULT_REGO_VERSION%", + fmt.Sprintf("%d", ast.DefaultRegoVersion.Int())) if string(b) != expVal { t.Fatalf("expected %v:\n\n%v\n\nbut got:\n\n%v", expName, expVal, string(b)) } @@ -1556,16 +1722,6 @@ q contains 1 if { if !found { t.Fatalf("unexpected file in bundle: %v", f.Name) } - - //if f.Name == "/policy.rego" { - // b, err := io.ReadAll(tr) - // if err != nil { - // t.Fatal(err) - // } - // if string(b) != tc.expBundle["policy.rego"] { - // t.Fatalf("expected policy.rego:\n\n%v\n\nbut got:\n\n%v", tc.expBundle["policy.rego"], string(b)) - // } - //} } } }) @@ -1573,16 +1729,18 @@ q contains 1 if { } } -func TestBuildWithV1CompatibleFlag(t *testing.T) { +func TestBuildWithCompatibleFlags(t *testing.T) { tests := []struct { note string + v0Compatible bool v1Compatible bool files map[string]string expectedFiles map[string]string expectedErr string }{ { - note: "default compatibility: policy with no rego.v1 or future.keywords imports", + note: "v0 compatibility: policy with no rego.v1 or future.keywords imports", + v0Compatible: true, files: map[string]string{ "test.rego": `package test allow if { @@ -1592,7 +1750,8 @@ func TestBuildWithV1CompatibleFlag(t *testing.T) { expectedErr: "rego_parse_error", }, { - note: "default compatibility: policy with rego.v1 imports", + note: "v0 compatibility: policy with rego.v1 imports", + v0Compatible: true, files: map[string]string{ "test.rego": `package test import rego.v1 @@ -1615,7 +1774,8 @@ allow if { }, }, { - note: "default compatibility: policy with future.keywords imports", + note: "v0 compatibility: policy with future.keywords imports", + v0Compatible: true, files: map[string]string{ "test.rego": `package test import future.keywords.if @@ -1638,7 +1798,7 @@ allow if { }, }, { - note: "1.0 compatibility: policy with no rego.v1 or future.keywords imports", + note: "v1 compatibility: policy with no rego.v1 or future.keywords imports", v1Compatible: true, files: map[string]string{ "test.rego": `package test @@ -1659,7 +1819,7 @@ allow if { }, }, { - note: "1.0 compatibility: policy with rego.v1 import", + note: "v1 compatibility: policy with rego.v1 import", v1Compatible: true, files: map[string]string{ "test.rego": `package test @@ -1681,7 +1841,7 @@ allow if { }, }, { - note: "1.0 compatibility: policy with future.keywords import", + note: "v1 compatibility: policy with future.keywords import", v1Compatible: true, files: map[string]string{ "test.rego": `package test @@ -1703,7 +1863,7 @@ allow if { }, }, { - note: "1.0 compatibility: missing keywords", + note: "v1 compatibility: missing keywords", v1Compatible: true, files: map[string]string{ "test.rego": `package test @@ -1713,6 +1873,69 @@ allow if { }, expectedErr: "rego_parse_error", }, + // v0 takes precedence over v1 + { + note: "v0+v1 compatibility: policy with no rego.v1 or future.keywords imports", + v0Compatible: true, + v1Compatible: true, + files: map[string]string{ + "test.rego": `package test + allow if { + 1 < 2 + }`, + }, + expectedErr: "rego_parse_error", + }, + { + note: "v0+v1 compatibility: policy with rego.v1 imports", + v0Compatible: true, + v1Compatible: true, + files: map[string]string{ + "test.rego": `package test + import rego.v1 + allow if { + 1 < 2 + }`, + }, + // Imports are preserved + expectedFiles: map[string]string{ + ".manifest": `{"revision":"","roots":[""],"rego_version":0} +`, + "test.rego": `package test + +import rego.v1 + +allow if { + 1 < 2 +} +`, + }, + }, + { + note: "v0+v1 compatibility: policy with future.keywords imports", + v0Compatible: true, + v1Compatible: true, + files: map[string]string{ + "test.rego": `package test + import future.keywords.if + allow if { + 1 < 2 + }`, + }, + // Imports are preserved + expectedFiles: map[string]string{ + ".manifest": `{"revision":"","roots":[""],"rego_version":0} +`, + "test.rego": `package test + +import future.keywords.if + +allow if { + 1 < 2 +} +`, + }, + }, } for _, tc := range tests { @@ -1720,6 +1943,7 @@ allow if { test.WithTempFS(tc.files, func(root string) { params := newBuildParams() params.outputFile = path.Join(root, "bundle.tar.gz") + params.v0Compatible = tc.v0Compatible params.v1Compatible = tc.v1Compatible err := dobuild(params, []string{root}) @@ -1939,6 +2163,7 @@ foo contains __local1__1 if { test.WithTempFS(tc.files, func(root string) { params := newBuildParams() params.outputFile = path.Join(root, "bundle.tar.gz") + params.v0Compatible = !tc.v1Compatible params.v1Compatible = tc.v1Compatible params.optimizationLevel = 1 @@ -2047,6 +2272,7 @@ func TestBuildWithFollowSymlinks(t *testing.T) { params.outputFile = path.Join(rootDir, "test.tar.gz") params.bundleMode = true params.followSymlinks = true + params.v1Compatible = true err = dobuild(params, []string{bundleDir}) if err != nil { @@ -2074,9 +2300,9 @@ func TestBuildWithFollowSymlinks(t *testing.T) { // map of file name -> file content expectedFiles := map[string]string{ - bundleDir + "/foo.rego": "package foo\n\none = 1", - bundleDir + "/bar.rego": "package foo\n\ntwo = 2", - "/.manifest": `{"revision":"","roots":[""],"rego_version":0}`, + bundleDir + "/foo.rego": "package foo\n\none := 1", + bundleDir + "/bar.rego": "package foo\n\ntwo := 2", + "/.manifest": `{"revision":"","roots":[""],"rego_version":1}`, "/data.json": "{}", } @@ -2159,6 +2385,7 @@ func TestBuildWithFollowSymlinksEntireDir(t *testing.T) { params.outputFile = path.Join(rootDir, "test.tar.gz") params.bundleMode = true params.followSymlinks = true + params.v1Compatible = true err = dobuild(params, []string{symlinkDir + "/linked/"}) if err != nil { @@ -2186,8 +2413,8 @@ func TestBuildWithFollowSymlinksEntireDir(t *testing.T) { // map of file name -> file content expectedFiles := map[string]string{ - path.Join(symlinkDir, "linked", "foo.rego"): "package foo\n\none = 1", - "/.manifest": `{"revision":"","roots":[""],"rego_version":0}`, + path.Join(symlinkDir, "linked", "foo.rego"): "package foo\n\none := 1", + "/.manifest": `{"revision":"","roots":[""],"rego_version":1}`, "/data.json": "{}", } diff --git a/cmd/check.go b/cmd/check.go index a361dc76b0..5fbb4e4f4e 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -27,6 +27,7 @@ type checkParams struct { schema *schemaFlags strict bool regoV1 bool + v0Compatible bool v1Compatible bool } @@ -45,10 +46,14 @@ func (p *checkParams) regoVersion() ast.RegoVersion { if p.regoV1 { return ast.RegoV0CompatV1 } + // The '--v0-compatible' flag takes precedence over the '--v1-compatible' flag. + if p.v0Compatible { + return ast.RegoV0 + } if p.v1Compatible { return ast.RegoV1 } - return ast.RegoV0 + return ast.DefaultRegoVersion } const ( @@ -183,6 +188,7 @@ func init() { addStrictFlag(checkCommand.Flags(), &checkParams.strict, false) addRegoV1FlagWithDescription(checkCommand.Flags(), &checkParams.regoV1, false, "check for Rego v1 compatibility (policies must also be compatible with current OPA version)") + addV0CompatibleFlag(checkCommand.Flags(), &checkParams.v0Compatible, false) addV1CompatibleFlag(checkCommand.Flags(), &checkParams.v1Compatible, false) RootCommand.AddCommand(checkCommand) } diff --git a/cmd/check_test.go b/cmd/check_test.go index d19d3c7cf4..4e4e97d6cf 100644 --- a/cmd/check_test.go +++ b/cmd/check_test.go @@ -136,6 +136,8 @@ import rego.v1`, params := newCheckParams() params.capabilities = caps params.bundleMode = tc.bundleMode + // Capabilities in test cases is pre v1 + params.v0Compatible = true err := checkModules(params, []string{root}) switch { @@ -211,10 +213,11 @@ func TestCheckFailsOnInvalidRego(t *testing.T) { func TestCheckWithSchemasAnnotationButNoSchemaFlag(t *testing.T) { policyWithSchemaRef := ` package test +import rego.v1 # METADATA # schemas: # - input: schema["input"] -p { +p if { rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation input.foo == 42 # type mismatch with schema that should be ignored }` @@ -226,10 +229,11 @@ p { policyWithInlinedSchema := ` package test +import rego.v1 # METADATA # schemas: # - input.foo: {"type": "boolean"} -p { +p if { rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation input.foo == 42 # type mismatch with schema that should be ignored }` @@ -372,14 +376,45 @@ p contains x if { } } -func TestCheckV1Compatible(t *testing.T) { +func TestCheckCompatibleFlags(t *testing.T) { cases := []struct { - note string - policy string - expErrs []string + note string + v0Compatible bool + v1Compatible bool + policy string + expErrs []string }{ { - note: "rego.v1 imported, v1 compliant", + note: "v0, no illegal keywords", + v0Compatible: true, + policy: `package test +p[x] { + x := [1,2,3] +}`, + }, + { + note: "v0, illegal keywords", + v0Compatible: true, + 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", + }, + }, + { + note: "v0, future.keywords imported", + v0Compatible: true, + policy: `package test +import future.keywords +p contains x if { + x := [1,2,3] +}`, + }, + { + note: "v0, rego.v1 imported", + v0Compatible: true, policy: `package test import rego.v1 p contains x if { @@ -387,7 +422,17 @@ p contains x if { }`, }, { - note: "rego.v1 imported, NOT v1 compliant (parser)", + note: "v1, rego.v1 imported, v1 compliant", + v1Compatible: true, + policy: `package test +import rego.v1 +p contains x if { + x := [1,2,3] +}`, + }, + { + note: "v1, rego.v1 imported, NOT v1 compliant (parser)", + v1Compatible: true, policy: `package test import rego.v1 p contains x { @@ -401,7 +446,8 @@ q.r`, }, }, { - note: "rego.v1 imported, NOT v1 compliant (compiler)", + note: "v1, rego.v1 imported, NOT v1 compliant (compiler)", + v1Compatible: true, policy: `package test import rego.v1 @@ -413,7 +459,8 @@ import data.bar as foo }, }, { - note: "keywords imported, v1 compliant", + note: "v1, keywords imported, v1 compliant", + v1Compatible: true, policy: `package test import future.keywords.if import future.keywords.contains @@ -422,7 +469,8 @@ p contains x if { }`, }, { - note: "keywords imported, NOT v1 compliant", + note: "v1, keywords imported, NOT v1 compliant", + v1Compatible: true, policy: `package test import future.keywords.contains p contains x { @@ -436,7 +484,8 @@ q.r`, }, }, { - note: "keywords imported, NOT v1 compliant (compiler)", + note: "v1, keywords imported, NOT v1 compliant (compiler)", + v1Compatible: true, policy: `package test import future.keywords.if @@ -448,13 +497,15 @@ input := 1 if { }, }, { - note: "no imports, v1 compliant", + note: "v1, no imports, v1 compliant", + v1Compatible: true, policy: `package test p := 1 `, }, { - note: "no imports, NOT v1 compliant but v0 compliant (compiler)", + note: "v1, no imports, NOT v1 compliant but v0 compliant (compiler)", + v1Compatible: true, policy: `package test p.x`, expErrs: []string{ @@ -462,8 +513,51 @@ p.x`, }, }, { - note: "no imports, v1 compliant but NOT v0 compliant", + note: "v1, no imports, v1 compliant but NOT v0 compliant", + v1Compatible: true, + policy: `package test +p contains x if { + x := [1,2,3] +}`, + }, + // v0 takes precedence over v1 + { + note: "v0+v1, no illegal keywords", + v0Compatible: true, + v1Compatible: true, + policy: `package test +p[x] { + x := [1,2,3] +}`, + }, + { + note: "v0+v1, illegal keywords", + v0Compatible: true, + v1Compatible: true, 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", + }, + }, + { + note: "v0+v1, future.keywords imported", + v0Compatible: true, + v1Compatible: true, + policy: `package test +import future.keywords +p contains x if { + x := [1,2,3] +}`, + }, + { + note: "v0+v1, rego.v1 imported", + v0Compatible: true, + v1Compatible: true, + policy: `package test +import rego.v1 p contains x if { x := [1,2,3] }`, @@ -478,7 +572,8 @@ p contains x if { test.WithTempFS(files, func(root string) { params := newCheckParams() - params.v1Compatible = true + params.v0Compatible = tc.v0Compatible + params.v1Compatible = tc.v1Compatible err := checkModules(params, []string{root}) switch { diff --git a/cmd/deps.go b/cmd/deps.go index 823758faa5..72c29b278b 100644 --- a/cmd/deps.go +++ b/cmd/deps.go @@ -26,14 +26,19 @@ type depsCommandParams struct { outputFormat *util.EnumFlag ignore []string bundlePaths repeatedStringFlag + v0Compatible bool v1Compatible bool } func (p *depsCommandParams) regoVersion() ast.RegoVersion { + // The '--v0-compatible' flag takes precedence over the '--v1-compatible' flag. + if p.v0Compatible { + return ast.RegoV0 + } if p.v1Compatible { return ast.RegoV1 } - return ast.RegoV0 + return ast.DefaultRegoVersion } const ( diff --git a/cmd/deps_test.go b/cmd/deps_test.go index 8f9750dccf..07b875774d 100644 --- a/cmd/deps_test.go +++ b/cmd/deps_test.go @@ -16,16 +16,18 @@ import ( "github.com/open-policy-agent/opa/util/test" ) -func TestDepsV1Compatible(t *testing.T) { +func TestDepsCompatibleFlags(t *testing.T) { tests := []struct { note string + v0Compatible bool v1Compatible bool module string query string expErrs []string }{ { - note: "v0.x, no keywords", + note: "v0, no keywords", + v0Compatible: true, module: `package test p[3] { input.x = 1 @@ -33,7 +35,8 @@ p[3] { query: `data.test.p`, }, { - note: "v0.x, keywords not imported, but used", + note: "v0, keywords not imported, but used", + v0Compatible: true, module: `package test p contains 3 if { input.x = 1 @@ -45,7 +48,8 @@ p contains 3 if { }, }, { - note: "v0.x, keywords imported", + note: "v0, keywords imported", + v0Compatible: true, module: `package test import future.keywords p contains 3 if { @@ -54,7 +58,8 @@ p contains 3 if { query: `data.test.p`, }, { - note: "v0.x, rego.v1 imported", + note: "v0, rego.v1 imported", + v0Compatible: true, module: `package test import rego.v1 p contains 3 if { @@ -63,7 +68,7 @@ p contains 3 if { query: `data.test.p`, }, { - note: "v1.0, no keywords", + note: "v1, no keywords", v1Compatible: true, module: `package test p[3] { @@ -76,7 +81,7 @@ p[3] { }, }, { - note: "v1.0, no keyword imports", + note: "v1, no keyword imports", v1Compatible: true, module: `package test p contains 3 if { @@ -85,7 +90,7 @@ p contains 3 if { query: `data.test.p`, }, { - note: "v1.0, keywords imported", + note: "v1, keywords imported", v1Compatible: true, module: `package test import future.keywords @@ -95,7 +100,54 @@ p contains 3 if { query: `data.test.p`, }, { - note: "v1.0, rego.v1 imported", + note: "v1, rego.v1 imported", + v1Compatible: true, + module: `package test +import rego.v1 +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + }, + // v0 takes precedence over v1 + { + note: "v0+v1, no keywords", + v0Compatible: true, + v1Compatible: true, + module: `package test +p[3] { + input.x = 1 +}`, + query: `data.test.p`, + }, + { + note: "v0+v1, keywords not imported, but used", + v0Compatible: true, + v1Compatible: true, + module: `package test +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v0+v1, keywords imported", + v0Compatible: true, + v1Compatible: true, + module: `package test +import future.keywords +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + }, + { + note: "v0+v1, rego.v1 imported", + v0Compatible: true, v1Compatible: true, module: `package test import rego.v1 @@ -114,6 +166,7 @@ p contains 3 if { test.WithTempFS(files, func(rootPath string) { params := newDepsCommandParams() + params.v0Compatible = tc.v0Compatible params.v1Compatible = tc.v1Compatible _ = params.outputFormat.Set(depsFormatPretty) diff --git a/cmd/eval.go b/cmd/eval.go index b10bf6459d..49e809331b 100644 --- a/cmd/eval.go +++ b/cmd/eval.go @@ -71,10 +71,20 @@ type evalCommandParams struct { optimizationLevel int entrypoints repeatedStringFlag strict bool + v0Compatible bool v1Compatible bool traceVarValues bool } +func (p *evalCommandParams) regoVersion() ast.RegoVersion { + if p.v0Compatible { + return ast.RegoV0 + } else if p.v1Compatible { + return ast.RegoV1 + } + return ast.DefaultRegoVersion +} + func newEvalCommandParams() evalCommandParams { return evalCommandParams{ capabilities: newcapabilitiesFlag(), @@ -332,6 +342,7 @@ access. addTargetFlag(evalCommand.Flags(), params.target) addCountFlag(evalCommand.Flags(), ¶ms.count, "benchmark") addStrictFlag(evalCommand.Flags(), ¶ms.strict, false) + addV0CompatibleFlag(evalCommand.Flags(), ¶ms.v0Compatible, false) addV1CompatibleFlag(evalCommand.Flags(), ¶ms.v1Compatible, false) RootCommand.AddCommand(evalCommand) @@ -535,7 +546,12 @@ func setupEval(args []string, params evalCommandParams) (*evalContext, error) { return nil, err } - regoArgs := []func(*rego.Rego){rego.Query(query), rego.Runtime(info)} + regoArgs := []func(*rego.Rego){ + rego.Query(query), + rego.Runtime(info), + rego.SetRegoVersion(params.regoVersion()), + } + evalArgs := []rego.EvalOption{ rego.EvalRuleIndexing(!params.disableIndexing), rego.EvalEarlyExit(!params.disableEarlyExit), @@ -675,10 +691,6 @@ func setupEval(args []string, params evalCommandParams) (*evalContext, error) { regoArgs = append(regoArgs, rego.Strict(params.strict)) } - if params.v1Compatible { - regoArgs = append(regoArgs, rego.SetRegoVersion(ast.RegoV1)) - } - evalCtx := &evalContext{ params: params, metrics: m, @@ -856,7 +868,8 @@ func generateOptimizedBundle(params evalCommandParams, asBundle bool, filter loa WithEntrypoints(params.entrypoints.v...). WithRegoAnnotationEntrypoints(true). WithPaths(paths...). - WithFilter(filter) + WithFilter(filter). + WithRegoVersion(params.regoVersion()) err := compiler.Build(context.Background()) if err != nil { diff --git a/cmd/eval_test.go b/cmd/eval_test.go index 5e4ea7ef55..b6efce8d5d 100755 --- a/cmd/eval_test.go +++ b/cmd/eval_test.go @@ -62,12 +62,13 @@ func TestEvalExitCode(t *testing.T) { func TestEvalWithShowBuiltinErrors(t *testing.T) { files := map[string]string{ "x.rego": `package x +import rego.v1 -p { +p if { 1/0 } -q { +q if { 1/0 }`, } @@ -131,8 +132,9 @@ q { func TestEvalWithProfiler(t *testing.T) { files := map[string]string{ "x.rego": `package x +import rego.v1 -p { +p if { a := 1 b := 2 c := 3 @@ -166,7 +168,7 @@ p { expectedNumEval := []int{3, 1, 1, 1, 1} expectedNumRedo := []int{3, 1, 1, 1, 1} - expectedRow := []int{7, 6, 5, 4, 1} + expectedRow := []int{8, 7, 6, 5, 1} expectedNumGenExpr := []int{3, 1, 1, 1, 1} for idx, actualExprStat := range output.Profile { @@ -270,9 +272,10 @@ func TestEvalWithOptimize(t *testing.T) { files := map[string]string{ "test.rego": ` package test + import rego.v1 default p = false - p { q } - q { input.x = data.foo }`, + p if { q } + q if { input.x = data.foo }`, "data.json": ` {"foo": 1}`, } @@ -300,8 +303,9 @@ func TestEvalIssue5368(t *testing.T) { files := map[string]string{ "test.rego": ` package system +import rego.v1 -object_key_exists(object, key) { +object_key_exists(object, key) if { _ = object[key] } @@ -309,7 +313,7 @@ default main = false # METADATA # entrypoint: true -main := results { +main := results if { object_key_exists(input, "queries") results := {key: result | result := input.queries[key] @@ -338,9 +342,10 @@ func TestEvalWithOptimizeBundleData(t *testing.T) { files := map[string]string{ "test.rego": ` package test + import rego.v1 default p = false - p { q } - q { input.x = data.foo }`, + p if { q } + q if { input.x = data.foo }`, "data.json": ` {"foo": 1}`, } @@ -512,10 +517,12 @@ func testEvalWithSchemasAnnotationButNoSchemaFlag(policy string) error { func TestEvalWithSchemasAnnotationButNoSchemaFlag(t *testing.T) { policyWithSchemaRef := ` package test +import rego.v1 + # METADATA # schemas: # - input: schema["input"] -p { +p if { rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation input.foo == 42 # type mismatch with schema that should be ignored }` @@ -527,10 +534,12 @@ p { policyWithInlinedSchema := ` package test +import rego.v1 + # METADATA # schemas: # - input.foo: {"type": "boolean"} -p { +p if { rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation input.foo == 42 # type mismatch with schema that should NOT be ignored since it is an inlined schema format }` @@ -671,10 +680,12 @@ func TestEvalWithJSONSchema(t *testing.T) { policyWithSchemasAnnotation := ` package test +import rego.v1 + # METADATA # schemas: # - input: schema -p { +p if { input.foo == 42 # type mismatch }` err = testEvalWithSchemaFile(t, input, query, schema, policyWithSchemasAnnotation, true) @@ -684,10 +695,12 @@ p { policyWithInlinedSchemasAnnotation := ` package test +import rego.v1 + # METADATA # schemas: # - input.foo: {"type": "boolean"} -p { +p if { input.foo == 42 # type mismatch }` err = testEvalWithSchemaFile(t, input, query, schema, policyWithInlinedSchemasAnnotation, true) @@ -747,7 +760,12 @@ func TestEvalWithSchemaFileWithRemoteRef(t *testing.T) { files := map[string]string{ "input.json": input, "schema.json": fmt.Sprintf(schemaFmt, ts.URL), - "p.rego": "package p\nr { input.metadata.clusterName == \"NAME\" }", + "p.rego": `package p +import rego.v1 + +r if { + input.metadata.clusterName == "NAME" +}`, } t.Run("all remote refs disabled", func(t *testing.T) { @@ -924,11 +942,13 @@ func TestEvalWithRegoEntrypointAnnotations(t *testing.T) { files := map[string]string{ "test.rego": ` package test +import rego.v1 + default p = false # METADATA # entrypoint: true -p { q } -q { input.x = data.foo }`, +p if { q } +q if { input.x = data.foo }`, "data.json": ` {"foo": 1}`, } @@ -1108,13 +1128,14 @@ func TestEvalDebugTraceJSONOutput(t *testing.T) { params.disableIndexing = true mod := `package x + import rego.v1 - p[a] { + p contains a if { a := input.z a == 1 } - p[b] { + p contains b if { b := input.y b == 1 } @@ -1182,15 +1203,15 @@ func TestEvalDebugTraceJSONOutput(t *testing.T) { expectedEvalLocationsAndVars := []locationAndVars{ { - location: ast.NewLocation(nil, policyFile, 4, 3), // a := input.z + location: ast.NewLocation(nil, policyFile, 5, 3), // a := input.z varBindings: map[string]string{"__local0__": "a"}, }, { - location: ast.NewLocation(nil, policyFile, 5, 3), // a == 1 + location: ast.NewLocation(nil, policyFile, 6, 3), // a == 1 varBindings: map[string]string{"__local0__": "a"}, }, { - location: ast.NewLocation(nil, policyFile, 9, 3), // b := input.y + location: ast.NewLocation(nil, policyFile, 10, 3), // b := input.y varBindings: map[string]string{"__local1__": "b"}, }, } @@ -1678,17 +1699,17 @@ func TestResetExprLocations(t *testing.T) { // support and cases where the locaiton is unset. The default causes support // and exprs with no location information. pq, err := rego.New(rego.Query("data.test.p = x"), rego.Module("test.rego", ` - package test + import rego.v1 default p = false - p { + p if { input.x = q[_] } - q[1] - q[2] + q contains 1 + q contains 2 `)).Partial(context.Background()) if err != nil { @@ -1881,6 +1902,7 @@ p contains __local0__1 if __local0__1 = input.v _ = 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) @@ -2323,7 +2345,8 @@ func TestIfElse(t *testing.T) { }) } -func TestElseNoIf(t *testing.T) { +// TestElseNoIfV0 only applies to v0 Rego +func TestElseNoIfV0(t *testing.T) { files := map[string]string{ "bug.rego": `package bug import future.keywords.if @@ -2339,6 +2362,7 @@ func TestElseNoIf(t *testing.T) { params.optimizationLevel = 1 params.dataPaths = newrepeatedStringFlag([]string{path}) params.entrypoints = newrepeatedStringFlag([]string{"bug/p"}) + params.v0Compatible = true var buf bytes.Buffer @@ -2375,7 +2399,8 @@ func TestElseIf(t *testing.T) { }) } -func TestElseIfElse(t *testing.T) { +// TestElseIfElseV0 only applies to v0 Rego +func TestElseIfElseV0(t *testing.T) { files := map[string]string{ "bug.rego": `package bug import future.keywords.if @@ -2394,6 +2419,7 @@ func TestElseIfElse(t *testing.T) { params.optimizationLevel = 1 params.dataPaths = newrepeatedStringFlag([]string{path}) params.entrypoints = newrepeatedStringFlag([]string{"bug/p"}) + params.v0Compatible = true var buf bytes.Buffer @@ -2476,16 +2502,18 @@ func TestUnexpectedElseIfErr(t *testing.T) { }) } -func TestEvalPolicyWithV1CompatibleFlag(t *testing.T) { +func TestEvalPolicyWithCompatibleFlags(t *testing.T) { tests := []struct { note string + v0Compatible bool v1Compatible bool modules map[string]string query string expectedErr string }{ { - note: "default compatibility: policy with no rego.v1 or future.keywords imports", + note: "v0 compatibility: policy with no rego.v1 or future.keywords imports", + v0Compatible: true, modules: map[string]string{ "test.rego": `package test allow if { @@ -2496,7 +2524,31 @@ func TestEvalPolicyWithV1CompatibleFlag(t *testing.T) { expectedErr: "rego_parse_error", }, { - note: "1.0 compatibility: policy with no rego.v1 or future.keywords imports", + note: "v0 compatibility: policy with rego.v1 import", + v0Compatible: true, + modules: map[string]string{ + "test.rego": `package test + import rego.v1 + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + }, + { + note: "v0 compatibility: policy with future.keywords import", + v0Compatible: true, + modules: map[string]string{ + "test.rego": `package test + import future.keywords + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + }, + { + note: "v1 compatibility: policy with no rego.v1 or future.keywords imports", v1Compatible: true, modules: map[string]string{ "test.rego": `package test @@ -2507,7 +2559,7 @@ func TestEvalPolicyWithV1CompatibleFlag(t *testing.T) { query: "data.test.allow", }, { - note: "1.0 compatibility: policy with rego.v1 import", + note: "v1 compatibility: policy with rego.v1 import", v1Compatible: true, modules: map[string]string{ "test.rego": `package test @@ -2519,7 +2571,7 @@ func TestEvalPolicyWithV1CompatibleFlag(t *testing.T) { query: "data.test.allow", }, { - note: "1.0 compatibility: policy with future.keywords import", + note: "v1 compatibility: policy with future.keywords import", v1Compatible: true, modules: map[string]string{ "test.rego": `package test @@ -2530,6 +2582,56 @@ func TestEvalPolicyWithV1CompatibleFlag(t *testing.T) { }, query: "data.test.allow", }, + { + note: "v0 + v1 compatibility: policy with no rego.v1 or future.keywords imports", + v0Compatible: true, + v1Compatible: true, + modules: map[string]string{ + "test.rego": `package test + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + expectedErr: "rego_parse_error", + }, + { + note: "v0 + v1 compatibility: policy with rego.v1 import", + v0Compatible: true, + v1Compatible: true, + modules: map[string]string{ + "test.rego": `package test + import rego.v1 + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + }, + { + note: "v0 + v1 compatibility: policy with future.keywords import", + v0Compatible: true, + v1Compatible: true, + modules: map[string]string{ + "test.rego": `package test + import future.keywords + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + }, + { + note: "v1 compatibility: policy with no rego.v1 or future.keywords imports", + v1Compatible: true, + modules: map[string]string{ + "test.rego": `package test + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + }, } setup := []struct { @@ -2558,6 +2660,7 @@ func TestEvalPolicyWithV1CompatibleFlag(t *testing.T) { test.WithTempFS(tc.modules, func(path string) { params := newEvalCommandParams() s.commandParams(¶ms, path) + params.v0Compatible = tc.v0Compatible params.v1Compatible = tc.v1Compatible var buf bytes.Buffer @@ -2891,11 +2994,13 @@ p contains 2 if { func TestWithQueryImports(t *testing.T) { tests := []struct { - note string - query string - imports []string - exp string - expErrs []string + note string + query string + imports []string + v0Compatible bool + v1Compatible bool + exp string + expErrs []string }{ { note: "no imports, none required", @@ -2915,20 +3020,28 @@ func TestWithQueryImports(t *testing.T) { exp: "true\n", }, { - note: "future keyword used, invalid rego.v2 imported", - query: `"b" in ["a", "b", "c"]`, - imports: []string{"rego.v2"}, + note: "future keyword used, invalid rego.v2 imported", + v0Compatible: true, + query: `"b" in ["a", "b", "c"]`, + imports: []string{"rego.v2"}, expErrs: []string{ "1:8: rego_parse_error: invalid import `rego.v2`, must be `rego.v1`", }, }, { - note: "future keyword used, no imports", - query: `"b" in ["a", "b", "c"]`, + note: "future keyword used, no imports (v0)", + v0Compatible: true, + query: `"b" in ["a", "b", "c"]`, expErrs: []string{ "1:5: rego_unsafe_var_error: var in is unsafe (hint: `import future.keywords.in` to import a future keyword)", }, }, + { + note: "future keyword used, no imports (v1)", + v1Compatible: true, + query: `"b" in ["a", "b", "c"]`, + exp: "true\n", + }, } for _, tc := range tests { @@ -2936,6 +3049,8 @@ func TestWithQueryImports(t *testing.T) { params := newEvalCommandParams() _ = params.outputFormat.Set(evalPrettyOutput) params.imports = newrepeatedStringFlag(tc.imports) + params.v0Compatible = tc.v0Compatible + params.v1Compatible = tc.v1Compatible var buf bytes.Buffer diff --git a/cmd/exec.go b/cmd/exec.go index 3dc22847db..a26065d61a 100644 --- a/cmd/exec.go +++ b/cmd/exec.go @@ -85,6 +85,7 @@ e.g., opa exec --decision /foo/bar/baz ... cmd.Flags().StringVar(¶ms.LogTimestampFormat, "log-timestamp-format", "", "set log timestamp format (OPA_LOG_TIMESTAMP_FORMAT environment variable)") cmd.Flags().BoolVarP(¶ms.StdIn, "stdin-input", "I", false, "read input document from stdin rather than a static file") cmd.Flags().DurationVar(¶ms.Timeout, "timeout", 0, "set exec timeout with a Go-style duration, such as '5m 30s'. (default unlimited)") + addV0CompatibleFlag(cmd.Flags(), ¶ms.V0Compatible, false) addV1CompatibleFlag(cmd.Flags(), ¶ms.V1Compatible, false) RootCommand.AddCommand(cmd) @@ -126,6 +127,7 @@ func runExecWithContext(ctx context.Context, params *exec.Params) error { Logger: stdLogger, ConsoleLogger: consoleLogger, Ready: ready, + V0Compatible: params.V0Compatible, V1Compatible: params.V1Compatible, }) if err != nil { diff --git a/cmd/exec_test.go b/cmd/exec_test.go index aaa56922da..635d22dffc 100644 --- a/cmd/exec_test.go +++ b/cmd/exec_test.go @@ -33,7 +33,8 @@ func TestExecBasic(t *testing.T) { s := sdk_test.MustNewServer(sdk_test.MockBundle("/bundles/bundle.tar.gz", map[string]string{ "test.rego": ` package system - main["hello"] + import rego.v1 + main contains "hello" `, })) @@ -84,7 +85,8 @@ func TestExecDecisionOption(t *testing.T) { s := sdk_test.MustNewServer(sdk_test.MockBundle("/bundles/bundle.tar.gz", map[string]string{ "test.rego": ` package foo - main["hello"] + import rego.v1 + main contains "hello" `, })) @@ -125,8 +127,8 @@ func TestExecBundleFlag(t *testing.T) { files := map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package system - - main["hello"]`, + import rego.v1 + main contains "hello"`, } test.WithTempFS(files, func(dir string) { @@ -156,22 +158,25 @@ func TestExecBundleFlag(t *testing.T) { }) } -func TestExecV1Compatible(t *testing.T) { +func TestExecCompatibleFlags(t *testing.T) { tests := []struct { note string + v0Compatible bool v1Compatible bool module string expErrs []string }{ { - note: "v0.x, no keywords used", + note: "v0, no keywords used", + v0Compatible: true, module: `package system main["hello"] { input.foo == "bar" }`, }, { - note: "v0.x, no keywords imported", + note: "v0, no keywords imported", + v0Compatible: true, module: `package system main contains "hello" if { input.foo == "bar" @@ -182,7 +187,8 @@ main contains "hello" if { }, }, { - note: "v0.x, keywords imported", + note: "v0, keywords imported", + v0Compatible: true, module: `package system import future.keywords main contains "hello" if { @@ -190,7 +196,8 @@ main contains "hello" if { }`, }, { - note: "v0.x, rego.v1 imported", + note: "v0, rego.v1 imported", + v0Compatible: true, module: `package system import rego.v1 main contains "hello" if { @@ -199,7 +206,7 @@ main contains "hello" if { }, { - note: "v1.0, no keywords used", + note: "v1, no keywords used", v1Compatible: true, module: `package system main["hello"] { @@ -211,7 +218,7 @@ main["hello"] { }, }, { - note: "v1.0, no keywords imported", + note: "v1, no keywords imported", v1Compatible: true, module: `package system main contains "hello" if { @@ -219,7 +226,7 @@ main contains "hello" if { }`, }, { - note: "v1.0, keywords imported", + note: "v1, keywords imported", v1Compatible: true, module: `package system import future.keywords @@ -228,7 +235,51 @@ main contains "hello" if { }`, }, { - note: "v1.0, rego.v1 imported", + note: "v1, rego.v1 imported", + v1Compatible: true, + module: `package system +import rego.v1 +main contains "hello" if { + input.foo == "bar" +}`, + }, + + // v0 takes precedence over v1 + { + note: "v0+v1, no keywords used", + v0Compatible: true, + v1Compatible: true, + module: `package system +main["hello"] { + input.foo == "bar" +}`, + }, + { + note: "v0+v1, no keywords imported", + v0Compatible: true, + v1Compatible: true, + module: `package system +main contains "hello" if { + input.foo == "bar" +}`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: string cannot be used for rule name", + }, + }, + { + note: "v0+v1, keywords imported", + v0Compatible: true, + v1Compatible: true, + module: `package system +import future.keywords +main contains "hello" if { + input.foo == "bar" +}`, + }, + { + note: "v0+v1, rego.v1 imported", + v0Compatible: true, v1Compatible: true, module: `package system import rego.v1 @@ -254,6 +305,7 @@ main contains "hello" if { var buf bytes.Buffer params := exec.NewParams(&buf) + params.V0Compatible = tc.v0Compatible params.V1Compatible = tc.v1Compatible _ = params.OutputFormat.Set("json") params.ConfigOverrides = []string{ @@ -729,15 +781,16 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package system - - test_fun := x { - x = false - x - } - - undefined_test { - test_fun - }`, + import rego.v1 + + test_fun := x if { + x = false + x + } + + undefined_test if { + test_fun + }`, }, expectError: false, expected: util.MustUnmarshalJSON([]byte(`{"result": [{ @@ -754,8 +807,9 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package system - - main["hello"]`, + import rego.v1 + + main contains "hello"`, }, decision: "", expectError: true, @@ -770,15 +824,16 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package fail.defined.flag + import rego.v1 - some_function { - input.foo == 7 - } - - default fail_test := false - fail_test { - some_function - }`, + some_function if { + input.foo == 7 + } + + default fail_test := false + fail_test if { + some_function + }`, }, decision: "fail/defined/flag/fail_test", expectError: true, @@ -793,11 +848,12 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package fail.defined.flag + import rego.v1 - default fail_test := false - fail_test { - false - }`, + default fail_test := false + fail_test if { + false + }`, }, decision: "fail/defined/flag/fail_test", expectError: true, @@ -812,15 +868,16 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package system + import rego.v1 - test_fun := x { - x = false - x - } - - undefined_test { - test_fun - }`, + test_fun := x if { + x = false + x + } + + undefined_test if { + test_fun + }`, }, expectError: true, expected: util.MustUnmarshalJSON([]byte(`{"result": [{ @@ -837,8 +894,9 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package system - - main["hello"]`, + import rego.v1 + + main contains "hello"`, }, expectError: false, expected: util.MustUnmarshalJSON([]byte(`{"result": [{ @@ -852,15 +910,16 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package fail.defined.flag + import rego.v1 - some_function { - input.foo == 7 - } - - default fail_test := false - fail_test { - some_function - }`, + some_function if { + input.foo == 7 + } + + default fail_test := false + fail_test if { + some_function + }`, }, decision: "fail/defined/flag/fail_test", expectError: false, @@ -875,11 +934,12 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package fail.defined.flag + import rego.v1 - default fail_test := false - fail_test { - false - }`, + default fail_test := false + fail_test if { + false + }`, }, decision: "fail/defined/flag/fail_test", expectError: false, @@ -894,15 +954,16 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package system + import rego.v1 - test_fun := x { - x = false - x - } - - undefined_test { - test_fun - }`, + test_fun := x if { + x = false + x + } + + undefined_test if { + test_fun + }`, }, expectError: false, expected: util.MustUnmarshalJSON([]byte(`{"result": [{ @@ -919,8 +980,9 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package system + import rego.v1 - main["hello"]`, + main contains "hello"`, }, decision: "", expectError: true, @@ -935,15 +997,16 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package fail.non.empty.flag + import rego.v1 - some_function { - input.foo == 7 - } - - default fail_test := false - fail_test { - some_function - }`, + some_function if { + input.foo == 7 + } + + default fail_test := false + fail_test if { + some_function + }`, }, decision: "fail/non/empty/flag/fail_test", expectError: true, @@ -958,11 +1021,12 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package fail.non.empty.flag + import rego.v1 - default fail_test := false - fail_test { - false - }`, + default fail_test := false + fail_test if { + false + }`, }, decision: "fail/non/empty/flag/fail_test", expectError: true, @@ -977,11 +1041,12 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package fail.non.empty.flag + import rego.v1 - default fail_test := ["something", "hello"] - fail_test := [] if { - input.foo == 7 - }`, + default fail_test := ["something", "hello"] + fail_test := [] if { + input.foo == 7 + }`, }, decision: "fail/non/empty/flag/fail_test", expectError: false, @@ -996,11 +1061,12 @@ func TestFailFlagCases(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package fail.non.empty.flag + import rego.v1 - fail_test[message] { - false - message := "not gonna happen" - }`, + fail_test contains message if { + false + message := "not gonna happen" + }`, }, decision: "fail/non/empty/flag/fail_test", expectError: false, @@ -1059,15 +1125,16 @@ func TestExecWithInvalidInputOptions(t *testing.T) { files: map[string]string{ "files/test.json": `{"foo": 7}`, "bundle/x.rego": `package system + import rego.v1 - test_fun := x { - x = false - x - } - - undefined_test { - test_fun - }`, + test_fun := x if { + x = false + x + } + + undefined_test if { + test_fun + }`, }, expectError: false, expected: "", @@ -1076,15 +1143,16 @@ func TestExecWithInvalidInputOptions(t *testing.T) { description: "no paths passed in as args should raise error if --stdin-input flag not set", files: map[string]string{ "bundle/x.rego": `package system + import rego.v1 - test_fun := x { - x = false - x - } - - undefined_test { - test_fun - }`, + test_fun := x if { + x = false + x + } + + undefined_test if { + test_fun + }`, }, expectError: true, expected: "requires at least 1 path arg, or the --stdin-input flag", @@ -1093,15 +1161,16 @@ func TestExecWithInvalidInputOptions(t *testing.T) { description: "should not raise error if --stdin-input flag is set when no paths passed in as args", files: map[string]string{ "bundle/x.rego": `package system + import rego.v1 - test_fun := x { - x = false - x - } - - undefined_test { - test_fun - }`, + test_fun := x if { + x = false + x + } + + undefined_test if { + test_fun + }`, }, stdIn: true, input: `{"foo": 7}`, diff --git a/cmd/flags.go b/cmd/flags.go index 0161900c21..51d8711d8c 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -157,8 +157,12 @@ func addRegoV1FlagWithDescription(fs *pflag.FlagSet, regoV1 *bool, value bool, d fs.BoolVar(regoV1, "rego-v1", value, description) } +func addV0CompatibleFlag(fs *pflag.FlagSet, v1Compatible *bool, value bool) { + fs.BoolVar(v1Compatible, "v0-compatible", value, "opt-in to OPA features and behaviors prior to the OPA v1.0 release. Takes precedence over --v1-compatible") +} + func addV1CompatibleFlag(fs *pflag.FlagSet, v1Compatible *bool, value bool) { - fs.BoolVar(v1Compatible, "v1-compatible", value, "opt-in to OPA features and behaviors that will be enabled by default in a future OPA v1.0 release") + fs.BoolVar(v1Compatible, "v1-compatible", value, "opt-in to OPA features and behaviors that are enabled by default in OPA v1.0") } func addE2EFlag(fs *pflag.FlagSet, e2e *bool, value bool) { diff --git a/cmd/fmt.go b/cmd/fmt.go index bd464e84ea..fb8353f380 100644 --- a/cmd/fmt.go +++ b/cmd/fmt.go @@ -26,6 +26,7 @@ type fmtCommandParams struct { diff bool fail bool regoV1 bool + v0Compatible bool v1Compatible bool checkResult bool } @@ -37,10 +38,14 @@ func (p *fmtCommandParams) regoVersion() ast.RegoVersion { if p.regoV1 { return ast.RegoV0CompatV1 } + // The '--v0-compatible' flag takes precedence over the '--v1-compatible' flag. + if p.v0Compatible { + return ast.RegoV0 + } if p.v1Compatible { return ast.RegoV1 } - return ast.RegoV0 + return ast.DefaultRegoVersion } var formatCommand = &cobra.Command{ @@ -130,6 +135,14 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o opts := format.Opts{ RegoVersion: params.regoVersion(), } + + if params.v0Compatible { + // v0 takes precedence over v1 + opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV0} + } else if params.v1Compatible { + opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV1} + } + formatted, err := format.SourceWithOpts(filename, contents, opts) if err != nil { return newError("failed to format Rego source file: %v", err) @@ -238,6 +251,7 @@ func init() { formatCommand.Flags().BoolVarP(&fmtParams.diff, "diff", "d", false, "only display a diff of the changes") formatCommand.Flags().BoolVar(&fmtParams.fail, "fail", false, "non zero exit code on reformat") addRegoV1FlagWithDescription(formatCommand.Flags(), &fmtParams.regoV1, false, "format module(s) to be compatible with both Rego v1 and current OPA version)") + addV0CompatibleFlag(formatCommand.Flags(), &fmtParams.v0Compatible, false) addV1CompatibleFlag(formatCommand.Flags(), &fmtParams.v1Compatible, false) formatCommand.Flags().BoolVar(&fmtParams.checkResult, "check-result", true, "assert that the formatted code is valid and can be successfully parsed (default true)") diff --git a/cmd/fmt_test.go b/cmd/fmt_test.go index f772546c9f..264e7df67b 100644 --- a/cmd/fmt_test.go +++ b/cmd/fmt_test.go @@ -14,7 +14,7 @@ import ( "github.com/open-policy-agent/opa/util/test" ) -const formatted = `package test +const formattedV0 = `package test p { a == 1 @@ -23,7 +23,16 @@ p { } ` -const unformatted = ` +const formattedV1 = `package test + +p if { + a == 1 + true + 1 + 3 +} +` + +const unformattedV0 = ` package test p { a == 1; true @@ -31,11 +40,22 @@ const unformatted = ` } +` + +const unformattedV1 = ` + package test + + p if{ a == 1; true + 1 + 3 + } + + ` const singleWrongArity = `package test +import rego.v1 -p { +p if { a := 1 b := 2 plus(a, b, c) == 3 @@ -43,8 +63,9 @@ p { ` const MultipleWrongArity = `package test +import rego.v1 -p { +p if { x:=5 y:=7 z:=6 @@ -63,26 +84,49 @@ func (ew errorWriter) Write(_ []byte) (n int, err error) { } func TestFmtFormatFile(t *testing.T) { - params := fmtCommandParams{} - var stdout bytes.Buffer - - files := map[string]string{ - "policy.rego": unformatted, + cases := []struct { + note string + params fmtCommandParams + unformatted string + formatted string + }{ + { + note: "v0", + params: fmtCommandParams{v0Compatible: true}, + unformatted: unformattedV0, + formatted: formattedV0, + }, + { + note: "v1", + params: fmtCommandParams{v1Compatible: true}, + unformatted: unformattedV1, + formatted: formattedV1, + }, } - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + var stdout bytes.Buffer - actual := stdout.String() - if actual != formatted { - t.Fatalf("Expected:%s\n\nGot:\n%s\n\n", formatted, actual) - } - }) + files := map[string]string{ + "policy.rego": tc.unformatted, + } + + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + actual := stdout.String() + if actual != tc.formatted { + t.Fatalf("Expected:%s\n\nGot:\n%s\n\n", tc.formatted, actual) + } + }) + }) + } } func TestFmtFormatFileFailToReadFile(t *testing.T) { @@ -94,7 +138,7 @@ func TestFmtFormatFileFailToReadFile(t *testing.T) { var stdout = bytes.Buffer{} files := map[string]string{ - "policy.rego": unformatted, + "policy.rego": unformattedV0, } notThere := "notThere.rego" @@ -116,249 +160,485 @@ func TestFmtFormatFileFailToReadFile(t *testing.T) { } func TestFmtFormatFileNoChanges(t *testing.T) { - params := fmtCommandParams{} - var stdout bytes.Buffer - - files := map[string]string{ - "policy.rego": formatted, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{v0Compatible: true}, + module: formattedV0, + }, + { + note: "v1", + params: fmtCommandParams{v1Compatible: true}, + module: formattedV1, + }, } - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + var stdout bytes.Buffer - actual := stdout.String() - if actual != formatted { - t.Fatalf("Expected:%s\n\nGot:\n%s\n\n", formatted, actual) - } - }) + files := map[string]string{ + "policy.rego": tc.module, + } + + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + actual := stdout.String() + if actual != tc.module { + t.Fatalf("Expected:%s\n\nGot:\n%s\n\n", tc.module, actual) + } + }) + }) + } } func TestFmtFailFormatFileNoChanges(t *testing.T) { - params := fmtCommandParams{ - fail: true, - diff: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + fail: true, + diff: true, + }, + module: formattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + fail: true, + diff: true, + }, + module: formattedV1, + }, } - var stdout bytes.Buffer - files := map[string]string{ - "policy.rego": formatted, - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + var stdout bytes.Buffer - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } + files := map[string]string{ + "policy.rego": tc.module, + } - actual := stdout.String() - if len(actual) > 0 { - t.Fatalf("Expected no output, got:\n%v\n\n", actual) - } - }) + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + actual := stdout.String() + if len(actual) > 0 { + t.Fatalf("Expected no output, got:\n%v\n\n", actual) + } + }) + }) + } } func TestFmtFormatFileDiff(t *testing.T) { - params := fmtCommandParams{ - diff: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + diff: true, + }, + module: formattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + diff: true, + }, + module: formattedV1, + }, } - var stdout bytes.Buffer - files := map[string]string{ - "policy.rego": formatted, - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + var stdout bytes.Buffer - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } + files := map[string]string{ + "policy.rego": tc.module, + } - actual := stdout.String() + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } - if len(actual) > 0 { - t.Fatalf("Expected no output, got:\n%s\n\n", actual) - } - }) + actual := stdout.String() + + if len(actual) > 0 { + t.Fatalf("Expected no output, got:\n%s\n\n", actual) + } + }) + }) + } } func TestFmtFormatFileFailToPrintDiff(t *testing.T) { - - params := fmtCommandParams{ - diff: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + diff: true, + }, + module: unformattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + diff: true, + }, + module: unformattedV1, + }, } - errMsg := "io.Write error" - var stdout = errorWriter{ErrMsg: errMsg} + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + errMsg := "io.Write error" + var stdout = errorWriter{ErrMsg: errMsg} - files := map[string]string{ - "policy.rego": unformatted, - } + files := map[string]string{ + "policy.rego": tc.module, + } - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err == nil { - t.Fatalf("Expected error, found none") - } + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err == nil { + t.Fatalf("Expected error, found none") + } - actual := err.Error() + actual := err.Error() - if !strings.Contains(actual, errMsg) { - t.Fatalf("Expected error message to include %s, got:\n%s\n\n", errMsg, actual) - } - }) + if !strings.Contains(actual, errMsg) { + t.Fatalf("Expected error message to include %s, got:\n%s\n\n", errMsg, actual) + } + }) + }) + } } func TestFmtFormatFileList(t *testing.T) { - params := fmtCommandParams{ - list: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + list: true, + }, + module: formattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + list: true, + }, + module: formattedV1, + }, } - var stdout bytes.Buffer - files := map[string]string{ - "policy.rego": formatted, - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + var stdout bytes.Buffer - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } + files := map[string]string{ + "policy.rego": tc.module, + } + + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } - actual := strings.TrimSpace(stdout.String()) + actual := strings.TrimSpace(stdout.String()) - if len(actual) > 0 { - t.Fatalf("Expected no output, got:\n%s\n\n", actual) - } - }) + if len(actual) > 0 { + t.Fatalf("Expected no output, got:\n%s\n\n", actual) + } + }) + }) + } } func TestFmtFailFormatFileList(t *testing.T) { - params := fmtCommandParams{ - fail: true, - list: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + fail: true, + list: true, + }, + module: formattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + fail: true, + list: true, + }, + module: formattedV1, + }, } - var stdout bytes.Buffer - files := map[string]string{ - "policy.rego": formatted, - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + var stdout bytes.Buffer - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } + files := map[string]string{ + "policy.rego": tc.module, + } - actual := strings.TrimSpace(stdout.String()) - if len(actual) > 0 { - t.Fatalf("Expected no output, got:\n%v\n\n", actual) - } - }) + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + actual := strings.TrimSpace(stdout.String()) + if len(actual) > 0 { + t.Fatalf("Expected no output, got:\n%v\n\n", actual) + } + }) + }) + } } func TestFmtFailFormatFileChangesList(t *testing.T) { - params := fmtCommandParams{ - fail: true, - list: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + fail: true, + list: true, + }, + module: unformattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + fail: true, + list: true, + }, + module: unformattedV1, + }, } - var stdout bytes.Buffer - files := map[string]string{ - "policy.rego": unformatted, - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + var stdout bytes.Buffer - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err == nil { - t.Fatalf("Unexpected error: %v", err) - } + files := map[string]string{ + "policy.rego": tc.module, + } - actual := strings.TrimSpace(stdout.String()) - if len(actual) == 0 { - t.Fatalf("Expected output, got:\n%v\n\n", actual) - } - }) + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err == nil { + t.Fatalf("Unexpected error: %v", err) + } + + actual := strings.TrimSpace(stdout.String()) + if len(actual) == 0 { + t.Fatalf("Expected output, got:\n%v\n\n", actual) + } + }) + }) + } } func TestFmtFailFileNoChanges(t *testing.T) { - params := fmtCommandParams{ - fail: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + fail: true, + }, + module: formattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + fail: true, + }, + module: formattedV1, + }, } - files := map[string]string{ - "policy.rego": formatted, - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "policy.rego": tc.module, + } - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, io.Discard, policyFile, info, err) - if err != nil { - t.Fatalf("Expected error but did not receive one") - } - }) + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, io.Discard, policyFile, info, err) + if err != nil { + t.Fatalf("Expected error but did not receive one") + } + }) + }) + } } func TestFmtFailFileChanges(t *testing.T) { - params := fmtCommandParams{ - fail: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + fail: true, + }, + module: unformattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + fail: true, + }, + module: unformattedV1, + }, } - files := map[string]string{ - "policy.rego": unformatted, - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "policy.rego": tc.module, + } - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, io.Discard, policyFile, info, err) - if err == nil { - t.Fatalf("Unexpected error: %s", err) - } - }) + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, io.Discard, policyFile, info, err) + if err == nil { + t.Fatalf("Unexpected error: %s", err) + } + }) + }) + } } func TestFmtFailFileChangesDiff(t *testing.T) { - params := fmtCommandParams{ - diff: true, - fail: true, + cases := []struct { + note string + params fmtCommandParams + module string + }{ + { + note: "v0", + params: fmtCommandParams{ + v0Compatible: true, + diff: true, + fail: true, + }, + module: unformattedV0, + }, + { + note: "v1", + params: fmtCommandParams{ + v1Compatible: true, + diff: true, + fail: true, + }, + module: unformattedV1, + }, } - var stdout bytes.Buffer - files := map[string]string{ - "policy.rego": unformatted, - } + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + var stdout bytes.Buffer - test.WithTempFS(files, func(path string) { - policyFile := filepath.Join(path, "policy.rego") - info, err := os.Stat(policyFile) - err = formatFile(¶ms, &stdout, policyFile, info, err) - if err == nil { - t.Fatalf("Unexpected error: %v", err) - } + files := map[string]string{ + "policy.rego": tc.module, + } - actual := strings.TrimSpace(stdout.String()) - if len(actual) == 0 { - t.Fatalf("Expected output, got:\n%v\n\n", actual) - } - }) + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(&tc.params, &stdout, policyFile, info, err) + if err == nil { + t.Fatalf("Unexpected error: %v", err) + } + + actual := strings.TrimSpace(stdout.String()) + if len(actual) == 0 { + t.Fatalf("Expected output, got:\n%v\n\n", actual) + } + }) + }) + } } func TestFmtSingleWrongArityError(t *testing.T) { @@ -377,7 +657,7 @@ func TestFmtSingleWrongArityError(t *testing.T) { t.Fatalf("Expected error but did not receive one") } - loc := ast.Location{File: policyFile, Row: 6} + loc := ast.Location{File: policyFile, Row: 7} errExp := ast.NewError(ast.TypeErr, &loc, "%s: %s", "plus", "arity mismatch") errExp.Details = &format.ArityFormatErrDetail{ Have: []string{"var", "var", "var"}, @@ -410,9 +690,9 @@ func TestFmtMultipleWrongArityError(t *testing.T) { } locations := []ast.Location{ - {File: policyFile, Row: 7}, {File: policyFile, Row: 8}, {File: policyFile, Row: 9}, + {File: policyFile, Row: 10}, } haveStrings := [][]string{ {"array"}, @@ -543,7 +823,9 @@ q := all([true, false]) for _, tc := range tests { t.Run(tc.note, func(t *testing.T) { params := fmtCommandParams{ - regoV1: true, + // Locking rego-version to v0, as it's only then the --rego-v1 flag is relevant + v0Compatible: true, + regoV1: true, } files := map[string]string{ diff --git a/cmd/internal/exec/params.go b/cmd/internal/exec/params.go index c45d85aa1c..28f0521e98 100644 --- a/cmd/internal/exec/params.go +++ b/cmd/internal/exec/params.go @@ -26,6 +26,7 @@ type Params struct { FailNonEmpty bool // exits with non-zero exit code on non-empty set (array) results StdIn bool // pull input from std-in, rather than input files Timeout time.Duration // timeout to prevent infinite hangs. If set to 0, the command will never time out + V0Compatible bool // use OPA 0.x compatibility mode V1Compatible bool // use OPA 1.0 compatibility mode Logger logging.Logger // Logger override. If set to nil, the default logger is used. } diff --git a/cmd/oracle.go b/cmd/oracle.go index d688aa0b68..31f0cf5a90 100644 --- a/cmd/oracle.go +++ b/cmd/oracle.go @@ -23,8 +23,21 @@ import ( ) type findDefinitionParams struct { - stdinBuffer bool - bundlePaths repeatedStringFlag + stdinBuffer bool + bundlePaths repeatedStringFlag + v0Compatible bool + v1Compatible bool +} + +func (p *findDefinitionParams) regoVersion() ast.RegoVersion { + // v0 takes precedence over v1 + if p.v0Compatible { + return ast.RegoV0 + } + if p.v1Compatible { + return ast.RegoV1 + } + return ast.DefaultRegoVersion } func init() { @@ -93,6 +106,8 @@ by the input location.`, findDefinitionCommand.Flags().BoolVarP(&findDefinitionParams.stdinBuffer, "stdin-buffer", "", false, "read buffer from stdin") addBundleFlag(findDefinitionCommand.Flags(), &findDefinitionParams.bundlePaths) oracleCommand.AddCommand(findDefinitionCommand) + addV0CompatibleFlag(oracleCommand.Flags(), &findDefinitionParams.v0Compatible, false) + addV1CompatibleFlag(oracleCommand.Flags(), &findDefinitionParams.v1Compatible, false) RootCommand.AddCommand(oracleCommand) } @@ -116,6 +131,7 @@ func dofindDefinition(params findDefinitionParams, stdin io.Reader, stdout io.Wr // only .rego will work reliably for the purpose of finding definitions return strings.HasPrefix(info.Name(), ".rego") }). + WithRegoVersion(params.regoVersion()). AsBundle(params.bundlePaths.v[0]) if err != nil { return err diff --git a/cmd/oracle_test.go b/cmd/oracle_test.go index 2998fbca28..7075daa35e 100644 --- a/cmd/oracle_test.go +++ b/cmd/oracle_test.go @@ -14,50 +14,115 @@ import ( ) func TestOracleFindDefinition(t *testing.T) { - - onDiskModule := `package test + cases := []struct { + note string + v0Compatible bool + v1Compatible bool + onDiskModule string + stdin string + paths []string + }{ + { + note: "v0", + v0Compatible: true, + onDiskModule: `package test p { r } -r = true` - - stdin := bytes.NewBufferString(`package test +r = true`, + stdin: `package test p { q } -q = true`) +q = true`, + paths: []string{ + "test.rego:10", + "test.rego:15", + "test.rego:18", + }, + }, + { + note: "v1", + v1Compatible: true, + onDiskModule: `package test + +p if { r } - files := map[string]string{ - "test.rego": onDiskModule, - "document.txt": "this should not be included", - "ignore.json": `{"neither": "should this"}`, - } +r = true`, + stdin: `package test + +p if { q } + +q = true`, + paths: []string{ + "test.rego:10", + "test.rego:15", + "test.rego:21", + }, + }, + // v0 takes precedence over v1 + { + note: "v0+v1", + v0Compatible: true, + v1Compatible: true, + onDiskModule: `package test - test.WithTempFS(files, func(rootDir string) { +p { r } + +r = true`, + stdin: `package test + +p { q } - params := findDefinitionParams{ - bundlePaths: repeatedStringFlag{ - v: []string{rootDir}, - isSet: true, +q = true`, + paths: []string{ + "test.rego:10", + "test.rego:15", + "test.rego:18", }, - stdinBuffer: true, - } + }, + } + + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + stdin := bytes.NewBufferString(tc.stdin) - stdout := bytes.NewBuffer(nil) + files := map[string]string{ + "test.rego": tc.onDiskModule, + "document.txt": "this should not be included", + "ignore.json": `{"neither": "should this"}`, + } + + test.WithTempFS(files, func(rootDir string) { + + params := findDefinitionParams{ + bundlePaths: repeatedStringFlag{ + v: []string{rootDir}, + isSet: true, + }, + stdinBuffer: true, + v0Compatible: tc.v0Compatible, + v1Compatible: tc.v1Compatible, + } + + stdout := bytes.NewBuffer(nil) - err := dofindDefinition(params, stdin, stdout, []string{path.Join(rootDir, "test.rego:10")}) - expectJSON(t, err, stdout, `{"error": {"code": "oracle_no_match_found"}}`) + err := dofindDefinition(params, stdin, stdout, []string{path.Join(rootDir, tc.paths[0])}) + expectJSON(t, err, stdout, `{"error": {"code": "oracle_no_match_found"}}`) - err = dofindDefinition(params, stdin, stdout, []string{path.Join(rootDir, "test.rego:15")}) - expectJSON(t, err, stdout, `{"error": {"code": "oracle_no_definition_found"}}`) + err = dofindDefinition(params, stdin, stdout, []string{path.Join(rootDir, tc.paths[1])}) + expectJSON(t, err, stdout, `{"error": {"code": "oracle_no_definition_found"}}`) - err = dofindDefinition(params, stdin, stdout, []string{path.Join(rootDir, "test.rego:18")}) - expectJSON(t, err, stdout, fmt.Sprintf(`{"result": { + err = dofindDefinition(params, stdin, stdout, []string{path.Join(rootDir, tc.paths[2])}) + expectJSON(t, err, stdout, fmt.Sprintf(`{"result": { "file": %q, "row": 5, "col": 1 }}`, path.Join(rootDir, "test.rego"))) - }) + }) + }) + } + } func expectJSON(t *testing.T, err error, buffer *bytes.Buffer, exp string) { diff --git a/cmd/parse.go b/cmd/parse.go index 408921fc5e..e525cff6d2 100644 --- a/cmd/parse.go +++ b/cmd/parse.go @@ -29,14 +29,19 @@ const ( type parseParams struct { format *util.EnumFlag jsonInclude string + v0Compatible bool v1Compatible bool } func (p *parseParams) regoVersion() ast.RegoVersion { + // the '--v0--compatible' flag takes precedence over the '--v1-compatible' flag + if p.v0Compatible { + return ast.RegoV0 + } if p.v1Compatible { return ast.RegoV1 } - return ast.RegoV0 + return ast.DefaultRegoVersion } var configuredParseParams = parseParams{ diff --git a/cmd/parse_test.go b/cmd/parse_test.go index 5d3ff49cc4..9edea0be66 100644 --- a/cmd/parse_test.go +++ b/cmd/parse_test.go @@ -487,9 +487,10 @@ func TestParseRulesBlockJSONOutputWithLocations(t *testing.T) { files := map[string]string{ "x.rego": `package x +import rego.v1 default allow = false -allow = true { +allow = true if { input.method == "GET" input.path = ["getUser", user] input.user == user @@ -538,6 +539,47 @@ allow = true { } ] }, + "imports": [ + { + "location": { + "file": "TEMPDIR/x.rego", + "row": 2, + "col": 1, + "text": "aW1wb3J0" + }, + "path": { + "location": { + "file": "TEMPDIR/x.rego", + "row": 2, + "col": 8, + "text": "cmVnby52MQ==" + }, + "type": "ref", + "value": [ + { + "location": { + "file": "TEMPDIR/x.rego", + "row": 2, + "col": 8, + "text": "cmVnbw==" + }, + "type": "var", + "value": "rego" + }, + { + "location": { + "file": "TEMPDIR/x.rego", + "row": 2, + "col": 13, + "text": "djE=" + }, + "type": "string", + "value": "v1" + } + ] + } + } + ], "rules": [ { "body": [ @@ -545,14 +587,14 @@ allow = true { "index": 0, "location": { "file": "TEMPDIR/x.rego", - "row": 3, + "row": 4, "col": 1, "text": "ZGVmYXVsdA==" }, "terms": { "location": { "file": "TEMPDIR/x.rego", - "row": 3, + "row": 4, "col": 1, "text": "ZGVmYXVsdA==" }, @@ -567,7 +609,7 @@ allow = true { "value": { "location": { "file": "TEMPDIR/x.rego", - "row": 3, + "row": 4, "col": 17, "text": "ZmFsc2U=" }, @@ -578,7 +620,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 3, + "row": 4, "col": 9, "text": "YWxsb3c=" }, @@ -588,14 +630,14 @@ allow = true { ], "location": { "file": "TEMPDIR/x.rego", - "row": 3, + "row": 4, "col": 9, "text": "YWxsb3cgPSBmYWxzZQ==" } }, "location": { "file": "TEMPDIR/x.rego", - "row": 3, + "row": 4, "col": 1, "text": "ZGVmYXVsdA==" } @@ -606,7 +648,7 @@ allow = true { "index": 0, "location": { "file": "TEMPDIR/x.rego", - "row": 5, + "row": 6, "col": 3, "text": "aW5wdXQubWV0aG9kID09ICJHRVQi" }, @@ -614,7 +656,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 5, + "row": 6, "col": 16, "text": "PT0=" }, @@ -623,7 +665,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 5, + "row": 6, "col": 16, "text": "PT0=" }, @@ -635,7 +677,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 5, + "row": 6, "col": 3, "text": "aW5wdXQubWV0aG9k" }, @@ -644,7 +686,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 5, + "row": 6, "col": 3, "text": "aW5wdXQ=" }, @@ -654,7 +696,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 5, + "row": 6, "col": 9, "text": "bWV0aG9k" }, @@ -666,7 +708,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 5, + "row": 6, "col": 19, "text": "IkdFVCI=" }, @@ -679,7 +721,7 @@ allow = true { "index": 1, "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 3, "text": "aW5wdXQucGF0aCA9IFsiZ2V0VXNlciIsIHVzZXJd" }, @@ -687,7 +729,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 14, "text": "PQ==" }, @@ -696,7 +738,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 14, "text": "PQ==" }, @@ -708,7 +750,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 3, "text": "aW5wdXQucGF0aA==" }, @@ -717,7 +759,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 3, "text": "aW5wdXQ=" }, @@ -727,7 +769,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 9, "text": "cGF0aA==" }, @@ -739,7 +781,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 16, "text": "WyJnZXRVc2VyIiwgdXNlcl0=" }, @@ -748,7 +790,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 17, "text": "ImdldFVzZXIi" }, @@ -758,7 +800,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 6, + "row": 7, "col": 28, "text": "dXNlcg==" }, @@ -773,7 +815,7 @@ allow = true { "index": 2, "location": { "file": "TEMPDIR/x.rego", - "row": 7, + "row": 8, "col": 3, "text": "aW5wdXQudXNlciA9PSB1c2Vy" }, @@ -781,7 +823,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 7, + "row": 8, "col": 14, "text": "PT0=" }, @@ -790,7 +832,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 7, + "row": 8, "col": 14, "text": "PT0=" }, @@ -802,7 +844,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 7, + "row": 8, "col": 3, "text": "aW5wdXQudXNlcg==" }, @@ -811,7 +853,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 7, + "row": 8, "col": 3, "text": "aW5wdXQ=" }, @@ -821,7 +863,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 7, + "row": 8, "col": 9, "text": "dXNlcg==" }, @@ -833,7 +875,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 7, + "row": 8, "col": 17, "text": "dXNlcg==" }, @@ -848,7 +890,7 @@ allow = true { "value": { "location": { "file": "TEMPDIR/x.rego", - "row": 4, + "row": 5, "col": 9, "text": "dHJ1ZQ==" }, @@ -859,7 +901,7 @@ allow = true { { "location": { "file": "TEMPDIR/x.rego", - "row": 4, + "row": 5, "col": 1, "text": "YWxsb3c=" }, @@ -869,16 +911,16 @@ allow = true { ], "location": { "file": "TEMPDIR/x.rego", - "row": 4, + "row": 5, "col": 1, "text": "YWxsb3cgPSB0cnVl" } }, "location": { "file": "TEMPDIR/x.rego", - "row": 4, + "row": 5, "col": 1, - "text": "YWxsb3cgPSB0cnVlIHsKICBpbnB1dC5tZXRob2QgPT0gIkdFVCIKICBpbnB1dC5wYXRoID0gWyJnZXRVc2VyIiwgdXNlcl0KICBpbnB1dC51c2VyID09IHVzZXIKfQ==" + "text": "YWxsb3cgPSB0cnVlIGlmIHsKICBpbnB1dC5tZXRob2QgPT0gIkdFVCIKICBpbnB1dC5wYXRoID0gWyJnZXRVc2VyIiwgdXNlcl0KICBpbnB1dC51c2VyID09IHVzZXIKfQ==" } } ] @@ -930,22 +972,25 @@ func TestParseJSONOutputComments(t *testing.T) { } } -func TestParseV1Compatible(t *testing.T) { +func TestParseCompatibleFlags(t *testing.T) { tests := []struct { note string + v0Compatible bool v1Compatible bool policy string expErrs []string }{ { - note: "v0.x, keywords not used", + note: "v0, keywords not used", + v0Compatible: true, policy: `package test p[v] { v := input.x }`, }, { - note: "v0.x, keywords not imported", + note: "v0, keywords not imported", + v0Compatible: true, policy: `package test p contains v if { v := input.x @@ -955,7 +1000,8 @@ p contains v if { }, }, { - note: "v0.x, keywords imported", + note: "v0, keywords imported", + v0Compatible: true, policy: `package test import future.keywords p contains v if { @@ -963,7 +1009,8 @@ p contains v if { }`, }, { - note: "v0.x, rego.v1 imported", + note: "v0, rego.v1 imported", + v0Compatible: true, policy: `package test import rego.v1 p contains v if { @@ -972,7 +1019,7 @@ p contains v if { }, { - note: "v1.0, keywords not used", + note: "v1, keywords not used", v1Compatible: true, policy: `package test p[v] { @@ -984,15 +1031,57 @@ p[v] { }, }, { - note: "v1.0, keywords not imported", + note: "v1, keywords not imported", + v1Compatible: true, + policy: `package test +p contains v if { + v := input.x +}`, + }, + { + note: "v1, keywords imported", v1Compatible: true, policy: `package test +import future.keywords +p contains v if { + v := input.x +}`, + }, + { + note: "v1, rego.v1 imported", + v1Compatible: true, + policy: `package test +import rego.v1 p contains v if { v := input.x }`, }, + + // v0 takes precedence over v1 + { + note: "v0+v1, keywords not used", + v0Compatible: true, + v1Compatible: true, + policy: `package test +p[v] { + v := input.x +}`, + }, + { + note: "v0+v1, keywords not imported", + v0Compatible: true, + v1Compatible: true, + policy: `package test +p contains v if { + v := input.x +}`, + expErrs: []string{ + "var cannot be used for rule name", + }, + }, { - note: "v1.0, keywords imported", + note: "v0+1, keywords imported", + v0Compatible: true, v1Compatible: true, policy: `package test import future.keywords @@ -1001,7 +1090,8 @@ p contains v if { }`, }, { - note: "v1.0, rego.v1 imported", + note: "v0+v1, rego.v1 imported", + v0Compatible: true, v1Compatible: true, policy: `package test import rego.v1 @@ -1019,6 +1109,7 @@ p contains v if { _, _, stderr, _ := testParse(t, files, &parseParams{ format: util.NewEnumFlag(parseFormatPretty, []string{parseFormatPretty, parseFormatJSON}), + v0Compatible: tc.v0Compatible, v1Compatible: tc.v1Compatible, }) diff --git a/cmd/refactor.go b/cmd/refactor.go index 4cad94e9cc..7ef187c914 100644 --- a/cmd/refactor.go +++ b/cmd/refactor.go @@ -22,9 +22,22 @@ import ( ) type moveCommandParams struct { - mapping repeatedStringFlag - ignore []string - overwrite bool + mapping repeatedStringFlag + ignore []string + overwrite bool + v0Compatible bool + v1Compatible bool +} + +func (m *moveCommandParams) regoVersion() ast.RegoVersion { + // v0 takes precedence over v1 + if m.v0Compatible { + return ast.RegoV0 + } + if m.v1Compatible { + return ast.RegoV1 + } + return ast.DefaultRegoVersion } func init() { @@ -88,6 +101,8 @@ The 'move' command outputs the below policy to stdout with the package name rewr moveCommand.Flags().BoolVarP(&moveCommandParams.overwrite, "write", "w", false, "overwrite the original source file") addIgnoreFlag(moveCommand.Flags(), &moveCommandParams.ignore) refactorCommand.AddCommand(moveCommand) + addV0CompatibleFlag(moveCommand.Flags(), &moveCommandParams.v0Compatible, false) + addV1CompatibleFlag(moveCommand.Flags(), &moveCommandParams.v1Compatible, false) RootCommand.AddCommand(refactorCommand) } @@ -107,7 +122,9 @@ func doMove(params moveCommandParams, args []string, out io.Writer) error { Ignore: params.ignore, } - result, err := loader.NewFileLoader().Filtered(args, f.Apply) + result, err := loader.NewFileLoader(). + WithRegoVersion(params.regoVersion()). + Filtered(args, f.Apply) if err != nil { return err } diff --git a/cmd/refactor_test.go b/cmd/refactor_test.go index 9279ac8791..c5cd54c0fc 100644 --- a/cmd/refactor_test.go +++ b/cmd/refactor_test.go @@ -13,94 +13,228 @@ import ( ) func TestDoMoveRenamePackage(t *testing.T) { - - files := map[string]string{ - "policy.rego": `package lib.foo - -# this is a comment -default allow = false - -allow { - input.message == "hello" # this is a comment too -}`, + cases := []struct { + note string + v0Compatible bool + v1Compatible bool + module string + expected *ast.Module + }{ + { + note: "v0", + v0Compatible: true, + module: `package lib.foo + + # this is a comment + default allow = false + + allow { + input.message == "hello" # this is a comment too + }`, + expected: ast.MustParseModuleWithOpts(`package baz.bar + + # this is a comment + default allow = false + + allow { + input.message == "hello" # this is a comment too + }`, ast.ParserOptions{RegoVersion: ast.RegoV0}), + }, + { + note: "v1", + v1Compatible: true, + module: `package lib.foo + + # this is a comment + default allow = false + + allow if { + input.message == "hello" # this is a comment too + }`, + expected: ast.MustParseModuleWithOpts(`package baz.bar + + # this is a comment + default allow = false + + allow if { + input.message == "hello" # this is a comment too + }`, ast.ParserOptions{RegoVersion: ast.RegoV1}), + }, + // v0 takes precedence over v1 + { + note: "v0+v1", + v0Compatible: true, + v1Compatible: true, + module: `package lib.foo + + # this is a comment + default allow = false + + allow { + input.message == "hello" # this is a comment too + }`, + expected: ast.MustParseModuleWithOpts(`package baz.bar + + # this is a comment + default allow = false + + allow { + input.message == "hello" # this is a comment too + }`, ast.ParserOptions{RegoVersion: ast.RegoV0}), + }, } - test.WithTempFS(files, func(path string) { - - mappings := []string{"data.lib.foo:data.baz.bar"} + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "policy.rego": tc.module, + } - params := moveCommandParams{ - mapping: newrepeatedStringFlag(mappings), - } + test.WithTempFS(files, func(path string) { - var buf bytes.Buffer + mappings := []string{"data.lib.foo:data.baz.bar"} - err := doMove(params, []string{path}, &buf) - if err != nil { - t.Fatal(err) - } + params := moveCommandParams{ + mapping: newrepeatedStringFlag(mappings), + v0Compatible: tc.v0Compatible, + v1Compatible: tc.v1Compatible, + } - expected := ast.MustParseModule(`package baz.bar + var buf bytes.Buffer -# this is a comment -default allow = false + err := doMove(params, []string{path}, &buf) + if err != nil { + t.Fatal(err) + } -allow { - input.message == "hello" # this is a comment too -}`) + formatted := format.MustAst(tc.expected) - formatted := format.MustAst(expected) - - if !reflect.DeepEqual(formatted, buf.Bytes()) { - t.Fatalf("Expected module:\n%v\n\nGot:\n%v\n", string(formatted), buf.String()) - } - }) + if !reflect.DeepEqual(formatted, buf.Bytes()) { + t.Fatalf("Expected module:\n%v\n\nGot:\n%v\n", string(formatted), buf.String()) + } + }) + }) + } } func TestDoMoveOverwriteFile(t *testing.T) { - - files := map[string]string{ - "policy.rego": `package lib.foo - -import data.x.q - -default allow = false -`, + cases := []struct { + note string + v0Compatible bool + v1Compatible bool + module string + expected *ast.Module + }{ + { + note: "v0", + v0Compatible: true, + module: `package lib.foo + + import data.x.q + + default allow := false + + allow { + input.message == "hello" + } + `, + expected: ast.MustParseModuleWithOpts(`package baz.bar + + import data.hidden.q + + default allow := false + + allow { + input.message == "hello" + }`, ast.ParserOptions{RegoVersion: ast.RegoV0}), + }, + { + note: "v1", + v1Compatible: true, + module: `package lib.foo + + import data.x.q + + default allow := false + + allow if { + input.message == "hello" + } + `, + expected: ast.MustParseModuleWithOpts(`package baz.bar + + import data.hidden.q + + default allow := false + + allow if { + input.message == "hello" + }`, ast.ParserOptions{RegoVersion: ast.RegoV1}), + }, + // v0 takes precedence over v1 + { + note: "v0+v1", + v0Compatible: true, + v1Compatible: true, + module: `package lib.foo + + import data.x.q + + default allow := false + + allow { + input.message == "hello" + } + `, + expected: ast.MustParseModuleWithOpts(`package baz.bar + + import data.hidden.q + + default allow := false + + allow { + input.message == "hello" + }`, ast.ParserOptions{RegoVersion: ast.RegoV0}), + }, } - test.WithTempFS(files, func(path string) { + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "policy.rego": tc.module, + } - mappings := []string{"data.lib.foo:data.baz.bar", "data.x: data.hidden"} + test.WithTempFS(files, func(path string) { - params := moveCommandParams{ - mapping: newrepeatedStringFlag(mappings), - overwrite: true, - } + mappings := []string{"data.lib.foo:data.baz.bar", "data.x: data.hidden"} - var buf bytes.Buffer + params := moveCommandParams{ + mapping: newrepeatedStringFlag(mappings), + overwrite: true, + v0Compatible: tc.v0Compatible, + v1Compatible: tc.v1Compatible, + } - err := doMove(params, []string{path}, &buf) - if err != nil { - t.Fatal(err) - } + var buf bytes.Buffer - data, err := os.ReadFile(filepath.Join(path, "policy.rego")) - if err != nil { - t.Fatal(err) - } + err := doMove(params, []string{path}, &buf) + if err != nil { + t.Fatal(err) + } - actual := ast.MustParseModule(string(data)) + data, err := os.ReadFile(filepath.Join(path, "policy.rego")) + if err != nil { + t.Fatal(err) + } - expected := ast.MustParseModule(`package baz.bar - - import data.hidden.q - - default allow = false`) + actual := ast.MustParseModule(string(data)) - if !expected.Equal(actual) { - t.Fatalf("Expected module:\n%v\n\nGot:\n%v\n", expected, actual) - } - }) + if !tc.expected.Equal(actual) { + t.Fatalf("Expected module:\n%v\n\nGot:\n%v\n", tc.expected, actual) + } + }) + }) + } } func TestParseSrcDstMap(t *testing.T) { diff --git a/cmd/run.go b/cmd/run.go index 28ac319e73..0b9fd80d38 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -220,6 +220,7 @@ See https://godoc.org/crypto/tls#pkg-constants for more information. runCommand.Flags().BoolVar(&cmdParams.rt.H2CEnabled, "h2c", false, "enable H2C for HTTP listeners") runCommand.Flags().StringVarP(&cmdParams.rt.OutputFormat, "format", "f", "pretty", "set shell output format, i.e, pretty, json") runCommand.Flags().BoolVarP(&cmdParams.rt.Watch, "watch", "w", false, "watch command line files for changes") + addV0CompatibleFlag(runCommand.Flags(), &cmdParams.rt.V0Compatible, false) addV1CompatibleFlag(runCommand.Flags(), &cmdParams.rt.V1Compatible, false) addMaxErrorsFlag(runCommand.Flags(), &cmdParams.rt.ErrorLimit) runCommand.Flags().BoolVar(&cmdParams.rt.PprofEnabled, "pprof", false, "enables pprof endpoints") @@ -365,7 +366,8 @@ func initRuntime(ctx context.Context, params runCmdParams, args []string, addrSe rt.SetDistributedTracingLogging() rt.Params.AddrSetByUser = addrSetByUser - if !addrSetByUser && rt.Params.V1Compatible { + // v0 negates v1 + if !addrSetByUser && !rt.Params.V0Compatible && rt.Params.V1Compatible { rt.Params.Addrs = &[]string{defaultLocalAddr} } diff --git a/cmd/run_test.go b/cmd/run_test.go index e1c41ceac4..88730492ce 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -201,10 +201,11 @@ func TestInitRuntimeSkipKnownSchemaCheck(t *testing.T) { fs := map[string]string{ "test/authz.rego": `package system.authz + import rego.v1 default allow := false - allow { + allow if { input.identty = "foo" # this is a typo }`, } diff --git a/cmd/test.go b/cmd/test.go index be266ab29d..7b7dfefe97 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -61,6 +61,7 @@ type testCommandParams struct { stopChan chan os.Signal output io.Writer errOutput io.Writer + v0Compatible bool v1Compatible bool varValues bool } @@ -79,10 +80,14 @@ func newTestCommandParams() testCommandParams { } func (p *testCommandParams) RegoVersion() ast.RegoVersion { + // v0 takes precedence over v1 + if p.v0Compatible { + return ast.RegoV0 + } if p.v1Compatible { return ast.RegoV1 } - return ast.RegoV0 + return ast.DefaultRegoVersion } func opaTest(args []string, testParams testCommandParams) (int, error) { @@ -552,6 +557,7 @@ recommended as some updates might cause them to be dropped by OPA. addTargetFlag(testCommand.Flags(), testParams.target) addCapabilitiesFlag(testCommand.Flags(), testParams.capabilities) addSchemaFlags(testCommand.Flags(), testParams.schema) + addV0CompatibleFlag(testCommand.Flags(), &testParams.v0Compatible, false) addV1CompatibleFlag(testCommand.Flags(), &testParams.v1Compatible, false) RootCommand.AddCommand(testCommand) diff --git a/cmd/test_test.go b/cmd/test_test.go index 5c9bad9123..a1f18ba6d9 100644 --- a/cmd/test_test.go +++ b/cmd/test_test.go @@ -173,34 +173,35 @@ func failTrace(t *testing.T) []*topdown.Event { t.Helper() mod := ` package testing + import rego.v1 - p { + p if { x # Always true trace("test test") q["foo"] } - x { + x if { y } - y { + y if { true } - q[x] { + q contains x if { some x trace("got this far") r[x] trace("got this far1") } - r[x] { + r contains x if { trace("got this far2") x := data.x } - test_p { + test_p if { p with data.x as "bar" } ` @@ -1540,7 +1541,13 @@ FAIL: 1/1 // Assert that ignore flag is correctly used when the bundle flag is activated func TestIgnoreFlag(t *testing.T) { files := map[string]string{ - "/test.rego": "package test\n p := input.foo == 42\ntest_p {\n p with input.foo as 42\n}", + "/test.rego": `package test +import rego.v1 + +p := input.foo == 42 +test_p if { + p with input.foo as 42 +}`, "/broken.rego": "package foo\n bar {", } @@ -1563,7 +1570,13 @@ func TestIgnoreFlag(t *testing.T) { // Assert that ignore flag is correctly used when the bundle flag is activated func TestIgnoreFlagWithBundleFlag(t *testing.T) { files := map[string]string{ - "/test.rego": "package test\n p := input.foo == 42\ntest_p {\n p with input.foo as 42\n}", + "/test.rego": `package test +import rego.v1 + +p := input.foo == 42 +test_p if { + p with input.foo as 42 +}`, "/broken.rego": "package foo\n bar {", } @@ -1606,15 +1619,17 @@ func testSchemasAnnotation(rego string) (int, error) { func TestSchemasAnnotation(t *testing.T) { policyWithSchemaRef := ` package test +import rego.v1 + # METADATA # schemas: # - input: schema["input"] -p { +p if { rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation input.foo == 42 # type mismatch with schema that should be ignored } -test_p { +test_p if { p with input.foo as 42 }` @@ -1626,14 +1641,16 @@ test_p { func TestSchemasAnnotationInline(t *testing.T) { policyWithInlinedSchema := ` package test +import rego.v1 + # METADATA # schemas: # - input.foo: {"type": "boolean"} -p { +p if { input.foo == 42 # type mismatch with schema that should NOT be ignored since it is an inlined schema format } -test_p { +test_p if { p with input.foo as 42 }` @@ -1671,15 +1688,17 @@ func testSchemasAnnotationWithJSONFile(rego string, schema string) (int, error) func TestJSONSchemaSuccess(t *testing.T) { regoContents := `package test +import rego.v1 + # METADATA # schemas: # - input: schema.demo_schema -p { -input.foo == 42 +p if { + input.foo == 42 } -test_p { -p with input.foo as 42 +test_p if { + p with input.foo as 42 }` schema := `{ @@ -1709,15 +1728,17 @@ p with input.foo as 42 func TestJSONSchemaFail(t *testing.T) { regoContents := `package test +import rego.v1 + # METADATA # schemas: # - input: schema.demo_schema -p { -input.foo == 42 +p if { + input.foo == 42 } -test_p { -p with input.foo as 42 +test_p if { + p with input.foo as 42 }` schema := `{ @@ -1749,8 +1770,13 @@ p with input.foo as 42 func TestWatchMode(t *testing.T) { files := map[string]string{ - "/policy.rego": "package foo\n p := 1", - "/policy_test.rego": "package foo\n test_p { p == 1 }", + "/policy.rego": `package foo +p := 1`, + "/policy_test.rego": `package foo +import rego.v1 +test_p if { + p == 1 +}`, } test.WithTempFS(files, func(root string) { @@ -1850,8 +1876,12 @@ Watching for changes ... func TestWatchModeWithDataFile(t *testing.T) { files := map[string]string{ - "/policy.rego": "package foo\n test_p { data.y == 1 }", - "/data.json": `{"y": 1}`, + "/policy.rego": `package foo +import rego.v1 +test_p if { + data.y == 1 +}`, + "/data.json": `{"y": 1}`, } test.WithTempFS(files, func(root string) { @@ -1930,8 +1960,12 @@ Watching for changes ... func TestWatchModeWhenDataFileRemoved(t *testing.T) { files := map[string]string{ - "/policy.rego": "package foo\n test_p { data.y == 1 }", - "/data.json": `{"y": 1}`, + "/policy.rego": `package foo +import rego.v1 +test_p if { + data.y == 1 +}`, + "/data.json": `{"y": 1}`, } test.WithTempFS(files, func(root string) { @@ -2046,8 +2080,13 @@ Watching for changes ...`, } files := map[string]string{ - "/policy.rego": "package foo\n p := 1", - "/policy_test.rego": "package foo\n test_p { p == 1 }", + "/policy.rego": `package foo +p := 1`, + "/policy_test.rego": `package foo +import rego.v1 +test_p if { + p == 1 +}`, } for _, tc := range tests { @@ -2146,57 +2185,64 @@ func TestExitCode(t *testing.T) { }{ "pass when no failed or skipped tests": { Test: `package foo - test_pass { true } + import rego.v1 + test_pass if { true } `, ExitZeroOnSkipped: false, ExpectedExitCode: 0, }, "fail when failed tests": { Test: `package foo - test_pass { true } - test_fail { false } + import rego.v1 + test_pass if { true } + test_fail if { false } `, ExitZeroOnSkipped: false, ExpectedExitCode: 2, }, "fail when skipped tests": { Test: `package foo - test_pass { true } - todo_test_skip { true } + import rego.v1 + test_pass if { true } + todo_test_skip if { true } `, ExitZeroOnSkipped: false, ExpectedExitCode: 2, }, "fail when failed tests and skipped tests": { Test: `package foo - test_pass { true } - test_fail { false } - todo_test_skip { true } + import rego.v1 + test_pass if { true } + test_fail if { false } + todo_test_skip if { true } `, ExitZeroOnSkipped: false, ExpectedExitCode: 2, }, "pass when skipped tests and exit zero on skipped": { Test: `package foo - test_pass { true } - todo_test_skip { true } + import rego.v1 + test_pass if { true } + todo_test_skip if { true } `, ExitZeroOnSkipped: true, ExpectedExitCode: 0, }, "fail when failed tests and exit zero on skipped": { Test: `package foo - test_pass { true } - test_fail { false } + import rego.v1 + test_pass if { true } + test_fail if { false } `, ExitZeroOnSkipped: true, ExpectedExitCode: 2, }, "fail when failed tests, skipped tests and exit zero on skipped": { Test: `package foo - test_pass { true } - test_fail { false } - todo_test_skip { true } + import rego.v1 + test_pass if { true } + test_fail if { false } + todo_test_skip if { true } `, ExitZeroOnSkipped: true, ExpectedExitCode: 2, @@ -2227,8 +2273,10 @@ func TestCoverageThreshold(t *testing.T) { note: "coverage threshold met", modules: map[string]string{ "test.rego": `package test + import rego.v1 + p := 1 - test_p { p == 1 }`, + test_p if { p == 1 }`, }, expectedExitCode: 0, }, @@ -2236,12 +2284,14 @@ func TestCoverageThreshold(t *testing.T) { note: "coverage threshold not met", modules: map[string]string{ "test.rego": `package test - p := 1 { + import rego.v1 + + p := 1 if { 1 == 1 } q := 2 r := 3 - test_q { q == 2 }`, + test_q if { q == 2 }`, }, threshold: 100, expectedExitCode: 2, @@ -2251,33 +2301,39 @@ func TestCoverageThreshold(t *testing.T) { note: "coverage threshold not met (verbose)", modules: map[string]string{ "test.rego": `package test - p := 1 { + import rego.v1 + + p := 1 if { 1 == 1 } q := 2 r := 3 - test_q { q == 2 }`, + test_q if { q == 2 }`, }, threshold: 100, expectedExitCode: 2, verbose: true, expectedErrOutput: `Code coverage threshold not met: got 40.00 instead of 100.00 Lines not covered: - %ROOT%/test.rego:2-3 - %ROOT%/test.rego:6 + %ROOT%/test.rego:4-5 + %ROOT%/test.rego:8 `, }, { note: "coverage threshold not met (verbose, multiple files)", modules: map[string]string{ "policy1.rego": `package test - p := 1 { + import rego.v1 + + p := 1 if { 1 == 1 } q := 2 r := 3`, "policy2.rego": `package test - s := 4 { + import rego.v1 + + s := 4 if { 1 == 1 2 == 2 } @@ -2285,18 +2341,20 @@ Lines not covered: u := 6 v := 7`, "test.rego": `package test - test_q { q == 2 } - test_t { t == 5 }`, + import rego.v1 + + test_q if { q == 2 } + test_t if { t == 5 }`, }, threshold: 100, expectedExitCode: 2, verbose: true, expectedErrOutput: `Code coverage threshold not met: got 33.33 instead of 100.00 Lines not covered: - %ROOT%/policy1.rego:2-3 - %ROOT%/policy1.rego:6 - %ROOT%/policy2.rego:2-4 - %ROOT%/policy2.rego:7-8 + %ROOT%/policy1.rego:4-5 + %ROOT%/policy1.rego:8 + %ROOT%/policy2.rego:4-6 + %ROOT%/policy2.rego:9-10 `, }, } @@ -2342,15 +2400,17 @@ func (t loadType) String() string { return [...]string{"file", "bundle", "bundle tarball"}[t] } -func TestWithV1CompatibleFlag(t *testing.T) { +func TestWithV1CompatibleFlags(t *testing.T) { tests := []struct { note string + v0Compatible bool v1Compatible bool files map[string]string expErr string }{ { - note: "0.x module, no imports", + note: "v0 module, no imports", + v0Compatible: true, files: map[string]string{ "/test.rego": `package test @@ -2366,7 +2426,8 @@ test_l if { expErr: "rego_parse_error", }, { - note: "0.x module, rego.v1 imported", + note: "v0 module, rego.v1 imported", + v0Compatible: true, files: map[string]string{ "/test.rego": `package test @@ -2383,7 +2444,8 @@ test_l if { }, }, { - note: "0.x module, future.keywords imported", + note: "v0 module, future.keywords imported", + v0Compatible: true, files: map[string]string{ "/test.rego": `package test @@ -2401,7 +2463,7 @@ test_l if { }, { - note: "1.0 compatible module, no imports", + note: "v1 compatible module, no imports", v1Compatible: true, files: map[string]string{ "/test.rego": `package test @@ -2417,7 +2479,64 @@ test_l if { }, }, { - note: "1.0 compatible module, rego.v1 imported", + note: "v1 compatible module, rego.v1 imported", + v1Compatible: true, + files: map[string]string{ + "/test.rego": `package test + +import rego.v1 + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v1 compatible module, future.keywords imported", + v1Compatible: true, + files: map[string]string{ + "/test.rego": `package test + +import future.keywords + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + }, + + // v0 takes precedence over v1 + { + note: "v0+v1 module, no imports", + v0Compatible: true, + v1Compatible: true, + files: map[string]string{ + "/test.rego": `package test + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + expErr: "rego_parse_error", + }, + { + note: "v0+v1 module, rego.v1 imported", + v0Compatible: true, v1Compatible: true, files: map[string]string{ "/test.rego": `package test @@ -2435,7 +2554,8 @@ test_l if { }, }, { - note: "1.0 compatible module, future.keywords imported", + note: "v0+v1 module, future.keywords imported", + v0Compatible: true, v1Compatible: true, files: map[string]string{ "/test.rego": `package test @@ -2489,6 +2609,7 @@ test_l if { var errBuf bytes.Buffer testParams := newTestCommandParams() + testParams.v0Compatible = tc.v0Compatible testParams.v1Compatible = tc.v1Compatible testParams.bundleMode = loadType == loadBundle testParams.count = 1 diff --git a/rego/rego.go b/rego/rego.go index 8672efd669..91465b003d 100644 --- a/rego/rego.go +++ b/rego/rego.go @@ -1967,6 +1967,7 @@ func (r *Rego) parseQuery(queryImports []*ast.Import, m metrics.Metrics) (ast.Bo if err != nil { return nil, err } + popts.RegoVersion = r.regoVersion popts, err = parserOptionsFromRegoVersionImport(queryImports, popts) if err != nil { return nil, err diff --git a/runtime/runtime.go b/runtime/runtime.go index 5da7c34a90..9b7ff6a06a 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -228,15 +228,31 @@ type Params struct { // UnixSocketPerm specifies the permission for the Unix domain socket if used to listen for connections UnixSocketPerm *string + // V0Compatible will enable OPA features and behaviors that were enabled by default in OPA v0.x releases. + // Takes precedence over V1Compatible. + V0Compatible bool + // V1Compatible will enable OPA features and behaviors that will be enabled by default in a future OPA v1.0 release. // This flag allows users to opt-in to the new behavior and helps transition to the future release upon which // the new behavior will be enabled by default. + // If V0Compatible is set, V1Compatible will be ignored. V1Compatible bool // CipherSuites specifies the list of enabled TLS 1.0–1.2 cipher suites CipherSuites *[]uint16 } +func (p *Params) regoVersion() ast.RegoVersion { + // v0 takes precedence over v1 + if p.V0Compatible { + return ast.RegoV0 + } + if p.V1Compatible { + return ast.RegoV1 + } + return ast.DefaultRegoVersion +} + // LoggingConfig stores the configuration for OPA's logging behaviour. type LoggingConfig struct { Level string @@ -339,12 +355,14 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) { } } - var regoVersion ast.RegoVersion - if params.V1Compatible { - regoVersion = ast.RegoV1 - } else { + regoVersion := ast.DefaultRegoVersion + if params.V0Compatible { + // v0 takes precedence over v1 regoVersion = ast.RegoV0 + } else if params.V1Compatible { + regoVersion = ast.RegoV1 } + loaded, err := initload.LoadPathsForRegoVersion(regoVersion, params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, false, false, nil, nil) if err != nil { return nil, fmt.Errorf("load error: %w", err) @@ -500,7 +518,7 @@ func (rt *Runtime) Serve(ctx context.Context) error { } serverInitializingMessage := "Initializing server." - if !rt.Params.AddrSetByUser && !rt.Params.V1Compatible { + if !rt.Params.AddrSetByUser && (rt.Params.V0Compatible || !rt.Params.V1Compatible) { serverInitializingMessage += " OPA is running on a public (0.0.0.0) network interface. Unless you intend to expose OPA outside of the host, binding to the localhost interface (--addr localhost:8181) is recommended. See https://www.openpolicyagent.org/docs/latest/security/#interface-binding" } @@ -711,7 +729,7 @@ func (rt *Runtime) StartREPL(ctx context.Context) { banner := rt.getBanner() repl := repl.New(rt.Store, rt.Params.HistoryPath, rt.Params.Output, rt.Params.OutputFormat, rt.Params.ErrorLimit, banner). WithRuntime(rt.Manager.Info). - WithV1Compatible(rt.Params.V1Compatible). + WithRegoVersion(rt.Params.regoVersion()). WithInitBundles(rt.loadedPathsResult.Bundles) if rt.Params.Watch { diff --git a/sdk/opa.go b/sdk/opa.go index 8b180ea1a6..59ebe74665 100644 --- a/sdk/opa.go +++ b/sdk/opa.go @@ -92,10 +92,7 @@ func New(ctx context.Context, opts Options) (*OPA, error) { opa.plugins = opts.Plugins opa.managerOpts = opts.ManagerOpts - opa.regoVersion = opts.RegoVersion - if opts.V1Compatible { - opa.regoVersion = ast.RegoV1 - } + opa.regoVersion = opts.regoVersion() return opa, opa.configure(ctx, opa.config, opts.Ready, opts.block) } diff --git a/sdk/options.go b/sdk/options.go index 0973f47c14..112093ca81 100644 --- a/sdk/options.go +++ b/sdk/options.go @@ -56,13 +56,19 @@ type Options struct { // Hooks allows hooking into the internals of SDK operations (TODO(sr): find better words) Hooks hooks.Hooks + // V0Compatible enables v0 compatibility mode when set to true. + // This is an opt-in to OPA features and behaviors that were enabled by default in OPA v0.x. + // Takes precedence over V1Compatible. + V0Compatible bool + // V1Compatible enables v1 compatibility mode when set to true. // This is an opt-in to OPA features and behaviors that will be enabled by default in OPA v1.0 and later. // See https://www.openpolicyagent.org/docs/latest/opa-1/ for more information. + // If V0Compatible is set to true, this field is ignored. V1Compatible bool // RegoVersion sets the version of the Rego language to use. - // If V1Compatible is set to true, this field is ignored and the Rego version is set to RegoV1. + // If V0Compatible or V1Compatible is set to true, this field is ignored. RegoVersion ast.RegoVersion // ManagerOpts allows customization of the plugin manager. @@ -74,6 +80,17 @@ type Options struct { block bool } +func (o *Options) regoVersion() ast.RegoVersion { + // v0 takes precedence over v1 + if o.V0Compatible { + return ast.RegoV0 + } + if o.V1Compatible { + return ast.RegoV1 + } + return o.RegoVersion +} + func (o *Options) init() error { if o.Ready == nil {