Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding --v1-compatible flag to opa build and opa eval #6478

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
6808e80
ast+cmd+rego: Adding `--rego-v1` flag to `opa eval`
johanfylling Dec 11, 2023
56b1e54
Adding `--rego-v1` flag to `opa build`
johanfylling Dec 11, 2023
45e5fdb
Formatting PE support modules to comply with rego-v1 when required
johanfylling Dec 12, 2023
44e6772
Removing rego.v1 import when formatting for rego-v1 (not rego-v0-comp…
johanfylling Dec 12, 2023
3da495e
touch up
johanfylling Dec 13, 2023
e38eb8a
Fixing linting issues
johanfylling Dec 14, 2023
46db9c2
Consolidating `Bundle.FormatModules()` and `Bundle.FormatModulesForRe…
johanfylling Dec 15, 2023
1b2e632
Adding descriptions to `RegoVersion`
johanfylling Dec 15, 2023
a8e033b
Using `--v1-compatible` flag instead of `--rego-v1`
johanfylling Dec 15, 2023
b811b7b
Updating docs
johanfylling Dec 15, 2023
b9559ad
Reintroducing `ParserOptions.RegoV1Compatible` to avoid breaking change
johanfylling Dec 19, 2023
326e873
cmd & tester
johanfylling Dec 19, 2023
d818bd7
Adding `--v1-compatible` flag to `opa fmt`
johanfylling Dec 19, 2023
fdc4db6
Adding `--v1-compatible` flag to `opa check`
johanfylling Dec 19, 2023
93462a9
Making linter happy
johanfylling Dec 19, 2023
d248201
Review modifications suggested by @ashutosh-narkar
johanfylling Dec 20, 2023
1e63193
Review modifications suggested by @ashutosh-narkar
johanfylling Dec 20, 2023
c5cde1d
Review modifications suggested by @ashutosh-narkar
johanfylling Dec 20, 2023
e9ff76c
Review modifications suggested by @ashutosh-narkar
johanfylling Dec 20, 2023
839fe01
Review modifications suggested by @ashutosh-narkar
johanfylling Dec 20, 2023
e5a5558
Merge branch 'main' into rego-v1/flag_on_all_commands
johanfylling Dec 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions ast/compile.go
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
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
)
johanfylling marked this conversation as resolved.
Show resolved Hide resolved

// 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
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
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
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
johanfylling marked this conversation as resolved.
Show resolved Hide resolved
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
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