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

Rule: no-defined-entrypoint #355

Merged
merged 1 commit into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ The following rules are currently available:
| custom | [naming-convention](https://docs.styra.com/regal/rules/custom/naming-convention) | Naming convention violation |
| idiomatic | [custom-has-key-construct](https://docs.styra.com/regal/rules/idiomatic/custom-has-key-construct) | Custom function may be replaced by `in` and `object.keys` |
| idiomatic | [custom-in-construct](https://docs.styra.com/regal/rules/idiomatic/custom-in-construct) | Custom function may be replaced by `in` keyword |
| idiomatic | [no-defined-entrypoint](https://docs.styra.com/regal/rules/idiomatic/no-defined-entrypoint) | Missing entrypoint annotation |
| idiomatic | [non-raw-regex-pattern](https://docs.styra.com/regal/rules/idiomatic/non-raw-regex-pattern) | Use raw strings for regex patterns |
| idiomatic | [use-in-operator](https://docs.styra.com/regal/rules/idiomatic/use-in-operator) | Use in to check for membership |
| idiomatic | [use-some-for-output-vars](https://docs.styra.com/regal/rules/idiomatic/use-some-for-output-vars) | Use `some` to declare output variables |
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 @@ -25,6 +25,8 @@ rules:
level: error
custom-in-construct:
level: error
no-defined-entrypoint:
level: error
non-raw-regex-pattern:
level: error
use-in-operator:
Expand Down
5 changes: 4 additions & 1 deletion bundle/regal/main.rego
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ lint.aggregates := aggregate

lint_aggregate.violations := aggregate_report

# METADATA
# description: Runs all rules against an input AST and produces a report
# entrypoint: true
report contains violation if {
not is_object(input)

Expand Down Expand Up @@ -95,7 +98,7 @@ aggregate_report contains violation if {

key := concat("/", [category, title])
input_for_rule := object.remove(
object.union(input, {"aggregate": input.aggregates_internal[key]}),
object.union(input, {"aggregate": object.get(input, ["aggregates_internal", key], [])}),
["aggregates_internal"],
)

Expand Down
26 changes: 26 additions & 0 deletions bundle/regal/rules/idiomatic/no_defined_entrypoint.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# METADATA
# description: Missing entrypoint annotation
package regal.rules.idiomatic["no-defined-entrypoint"]

import future.keywords.contains
import future.keywords.if
import future.keywords.in

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

aggregate contains entry if {
some annotation in input.annotations
annotation.entrypoint == true

entry := result.aggregate(rego.metadata.chain(), {"entrypoint": annotation.location})
}

# METADATA
# schemas:
# - input: schema.regal.aggregate
aggregate_report contains violation if {
count(input.aggregate) == 0

violation := result.fail(rego.metadata.chain(), {})
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These rules are just so simple! @sesponda note how input aggregate always is defined and at minimum an empty set/array, like how you wanted it to be. This is possible now since we're not even evaluating these rules if aggregate rules are disabled (because only one file linted or for other reasons).

73 changes: 73 additions & 0 deletions bundle/regal/rules/idiomatic/no_defined_entrypoint_test.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package regal.rules.idiomatic["no-defined-entrypoint_test"]

import future.keywords.if
import future.keywords.in

import data.regal.ast
import data.regal.config

import data.regal.rules.idiomatic["no-defined-entrypoint"] as rule

test_aggregate_entrypoints if {
module := regal.parse_module("policy.rego", `
# METADATA
# entrypoint: true
package p

# METADATA
# entrypoint: true
allow := false`)

aggregate := rule.aggregate with input as module
aggregate == {
{
"aggregate_data": {"entrypoint": {"col": 1, "file": "policy.rego", "row": 2, "text": "IyBNRVRBREFUQQ=="}},
"aggregate_source": {"file": "policy.rego", "package_path": ["p"]},
"rule": {"category": "idiomatic", "title": "no-defined-entrypoint"},
},
{
"aggregate_data": {"entrypoint": {"col": 1, "file": "policy.rego", "row": 6, "text": "IyBNRVRBREFUQQ=="}},
"aggregate_source": {"file": "policy.rego", "package_path": ["p"]},
"rule": {"category": "idiomatic", "title": "no-defined-entrypoint"},
},
}
}

test_fail_no_entrypoint_defined if {
r := rule.aggregate_report with input as {"aggregate": set()}
r == {{
"category": "idiomatic",
"description": "Missing entrypoint annotation",
"level": "error",
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/no-defined-entrypoint", "idiomatic"),
}],
"title": "no-defined-entrypoint",
}}
}

test_success_single_entrypoint_defined if {
r := rule.aggregate_report with input as {"aggregate": [{
"aggregate_data": {"entrypoint": {"col": 1, "file": "policy.rego", "row": 2}},
"aggregate_source": {"file": "policy.rego", "package_path": ["p"]},
"rule": {"category": "idiomatic", "title": "no-defined-entrypoint"},
}]}
r == set()
}

test_success_multiple_entrypoints_defined if {
r := rule.aggregate_report with input as {"aggregate": [
{
"aggregate_data": {"entrypoint": {"col": 1, "file": "policy.rego", "row": 2}},
"aggregate_source": {"file": "policy.rego", "package_path": ["p"]},
"rule": {"category": "idiomatic", "title": "no-defined-entrypoint"},
},
{
"aggregate_data": {"entrypoint": {"col": 1, "file": "policy.rego", "row": 6}},
"aggregate_source": {"file": "policy.rego", "package_path": ["p"]},
"rule": {"category": "idiomatic", "title": "no-defined-entrypoint"},
},
]}
r == set()
}
99 changes: 99 additions & 0 deletions docs/rules/idiomatic/no-defined-entrypoint.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# no-defined-entrypoint

**Summary**: Missing entrypoint annotation

**Category**: Idiomatic

**Type**: Aggregate - only runs when more than one file is provided for linting

**Avoid**
```rego
package policy

import future.keywords.if
import future.keywords.in

default allow := false

# Nothing wrong with this rule, but an
# entrypoint should be documented as such
allow if user_is_admin
allof if public_resource_read

user_is_admin if {
some role in input.user.roles
role in data.permissions.admin_roles
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would these examples be clearer if the unnamed entrypoint allow showed how entrypoints often gather the results from various other rules?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.. I agree that'd be good! I've wanted to make all examples self-contained, so that they'd be copy-pasteable... but might be worth expanding a bit on this example and have a helper rule or two. I'll take a look!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to include two helper rules :)


public_resource_read if {
input.request.method == "GET"
input.request.path[0] == "public"
}
```

**Prefer**
```rego
package policy

import future.keywords.if
import future.keywords.in

default allow := false

# METADATA
# description: Allow only admins, or reading public resources
# entrypoint: true
allow if user_is_admin
allof if public_resource_read

user_is_admin if {
some role in input.user.roles
role in data.permissions.admin_roles
}

public_resource_read if {
input.request.method == "GET"
input.request.path[0] == "public"
}
```

## Rationale

Defining one or more entrypoints for your policies is a good practice to follow. An entrypoint is simply a package or
rule that is meant to be queried for decisions from the outside. While it might seem obvious to the policy author which
rules are meant to be queried, adding an extra line of two of metadata will help make it obvious to others.

Marking a package or rule via an
[entrypoint annotation attribute](https://www.openpolicyagent.org/docs/latest/policy-language/#entrypoint) not only
provides good documentation for others, but also unlocks programmatic possibilities, like:

1. Your policy library may be compiled to WebAssembly without extra entrypoint arguments
1. Your policy library may be compiled to an
[intermediate representation](https://blog.openpolicyagent.org/i-have-a-plan-exploring-the-opa-intermediate-representation-ir-format-7319cd94b37d)
(IR) format without extra entrypoint arguments
1. External applications may present your entrypoints as part of rendered documentation
1. External applications may use your entrypoints to know what to evaluate
1. External applications — like Regal — may use this information to determine what other rules are used or not

## Configuration Options

This linter rule provides the following configuration options:

```yaml
rules:
idiomatic:
no-defined-entrypoint:
# one of "error", "warning", "ignore"
level: error
```

## Related Resources

- OPA Docs [Metadata](https://www.openpolicyagent.org/docs/latest/policy-language/#metadata)
- OPA Docs [Entrypoint](https://www.openpolicyagent.org/docs/latest/policy-language/#entrypoint)

## Community

If you think you've found a problem with this rule or its documentation, would like to suggest improvements, new rules,
or just talk about Regal in general, please join us in the `#regal` channel in the Styra Community
[Slack](https://communityinviter.com/apps/styracommunity/signup)!
2 changes: 2 additions & 0 deletions e2e/testdata/aggregates/three_policies/policy_1.rego
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
package mypolicy1.public

# METADATA
# entrypoint: true
my_policy_1 := true
2 changes: 2 additions & 0 deletions e2e/testdata/aggregates/two_policies/policy_1.rego
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
package mypolicy1.public

# METADATA
# entrypoint: true
my_policy_1 := true
Loading