diff --git a/ast/compile.go b/ast/compile.go index 67c8ef3846..704e7c4d3f 100644 --- a/ast/compile.go +++ b/ast/compile.go @@ -1537,7 +1537,7 @@ func (c *Compiler) checkUnsafeBuiltins() { func (c *Compiler) checkDeprecatedBuiltins() { for _, name := range c.sorted { mod := c.Modules[name] - if c.strict || mod.regoV1Compatible { + if c.strict || mod.regoV1Compatible() { errs := checkDeprecatedBuiltins(c.deprecatedBuiltinsMap, mod) for _, err := range errs { c.err(err) @@ -1683,7 +1683,7 @@ func (c *Compiler) checkDuplicateImports() { for _, name := range c.sorted { mod := c.Modules[name] - if c.strict || mod.regoV1Compatible { + if c.strict || mod.regoV1Compatible() { modules = append(modules, mod) } } @@ -1697,7 +1697,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 || mod.regoV1Compatible() { errs := checkRootDocumentOverrides(mod) for _, err := range errs { c.err(err) diff --git a/ast/parser.go b/ast/parser.go index a2a3088952..25b1a4f1a8 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -27,6 +27,22 @@ import ( var RegoV1CompatibleRef = Ref{VarTerm("rego"), StringTerm("v1")} +// RegoVersion defines the Rego syntax requirements for a module. +type RegoVersion int + +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 +) + // Note: This state is kept isolated from the parser so that we // can do efficient shallow copies of these values when doing a // save() and restore(). @@ -99,14 +115,27 @@ func (e *parsedTermCacheItem) String() string { // ParserOptions defines the options for parsing Rego statements. type ParserOptions struct { - Capabilities *Capabilities - ProcessAnnotation bool - AllFutureKeywords bool - FutureKeywords []string - SkipRules bool - JSONOptions *astJSON.Options - unreleasedKeywords bool // TODO(sr): cleanup + Capabilities *Capabilities + ProcessAnnotation bool + AllFutureKeywords bool + FutureKeywords []string + SkipRules bool + JSONOptions *astJSON.Options + // RegoVersion is the version of Rego to parse for. + // RegoV1Compatible additionally affects the Rego version. Use EffectiveRegoVersion to get the effective Rego version. + RegoVersion RegoVersion + // RegoV1Compatible is equivalent to setting RegoVersion to RegoV0CompatV1. + // RegoV1Compatible takes precedence, and if set to true, RegoVersion is ignored. + // Deprecated: use RegoVersion instead. Will be removed in a future version of OPA. RegoV1Compatible bool + unreleasedKeywords bool // TODO(sr): cleanup +} + +func (po *ParserOptions) EffectiveRegoVersion() RegoVersion { + if po.RegoV1Compatible { + return RegoV0CompatV1 + } + return po.RegoVersion } // NewParser creates and initializes a Parser. @@ -189,6 +218,11 @@ func (p *Parser) WithJSONOptions(jsonOptions *astJSON.Options) *Parser { return p } +func (p *Parser) WithRegoVersion(version RegoVersion) *Parser { + p.po.RegoVersion = version + return p +} + func (p *Parser) parsedTermCacheLookup() (*Term, *state) { l := p.s.loc.Offset // stop comparing once the cached offsets are lower than l @@ -257,16 +291,23 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { allowedFutureKeywords := map[string]tokens.Token{} - for _, kw := range p.po.Capabilities.FutureKeywords { - var ok bool - allowedFutureKeywords[kw], ok = futureKeywords[kw] - if !ok { - return nil, nil, Errors{ - &Error{ - Code: ParseErr, - Message: fmt.Sprintf("illegal capabilities: unknown keyword: %v", kw), - Location: nil, - }, + if p.po.EffectiveRegoVersion() == RegoV1 { + // RegoV1 includes all future keywords in the default language definition + for k, v := range futureKeywords { + allowedFutureKeywords[k] = v + } + } else { + for _, kw := range p.po.Capabilities.FutureKeywords { + var ok bool + allowedFutureKeywords[kw], ok = futureKeywords[kw] + if !ok { + return nil, nil, Errors{ + &Error{ + Code: ParseErr, + Message: fmt.Sprintf("illegal capabilities: unknown keyword: %v", kw), + Location: nil, + }, + } } } } @@ -284,7 +325,7 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { } selected := map[string]tokens.Token{} - if p.po.AllFutureKeywords { + if p.po.AllFutureKeywords || p.po.EffectiveRegoVersion() == RegoV1 { for kw, tok := range allowedFutureKeywords { selected[kw] = tok } @@ -305,6 +346,12 @@ func (p *Parser) Parse() ([]Statement, []*Comment, Errors) { } p.s.s = p.s.s.WithKeywords(selected) + if p.po.EffectiveRegoVersion() == RegoV1 { + for kw, tok := range allowedFutureKeywords { + p.s.s.AddKeyword(kw, tok) + } + } + // read the first token to initialize the parser p.scan() @@ -2567,6 +2614,11 @@ func (p *Parser) regoV1Import(imp *Import) { return } + if p.po.EffectiveRegoVersion() == RegoV1 { + // We're parsing for Rego v1, where the 'rego.v1' import is a no-op. + return + } + path := imp.Path.Value.(Ref) if len(path) == 1 || !path[1].Equal(RegoV1CompatibleRef[1]) || len(path) > 2 { diff --git a/ast/parser_ext.go b/ast/parser_ext.go index c255915234..413416da29 100644 --- a/ast/parser_ext.go +++ b/ast/parser_ext.go @@ -477,7 +477,7 @@ func ParseModuleWithOpts(filename, input string, popts ParserOptions) (*Module, if err != nil { return nil, err } - return parseModule(filename, stmts, comments, popts.RegoV1Compatible) + return parseModule(filename, stmts, comments, popts.EffectiveRegoVersion()) } // ParseBody returns exactly one body. @@ -626,6 +626,7 @@ func ParseStatementsWithOpts(filename, input string, popts ParserOptions) ([]Sta WithCapabilities(popts.Capabilities). WithSkipRules(popts.SkipRules). WithJSONOptions(popts.JSONOptions). + WithRegoVersion(popts.EffectiveRegoVersion()). withUnreleasedKeywords(popts.unreleasedKeywords) stmts, comments, errs := parser.Parse() @@ -637,7 +638,7 @@ func ParseStatementsWithOpts(filename, input string, popts ParserOptions) ([]Sta return stmts, comments, nil } -func parseModule(filename string, stmts []Statement, comments []*Comment, regoV1Compatible bool) (*Module, error) { +func parseModule(filename string, stmts []Statement, comments []*Comment, regoCompatibilityMode RegoVersion) (*Module, error) { if len(stmts) == 0 { return nil, NewError(ParseErr, &Location{File: filename}, "empty module") @@ -658,14 +659,14 @@ func parseModule(filename string, stmts []Statement, comments []*Comment, regoV1 // The comments slice only holds comments that were not their own statements. mod.Comments = append(mod.Comments, comments...) - mod.regoV1Compatible = regoV1Compatible + mod.regoVersion = regoCompatibilityMode for i, stmt := range stmts[1:] { switch stmt := stmt.(type) { case *Import: mod.Imports = append(mod.Imports, stmt) - if Compare(stmt.Path.Value, RegoV1CompatibleRef) == 0 { - mod.regoV1Compatible = true + if mod.regoVersion == RegoV0 && Compare(stmt.Path.Value, RegoV1CompatibleRef) == 0 { + mod.regoVersion = RegoV0CompatV1 } case *Rule: setRuleModule(stmt, mod) @@ -694,7 +695,7 @@ func parseModule(filename string, stmts []Statement, comments []*Comment, regoV1 } } - if mod.regoV1Compatible { + if mod.regoVersion == RegoV0CompatV1 || mod.regoVersion == RegoV1 { for _, rule := range mod.Rules { for r := rule; r != nil; r = r.Else { var t string diff --git a/ast/policy.go b/ast/policy.go index 2679a5f10c..187bb8d765 100644 --- a/ast/policy.go +++ b/ast/policy.go @@ -145,13 +145,13 @@ type ( // within a namespace (defined by the package) and optional // dependencies on external documents (defined by imports). Module struct { - Package *Package `json:"package"` - Imports []*Import `json:"imports,omitempty"` - Annotations []*Annotations `json:"annotations,omitempty"` - Rules []*Rule `json:"rules,omitempty"` - Comments []*Comment `json:"comments,omitempty"` - stmts []Statement - regoV1Compatible bool + Package *Package `json:"package"` + Imports []*Import `json:"imports,omitempty"` + Annotations []*Annotations `json:"annotations,omitempty"` + Rules []*Rule `json:"rules,omitempty"` + Comments []*Comment `json:"comments,omitempty"` + stmts []Statement + regoVersion RegoVersion } // Comment contains the raw text from the comment in the definition. @@ -399,6 +399,14 @@ func (mod *Module) UnmarshalJSON(bs []byte) error { return nil } +func (mod *Module) regoV1Compatible() bool { + return mod.regoVersion == RegoV1 || mod.regoVersion == RegoV0CompatV1 +} + +func (mod *Module) RegoVersion() RegoVersion { + return mod.regoVersion +} + // NewComment returns a new Comment object. func NewComment(text []byte) *Comment { return &Comment{ diff --git a/bundle/bundle.go b/bundle/bundle.go index 66c8ce37c6..72927123a8 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -400,6 +400,7 @@ type Reader struct { lazyLoadingMode bool name string persist bool + regoVersion ast.RegoVersion } // NewReader is deprecated. Use NewCustomReader instead. @@ -503,11 +504,17 @@ func (r *Reader) WithBundlePersistence(persist bool) *Reader { return r } +func (r *Reader) WithRegoVersion(version ast.RegoVersion) *Reader { + r.regoVersion = version + return r +} + func (r *Reader) ParserOptions() ast.ParserOptions { return ast.ParserOptions{ ProcessAnnotation: r.processAnnotations, Capabilities: r.capabilities, JSONOptions: r.jsonOptions, + RegoVersion: r.regoVersion, } } @@ -1005,12 +1012,18 @@ func hashBundleFiles(hash SignatureHasher, b *Bundle) ([]FileInfo, error) { } // FormatModules formats Rego modules +// Modules will be formatted to comply with rego-v1, but Rego compatibility of individual parsed modules will be respected (e.g. if 'rego.v1' is imported). func (b *Bundle) FormatModules(useModulePath bool) error { + return b.FormatModulesForRegoVersion(ast.RegoV0, true, useModulePath) +} + +// FormatModulesForRegoVersion formats Rego modules to comply with a given Rego version +func (b *Bundle) FormatModulesForRegoVersion(version ast.RegoVersion, preserveModuleRegoVersion bool, useModulePath bool) error { var err error for i, module := range b.Modules { if module.Raw == nil { - module.Raw, err = format.Ast(module.Parsed) + module.Raw, err = format.AstWithOpts(module.Parsed, format.Opts{RegoVersion: version}) if err != nil { return err } @@ -1020,7 +1033,14 @@ func (b *Bundle) FormatModules(useModulePath bool) error { path = module.Path } - module.Raw, err = format.Source(path, module.Raw) + opts := format.Opts{} + if preserveModuleRegoVersion { + opts.RegoVersion = module.Parsed.RegoVersion() + } else { + opts.RegoVersion = version + } + + module.Raw, err = format.SourceWithOpts(path, module.Raw, opts) if err != nil { return err } diff --git a/bundle/store.go b/bundle/store.go index d93d147c05..45bcf6e559 100644 --- a/bundle/store.go +++ b/bundle/store.go @@ -301,6 +301,7 @@ type ActivateOpts struct { Bundles map[string]*Bundle // Optional ExtraModules map[string]*ast.Module // Optional AuthorizationDecisionRef ast.Ref + ParserOptions ast.ParserOptions legacy bool } @@ -314,10 +315,11 @@ func Activate(opts *ActivateOpts) error { // DeactivateOpts defines options for the Deactivate API call type DeactivateOpts struct { - Ctx context.Context - Store storage.Store - Txn storage.Transaction - BundleNames map[string]struct{} + Ctx context.Context + Store storage.Store + Txn storage.Transaction + BundleNames map[string]struct{} + ParserOptions ast.ParserOptions } // Deactivate the bundle(s). This will erase associated data, policies, and the manifest entry from the store. @@ -332,7 +334,7 @@ func Deactivate(opts *DeactivateOpts) error { erase[root] = struct{}{} } } - _, err := eraseBundles(opts.Ctx, opts.Store, opts.Txn, opts.BundleNames, erase) + _, err := eraseBundles(opts.Ctx, opts.Store, opts.Txn, opts.ParserOptions, opts.BundleNames, erase) return err } @@ -382,7 +384,7 @@ func activateBundles(opts *ActivateOpts) error { // Erase data and policies at new + old roots, and remove the old // manifests before activating a new snapshot bundle. - remaining, err := eraseBundles(opts.Ctx, opts.Store, opts.Txn, names, erase) + remaining, err := eraseBundles(opts.Ctx, opts.Store, opts.Txn, opts.ParserOptions, names, erase) if err != nil { return err } @@ -585,13 +587,13 @@ func activateDeltaBundles(opts *ActivateOpts, bundles map[string]*Bundle) error // erase bundles by name and roots. This will clear all policies and data at its roots and remove its // manifest from storage. -func eraseBundles(ctx context.Context, store storage.Store, txn storage.Transaction, names map[string]struct{}, roots map[string]struct{}) (map[string]*ast.Module, error) { +func eraseBundles(ctx context.Context, store storage.Store, txn storage.Transaction, parserOpts ast.ParserOptions, names map[string]struct{}, roots map[string]struct{}) (map[string]*ast.Module, error) { if err := eraseData(ctx, store, txn, roots); err != nil { return nil, err } - remaining, err := erasePolicies(ctx, store, txn, roots) + remaining, err := erasePolicies(ctx, store, txn, parserOpts, roots) if err != nil { return nil, err } @@ -633,7 +635,7 @@ func eraseData(ctx context.Context, store storage.Store, txn storage.Transaction return nil } -func erasePolicies(ctx context.Context, store storage.Store, txn storage.Transaction, roots map[string]struct{}) (map[string]*ast.Module, error) { +func erasePolicies(ctx context.Context, store storage.Store, txn storage.Transaction, parserOpts ast.ParserOptions, roots map[string]struct{}) (map[string]*ast.Module, error) { ids, err := store.ListPolicies(ctx, txn) if err != nil { @@ -647,7 +649,7 @@ func erasePolicies(ctx context.Context, store storage.Store, txn storage.Transac if err != nil { return nil, err } - module, err := ast.ParseModule(id, string(bs)) + module, err := ast.ParseModuleWithOpts(id, string(bs), parserOpts) if err != nil { return nil, err } diff --git a/bundle/store_test.go b/bundle/store_test.go index fb433adc3d..7d6bae7d81 100644 --- a/bundle/store_test.go +++ b/bundle/store_test.go @@ -4294,7 +4294,7 @@ func TestErasePolicies(t *testing.T) { for _, root := range tc.roots { roots[root] = struct{}{} } - remaining, err := erasePolicies(ctx, mockStore, txn, roots) + remaining, err := erasePolicies(ctx, mockStore, txn, ast.ParserOptions{}, roots) if !tc.expectErr && err != nil { t.Fatalf("unepected error: %s", err) } else if tc.expectErr && err == nil { diff --git a/cmd/build.go b/cmd/build.go index ac81a5937d..513f00cf47 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -43,6 +43,7 @@ type buildParams struct { excludeVerifyFiles []string plugin string ns string + v1Compatible bool } func newBuildParams() buildParams { @@ -253,6 +254,8 @@ against OPA v0.22.0: addSigningPluginFlag(buildCommand.Flags(), &buildParams.plugin) addClaimsFileFlag(buildCommand.Flags(), &buildParams.claimsFile) + addV1CompatibleFlag(buildCommand.Flags(), &buildParams.v1Compatible, false) + RootCommand.AddCommand(buildCommand) } @@ -300,6 +303,10 @@ func dobuild(params buildParams, args []string) error { WithBundleSigningConfig(bsc). WithPartialNamespace(params.ns) + if params.v1Compatible { + compiler = compiler.WithRegoVersion(ast.RegoV1) + } + if params.revision.isSet { compiler = compiler.WithRevision(*params.revision.v) } diff --git a/cmd/build_test.go b/cmd/build_test.go index b2c062d497..49a478b8e5 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -976,3 +976,322 @@ func TestBuildBundleModeIgnoreFlag(t *testing.T) { } }) } + +func TestBuildWithV1CompatibleFlag(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + files map[string]string + expectedFiles map[string]string + expectedErr string + }{ + { + note: "default compatibility: policy with no rego.v1 or future.keywords imports", + files: map[string]string{ + "test.rego": `package test + allow if { + 1 < 2 + }`, + }, + expectedErr: "rego_parse_error", + }, + { + note: "default compatibility: policy with rego.v1 imports", + files: map[string]string{ + "test.rego": `package test + import rego.v1 + allow if { + 1 < 2 + }`, + }, + // Imports are preserved + expectedFiles: map[string]string{ + "test.rego": `package test + +import rego.v1 + +allow if { + 1 < 2 +} +`, + }, + }, + { + note: "default compatibility: policy with future.keywords imports", + files: map[string]string{ + "test.rego": `package test + import future.keywords.if + allow if { + 1 < 2 + }`, + }, + // Imports are preserved + expectedFiles: map[string]string{ + "test.rego": `package test + +import future.keywords.if + +allow if { + 1 < 2 +} +`, + }, + }, + { + note: "1.0 compatibility: policy with no rego.v1 or future.keywords imports", + v1Compatible: true, + files: map[string]string{ + "test.rego": `package test + allow if { + 1 < 2 + }`, + }, + // Imports are not added in + expectedFiles: map[string]string{ + "test.rego": `package test + +allow if { + 1 < 2 +} +`, + }, + }, + { + note: "1.0 compatibility: policy with rego.v1 import", + v1Compatible: true, + files: map[string]string{ + "test.rego": `package test + import rego.v1 + allow if { + 1 < 2 + }`, + }, + // the rego.v1 import is obsolete in rego-v1, and is removed + expectedFiles: map[string]string{ + "test.rego": `package test + +allow if { + 1 < 2 +} +`, + }, + }, + { + note: "1.0 compatibility: policy with future.keywords import", + v1Compatible: true, + files: map[string]string{ + "test.rego": `package test + import future.keywords.if + allow if { + 1 < 2 + }`, + }, + // future.keywords imports are obsolete in rego-v1, and are removed + expectedFiles: map[string]string{ + "test.rego": `package test + +allow if { + 1 < 2 +} +`, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + test.WithTempFS(tc.files, func(root string) { + params := newBuildParams() + params.outputFile = path.Join(root, "bundle.tar.gz") + params.v1Compatible = tc.v1Compatible + + err := dobuild(params, []string{root}) + + if tc.expectedErr != "" { + if err == nil { + t.Fatal("expected error but got nil") + } + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Fatalf("expected error %v, got %v", tc.expectedErr, err) + } + } else { + if err != nil { + t.Fatal(err) + } + + fl := loader.NewFileLoader() + if tc.v1Compatible { + fl = fl.WithRegoVersion(ast.RegoV1) + } + _, err = fl.AsBundle(params.outputFile) + if err != nil { + t.Fatal(err) + } + + // Check that manifest is not written given no input manifest and no other flags + f, err := os.Open(params.outputFile) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + gr, err := gzip.NewReader(f) + if err != nil { + t.Fatal(err) + } + + tr := tar.NewReader(gr) + + for { + f, err := tr.Next() + if err == io.EOF { + break + } else if err != nil { + t.Fatal(err) + } + expectedFile := tc.expectedFiles[path.Base(f.Name)] + if expectedFile != "" { + data, err := io.ReadAll(tr) + if err != nil { + t.Fatal(err) + } + actualFile := string(data) + if actualFile != expectedFile { + t.Fatalf("expected optimized module:\n\n%v\n\ngot:\n\n%v", expectedFile, actualFile) + } + } + } + } + }) + }) + } +} + +func TestBuildWithV1CompatibleFlagOptimized(t *testing.T) { + tests := []struct { + note string + files map[string]string + expectedFiles map[string]string + }{ + { + note: "No imports", + files: map[string]string{ + "test.rego": `package test +# METADATA +# entrypoint: true +p[k] contains v if { + k := "foo" + v := input.v +} +`, + }, + expectedFiles: map[string]string{ + "/optimized/test/p.rego": `package test.p + +foo contains __local1__1 if { + __local1__1 = input.v +} +`, + }, + }, + { + note: "rego.v1 imported", + files: map[string]string{ + "test.rego": `package test +import rego.v1 +# METADATA +# entrypoint: true +p[k] contains v if { + k := "foo" + v := input.v +} +`, + }, + // Note: the rego.v1 import isn't added to the optimized module. + // This is ok, as the bundle was built with the --v1-compatible flag, + // and is therefore only guaranteed to work with OPA 1.0 or when + // OPA 0.x is run with the --rego-v1 flag. + // TODO: add rego-v1 flag to bundle, so `opa run` etc. doesn't need the --rego-v1 flag to consume it. + expectedFiles: map[string]string{ + "/optimized/test/p.rego": `package test.p + +foo contains __local1__1 if { + __local1__1 = input.v +} +`, + }, + }, + { + note: "future.keywords imported", + files: map[string]string{ + "test.rego": `package test +import future.keywords +# METADATA +# entrypoint: true +p[k] contains v if { + k := "foo" + v := input.v +} +`, + }, + expectedFiles: map[string]string{ + "/optimized/test/p.rego": `package test.p + +foo contains __local1__1 if { + __local1__1 = input.v +} +`, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + test.WithTempFS(tc.files, func(root string) { + params := newBuildParams() + params.outputFile = path.Join(root, "bundle.tar.gz") + params.v1Compatible = true + params.optimizationLevel = 1 + + err := dobuild(params, []string{root}) + + if err != nil { + t.Fatal(err) + } + + f, err := os.Open(params.outputFile) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + gr, err := gzip.NewReader(f) + if err != nil { + t.Fatal(err) + } + + tr := tar.NewReader(gr) + + for { + f, err := tr.Next() + if err == io.EOF { + break + } else if err != nil { + t.Fatal(err) + } + expectedFile := tc.expectedFiles[f.Name] + if expectedFile != "" { + data, err := io.ReadAll(tr) + if err != nil { + t.Fatal(err) + } + actualFile := string(data) + if actualFile != expectedFile { + t.Fatalf("expected optimized module:\n\n%v\n\ngot:\n\n%v", expectedFile, actualFile) + } + } + } + }) + }) + } +} diff --git a/cmd/check.go b/cmd/check.go index 2e07721404..b32d6c40c8 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -26,6 +26,7 @@ type checkParams struct { schema *schemaFlags strict bool regoV1 bool + v1Compatible bool } func newCheckParams() checkParams { @@ -38,6 +39,17 @@ func newCheckParams() checkParams { } } +func (p *checkParams) regoVersion() ast.RegoVersion { + // The '--rego-v1' flag takes precedence over the '--v1-compatible' flag. + if p.regoV1 { + return ast.RegoV0CompatV1 + } + if p.v1Compatible { + return ast.RegoV1 + } + return ast.RegoV0 +} + const ( checkFormatPretty = "pretty" checkFormatJSON = "json" @@ -65,7 +77,7 @@ func checkModules(params checkParams, args []string) error { if params.bundleMode { for _, path := range args { b, err := loader.NewFileLoader(). - WithRegoV1Compatible(params.regoV1). + WithRegoVersion(params.regoVersion()). WithSkipBundleVerification(true). WithProcessAnnotation(true). WithCapabilities(capabilities). @@ -84,7 +96,7 @@ func checkModules(params checkParams, args []string) error { } result, err := loader.NewFileLoader(). - WithRegoV1Compatible(params.regoV1). + WithRegoVersion(params.regoVersion()). WithProcessAnnotation(true). WithCapabilities(capabilities). Filtered(args, f.Apply) @@ -168,6 +180,8 @@ func init() { addCapabilitiesFlag(checkCommand.Flags(), checkParams.capabilities) addSchemaFlags(checkCommand.Flags(), checkParams.schema) addStrictFlag(checkCommand.Flags(), &checkParams.strict, false) - checkCommand.Flags().BoolVar(&checkParams.regoV1, "rego-v1", false, "check for Rego v1 compatibility (policies must also be compatible with current OPA version)") + addRegoV1FlagWithDescription(checkCommand.Flags(), &checkParams.regoV1, false, + "check for Rego v1 compatibility (policies must also be compatible with current OPA version)") + addV1CompatibleFlag(checkCommand.Flags(), &checkParams.v1Compatible, false) RootCommand.AddCommand(checkCommand) } diff --git a/cmd/check_test.go b/cmd/check_test.go index bbc9501e95..6eedae4c4c 100644 --- a/cmd/check_test.go +++ b/cmd/check_test.go @@ -367,3 +367,130 @@ p contains x if { }) } } + +func TestCheckV1Compatible(t *testing.T) { + cases := []struct { + note string + policy string + expErrs []string + }{ + { + note: "rego.v1 imported, v1 compliant", + policy: `package test +import rego.v1 +p contains x if { + x := [1,2,3] +}`, + }, + { + note: "rego.v1 imported, NOT v1 compliant (parser)", + policy: `package test +import rego.v1 +p contains x { + x := [1,2,3] +} + +q.r`, + expErrs: []string{ + "test.rego:3: rego_parse_error: `if` keyword is required before rule body", + "test.rego:7: rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "rego.v1 imported, NOT v1 compliant (compiler)", + policy: `package test +import rego.v1 + +import data.foo +import data.bar as foo +`, + expErrs: []string{ + "test.rego:5: rego_compile_error: import must not shadow import data.foo", + }, + }, + { + note: "keywords imported, v1 compliant", + policy: `package test +import future.keywords.if +import future.keywords.contains +p contains x if { + x := [1,2,3] +}`, + }, + { + note: "keywords imported, NOT v1 compliant", + policy: `package test +import future.keywords.contains +p contains x { + x := [1,2,3] +} + +q.r`, + expErrs: []string{ + "test.rego:3: rego_parse_error: `if` keyword is required before rule body", + "test.rego:7: rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "keywords imported, NOT v1 compliant (compiler)", + policy: `package test +import future.keywords.if + +input := 1 if { + 1 == 2 +}`, + expErrs: []string{ + "test.rego:4: rego_compile_error: rules must not shadow input (use a different rule name)", + }, + }, + { + note: "no imports, v1 compliant", + policy: `package test +p := 1 +`, + }, + { + note: "no imports, NOT v1 compliant but v0 compliant (compiler)", + policy: `package test +p.x`, + expErrs: []string{ + "test.rego:2: rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "no imports, v1 compliant but NOT v0 compliant", + policy: `package test +p contains x if { + x := [1,2,3] +}`, + }, + } + + for _, tc := range cases { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "test.rego": tc.policy, + } + + test.WithTempFS(files, func(root string) { + params := newCheckParams() + params.v1Compatible = true + + err := checkModules(params, []string{root}) + switch { + case err != nil && len(tc.expErrs) > 0: + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("expected err:\n\n%v\n\ngot:\n\n%v", expErr, err) + } + } + return // don't read back bundle below + case err != nil && len(tc.expErrs) == 0: + t.Fatalf("unexpected error: %v", err) + case err == nil && len(tc.expErrs) > 0: + t.Fatalf("expected error:\n\n%v\n\ngot: none", tc.expErrs) + } + }) + }) + } +} diff --git a/cmd/eval.go b/cmd/eval.go index 53911c9818..c32f2fc5df 100644 --- a/cmd/eval.go +++ b/cmd/eval.go @@ -70,6 +70,7 @@ type evalCommandParams struct { optimizationLevel int entrypoints repeatedStringFlag strict bool + v1Compatible bool } func newEvalCommandParams() evalCommandParams { @@ -326,6 +327,7 @@ access. addTargetFlag(evalCommand.Flags(), params.target) addCountFlag(evalCommand.Flags(), ¶ms.count, "benchmark") addStrictFlag(evalCommand.Flags(), ¶ms.strict, false) + addV1CompatibleFlag(evalCommand.Flags(), ¶ms.v1Compatible, false) RootCommand.AddCommand(evalCommand) } @@ -663,6 +665,10 @@ 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, diff --git a/cmd/eval_test.go b/cmd/eval_test.go index caaa233bba..1bb2ed9059 100755 --- a/cmd/eval_test.go +++ b/cmd/eval_test.go @@ -1880,3 +1880,113 @@ func TestUnexpectedElseIfErr(t *testing.T) { } }) } + +func TestEvalPolicyWithV1CompatibleFlag(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + modules map[string]string + query string + expectedErr string + }{ + { + note: "default compatibility: policy with no rego.v1 or future.keywords imports", + modules: map[string]string{ + "test.rego": `package test + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + expectedErr: "rego_parse_error", + }, + { + note: "1.0 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", + }, + { + note: "1.0 compatibility: policy with rego.v1 import", + v1Compatible: true, + modules: map[string]string{ + "test.rego": `package test + import rego.v1 + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + }, + { + note: "1.0 compatibility: policy with future.keywords import", + v1Compatible: true, + modules: map[string]string{ + "test.rego": `package test + import future.keywords.if + allow if { + 1 < 2 + }`, + }, + query: "data.test.allow", + }, + } + + setup := []struct { + name string + commandParams func(params *evalCommandParams, path string) + }{ + { + name: "Files", + commandParams: func(params *evalCommandParams, path string) { + params.dataPaths = newrepeatedStringFlag([]string{path}) + }, + }, + { + name: "Bundle", + commandParams: func(params *evalCommandParams, path string) { + if err := params.bundlePaths.Set(path); err != nil { + t.Fatal(err) + } + }, + }, + } + + for _, s := range setup { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s: %s", s.name, tc.note), func(t *testing.T) { + test.WithTempFS(tc.modules, func(path string) { + params := newEvalCommandParams() + s.commandParams(¶ms, path) + params.v1Compatible = tc.v1Compatible + + var buf bytes.Buffer + + defined, err := eval([]string{tc.query}, params, &buf) + + if tc.expectedErr == "" { + if err != nil { + t.Fatalf("Unexpected error: %v, buf: %s", err, buf.String()) + } else if !defined { + t.Fatal("expected result to be defined") + } + } else { + if err == nil { + t.Fatal("expected error, got none") + } + + actual := buf.String() + if !strings.Contains(actual, tc.expectedErr) { + t.Fatalf("expected error:\n\n%v\n\ngot\n\n%v", tc.expectedErr, actual) + } + } + }) + }) + } + } +} diff --git a/cmd/flags.go b/cmd/flags.go index 264fdb3c82..0161900c21 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -153,6 +153,14 @@ func addStrictFlag(fs *pflag.FlagSet, strict *bool, value bool) { fs.BoolVarP(strict, "strict", "S", value, "enable compiler strict mode") } +func addRegoV1FlagWithDescription(fs *pflag.FlagSet, regoV1 *bool, value bool, description string) { + fs.BoolVar(regoV1, "rego-v1", value, description) +} + +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") +} + func addE2EFlag(fs *pflag.FlagSet, e2e *bool, value bool) { fs.BoolVar(e2e, "e2e", value, "run benchmarks against a running OPA server") } diff --git a/cmd/fmt.go b/cmd/fmt.go index c14519209e..9876fb9fff 100644 --- a/cmd/fmt.go +++ b/cmd/fmt.go @@ -14,20 +14,33 @@ import ( "github.com/sergi/go-diff/diffmatchpatch" "github.com/spf13/cobra" + "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/format" fileurl "github.com/open-policy-agent/opa/internal/file/url" ) type fmtCommandParams struct { - overwrite bool - list bool - diff bool - fail bool - regoV1 bool + overwrite bool + list bool + diff bool + fail bool + regoV1 bool + v1Compatible bool } var fmtParams = fmtCommandParams{} +func (p *fmtCommandParams) regoVersion() ast.RegoVersion { + // The '--rego-v1' flag takes precedence over the '--v1-compatible' flag. + if p.regoV1 { + return ast.RegoV0CompatV1 + } + if p.v1Compatible { + return ast.RegoV1 + } + return ast.RegoV0 +} + var formatCommand = &cobra.Command{ Use: "fmt [path [...]]", Short: "Format Rego source files", @@ -109,7 +122,9 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o return newError("failed to open file: %v", err) } - formatted, err := format.SourceWithOpts(filename, contents, format.Opts{RegoV1: params.regoV1}) + opts := format.Opts{} + opts.RegoVersion = params.regoVersion() + formatted, err := format.SourceWithOpts(filename, contents, opts) if err != nil { return newError("failed to format Rego source file: %v", err) } @@ -170,7 +185,9 @@ func formatStdin(params *fmtCommandParams, r io.Reader, w io.Writer) error { return err } - formatted, err := format.SourceWithOpts("stdin", contents, format.Opts{RegoV1: params.regoV1}) + opts := format.Opts{} + opts.RegoVersion = params.regoVersion() + formatted, err := format.SourceWithOpts("stdin", contents, opts) if err != nil { return err } @@ -206,6 +223,8 @@ func init() { formatCommand.Flags().BoolVarP(&fmtParams.list, "list", "l", false, "list all files who would change when formatted") 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") - formatCommand.Flags().BoolVar(&fmtParams.regoV1, "rego-v1", false, "format as Rego v1") + addRegoV1FlagWithDescription(formatCommand.Flags(), &fmtParams.regoV1, false, "format module(s) to be compatible with both Rego v1 and current OPA version)") + addV1CompatibleFlag(formatCommand.Flags(), &fmtParams.v1Compatible, false) + RootCommand.AddCommand(formatCommand) } diff --git a/cmd/fmt_test.go b/cmd/fmt_test.go index 5c007de17b..f36405aa8f 100644 --- a/cmd/fmt_test.go +++ b/cmd/fmt_test.go @@ -585,3 +585,161 @@ q := all([true, false]) }) } } + +func TestFmtV1Compatible(t *testing.T) { + tests := []struct { + note string + input string + expected string + expectedErrs []string + }{ + { + note: "no keywords used", + input: `package test +p { + input.x == 1 +} + +q.foo { + input.x == 2 +} +`, + expectedErrs: []string{ + "policy.rego:2: rego_parse_error: `if` keyword is required before rule body", + "policy.rego:6: rego_parse_error: `if` keyword is required before rule body", + "policy.rego:6: rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "no imports", + input: `package test +p if { + input.x == 1 +} + +q contains "foo" if { + input.x == 2 +} +`, + expected: `package test + +p if { + input.x == 1 +} + +q contains "foo" if { + input.x == 2 +} +`, + }, + { + note: "future imports", + input: `package test +import future.keywords +p if { + input.x == 1 +} + +q contains "foo" if { + input.x == 2 +} +`, + expected: `package test + +p if { + input.x == 1 +} + +q contains "foo" if { + input.x == 2 +} +`, + }, + { + note: "duplicate imports", + input: `package test +import data.foo +import data.bar as foo +`, + expectedErrs: []string{ + `policy.rego:3: rego_compile_error: import must not shadow import data.foo`, + }, + }, + { + note: "root document overrides", + input: `package test +input if { + 1 == 1 +} + +p if { + data := 2 +}`, + expectedErrs: []string{ + `policy.rego:2: rego_compile_error: rules must not shadow input (use a different rule name)`, + `policy.rego:7: rego_compile_error: variables must not shadow data (use a different variable name)`, + }, + }, + { + note: "deprecated built-in", + input: `package test +p if { + any([true, false]) +} + +q := all([true, false]) +`, + expectedErrs: []string{ + `policy.rego:3: rego_type_error: deprecated built-in function calls in expression: any`, + `policy.rego:6: rego_type_error: deprecated built-in function calls in expression: all`, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + params := fmtCommandParams{ + v1Compatible: true, + } + + files := map[string]string{ + "policy.rego": tc.input, + } + + var stdout bytes.Buffer + + test.WithTempFS(files, func(root string) { + policyFile := filepath.Join(root, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(¶ms, &stdout, policyFile, info, err) + + if len(tc.expectedErrs) > 0 { + if err == nil { + t.Fatalf("Expected error but got: %s", stdout.String()) + } + + for _, expectedErr := range tc.expectedErrs { + var actualErr string + switch err := err.(type) { + case fmtError: + actualErr = err.msg + default: + actualErr = err.Error() + } + if !strings.Contains(actualErr, expectedErr) { + t.Fatalf("Expected error to contain:\n\n%s\n\nGot error:\n\n%s\n\n", expectedErr, actualErr) + } + } + } else { + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + actual := stdout.String() + if actual != tc.expected { + t.Fatalf("Expected:%s\n\nGot:\n%s\n\n", tc.expected, actual) + } + } + }) + }) + } +} diff --git a/cmd/run.go b/cmd/run.go index 577fd2fddf..ba5196e652 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -205,7 +205,7 @@ Current behaviors enabled by this flag include: 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") - runCommand.Flags().BoolVar(&cmdParams.rt.V1Compatible, "v1-compatible", false, "opt-in to OPA features and behaviors that will be enabled by default in a future OPA v1.0 release") + addV1CompatibleFlag(runCommand.Flags(), &cmdParams.rt.V1Compatible, false) addMaxErrorsFlag(runCommand.Flags(), &cmdParams.rt.ErrorLimit) runCommand.Flags().BoolVar(&cmdParams.rt.PprofEnabled, "pprof", false, "enables pprof endpoints") runCommand.Flags().StringVar(&cmdParams.tlsCertFile, "tls-cert-file", "", "set path of TLS certificate file") diff --git a/cmd/test.go b/cmd/test.go index 36b863601f..01e32b6a0e 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -60,6 +60,7 @@ type testCommandParams struct { stopChan chan os.Signal output io.Writer errOutput io.Writer + v1Compatible bool } func newTestCommandParams() testCommandParams { @@ -75,6 +76,13 @@ func newTestCommandParams() testCommandParams { } } +func (p *testCommandParams) RegoVersion() ast.RegoVersion { + if p.v1Compatible { + return ast.RegoV1 + } + return ast.RegoV0 +} + func opaTest(args []string, testParams testCommandParams) (int, error) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -101,10 +109,10 @@ func opaTest(args []string, testParams testCommandParams) (int, error) { var store storage.Store if testParams.bundleMode { - bundles, err = tester.LoadBundles(args, filter.Apply) + bundles, err = tester.LoadBundlesWithRegoVersion(args, filter.Apply, testParams.RegoVersion()) store = inmem.NewWithOpts(inmem.OptRoundTripOnWrite(false)) } else { - modules, store, err = tester.Load(args, filter.Apply) + modules, store, err = tester.LoadWithRegoVersion(args, filter.Apply, testParams.RegoVersion()) } if err != nil { @@ -538,6 +546,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) + addV1CompatibleFlag(testCommand.Flags(), &testParams.v1Compatible, false) RootCommand.AddCommand(testCommand) } diff --git a/cmd/test_test.go b/cmd/test_test.go index e7d62a76fc..c6731fe0f3 100644 --- a/cmd/test_test.go +++ b/cmd/test_test.go @@ -3,6 +3,7 @@ package cmd import ( "bytes" "context" + "fmt" "io" "os" "path" @@ -15,6 +16,7 @@ import ( "time" "github.com/open-policy-agent/opa/ast" + "github.com/open-policy-agent/opa/bundle" "github.com/open-policy-agent/opa/rego" "github.com/open-policy-agent/opa/topdown" "github.com/open-policy-agent/opa/util/test" @@ -1012,6 +1014,207 @@ Lines not covered: } } +type loadType int + +const ( + loadFile loadType = iota + loadBundle + loadTarball +) + +func (t loadType) String() string { + return [...]string{"file", "bundle", "bundle tarball"}[t] +} + +func TestWithV1CompatibleFlag(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + files map[string]string + expErr string + }{ + { + note: "0.x module, no imports", + 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: "0.x module, rego.v1 imported", + 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: "0.x module, future.keywords imported", + 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 +}`, + }, + }, + + { + note: "1.0 compatible module, no imports", + 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 +}`, + }, + }, + { + note: "1.0 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: "1.0 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 +}`, + }, + }, + } + + loadTypes := []loadType{loadFile, loadBundle, loadTarball} + + for _, tc := range tests { + for _, loadType := range loadTypes { + t.Run(fmt.Sprintf("%s (%s)", tc.note, loadType), func(t *testing.T) { + var files map[string]string + if loadType != loadTarball { + files = tc.files + } + test.WithTempFS(files, func(root string) { + if loadType == loadTarball { + f, err := os.Create(filepath.Join(root, "bundle.tar.gz")) + if err != nil { + t.Fatal(err) + } + + testBundle := bundle.Bundle{ + Data: map[string]interface{}{}, + } + for k, v := range tc.files { + testBundle.Modules = append(testBundle.Modules, bundle.ModuleFile{ + Path: k, + Raw: []byte(v), + }) + } + + if err := bundle.Write(f, testBundle); err != nil { + t.Fatal(err) + } + } + + var buf bytes.Buffer + var errBuf bytes.Buffer + + testParams := newTestCommandParams() + testParams.v1Compatible = tc.v1Compatible + testParams.bundleMode = loadType == loadBundle + testParams.count = 1 + testParams.output = &buf + testParams.errOutput = &errBuf + + var paths []string + if loadType == loadTarball { + paths = []string{filepath.Join(root, "bundle.tar.gz")} + } else { + paths = []string{root} + } + + exitCode, _ := opaTest(paths, testParams) + if tc.expErr != "" { + if exitCode == 0 { + t.Fatalf("expected non-zero exit code") + } + + if actual := errBuf.String(); !strings.Contains(actual, tc.expErr) { + t.Fatalf("expected error output to contain:\n\n%q\n\nbut got:\n\n%q", tc.expErr, actual) + } + } else { + if exitCode != 0 { + t.Fatalf("unexpected exit code: %d", exitCode) + } + + if errBuf.Len() > 0 { + t.Fatalf("expected no error output but got:\n\n%q", buf.String()) + } + + expected := "PASS: 1/1" + if actual := buf.String(); !strings.Contains(actual, expected) { + t.Fatalf("expected output to contain:\n\n%s\n\nbut got:\n\n%q", expected, actual) + } + } + }) + }) + } + } +} + type blockingWriter struct { m sync.Mutex buf bytes.Buffer diff --git a/compile/compile.go b/compile/compile.go index 798f0e8fcf..5b08504ac5 100644 --- a/compile/compile.go +++ b/compile/compile.go @@ -83,6 +83,7 @@ type Compiler struct { metadata *map[string]interface{} // represents additional data included in .manifest file fsys fs.FS // file system to use when loading paths ns string + regoVersion ast.RegoVersion } // New returns a new compiler instance that can be invoked. @@ -242,6 +243,11 @@ func (c *Compiler) WithPartialNamespace(ns string) *Compiler { return c } +func (c *Compiler) WithRegoVersion(v ast.RegoVersion) *Compiler { + c.regoVersion = v + return c +} + func addEntrypointsFromAnnotations(c *Compiler, ar []*ast.AnnotationsRef) error { for _, ref := range ar { var entrypoint ast.Ref @@ -356,8 +362,14 @@ func (c *Compiler) Build(ctx context.Context) error { c.bundle.Manifest.Metadata = *c.metadata } - if err := c.bundle.FormatModules(false); err != nil { - return err + if c.regoVersion == ast.RegoV1 { + if err := c.bundle.FormatModulesForRegoVersion(c.regoVersion, false, false); err != nil { + return err + } + } else { + if err := c.bundle.FormatModules(false); err != nil { + return err + } } if c.bsc != nil { @@ -432,7 +444,7 @@ func (c *Compiler) initBundle() error { // TODO(tsandall): the metrics object should passed through here so we that // we can track read and parse times. - load, err := initload.LoadPaths(c.paths, c.filter, c.asBundle, c.bvc, false, c.useRegoAnnotationEntrypoints, c.capabilities, c.fsys) + load, err := initload.LoadPathsForRegoVersion(c.regoVersion, c.paths, c.filter, c.asBundle, c.bvc, false, c.useRegoAnnotationEntrypoints, c.capabilities, c.fsys) if err != nil { return fmt.Errorf("load error: %w", err) } diff --git a/docs/content/cli.md b/docs/content/cli.md index 734c649e51..4b8b5e0090 100755 --- a/docs/content/cli.md +++ b/docs/content/cli.md @@ -249,6 +249,7 @@ opa build [ [...]] [flags] --signing-key string set the secret (HMAC) or path of the PEM file containing the private key (RSA and ECDSA) --signing-plugin string name of the plugin to use for signing/verification (see https://www.openpolicyagent.org/docs/latest/management-bundles/#signature-plugin -t, --target {rego,wasm,plan} set the output bundle target type (default rego) + --v1-compatible opt-in to OPA features and behaviors that will be enabled by default in a future OPA v1.0 release --verification-key string set the secret (HMAC) or path of the PEM file containing the public key (RSA and ECDSA) --verification-key-id string name assigned to the verification key used for bundle verification (default "default") ``` @@ -348,6 +349,7 @@ opa check [path [...]] [flags] --rego-v1 check for Rego v1 compatibility (policies must also be compatible with current OPA version) -s, --schema string set schema file path or directory path -S, --strict enable compiler strict mode + --v1-compatible opt-in to OPA features and behaviors that will be enabled by default in a future OPA v1.0 release ``` ____ @@ -560,6 +562,7 @@ opa eval [flags] -t, --target {rego,wasm} set the runtime to exercise (default rego) --timeout duration set eval timeout (default unlimited) -u, --unknowns stringArray set paths to treat as unknown during partial evaluation (default [input]) + --v1-compatible opt-in to OPA features and behaviors that will be enabled by default in a future OPA v1.0 release ``` ____ @@ -646,12 +649,13 @@ opa fmt [path [...]] [flags] ### Options ``` - -d, --diff only display a diff of the changes - --fail non zero exit code on reformat - -h, --help help for fmt - -l, --list list all files who would change when formatted - --rego-v1 format as Rego v1 - -w, --write overwrite the original source file + -d, --diff only display a diff of the changes + --fail non zero exit code on reformat + -h, --help help for fmt + -l, --list list all files who would change when formatted + --rego-v1 format as Rego v1 + --v1-compatible opt-in to OPA features and behaviors that will be enabled by default in a future OPA v1.0 release + -w, --write overwrite the original source file ``` ____ @@ -1102,6 +1106,7 @@ opa test [path [...]] [flags] -t, --target {rego,wasm} set the runtime to exercise (default rego) --threshold float set coverage threshold and exit with non-zero status if coverage is less than threshold % --timeout duration set test timeout (default 5s, 30s when benchmarking) + --v1-compatible opt-in to OPA features and behaviors that will be enabled by default in a future OPA v1.0 release -v, --verbose set verbose reporting mode -w, --watch watch command line files for changes ``` diff --git a/docs/content/opa-1.md b/docs/content/opa-1.md index a1b34708d0..545ff53d8e 100644 --- a/docs/content/opa-1.md +++ b/docs/content/opa-1.md @@ -13,7 +13,9 @@ The full set of breaking changes in OPA v1.0 is not yet finalized. The tools mentioned in the below sections - such as the `rego.v1` import and the `--rego-v1` command flag - may change over time, so it's good practice to regularly employ them to ensure your policies are compatible with the future release of OPA v1.0. {{< /info >}} -## The `future.keywords` imports +## What's changing in OPA 1.0 + +### The `future.keywords` imports The `in`, `every`, `if` and `contains` keywords have been introduced over time, and currently require opt-in to prevent them from breaking policies that existed before their introduction. The `future.keywords` imports facilitate this opt-in mechanism. @@ -23,14 +25,14 @@ There is growing adoption of these keywords and their usage is prevalent in the In OPA v1.0 the `in`, `every`, `if` and `contains` keywords will be part of the language by default and the `future.keywords` imports will become a no-op. A policy that makes use of these keywords, but doesn't import `future.keywords` is valid in OPA v1.0 but not in older versions of OPA. -### Making new and existing policies compatible with OPA v1.0 +#### Making new and existing policies compatible with OPA v1.0 1. A new [rego.v1](../policy-language/#the-regov1-import) import has been introduced that, when used, makes OPA apply all restrictions that will eventually be enforced by default in OPA v1.0. If a Rego module imports `rego.v1`, it means applicable `future.keywords` imports are implied. It is illegal to import both `rego.v1` and `future.keywords` in the same module. 2. The `--rego-v1` flag on the `opa fmt` command will rewrite existing modules to use the `rego.v1` import instead of `future.keywords` imports. 3. The `--rego-v1` flag on the `opa check` command will check that either the `rego.v1` import or applicable `future.keywords` imports are present if any of the `in`, `every`, `if` and `contains` keywords are used in a module. -### Backwards compatibility in OPA v1.0 +#### Backwards compatibility in OPA v1.0 To avoid breaking existing policies and give users a chance to update them such that these future keywords don’t clash with, for example, existing variable names, a new flag called `--v0-compatible` will be added to OPA commands such as `opa run` to maintain backward compatibility. OPA’s Go SDK and Go API will be equipped with similar functionality, as well. @@ -38,7 +40,7 @@ In addition to this, the bundle manifest will also be updated to include a bit t This should be useful for cases when pre-OPA v1.0 functionality is desired but the users themselves have no control over the OPA deployment and hence cannot set the CLI flag. OPA tooling such as the `opa build` command will be equipped with a new flag that will allow users to control the relevant bit in the manifest. -## Enforce use of `if` and `contains` keywords in rule head declarations +### Enforce use of `if` and `contains` keywords in rule head declarations Currently, there is semantic ambiguity between rules like `a.b {true}` and `a.b.c {true}`. Although syntactically similar, the former generates a set with the entry `b` at path `data.a`, while the latter generates an object with the attribute `"c": true` at path `data.a.b`. This inconsistency makes it difficult for new users to understand how Rego works. @@ -132,38 +134,38 @@ When the above rule is evaluated the output is: The requirement of `if` and `contains` keywords will remove the ambiguity between single-value and multi-value rule declaration. This will make Rego code easier to author and read; thereby making it simpler for users to author their policies. -### Making new and existing policies compatible with OPA v1.0 +#### Making new and existing policies compatible with OPA v1.0 1. A new [rego.v1](../policy-language/#the-regov1-import) import has been introduced that, when used, makes OPA apply all restrictions that will eventually be enforced by default in OPA v1.0. If a Rego module imports `rego.v1`, it means the `if` and `contains` keywords are required when declaring rules. Constants, rules that only consist of a value assignment, are exempted. 2. The `--rego-v1` flag on the `opa fmt` command will rewrite existing modules to use the `if` and `contains` keywords where applicable. 3. The `--rego-v1` flag on the `opa check` command will check that the `if` and `contains` keywords are used where applicable in a module. -### Backwards compatibility in OPA v1.0 +#### Backwards compatibility in OPA v1.0 By default, OPA v1.0 will require the usage of `if` and `contains` as described above, but to maintain backward compatibility for policies written pre-OPA v1.0, a new flag called `--v0-compatible` will be added to OPA commands such as `opa run`, to avoid breaking existing policies and give users time to update them. Similar functionality will be added to OPA’s Go SDK and Go API to assist users that embed OPA as a library in their Go services and also to OPA’s build command. -## Prohibit duplicate imports +### Prohibit duplicate imports The Rego compiler supports [strict mode](../policy-language/#strict-mode) which enforces additional constraints and safety checks during compilation. One of these checks is to prohibit duplicate imports where one import shadows another. OPA v1.0 will enforce this check by default. An import shadowing another is most likely an authoring error and probably unintentional. OPA checking this by default will help to avoid policy evaluations resulting in error-prone decisions. -### Making new and existing policies compatible with OPA v1.0 +#### Making new and existing policies compatible with OPA v1.0 1. A new [rego.v1](../policy-language/#the-regov1-import) import has been introduced that, when used, makes OPA apply all restrictions that will eventually be enforced by default in OPA v1.0. If a Rego module imports `rego.v1`, duplicate imports are prohibited. 2. The `--rego-v1` flag on the `opa fmt` command will reject modules with duplicate imports. 3. The `--rego-v1` flag on the `opa check` command will check that duplicate imports are not present in a module. -### Backwards compatibility in OPA v1.0 +#### Backwards compatibility in OPA v1.0 If pre-OPA v1.0 behavior is desired where this check is only enforced when `strict mode` is enabled, a new `--v0-compatible` flag will be added to the OPA CLI to achieve that. Similar functionality will be added to OPA’s Go SDK, Go API and build command. -## `input` and `data` keywords are reserved +### `input` and `data` keywords are reserved The Rego compiler supports [strict mode](../policy-language/#strict-mode), which enforces additional constraints and safety checks during compilation. One of these checks is to ensure that `input` and `data` are reserved keywords and may not be used as names for rules and variable assignments. @@ -173,7 +175,7 @@ Hence, if a rule or variable shadows `input` or `data` you have the unintended c Note, using the [with](../policy-language/#with-keyword) keyword to insert values into - or to fully replace - the `input` or `data` documents, as in `my_func(x) with input as {...}` does not constitute shadowing and is therefore allowed in OPA v1.0. -### Making new and existing policies compatible with OPA v1.0 +#### Making new and existing policies compatible with OPA v1.0 1. A new [rego.v1](../policy-language/#the-regov1-import) import has been introduced that, when used, makes OPA apply all restrictions that will eventually be enforced by default in OPA v1.0. If a Rego module imports `rego.v1`, it means `input` and `data` are reserved keywords and may not be used as names for rules and variable assignments. @@ -181,12 +183,12 @@ Note, using the [with](../policy-language/#with-keyword) keyword to insert value In a future release, a `--refactor-local-variables` flag will be added to `opa fmt` to refactor local variable assignments. 3. The `--rego-v1` flag on the `opa check` command will check that `input` and `data` are not used as names for rules and local variable assignments in a module. -### Backwards compatibility in OPA v1.0 +#### Backwards compatibility in OPA v1.0 OPA v1.0 will enforce this check by default. If pre-OPA v1.0 behavior is desired where this check is only enforced when `strict mode` is enabled, a new flag `--v0-compatible` will be added to the OPA CLI to achieve that. Similar functionality will be added to OPA’s Go SDK, Go API and build command. -## Prohibit use of deprecated builtins +### Prohibit use of deprecated builtins The Rego compiler supports [strict mode](../policy-language/#strict-mode), which enforces additional constraints and safety checks during compilation. One of these checks is to prohibit use of deprecated built-in functions. In OPA v1.0, these built-ins will be removed. @@ -194,7 +196,7 @@ One of these checks is to prohibit use of deprecated built-in functions. In OPA The following built-in functions are deprecated: `any`, `all`, `re_match`, `net.cidr_overlap`, `set_diff`, `cast_array`, `cast_set`, `cast_string`, `cast_boolean`, `cast_null`, `cast_object`. In some cases, new built-in functions have been added that provide functionality at least similar to a deprecated built-in. -### Making new and existing policies compatible with OPA v1.0 +#### Making new and existing policies compatible with OPA v1.0 1. A new [rego.v1](../policy-language/#the-regov1-import) import has been introduced that, when used, makes OPA apply all restrictions that will eventually be enforced by default in OPA v1.0. If a Rego module imports `rego.v1`, it means deprecated built-in functions are prohibited. @@ -203,12 +205,12 @@ In some cases, new built-in functions have been added that provide functionality An FAQ section will be published that explains why each built-in function is deprecated and why it should be removed or replaced in the policy. 3. The `--rego-v1` flag on the `opa check` command will check that deprecated built-in functions are not used in a module. -### Backwards compatibility in OPA v1.0 +#### Backwards compatibility in OPA v1.0 If pre-OPA v1.0 behavior is desired, where this check is only enforced when `strict` mode is enabled, a new flag `--v0-compatible` will be added to the OPA CLI to achieve that. Similar functionality will be added to OPA’s Go SDK, Go API and build command. -## Binding the OPA server to the `localhost` interface by default +### Binding the OPA server to the `localhost` interface by default By default, OPA binds to the `0.0.0.0` interface, which allows the OPA server to be exposed to services running outside of the local machine. Though not inherently insecure in a trusted environment, it's good practice to bind OPA to the `localhost` interface by default if OPA is not intended to be exposed to remote services. @@ -217,3 +219,22 @@ In OPA v1.0, the OPA server will bind to the `localhost` interface by default. The `--v1-compatible` flag on the `opa run` command makes OPA employ configuration defaults that will eventually be used in OPA v1.0. Binding to the `localhost` interface is one such default. + +## Running OPA in 1.0 compatibility mode + +OPA can be run in 1.0 compatibility mode by using the `--v1-compatible` flag. When this mode is enabled, the current release of OPA will behave as OPA v1.0 will eventually behave by default when it comes to the changes described [here](#whats-changing-in-opa-10). + +The `--v1-compatible` flag is currently supported on the following commands: + +* `build`: requires modules to be compatible with OPA v1.0 syntax +* `check`*: requires modules to be compatible with OPA v1.0 syntax +* `fmt`*: formats modules to be compatible with OPA v1.0 syntax, but not the current 0.x syntax. +* `eval`: requires modules to be compatible with OPA v1.0 syntax +* `test`: requires modules to be compatible with OPA v1.0 syntax + +Note: the `check` and `fmt` commands also support the `--rego-v1` flag, which will check/format Rego modules as if compatible with the Rego syntax of _both_ the current 0.x OPA version and OPA v1.0. +If both flags are used at the same time, `--rego-v1` takes precedence over `--v1-compatible`. + +{{< info >}} +Support for more commands will be added over time, leading up to the release of OPA 1.0. +{{< /info >}} diff --git a/format/format.go b/format/format.go index c8c0716b05..0550f18251 100644 --- a/format/format.go +++ b/format/format.go @@ -26,7 +26,18 @@ type Opts struct { // carry along their original source locations. IgnoreLocations bool - RegoV1 bool + // RegoV1 is equivalent to setting RegoVersion to ast.RegoV0Compat1. + // RegoV1 takes precedence over RegoVersion. + // Deprecated: use RegoVersion instead. + RegoV1 bool + RegoVersion ast.RegoVersion +} + +func (o *Opts) effectiveRegoVersion() ast.RegoVersion { + if o.RegoV1 { + return ast.RegoV0CompatV1 + } + return o.RegoVersion } // defaultLocationFile is the file name used in `Ast()` for terms @@ -42,12 +53,19 @@ func Source(filename string, src []byte) ([]byte, error) { } func SourceWithOpts(filename string, src []byte, opts Opts) ([]byte, error) { - module, err := ast.ParseModule(filename, string(src)) + parserOpts := ast.ParserOptions{} + if opts.effectiveRegoVersion() == ast.RegoV1 { + // If the rego version is V1, wee need to parse it as such, to allow for future keywords not being imported. + // Otherwise, we'll default to RegoV0 + parserOpts.RegoVersion = ast.RegoV1 + } + + module, err := ast.ParseModuleWithOpts(filename, string(src), parserOpts) if err != nil { return nil, err } - if opts.RegoV1 { + if opts.effectiveRegoVersion() == ast.RegoV0CompatV1 || opts.effectiveRegoVersion() == ast.RegoV1 { errors := ast.CheckRegoV1(module) if len(errors) > 0 { return nil, errors @@ -115,7 +133,7 @@ func AstWithOpts(x interface{}, opts Opts) ([]byte, error) { o := fmtOpts{} - if opts.RegoV1 { + if opts.effectiveRegoVersion() == ast.RegoV0CompatV1 || opts.effectiveRegoVersion() == ast.RegoV1 { o.regoV1 = true o.ifs = true o.contains = true @@ -176,10 +194,13 @@ func AstWithOpts(x interface{}, opts Opts) ([]byte, error) { switch x := x.(type) { case *ast.Module: - if o.regoV1 { + if opts.effectiveRegoVersion() == ast.RegoV1 { + x.Imports = filterRegoV1Import(x.Imports) + } else if opts.effectiveRegoVersion() == ast.RegoV0CompatV1 { x.Imports = ensureRegoV1Import(x.Imports) } - if o.regoV1 || moduleIsRegoV1Compatible(x) { + + if opts.effectiveRegoVersion() == ast.RegoV0CompatV1 || opts.effectiveRegoVersion() == ast.RegoV1 || moduleIsRegoV1Compatible(x) { x.Imports = future.FilterFutureImports(x.Imports) } else { for kw := range extraFutureKeywordImports { @@ -1448,6 +1469,17 @@ func ensureRegoV1Import(imps []*ast.Import) []*ast.Import { return ensureImport(imps, ast.RegoV1CompatibleRef) } +func filterRegoV1Import(imps []*ast.Import) []*ast.Import { + var ret []*ast.Import + for _, imp := range imps { + path := imp.Path.Value.(ast.Ref) + if !ast.RegoV1CompatibleRef.Equal(path) { + ret = append(ret, imp) + } + } + return ret +} + func ensureImport(imps []*ast.Import, path ast.Ref) []*ast.Import { for _, imp := range imps { p := imp.Path.Value.(ast.Ref) diff --git a/format/format_test.go b/format/format_test.go index dfe28d877f..34f3e8404b 100644 --- a/format/format_test.go +++ b/format/format_test.go @@ -150,7 +150,7 @@ func TestFormatSourceToRegoV1(t *testing.T) { } if errorExpected { - formatted, err := SourceWithOpts(rego, contents, Opts{RegoV1: true}) + formatted, err := SourceWithOpts(rego, contents, Opts{RegoVersion: ast.RegoV0CompatV1}) if err == nil { t.Fatalf("Expected error, got: %s", formatted) } @@ -158,7 +158,7 @@ func TestFormatSourceToRegoV1(t *testing.T) { t.Fatalf("Expected error:\n\n'%s'\n\ngot:\n\n'%s'", expected, err.Error()) } } else { - formatted, err := SourceWithOpts(rego, contents, Opts{RegoV1: true}) + formatted, err := SourceWithOpts(rego, contents, Opts{RegoVersion: ast.RegoV0CompatV1}) if err != nil { t.Fatalf("Failed to format file: %v", err) } @@ -171,7 +171,7 @@ func TestFormatSourceToRegoV1(t *testing.T) { t.Fatalf("Failed to parse formatted bytes: %v", err) } - formatted, err = SourceWithOpts(rego, formatted, Opts{RegoV1: true}) + formatted, err = SourceWithOpts(rego, formatted, Opts{RegoVersion: ast.RegoV0CompatV1}) if err != nil { t.Fatalf("Failed to double format file") } diff --git a/internal/runtime/init/init.go b/internal/runtime/init/init.go index bd72e1bd0c..ba316efcb8 100644 --- a/internal/runtime/init/init.go +++ b/internal/runtime/init/init.go @@ -122,6 +122,18 @@ func LoadPaths(paths []string, processAnnotations bool, caps *ast.Capabilities, fsys fs.FS) (*LoadPathsResult, error) { + return LoadPathsForRegoVersion(ast.RegoV0, paths, filter, asBundle, bvc, skipVerify, processAnnotations, caps, fsys) +} + +func LoadPathsForRegoVersion(regoVersion ast.RegoVersion, + paths []string, + filter loader.Filter, + asBundle bool, + bvc *bundle.VerificationConfig, + skipVerify bool, + processAnnotations bool, + caps *ast.Capabilities, + fsys fs.FS) (*LoadPathsResult, error) { if caps == nil { caps = ast.CapabilitiesForThisVersion() @@ -146,6 +158,7 @@ func LoadPaths(paths []string, WithFilter(filter). WithProcessAnnotation(processAnnotations). WithCapabilities(caps). + WithRegoVersion(regoVersion). AsBundle(path) if err != nil { return nil, err @@ -161,6 +174,7 @@ func LoadPaths(paths []string, WithFS(fsys). WithProcessAnnotation(processAnnotations). WithCapabilities(caps). + WithRegoVersion(regoVersion). Filtered(nonBundlePaths, filter) if err != nil { diff --git a/loader/loader.go b/loader/loader.go index 7b3eac40a5..2960cababb 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -103,7 +103,11 @@ type FileLoader interface { WithCapabilities(*ast.Capabilities) FileLoader WithJSONOptions(*astJSON.Options) FileLoader + // WithRegoV1Compatible + // Deprecated: use WithRegoVersion instead WithRegoV1Compatible(bool) FileLoader + + WithRegoVersion(ast.RegoVersion) FileLoader } // NewFileLoader returns a new FileLoader instance. @@ -183,11 +187,20 @@ func (fl *fileLoader) WithJSONOptions(opts *astJSON.Options) FileLoader { return fl } +// WithRegoV1Compatible enforces Rego v0 with Rego v1 compatibility. +// See ParserOptions.RegoV1Compatible for more details. +// Deprecated: use WithRegoVersion instead func (fl *fileLoader) WithRegoV1Compatible(compatible bool) FileLoader { fl.opts.RegoV1Compatible = compatible return fl } +// WithRegoVersion sets the ast.RegoVersion to use when parsing and compiling modules. +func (fl *fileLoader) WithRegoVersion(version ast.RegoVersion) FileLoader { + fl.opts.RegoVersion = version + return fl +} + // All returns a Result object loaded (recursively) from the specified paths. func (fl fileLoader) All(paths []string) (*Result, error) { return fl.Filtered(paths, nil) @@ -256,7 +269,8 @@ func (fl fileLoader) AsBundle(path string) (*bundle.Bundle, error) { WithSkipBundleVerification(fl.skipVerify). WithProcessAnnotations(fl.opts.ProcessAnnotation). WithCapabilities(fl.opts.Capabilities). - WithJSONOptions(fl.opts.JSONOptions) + WithJSONOptions(fl.opts.JSONOptions). + WithRegoVersion(fl.opts.EffectiveRegoVersion()) // For bundle directories add the full path in front of module file names // to simplify debugging. @@ -706,7 +720,7 @@ func loadKnownTypes(path string, bs []byte, m metrics.Metrics, opts ast.ParserOp return loadYAML(path, bs, m) default: if strings.HasSuffix(path, ".tar.gz") { - r, err := loadBundleFile(path, bs, m) + r, err := loadBundleFile(path, bs, m, opts) if err != nil { err = fmt.Errorf("bundle %s: %w", path, err) } @@ -732,9 +746,15 @@ func loadFileForAnyType(path string, bs []byte, m metrics.Metrics, opts ast.Pars return nil, unrecognizedFile(path) } -func loadBundleFile(path string, bs []byte, m metrics.Metrics) (bundle.Bundle, error) { +func loadBundleFile(path string, bs []byte, m metrics.Metrics, opts ast.ParserOptions) (bundle.Bundle, error) { tl := bundle.NewTarballLoaderWithBaseURL(bytes.NewBuffer(bs), path) - br := bundle.NewCustomReader(tl).WithMetrics(m).WithSkipBundleVerification(true).IncludeManifestInData(true) + br := bundle.NewCustomReader(tl). + WithRegoVersion(opts.RegoVersion). + WithJSONOptions(opts.JSONOptions). + WithProcessAnnotations(opts.ProcessAnnotation). + WithMetrics(m). + WithSkipBundleVerification(true). + IncludeManifestInData(true) return br.Read() } diff --git a/rego/rego.go b/rego/rego.go index 0c6a670099..17873e380a 100644 --- a/rego/rego.go +++ b/rego/rego.go @@ -594,6 +594,7 @@ type Rego struct { pluginMgr *plugins.Manager plugins []TargetPlugin targetPrepState TargetPluginEval + regoVersion ast.RegoVersion } // Function represents a built-in function that is callable in Rego. @@ -1193,6 +1194,12 @@ func Strict(yes bool) func(r *Rego) { } } +func SetRegoVersion(version ast.RegoVersion) func(r *Rego) { + return func(r *Rego) { + r.regoVersion = version + } +} + // New returns a new Rego object. func New(options ...func(r *Rego)) *Rego { @@ -1803,7 +1810,7 @@ func (r *Rego) parseModules(ctx context.Context, txn storage.Transaction, m metr return err } - parsed, err := ast.ParseModule(id, string(bs)) + parsed, err := ast.ParseModuleWithOpts(id, string(bs), ast.ParserOptions{RegoVersion: r.regoVersion}) if err != nil { errs = append(errs, err) } @@ -1813,7 +1820,7 @@ func (r *Rego) parseModules(ctx context.Context, txn storage.Transaction, m metr // Parse any passed in as arguments to the Rego object for _, module := range r.modules { - p, err := module.Parse() + p, err := module.ParseWithOpts(ast.ParserOptions{RegoVersion: r.regoVersion}) if err != nil { switch errorWithType := err.(type) { case ast.Errors: @@ -1845,6 +1852,7 @@ func (r *Rego) loadFiles(ctx context.Context, txn storage.Transaction, m metrics result, err := loader.NewFileLoader(). WithMetrics(m). WithProcessAnnotation(true). + WithRegoVersion(r.regoVersion). Filtered(r.loadPaths.paths, r.loadPaths.filter) if err != nil { return err @@ -1875,6 +1883,7 @@ func (r *Rego) loadBundles(ctx context.Context, txn storage.Transaction, m metri WithMetrics(m). WithProcessAnnotation(true). WithSkipBundleVerification(r.skipBundleVerification). + WithRegoVersion(r.regoVersion). AsBundle(path) if err != nil { return fmt.Errorf("loading error: %s", err) @@ -1940,13 +1949,14 @@ func (r *Rego) compileModules(ctx context.Context, txn storage.Transaction, m me // Use this as the single-point of compiling everything only a // single time. opts := &bundle.ActivateOpts{ - Ctx: ctx, - Store: r.store, - Txn: txn, - Compiler: r.compilerForTxn(ctx, r.store, txn), - Metrics: m, - Bundles: r.bundles, - ExtraModules: r.parsedModules, + Ctx: ctx, + Store: r.store, + Txn: txn, + Compiler: r.compilerForTxn(ctx, r.store, txn), + Metrics: m, + Bundles: r.bundles, + ExtraModules: r.parsedModules, + ParserOptions: ast.ParserOptions{RegoVersion: r.regoVersion}, } err := bundle.Activate(opts) if err != nil { @@ -2614,6 +2624,10 @@ func (m rawModule) Parse() (*ast.Module, error) { return ast.ParseModule(m.filename, m.module) } +func (m rawModule) ParseWithOpts(opts ast.ParserOptions) (*ast.Module, error) { + return ast.ParseModuleWithOpts(m.filename, m.module, opts) +} + type extraStage struct { after string stage ast.QueryCompilerStageDefinition diff --git a/rego/rego_test.go b/rego/rego_test.go index ce67cd30c2..f7a59143f1 100644 --- a/rego/rego_test.go +++ b/rego/rego_test.go @@ -1167,6 +1167,81 @@ func TestPrepareAndPartial(t *testing.T) { } } +func TestPartialWithRegoV1(t *testing.T) { + tests := []struct { + note string + module string + expQuery string + expSupport string + }{ + { + note: "No imports", + module: `package test + p[k] contains v if { + k := "foo" + v := input.v + }`, + expQuery: `data.partial.test.p = x`, + expSupport: `package partial.test.p + +foo[__local1__1] { __local1__1 = input.v }`, + }, + { + note: "rego.v1 imported", + module: `package test + import rego.v1 + p[k] contains v if { + k := "foo" + v := input.v + }`, + expQuery: `data.partial.test.p = x`, + expSupport: `package partial.test.p + +foo[__local1__1] { __local1__1 = input.v }`, + }, + { + note: "future.keywords imported", + module: `package test + import future.keywords + p[k] contains v if { + k := "foo" + v := input.v + }`, + expQuery: `data.partial.test.p = x`, + expSupport: `package partial.test.p + +foo[__local1__1] { __local1__1 = input.v }`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + r := New( + Query("data.test.p = x"), + Module("test.rego", tc.module), + SetRegoVersion(ast.RegoV1), + ) + + ctx := context.Background() + + partialQuery, err := r.Partial(ctx) + if err != nil { + t.Fatal(err) + } + + actualQuery := partialQuery.Queries[0].String() + if tc.expQuery != actualQuery { + t.Fatalf("Expected partial query to be:\n\n%s\n\nbut got:\n\n%s", tc.expQuery, actualQuery) + } + + actualSupport := partialQuery.Support[0].String() + if tc.expSupport != actualSupport { + t.Fatalf("Expected support module to be:\n\n%s\n\nbut got:\n\n%s", tc.expSupport, actualSupport) + } + }) + } +} + func TestPartialNamespace(t *testing.T) { r := New( @@ -1840,6 +1915,189 @@ func TestRegoEvalModulesOnCompiler(t *testing.T) { assertResultSet(t, rs, `[[1]]`) } +func TestRegoEvalWithRegoV1(t *testing.T) { + tests := []struct { + note string + regoVersion ast.RegoVersion + policies map[string]string + query string + expectedResult string + expectedErr string + }{ + { + note: "Rego v0", + policies: map[string]string{ + "policy.rego": `package test + x[y] { y := 1 }`, + }, + expectedResult: `[[{"x": [1]}]]`, + }, + { + note: "Rego v0, forced v1 compatibility", + regoVersion: ast.RegoV0CompatV1, + policies: map[string]string{ + "policy.rego": `package test + import rego.v1 + x contains y if { y := 1 }`, + }, + expectedResult: `[[{"x": [1]}]]`, + }, + { + note: "Rego v0, forced v1 compatibility, invalid rule head", + regoVersion: ast.RegoV0CompatV1, + policies: map[string]string{ + "policy.rego": `package test + import future.keywords.contains + x contains y { y := 1 }`, + }, + expectedErr: "rego_parse_error: `if` keyword is required before rule body", + }, + { + note: "Rego v0, forced v1 compatibility, missing required imports", + regoVersion: ast.RegoV0CompatV1, + policies: map[string]string{ + "policy.rego": `package test + x contains y if { y := 1 }`, + }, + expectedErr: "rego_parse_error: var cannot be used for rule name", // FIXME: Improve error message + }, + { + note: "Rego v1", + regoVersion: ast.RegoV1, + policies: map[string]string{ + "policy.rego": `package test + x contains y if { y := 1 }`, + }, + expectedResult: `[[{"x": [1]}]]`, + }, + { + note: "Rego v1, invalid rule head", + regoVersion: ast.RegoV1, + policies: map[string]string{ + "policy.rego": `package test + x contains y { y := 1 }`, + }, + expectedErr: "rego_parse_error: `if` keyword is required before rule body", + }, + { + note: "Rego v1, multiple files", + regoVersion: ast.RegoV1, + policies: map[string]string{ + "one.rego": `package test + x contains v if { v := 1 }`, + "two.rego": `package test + import rego.v1 + y contains v if { v := 1 }`, + "three.rego": `package test + import future.keywords + z contains v if { v := 1 }`, + }, + expectedResult: `[[{"x": [1], "y": [1], "z": [1]}]]`, + }, + } + + setup := []struct { + name string + options func(path string, policies map[string]string, t *testing.T, ctx context.Context) []func(*Rego) + }{ + { + name: "File", + options: func(path string, _ map[string]string, _ *testing.T, _ context.Context) []func(*Rego) { + return []func(*Rego){ + Load([]string{path}, nil), + } + }, + }, + { + name: "Bundle", + options: func(path string, _ map[string]string, _ *testing.T, _ context.Context) []func(*Rego) { + return []func(*Rego){ + LoadBundle(path), + } + }, + }, + { + name: "Bundle URL", + options: func(path string, _ map[string]string, _ *testing.T, _ context.Context) []func(*Rego) { + return []func(*Rego){ + LoadBundle("file://" + path), + } + }, + }, + { + name: "Store", + options: func(path string, policies map[string]string, t *testing.T, ctx context.Context) []func(*Rego) { + t.Helper() + store := mock.New() + txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams) + + for name, policy := range policies { + err := store.UpsertPolicy(ctx, txn, name, []byte(policy)) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + } + + err := store.Commit(ctx, txn) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + return []func(*Rego){ + // This extra module is required for modules in the store to be parsed + Module("extra.rego", "package extra\np = 1"), + Store(store), + } + }, + }, + } + + for _, s := range setup { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s: %s", s.name, tc.note), func(t *testing.T) { + test.WithTempFS(tc.policies, func(path string) { + ctx := context.Background() + + options := append(s.options(path, tc.policies, t, ctx), + Query("data.test"), + func(r *Rego) { + if tc.regoVersion != ast.RegoV0 { + SetRegoVersion(tc.regoVersion)(r) + } + }, + ) + + pq, err := New( + options..., + ).PrepareForEval(ctx) + + if tc.expectedErr != "" { + if err == nil { + t.Fatal("Expected error, got none") + } + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Fatalf("Expected error:\n\n%s\n\ngot:\n\n%s", err, tc.expectedErr) + } + } else { + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + rs, err := pq.Eval(ctx) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + if tc.expectedResult != "" { + assertResultSet(t, rs, tc.expectedResult) + } + } + }) + }) + } + } +} + func TestRegoLoadFilesWithProvidedStore(t *testing.T) { ctx := context.Background() store := mock.New() @@ -2451,6 +2709,35 @@ func TestPrepareAndCompileWithSchema(t *testing.T) { } } +func TestPrepareAndCompileWithRegoV1(t *testing.T) { + module := `package test +x contains v if { + v := input.y +}` + + r := New( + Query("data.test.x"), + Module("", module), + SetRegoVersion(ast.RegoV1), + ) + + ctx := context.Background() + + pq, err := r.PrepareForEval(ctx) + if err != nil { + t.Fatalf("Unexpected error: %s", err.Error()) + } + + assertPreparedEvalQueryEval(t, pq, []EvalOption{ + EvalInput(map[string]int{"y": 1}), + }, "[[[1]]]") + + _, err = r.Compile(ctx) + if err != nil { + t.Errorf("Unexpected error when compiling: %s", err.Error()) + } +} + func TestGenerateJSON(t *testing.T) { r := New( Query("input"), diff --git a/tester/runner.go b/tester/runner.go index 0092ca627e..b489cea03a 100644 --- a/tester/runner.go +++ b/tester/runner.go @@ -606,7 +606,14 @@ 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.RegoV0) +} + +// LoadWithRegoVersion returns modules and an in-memory store for running tests. +// Modules are parsed in accordance with the given RegoVersion. +func LoadWithRegoVersion(args []string, filter loader.Filter, regoVersion ast.RegoVersion) (map[string]*ast.Module, storage.Store, error) { loaded, err := loader.NewFileLoader(). + WithRegoVersion(regoVersion). WithProcessAnnotation(true). Filtered(args, filter) if err != nil { @@ -634,9 +641,16 @@ func Load(args []string, filter loader.Filter) (map[string]*ast.Module, storage. // LoadBundles will load the given args as bundles, either tarball or directory is OK. func LoadBundles(args []string, filter loader.Filter) (map[string]*bundle.Bundle, error) { + return LoadBundlesWithRegoVersion(args, filter, ast.RegoV0) +} + +// LoadBundlesWithRegoVersion will load the given args as bundles, either tarball or directory is OK. +// Bundles are parsed in accordance with the given RegoVersion. +func LoadBundlesWithRegoVersion(args []string, filter loader.Filter, regoVersion ast.RegoVersion) (map[string]*bundle.Bundle, error) { bundles := map[string]*bundle.Bundle{} for _, bundleDir := range args { b, err := loader.NewFileLoader(). + WithRegoVersion(regoVersion). WithProcessAnnotation(true). WithSkipBundleVerification(true). WithFilter(filter).