Skip to content

Commit

Permalink
Rule: use-rego-v1
Browse files Browse the repository at this point in the history
Recommend using `import rego.v1`.

Also:
* Ignore `import future` rules when `rego.v1` rule in capabilities
* Update the documentation for all rules to use the `rego.v1` import
* Update all Rego used in tests to use `rego.v1` import

Signed-off-by: Anders Eknert <anders@styra.com>
  • Loading branch information
anderseknert committed Feb 5, 2024
1 parent c911cb8 commit 5eabcd0
Show file tree
Hide file tree
Showing 51 changed files with 309 additions and 165 deletions.
36 changes: 19 additions & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,16 @@ First, author some Rego!
```rego
package authz
import future.keywords
import rego.v1
default allow = false
deny if {
"admin" != input.user.roles[_]
allow if {
isEmployee
"developer" in input.user.roles
}
allow if not deny
isEmployee if regex.match("@acmecorp\\.com$", input.user.email)
```

Next, run `regal lint` pointed at one or more files or directories to have them linted.
Expand All @@ -116,19 +117,12 @@ regal lint policy/
<!-- markdownlint-capture -->
<!-- markdownlint-disable MD010 -->
```text
Rule: not-equals-in-loop
Description: Use of != in loop
Category: bugs
Location: policy/authz.rego:8:10
Text: "admin" != input.user.roles[_]
Documentation: https://docs.styra.com/regal/rules/bugs/not-equals-in-loop
Rule: implicit-future-keywords
Description: Use explicit future keyword imports
Category: imports
Location: policy/authz.rego:3:8
Text: import future.keywords
Documentation: https://docs.styra.com/regal/rules/imports/implicit-future-keywords
Rule: non-raw-regex-pattern
Description: Use raw strings for regex patterns
Category: idiomatic
Location: policy/authz.rego:12:27
Text: isEmployee if regex.match("@acmecorp\\.com$", input.user.email)
Documentation: https://docs.styra.com/regal/rules/idiomatic/non-raw-regex-pattern
Rule: use-assignment-operator
Description: Prefer := over = for assignment
Expand All @@ -137,6 +131,13 @@ Location: policy/authz.rego:5:1
Text: default allow = false
Documentation: https://docs.styra.com/regal/rules/style/use-assignment-operator
Rule: prefer-snake-case
Description: Prefer snake_case for names
Category: style
Location: policy/authz.rego:12:1
Text: isEmployee if regex.match("@acmecorp\\.com$", input.user.email)
Documentation: https://docs.styra.com/regal/rules/style/prefer-snake-case
1 file linted. 3 violations found.
```
<!-- markdownlint-restore -->
Expand Down Expand Up @@ -224,6 +225,7 @@ The following rules are currently available:
| imports | [prefer-package-imports](https://docs.styra.com/regal/rules/imports/prefer-package-imports) | Prefer importing packages over rules |
| imports | [redundant-alias](https://docs.styra.com/regal/rules/imports/redundant-alias) | Redundant alias |
| imports | [redundant-data-import](https://docs.styra.com/regal/rules/imports/redundant-data-import) | Redundant import of data |
| imports | [use-rego-v1](https://docs.styra.com/regal/rules/imports/use-rego-v1) | Use `import rego.v1` |
| style | [avoid-get-and-list-prefix](https://docs.styra.com/regal/rules/style/avoid-get-and-list-prefix) | Avoid `get_` and `list_` prefix for rules and functions |
| style | [chained-rule-body](https://docs.styra.com/regal/rules/style/chained-rule-body) | Avoid chaining rule bodies |
| style | [default-over-else](https://docs.styra.com/regal/rules/style/default-over-else) | Prefer default assignment over fallback else |
Expand Down
2 changes: 2 additions & 0 deletions bundle/regal/config/provided/data.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ rules:
level: error
redundant-data-import:
level: error
use-rego-v1:
level: error
style:
avoid-get-and-list-prefix:
level: error
Expand Down
2 changes: 2 additions & 0 deletions bundle/regal/regal.rego
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@
# schemas:
# - input: schema.regal.ast
package regal

import rego.v1
4 changes: 0 additions & 4 deletions bundle/regal/rules/idiomatic/use_if.rego
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ import data.regal.ast
import data.regal.capabilities
import data.regal.result

# Note: think more about what UX we want when import_rego_v1
# capbility is available. Should we simply just recommend that
# and silence this rule in that case? I'm inclined to say yes.

# METADATA
# description: Missing capability for keyword `if`
# custom:
Expand Down
21 changes: 21 additions & 0 deletions bundle/regal/rules/imports/use_rego_v1.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# METADATA
# description: Use `import rego.v1`
package regal.rules.imports["use-rego-v1"]

import rego.v1

import data.regal.ast
import data.regal.capabilities
import data.regal.result

# METADATA
# description: Missing capability for `import rego.v1`
# custom:
# severity: warning
notices contains result.notice(rego.metadata.chain()) if not capabilities.has_rego_v1_feature

report contains violation if {
not ast.imports_has_path(ast.imports, ["rego", "v1"])

violation := result.fail(rego.metadata.chain(), result.location(input["package"]))
}
37 changes: 37 additions & 0 deletions bundle/regal/rules/imports/use_rego_v1_test.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package regal.rules.imports["use-rego-v1_test"]

import rego.v1

import data.regal.capabilities
import data.regal.config
import data.regal.rules.imports["use-rego-v1"] as rule

test_fail_missing_rego_v1_import if {
r := rule.report with input as regal.parse_module("policy.rego", `package policy
import future.keywords
foo if not bar
`)
with data.internal.combined_config as {"capabilities": capabilities.provided}
r == {{
"category": "imports",
"description": "Use `import rego.v1`",
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/use-rego-v1", "imports"),
}],
"title": "use-rego-v1",
"location": {"col": 1, "file": "policy.rego", "row": 1, "text": "package policy"},
"level": "error",
}}
}

test_success_rego_v1_import if {
r := rule.report with input as regal.parse_module("policy.rego", `package policy
import rego.v1
foo if not bar
`)
with data.internal.combined_config as {"capabilities": capabilities.provided}
r == set()
}
13 changes: 5 additions & 8 deletions docs/custom-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ An example policy to implement this requirement might look something like this:
# - input: schema.regal.ast
package custom.regal.rules.naming["acme-corp-package"]
import future.keywords.contains
import future.keywords.if
import rego.v1
import data.regal.result
Expand Down Expand Up @@ -186,25 +185,23 @@ from files, and one that actually lints and reports violations using that data.
# - input: schema.regal.ast
package custom.regal.rules.organizational["at-least-one-allow"]
import future.keywords.contains
import future.keywords.if
import future.keywords.in
import rego.v1
import data.regal.ast
import data.regal.result
aggregate contains entry if {
# ast.rules is input.rules with functions filtered out
some rule in ast.rules
# search for rule named alllow
ast.name(rule) == "allow"
# make sure it's a default assignemnt
# ideally we'll want more than that, but the *requiremnt* is only
# that such a rule exists...
rule["default"] == true
# ...and that it defaults to false
rule.head.value.type == "boolean"
rule.head.value.value == false
Expand Down
4 changes: 2 additions & 2 deletions docs/rules/bugs/constant-condition.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
```rego
package policy
import future.keywords.if
import rego.v1
allow if {
1 == 1
Expand All @@ -32,7 +32,7 @@ harmless, it has no place in production policy, and should be replaced or remove
This linter rule provides the following configuration options:

```yaml
rules:
rules:
bugs:
constant-condition:
# one of "error", "warning", "ignore"
Expand Down
4 changes: 2 additions & 2 deletions docs/rules/bugs/duplicate-rule.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
```rego
package policy
import future.keywords.if
import rego.v1
allow if user.is_admin
Expand All @@ -22,7 +22,7 @@ allow if user.is_admin
```rego
package policy
import future.keywords.if
import rego.v1
allow if user.is_admin
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/bugs/if-empty-object.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
```rego
package policy
import future.keywords.if
import rego.v1
allow if {}
```
Expand Down
11 changes: 4 additions & 7 deletions docs/rules/bugs/inconsistent-args.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
```rego
package policy
import future.keywords.if
import future.keywords.in
import rego.v1
find_vars(rule, node) if node in rule
Expand All @@ -24,8 +23,7 @@ find_vars(node, rule) if {
```rego
package policy
import future.keywords.if
import future.keywords.in
import rego.v1
find_vars(rule, node) if node in rule
Expand All @@ -48,8 +46,7 @@ Using wildcards (`_`) in place of unused arguments is always allowed, and in fac
```rego
package policy
import future.keywords.if
import future.keywords.in
import rego.v1
find_vars(rule, node) if node in rule
Expand All @@ -68,7 +65,7 @@ also allowed.
This linter rule provides the following configuration options:

```yaml
rules:
rules:
bugs:
inconsistent-args:
# one of "error", "warning", "ignore"
Expand Down
7 changes: 3 additions & 4 deletions docs/rules/bugs/not-equals-in-loop.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
```rego
package policy
import future.keywords.if
import rego.v1
deny if {
"admin" != input.user.roles[_]
Expand All @@ -19,8 +19,7 @@ deny if {
```rego
package policy
import future.keywords.if
import future.keywords.in
import rego.v1
deny if {
not "admin" in input.user.roles
Expand All @@ -39,7 +38,7 @@ day. If it doesn't mean "not in", what does it mean?
```rego
package policy
import future.keywords.if
import rego.v1
deny if {
"admin" != input.user.roles[_]
Expand Down
10 changes: 5 additions & 5 deletions docs/rules/bugs/rule-named-if.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ allow := true if {
```rego
package policy
import future.keywords.if
import rego.v1
allow := true if {
authorized
Expand All @@ -26,10 +26,10 @@ allow := true if {

## Rationale

Forgetting to import the `if` keyword (using `import future.keywords.if`) is a common mistake. While this often results
in a parse error, there are some situations where the parser can't tell if the `if` is intended to be used as the
imported keyword, or a new rule named `if`. This is almost always a mistake, and if it isn't — consider using a better
name for your rule!
Forgetting to import the `if` keyword (using `import future.keywords.if`, or from OPA v0.59.0+ `import rego.v1`) is a
common mistake. While this often results in a parse error, there are some situations where the parser can't tell if the
`if` is intended to be used as the imported keyword, or a new rule named `if`. This is almost always a mistake, and if
it isn't — consider using a better name for your rule!

## Configuration Options

Expand Down
4 changes: 2 additions & 2 deletions docs/rules/bugs/unused-return-value.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
```rego
package policy
import future.keywords.if
import rego.v1
allow if {
# return value unused
Expand Down Expand Up @@ -40,7 +40,7 @@ is almost certainly a mistake.
This linter rule provides the following configuration options:

```yaml
rules:
rules:
bugs:
unused-return-value:
# one of "error", "warning", "ignore"
Expand Down
6 changes: 2 additions & 4 deletions docs/rules/community/circular-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ graph LR
# authz.rego
package authz
import future.keywords.if
import future.keywords.in
import rego.v1
import data.shared
Expand Down Expand Up @@ -71,8 +70,7 @@ graph LR
# authz.rego
package authz
import future.keywords.if
import future.keywords.in
import rego.v1
import data.shared
Expand Down
4 changes: 2 additions & 2 deletions docs/rules/community/double-negative.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
```rego
package negative
import future.keywords.if
import rego.v1
fine if not not_fine
Expand All @@ -30,7 +30,7 @@ without_friends if count(input.friends) == 0
```rego
package negative
import future.keywords.if
import rego.v1
fine if input.fine == true
Expand Down
Loading

0 comments on commit 5eabcd0

Please sign in to comment.