ewilliams28/releases
1
0
Fork 0
mirror of https://github.com/open-policy-agent/opa.git synced 2026-08-13 03:42:35 -06:00
Code Issues Packages Projects Releases Wiki Activity

format: Produce error when --rego-v1 formatted module has rule name conflicting with keyword (#6867)

Browse Source 
Fixes: #6833
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
This commit is contained in:
Johan Fylling
2024-07-11 10:31:14 +02:00
committed by GitHub
parent 9bd7b5a528
commit e8aa24c262
6 changed files with 125 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ast/policy.go
+41 -1
View File
@@ -100,7 +100,9 @@ var Wildcard = &Term{Value: Var("_")}
var WildcardPrefix = "$"
// Keywords contains strings that map to language keywords.
var Keywords = [...]string{
var Keywords = KeywordsV0
var KeywordsV0 = [...]string{
"not",
"package",
"import",
@@ -114,6 +116,24 @@ var Keywords = [...]string{
"some",
}
var KeywordsV1 = [...]string{
"not",
"package",
"import",
"as",
"default",
"else",
"with",
"null",
"true",
"false",
"some",
"if",
"contains",
"in",
"every",
}
// IsKeyword returns true if s is a language keyword.
func IsKeyword(s string) bool {
for _, x := range Keywords {
@@ -124,6 +144,26 @@ func IsKeyword(s string) bool {
return false
}
// IsKeywordInRegoVersion returns true if s is a language keyword.
func IsKeywordInRegoVersion(s string, regoVersion RegoVersion) bool {
switch regoVersion {
case RegoV0:
for _, x := range KeywordsV0 {
if x == s {
return true
}
}
case RegoV1, RegoV0CompatV1:
for _, x := range KeywordsV1 {
if x == s {
return true
}
}
}
return false
}
type (
// Node represents a node in an AST. Nodes may be statements in a policy module
// or elements of an ad-hoc query, expression, etc.
ast/rego_v1.go
+51 -10
View File
@@ -122,27 +122,65 @@ func checkDeprecatedBuiltinsForCurrentVersion(node interface{}) Errors {
return checkDeprecatedBuiltins(deprecatedBuiltins, node)
}
type RegoCheckOptions struct {
NoDuplicateImports bool
NoRootDocumentOverrides bool
NoDeprecatedBuiltins bool
NoKeywordsAsRuleNames bool
RequireIfKeyword bool
RequireContainsKeyword bool
RequireRuleBodyOrValue bool
}
func NewRegoCheckOptions() RegoCheckOptions {
// all options are enabled by default
return RegoCheckOptions{
NoDuplicateImports: true,
NoRootDocumentOverrides: true,
NoDeprecatedBuiltins: true,
NoKeywordsAsRuleNames: true,
RequireIfKeyword: true,
RequireContainsKeyword: true,
RequireRuleBodyOrValue: true,
}
}
// CheckRegoV1 checks the given module or rule for errors that are specific to Rego v1.
// Passing something other than an *ast.Rule or *ast.Module is considered a programming error, and will cause a panic.
func CheckRegoV1(x interface{}) Errors {
return CheckRegoV1WithOptions(x, NewRegoCheckOptions())
}
func CheckRegoV1WithOptions(x interface{}, opts RegoCheckOptions) Errors {
switch x := x.(type) {
case *Module:
return checkRegoV1Module(x)
return checkRegoV1Module(x, opts)
case *Rule:
return checkRegoV1Rule(x)
return checkRegoV1Rule(x, opts)
}
panic(fmt.Sprintf("cannot check rego-v1 compatibility on type %T", x))
}
func checkRegoV1Module(module *Module) Errors {
func checkRegoV1Module(module *Module, opts RegoCheckOptions) Errors {
var errors Errors
errors = append(errors, checkDuplicateImports([]*Module{module})...)
errors = append(errors, checkRootDocumentOverrides(module)...)
errors = append(errors, checkDeprecatedBuiltinsForCurrentVersion(module)...)
if opts.NoDuplicateImports {
errors = append(errors, checkDuplicateImports([]*Module{module})...)
}
if opts.NoRootDocumentOverrides {
errors = append(errors, checkRootDocumentOverrides(module)...)
}
if opts.NoDeprecatedBuiltins {
errors = append(errors, checkDeprecatedBuiltinsForCurrentVersion(module)...)
}
for _, rule := range module.Rules {
errors = append(errors, checkRegoV1Rule(rule, opts)...)
}
return errors
}
func checkRegoV1Rule(rule *Rule) Errors {
func checkRegoV1Rule(rule *Rule, opts RegoCheckOptions) Errors {
t := "rule"
if rule.isFunction() {
t = "function"
@@ -150,13 +188,16 @@ func checkRegoV1Rule(rule *Rule) Errors {
var errs Errors
if rule.generatedBody && rule.Head.generatedValue {
if opts.NoKeywordsAsRuleNames && IsKeywordInRegoVersion(rule.Head.Name.String(), RegoV1) {
errs = append(errs, NewError(ParseErr, rule.Location, fmt.Sprintf("%s keyword cannot be used for rule name", rule.Head.Name.String())))
}
if opts.RequireRuleBodyOrValue && rule.generatedBody && rule.Head.generatedValue {
errs = append(errs, NewError(ParseErr, rule.Location, "%s must have value assignment and/or body declaration", t))
}
if rule.Body != nil && !rule.generatedBody && !ruleDeclarationHasKeyword(rule, tokens.If) && !rule.Default {
if opts.RequireIfKeyword && rule.Body != nil && !rule.generatedBody && !ruleDeclarationHasKeyword(rule, tokens.If) && !rule.Default {
errs = append(errs, NewError(ParseErr, rule.Location, "`if` keyword is required before %s body", t))
}
if rule.Head.RuleKind() == MultiValue && !ruleDeclarationHasKeyword(rule, tokens.Contains) {
if opts.RequireContainsKeyword && rule.Head.RuleKind() == MultiValue && !ruleDeclarationHasKeyword(rule, tokens.Contains) {
errs = append(errs, NewError(ParseErr, rule.Location, "`contains` keyword is required for partial set rules"))
}
cmd/fmt.go
+13 -2
View File
@@ -27,6 +27,7 @@ type fmtCommandParams struct {
fail bool
regoV1 bool
v1Compatible bool
checkResult bool
}
var fmtParams = fmtCommandParams{}
@@ -126,13 +127,22 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o
return newError("failed to open file: %v", err)
}
opts := format.Opts{}
opts.RegoVersion = params.regoVersion()
opts := format.Opts{
RegoVersion: params.regoVersion(),
}
formatted, err := format.SourceWithOpts(filename, contents, opts)
if err != nil {
return newError("failed to format Rego source file: %v", err)
}
if params.checkResult {
popts := ast.ParserOptions{RegoVersion: params.regoVersion()}
_, err := ast.ParseModuleWithOpts("formatted", string(formatted), popts)
if err != nil {
return newError("%s was successfully formatted, but the result is invalid: %v\n\nTo inspect the formatted Rego, you can turn off this check with --check-result=false.", filename, err)
}
}
changed := !bytes.Equal(contents, formatted)
if params.fail && !params.list && !params.diff {
@@ -229,6 +239,7 @@ func init() {
formatCommand.Flags().BoolVar(&fmtParams.fail, "fail", false, "non zero exit code on reformat")
addRegoV1FlagWithDescription(formatCommand.Flags(), &fmtParams.regoV1, false, "format module(s) to be compatible with both Rego v1 and current OPA version)")
addV1CompatibleFlag(formatCommand.Flags(), &fmtParams.v1Compatible, false)
formatCommand.Flags().BoolVar(&fmtParams.checkResult, "check-result", true, "assert that the formatted code is valid and can be successfully parsed (default true)")
RootCommand.AddCommand(formatCommand)
}
format/format.go
+6 -1
View File
@@ -56,7 +56,12 @@ func SourceWithOpts(filename string, src []byte, opts Opts) ([]byte, error) {
}
if opts.RegoVersion == ast.RegoV0CompatV1 || opts.RegoVersion == ast.RegoV1 {
errors := ast.CheckRegoV1(module)
checkOpts := ast.NewRegoCheckOptions()
// The module is parsed as v0, so we need to disable checks that will be automatically amended by the AstWithOpts call anyways.
checkOpts.RequireIfKeyword = false
checkOpts.RequireContainsKeyword = false
checkOpts.RequireRuleBodyOrValue = false
errors := ast.CheckRegoV1WithOptions(module, checkOpts)
if len(errors) > 0 {
return nil, errors
}
format/testfiles/rego_v1/keywords.rego
+9
View File
@@ -0,0 +1,9 @@
package test
if := 1
contains := 2
in := 3
every := 4
format/testfiles/rego_v1/keywords.rego.error
+5
View File
@@ -0,0 +1,5 @@
4 errors occurred:
testfiles/rego_v1/keywords.rego:3: rego_parse_error: if keyword cannot be used for rule name
testfiles/rego_v1/keywords.rego:5: rego_parse_error: contains keyword cannot be used for rule name
testfiles/rego_v1/keywords.rego:7: rego_parse_error: in keyword cannot be used for rule name
testfiles/rego_v1/keywords.rego:9: rego_parse_error: every keyword cannot be used for rule name