Skip to content

Commit

Permalink
Adding --v1-compatible flag to build, opa eval (#6478)
Browse files Browse the repository at this point in the history 
* ast+cmd+rego: Adding `--rego-v1` flag to `opa eval`

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

* Adding `--rego-v1` flag to `opa build`

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

* Formatting PE support modules to comply with rego-v1 when required

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

* Removing rego.v1 import when formatting for rego-v1 (not rego-v0-compat-v1)

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

* touch up

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

* Fixing linting issues

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

* Consolidating `Bundle.FormatModules()` and `Bundle.FormatModulesForRegoVersion()`

Suggested by @ashutosh-narkar

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

* Adding descriptions to `RegoVersion`

Requested by @ashutosh-narkar

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

* Using `--v1-compatible` flag instead of `--rego-v1`

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

* Updating docs

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

* Reintroducing `ParserOptions.RegoV1Compatible` to avoid breaking change

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

* cmd & tester

Adding `--v1-compatible` flag to `opa test`

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

* Adding `--v1-compatible` flag to `opa fmt`

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

* Adding `--v1-compatible` flag to `opa check`

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

* Making linter happy

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

* Review modifications suggested by @ashutosh-narkar

* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description

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

* Review modifications suggested by @ashutosh-narkar

* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
* Adding back `Opts.RegoV1` and deprecating.
  * Making `Opts.RegoV1` take precedence over `Opts.RegoVersion`

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

* Review modifications suggested by @ashutosh-narkar

* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
* Adding back `Opts.RegoV1` and deprecating.
  * Making `Opts.RegoV1` take precedence over `Opts.RegoVersion`
* `TestPartialWitRegoV1` -> `TestPartialWithRegoV1`

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

* Review modifications suggested by @ashutosh-narkar

* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
* Adding back `Opts.RegoV1` and deprecating.
  * Making `Opts.RegoV1` take precedence over `Opts.RegoVersion`
* `TestPartialWitRegoV1` -> `TestPartialWithRegoV1`
* removing `Println` in test

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

* Review modifications suggested by @ashutosh-narkar

* Changing `ParserOptions.RegoV1Compatible` take precedence over `ParserOptions.RegoVersion`
* Fixing comment in test
* Updating `fmt --rego-v1` CLI description
* Adding back `Opts.RegoV1` and deprecating.
  * Making `Opts.RegoV1` take precedence over `Opts.RegoVersion`
* `TestPartialWitRegoV1` -> `TestPartialWithRegoV1`
* removing `Println` in test
* Updating docs with per-command behavioural descriptions for `--v1-compatible`.

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

---------

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
  • Loading branch information
@johanfylling
johanfylling committed Dec 20, 2023
1 parent 84751f0 commit 38c2f0c
Show file tree
Hide file tree
Showing 29 changed files with 1,590 additions and 108 deletions.
6 changes: 3 additions & 3 deletions ast/compile.go
View file
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
Expand Down
88 changes: 70 additions & 18 deletions ast/parser.go
View file
Expand Up @@ -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().
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
},
}
}
}
}
Expand All @@ -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
}
Expand All @@ -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()

Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 7 additions & 6 deletions ast/parser_ext.go
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand All @@ -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")
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
22 changes: 15 additions & 7 deletions ast/policy.go
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand Down
24 changes: 22 additions & 2 deletions bundle/bundle.go
View file
Expand Up @@ -400,6 +400,7 @@ type Reader struct {
lazyLoadingMode bool
name string
persist bool
regoVersion ast.RegoVersion
}

// NewReader is deprecated. Use NewCustomReader instead.
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
22 changes: 12 additions & 10 deletions bundle/store.go
View file
Expand Up @@ -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
}
Expand All @@ -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.
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down

0 comments on commit 38c2f0c

Please sign in to comment.