From a35a034f0a309002429f18d06b120e8fef610c98 Mon Sep 17 00:00:00 2001 From: Anders Eknert Date: Fri, 2 Jun 2023 09:37:25 +0200 Subject: [PATCH] Various improvements for release (#136) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Revert disabling of `style` rules — most will try this on small libs - Include download/installation instructions in README - Including the "level" in the default report is unnecessary - Add roadmap Signed-off-by: Anders Eknert --- .github/workflows/build.yaml | 2 + README.md | 111 ++++++++++++++++-- bundle/regal/config/provided/data.yaml | 18 +-- .../{rules => bugs}/rule-shadows-builtin.md | 4 +- .../avoid-get-and-list-prefix.md | 4 +- .../external-reference.md | 4 +- .../rules/{comments => style}/todo-comment.md | 4 +- .../unconditional-assignment.md | 4 +- .../use-assignment-operator.md | 4 +- .../print-or-trace-call.md | 4 +- pkg/linter/linter_test.go | 2 +- pkg/reporter/reporter.go | 7 +- pkg/reporter/reporter_test.go | 3 +- 13 files changed, 131 insertions(+), 40 deletions(-) rename docs/rules/{rules => bugs}/rule-shadows-builtin.md (96%) rename docs/rules/{rules => style}/avoid-get-and-list-prefix.md (98%) rename docs/rules/{functions => style}/external-reference.md (97%) rename docs/rules/{comments => style}/todo-comment.md (96%) rename docs/rules/{variables => style}/unconditional-assignment.md (96%) rename docs/rules/{assignment => style}/use-assignment-operator.md (98%) rename docs/rules/{functions => testing}/print-or-trace-call.md (97%) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index db6e2a01..fbe2a15e 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -15,6 +15,8 @@ jobs: with: go-version: 1.20.1 - uses: golangci/golangci-lint-action@v3 + with: + version: v1.52.2 - run: go test ./... - run: go build - run: chmod +x regal diff --git a/README.md b/README.md index 4e51f0c2..cecdc9aa 100644 --- a/README.md +++ b/README.md @@ -25,28 +25,103 @@ using the JSON representation of the Rego abstract syntax tree (AST) as input, a few additional custom built-in functions and some indexed data structures to help with linting. -## Status +## Getting Started + +### Download Regal + +**MacOS and Linux** +```shell +brew install styrainc/packages/regal +``` + +
+ Manual download options + +**MacOS (Apple Silicon)** +```shell +curl -L -o regal "https://github.com/StyraInc/regal/releases/latest/download/regal_Darwin_arm64" +``` + +**MacOS (x86_64)** +```shell +curl -L -o regal "https://github.com/StyraInc/regal/releases/latest/download/regal_Darwin_x86_64" +``` + +**Linux (x86_64)** +```shell +curl -L -o regal "https://github.com/StyraInc/regal/releases/latest/download/regal_Linux_x86_64" +chmod +x regal +``` + +**Windows** +```shell +curl.exe -L -o regal.exe "https://github.com/StyraInc/regal/releases/latest/download/regal_Windows_x86_64.exe" +``` + +See all versions, and checksum files, at the Regal [releases](https://github.com/StyraInc/regal/releases/) page. + +

-Regal is currently in an alpha stage. While we'd like to think of it as helpful already, **every** feature, API, name, -config attribute and paragraph of documentation is subject to change. If you'd still like to use it, we'd love to hear -what you think! +### Try it out! -## Try it out! +First, author some Rego! + +**policy/authz.rego** +```rego +package authz + +import future.keywords + +default deny = true + +deny if { + "admin" != input.user.roles[_] +} +``` -Run `regal lint` pointed at one or more files or directories to have them linted: +Next, run `regal lint` pointed at one or more files or directories to have them linted. ```shell regal lint policy/ ``` +```text +Rule: not-equals-in-loop +Description: Use of != in loop +Category: bugs +Location: p.rego:8:10 +Text: "admin" != input.user.roles[_] +Documentation: https://github.com/StyraInc/regal/blob/main/docs/rules/bugs/not-equals-in-loop.md + +Rule: implicit-future-keywords +Description: Use explicit future keyword imports +Category: imports +Location: p.rego:3:8 +Text: import future.keywords +Documentation: https://github.com/StyraInc/regal/blob/main/docs/rules/imports/implicit-future-keywords.md + +Rule: use-assignment-operator +Description: Prefer := over = for assignment +Category: style +Location: p.rego:5:1 +Text: default deny = true +Documentation: https://github.com/StyraInc/regal/blob/main/docs/rules/style/use-assignment-operator.md + +1 file linted. 3 violations found. +``` + +> **Note** +> If you're running Regal on an existing policy library, you may want to disable the `style` category initially, as it +> will likely generate a lot of violations. You can do this by passing the `--disable-category style` flag to +> `regal lint`. ## Rules Regal comes with a set of built-in rules, grouped by category. -- **bugs**: Common mistakes, potential bugs and inefficiencies in Rego policies. All rules **enabled** by default. -- **imports**: Best practices for imports. All rules **enabled** by default. -- **style**: [Rego Style Guide](https://github.com/StyraInc/rego-style-guide) rules. All rules **disabled** by default. -- **testing**: Rules for testing and development. All rules **enabled** by default. +- **bugs**: Common mistakes, potential bugs and inefficiencies in Rego policies. +- **imports**: Best practices for imports. +- **style**: [Rego Style Guide](https://github.com/StyraInc/rego-style-guide) rules. +- **testing**: Rules for testing and development. The following rules are currently available: @@ -81,6 +156,8 @@ The following rules are currently available: +By default, all rules are currently **enabled**. + If you'd like to see more rules, please [open an issue](https://github.com/StyraInc/regal/issues) for your feature request, or better yet, submit a PR! See the [custom rules](#custom-rules) section for more information on how to develop your own rules, for yourself or for inclusion in Regal. @@ -181,6 +258,20 @@ rules altogether. - [Development](/docs/development) for info about how to hack on Regal itself - [Rego Style Guide](/docs/rego-style-guide) contains notes on implementing the [Rego Style Guide](https://github.com/StyraInc/rego-style-guide) rules +## Status + +Regal is currently in beta. End-users should not expect any drastic changes, but any API may change without notice. + +## Roadmap + +- [ ] More rules! +- [ ] Add `custom` category for built-in "custom", or customizable rules, to enforce things like naming conventions +- [ ] Simplify custom rules authoring by providing command for scaffolding +- [ ] Improvements to assist writing rules that can't be enforced using the AST alone +- [ ] Make more rules consider nesting +- [ ] GitHub Action +- [ ] VS Code extension + ## Community For questions, discussions and announcements related to Styra products, services and open source projects, please join diff --git a/bundle/regal/config/provided/data.yaml b/bundle/regal/config/provided/data.yaml index 08236718..0c689cba 100644 --- a/bundle/regal/config/provided/data.yaml +++ b/bundle/regal/config/provided/data.yaml @@ -23,24 +23,24 @@ rules: level: error style: avoid-get-and-list-prefix: - level: ignore + level: error external-reference: - level: ignore + level: error line-length: - level: ignore + level: error max-line-length: 120 opa-fmt: - level: ignore + level: error prefer-snake-case: - level: ignore + level: error todo-comment: - level: ignore + level: error unconditional-assignment: - level: ignore + level: error use-assignment-operator: - level: ignore + level: error use-in-operator: - level: ignore + level: error testing: file-missing-test-suffix: level: error diff --git a/docs/rules/rules/rule-shadows-builtin.md b/docs/rules/bugs/rule-shadows-builtin.md similarity index 96% rename from docs/rules/rules/rule-shadows-builtin.md rename to docs/rules/bugs/rule-shadows-builtin.md index 1ed4b68a..6a5dd271 100644 --- a/docs/rules/rules/rule-shadows-builtin.md +++ b/docs/rules/bugs/rule-shadows-builtin.md @@ -2,7 +2,7 @@ **Summary**: Rule name shadows built-in -**Category**: Rules +**Category**: Bugs **Avoid** ```rego @@ -26,7 +26,7 @@ This linter rule provides the following configuration options: ```yaml rules: - rules: + bugs: rule-shadows-builtin: # one of "error", "warning", "ignore" level: error diff --git a/docs/rules/rules/avoid-get-and-list-prefix.md b/docs/rules/style/avoid-get-and-list-prefix.md similarity index 98% rename from docs/rules/rules/avoid-get-and-list-prefix.md rename to docs/rules/style/avoid-get-and-list-prefix.md index 2273f15a..09d211d3 100644 --- a/docs/rules/rules/avoid-get-and-list-prefix.md +++ b/docs/rules/style/avoid-get-and-list-prefix.md @@ -2,7 +2,7 @@ **Summary**: Avoid `get_` and `list_` prefix for rules and functions -**Category**: Rules +**Category**: Style **Avoid** ```rego @@ -49,7 +49,7 @@ This linter rule provides the following configuration options: ```yaml rules: - rules: + style: avoid-get-and-list-prefix: # one of "error", "warning", "ignore" level: error diff --git a/docs/rules/functions/external-reference.md b/docs/rules/style/external-reference.md similarity index 97% rename from docs/rules/functions/external-reference.md rename to docs/rules/style/external-reference.md index 162ca750..f592e8a8 100644 --- a/docs/rules/functions/external-reference.md +++ b/docs/rules/style/external-reference.md @@ -2,7 +2,7 @@ **Summary**: Reference to input, data or rule ref in function body -**Category**: Functions +**Category**: Style **Avoid** ```rego @@ -46,7 +46,7 @@ This linter rule provides the following configuration options: ```yaml rules: - functions: + style: external-reference: # one of "error", "warning", "ignore" level: error diff --git a/docs/rules/comments/todo-comment.md b/docs/rules/style/todo-comment.md similarity index 96% rename from docs/rules/comments/todo-comment.md rename to docs/rules/style/todo-comment.md index 84cb9887..60738bd0 100644 --- a/docs/rules/comments/todo-comment.md +++ b/docs/rules/style/todo-comment.md @@ -2,7 +2,7 @@ **Summary**: Avoid TODO Comments -**Category**: Comments +**Category**: Style **Avoid** ```rego @@ -40,7 +40,7 @@ This linter rule provides the following configuration options: ```yaml rules: - comments: + style: todo-comment: # one of "error", "warning", "ignore" level: error diff --git a/docs/rules/variables/unconditional-assignment.md b/docs/rules/style/unconditional-assignment.md similarity index 96% rename from docs/rules/variables/unconditional-assignment.md rename to docs/rules/style/unconditional-assignment.md index 6fcbf13c..3f379602 100644 --- a/docs/rules/variables/unconditional-assignment.md +++ b/docs/rules/style/unconditional-assignment.md @@ -2,7 +2,7 @@ **Summary**: Unconditional assignment in rule body -**Category**: Variables +**Category**: Style **Avoid** ```rego @@ -37,7 +37,7 @@ This linter rule provides the following configuration options: ```yaml rules: - variables: + style: unconditional-assignment: # one of "error", "warning", "ignore" level: error diff --git a/docs/rules/assignment/use-assignment-operator.md b/docs/rules/style/use-assignment-operator.md similarity index 98% rename from docs/rules/assignment/use-assignment-operator.md rename to docs/rules/style/use-assignment-operator.md index 92be4f6f..0d213756 100644 --- a/docs/rules/assignment/use-assignment-operator.md +++ b/docs/rules/style/use-assignment-operator.md @@ -2,7 +2,7 @@ **Summary**: Prefer `:=` over `=` for assignment -**Category**: Assignment +**Category**: Style **Avoid** ```rego @@ -74,7 +74,7 @@ This linter rule provides the following configuration options: ```yaml rules: - assignment: + style: use-assignment-operator: # one of "error", "warning", "ignore" level: error diff --git a/docs/rules/functions/print-or-trace-call.md b/docs/rules/testing/print-or-trace-call.md similarity index 97% rename from docs/rules/functions/print-or-trace-call.md rename to docs/rules/testing/print-or-trace-call.md index 2ea4dbf4..fd28178c 100644 --- a/docs/rules/functions/print-or-trace-call.md +++ b/docs/rules/testing/print-or-trace-call.md @@ -2,7 +2,7 @@ **Summary**: Call to `print` or `trace` function -**Category**: Functions +**Category**: Testing **Avoid** ```rego @@ -35,7 +35,7 @@ This linter rule provides the following configuration options: ```yaml rules: - functions: + testing: print-or-trace-call: # one of "error", "warning", "ignore" level: error diff --git a/pkg/linter/linter_test.go b/pkg/linter/linter_test.go index cd755453..3fedeaa3 100644 --- a/pkg/linter/linter_test.go +++ b/pkg/linter/linter_test.go @@ -71,7 +71,7 @@ func TestLintWithUserConfig(t *testing.T) { policy := `package p foo := input.bar[_] - + or := 1 ` diff --git a/pkg/reporter/reporter.go b/pkg/reporter/reporter.go index 00807d91..4c59c0b8 100644 --- a/pkg/reporter/reporter.go +++ b/pkg/reporter/reporter.go @@ -96,16 +96,15 @@ func buildPrettyViolationsTable(violations []report.Violation) string { red := color.New(color.FgRed).SprintFunc() for i, violation := range violations { - level := red(violation.Level) + description := red(violation.Description) if violation.Level == "warning" { - level = yellow(violation.Level) + description = yellow(violation.Description) } table.AddRow(yellow("Rule:"), violation.Title) - table.AddRow(yellow("Description:"), red(violation.Description)) + table.AddRow(yellow("Description:"), description) table.AddRow(yellow("Category:"), violation.Category) table.AddRow(yellow("Location:"), cyan(violation.Location.String())) - table.AddRow(yellow("Level:"), level) if violation.Location.Text != nil { table.AddRow(yellow("Text:"), strings.TrimSpace(*violation.Location.Text)) diff --git a/pkg/reporter/reporter_test.go b/pkg/reporter/reporter_test.go index 479ae203..58dd7ee8 100644 --- a/pkg/reporter/reporter_test.go +++ b/pkg/reporter/reporter_test.go @@ -71,8 +71,7 @@ func TestPrettyReporterPublish(t *testing.T) { } // TODO(anders): I cannot for the life of me get this to work using a raw string 🫠 - expect := "Rule: \tbreaking-the-law \nDescription: \tRego must not break the law! \nCategory: \tlegal \nLocation: \ta.rego:1:1 \nLevel: \terror \nText: \tpackage illegal \nDocumentation:\thttps://example.com/illegal \n \nRule: \tquestionable-decision \nDescription: \tQuestionable decision found \nCategory: \treally? \nLocation: \tb.rego:22:18 \nLevel: \twarning \nText: \tdefault allow = true \nDocumentation:\thttps://example.com/questionable\n\n3 files linted. 2 violations found in 2 files.\n" //nolint:lll - + expect := "Rule: \tbreaking-the-law \nDescription: \tRego must not break the law! \nCategory: \tlegal \nLocation: \ta.rego:1:1 \nText: \tpackage illegal \nDocumentation:\thttps://example.com/illegal \n \nRule: \tquestionable-decision \nDescription: \tQuestionable decision found \nCategory: \treally? \nLocation: \tb.rego:22:18 \nText: \tdefault allow = true \nDocumentation:\thttps://example.com/questionable\n\n3 files linted. 2 violations found in 2 files.\n" //nolint: lll if buf.String() != expect { t.Errorf("expected %q, got %q", expect, buf.String()) }