From d9948db2fa8e175d3e7d4455b97670eff0df7313 Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Wed, 23 Oct 2024 10:36:52 +0200 Subject: [PATCH 1/7] Adding `rego_v1` build flag For toggling which rego-version is default. This is groundwork that can be replaced/augmented with a `rego_v0` build tag in OPA v1.0. NOTE: This is a PoC proposal, not to be merged unless no other alternatives can be devised. Signed-off-by: Johan Fylling --- Makefile | 6 +++++- ast/parser.go | 13 ------------- ast/regov0.go | 20 ++++++++++++++++++++ ast/regov1.go | 21 +++++++++++++++++++++ cmd/version.go | 2 ++ cmd/version_test.go | 2 ++ 6 files changed, 50 insertions(+), 14 deletions(-) create mode 100644 ast/regov0.go create mode 100644 ast/regov1.go diff --git a/Makefile b/Makefile index f11f569e37..d8159f33dc 100644 --- a/Makefile +++ b/Makefile @@ -112,7 +112,7 @@ install: generate $(GO) install $(GO_TAGS) -ldflags $(LDFLAGS) .PHONY: test -test: go-test wasm-test +test: go-test go-test-v1 wasm-test .PHONY: go-build go-build: generate @@ -122,6 +122,10 @@ go-build: generate go-test: generate $(GO) test $(GO_TAGS),slow ./... +.PHONY: go-test-v1 +go-test-v1: generate + $(GO) test $(GO_TAGS),rego_v1,slow ./... + .PHONY: race-detector race-detector: generate $(GO) test $(GO_TAGS),slow -race -vet=off ./... diff --git a/ast/parser.go b/ast/parser.go index 09ede2baec..b90ae0ae23 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -32,19 +32,6 @@ type RegoVersion int const DefaultRegoVersion = RegoVersion(0) -const ( - // RegoV0 is the default, original Rego syntax. - RegoV0 RegoVersion = iota - // RegoV0CompatV1 requires modules to comply with both the RegoV0 and RegoV1 syntax (as when 'rego.v1' is imported in a module). - // Shortly, RegoV1 compatibility is required, but 'rego.v1' or 'future.keywords' must also be imported. - RegoV0CompatV1 - // RegoV1 is the Rego syntax enforced by OPA 1.0; e.g.: - // future.keywords part of default keyword set, and don't require imports; - // 'if' and 'contains' required in rule heads; - // (some) strict checks on by default. - RegoV1 -) - func (v RegoVersion) Int() int { if v == RegoV1 { return 1 diff --git a/ast/regov0.go b/ast/regov0.go new file mode 100644 index 0000000000..2637537a5f --- /dev/null +++ b/ast/regov0.go @@ -0,0 +1,20 @@ +// Copyright 2024 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +//go:build !rego_v1 + +package ast + +const ( + // RegoV0 is the default, original Rego syntax. + RegoV0 RegoVersion = iota + // RegoV0CompatV1 requires modules to comply with both the RegoV0 and RegoV1 syntax (as when 'rego.v1' is imported in a module). + // Shortly, RegoV1 compatibility is required, but 'rego.v1' or 'future.keywords' must also be imported. + RegoV0CompatV1 + // RegoV1 is the Rego syntax enforced by OPA 1.0; e.g.: + // future.keywords part of default keyword set, and don't require imports; + // 'if' and 'contains' required in rule heads; + // (some) strict checks on by default. + RegoV1 +) diff --git a/ast/regov1.go b/ast/regov1.go new file mode 100644 index 0000000000..9d313e269b --- /dev/null +++ b/ast/regov1.go @@ -0,0 +1,21 @@ +// Copyright 2024 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +//go:build rego_v1 +// +build rego_v1 + +package ast + +const ( + // RegoV1 is the Rego syntax enforced by OPA 1.0; e.g.: + // future.keywords part of default keyword set, and don't require imports; + // 'if' and 'contains' required in rule heads; + // (some) strict checks on by default. + RegoV1 RegoVersion = iota + // RegoV0CompatV1 requires modules to comply with both the RegoV0 and RegoV1 syntax (as when 'rego.v1' is imported in a module). + // Shortly, RegoV1 compatibility is required, but 'rego.v1' or 'future.keywords' must also be imported. + RegoV0CompatV1 + // RegoV0 is the default, original Rego syntax. + RegoV0 +) diff --git a/cmd/version.go b/cmd/version.go index 3d3b0de4ab..d2abf2fb81 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -11,6 +11,7 @@ import ( "io" "os" + "github.com/open-policy-agent/opa/ast" "github.com/spf13/cobra" "github.com/open-policy-agent/opa/cmd/internal/env" @@ -47,6 +48,7 @@ func generateCmdOutput(out io.Writer, check bool) { fmt.Fprintln(out, "Build Hostname: "+version.Hostname) fmt.Fprintln(out, "Go Version: "+version.GoVersion) fmt.Fprintln(out, "Platform: "+version.Platform) + fmt.Fprintln(out, "Rego Version: "+ast.DefaultRegoVersion.String()) var wasmAvailable string diff --git a/cmd/version_test.go b/cmd/version_test.go index 830c9af2b5..e9c25e5556 100644 --- a/cmd/version_test.go +++ b/cmd/version_test.go @@ -29,6 +29,7 @@ func TestGenerateCmdOutputDisableCheckFlag(t *testing.T) { "Go Version", "Platform", "WebAssembly", + "Rego Version", }) } @@ -60,6 +61,7 @@ func TestGenerateCmdOutputWithCheckFlagNoError(t *testing.T) { "Latest Upstream Version", "Release Notes", "Download", + "Rego Version", }) } From f841ea7e5c89170fd6ce1a5238f7f589e609563d Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Mon, 18 Nov 2024 17:23:57 +0100 Subject: [PATCH 2/7] * Adding `ast.RegoUndefined` * Making `ast.DefaultRefoVersion` a func * conditioning `ast.defaultRegoVersion` on `opa_rego_v1` built tag Signed-off-by: Johan Fylling --- Makefile | 2 +- ast/compile.go | 18 ++++++++++++++++-- ast/parser.go | 39 ++++++++++++++++++++++++++++++++------- ast/parser_ext.go | 7 ++++++- ast/policy.go | 15 +++++++++++---- ast/policy_test.go | 4 ++-- ast/rego_default_v0.go | 9 +++++++++ ast/rego_default_v1.go | 10 ++++++++++ ast/regov0.go | 20 -------------------- ast/regov1.go | 21 --------------------- bundle/bundle.go | 16 ++++++++++++---- cmd/build.go | 2 +- cmd/build_test.go | 4 ++-- cmd/check.go | 2 +- cmd/deps.go | 2 +- cmd/eval.go | 2 +- cmd/fmt.go | 2 +- cmd/oracle.go | 2 +- cmd/parse.go | 2 +- cmd/refactor.go | 4 ++-- cmd/refactor_test.go | 14 ++++++++++++-- cmd/test.go | 2 +- cmd/version.go | 2 +- compile/compile.go | 5 +++++ compile/compile_test.go | 12 ++++++------ format/format.go | 23 +++++++++++++++++------ repl/repl.go | 2 +- runtime/runtime.go | 2 +- sdk/test/test.go | 2 +- tester/runner.go | 2 +- 30 files changed, 156 insertions(+), 93 deletions(-) create mode 100644 ast/rego_default_v0.go create mode 100644 ast/rego_default_v1.go delete mode 100644 ast/regov0.go delete mode 100644 ast/regov1.go diff --git a/Makefile b/Makefile index 6b7129d4d7..42cc5dd0aa 100644 --- a/Makefile +++ b/Makefile @@ -124,7 +124,7 @@ go-test: generate .PHONY: go-test-v1 go-test-v1: generate - $(GO) test $(GO_TAGS),rego_v1,slow ./... + $(GO) test $(GO_TAGS),opa_rego_v1,slow ./... .PHONY: race-detector race-detector: generate diff --git a/ast/compile.go b/ast/compile.go index b26490f193..9a8d286183 100644 --- a/ast/compile.go +++ b/ast/compile.go @@ -1717,7 +1717,7 @@ func (c *Compiler) checkDuplicateImports() { for _, name := range c.sorted { mod := c.Modules[name] - if c.strict || mod.regoV1Compatible() { + if c.strict || moduleIsRegoV1(mod) { modules = append(modules, mod) } } @@ -1731,7 +1731,7 @@ func (c *Compiler) checkDuplicateImports() { func (c *Compiler) checkKeywordOverrides() { for _, name := range c.sorted { mod := c.Modules[name] - if c.strict || mod.regoV1Compatible() { + if c.strict || moduleIsRegoV1(mod) { errs := checkRootDocumentOverrides(mod) for _, err := range errs { c.err(err) @@ -1740,6 +1740,20 @@ func (c *Compiler) checkKeywordOverrides() { } } +func moduleIsRegoV1(mod *Module) bool { + switch mod.regoVersion { + case RegoUndefined: + switch DefaultRegoVersion() { + case RegoV1, RegoV0CompatV1: + return true + default: + return false + } + } + + return mod.regoV1Compatible() +} + // resolveAllRefs resolves references in expressions to their fully qualified values. // // For instance, given the following module: diff --git a/ast/parser.go b/ast/parser.go index 0f0527e777..86f7a19fb9 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -30,7 +30,30 @@ var RegoV1CompatibleRef = Ref{VarTerm("rego"), StringTerm("v1")} // RegoVersion defines the Rego syntax requirements for a module. type RegoVersion int -const DefaultRegoVersion = RegoVersion(0) +const ( + RegoUndefined RegoVersion = iota + // RegoV0 is the default, original Rego syntax. + RegoV0 + // RegoV0CompatV1 requires modules to comply with both the RegoV0 and RegoV1 syntax (as when 'rego.v1' is imported in a module). + // Shortly, RegoV1 compatibility is required, but 'rego.v1' or 'future.keywords' must also be imported. + RegoV0CompatV1 + // RegoV1 is the Rego syntax enforced by OPA 1.0; e.g.: + // future.keywords part of default keyword set, and don't require imports; + // 'if' and 'contains' required in rule heads; + // (some) strict checks on by default. + RegoV1 +) + +func DefaultRegoVersion() RegoVersion { + return defaultRegoVersion +} + +func EffectiveRegoVersion(regoVersion RegoVersion) RegoVersion { + if regoVersion == RegoUndefined { + return DefaultRegoVersion() + } + return regoVersion +} func (v RegoVersion) Int() int { if v == RegoV1 { @@ -143,8 +166,10 @@ type ParserOptions struct { } // EffectiveRegoVersion returns the effective RegoVersion to use for parsing. -// Deprecated: Use RegoVersion instead. -func (po *ParserOptions) EffectiveRegoVersion() RegoVersion { +func (po ParserOptions) EffectiveRegoVersion() RegoVersion { + if po.RegoVersion == RegoUndefined { + return DefaultRegoVersion() + } return po.RegoVersion } @@ -301,7 +326,7 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { allowedFutureKeywords := map[string]tokens.Token{} - if p.po.RegoVersion == RegoV1 { + if p.po.EffectiveRegoVersion() == RegoV1 { // RegoV1 includes all future keywords in the default language definition for k, v := range futureKeywords { allowedFutureKeywords[k] = v @@ -360,7 +385,7 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { } selected := map[string]tokens.Token{} - if p.po.AllFutureKeywords || p.po.RegoVersion == RegoV1 { + if p.po.AllFutureKeywords || p.po.EffectiveRegoVersion() == RegoV1 { for kw, tok := range allowedFutureKeywords { selected[kw] = tok } @@ -381,7 +406,7 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { } p.s.s = p.s.s.WithKeywords(selected) - if p.po.RegoVersion == RegoV1 { + if p.po.EffectiveRegoVersion() == RegoV1 { for kw, tok := range allowedFutureKeywords { p.s.s.AddKeyword(kw, tok) } @@ -2697,7 +2722,7 @@ func (p *Parser) regoV1Import(imp *Import) { return } - if p.po.RegoVersion == RegoV1 { + if p.po.EffectiveRegoVersion() == RegoV1 { // We're parsing for Rego v1, where the 'rego.v1' import is a no-op. return } diff --git a/ast/parser_ext.go b/ast/parser_ext.go index 10187a5825..4b9645816d 100644 --- a/ast/parser_ext.go +++ b/ast/parser_ext.go @@ -689,7 +689,12 @@ func parseModule(filename string, stmts []Statement, comments []*Comment, regoCo // The comments slice only holds comments that were not their own statements. mod.Comments = append(mod.Comments, comments...) - mod.regoVersion = regoCompatibilityMode + + if regoCompatibilityMode == RegoUndefined { + mod.regoVersion = DefaultRegoVersion() + } else { + mod.regoVersion = regoCompatibilityMode + } for i, stmt := range stmts[1:] { switch stmt := stmt.(type) { diff --git a/ast/policy.go b/ast/policy.go index 0983630964..6cb6151727 100644 --- a/ast/policy.go +++ b/ast/policy.go @@ -100,7 +100,7 @@ var Wildcard = &Term{Value: Var("_")} const WildcardPrefix = "$" // Keywords contains strings that map to language keywords. -var Keywords = KeywordsForRegoVersion(DefaultRegoVersion) +var Keywords = KeywordsForRegoVersion(DefaultRegoVersion()) var KeywordsV0 = [...]string{ "not", @@ -791,6 +791,13 @@ type toStringOpts struct { regoVersion RegoVersion } +func (o toStringOpts) RegoVersion() RegoVersion { + if o.regoVersion == RegoUndefined { + return DefaultRegoVersion() + } + return o.regoVersion +} + func (rule *Rule) stringWithOpts(opts toStringOpts) string { buf := []string{} if rule.Default { @@ -798,7 +805,7 @@ func (rule *Rule) stringWithOpts(opts toStringOpts) string { } buf = append(buf, rule.Head.stringWithOpts(opts)) if !rule.Default { - switch opts.regoVersion { + switch opts.RegoVersion() { case RegoV1, RegoV0CompatV1: buf = append(buf, "if") } @@ -861,7 +868,7 @@ func (rule *Rule) elseString(opts toStringOpts) string { buf = append(buf, value.String()) } - switch opts.regoVersion { + switch opts.RegoVersion() { case RegoV1, RegoV0CompatV1: buf = append(buf, "if") } @@ -1043,7 +1050,7 @@ func (head *Head) stringWithOpts(opts toStringOpts) string { case len(head.Args) != 0: buf.WriteString(head.Args.String()) case len(head.Reference) == 1 && head.Key != nil: - switch opts.regoVersion { + switch opts.RegoVersion() { case RegoV0: buf.WriteRune('[') buf.WriteString(head.Key.String()) diff --git a/ast/policy_test.go b/ast/policy_test.go index b70f901414..b3979fd694 100644 --- a/ast/policy_test.go +++ b/ast/policy_test.go @@ -616,7 +616,7 @@ func TestRuleString(t *testing.T) { assertRuleString(t, tc.rule, tc.expV0, toStringOpts{regoVersion: RegoV0}) assertRuleString(t, tc.rule, tc.expV1, toStringOpts{regoVersion: RegoV1}) - switch DefaultRegoVersion { + switch DefaultRegoVersion() { case RegoV0: assertRuleString(t, tc.rule, tc.expV0, toStringOpts{}) case RegoV1: @@ -863,7 +863,7 @@ import rego.v1 p := 7 if { true }` if module.String() != exp { - t.Fatalf("expected %q but got %q", exp, module.String()) + t.Fatalf("expected:\n\n%q\n\nbut got:\n\n%q", exp, module.String()) } } diff --git a/ast/rego_default_v0.go b/ast/rego_default_v0.go new file mode 100644 index 0000000000..d3bf29c431 --- /dev/null +++ b/ast/rego_default_v0.go @@ -0,0 +1,9 @@ +// Copyright 2024 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +//go:build !opa_rego_v1 + +package ast + +var defaultRegoVersion = RegoV0 diff --git a/ast/rego_default_v1.go b/ast/rego_default_v1.go new file mode 100644 index 0000000000..82c5a24285 --- /dev/null +++ b/ast/rego_default_v1.go @@ -0,0 +1,10 @@ +// Copyright 2024 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +//go:build opa_rego_v1 +// +build opa_rego_v1 + +package ast + +var defaultRegoVersion = RegoV1 diff --git a/ast/regov0.go b/ast/regov0.go deleted file mode 100644 index 2637537a5f..0000000000 --- a/ast/regov0.go +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2024 The OPA Authors. All rights reserved. -// Use of this source code is governed by an Apache2 -// license that can be found in the LICENSE file. - -//go:build !rego_v1 - -package ast - -const ( - // RegoV0 is the default, original Rego syntax. - RegoV0 RegoVersion = iota - // RegoV0CompatV1 requires modules to comply with both the RegoV0 and RegoV1 syntax (as when 'rego.v1' is imported in a module). - // Shortly, RegoV1 compatibility is required, but 'rego.v1' or 'future.keywords' must also be imported. - RegoV0CompatV1 - // RegoV1 is the Rego syntax enforced by OPA 1.0; e.g.: - // future.keywords part of default keyword set, and don't require imports; - // 'if' and 'contains' required in rule heads; - // (some) strict checks on by default. - RegoV1 -) diff --git a/ast/regov1.go b/ast/regov1.go deleted file mode 100644 index 9d313e269b..0000000000 --- a/ast/regov1.go +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2024 The OPA Authors. All rights reserved. -// Use of this source code is governed by an Apache2 -// license that can be found in the LICENSE file. - -//go:build rego_v1 -// +build rego_v1 - -package ast - -const ( - // RegoV1 is the Rego syntax enforced by OPA 1.0; e.g.: - // future.keywords part of default keyword set, and don't require imports; - // 'if' and 'contains' required in rule heads; - // (some) strict checks on by default. - RegoV1 RegoVersion = iota - // RegoV0CompatV1 requires modules to comply with both the RegoV0 and RegoV1 syntax (as when 'rego.v1' is imported in a module). - // Shortly, RegoV1 compatibility is required, but 'rego.v1' or 'future.keywords' must also be imported. - RegoV0CompatV1 - // RegoV0 is the default, original Rego syntax. - RegoV0 -) diff --git a/bundle/bundle.go b/bundle/bundle.go index 0e159384ef..496ff9aff4 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -244,8 +244,8 @@ func (m Manifest) Copy() Manifest { func (m Manifest) String() string { m.Init() if m.RegoVersion != nil { - return fmt.Sprintf("", - m.Revision, *m.RegoVersion, *m.Roots, m.WasmResolvers, m.Metadata) + return fmt.Sprintf("", + m.Revision, *m.RegoVersion, m.FileRegoVersions, *m.Roots, m.WasmResolvers, m.Metadata) } return fmt.Sprintf("", m.Revision, *m.Roots, m.WasmResolvers, m.Metadata) @@ -711,10 +711,10 @@ func (r *Reader) Read() (Bundle, error) { // Parse modules popts := r.ParserOptions() - popts.RegoVersion = bundle.RegoVersion(popts.RegoVersion) + popts.RegoVersion = bundle.RegoVersion(popts.EffectiveRegoVersion()) for _, mf := range modules { modulePopts := popts - if modulePopts.RegoVersion, err = bundle.RegoVersionForFile(mf.RelativePath, popts.RegoVersion); err != nil { + if modulePopts.RegoVersion, err = bundle.RegoVersionForFile(mf.RelativePath, popts.EffectiveRegoVersion()); err != nil { return bundle, err } r.metrics.Timer(metrics.RegoModuleParse).Start() @@ -1203,6 +1203,10 @@ func (b *Bundle) SetRegoVersion(v ast.RegoVersion) { // If there is no defined version for the given path, the default version def is returned. // If the version does not correspond to ast.RegoV0 or ast.RegoV1, an error is returned. func (b *Bundle) RegoVersionForFile(path string, def ast.RegoVersion) (ast.RegoVersion, error) { + if def == ast.RegoUndefined { + def = ast.DefaultRegoVersion() + } + version, err := b.Manifest.numericRegoVersionForFile(path) if err != nil { return def, err @@ -1410,6 +1414,10 @@ func MergeWithRegoVersion(bundles []*Bundle, regoVersion ast.RegoVersion, usePat return nil, errors.New("expected at least one bundle") } + if regoVersion == ast.RegoUndefined { + regoVersion = ast.DefaultRegoVersion() + } + if len(bundles) == 1 { result := bundles[0] // We respect the bundle rego-version, defaulting to the provided rego version if not set. diff --git a/cmd/build.go b/cmd/build.go index 7e33eefdf2..e3a7710d5d 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -309,7 +309,7 @@ func dobuild(params buildParams, args []string) error { WithPartialNamespace(params.ns). WithFollowSymlinks(params.followSymlinks) - regoVersion := ast.DefaultRegoVersion + regoVersion := ast.DefaultRegoVersion() if params.v0Compatible { // v0 takes precedence over v1 regoVersion = ast.RegoV0 diff --git a/cmd/build_test.go b/cmd/build_test.go index 9eaf359078..b80aa9d810 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -905,7 +905,7 @@ p2 := 2 tr := tar.NewReader(gr) expManifest := strings.ReplaceAll(tc.manifest, "%REGO_VERSION%", - fmt.Sprintf("%d", ast.DefaultRegoVersion.Int())) + fmt.Sprintf("%d", ast.DefaultRegoVersion().Int())) found := false for { @@ -1712,7 +1712,7 @@ q contains 1 if { } expVal = strings.ReplaceAll(expVal, "%ROOT%", root) expVal = strings.ReplaceAll(expVal, "%DEFAULT_REGO_VERSION%", - fmt.Sprintf("%d", ast.DefaultRegoVersion.Int())) + 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)) } diff --git a/cmd/check.go b/cmd/check.go index db874812bd..9d18e1cc63 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -54,7 +54,7 @@ func (p *checkParams) regoVersion() ast.RegoVersion { if p.v1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } const ( diff --git a/cmd/deps.go b/cmd/deps.go index 72c29b278b..15c8cd0454 100644 --- a/cmd/deps.go +++ b/cmd/deps.go @@ -38,7 +38,7 @@ func (p *depsCommandParams) regoVersion() ast.RegoVersion { if p.v1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } const ( diff --git a/cmd/eval.go b/cmd/eval.go index ecaa8a731b..b06096659e 100644 --- a/cmd/eval.go +++ b/cmd/eval.go @@ -83,7 +83,7 @@ func (p *evalCommandParams) regoVersion() ast.RegoVersion { } else if p.v1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } func newEvalCommandParams() evalCommandParams { diff --git a/cmd/fmt.go b/cmd/fmt.go index 303f34a28b..e3e6d45d59 100644 --- a/cmd/fmt.go +++ b/cmd/fmt.go @@ -45,7 +45,7 @@ func (p *fmtCommandParams) regoVersion() ast.RegoVersion { if p.v1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } var formatCommand = &cobra.Command{ diff --git a/cmd/oracle.go b/cmd/oracle.go index 31f0cf5a90..3175c5c621 100644 --- a/cmd/oracle.go +++ b/cmd/oracle.go @@ -37,7 +37,7 @@ func (p *findDefinitionParams) regoVersion() ast.RegoVersion { if p.v1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } func init() { diff --git a/cmd/parse.go b/cmd/parse.go index e525cff6d2..d367c21118 100644 --- a/cmd/parse.go +++ b/cmd/parse.go @@ -41,7 +41,7 @@ func (p *parseParams) regoVersion() ast.RegoVersion { if p.v1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } var configuredParseParams = parseParams{ diff --git a/cmd/refactor.go b/cmd/refactor.go index 7ef187c914..5925308db6 100644 --- a/cmd/refactor.go +++ b/cmd/refactor.go @@ -37,7 +37,7 @@ func (m *moveCommandParams) regoVersion() ast.RegoVersion { if m.v1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } func init() { @@ -149,7 +149,7 @@ func doMove(params moveCommandParams, args []string, out io.Writer) error { return err } - formatted, err := format.Ast(mod) + formatted, err := format.AstWithOpts(mod, format.Opts{RegoVersion: params.regoVersion()}) if err != nil { return newError("failed to parse Rego source file: %v", err) } diff --git a/cmd/refactor_test.go b/cmd/refactor_test.go index c5cd54c0fc..31f161d006 100644 --- a/cmd/refactor_test.go +++ b/cmd/refactor_test.go @@ -107,7 +107,12 @@ func TestDoMoveRenamePackage(t *testing.T) { t.Fatal(err) } - formatted := format.MustAst(tc.expected) + var formatted []byte + if tc.v0Compatible { + formatted = format.MustAstWithOpts(tc.expected, format.Opts{RegoVersion: ast.RegoV0}) + } else { + formatted = format.MustAstWithOpts(tc.expected, format.Opts{RegoVersion: ast.RegoV1}) + } if !reflect.DeepEqual(formatted, buf.Bytes()) { t.Fatalf("Expected module:\n%v\n\nGot:\n%v\n", string(formatted), buf.String()) @@ -227,7 +232,12 @@ func TestDoMoveOverwriteFile(t *testing.T) { t.Fatal(err) } - actual := ast.MustParseModule(string(data)) + var actual *ast.Module + if tc.v0Compatible { + actual = ast.MustParseModuleWithOpts(string(data), ast.ParserOptions{RegoVersion: ast.RegoV0}) + } else { + actual = ast.MustParseModuleWithOpts(string(data), ast.ParserOptions{RegoVersion: ast.RegoV1}) + } if !tc.expected.Equal(actual) { t.Fatalf("Expected module:\n%v\n\nGot:\n%v\n", tc.expected, actual) diff --git a/cmd/test.go b/cmd/test.go index 1ad3e3ac07..0c1dba0b25 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -88,7 +88,7 @@ func (p *testCommandParams) RegoVersion() ast.RegoVersion { if p.v1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } func opaTest(args []string, testParams testCommandParams) (int, error) { diff --git a/cmd/version.go b/cmd/version.go index d2abf2fb81..25add730d5 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -48,7 +48,7 @@ func generateCmdOutput(out io.Writer, check bool) { fmt.Fprintln(out, "Build Hostname: "+version.Hostname) fmt.Fprintln(out, "Go Version: "+version.GoVersion) fmt.Fprintln(out, "Platform: "+version.Platform) - fmt.Fprintln(out, "Rego Version: "+ast.DefaultRegoVersion.String()) + fmt.Fprintln(out, "Rego Version: "+ast.DefaultRegoVersion().String()) var wasmAvailable string diff --git a/compile/compile.go b/compile/compile.go index 0b165e6519..1e27863246 100644 --- a/compile/compile.go +++ b/compile/compile.go @@ -94,6 +94,7 @@ func New() *Compiler { optimizationLevel: 0, target: TargetRego, debug: debug.Discard(), + regoVersion: ast.DefaultRegoVersion(), } } @@ -294,6 +295,10 @@ func addEntrypointsFromAnnotations(c *Compiler, arefs []*ast.AnnotationsRef) err // Build compiles and links the input files and outputs a bundle to the writer. func (c *Compiler) Build(ctx context.Context) error { + if c.regoVersion == ast.RegoUndefined { + return fmt.Errorf("rego-version not set") + } + if err := c.init(); err != nil { return err } diff --git a/compile/compile_test.go b/compile/compile_test.go index 80016a1e14..ceab56fadd 100644 --- a/compile/compile_test.go +++ b/compile/compile_test.go @@ -98,7 +98,7 @@ func TestCompilerLoadError(t *testing.T) { func TestCompilerLoadAsBundleSuccess(t *testing.T) { ctx := context.Background() - rv := fmt.Sprintf("%d", ast.DefaultRegoVersion.Int()) + rv := fmt.Sprintf("%d", ast.DefaultRegoVersion().Int()) files := map[string]string{ "b1/.manifest": `{"roots": ["b1"], "rego_version": ` + rv + `}`, @@ -159,10 +159,10 @@ func TestCompilerLoadAsBundleSuccess(t *testing.T) { expManifest := bundle.Manifest{ Roots: &expRoots, } - expManifest.SetRegoVersion(ast.DefaultRegoVersion) + expManifest.SetRegoVersion(ast.DefaultRegoVersion()) if !compiler.bundle.Manifest.Equal(expManifest) { - t.Fatalf("expected %v but got %v", compiler.bundle.Manifest, expManifest) + t.Fatalf("expected:\n\n%v\n\nbut got:\n\n%v", compiler.bundle.Manifest, expManifest) } }) } @@ -474,7 +474,7 @@ func TestCompilerBundleMergeWithBundleRegoVersion(t *testing.T) { Modules: []bundle.ModuleFile{}, }, }, - expGlobalRegoVersion: pointTo(ast.DefaultRegoVersion.Int()), + expGlobalRegoVersion: pointTo(ast.DefaultRegoVersion().Int()), expFileRegoVersions: map[string]int{}, }, { @@ -2035,7 +2035,7 @@ func TestCompilerWasmTargetMultipleEntrypoints(t *testing.T) { expManifest := bundle.Manifest{} expManifest.Init() - expManifest.SetRegoVersion(ast.DefaultRegoVersion) + expManifest.SetRegoVersion(ast.DefaultRegoVersion()) expManifest.WasmResolvers = []bundle.WasmResolver{ { Entrypoint: "test/p", @@ -2176,7 +2176,7 @@ func TestCompilerWasmTargetEntrypointDependents(t *testing.T) { expManifest := bundle.Manifest{} expManifest.Init() - expManifest.SetRegoVersion(ast.DefaultRegoVersion) + expManifest.SetRegoVersion(ast.DefaultRegoVersion()) expManifest.WasmResolvers = []bundle.WasmResolver{ { Entrypoint: "test/r", diff --git a/format/format.go b/format/format.go index 75626fff61..cc2bf5ffa9 100644 --- a/format/format.go +++ b/format/format.go @@ -33,6 +33,13 @@ type Opts struct { ParserOptions *ast.ParserOptions } +func (o Opts) effectiveRegoVersion() ast.RegoVersion { + if o.RegoVersion == ast.RegoUndefined { + return ast.DefaultRegoVersion() + } + return o.RegoVersion +} + // defaultLocationFile is the file name used in `Ast()` for terms // without a location, as could happen when pretty-printing the // results of partial eval. @@ -47,10 +54,13 @@ func Source(filename string, src []byte) ([]byte, error) { func SourceWithOpts(filename string, src []byte, opts Opts) ([]byte, error) { var parserOpts ast.ParserOptions + + regoVersion := opts.effectiveRegoVersion() + if opts.ParserOptions != nil { parserOpts = *opts.ParserOptions } else { - if opts.RegoVersion == ast.RegoV1 { + if regoVersion == ast.RegoV1 { // If the rego version is V1, we need to parse it as such, to allow for future keywords not being imported. // Otherwise, we'll default to the default rego-version. parserOpts.RegoVersion = ast.RegoV1 @@ -62,7 +72,7 @@ func SourceWithOpts(filename string, src []byte, opts Opts) ([]byte, error) { return nil, err } - if opts.RegoVersion == ast.RegoV0CompatV1 || opts.RegoVersion == ast.RegoV1 { + if regoVersion == ast.RegoV0CompatV1 || regoVersion == ast.RegoV1 { checkOpts := ast.NewRegoCheckOptions() // The module is parsed as v0, so we need to disable checks that will be automatically amended by the AstWithOpts call anyways. checkOpts.RequireIfKeyword = false @@ -154,7 +164,8 @@ func AstWithOpts(x interface{}, opts Opts) ([]byte, error) { o := fmtOpts{} - if opts.RegoVersion == ast.RegoV0CompatV1 || opts.RegoVersion == ast.RegoV1 { + regoVersion := opts.effectiveRegoVersion() + if regoVersion == ast.RegoV0CompatV1 || regoVersion == ast.RegoV1 { o.regoV1 = true o.ifs = true o.contains = true @@ -220,13 +231,13 @@ func AstWithOpts(x interface{}, opts Opts) ([]byte, error) { switch x := x.(type) { case *ast.Module: - if opts.RegoVersion == ast.RegoV1 { + if regoVersion == ast.RegoV1 { x.Imports = filterRegoV1Import(x.Imports) - } else if opts.RegoVersion == ast.RegoV0CompatV1 { + } else if regoVersion == ast.RegoV0CompatV1 { x.Imports = ensureRegoV1Import(x.Imports) } - if opts.RegoVersion == ast.RegoV0CompatV1 || opts.RegoVersion == ast.RegoV1 || moduleIsRegoV1Compatible(x) { + if regoVersion == ast.RegoV0CompatV1 || regoVersion == ast.RegoV1 || moduleIsRegoV1Compatible(x) { x.Imports = future.FilterFutureImports(x.Imports) } else { for kw := range extraFutureKeywordImports { diff --git a/repl/repl.go b/repl/repl.go index fc8b73c66f..7eb5b00959 100644 --- a/repl/repl.go +++ b/repl/repl.go @@ -364,7 +364,7 @@ func (r *REPL) WithV1Compatible(v1Compatible bool) *REPL { if v1Compatible { r.regoVersion = ast.RegoV1 } else { - r.regoVersion = ast.DefaultRegoVersion + r.regoVersion = ast.DefaultRegoVersion() } return r } diff --git a/runtime/runtime.go b/runtime/runtime.go index d39c9efb5f..abcd133156 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -256,7 +256,7 @@ func (p *Params) regoVersion() ast.RegoVersion { if p.V1Compatible { return ast.RegoV1 } - return ast.DefaultRegoVersion + return ast.DefaultRegoVersion() } // LoggingConfig stores the configuration for OPA's logging behaviour. diff --git a/sdk/test/test.go b/sdk/test/test.go index 5c56ccde3d..80ec676094 100644 --- a/sdk/test/test.go +++ b/sdk/test/test.go @@ -141,7 +141,7 @@ func (s *Server) buildBundles(ref string, policies map[string]string) error { // interpreted as v0 on the receiving end, which will cause problems if modules are parsed/compiled // as v1 on this end, which will drop 'rego.v1' and 'future.keywords' imports. bundleManifest := bundle.Manifest{} - bundleManifest.SetRegoVersion(ast.DefaultRegoVersion) + bundleManifest.SetRegoVersion(ast.DefaultRegoVersion()) bundleManifest.Init() err := compile.New().WithOutput(buf).WithBundle(&bundle.Bundle{ diff --git a/tester/runner.go b/tester/runner.go index 4b1d6a1a7f..f55a151420 100644 --- a/tester/runner.go +++ b/tester/runner.go @@ -606,7 +606,7 @@ func (r *Runner) runBenchmark(ctx context.Context, txn storage.Transaction, mod // Load returns modules and an in-memory store for running tests. func Load(args []string, filter loader.Filter) (map[string]*ast.Module, storage.Store, error) { - return LoadWithRegoVersion(args, filter, ast.DefaultRegoVersion) + return LoadWithRegoVersion(args, filter, ast.DefaultRegoVersion()) } // LoadWithRegoVersion returns modules and an in-memory store for running tests. From 70dea5282b259ea3e67327d927ace31911a3a2d5 Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Mon, 18 Nov 2024 19:03:48 +0100 Subject: [PATCH 3/7] Adding `github.com/open-policy-agent/opa/internal/rego_default_v1` To be imported for side effects. Signed-off-by: Johan Fylling --- Makefile | 6 +- ast/parser.go | 20 +++-- features/rego_default_v1/rego_default_v1.go | 16 ++++ test/rego_default/rego_default_test.go | 87 +++++++++++++++++++++ 4 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 features/rego_default_v1/rego_default_v1.go create mode 100644 test/rego_default/rego_default_test.go diff --git a/Makefile b/Makefile index 42cc5dd0aa..4aee511101 100644 --- a/Makefile +++ b/Makefile @@ -112,7 +112,7 @@ install: generate $(GO) install $(GO_TAGS) -ldflags $(LDFLAGS) .PHONY: test -test: go-test go-test-v1 wasm-test +test: go-test go-test-v1 go-test-v1-import wasm-test .PHONY: go-build go-build: generate @@ -126,6 +126,10 @@ go-test: generate go-test-v1: generate $(GO) test $(GO_TAGS),opa_rego_v1,slow ./... +.PHONY: go-test-v1-import +go-test-v1-import: generate + $(GO) test $(GO_TAGS),test_rego_default_v1_import ./test/rego_default/... + .PHONY: race-detector race-detector: generate $(GO) test $(GO_TAGS),slow -race -vet=off ./... diff --git a/ast/parser.go b/ast/parser.go index 86f7a19fb9..1ae0781a6e 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -48,12 +48,20 @@ func DefaultRegoVersion() RegoVersion { return defaultRegoVersion } -func EffectiveRegoVersion(regoVersion RegoVersion) RegoVersion { - if regoVersion == RegoUndefined { - return DefaultRegoVersion() - } - return regoVersion -} +// SetDefaultRegoVersion sets the default RegoVersion. +// +// Direct use of this function is not recommended. +// Use [github.com/open-policy-agent/opa/features/rego_default_v1] instead to set the default RegoVersion to RegoV1. +func SetDefaultRegoVersion(v RegoVersion) { + defaultRegoVersion = v +} + +//func EffectiveRegoVersion(regoVersion RegoVersion) RegoVersion { +// if regoVersion == RegoUndefined { +// return DefaultRegoVersion() +// } +// return regoVersion +//} func (v RegoVersion) Int() int { if v == RegoV1 { diff --git a/features/rego_default_v1/rego_default_v1.go b/features/rego_default_v1/rego_default_v1.go new file mode 100644 index 0000000000..72b5435da0 --- /dev/null +++ b/features/rego_default_v1/rego_default_v1.go @@ -0,0 +1,16 @@ +// Copyright 2024 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +// Package rego_default_v1 sets the default [ast.RegoVersion] to [ast.RegoV1]. +// +// Usage (import side effects only): +// +// import _ "github.com/open-policy-agent/opa/internal/rego_default_v1" +package rego_default_v1 + +import "github.com/open-policy-agent/opa/ast" + +func init() { + ast.SetDefaultRegoVersion(ast.RegoV1) +} diff --git a/test/rego_default/rego_default_test.go b/test/rego_default/rego_default_test.go new file mode 100644 index 0000000000..64c275414a --- /dev/null +++ b/test/rego_default/rego_default_test.go @@ -0,0 +1,87 @@ +// Copyright 2024 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +//go:build test_rego_default_v1_import +// +build test_rego_default_v1_import + +package rego_default + +import ( + "strings" + "testing" + + "github.com/open-policy-agent/opa/ast" + _ "github.com/open-policy-agent/opa/features/rego_default_v1" +) + +func TestDefaultRegoVersion(t *testing.T) { + if ast.DefaultRegoVersion() != ast.RegoV1 { + t.Fatal("expected default rego version to be v1") + } +} + +func TestParseModule(t *testing.T) { + tests := []struct { + note string + module string + expRules []string + expErrs []string + }{ + { + note: "v1 module", + module: `package test + +p contains x if { + x in [1, 2, 3] +}`, + expRules: []string{"p"}, + }, + { + note: "v0 module", + module: `package test + +p[x] { + x := [1, 2, 3][_] +}`, + expErrs: []string{ + "test.rego:3: rego_parse_error: `if` keyword is required before rule body", + "test.rego:3: rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + mod, err := ast.ParseModule("test.rego", tc.module) + + if len(tc.expErrs) > 0 { + if err == nil { + t.Fatalf("expected error(s) %q, got nil", tc.expErrs) + } + + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("expected error to contain:\n\n%s\n\ngot:\n\n%s", expErr, err) + } + } + } else { + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if len(mod.Rules) != len(tc.expRules) { + t.Fatalf("expected %d rules, got %d", len(tc.expRules), len(mod.Rules)) + } + + for i, rule := range mod.Rules { + if rule.Head.Name.String() != tc.expRules[i] { + t.Fatalf("expected rule %q, got %q", tc.expRules[i], rule.Head.Name.String()) + } + } + } + + }) + } + +} From 7bd64fd51a609c361e394395580f15c44dca7e09 Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Mon, 18 Nov 2024 22:22:55 +0100 Subject: [PATCH 4/7] Moving `ast.defaultRegoVersion`->`internal/rego_default.DefaultRegoVersion` Note: can't use ast.RegoVersion values directly because of circular dependencies. `DefaultRegoVersion` could alternatively be set to int-representation (manifest representation), but the required call to `ast.RegoVersionFromInt()` from `ast.DefaultRegoVersion()` is likely somewhat more costly. Signed-off-by: Johan Fylling --- ast/parser.go | 18 ++---------------- features/rego_default_v1/rego_default_v1.go | 7 +++++-- .../rego_default}/rego_default_v0.go | 5 +++-- .../rego_default}/rego_default_v1.go | 5 +++-- 4 files changed, 13 insertions(+), 22 deletions(-) rename {ast => internal/rego_default}/rego_default_v0.go (58%) rename {ast => internal/rego_default}/rego_default_v1.go (60%) diff --git a/ast/parser.go b/ast/parser.go index 1ae0781a6e..d37e7de25b 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -17,6 +17,7 @@ import ( "strings" "unicode/utf8" + "github.com/open-policy-agent/opa/internal/rego_default" "gopkg.in/yaml.v3" "github.com/open-policy-agent/opa/ast/internal/scanner" @@ -45,24 +46,9 @@ const ( ) func DefaultRegoVersion() RegoVersion { - return defaultRegoVersion + return RegoVersion(rego_default.DefaultRegoVersion) } -// SetDefaultRegoVersion sets the default RegoVersion. -// -// Direct use of this function is not recommended. -// Use [github.com/open-policy-agent/opa/features/rego_default_v1] instead to set the default RegoVersion to RegoV1. -func SetDefaultRegoVersion(v RegoVersion) { - defaultRegoVersion = v -} - -//func EffectiveRegoVersion(regoVersion RegoVersion) RegoVersion { -// if regoVersion == RegoUndefined { -// return DefaultRegoVersion() -// } -// return regoVersion -//} - func (v RegoVersion) Int() int { if v == RegoV1 { return 1 diff --git a/features/rego_default_v1/rego_default_v1.go b/features/rego_default_v1/rego_default_v1.go index 72b5435da0..7c8d42f6dc 100644 --- a/features/rego_default_v1/rego_default_v1.go +++ b/features/rego_default_v1/rego_default_v1.go @@ -9,8 +9,11 @@ // import _ "github.com/open-policy-agent/opa/internal/rego_default_v1" package rego_default_v1 -import "github.com/open-policy-agent/opa/ast" +import ( + "github.com/open-policy-agent/opa/ast" + "github.com/open-policy-agent/opa/internal/rego_default" +) func init() { - ast.SetDefaultRegoVersion(ast.RegoV1) + rego_default.DefaultRegoVersion = int(ast.RegoV1) } diff --git a/ast/rego_default_v0.go b/internal/rego_default/rego_default_v0.go similarity index 58% rename from ast/rego_default_v0.go rename to internal/rego_default/rego_default_v0.go index d3bf29c431..37be1982dc 100644 --- a/ast/rego_default_v0.go +++ b/internal/rego_default/rego_default_v0.go @@ -4,6 +4,7 @@ //go:build !opa_rego_v1 -package ast +package rego_default -var defaultRegoVersion = RegoV0 +// DefaultRegoVersion is the default Rego version (v0). +var DefaultRegoVersion = 1 // enum value for ast.RegoV0 diff --git a/ast/rego_default_v1.go b/internal/rego_default/rego_default_v1.go similarity index 60% rename from ast/rego_default_v1.go rename to internal/rego_default/rego_default_v1.go index 82c5a24285..847a59d90e 100644 --- a/ast/rego_default_v1.go +++ b/internal/rego_default/rego_default_v1.go @@ -5,6 +5,7 @@ //go:build opa_rego_v1 // +build opa_rego_v1 -package ast +package rego_default -var defaultRegoVersion = RegoV1 +// DefaultRegoVersion is the default Rego version (v1). +var DefaultRegoVersion = 3 // enum value for ast.RegoV1 From aa04bdf0e9a7fa6f6cd35120829bd2cf04e6bb1e Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Tue, 19 Nov 2024 10:26:07 +0100 Subject: [PATCH 5/7] Making v1 the default rego-version Signed-off-by: Johan Fylling --- Makefile | 14 +++++----- .../rego_default_v0.go} | 8 +++--- internal/rego_default/rego_default_v0.go | 3 ++- internal/rego_default/rego_default_v1.go | 3 +-- test/rego_default/rego_default_test.go | 27 +++++++++---------- 5 files changed, 26 insertions(+), 29 deletions(-) rename features/{rego_default_v1/rego_default_v1.go => rego_default_v0/rego_default_v0.go} (70%) diff --git a/Makefile b/Makefile index 4aee511101..da4be9541f 100644 --- a/Makefile +++ b/Makefile @@ -112,7 +112,7 @@ install: generate $(GO) install $(GO_TAGS) -ldflags $(LDFLAGS) .PHONY: test -test: go-test go-test-v1 go-test-v1-import wasm-test +test: go-test go-test-v0 go-test-v0-import wasm-test .PHONY: go-build go-build: generate @@ -122,13 +122,13 @@ go-build: generate go-test: generate $(GO) test $(GO_TAGS),slow ./... -.PHONY: go-test-v1 -go-test-v1: generate - $(GO) test $(GO_TAGS),opa_rego_v1,slow ./... +.PHONY: go-test-v0 +go-test-v0: generate + $(GO) test $(GO_TAGS),opa_rego_v0,slow ./... -.PHONY: go-test-v1-import -go-test-v1-import: generate - $(GO) test $(GO_TAGS),test_rego_default_v1_import ./test/rego_default/... +.PHONY: go-test-v0-import +go-test-v0-import: generate + $(GO) test $(GO_TAGS),test_rego_default_v0_import ./test/rego_default/... .PHONY: race-detector race-detector: generate diff --git a/features/rego_default_v1/rego_default_v1.go b/features/rego_default_v0/rego_default_v0.go similarity index 70% rename from features/rego_default_v1/rego_default_v1.go rename to features/rego_default_v0/rego_default_v0.go index 7c8d42f6dc..d82a1932aa 100644 --- a/features/rego_default_v1/rego_default_v1.go +++ b/features/rego_default_v0/rego_default_v0.go @@ -2,12 +2,12 @@ // Use of this source code is governed by an Apache2 // license that can be found in the LICENSE file. -// Package rego_default_v1 sets the default [ast.RegoVersion] to [ast.RegoV1]. +// Package rego_default_v0 sets the default [ast.RegoVersion] to [ast.RegoV0]. // // Usage (import side effects only): // -// import _ "github.com/open-policy-agent/opa/internal/rego_default_v1" -package rego_default_v1 +// import _ "github.com/open-policy-agent/opa/internal/rego_default_v0" +package rego_default_v0 import ( "github.com/open-policy-agent/opa/ast" @@ -15,5 +15,5 @@ import ( ) func init() { - rego_default.DefaultRegoVersion = int(ast.RegoV1) + rego_default.DefaultRegoVersion = int(ast.RegoV0) } diff --git a/internal/rego_default/rego_default_v0.go b/internal/rego_default/rego_default_v0.go index 37be1982dc..91f0521ed9 100644 --- a/internal/rego_default/rego_default_v0.go +++ b/internal/rego_default/rego_default_v0.go @@ -2,7 +2,8 @@ // Use of this source code is governed by an Apache2 // license that can be found in the LICENSE file. -//go:build !opa_rego_v1 +//go:build opa_rego_v0 +// +build opa_rego_v0 package rego_default diff --git a/internal/rego_default/rego_default_v1.go b/internal/rego_default/rego_default_v1.go index 847a59d90e..144c2a6e6e 100644 --- a/internal/rego_default/rego_default_v1.go +++ b/internal/rego_default/rego_default_v1.go @@ -2,8 +2,7 @@ // Use of this source code is governed by an Apache2 // license that can be found in the LICENSE file. -//go:build opa_rego_v1 -// +build opa_rego_v1 +//go:build !opa_rego_v0 package rego_default diff --git a/test/rego_default/rego_default_test.go b/test/rego_default/rego_default_test.go index 64c275414a..a02d48b486 100644 --- a/test/rego_default/rego_default_test.go +++ b/test/rego_default/rego_default_test.go @@ -2,8 +2,8 @@ // Use of this source code is governed by an Apache2 // license that can be found in the LICENSE file. -//go:build test_rego_default_v1_import -// +build test_rego_default_v1_import +//go:build test_rego_default_v0_import +// +build test_rego_default_v0_import package rego_default @@ -12,12 +12,12 @@ import ( "testing" "github.com/open-policy-agent/opa/ast" - _ "github.com/open-policy-agent/opa/features/rego_default_v1" + _ "github.com/open-policy-agent/opa/features/rego_default_v0" ) func TestDefaultRegoVersion(t *testing.T) { - if ast.DefaultRegoVersion() != ast.RegoV1 { - t.Fatal("expected default rego version to be v1") + if ast.DefaultRegoVersion() != ast.RegoV0 { + t.Fatalf("expected default rego version to be v0, got %s", ast.DefaultRegoVersion()) } } @@ -29,24 +29,23 @@ func TestParseModule(t *testing.T) { expErrs []string }{ { - note: "v1 module", + note: "v0 module", module: `package test -p contains x if { - x in [1, 2, 3] +p[x] { + x := [1, 2, 3][_] }`, expRules: []string{"p"}, }, { - note: "v0 module", + note: "v1 module", module: `package test -p[x] { - x := [1, 2, 3][_] +p contains x if { + x in [1, 2, 3] }`, expErrs: []string{ - "test.rego:3: rego_parse_error: `if` keyword is required before rule body", - "test.rego:3: rego_parse_error: `contains` keyword is required for partial set rules", + "test.rego:4: rego_parse_error: unexpected identifier token: expected \\n or ; or }", }, }, } @@ -80,8 +79,6 @@ p[x] { } } } - }) } - } From c13780e013a09a552563e1d73881229fe771274b Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Tue, 19 Nov 2024 14:20:17 +0100 Subject: [PATCH 6/7] Refactoring package names to avoid underscores. Signed-off-by: Johan Fylling --- ast/parser.go | 4 ++-- features/{rego_default_v0 => rego/v0}/rego_default_v0.go | 8 ++++---- .../{rego_default => rego/version}/rego_default_v0.go | 2 +- .../{rego_default => rego/version}/rego_default_v1.go | 2 +- test/rego_default/rego_default_test.go | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) rename features/{rego_default_v0 => rego/v0}/rego_default_v0.go (63%) rename internal/{rego_default => rego/version}/rego_default_v0.go (93%) rename internal/{rego_default => rego/version}/rego_default_v1.go (93%) diff --git a/ast/parser.go b/ast/parser.go index d37e7de25b..ba937b4a75 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -17,7 +17,7 @@ import ( "strings" "unicode/utf8" - "github.com/open-policy-agent/opa/internal/rego_default" + "github.com/open-policy-agent/opa/internal/rego/version" "gopkg.in/yaml.v3" "github.com/open-policy-agent/opa/ast/internal/scanner" @@ -46,7 +46,7 @@ const ( ) func DefaultRegoVersion() RegoVersion { - return RegoVersion(rego_default.DefaultRegoVersion) + return RegoVersion(version.DefaultRegoVersion) } func (v RegoVersion) Int() int { diff --git a/features/rego_default_v0/rego_default_v0.go b/features/rego/v0/rego_default_v0.go similarity index 63% rename from features/rego_default_v0/rego_default_v0.go rename to features/rego/v0/rego_default_v0.go index d82a1932aa..01729fff11 100644 --- a/features/rego_default_v0/rego_default_v0.go +++ b/features/rego/v0/rego_default_v0.go @@ -6,14 +6,14 @@ // // Usage (import side effects only): // -// import _ "github.com/open-policy-agent/opa/internal/rego_default_v0" -package rego_default_v0 +// import _ "github.com/open-policy-agent/opa/features/rego/v0 +package v0 import ( "github.com/open-policy-agent/opa/ast" - "github.com/open-policy-agent/opa/internal/rego_default" + "github.com/open-policy-agent/opa/internal/rego/version" ) func init() { - rego_default.DefaultRegoVersion = int(ast.RegoV0) + version.DefaultRegoVersion = int(ast.RegoV0) } diff --git a/internal/rego_default/rego_default_v0.go b/internal/rego/version/rego_default_v0.go similarity index 93% rename from internal/rego_default/rego_default_v0.go rename to internal/rego/version/rego_default_v0.go index 91f0521ed9..0ca4d6e2ff 100644 --- a/internal/rego_default/rego_default_v0.go +++ b/internal/rego/version/rego_default_v0.go @@ -5,7 +5,7 @@ //go:build opa_rego_v0 // +build opa_rego_v0 -package rego_default +package version // DefaultRegoVersion is the default Rego version (v0). var DefaultRegoVersion = 1 // enum value for ast.RegoV0 diff --git a/internal/rego_default/rego_default_v1.go b/internal/rego/version/rego_default_v1.go similarity index 93% rename from internal/rego_default/rego_default_v1.go rename to internal/rego/version/rego_default_v1.go index 144c2a6e6e..680de9500d 100644 --- a/internal/rego_default/rego_default_v1.go +++ b/internal/rego/version/rego_default_v1.go @@ -4,7 +4,7 @@ //go:build !opa_rego_v0 -package rego_default +package version // DefaultRegoVersion is the default Rego version (v1). var DefaultRegoVersion = 3 // enum value for ast.RegoV1 diff --git a/test/rego_default/rego_default_test.go b/test/rego_default/rego_default_test.go index a02d48b486..4c9b3c3d85 100644 --- a/test/rego_default/rego_default_test.go +++ b/test/rego_default/rego_default_test.go @@ -12,7 +12,7 @@ import ( "testing" "github.com/open-policy-agent/opa/ast" - _ "github.com/open-policy-agent/opa/features/rego_default_v0" + _ "github.com/open-policy-agent/opa/features/rego/v0" ) func TestDefaultRegoVersion(t *testing.T) { From 4d29b440e30bd5450ec43b9a11de7b111f35938a Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Tue, 19 Nov 2024 15:05:54 +0100 Subject: [PATCH 7/7] Fixing broken tests Signed-off-by: Johan Fylling --- internal/wasm/sdk/opa/opa_test.go | 36 +++++++++++++++++++------------ rego/rego_wasmtarget_test.go | 15 ++++++++----- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/internal/wasm/sdk/opa/opa_test.go b/internal/wasm/sdk/opa/opa_test.go index 0994406edf..7050d0b867 100644 --- a/internal/wasm/sdk/opa/opa_test.go +++ b/internal/wasm/sdk/opa/opa_test.go @@ -119,46 +119,50 @@ func TestOPA(t *testing.T) { }, { Description: "Runtime error/var assignment conflict", - Policy: `a = "b" { input > 1 } -a = "c" { input > 2 }`, + Policy: ` + import rego.v1 + a = "b" if { input > 1 } + a = "c" if { input > 2 }`, Query: "data.p.a = x", Evals: []Eval{ {Input: "3"}, }, - WantErr: "internal_error: module.rego:3:1: var assignment conflict", + WantErr: "internal_error: module.rego:5:5: var assignment conflict", }, { Description: "Runtime error/else conflict-1", Query: `data.p.q`, Policy: ` - q { + import rego.v1 + q if { false } - else = true { + else = true if { true } q = false`, Evals: []Eval{{}}, - WantErr: "internal_error: module.rego:9:5: var assignment conflict", + WantErr: "internal_error: module.rego:10:5: var assignment conflict", }, { Description: "Runtime error/else conflict-2", Query: `data.p.q`, Policy: ` - q { + import rego.v1 + q if { false } - else = false { + else = false if { true } - q { + q if { false } - else = true { + else = true if { true }`, Evals: []Eval{{}}, - WantErr: "internal_error: module.rego:12:5: var assignment conflict", + WantErr: "internal_error: module.rego:13:5: var assignment conflict", }, // NOTE(sr): The next two test cases were used to replicate issue // https://github.com/open-policy-agent/opa/issues/2962 -- their raison d'être @@ -166,8 +170,9 @@ a = "c" { input > 2 }`, { Description: "Only input changing, regex.match", Policy: ` + import rego.v1 default hello = false - hello { + hello if { regex.match("^world$", input.message) }`, Query: "data.p.hello = x", @@ -179,8 +184,9 @@ a = "c" { input > 2 }`, { Description: "Only input changing, glob.match", Policy: ` + import rego.v1 default hello = false - hello { + hello if { glob.match("world", [":"], input.message) }`, Query: "data.p.hello = x", @@ -215,7 +221,9 @@ a = "c" { input > 2 }`, { Description: "mpd init problem (#3110)", Query: `data.p.main = x`, - Policy: `main { numbers.range(1, 2)[_] == 2 }`, + Policy: ` + import rego.v1 + main if { numbers.range(1, 2)[_] == 2 }`, Evals: []Eval{ {Result: `{{"x": true}}`}, {Result: `{{"x": true}}`}, diff --git a/rego/rego_wasmtarget_test.go b/rego/rego_wasmtarget_test.go index 9c2d100985..33fa2461a6 100644 --- a/rego/rego_wasmtarget_test.go +++ b/rego/rego_wasmtarget_test.go @@ -38,8 +38,9 @@ func TestPrepareAndEvalWithWasmTarget(t *testing.T) { mod := ` package test + import rego.v1 default p = false - p { + p if { input.x == 1 } ` @@ -86,8 +87,9 @@ func TestPrepareAndEvalWithWasmTargetModulesOnCompiler(t *testing.T) { mod := ` package test + import rego.v1 default p = false - p { + p if { input.x == data.x.p } ` @@ -161,13 +163,15 @@ func TestEvalWithContextTimeout(t *testing.T) { // but calls the topdown function from the wasm instance's execution. // Also, it uses the topdown.Cancel mechanism for cancellation. cidrExpand := `package p -allow { +import rego.v1 +allow if { net.cidr_expand("1.0.0.0/1") }` // Also a host function, but uses context.Context for cancellation. httpSend := fmt.Sprintf(`package p -allow { +import rego.v1 +allow if { http.send({"method": "get", "url": "%s", "raise_error": true}) }`, ts.URL) @@ -175,7 +179,8 @@ allow { // This is a natively-implemented (for the wasm target) function that // takes long. numbersRange := `package p -allow { +import rego.v1 +allow if { numbers.range(1, 1e8)[_] == 1e8 }`