diff --git a/docs/Makefile b/docs/Makefile index d151576140..9545b55ed0 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -8,7 +8,10 @@ ci: .PHONY: dev dev: - npx docusaurus start + # --no-open means that the browser will not be opened on start. + # This is done to avoid opening many tabs repeatedly when editing + # docusaurus.config.js. + npx docusaurus start --no-open .PHONY: build build: diff --git a/docs/bin/import-regal-docs.sh b/docs/bin/import-regal-docs.sh new file mode 100755 index 0000000000..08964e68a8 --- /dev/null +++ b/docs/bin/import-regal-docs.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash + +set -euo pipefail + +if ! command -v curl >/dev/null 2>&1 +then + echo "curl could not be found" + exit 1 +fi + +if ! command -v unzip >/dev/null 2>&1 +then + echo "unzip could not be found" + exit 1 +fi + +download_regal() { + ref="heads/main" + if [[ -v VERSION ]]; then + ref="tags/$VERSION"; + fi + + # examples + # https://github.com/open-policy-agent/regal/archive/refs/heads/main.zip + # https://github.com/open-policy-agent/regal/archive/refs/tags/v0.35.1.zip + url="https://github.com/open-policy-agent/regal/archive/refs/$ref.zip" + + curl --silent -L -o regal.zip "$url" +} + +if [[ ! -e regal.zip ]]; then + download_regal +else + echo "Using existing regal.zip" +fi + +tempdir=$(mktemp -d) + +unzip regal.zip -d "$tempdir" 2>&1 > /dev/null + +mv $tempdir/*/* $tempdir + +regal_docs_src="$tempdir/docs" +regal_docs_dest="projects/regal" + +rm -rf "$regal_docs_dest" +mkdir -p "$regal_docs_dest" + +# copy assets +rsync -ah "$regal_docs_src/assets/." "$regal_docs_dest/assets" --delete + +# generate index +readme_sections_dir="$regal_docs_src/readme-sections" +manifest="$readme_sections_dir/website-manifest" + +tmpfile=$(mktemp) + +while IFS= read -r file; do + section_path="$readme_sections_dir/$file" + + if [[ -f "$section_path" ]]; then + cat "$section_path" >> "$tmpfile" + echo -e "\n" >> "$tmpfile" + else + echo "Section file not found: $section_path" >&2 + exit 1 + fi +done < "$manifest" + +mv $tmpfile "$regal_docs_dest/index.md" + +# copy in rules +cp -r "$regal_docs_src/rules" "$regal_docs_dest/" + +# generate other files +find "$regal_docs_src" -type f -name '*.md.yaml' | while read -r yaml_file; do + md_file="$(dirname $yaml_file)/$(basename "$yaml_file" .yaml)" + md_file_rel=${md_file#"$regal_docs_src/"} + dest_md_file="$regal_docs_dest/$md_file_rel" + + mkdir -p "$(dirname $dest_md_file)" + + if [[ ! -e $md_file ]]; then + echo "Warning: $md_file missing" + else + echo -e "---\n$(cat $yaml_file)\n---\n\n" > $dest_md_file + cat $md_file >> $dest_md_file + fi +done diff --git a/docs/docusaurus.config.js b/docs/docusaurus.config.js index ef31bdc34b..966b0fb53d 100644 --- a/docs/docusaurus.config.js +++ b/docs/docusaurus.config.js @@ -6,6 +6,7 @@ import fs from "fs/promises"; const path = require("path"); const { loadPages } = require("./src/lib/ecosystem/loadPages"); +const { loadRules } = require("./src/lib/projects/regal/loadRules"); const baseUrl = "/"; @@ -136,13 +137,28 @@ const baseUrl = "/"; { href: "https://blog.openpolicyagent.org/", label: "Blog" }, ], }, + { + type: "dropdown", + label: "Projects", + position: "right", + items: [ + { to: "/docs", label: "OPA" }, + { to: "/projects/regal", label: "Regal" }, + { + type: "html", + value: "
", + }, + { href: "https://open-policy-agent.github.io/gatekeeper/website/", label: "OPA Gatekeeper" }, + { href: "https://www.conftest.dev", label: "Conftest" }, + ], + }, { to: "/ecosystem/", label: "Ecosystem", position: "right" }, { href: "https://play.openpolicyagent.org/", label: "Play", position: "right" }, { type: "html", position: "right", value: ` - @@ -267,6 +283,15 @@ The Linux Foundation has registered trademarks and uses trademarks. For a list o }, plugins: [ + [ + "@docusaurus/plugin-content-docs", + { + id: "regal", + path: "projects/regal", + routeBasePath: "projects/regal", + sidebarPath: require.resolve("./src/lib/sidebar-regal.js"), + }, + ], [ require.resolve("@easyops-cn/docusaurus-search-local"), { @@ -489,6 +514,22 @@ The Linux Foundation has registered trademarks and uses trademarks. For a list o }, }; }, + + async function ecosystemData(context, options) { + return { + name: "regal", + + async loadContent() { + const rules = await loadRules(); + + return { rules }; + }, + + async contentLoaded({ content, actions }) { + await actions.createData("rules.json", JSON.stringify(content.rules, null, 2)); + }, + }; + }, ], clientModules: [ require.resolve("./src/lib/playground.js"), diff --git a/docs/package-lock.json b/docs/package-lock.json index 2e9de0acee..312cddf50d 100644 --- a/docs/package-lock.json +++ b/docs/package-lock.json @@ -10,6 +10,7 @@ "license": "ISC", "dependencies": { "@docusaurus/core": "^3.8.1", + "@docusaurus/plugin-content-docs": "^3.8.1", "@docusaurus/plugin-google-gtag": "^3.8.1", "@docusaurus/preset-classic": "^3.8.1", "@docusaurus/theme-mermaid": "^3.8.1", diff --git a/docs/package.json b/docs/package.json index 38fae8dc83..3324a64a7a 100644 --- a/docs/package.json +++ b/docs/package.json @@ -11,6 +11,7 @@ "description": "", "dependencies": { "@docusaurus/core": "^3.8.1", + "@docusaurus/plugin-content-docs": "^3.8.1", "@docusaurus/plugin-google-gtag": "^3.8.1", "@docusaurus/preset-classic": "^3.8.1", "@docusaurus/theme-mermaid": "^3.8.1", diff --git a/docs/projects/regal/adopters.md b/docs/projects/regal/adopters.md new file mode 100644 index 0000000000..1a544dee95 --- /dev/null +++ b/docs/projects/regal/adopters.md @@ -0,0 +1,83 @@ +--- +sidebar_position: 13 +--- + + +# Adopters + +We're happy to be trusted by the following projects and organizations. + +If you're using Regal, please consider opening a pull request to add your project or organization to this list! + +## Open Source Projects + +Public open source projects integrating Regal for linting in their CI/CD pipelines. + + +- [Brainiac](https://github.com/carbonetes/brainiac) +- [Cloudbeat](https://github.com/elastic/cloudbeat) +- [Conftest](https://github.com/open-policy-agent/conftest) +- [CVAT](https://github.com/opencv/cvat) +- [GKE Policy Automation](https://github.com/google/gke-policy-automation) +- [Konstraint](https://github.com/plexsystems/konstraint) +- [Kubescape](https://github.com/kubescape/regolibrary) +- [Legitify](https://github.com/Legit-Labs/legitify) +- [Matrix Authentication Service](https://github.com/element-hq/matrix-authentication-service/) +- [Minder](https://github.com/stacklok/minder) +- [Modernisation Platform](https://github.com/ministryofjustice/modernisation-platform) +- [OPA Library](https://github.com/open-policy-agent/library) +- [Red Hat COP](https://github.com/redhat-cop/rego-policies) +- [ScubaGear](https://github.com/cisagov/ScubaGear) +- [ScubaGoggles](https://github.com/cisagov/ScubaGoggles) +- [Spacelift Policy Library](https://github.com/spacelift-io/spacelift-policies-example-library) +- [Trino Operator](https://github.com/stackabletech/trino-operator) +- [Trivy](https://github.com/aquasecurity/trivy-checks) +- [Phylum](https://github.com/phylum-dev/policy) + + +## Integrations + +Projects and products that integrate Regal into their offerings. + + +- [Dependency Management Data](https://gitlab.com/tanna.dev/dependency-management-data) +- [Enterprise OPA](https://github.com/styrainc/enterprise-opa) +- [The Rego Playground](https://play.openpolicyagent.org) +- [Trunk Check](https://trunk.io/check) +- [reviewdog/action-regal](https://github.com/reviewdog/action-regal) + + +## Packaging + +The following package managers include Regal in their repositories, either natively or via plugins. + +- [Homebrew](https://brew.sh/) via the [regal](https://formulae.brew.sh/formula/regal) formula +- [asdf](https://asdf-vm.com/) via [asdf-regal](https://github.com/asdf-community/asdf-regal) +- [mise](https://mise.jdx.dev/) via its [aqua](https://aquaproj.github.io/) backend and [aqua's regal definition](https://github.com/aquaproj/aqua-registry/tree/main/pkgs/StyraInc/regal) +- [pkgsrc](https://www.pkgsrc.se/) and the [regal](https://pkgsrc.se/devel/regal) package +- [Nix](https://nixos.org/): [regal](https://search.nixos.org/packages?channel=24.05&show=regal&from=0&size=50&sort=relevance&type=packages&query=regal) +- [mason.vim](https://github.com/williamboman/mason.nvim): [regal](https://github.com/mason-org/mason-registry/blob/main/packages/regal/package.yaml) + +## Companies and Organizations + +Some companies and organizations using Regal. + + +- [ARMO](https://www.armosec.io) +- [Aqua Security](https://www.aquasec.com) +- [Atlassian](https://www.atlassian.com) +- [Bankdata](https://www.bankdata.dk) +- [CISA](https://www.cisa.gov) +- [Elastic](https://www.elastic.co) +- [Google](https://www.google.com) +- [Microsoft](https://www.microsoft.com) +- [Ministry of Justice](https://www.gov.uk/government/organisations/ministry-of-justice) +- [Miro](https://miro.com) +- [OpenCV](https://opencv.org) +- [Red Hat](https://www.redhat.com) +- [Spacelift](https://www.spacelift.io) +- [Stacklok](https://stacklok.com) +- [Styra](https://www.styra.com) +- [UNIwise](https://uniwise.eu/) +- [VodafoneZiggo](https://www.vodafoneziggo.nl) + diff --git a/docs/projects/regal/architecture.md b/docs/projects/regal/architecture.md new file mode 100644 index 0000000000..d90ab50159 --- /dev/null +++ b/docs/projects/regal/architecture.md @@ -0,0 +1,47 @@ +--- +sidebar_position: 4 +--- + + +# Architecture + +Or "How does Regal work?" + +As you might have [read](https://www.styra.com/blog/guarding-the-guardrails-introducing-regal-the-rego-linter/), Regal +[uses Rego for linting Rego](https://www.styra.com/blog/linting-rego-with-rego/) — or rather, Rego policies turned into +a JSON representation of their abstract syntax tree (AST). + +## High-level Overview + +When running Regal against a directory, like `regal lint my-policies/`, Regal does the following: + +- For each source file provided for linting, Regal parses the Rego into its AST representation. This AST representation + is then turned into JSON and provided as the **input** variable to the linter rules. +- Each linter rule (and there are almost 40 of them at the time of writing this) uses the **input**, which contains + information such as the package name, what imports are used, and all the rules and the expressions they contain, to + determine whether the Rego policy linted contains any violations against the rule. An example could be a rule that + [forbids shadowing](https://openpolicyagent.org/projects/regal/rules/bugs/rule-shadows-builtin) (i.e. using the same name as) + built-in functions and operators. +- Since rule bodies aren’t necessarily flat, but may contain nested bodies of constructs such as + [comprehensions](https://www.openpolicyagent.org/docs/policy-language/#comprehensions) or + [every](https://www.openpolicyagent.org/docs/policy-language/#every-keyword) blocks, many linter rules need to + traverse all expressions in order to find what they are looking for. This is normally done with the help of the + built-in [walk](https://www.openpolicyagent.org/docs/policy-reference/#graph) function. +- Traversing huge AST structures — and some policies contain millions of AST nodes! — takes time. This isn’t noticeable + when linting a single file, but for some of the largest policy repositories out there, with several thousands of + policy files and tests, the cost may be prohibitive. To alleviate this, Regal is implemented to process files + concurrently to minimize the impact of IO bound tasks, and to make use of multiple cores when available. +- The result of linting each file is eventually collected and compiled into a linter report, which is presented to the + user in one of the available [output formats](https://openpolicyagent.org/projects/regal#output-formats). + +## Rego Rules Evaluation + +The main entrypoint for Rego rule evaluation is unsurprisingly found in +[main.rego](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/main.rego), in which we query the `report` +rule from the [Go](https://github.com/open-policy-agent/regal/blob/main/pkg/linter/linter.go) application. + +The `report` rule in turn uses +[dynamic policy composition](https://www.styra.com/blog/dynamic-policy-composition-for-opa/) to query all rules named +`report` under `data.regal.rules[category][title]` for built-in rules, and `data.custom.regal.rules[category][title]` +for custom rules. The violations reported from each rule is added to the `report` set and sent back to the application, +which will compile a final report and present it to the user. diff --git a/docs/projects/regal/assets/dap/animation.gif b/docs/projects/regal/assets/dap/animation.gif new file mode 100644 index 0000000000..d89126898b Binary files /dev/null and b/docs/projects/regal/assets/dap/animation.gif differ diff --git a/docs/projects/regal/assets/dap/breakpoint.png b/docs/projects/regal/assets/dap/breakpoint.png new file mode 100644 index 0000000000..d360b6fb51 Binary files /dev/null and b/docs/projects/regal/assets/dap/breakpoint.png differ diff --git a/docs/projects/regal/assets/dap/codeaction.png b/docs/projects/regal/assets/dap/codeaction.png new file mode 100644 index 0000000000..5b815b9207 Binary files /dev/null and b/docs/projects/regal/assets/dap/codeaction.png differ diff --git a/docs/projects/regal/assets/dap/print.png b/docs/projects/regal/assets/dap/print.png new file mode 100644 index 0000000000..ec402c35a0 Binary files /dev/null and b/docs/projects/regal/assets/dap/print.png differ diff --git a/docs/projects/regal/assets/dap/variables.png b/docs/projects/regal/assets/dap/variables.png new file mode 100644 index 0000000000..1597d38fd8 Binary files /dev/null and b/docs/projects/regal/assets/dap/variables.png differ diff --git a/docs/projects/regal/assets/editors-neovim.png b/docs/projects/regal/assets/editors-neovim.png new file mode 100755 index 0000000000..9d84fcba08 Binary files /dev/null and b/docs/projects/regal/assets/editors-neovim.png differ diff --git a/docs/projects/regal/assets/evalcustom.png b/docs/projects/regal/assets/evalcustom.png new file mode 100644 index 0000000000..ab155d1ee8 Binary files /dev/null and b/docs/projects/regal/assets/evalcustom.png differ diff --git a/docs/projects/regal/assets/lsp/code_action_fix.png b/docs/projects/regal/assets/lsp/code_action_fix.png new file mode 100644 index 0000000000..c36940193b Binary files /dev/null and b/docs/projects/regal/assets/lsp/code_action_fix.png differ diff --git a/docs/projects/regal/assets/lsp/code_action_show.png b/docs/projects/regal/assets/lsp/code_action_show.png new file mode 100644 index 0000000000..9942fe6234 Binary files /dev/null and b/docs/projects/regal/assets/lsp/code_action_show.png differ diff --git a/docs/projects/regal/assets/lsp/codeaction.png b/docs/projects/regal/assets/lsp/codeaction.png new file mode 100644 index 0000000000..eb92aba844 Binary files /dev/null and b/docs/projects/regal/assets/lsp/codeaction.png differ diff --git a/docs/projects/regal/assets/lsp/completion.png b/docs/projects/regal/assets/lsp/completion.png new file mode 100644 index 0000000000..0412146732 Binary files /dev/null and b/docs/projects/regal/assets/lsp/completion.png differ diff --git a/docs/projects/regal/assets/lsp/diagnostics.png b/docs/projects/regal/assets/lsp/diagnostics.png new file mode 100644 index 0000000000..9e010024d3 Binary files /dev/null and b/docs/projects/regal/assets/lsp/diagnostics.png differ diff --git a/docs/projects/regal/assets/lsp/documentsymbols.png b/docs/projects/regal/assets/lsp/documentsymbols.png new file mode 100644 index 0000000000..9e13feb833 Binary files /dev/null and b/docs/projects/regal/assets/lsp/documentsymbols.png differ diff --git a/docs/projects/regal/assets/lsp/documentsymbols2.png b/docs/projects/regal/assets/lsp/documentsymbols2.png new file mode 100644 index 0000000000..3ac9993f02 Binary files /dev/null and b/docs/projects/regal/assets/lsp/documentsymbols2.png differ diff --git a/docs/projects/regal/assets/lsp/eval_use_as_input.png b/docs/projects/regal/assets/lsp/eval_use_as_input.png new file mode 100644 index 0000000000..0600c2c0dd Binary files /dev/null and b/docs/projects/regal/assets/lsp/eval_use_as_input.png differ diff --git a/docs/projects/regal/assets/lsp/evalcodelens.png b/docs/projects/regal/assets/lsp/evalcodelens.png new file mode 100644 index 0000000000..d47c9fac59 Binary files /dev/null and b/docs/projects/regal/assets/lsp/evalcodelens.png differ diff --git a/docs/projects/regal/assets/lsp/evalcodelensprint.png b/docs/projects/regal/assets/lsp/evalcodelensprint.png new file mode 100644 index 0000000000..b602831397 Binary files /dev/null and b/docs/projects/regal/assets/lsp/evalcodelensprint.png differ diff --git a/docs/projects/regal/assets/lsp/folding.png b/docs/projects/regal/assets/lsp/folding.png new file mode 100644 index 0000000000..ea46a9cb66 Binary files /dev/null and b/docs/projects/regal/assets/lsp/folding.png differ diff --git a/docs/projects/regal/assets/lsp/format.png b/docs/projects/regal/assets/lsp/format.png new file mode 100644 index 0000000000..b3f2bb1d46 Binary files /dev/null and b/docs/projects/regal/assets/lsp/format.png differ diff --git a/docs/projects/regal/assets/lsp/hover.png b/docs/projects/regal/assets/lsp/hover.png new file mode 100644 index 0000000000..f309301b40 Binary files /dev/null and b/docs/projects/regal/assets/lsp/hover.png differ diff --git a/docs/projects/regal/assets/lsp/inlay.png b/docs/projects/regal/assets/lsp/inlay.png new file mode 100644 index 0000000000..19b79d5ecd Binary files /dev/null and b/docs/projects/regal/assets/lsp/inlay.png differ diff --git a/docs/projects/regal/assets/regal-banner.png b/docs/projects/regal/assets/regal-banner.png new file mode 100644 index 0000000000..b276c4b925 Binary files /dev/null and b/docs/projects/regal/assets/regal-banner.png differ diff --git a/docs/projects/regal/assets/regal.jpg b/docs/projects/regal/assets/regal.jpg new file mode 100644 index 0000000000..6e2c892aa6 Binary files /dev/null and b/docs/projects/regal/assets/regal.jpg differ diff --git a/docs/projects/regal/assets/regal_cncf_london.png b/docs/projects/regal/assets/regal_cncf_london.png new file mode 100644 index 0000000000..79866599c7 Binary files /dev/null and b/docs/projects/regal/assets/regal_cncf_london.png differ diff --git a/docs/projects/regal/assets/rules/pkg_name_completion.png b/docs/projects/regal/assets/rules/pkg_name_completion.png new file mode 100644 index 0000000000..0b68842c23 Binary files /dev/null and b/docs/projects/regal/assets/rules/pkg_name_completion.png differ diff --git a/docs/projects/regal/cicd.md b/docs/projects/regal/cicd.md new file mode 100644 index 0000000000..f881011020 --- /dev/null +++ b/docs/projects/regal/cicd.md @@ -0,0 +1,62 @@ +--- +sidebar_label: CI/CD +sidebar_position: 8 +--- + + +# Using Regal in your build pipeline + +Its possible to use Regal to lint your Rego policies in your CI/CD pipeline(s)! + +This document will guide you on how to do so. Please also review the +[CLI](./cli) documentation for more information on the available options. + +## GitHub Actions + +If you'd like to run Regal in GitHub actions, please consider using +[`setup-regal`](https://github.com/open-policy-agent/setup-regal). A simple `.github/workflows/lint.yml` to run regal +on PRs could look like this, where `policy` contains Rego files: + +```yaml +name: Regal Lint +on: + pull_request: +jobs: + lint-rego: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: open-policy-agent/setup-regal@v1 + with: + # For production workflows, use a specific version, like v0.22.0 + version: latest + + - name: Lint + run: regal lint --format=github ./policy +``` + +Please see [`setup-regal`](https://github.com/open-policy-agent/setup-regal) for more information. + +## GitLab CI/CD + +To use Regal in GitLab CI/CD, you could for example use the following stage in your `.gitlab-ci.yml`: + +```yaml +regal_lint_policies: + stage: regal-lint + image: + # For production workflows, use a specific version, like v0.22.0 + name: ghcr.io/open-policy-agent/regal:latest + entrypoint: ['/bin/sh', '-c'] + script: + - regal lint ./policy --format junit > regal-results.xml + artifacts: + reports: + junit: regal-results.xml + when: always + rules: + - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' +``` + +The above will run Regal on the `policy` directory when a merge request is created or updated and will show linting +violations as part of the merge request. diff --git a/docs/projects/regal/cli.md b/docs/projects/regal/cli.md new file mode 100644 index 0000000000..d1ea747c8c --- /dev/null +++ b/docs/projects/regal/cli.md @@ -0,0 +1,55 @@ +--- +sidebar_position: 4 +sidebar_label: CLI +--- + + +# CLI + +Regal's CLI is the main way to interact with Regal. In order to support +different use cases (Local, CI, etc.) Regal's CLI is designed with a number of +different formats and exit behaviors. + +## Output Formats + +The `regal lint` command allows specifying the output format by using the `--format` flag. The available output formats +are: + +- `pretty` (default) - Human-readable table-like output where each violation is printed with a detailed explanation +- `compact` - Human-readable output where each violation is printed on a single line +- `json` - JSON output, suitable for programmatic consumption +- `github` - GitHub [workflow command](https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions) + output, ideal for use in GitHub Actions. Annotates PRs and creates a + [job summary](https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary) + from the linter report +- `sarif` - [SARIF](https://sarifweb.azurewebsites.net/) JSON output, for consumption by tools processing code analysis + reports +- `junit` - JUnit XML output, e.g. for CI servers like GitLab that show these results in a merge request. + +## Exit Codes + +Exit codes are used to indicate the result of the `lint` command. The `--fail-level` provided for `regal lint` may be +used to change the exit code behavior, and allows a value of either `warning` or `error` (default). + +If `--fail-level error` is supplied, exit code will be zero even if warnings are present: + +- `0`: no errors were found +- `0`: one or more warnings were found +- `3`: one or more errors were found + +This is the default behavior. + +If `--fail-level warning` is supplied, warnings will result in a non-zero exit code: + +- `0`: no errors or warnings were found +- `2`: one or more warnings were found +- `3`: one or more errors were found + +## OPA Check and Strict Mode + +OPA itself provides a "linter" of sorts, via the `opa check` command and its `--strict` flag. This checks the provided +Rego files not only for syntax errors, but also for OPA +[strict mode](https://www.openpolicyagent.org/docs/policy-language/#strict-mode) violations. Most of the strict +mode checks from before OPA 1.0 have now been made default checks in OPA, and only two additional checks are currently +provided by the `--strict` flag. Those are both important checks not covered by Regal though, so our recommendation is +to run `opa check --strict` against your policies before linting with Regal. diff --git a/docs/projects/regal/configuration/capabilities.md b/docs/projects/regal/configuration/capabilities.md new file mode 100644 index 0000000000..fc1310d2c6 --- /dev/null +++ b/docs/projects/regal/configuration/capabilities.md @@ -0,0 +1,82 @@ +--- +sidebar_position: 5 +--- + + +# Capabilities + +By default, Regal will lint your policies using the +[capabilities](https://www.openpolicyagent.org/docs/deployments/#capabilities) of the latest version of OPA +known to Regal (i.e. the latest version of OPA at the time Regal was released). Sometimes you might want to tell Regal +that some rules aren't applicable to your project (yet!). As an example, if you're running OPA v0.46.0, you likely won't +be helped by the [custom-has-key](https://openpolicyagent.org/projects/regal/rules/idiomatic/custom-has-key-construct) rule, as it +suggests using the `object.keys` built-in function introduced in OPA v0.47.0. The opposite could also be true — +sometimes new versions of OPA will invalidate rules that applied to older versions. An example of this is the upcoming +introduction of `import rego.v1`, which will make +[implicit-future-keywords](https://openpolicyagent.org/projects/regal/rules/imports/implicit-future-keywords) obsolete, as importing +`rego.v1` automatically imports all "future" functions. + +Capabilities help you tell Regal which features to take into account, and rules with dependencies to capabilities +not available or not applicable in the given version will be skipped. + +If you'd like to target a specific version of OPA, you can include a `capabilities` section in your configuration, +providing either a specific `version` of an `engine` (currently only `opa` supported): + +```yaml +capabilities: + from: + engine: opa + version: v0.58.0 +``` + +You can also choose to import capabilities from a file: + +```yaml +capabilities: + from: + file: build/capabilities.json +``` + +You can use `plus` and `minus` to add or remove built-in functions from the given set of capabilities: + +```yaml +capabilities: + from: + engine: opa + version: v0.58.0 + minus: + builtins: + # exclude rules that depend on the http.send built-in function + - name: http.send + plus: + builtins: + # make Regal aware of a custom "ldap.query" function + - name: ldap.query + type: function + decl: + args: + - type: string + result: + type: object +``` + +## Loading Capabilities from URLs + +Starting with Regal version v0.26.0, Regal can load capabilities from URLs with the `http`, or `https` schemes using +the `capabilities.from.url` config key. For example, to load capabilities from `https://example.org/capabilities.json`, +this configuration could be used: + +```yaml +capabilities: + from: + url: https://example.org/capabilities.json +``` + +## Supported Engines + +Regal includes capabilities files for the following engines: + +| Engine | Website | Description | +| ------ | --------------------------------------------------------------- | -------------------- | +| `opa` | [OPA website](https://www.openpolicyagent.org/) | Open Policy Agent | +| `eopa` | [Enterprise OPA website](https://www.styra.com/enterprise-opa/) | Styra Enterprise OPA | diff --git a/docs/projects/regal/configuration/ignore-rules.md b/docs/projects/regal/configuration/ignore-rules.md new file mode 100644 index 0000000000..0bbf4439a5 --- /dev/null +++ b/docs/projects/regal/configuration/ignore-rules.md @@ -0,0 +1,148 @@ +--- +sidebar_position: 6 +--- + + +# Ignoring Rules + +If one of Regal's rules doesn't align with your team's preferences, don't worry! Regal is not meant to be the law, +and some rules may not make sense for your project, or parts of it. +Regal provides several different methods to ignore rules with varying precedence. +The available methods are (ranked highest to lowest precedence): + +- [Inline Ignore Directives](#inline-ignore-directives) cannot be overridden by any other method. +- Enabling or Disabling Rules with CLI flags. + - Enabling or Disabling Rules with `--enable` and `--disable` CLI flags. + - Enabling or Disabling Rules with `--enable-category` and `--disable-category` CLI flags. + - Enabling or Disabling All Rules with `--enable-all` and `--disable-all` CLI flags. + - See [Ignoring Rules via CLI Flags](#ignoring-rules-via-cli-flags) for more details. +- [Ignoring a Rule In Config](#ignoring-a-rule-in-config) +- [Ignoring a Category In Config](#ignoring-a-category-in-config) +- [Ignoring All Rules In Config](#ignoring-all-rules-in-config) + +In summary, the CLI flags will override any configuration provided in the file, and inline ignore directives for a +specific line will override any other method. + +It's also possible to ignore messages on a per-file basis. The available methods are (ranked High to Lowest precedence): + + + +- Using the `--ignore-files` CLI flag. + See [Ignoring Rules via CLI Flags](#ignoring-rules-via-cli-flags). +- [Ignoring Files Globally](#ignoring-files-globally) or + [Ignoring a Rule in Some Files](#ignoring-a-rule-in-some-files). + +## Ignoring a Rule in Config + +If you want to ignore a rule, set its level to `ignore` in the configuration file: + +```yaml +rules: + style: + prefer-snake-case: + # At example.com, we use camel case to comply with our naming conventions + level: ignore +``` + +## Ignoring a Category in Config + +If you want to ignore a category of rules, set its default level to `ignore` in the configuration file: + +```yaml +rules: + style: + default: + level: ignore +``` + +## Ignoring All Rules in Config + +If you want to ignore all rules, set the default level to `ignore` in the configuration file: + +```yaml +rules: + default: + level: ignore + # then you can re-enable specific rules or categories + testing: + default: + level: error + style: + opa-fmt: + level: error +``` + +**Tip**: providing a comment on ignored rules is a good way to communicate why the decision was made. + +## Ignoring a Rule in Some Files + +You can use the `ignore` attribute inside any rule configuration to provide a list of files, or patterns, that should +be ignored for that rule: + +```yaml +rules: + style: + line-length: + level: error + ignore: + files: + # ignore line length in test files to accommodate messy test data + - "*_test.rego" + # specific file used only for testing + - "scratch.rego" +``` + +## Ignoring Files Globally + +**Note**: Ignoring files will disable most language server features +for those files. Only formatting will remain available. +Ignored files won't be used for completions, linting, or definitions +in other files. + +If you want to ignore certain files for all rules, you can use the global ignore attribute in your configuration file: + +```yaml +ignore: + files: + - file1.rego + - "*_tmp.rego" +``` + +## Inline Ignore Directives + +If you'd like to ignore a specific violation in a file, you can add an ignore directive above the line in question, or +alternatively on the same line to the right of the expression: + +```rego +package policy + +# regal ignore:prefer-snake-case +camelCase := "yes" + +list_users contains user if { # regal ignore:avoid-get-and-list-prefix + some user in data.db.users + # ... +} +``` + +The format of an ignore directive is `regal ignore:,...`, where `` is the name of the +rule to ignore. Multiple rules may be added to the same ignore directive, separated by commas. + +Note that at this point in time, Regal only considers the same line or the line following the ignore directive, i.e. it +does not apply to entire blocks of code (like rules, functions or even packages). See [configuration](#configuration) +if you want to ignore certain rules altogether. + +## Ignoring Rules via CLI Flags + +For development and testing, rules or classes of rules may quickly be enabled or disabled using the relevant CLI flags +for the `regal lint` command: + +- `--disable-all` disables **all** rules +- `--disable-category` disables all rules in a category, overriding `--enable-all` (may be repeated) +- `--disable` disables a specific rule, overriding `--enable-all` and `--enable-category` (may be repeated) +- `--enable-all` enables **all** rules +- `--enable-category` enables all rules in a category, overriding `--disable-all` (may be repeated) +- `--enable` enables a specific rule, overriding `--disable-all` and `--disable-category` (may be repeated) +- `--ignore-files` ignores files using glob patterns, overriding `ignore` in the config file (may be repeated) + +**Note:** all CLI flags override configuration provided in file. diff --git a/docs/projects/regal/configuration/index.md b/docs/projects/regal/configuration/index.md new file mode 100644 index 0000000000..eaf4ff7d42 --- /dev/null +++ b/docs/projects/regal/configuration/index.md @@ -0,0 +1,99 @@ +--- +sidebar_position: 4 +--- + + +# Configuration + +A custom configuration file may be used to override the [default configuration](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/config/provided/data.yaml) +options provided by Regal. The most common use case for this is to change the severity level of a rule. These three +levels are available: + +- `ignore` — disable the rule entirely +- `warning` — report the violation without changing the exit code of the lint command +- `error` — report the violation and have the lint command exit with a non-zero exit code (default) + +Additionally, some rules may have configuration options of their own. See the documentation page for a rule to learn +more about it. + +**.regal/config.yaml** or **.regal.yaml** + +```yaml +rules: + style: + todo-comment: + # don't report on todo comments + level: ignore + line-length: + # custom rule configuration + max-line-length: 100 + # warn on too long lines, but don't fail + level: warning + opa-fmt: + # not needed as error is the default, but + # being explicit won't hurt + level: error + # files can be ignored for any individual rule + # in this example, test files are ignored + ignore: + files: + - "*_test.rego" + custom: + # custom rule configuration + naming-convention: + level: error + conventions: + # ensure all package names start with "acmecorp" or "system" + - pattern: '^acmecorp\.[a-z_\.]+$|^system\.[a-z_\.]+$' + targets: + - package + +capabilities: + from: + # optionally configure Regal to target a specific version of OPA + # this will disable rules that has dependencies to e.g. built-in + # functions or features not supported by the given version + # + # if not provided, Regal will use the capabilities of the latest + # version of OPA available at the time of the Regal release + engine: opa + version: v0.58.0 + +ignore: + # files can be excluded from all lint rules according to glob-patterns + files: + - file1.rego + - "*_tmp.rego" + +project: + roots: + # declares the 'main' and 'lib/jwt' directories as project roots + - main + - lib/jwt + # may also be provided as an object with additional options + - path: lib/legacy + rego-version: 0 +``` + +Regal will automatically search for a configuration file (`.regal/config.yaml` +or `.regal.yaml`) in the current directory, and if not found, traverse the +parent directories either until either one is found, or the top of the directory +hierarchy is reached. If no configuration file is found, and no file is found at +`~/.config/regal/config.yaml` either, Regal will use the default configuration. + +A custom configuration may be also be provided using the `--config-file`/`-c` +option for `regal lint`, which when provided will be used to override the +default configuration. + +## User-level Configuration + +Generally, users will want to commit their Regal configuration file to the repo +containing their Rego source code. This allows configurations to be shared +among team members and makes the configuration options available to Regal when +running as a [CI linter](https://openpolicyagent.org/projects/regal/cicd) too. + +Sometimes however it can be handy to have some user defaults when a project +configuration file is not found, hasn't been created yet or is not applicable. + +In such cases Regal will check for a configuration file at +`~/.config/regal/config.yaml` instead. diff --git a/docs/projects/regal/configuration/project-roots.md b/docs/projects/regal/configuration/project-roots.md new file mode 100644 index 0000000000..d1e03a0312 --- /dev/null +++ b/docs/projects/regal/configuration/project-roots.md @@ -0,0 +1,41 @@ +--- +sidebar_position: 7 +--- + + +# Project Roots + +While many projects consider the project's root directory (in editors often referred to as **workspace**) their +"main" directory for policies, some projects may contain code from other languages, policy "subprojects", or multiple +[bundles](https://www.openpolicyagent.org/docs/management-bundles/). While most of Regal's features works +independently of this — linting, for example, doesn't consider where in a workspace policies are located as long as +those locations aren't [ignored](./ignore-rules) — some features, like automatically +[fixing](https://openpolicyagent.org/projects/regal/fixing) violations, benefit from knowing when a project contains multiple roots. + +To provide an example, consider the +[directory-package-mismatch](https://openpolicyagent.org/projects/regal/rules/idiomatic/directory-package-mismatch) rule, which states +that a file declaring a `package` path like `policy.permissions.users` should also be located in a directory structure +that mirrors that package, i.e. `policy/permissions/users`. When a violation against this rule is reported, the +`regal fix` command, or its equivalent [Code Action](https://openpolicyagent.org/projects/regal#regal-language-server) in editors, +may when invoked remediate the issue by moving the file to the correct location. +But where should the `policy/permissions/users` directory _itself_ reside? + +Normally, the answer to that question would be the **project**, or **workspace** root. But if the file was found +in a subdirectory containing a **bundle**, the directory naturally belongs under that _bundle's root_ instead. The +`roots` configuration option under the top-level `project` object allows you to tell Regal where these roots are, +and have features like the `directory-package-mismatch` fixer work as you'd expect. + +```yaml +project: + roots: + - bundle1 + - bundle2 +``` + +The configuration file is not the only way Regal may determine project roots. Other ways include: + +- A directory containing a `.manifest` file will automatically be registered as a root +- A directory containing a `.regal` directory will be registered as a root (this is normally the project root) + +If a feature that depends on project roots fails to identify any, it will either fail or fall back on the directory +in which the command was run. diff --git a/docs/projects/regal/configuration/rego-version.md b/docs/projects/regal/configuration/rego-version.md new file mode 100644 index 0000000000..bb544daeb3 --- /dev/null +++ b/docs/projects/regal/configuration/rego-version.md @@ -0,0 +1,36 @@ +--- +sidebar_position: 8 +sidebar_label: Rego Version +--- + + +# Configuring Rego Version + +From OPA 1.0 and onwards, it is no longer necessary to include `import rego.v1` in your policies in order to use +keywords like `if` and `contains`. Since Regal works with with both 1.0+ policies and older versions of Rego, the linter +will first try to parse a policy as 1.0 and if that fails, parse using "v0" rules. This process isn't 100% foolproof, +as some policies are valid in both versions. Additionally, parsing the same file multiple times adds some overhead that +can be skipped if the version is known beforehand. To help Regal determine (and enforce) the version of your policies, +the `rego-version` attribute can be set in the `project` configuration: + +```yaml +project: + # Rego version 1.0, set to 0 for pre-1.0 policies + rego-version: 1 +``` + +It is also possible to set the Rego version for individual project roots (see below for more information): + +```yaml +project: + roots: + - path: lib/legacy + rego-version: 0 + - path: main + rego-version: 1 +``` + +Additionally, Regal will scan the project for any `.manifest` files, and user any `rego_version` found in the manifest +for all policies under that directory. + +Note: the `rego-version` attribute in the configuration file has precedence over `rego_version` found in manifest files. diff --git a/docs/projects/regal/custom-rules/index.md b/docs/projects/regal/custom-rules/index.md new file mode 100644 index 0000000000..4fa37ea016 --- /dev/null +++ b/docs/projects/regal/custom-rules/index.md @@ -0,0 +1,381 @@ +--- +sidebar_position: 5 +--- + + +# Custom Rules + +Regal is built to be easily extended. Using custom rules is a great way to enforce naming conventions, best practices +or more opinionated rules across teams and organizations. + +There are two types of custom rules to be aware of — those that are included with Regal in the `custom` category, and +those that you write yourself. The rules in the `custom` category provide a way to enforce common organizational +requirements, like naming conventions, by means of _configuration_ rather than code. If your requirements aren't +fulfilled by the rules in this category, your other option is to write your own custom rules using Rego. + +## Your Own Custom Rules + +If you'd like to provide your own linter rules for a project, you may do so by placing them in a `rules` directory +inside the `.regal` directory preferably placed in the root of your project (which is also where custom configuration +resides). The directory structure of a policy repository with custom linter rules might then look something like this: + +```text +. +├── .regal +│ ├── config.yaml +│ └── rules +│ ├── naming.rego +│ └── naming_test.rego +└── policy + ├── authz.rego + └── authz_test.rego +``` + +If you so prefer, custom rules may also be provided using the `--rules` option for `regal lint`, which may point either +to a Rego file, or a directory containing Rego files and potentially data (JSON or YAML). + +## Creating a New Rule + +The simplest way to create a new rule is to use the `regal new rule` command. This command provides scaffolding for +quickly creating a new rule, including a file for testing. The command has two required arguments: `--category` and +`--name`, which should be self-explanatory. To create a new custom rule: + +```shell +regal new rule --category naming --name foo-bar-baz +``` + +This will create a `.regal/rules` directory in the current working directory, if one does not already exist, and place +a starter policy and a test in a directory structure based on `--category` and `--name` in it. Following the above +example would create the following directory structure under `.regal/rules`: + +```text +custom/regal/rules/naming/foo-bar-baz/foo_bar_baz.rego +custom/regal/rules/naming/foo-bar-baz/foo_bar_baz_test.rego +``` + +If you'd rather create this directory structure in some other place than the current working directory, you may use the +`--output` flag to specify a different location. The generated rule includes a simple example, which can be verified by +running `regal test .regal/rules`. Modify the rule and the test to suit your needs! + +If you'd like to create a new built-in rule for submitting a PR in Regal, you may add the `--type builtin` flag to the +command (the default is `custom`). This will create a similar scaffolding under `bundle/regal/rules` in the Regal +repository. + +## Developing Rules + +Regal rules works primarily with the [abstract syntax tree](https://en.wikipedia.org/wiki/Abstract_syntax_tree) (AST) +provided by OPA's parser as input. The top level item in the AST is the +[module](https://pkg.go.dev/github.com/open-policy-agent/opa/ast#Module), which contains nodes for everything found +in a policy, like the package declaration, imports and rules. + +Since OPA expects JSON as input, the module and all its child nodes are serialized, and then made available as `input` +in Regal linter rule policies. The `opa parse --format json` command can be used to get an idea of what the structure +of the serialized AST looks like. However, recent versions of Regal leverage an optimized AST JSON representation called +[roast](roast.md), which is both more compact and performant to traverse as part of +linting. See the roast docs for more information on how the format differs from the "normal" OPA AST. + +In order to view the JSON AST representation of a policy, use the `regal parse` command. This works similarly to +`opa parse`, but only outputs the roast JSON format, including additions made by Regal. + +If we were to write the simplest policy possible, and parse it using `regal parse`, it would contain nothing but a +package declaration: + +**policy.rego** + +```rego +package policy +``` + +Using `regal parse policy.rego`, we're provided with the AST of the above policy: + +```json +{ + "package": { + "location": "1:1:1:8", + "path": [ + { + "type": "var", + "value": "data" + }, + { + "location": "1:9:1:15", + "type": "string", + "value": "policy" + } + ] + }, + "regal": { + "file": { + "name": "policy.rego", + "lines": [ + "package policy", + "" + ], + "abs": "/Users/anderseknert/tmp/custom/policy.rego" + }, + "environment": { + "path_separator": "/" + } + } +} +``` + +As trivial as may be, it's enough to build our first linter rule! Let's say we'd like to enforce a uniform naming +convention on any policy in a repository. Packages may be named anything, but must start with the name of the +organization (Acme Corp). So `package acme.corp.policy` should be allowed, but not `package policy` or +`package policy.acme.corp`. One exception: policy authors should be allowed to write policy for the `system.log` package +provided by OPA to allow +[masking](https://www.openpolicyagent.org/docs/management-decision-logs/#masking-sensitive-data) sensitive data +from decision logs. + +An example policy to implement this requirement might look something like this: + +```rego +# METADATA +# description: All packages must use "acme.corp" base name +# related_resources: +# - description: documentation +# ref: https://www.acmecorp.example.org/docs/regal/package +# schemas: +# - input: schema.regal.ast +package custom.regal.rules.naming["acme-corp-package"] + +import data.regal.result + +report contains violation if { + not acme_corp_package + not system_log_package + + violation := result.fail(rego.metadata.chain(), result.location(input.package.path[1])) +} + +acme_corp_package if { + input.package.path[1].value == "acme" + input.package.path[2].value == "corp" +} + +system_log_package if { + input.package.path[1].value == "system" + input.package.path[2].value == "log" +} +``` + +Starting from top to bottom, these are the components comprising our custom rule: + +1. The package of custom rules **must** start with `custom.regal.rules`, followed by the category of the rule, and the + title (which is commonly quoted as rule names use `-` for spaces). +1. The `data.regal.result` provides some helpers for formatting the result of a violation for inclusion in a report. +1. Regal rules make heavy use of [metadata annotations](https://www.openpolicyagent.org/docs/policy-language/#annotations) + in order to document the purpose of the rule, along with any other + information that could potentially be useful. All rule packages **must** have + a `description`. Providing links to additional documentation under + `related_resources` is recommended, but not required. +1. Note the `schema` attribute present in the metadata annotation. Adding this is optional, but highly recommended, as + it will make the compiler aware of the structure of the input, i.e. the AST. This allows the compiler to fail when + unknown attributes are referenced, due to typos or other mistakes. The compiler will also fail when an attribute is + referenced using a type it does not have, like referring to a string as if it was a number. Set to `schema.regal.ast` + to use the AST schema provided by Regal. +1. Regal will evaluate any rule named `report` in each linter policy, so at least one `report` rule **must** be present. +1. In our example `report` rule, we evaluate another rule (`acme_corp_package`) in order to know if the package name + starts with `acme.corp`, and another rule (`system_log_package`) to know if it starts with `system.log`. If neither + of the conditions are true, the rule fails and violation is created. +1. The violation is created by calling `result.fail`, which takes the metadata from the package (using + `rego.metadata.chain` which conveniently also includes the path of the package) and returns a result, which + will later be included in the final report provided by Regal. +1. The `result.location` helps extract the location from the element failing the test. Make sure to use it! + +### Rule Development Workflow + +In addition to making use of the `regal parse` command to inspect the AST of a policy, using Regal's +[language server](https://openpolicyagent.org/projects/regal/language-server) for rule development provides the absolute best rule +development experience. + +#### Code Lens for Evaluation + +If you're using VS Code and the [OPA VS Code extension](https://github.com/open-policy-agent/vscode-opa), you may +use the [Code Lens for Evaluation](https://openpolicyagent.org/projects/regal/language-server#code-lenses-evaluation) to directly +evaluate packages and rules using the `input.json` file as input, and see the result directly in your editor on the +line you clicked to evaluate. + +To start evaluating a policy against your custom rule. First turn the parse result of the policy into an input file: + +```shell +regal parse path/to/policy.rego > input.json +``` + +You should now be able evaluate your custom rule against the `input.json` AST: + +![Code Lens for Evaluation of custom rule](../assets/evalcustom.png) + +**Tips:** + +- You can hover the inlined result to see the full output +- Calls to `print` inside rule bodies will have the print output displayed on the same line + +As another convenience, any `.rego` file where the first comment in the policy is `# regal eval:use-as-input` will have +the evaluation feature automatically use the AST of the file as input. This allows building queries against the AST of +the policy you're working on, providing an extremely fast feedback loop for developing new rules! + +![Use AST of file as input](../assets/lsp/eval_use_as_input.png) + +#### Test-Driven Development + +Using a test-driven approach to custom rule development is a great way to both understand how your rule works, and to +assert that it works as expected even as you make changes to the code. Use the `regal test` command the same way as you +would use `opa test`: + +```shell +regal test .regal/rules +``` + +To debug failures in your test, the `--var-values` flag can help by providing more information about which values +failed to match the expected output. You can also use the `print` function anywhere in your policy, which will have +it's output printed by the test runner. + +Tests commonly first parse a policy, then provide that as input to the rule being tested. You can either use the +built-in `regal.parse_module(name, policy)` function to parse a policy, or one of the provided helpers in the +`regal.ast` package: + +Using `ast.with_rego_v1(policy)` will have a package declararation and `import rego.v1` added to the policy, allowing +you to get straight to what you actually want to test for: + +```rego +test_fail_constant_condition if { + module := ast.with_rego_v1(`allow if true`) + report := rule.report with input as module + + count(report) == 1 + some violation in report + violation.title == "constant-conditoon" +} +``` + +The `ast.policy(policy)` adds only a package declaration and not `import rego.v1`. + +## Aggregate Rules + +Aggregate rules are a special type of rule that allows you to collect data from multiple files before making a decision. +This is needed in some cases where a single policy file won't be enough for a linter rule to make a decision. For +example, you may want to enforce that in a policy repository, there must be at least one package annotated with an +`authors` attribute. This requires first collecting annotation data from all the provided policies, and then have the +rule use this data to make a decision. The structure of an aggregate rule is similar to a regular rule, but with a few +notable differences. The most significant one is that linting happens in two separate phases — one that aggregates data +from files, and one that actually lints and reports violations using that data. + +```rego +# METADATA +# description: | +# There must be at least one boolean rule named `allow`, and it must +# have a default value of `false` +# related_resources: +# - description: documentation +# ref: https://www.acmecorp.example.org/docs/regal/aggregate-allow +# schemas: +# - input: schema.regal.ast +package custom.regal.rules.organizational["at-least-one-allow"] + +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 allow + ast.ref_to_string(rule.head.ref) == "allow" + + # make sure it's a default assignment + # ideally we'll want more than that, but the *requirement* 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 + + # if found, collect the result into our aggregate collection + # we don't really need the location here, but showing for demonstration + entry := result.aggregate(rego.metadata.chain(), { + # optional metadata here + "package": input.package, + }) +} + +# METADATA +# description: | +# This is called once all aggregates have been collected. Note the use of a +# different schema here for type checking, as the input is no longer the AST +# of a Rego policy, but our collected data. +# schemas: +# - input: schema.regal.aggregate +aggregate_report contains violation if { + # input.aggregate contains only the entries collected by *this* aggregate rule, + # so you don't need to worry about counting entries from other sources here! + count(input.aggregate) == 0 + + # no aggregated data found, so we'll report a violation + # another rule may of course want to make use of the data collected in the aggregation + violation := result.fail(rego.metadata.chain(), { + "message": "At least one rule named `allow` must exist, and it must have a default value of `false`", + }) +} +``` + +As you can see, the aggregate rule is split into two parts — one that collects data (`aggregate`), and one that reports +violations (`aggregate_report`). + +Use of helper functions like `result.aggregate` is optional, but **highly** recommended, as it will have any aggregate +entry contain information like file, location and package, which is useful both for reporting, but also for debugging. +Use a `print` or two in the `aggregate_report` rule to see exactly what's included! + +## Parsing and Testing + +Regal provides a few tools mirrored from OPA in order to help test and debug custom rules. These are necessary since OPA +is not aware of the custom [built-in functions](#built-in-functions) included in Regal, and will fail when encountering +e.g. `regal.parse_module` in a custom linter policy. The following commands are included with Regal to help you author +custom rules: + +- `regal parse` works similarly to `opa parse`, but will always output JSON and include location information, and any + additional data added to the AST by Regal. Use this if you want to know exactly what the `input` will look like for + any given policy, when provided to Regal for linting. +- `regal test` works like `opa test`, but aware of any custom Regal additions, and the schema used for the AST. Use this + to test custom linter rules, e.g. `regal test .regal/rules`. + +Note that the `print` built-in function is enabled for `regal test`. Good to use for quick debugging! + +## Built-in Functions + +Regal provides a few custom built-in functions tailor-made for linter policies. + +### `regal.parse_module(filename, policy)` + +Works just like `rego.parse_module`, but provides an AST including location information, and custom additions added +by Regal, like the text representation of each line in the original policy. This is useful for authoring tests to assert +linter rules work as expected. This is the built-in function equivalent of the `regal parse` command. + +If the `filename` provided ends with `_v0.rego`, the policy will be parsed as a Rego v0 module. + +### `regal.last(array)` + +This built-in function is a much more performant way to express `array[count(array) - 1]`. This performance difference +is almost always irrelevant in "normal" Rego policies, but can have a significant impact in linter rules where it's +sometimes called thousands of times as part of traversing the input AST. + +## Rego Library + +In addition to this, Regal provides many helpful functions, rules and utilities in Rego. Browsing the source code of the +[regal.ast](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/ast/ast.rego) package to see what's +available is recommended! + +Note however that at this point in time, the Rego API is not considered stable, and breaking changes are likely to +occur between versions. If you need stable versions of rules and functions found here, consider copying them into a +library of your own, and use in your custom rules. Or engage with the Regal community and tell us what you need and +depend on, and we'll try to take it into account, or at least help you find ways to make it work! + +## Language server + +When using custom rules with the Regal language server, rules will be loaded +from the `.regal/rules` directory relative to the selected workspace root. + +Custom rules will be loaded each time a lint is run for a file or the workspace. +If your custom rules are broken, fixes will be used on the next linting run. diff --git a/docs/projects/regal/custom-rules/roast.md b/docs/projects/regal/custom-rules/roast.md new file mode 100644 index 0000000000..241f4c8892 --- /dev/null +++ b/docs/projects/regal/custom-rules/roast.md @@ -0,0 +1,228 @@ +--- +sidebar_position: 2 +--- + + +# Roast (Regal's Optimized AST) + +Roast is an optimized JSON format for [Rego](https://www.openpolicyagent.org/docs/latest/policy-language/) ASTs, as well +as common utilities for working with the both the Roast format and OPA's AST APIs. + +Roast is used by [Regal](https://openpolicyagent.org/projects/regal), where the JSON representation of Rego's AST is used input for +static analysis [performed by Rego itself](https://www.styra.com/blog/linting-rego-with-rego/) to determine whether +policies conform to Regal's linter rules. + +> [!IMPORTANT] +> This library changes frequently and provides no guarantees for what its API looks like. Depending on this library +> directly is not recommended! If you find any code here useful, feel free to use it as your own in your projects, but +> be aware that it may change at any time, and you may be better off copy-pasting whatever code you need. + +## Goals + +- Fast to traverse and process in Rego +- Usable without having to deal with quirks and inconsistencies +- As easy to read as the original AST JSON format + +While this module provides a way to encode an `ast.Module` to an optimized JSON format, it does not provide a decoder. +In other words, there's currently no way to turn optimized AST JSON back into an `ast.Module` (or other AST types). +While this would be possible to do, there's no real need for that given our current use-case for this format, which is +to help work with the AST efficiently in Rego. Roast should not be considered a general purpose format for serializing +the Rego AST. + +## Differences + +The following section outlines the differences between the original AST JSON format and the Roast format. + +### Compact `location` format + +The perhaps most visually apparent change to the AST JSON format is how `location` attributes are represented. These +attributes are **everywhere** in the AST, so optimizing these for fast traversal has a huge impact on both the size of +the format and the speed at which it can be processed. + +In the original AST, a `location` is represented as an object: + +```json +{ + "file": "p.rego", + "row": 5, + "col": 1, + "text": "Y29sbGVjdGlvbg==" +} +``` + +And in the optimized format as a string: + +```json +"5:1:5:11" +``` + +The first two numbers are present in both formats, i.e. `row` and `col`. In the optimized format, the third and fourth +number is the end location, determined by the length of the `text` attribute decoded. In this case `Y29sbGVjdGlvbg==` +decodes to `collection`, which is 10 characters long. The end location is therefore `5:11`. The text can later be +retrieved when needed using the original source document as a lookup table of sorts. + +While this may come with a small cost for when the `location` is actually needed, it's a huge win for when it's not. +Having to `split` the result and parse the row and column values when needed occurs some overhead, but only a small +percentage of `location` attributes are commonly used in practice. + +Note that the `file` attribute is omitted entirely in the optimized format, as this would otherwise have to be repeated +for each `location` value. This can easily be retrieved by other means. + +### "Empty" rule and `else` bodies + +Rego rules don't necessarily have a body, or at least not one that's printed. Examples of this include: + +```rego +package policy + +default rule := "value" + +map["key"] := "value" + +collection contains "value" +``` + +OPA represents such rules internally (that is, in the AST) as having a body with a single expression containing the +boolean value `true`. This creates a uniform way to represent rules, so a rule like: + +```rego +collection contains "value" +``` + +Would in the AST be identical to: + +```rego +collection contains "value" if { + true +} +``` + +And in the OPA JSON AST format: + +```json +{ + "body": [ + { + "index": 0, + "location": { + "file": "p.rego", + "row": 5, + "col": 1, + "text": "Y29sbGVjdGlvbg==" + }, + "terms": { + "location": { + "file": "p.rego", + "row": 5, + "col": 1, + "text": "Y29sbGVjdGlvbg==" + }, + "type": "boolean", + "value": true + } + } + ], + "head": { + "name": "collection", + "key": { + "location": { + "file": "p.rego", + "row": 5, + "col": 21, + "text": "InZhbHVlIg==" + }, + "type": "string", + "value": "value" + }, + "ref": [ + { + "location": { + "file": "p.rego", + "row": 5, + "col": 1, + "text": "Y29sbGVjdGlvbg==" + }, + "type": "var", + "value": "collection" + } + ], + "location": { + "file": "p.rego", + "row": 5, + "col": 1, + "text": "Y29sbGVjdGlvbiBjb250YWlucyAidmFsdWUi" + } + }, + "location": { + "file": "p.rego", + "row": 5, + "col": 1, + "text": "Y29sbGVjdGlvbg==" + } +} +``` + +Notice how there's 20 lines of JSON just to represent the body, even though there isn't really one! + +The optimized Rego AST format discards generated bodies entirely, and the same rule would be represented as: + +```json +{ + "head": { + "location": "5:1:5:11", + "ref": [ + { + "location": "5:1:5:11", + "type": "var", + "value": "collection" + } + ], + "key": { + "type": "string", + "value": "value", + "location": "5:21:5:27" + } + }, + "location": "5:1:5:11" +} +``` + +Note that this applies equally to empty `else` bodies, which are represented the same way in the original AST, and +omitted entirely in the optimized format. + +Similarly, Roast discards `location` attributes from attributes that don't have an actual location in the source code. +An example of this is the `data` term of a package path, which is present only in the AST. + +### Removed `annotations` attribute from module + +OPA already attaches `annotations` to rules. With the Roast format attaching `package` and `subpackages` scoped +`annotations` to the `package` as well, there is no need to store `annotations` at the module level, as that's +effectively just duplicating data. Having this removed can save a considerable amount of space in well-documented +policies, as they should be! + +### Removed `index` attribute from body expressions + +In the original AST, each expression in a body carries a numeric `index` attribute. While this doesn't take much space, +it is largely redundant, as the same number can be inferred from the order of the expressions in the body array. It's +therefore been removed from the Roast format. + +### Removed `name` attribute from rule heads + +The `name` attribute found in the OPA AST for `rules` is unreliable, as it's not always present. The `ref` +attribute however always is. While this doesn't come with any real cost in terms of AST size or performance, consistency +is key. + +### Fixed inconsistencies in the original Rego AST + +A few inconsistencies exist in the original AST JSON format: + +- `comments` attributes having a `Text` attribute rather than the expected `text` +- `comments` attributes having a `Location` attribute rather than the expected `location` + +Fixing these in the original format would be a breaking change. The Roast format corrects these inconsistencies, and +uses `text` and `location` consistently. + +## Performance + +While the numbers may vary some, the Roast format is currently about 40-50% smaller in size than the original AST JSON +format, and can be processed (in Rego, using `walk` and so on) about 1.25 times faster. diff --git a/docs/projects/regal/debug-adapter.md b/docs/projects/regal/debug-adapter.md new file mode 100644 index 0000000000..75b34118ff --- /dev/null +++ b/docs/projects/regal/debug-adapter.md @@ -0,0 +1,69 @@ +--- +sidebar_position: 10 +--- + + +# Debug Adapter + +In addition to being a [language server](https://openpolicyagent.org/projects/regal/language-server), +Regal can act as a +[Debug Adapter](https://microsoft.github.io/debug-adapter-protocol/). +A Debug Adapter is a program that can communicate with a debugger client, +such as Visual Studio Code's debugger, to provide debugging capabilities +for a language. + +Animation showing the a debugging session in VS Code +_A debugging session in VS Code_ + +:::info +In order to use the Debug Adapter, you must be using +[Regal v0.27.0](https://github.com/open-policy-agent/regal/releases/v0.27.0) or greater, +as well as a compatible client. See [Editor Support](https://openpolicyagent.org/projects/regal/editor-support) for +more details. +::: + +## Getting Started + +See the documentation in the Editor Support page for supported clients: + +* [Visual Studio Code](https://openpolicyagent.org/projects/regal/editor-support#visual-studio-code) +* [Neovim](https://openpolicyagent.org/projects/regal/editor-support#neovim) + +## Features + +The Regal Debug Adapter currently supports the following features: + +### Breakpoints + +Breakpoints allow you to continue execution of a policy until a given point. +This can be helpful for: + +* Inspection of variables at a given point in time +* Seeing how many times a given block of Rego code is executed, if at all +* Avoiding the need to step through code as it's run line by line + +Screenshot of a breakpoint in VS Code + +### Variable Inspection + +Either at a breakpoint or while stepping through code, you can inspect the +local variables in scope as well as the contents of the global `input` and +`data` documents. + +`input` will be loaded from `input.json` in the workspace if it exists. + +Variables being inspected during execution in VS Code + +### Print Statements + +Print statements are also supported, these are shown in the debug console: + +Print statements shown in the debug output console diff --git a/docs/projects/regal/editor-support.md b/docs/projects/regal/editor-support.md new file mode 100644 index 0000000000..ae4e8d1072 --- /dev/null +++ b/docs/projects/regal/editor-support.md @@ -0,0 +1,161 @@ +--- +sidebar_position: 7 +--- + + +# Editor Support + +## Visual Studio Code + +[vscode-opa](https://marketplace.visualstudio.com/items?itemName=tsandall.opa) - +the official OPA extension for Visual Studio Code - now supports the Regal language server. + +To see Regal linting as you work, install the extension at version `0.13.3` or later +and open a workspace with Rego files. + +The plugin will automatically find and use [Regal config](https://openpolicyagent.org/projects/regal#configuration). + +### Debug Adapter Protocol (DAP) + +From +[`v0.17.0`](https://github.com/open-policy-agent/vscode-opa/blob/main/CHANGELOG.md#0170) +onwards, the OPA extension for Visual Studio Code supports the +[Regal Debug Adapter](https://openpolicyagent.org/projects/regal/debug-adapter). + +To start a new debug session use the code action `Debug` found above a Rego rule +or package. + +Code Action in VS Code + +Breakpoints can be added by clicking in the gutter to the left of the editor. +Print statements will be shown in the debug console. + +## Zed + +[Zed](https://zed.dev) is a modern open-source code editor with focus on performance and simplicity. + +Zed supports Rego via Regal and the [zed-rego](https://github.com/StyraInc/zed-rego) extension developed by the Styra +community. The extension provides syntax highlighting, linting, and most of the other language server features provided +by Regal. + +## Neovim + +[Neovim](https://neovim.io/) supports both the Language Server Protocol and the Debug Adapter Protocol. + +Generally, the Regal binary should be [installed](https://openpolicyagent.org/projects/regal#getting-started) +first. [`mason.vim`](https://github.com/williamboman/mason.nvim) users can install the +Regal binary with `:MasonInstall regal` +([package definition](https://github.com/mason-org/mason-registry/blob/2024-07-23-asian-hate/packages/regal/package.yaml)). + +### Language Server Protocol (LSP) + +There are a number of different plugins available for Neovim which integrate +with language servers using the Language Server Protocol. + +Below are a number of different plugin options to configure a language server +client for Regal in Neovim. + +#### nvim-lspconfig + +[nvim-lspconfig](https://github.com/neovim/nvim-lspconfig) has native support for the +Regal language server. Use the configuration below to configure Regal: + +```lua +require('lspconfig').regal.setup() +``` + +#### none-ls + +[none-ls](https://github.com/nvimtools/none-ls.nvim) - Use Neovim as a +language server to inject LSP diagnostics, code actions, and more via Lua. + +Minimal installation via [VimPlug](https://github.com/junegunn/vim-plug) + +```vim +Plug 'nvim-lua/plenary.nvim' +Plug 'nvimtools/none-ls.nvim' + +lua < regal fix bundle +3 fixes applied: +In project root: /Users/john/projects/authz/bundle + +lib/roles.rego: +- use-rego-v1 + +policy.rego -> main/policy.rego: +- directory-package-mismatch +- no-whitespace-comment +``` + +In the example above, Regal made fixes corresponding to the linter rules `use-rego-v1`, `directory-package-mismatch`, +and `no-whitespace-comment` in `lib/roles.rego` and `policy.rego`. While the number of fixes applied was reported as 3, +the number of _violations_ fixed could of course have been higher, as e.g. the `no-whitespace-comment` rule might have +been violated in multiple places in `policy.rego`. Note also how one of the fixes (`directory-package-mismatch`) +involved **moving** `policy.rego` to `main/policy.rego`, as that rule requires the file to be in a directory structure +matching its package path (`package main`). + +### Project Roots + +All paths are relative to its closest **project root**, as reported in the second line of the output. Most policy +projects will likely only have one "root", which is the workspace directory itself. More complex projects may however +host multiple roots inside the workspace, and defining these roots — either by configuration, or by `.manifest` files — +will in some cases (like the previously mentioned `directory-package-mismatch` fix) help Regal better apply the correct +fixes. See the documentation on [project roots](https://openpolicyagent.org/projects/regal#project-roots) for more information. + +### Dry Run + +Using the `--dry-run` flag is a great way to see what changes will be made without actually applying them. Following our +example from above, adding the `--dry-run` flag to `regal fix bundle` would have told us beforehand what changes we +should expect to see. Make it a habit to dry-run your fixes before applying them, and make sure you've commited any +other changes before running the fixer! + +## Fixing Violations in Editors + +In addition to the `regal fix` command, users integratiing Regal with their editors can fix violations directly as +they are reported in the file being edited. This is done by means of Code Actions, which commonly displays a lightbulb +icon next to where a violation occurs. Clicking on the lightbulb will show a list of available actions, which in Regal's +case maps directly to the available fix for the violation reported (if any). + +Example of code action in VS Code, where available fixes can be listed either by clicking the lightbulb icon to the +right, or by clicking "Quick Fix..." in the tooltip window: + + + +Example of suggested Code Action for the +[use-assignment-operator](https://openpolicyagent.org/projects/regal/rules/style/use-assignment-operator) rule. Click to fix! + + + +### Limitations + +Compared to `regal fix`, automatically fixing violations in editors has some limitations: + +- Normally works on one file at a time, not entire directories +- No ability to dry-run a fix, but on the other hand, the editor's **Undo** feature will let you easily revert any + changes made. + +:::tip +If you're curious about using Regal to fix problems directly in your editor, see the docs on editor support +[here](https://openpolicyagent.org/projects/regal/editor-support) to learn more! +::: diff --git a/docs/projects/regal/index.md b/docs/projects/regal/index.md new file mode 100644 index 0000000000..4eaf5284de --- /dev/null +++ b/docs/projects/regal/index.md @@ -0,0 +1,285 @@ +--- +image: /img/regal.png +sidebar_position: 1 +title: Introduction +--- + + + +import Intro from '@site/src/components/projects/regal/Intro'; + + + +# Regal + +Regal is a linter and language server for +[Rego](https://www.openpolicyagent.org/docs/policy-language/), making +your Rego magnificent, and you the ruler of rules! + +With its extensive set of linter rules, documentation and editor integrations, +Regal is the perfect companion for policy development, whether you're an +experienced Rego developer or just starting out. + + + + + + + + +## Goals + +- Deliver an outstanding policy development experience by providing the best possible tools for that purpose +- Identify common mistakes, bugs and inefficiencies in Rego policies, and suggest better approaches +- Provide advice on [best practices](https://github.com/StyraInc/rego-style-guide), coding style, and tooling +- Allow users, teams and organizations to enforce custom rules on their policy code + + + + +## What People Say About Regal + +> I really like that at each release of Regal I learn something new! +> Of all the linters I'm exposed to, Regal is probably the most instructive one. + +— Leonardo Taccari, [NetBSD](https://www.netbsd.org/) + +> Reviewing the Regal rules documentation. Pure gold. + +— Dima Korolev, [Miro](https://miro.com/) + +> Such an awesome project! + +— Shawn McGuire, [Atlassian](https://www.atlassian.com/) + +> I am really impressed with Regal. It has helped me write more expressive and deterministic Rego. + +— Jimmy Ray, [Boeing](https://www.boeing.com/) + +See the [adopters](https://openpolicyagent.org/projects/regal/adopters) file for more Regal users. + + + + +## Getting Started + +### Download Regal + +**MacOS and Linux** + +```shell +brew install regal +``` + +
+ Other Installation Options + +Please see [Packages](https://openpolicyagent.org/projects/regal/adopters#packaging) +for a list of package repositories which distribute Regal. + +Manual installation commands: + +**MacOS (Apple Silicon)** + +```shell +curl -L -o regal "https://github.com/open-policy-agent/regal/releases/latest/download/regal_Darwin_arm64" +``` + +**MacOS (x86_64)** + +```shell +curl -L -o regal "https://github.com/open-policy-agent/regal/releases/latest/download/regal_Darwin_x86_64" +``` + +**Linux (x86_64)** + +```shell +curl -L -o regal "https://github.com/open-policy-agent/regal/releases/latest/download/regal_Linux_x86_64" +chmod +x regal +``` + +**Windows** + +```shell +curl.exe -L -o regal.exe "https://github.com/open-policy-agent/regal/releases/latest/download/regal_Windows_x86_64.exe" +``` + +**Docker** + +```shell +docker pull ghcr.io/styrainc/regal:latest +``` + +See all versions, and checksum files, at the Regal [releases](https://github.com/open-policy-agent/regal/releases/) +page, and published Docker images at the [packages](https://github.com/open-policy-agent/regal/pkgs/container/regal) +page. + +
+ +### Try it out! + +First, author some Rego! + +**policy/authz.rego** + +```rego +package authz + +default allow = false + +allow if { + isEmployee + "developer" in input.user.roles +} + +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. + +```shell +regal lint policy/ +``` + + + + +```text +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://openpolicyagent.org/projects/regal/rules/idiomatic/non-raw-regex-pattern + +Rule: use-assignment-operator +Description: Prefer := over = for assignment +Category: style +Location: policy/authz.rego:5:1 +Text: default allow = false +Documentation: https://openpolicyagent.org/projects/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://openpolicyagent.org/projects/regal/rules/style/prefer-snake-case + +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`. + +### Using Regal in Your Editor + +Linting from the command line is a great way to get started with Regal, and even for some experienced developers +the preferred way to work with the linter. However, not only is Regal a linter, but a full-fledged development +companion for Rego development! + +Integrating Regal in your favorite editor means you'll get immediate feedback from the linter as you work on your +policies. More than that, it'll unlock a whole new set of features that leverage Regal's +[language server](https://openpolicyagent.org/projects/regal/language-server), +like context-aware completion suggestions, informative tooltips on hover, +or go-to-definition. + +Elevate your policy development experience with Regal in VS Code, Neovim, Zed, Helix +and more on our [Editor Support page](https://openpolicyagent.org/projects/regal/editor-support)! + +To learn more about the features provided by the Regal language server, see the +[Language Server](https://openpolicyagent.org/projects/regal/language-server) page. + +### Using Regal in Your Build Pipeline + +To ensure Regal's rules are enforced consistently in your project or organization, +we've made it easy to run Regal as part of your builds. +See the docs on [Using Regal in your build pipeline](https://openpolicyagent.org/projects/regal/cicd) to learn more +about how to set up Regal to lint your policies on every commit or pull request. + + + + +## Next Steps + +Now you're up and running with Regal, take a look around some of our documentation +to get a feel for the different features and capabilities of Regal. + +- [Rules](https://openpolicyagent.org/projects/regal/rules) + - [Bugs](https://openpolicyagent.org/projects/regal/rules/bugs): Common mistakes, potential bugs and inefficiencies in Rego policies. + - [Idiomatic](https://openpolicyagent.org/projects/regal/rules/idiomatic): Suggestions for more idiomatic constructs. + - [Imports](https://openpolicyagent.org/projects/regal/rules/imports): Best practices for imports. + - [Performance](https://openpolicyagent.org/projects/regal/rules/performance): Rules for improving performance of policies. + - [Style](https://openpolicyagent.org/projects/regal/rules/style): Rego Style Guide rules. + - [Testing](https://openpolicyagent.org/projects/regal/rules/testing): Rules for testing and development. + - [Custom](https://openpolicyagent.org/projects/regal/rules/custom): Custom rules where enforcement can be adjusted to match your preferences. +- [Configuration](https://openpolicyagent.org/projects/regal/configuration): Dig into some of the different configuration options available. +- [Editor Support](https://openpolicyagent.org/projects/regal/editor-support): Get Regal integrated into your editor of choice. + - [Language Server](https://openpolicyagent.org/projects/regal/language-server): Learn more + about Regal's advanced editor capabilities. + - [DAP](https://openpolicyagent.org/projects/regal/debug-adapter): Live debug your Rego policies with Regal's DAP support. +- [Fixing Issues](https://openpolicyagent.org/projects/regal/fixing): See Regal can help you fix issues in your Rego policies automatically. +- [CI/CD](https://openpolicyagent.org/projects/regal/ci-cd): Run Regal as part of your automated checks. +- [Custom Rules](https://openpolicyagent.org/projects/regal/custom-rules): Learn how to write your own rules for Regal. +- [Adopters](https://openpolicyagent.org/projects/regal/adopters): See who else is using Regal. + + + + + + +## Learn More + +[Contributing](https://github.com/open-policy-agent/regal/blob/main/docs/CONTRIBUTING.md) +contains information about how to hack on Regal itself. + +### Talks + +- [OPA Maintainer Track, featuring Regal](https://www.youtube.com/watch?v=XtA-NKoJDaI), KubeCon London, 2025 +- [Regal the Rego Linter](https://www.youtube.com/watch?v=Xx8npd2TQJ0&t=2567s), CNCF London meetup, June 2023 + [![Regal the Rego Linter](./assets/regal_cncf_london.png)](https://www.youtube.com/watch?v=Xx8npd2TQJ0&t=2567s) + +### Blogs and Articles + +- [Guarding the Guardrails - Introducing Regal the Rego Linter](https://www.styra.com/blog/guarding-the-guardrails-introducing-regal-the-rego-linter/) + by Anders Eknert ([@anderseknert](https://github.com/anderseknert)) +- [Scaling Open Source Community by Getting Closer to Users](https://thenewstack.io/scaling-open-source-community-by-getting-closer-to-users/) + by Charlie Egan ([@charlieegan3](https://github.com/charlieegan3)) +- [Renovating Rego](https://www.styra.com/blog/renovating-rego/) by Anders Eknert ([@anderseknert](https://github.com/anderseknert)) +- [Linting Rego with... Rego!](https://www.styra.com/blog/linting-rego-with-rego/) by Anders Eknert ([@anderseknert](https://github.com/anderseknert)) +- [Regal: Rego(OPA)用リンタの導入手順](https://zenn.dev/erueru_tech/articles/6cfb886d92858a) by Jun Fujita ([@erueru-tech](https://github.com/erueru-tech)) +- [Regal を使って Rego を Lint する](https://tech.dentsusoken.com/entry/2024/12/05/Regal_%E3%82%92%E4%BD%BF%E3%81%A3%E3%81%A6_Rego_%E3%82%92_Lint_%E3%81%99%E3%82%8B) + by Shibata Takao ([@shibata.takao](https://shodo.ink/@shibata.takao/)) + + + + +## Status + +Regal is currently in beta. End-users should not expect any drastic changes, but any API may change without notice. +If you want to embed Regal in another project or product, please reach out! + + + + +## Roadmap + +The current Roadmap items are all related to the preparation for +[Regal 1.0](https://github.com/open-policy-agent/regal/issues/979): + +- [Go API: Refactor the Location object in Violation (#1554)](https://github.com/open-policy-agent/regal/issues/1554) +- [Rego API: Provide a stable and well-documented Rego API (#1555)](https://github.com/open-policy-agent/regal/issues/1555) +- [Go API: Audit and reduce the public Go API surface (#1556)](https://github.com/open-policy-agent/regal/issues/1556) +- [Custom Rules: Tighten up Authoring experience (#1559)](https://github.com/open-policy-agent/regal/issues/1559) +- [docs: Improve automated documentation generation (#1557)](https://github.com/open-policy-agent/regal/issues/1557) +- [docs: Break down README into smaller units (#1558)](https://github.com/open-policy-agent/regal/issues/1558) +- [lsp: Support a JetBrains LSP client (#1560)](https://github.com/open-policy-agent/regal/issues/1560) + +If there's something you'd like to have added to the roadmap, either open an issue, or reach out in the community Slack! + + diff --git a/docs/projects/regal/integration.md b/docs/projects/regal/integration.md new file mode 100644 index 0000000000..4a25719479 --- /dev/null +++ b/docs/projects/regal/integration.md @@ -0,0 +1,65 @@ +--- +sidebar_position: 12 +sidebar_label: Go Integration +--- + + +# Go Integration + +:warning: Using Regal from Go is currently experimental and subject to change. Please swing by +[Slack](https://slack.openpolicyagent.org) if you're keen to use Regal in a production system. + +If you'd like to integrate Regal into a Go application, this guide contains some pointers. + +## Using Regal from Go + +Regal can be used from a Go application by importing the `linter` package: + +```go +import "github.com/open-policy-agent/regal/pkg/linter" +``` + +### Input + +Input to `Lint` can be provided in a number of ways: + +* Using the `InputFromPaths` helper to load Rego files from the filesystem, +* Using the `InputFromText` helper to parse a single Rego module from a string, + +#### Using `InputFromPaths` + +```go +paths := []string{"foo.rego", "bar.rego"} + +input, err := rules.InputFromPaths(paths) +if err != nil { + // handle error +} +``` + +#### Using `InputFromText` + +```go + +regoText := `package foo...` + +input, err := rules.InputFromText("policy.rego", regoText) +if err != nil { + // handle error +} +``` + +### Linting + +To get a Regal report back for the provided input, create a Regal instance and call `Lint`: + +```go +regalInstance := linter.NewLinter().WithInputModules(&input) + +lintingReport, err := regalInstance.Lint(r.Context()) +if err != nil { + response.ErrorMessage = err.Error() + writeJSON(w, http.StatusOK, response) + return +} +``` diff --git a/docs/projects/regal/language-server.md b/docs/projects/regal/language-server.md new file mode 100644 index 0000000000..3d01cb1277 --- /dev/null +++ b/docs/projects/regal/language-server.md @@ -0,0 +1,236 @@ +--- +sidebar_position: 9 +--- + + +# Language Server + +In order to support Rego policy development in editors like +[VS Code](https://github.com/open-policy-agent/vscode-opa) or [Zed](https://github.com/StyraInc/zed-rego), +Regal provides an implementation of the +[Language Server Protocol](https://microsoft.github.io/language-server-protocol/) (LSP) for Rego. + +This implementation allows the result of linting to be presented directly in your editor as you work on your policies, +and without having to call Regal from the command line. The language server however provides much more than just +linting! + +:::tip +Check support for your editor on the [editor support](./editor-support.md) page. +::: + +## Features + +The Regal language server currently supports the following LSP features: + +### Diagnostics + +Diagnostics are errors, warnings, and information messages that are shown in the editor as you type. Regal currently +uses diagnostics to present users with either parsing errors in case of syntax issues, and linter violations reported +by the Regal linter. + +Screenshot of diagnostics as displayed in Zed + +Future versions of Regal may include also [compilation errors](https://github.com/open-policy-agent/regal/issues/745) +as part of diagnostics messages. + +### Hover + +The hover feature means that moving the mouse over certain parts of the code will bring up a tooltip with documentation +for the code under the cursor. This is particularly useful for built-in functions, as it allows you to quickly look up +the meaning of the function, and the arguments it expects. + +Screenshot of hover as displayed in VS Code + +The Regal language server currently supports hover for all built-in functions OPA provides. + +### Go to definition + +Go to definition allows references to rules and functions to be clicked on (while holding `ctrl/cmd`), and the editor +will navigate to the definition of the rule or function. + +### Folding ranges + +Regal provides folding ranges for any policy being edited. Folding ranges are areas of the code that can be collapsed +or expanded, which may be useful for hiding content that is not relevant to the current task. + +Screenshot of folding ranges as displayed in Zed + +Regal supports folding ranges for blocks, imports and comments. + +### Document and workspace symbols + +Document and workspace symbols allow policy authors to quickly scan and navigate to symbols (like rules and functions) +anywhere in the document or workspace. + +Screenshot showing search on workspace symbols in Zed + +VS Code additionally provides an "Outline" view, which is a nice visual representation of the symbols in the document. + +Screenshot showing outline view of document symbols in VS Code + +### Inlay hints + +Inlay hints help developers quickly understand the meaning of the arguments passed passed to functions in the code, +by showing the name of the argument next to the value. Inlay hints can additionally be hovered for more information, +like the expected type of the argument. + +Screenshot showing inlay hints in VS Code + +Regal currently supports inlay hints for all built-in functions. Future versions may support inlay hints for +user-defined functions too. + +### Formatting + +By default, Regal uses the `opa fmt` formatter for formatting Rego. This is made available as a command in editors, +but also via a [code action](#code-actions) when unformatted files are encountered. + +Screenshot of diagnostics as displayed in Zed + +Two other formatters are also available — `opa fmt --rego-v1` and `regal fix`. See the docs on +[Fixing Violations](fixing.md) for more information about the `fix` command. Which formatter to use +can be set via the `formatter` configuration option, which can be passed to Regal via the client (see +the documentation for your client for how to do that). + +### Code completions + +Code completions, or suggestions, is likely one of the most useful features of the Regal language server. And best of +all, you don't need to do anything special for it to happen! Just write your policy as you normally would, and Regal +will provide suggestions for anything that could be relevant in the context that you're typing. This could for example +be suggestions for: + +- Built-in functions +- Local variables +- Imported packages +- References from anywhere in the workspace +- And much more! + +Screenshot of completion suggestions as displayed in Zed + +New completion providers are added continuously, so if you have a suggestion for +a new completion, please +[open an issue](https://github.com/open-policy-agent/regal/issues)! + +### Code actions + +Code actions are actions that appear in the editor when certain conditions are met. One example would be "quick fixes" +that may appear when a linter rule has been violated. Code actions can be triggered by clicking on the lightbulb icon +that appears on the line with a diagnostic message, or by pressing `ctrl/cmd + .` when the cursor is on the line. + + + +Regal currently provides **quick fix actions** for the following linter rules: + +- [opa-fmt](https://openpolicyagent.org/projects/regal/rules/style/opa-fmt) +- [use-rego-v1](https://openpolicyagent.org/projects/regal/rules/imports/use-rego-v1) +- [use-assignment-operator](https://openpolicyagent.org/projects/regal/rules/style/use-assignment-operator) +- [no-whitespace-comment](https://openpolicyagent.org/projects/regal/rules/style/no-whitespace-comment) +- [directory-package-mismatch](https://openpolicyagent.org/projects/regal/rules/idiomatic/directory-package-mismatch) + +Regal also provides **source actions** — actions that apply to a whole file and aren't triggered by linter issues: + +- **Explore compiler stages for policy** — Opens a browser window with an embedded version of the + [opa-explorer](https://github.com/srenatus/opa-explorer), where advanced users can explore the different stages + of the Rego compiler's output for a given policy. + +### Code lenses (Evaluation) + +The code lens feature provides language servers a way to add actionable commands just next to the code that the action +belongs to. Regal provides code lenses for doing **evaluation** of any package or rule directly in the editor. This +allows for an extremely fast feedback loop, where you can see the result of writing of modifying rules directly as you +work with them, and without having to launch external commands or terminals. In any editor that supports code lenses, +simply press `Evaluate` on top of a package or rule declaration to have it evaluated. The result is displayed on the +same line. + +Screenshot of evaluation performed via code lens + +Once evaluation has completed, the result is also pretty-printed in a tooltip when hovering the rule. This is +particularly useful when the result contains more data than can fit on a single line! + +Note that when evaluating incrementally defined rules, the result reflects evaluation of the whole **document**, +not a single rule definition. To make this clear, the result will be displayed next to each definition of the +same rule. + +In addition to showing the result of evaluation, the "Evaluate" code lens will also display the output of any +`print` calls made in rule bodies. This can be really helpful when trying to figure out _why_ the rule evaluated +the way it did, or where rule evaluation failed. + +Screenshot of evaluation with print call performed via code lens + +Policy evaluation often depends on **input**. This can be provided via an `input.json` or `input.yaml` file which +Regal will search for first in the same directory as the policy file evaluated. If not found there, Regal will proceed +to search each parent directory up until the workspace root directory. It is recommended to add `input.json/yaml` to +your `.gitignore` file so that you can work freely with evaluation in any directory without having your input +accidentally committed. + +#### Editor support + +The Evaluation code lens is supported in any language server client that +supports the running of code lenses. The evaluation result is saved to +`output.json` in the default case. + +The displaying of evaluation results in the current file or buffer is currently +only supported in the +[OPA VS Code extension](https://github.com/open-policy-agent/vscode-opa) and +for Neovim users in +[nvim-dap-rego](https://github.com/rinx/nvim-dap-rego/). + +### Code lenses (Debugging) + +Regal also implements the +[Debug Adapter Protocol](https://microsoft.github.io/debug-adapter-protocol/). +This allows users to trigger debugging sessions for their policies by invoking a +code lens on a rule. For more information, see the [Debug Adapter](./debug-adapter.md) +page. + +#### Editor support + +While the code lens feature is part of the LSP specification, the action that is triggered by a code lens +isn't necessarily part of the standard. The language server protocol does not provide a native method for requesting +evaluation, so Regal will handle that on its own, and differently depending on what the client supports. + +- Currently, only the [OPA VS Code extension](https://github.com/open-policy-agent/vscode-opa) and + [nvim-dap-rego](https://github.com/rinx/nvim-dap-rego/) is capable of handling + the request to display evaluation results on the same line as the package or rule evaluated. +- [Neovim](https://neovim.io/) does not support the requests natively, but + [nvim-dap-rego](https://github.com/rinx/nvim-dap-rego/) provides handlers to support them. + Please follow [the instructions](https://github.com/rinx/nvim-dap-rego/blob/main/README.md#lsp-handlers) in + nvim-dap-rego README. +- [Zed](https://github.com/StyraInc/zed-rego) does not support the code lens feature at all at this point in time. As + soon as it does, Regal will provide them. +- Displaying the result of evaluation requires customized code in the client. Currently only VS Code and Neovim + has the required modifications to handle this, and is thus the only editor to currently support "inline display" + of the result. + For other editors that support the code lens feature, Regal will instead write the result of evaluation to an + `output.json` file. + +## Unsupported features + +See the +[open issues](https://github.com/open-policy-agent/regal/issues?q=is%3Aissue+is%3Aopen+label%3A%22language+server+protocol%22) +with the `language server protocol` label for a list of features that are not yet supported by the Regal language +server, but that are planned for the future. If you have suggestions for anything else, please create a new issue! + +Also note that not all clients (i.e. editors) may support all features of a language server! See the +[editor support](./editor-support.md) page for information about Regal support in different editors. diff --git a/docs/projects/regal/opa-one-dot-zero.md b/docs/projects/regal/opa-one-dot-zero.md new file mode 100644 index 0000000000..f64e8a4462 --- /dev/null +++ b/docs/projects/regal/opa-one-dot-zero.md @@ -0,0 +1,77 @@ +--- +sidebar_label: OPA 1.0 +sidebar_position: 14 +--- + + +# OPA 1.0 and Regal + +While we always recommend using the latest version of OPA, we're well aware that there may be situations where — for +one reason or another — that might not be possible. As we want everyone to benefit from Regal, we do our very best to +ensure it works seamlessly with OPA versions both before and after 1.0, and even projects that use a mix of both! While +this should mostly work out of the box and without additional configuration, it's good to be aware of how Regal parses +and lints policies of different versions of Rego, and how you can tell Regal to target only a specific version. + +**Note:** This document does not cover the specifics of OPA 1.0, but rather how Regal works with it. If you want to +learn more about what OPA 1.0 is and how to upgrade, see the [related resources](#related-resources) at the bottom of +this page. + +## Telling Regal which Rego version to target + +While Regal pretty accurately guesses the Rego version of the policies it's linting — and will adapt how it parses and +asseses Rego files accordingly — telling Regal which version to target is always going to produce the most reliable +results — and much faster too! Guessing which Rego version to target often involves multiple passes of parsing, and +as some files are both valid Rego v0 and v1, there will always be some ambiguity. In order to avoid this, our +recommendation is to always provide Regal with the Rego version(s) targeted. This can be done in a couple of ways, and +the precedence of these methods is as listed below: + +1. Setting the `rego-version` configuration option under `project.roots` attribute +2. Setting the `rego-version` configuration option under `project` attribute +3. Setting the `rego_version` in a `.manifest` file in any directory (will apply to that directory and any below it) + +Note that it's is perfectly possible to use different `rego-version`s for different roots of a project: + +```yaml +project: + rego-version: 1 + roots: + # lib/legacy overriding project version to set versin 0 + - path: lib/legacy + rego-version: 0 + # main directory will inherit version 1 from project + - path: main +``` + +See the documentation covering Regal's [configuration](https://openpolicyagent.org/projects/regal#configuration) for more information +on [configuring Rego version](https://openpolicyagent.org/projects/regal#configuring-rego-version) for your project. + +Finally, Regal will automatically parse and lint any file with a `_v0.rego` suffix as Rego v0. This is intended only +for testing and development, where you sometimes may want to try something out using and older Rego version without +configuration. Note that this has lower precedence than Rego versions set by other means, and should not be considered +as anything but a convenience for testing. + +## Rules disabled with OPA 1.0 + +Some linter rules don't really make sense to enforce post OPA 1.0, as they are now either enforced by OPA itself or +otherwise no longer relevant. The following rules are now disabled by default, unless Regal is configured to target +Rego versions before 1.0, or in the case where no configuration is provided, Regal determines that the project being +linted is not yet using OPA 1.0: + +- [deprecated-builtin](https://openpolicyagent.org/projects/regal/rules/bugs/deprecated-builtin) +- [import-shadows-import](https://openpolicyagent.org/projects/regal/rules/imports/import-shadows-import) +- [rule-named-if](https://openpolicyagent.org/projects/regal/rules/bugs/rule-named-if) +- [use-contains](https://openpolicyagent.org/projects/regal/rules/idiomatic/use-contains) +- [use-if](https://openpolicyagent.org/projects/regal/rules/idiomatic/use-if) +- [use-rego-v1](https://openpolicyagent.org/projects/regal/rules/imports/use-rego-v1) + +Except for the `deprecated-bultin` rule — which is disabled simply because there currently are no deprecated built-ins +in OPA 1.0 — these rules are now enforced automatically by OPA, and so there's no reason for Regal to duplicate that +effort. + +## Related Resources + +- OPA Docs: [Upgrading to v1.0](https://www.openpolicyagent.org/docs/v0-upgrade/) +- OPA Docs: [v0 Backwards Compatibility](https://www.openpolicyagent.org/docs/v0-compatibility/) +- Styra Blog: [Renovating Rego](https://www.styra.com/blog/renovating-rego/) +- OPA Blog: [OPA 1.0 Is Coming, Here's What You Need to Know](https://blog.openpolicyagent.org/opa-1-0-is-coming-heres-what-you-need-to-know-c8fb0d258368) +- OPA Blog: [Announcing OPA 1.0: A New Standard for Policy as Code](https://blog.openpolicyagent.org/announcing-opa-1-0-a-new-standard-for-policy-as-code-a6d8427ee828) diff --git a/docs/projects/regal/pre-commit-hooks.md b/docs/projects/regal/pre-commit-hooks.md new file mode 100644 index 0000000000..7df85bc2e3 --- /dev/null +++ b/docs/projects/regal/pre-commit-hooks.md @@ -0,0 +1,47 @@ +--- +sidebar_position: 6 +--- + + +# Pre-Commit Hooks + +[Pre-Commit](https://pre-commit.com) is a framework for managing and maintaining multi-language pre-commit hooks. +This allows running Regal automatically whenever (and as the name implied, before )a Rego file is about to be committed. + +To use Regal with pre-commit, add this to your `.pre-commit-config.yaml` + +```yaml +- repo: https://github.com/open-policy-agent/regal + rev: v0.7.0 # Use the ref you want to point at + hooks: + - id: regal-lint + # - id: ... +``` + +## Hooks Available + +### `regal-lint` + +![commit-msg hook](https://img.shields.io/badge/hook-pre--commit-informational?logo=git) + +Runs Regal against all staged `.rego` files, aborting the commit if any fail. + +- requires the `go` build chain is installed and available on `$PATH` +- will build and install the tagged version of Regal in an isolated `GOPATH` +- ensures compatibility between versions + +### `regal-lint-use-path` + +![commit-msg hook](https://img.shields.io/badge/hook-pre--commit-informational?logo=git) + +Runs Regal against all staged `.rego` files, aborting the commit if any fail. + +- requires the `regal` package is already installed and available on `$PATH`. + +### `regal-download` + +![commit-msg hook](https://img.shields.io/badge/hook-pre--commit-informational?logo=git) + +Runs Regal against all staged `.rego` files, aborting the commit if any fail. + +- Downloads the latest `regal` binary from Github. diff --git a/docs/projects/regal/remote-features.md b/docs/projects/regal/remote-features.md new file mode 100644 index 0000000000..e7731eae5d --- /dev/null +++ b/docs/projects/regal/remote-features.md @@ -0,0 +1,32 @@ +--- +sidebar_position: 11 +--- + + +# Remote Features + +This page outlines the features of Regal that need internet access to function. + +## Checking for Updates + +Regal will check for updates on startup. If a new version is available, +Regal will notify you by writing a message in stderr. + +An example of such a message is: + +```txt +A new version of Regal is available (v0.23.1). You are running v0.23.0. +See https://github.com/open-policy-agent/regal/releases/tag/v0.23.1 for the latest release. +``` + +This message is based on the local version set in the Regal binary, and **no +user data is sent** to GitHub where the releases are hosted. + +This same function will also write to the file at: `$HOME/.config/regal/latest_version.json`, +this is used as a cache of the latest version to avoid consuming excessive +GitHub API rate limits when using Regal. + +This functionality can be disabled in two ways: + +* Using `.regal/config.yaml` / `.regal.yaml`: set `features.remote.check-version` to `false`. +* Using an environment variable: set `REGAL_DISABLE_CHECK_VERSION` to `true`. diff --git a/docs/projects/regal/rules/_category_.json b/docs/projects/regal/rules/_category_.json new file mode 100644 index 0000000000..f8f3c8357a --- /dev/null +++ b/docs/projects/regal/rules/_category_.json @@ -0,0 +1 @@ +{ "collapsed": false } diff --git a/docs/projects/regal/rules/bugs/annotation-without-metadata.md b/docs/projects/regal/rules/bugs/annotation-without-metadata.md new file mode 100644 index 0000000000..baeb022d26 --- /dev/null +++ b/docs/projects/regal/rules/bugs/annotation-without-metadata.md @@ -0,0 +1,49 @@ +# annotation-without-metadata + +**Summary**: Annotation without metadata + +**Category**: Bugs + +**Avoid** +```rego +package policy + +# description: allow allows +allow if { + # ... some conditions +} +``` + +**Prefer** +```rego +package policy + +# METADATA +# description: allow allows +allow if { + # ... some conditions +} +``` + +## Rationale + +A comment that starts with `:` but is not part of a metadata block is likely a mistake. Add +`# METADATA` above the line to turn it into a +[metadata](https://www.openpolicyagent.org/docs/policy-language/#annotations) block. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + annotation-without-metadata: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Annotations](https://www.openpolicyagent.org/docs/policy-language/#annotations) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/annotation-without-metadata/annotation_without_metadata.rego) diff --git a/docs/projects/regal/rules/bugs/argument-always-wildcard.md b/docs/projects/regal/rules/bugs/argument-always-wildcard.md new file mode 100644 index 0000000000..10c42ad175 --- /dev/null +++ b/docs/projects/regal/rules/bugs/argument-always-wildcard.md @@ -0,0 +1,88 @@ +# argument-always-wildcard + +**Summary**: Argument is always a wildcard + +**Category**: Bugs + +**Avoid** +```rego +package policy + +# there's only one definition of the last_name function in +# this package, and the second argument is never used +last_name(name, _) := lname if { + parts := split(name, " ") + lname := parts[count(parts) - 1] +} +``` + +**Prefer** +```rego +package policy + +last_name(name) := lname if { + parts := split(name, " ") + lname := parts[count(parts) - 1] +} +``` + +## Rationale + +Function definitions may use wildcard variables as arguments to indicate that the value is not used in the body of +the function. This helps make the function definition more readable, as it's immediately clear which of the arguments +are used in that definition of the function. This is particularly useful for incrementally defined functions: + +```rego +package policy + +default authorized(_, _) := false + +authorized(user, _) if { + # some logic to determine if authorized +} + +# or + +authorized(user, _) if { + # some further logic to determine if authorized +} +``` + +In the example above, the second argument is a wildcard in all definitions, and could just as well be removed for a +cleaner definition. More likely, the argument was meant to be _used_, if only in one of the definitions: + +```rego +package policy + +default authorized(_, _) := false + +authorized(user, _) if { + # some logic to determine if authorized +} + +# or + +authorized(_, request) if { + # some further logic to determine if authorized +} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + argument-always-wildcard: + # one of "error", "warning", "ignore" + level: error + # function name patterns for which this rule should make an exception + # default is to ignore any function name starting with "mock_" as these + # commonly don't need named arguments + except-function-name-pattern: "^mock_" +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/argument-always-wildcard/argument_always_wildcard.rego) diff --git a/docs/projects/regal/rules/bugs/constant-condition.md b/docs/projects/regal/rules/bugs/constant-condition.md new file mode 100644 index 0000000000..70b7ac2db4 --- /dev/null +++ b/docs/projects/regal/rules/bugs/constant-condition.md @@ -0,0 +1,42 @@ +# constant-condition + +**Summary**: Constant condition + +**Category**: Bugs + +**Avoid** +```rego +package policy + +allow if { + 1 == 1 +} +``` + +**Prefer** +```rego +package policy + +allow := true +``` + +## Rationale + +While most often a mistake, constant conditions are sometimes used as placeholders, or "TODO logic". While this is +harmless, it has no place in production policy, and should be replaced or removed before deployment. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + constant-condition: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/constant-condition/constant_condition.rego) diff --git a/docs/projects/regal/rules/bugs/deprecated-builtin.md b/docs/projects/regal/rules/bugs/deprecated-builtin.md new file mode 100644 index 0000000000..3148cf2954 --- /dev/null +++ b/docs/projects/regal/rules/bugs/deprecated-builtin.md @@ -0,0 +1,141 @@ +# deprecated-builtin + +**Summary**: Constant condition + +**Category**: Bugs + +## Notice: Rule disabled with OPA 1.0 + +Since Regal v0.30.0, this rule is only enabled for projects that have either been explicitly configured to target +versions of OPA before 1.0, or if no configuration is provided — where Regal is able to determine that an older version +of OPA/Rego is being targeted. Consult the documentation on Regal's +[configuration](https://openpolicyagent.org/projects/regal#configuration) for information on how to best work with older versions of +OPA and Rego. + +Since OPA v1.0, this rule is automatically disabled, as there currently are no deprecated built-in functions +in that version, and trying to use a previously deprecated function will result in a parser error. Note however that +this may change if later OPA versions deprecate current built-in functions. If/when that happens, this rule will be +re-enabled. + +**Avoid** +```rego +package policy + +import future.keywords.if + +# call to deprecated `any` built-in function +allow if any([input.user.is_admin, input.user.is_root]) +``` + +**Prefer** +```rego +package policy + +import future.keywords.if + +allow if input.user.is_admin +allow if input.user.is_root +``` + +## Rationale + +Calling deprecated built-in functions should always be avoided, and replacing them is usually trivial. + +## Replacing Deprecated Built-in Functions + +### `any` + +Use the `in` keyword (OPA v0.34.0+) to replace the `any` function: + +**Instead of** +```rego +a := any([input.foo, input.bar]) +``` + +**Do this** +```rego +import future.keywords.in # or `import rego.v1` (OPA v0.59.0+) + +a := true in [input.foo, input.bar] +``` + +Using `in` additionally has the benefit that it can be used to check for any type of value, and not just boolean +`true`! + +### `all` + +Use the `every` keyword (OPA v0.34.0+) to replace the `all` function: + +**Instead of** +```rego +a { + all([input.foo, input.bar]) +} +``` + +**Do this** +```rego +import future.keywords.every # or `import rego.v1` (OPA v0.59.0+) + +a { + every x in [input.foo, input.bar] { + x == true + } +} +``` + +Just like `in` may be used for much more than `any`, `every` can be used to evaluate complex expressions! + +### `set_diff` + +Use the minus (`-`) operator instead, of `set_diff`: + +**Instead of** +```rego +a := set_diff(s1, s2) +``` + +**Do this** +```rego +a := s1 - s2 +``` + +### `re_match` and `net.cidr_overlap` + +These built-in function were renamed `regex.match` and `net.cidr_intersects` respectively, so simply use the new names +instead. + +### `cast_array`, `cast_set`, `cast_string`, `cast_boolean`, `cast_null`, `cast_object` + +Use the `is_X` equivalent built-in function in their place: + +**Instead of** +```rego +a { + cast_string(input.name) +} +``` + +**Do this** +```rego +a { + is_string(input.name) +} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + deprecated-builtin: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Strict Mode](https://www.openpolicyagent.org/docs/policy-language/#strict-mode) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/deprecated-builtin/deprecated_builtin.rego) diff --git a/docs/projects/regal/rules/bugs/duplicate-rule.md b/docs/projects/regal/rules/bugs/duplicate-rule.md new file mode 100644 index 0000000000..a522d426fa --- /dev/null +++ b/docs/projects/regal/rules/bugs/duplicate-rule.md @@ -0,0 +1,55 @@ +# duplicate-rule + +**Summary**: Duplicate rule + +**Category**: Bugs + +**Avoid** +```rego +package policy + +allow if user.is_admin + +allow if user.is_developer + +# we already covered this! +allow if user.is_admin +``` + +**Prefer** +```rego +package policy + +allow if user.is_admin + +allow if user.is_developer +``` + +## Rationale + +Duplicated rules are likely a mistake, perhaps from pasting contents from another file. + +This rule identifies rules that are _identical_ in terms of their name, assigned value, and body — excluding +whitespace. In technical terms, if two or more rules share the same abstract syntax tree, they are considered +to be duplicates. + +## Exceptions + +Note that this rule currently works at the scope of a single file. If you're using the same package across multiple +files, there could still be duplicates across those files. This will be addressed in a future version of this rule. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + duplicate-rule: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/duplicate-rule/duplicate_rule.rego) diff --git a/docs/projects/regal/rules/bugs/if-empty-object.md b/docs/projects/regal/rules/bugs/if-empty-object.md new file mode 100644 index 0000000000..c8aa2eecb4 --- /dev/null +++ b/docs/projects/regal/rules/bugs/if-empty-object.md @@ -0,0 +1,40 @@ +# if-empty-object + +**This rule has been deprecated and replaced by the +[if-object-literal](https://openpolicyagent.org/projects/regal/rules/bugs/if-object-literal) rule. Documentation kept here only for +the sake of posterity.** + +**Summary**: Empty object following `if` + +**Category**: Bugs + +**Avoid** +```rego +package policy + +allow if {} +``` + +## Rationale + +An empty rule body would previously be considered an error by OPA. With the introduction, and use of the `if` keyword, +that is no longer the case. In fact, empty `{}` is not considered a rule body _at all_, but rather an empty object, +meaning that `if {}` will always evaluate. This is likely a mistake, and while hopefully caught by tests, should be +avoided. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + if-empty-object: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [constant-condition](https://openpolicyagent.org/projects/regal/rules/bugs/constant-condition) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/if-empty-object/if_empty_object.rego) diff --git a/docs/projects/regal/rules/bugs/if-object-literal.md b/docs/projects/regal/rules/bugs/if-object-literal.md new file mode 100644 index 0000000000..b79403b79c --- /dev/null +++ b/docs/projects/regal/rules/bugs/if-object-literal.md @@ -0,0 +1,41 @@ +# if-object-literal + +**Summary**: Object literal following `if` + +**Category**: Bugs + +**Avoid** +```rego +package policy + +# {} interpreted as object, not a rule body +allow if {} + +allow if { + # perhaps meant to be comparison? + # but this too is an object + input.x: 10 +} +``` + +## Rationale + +An object literal immediately following an `if` is almost certainly a mistake, and the intention was likely to express +a rule body in its place. This isn't too common, but can happen when either an empty object `{}` is all that follows the +`if`, or an expression in the "body" mistakenly is written as a `key: value` pair. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + if-object-literal: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/if-object-literal/if_object_literal.rego) diff --git a/docs/projects/regal/rules/bugs/import-shadows-rule.md b/docs/projects/regal/rules/bugs/import-shadows-rule.md new file mode 100644 index 0000000000..74ab29bcbe --- /dev/null +++ b/docs/projects/regal/rules/bugs/import-shadows-rule.md @@ -0,0 +1,58 @@ +# import-shadows-rule + +**Summary**: Import shadows rule + +**Category**: Bugs + +**Avoid** +```rego +package policy + +import data.resources + +# 'resources' shadowed by import +resources contains resource if { + # ... +} +``` + +**Prefer** +```rego +package policy + +import data.resources + +# using a different name for the rule +report contains resource if { + # ... +} +``` + +```rego +package policy + +# using an alias to avoid shadowing 'resources' rule +import data.resources as inventory + +resources contains resource if { + # ... +} +``` + +## Rationale + +Imported identifers like `bar` in `import data.foo.bar` has higher precedence than a rule named `bar` in the same +package. This means that any rule that is shadowed by an import is effectively unreachable inside of the module. +Avoid shadowing either by renaming your rule or by using an alias for the imported identifier. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + import-shadows-rule: + # one of "error", "warning", "ignore" + level: error +``` diff --git a/docs/projects/regal/rules/bugs/impossible-not.md b/docs/projects/regal/rules/bugs/impossible-not.md new file mode 100644 index 0000000000..506e1dc63f --- /dev/null +++ b/docs/projects/regal/rules/bugs/impossible-not.md @@ -0,0 +1,72 @@ +# impossible-not + +**Summary**: Impossible `not` condition + +**Category**: Bugs + +**Type**: Aggregate - runs both on single files as well as when more than one file is provided for linting + +**Avoid** +```rego +package policy + +report contains violation if { + # ... some conditions +} +``` + +```rego +package policy_test + +import data.policy + +test_report_is_empty { + # evaluation will stop here, as even an empty set is "true" + not policy.report +} +``` + +**Prefer** +```rego +package policy + +report contains violation if { + # ... some conditions +} +``` + +```rego +package policy_test + +import data.policy + +test_report_is_empty { + count(policy.report) == 0 +} +``` + +## Rationale + +The `not` keyword negates the expression that follows it. A common mistake, especially in tests, is to use `not` +to test the result of evaluating a partial (i.e. multi-value) rule. However, as even an empty set is considered +"truthy", the `not` will in that case always evaluate to `false`. There are more cases where `not` is impossible, +or a [constant condition](https://openpolicyagent.org/projects/regal/rules/bugs/constant-condition), but references to partial +rules are by far the most common. For tests where you want to assert the set is empty or has a specific number of +items, use the built-in `count` function instead. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + impossible-not: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [constant-condition](https://openpolicyagent.org/projects/regal/rules/bugs/constant-condition) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/impossible-not/impossible_not.rego) diff --git a/docs/projects/regal/rules/bugs/inconsistent-args.md b/docs/projects/regal/rules/bugs/inconsistent-args.md new file mode 100644 index 0000000000..8e263482ff --- /dev/null +++ b/docs/projects/regal/rules/bugs/inconsistent-args.md @@ -0,0 +1,72 @@ +# inconsistent-args + +**Summary**: Inconsistently named function arguments + +**Category**: Bugs + +**Avoid** +```rego +package policy + +find_vars(rule, node) if node in rule + +# Order of arguments changed, or at least it looks like it +find_vars(node, rule) if { + walk(rule, [path, value]) + # ... +} +``` + +**Prefer** +```rego +package policy + +find_vars(rule, node) if node in rule + +find_vars(rule, node) if { + walk(rule, [path, value]) + # ... +} +``` + +## Rationale + +Whenever a custom function declaration is repeated, the argument names should remain consistent in each declaration. + +Inconsistently named function arguments is a likely source of bugs, and should be avoided. + +## Exceptions + +Using wildcards (`_`) in place of unused arguments is always allowed, and in fact enforced by the compiler: + +```rego +package policy + +find_vars(rule, node) if node in rule + +# We don't use `node` here +find_vars(rule, _) if { + walk(rule, [path, value]) + # ... +} +``` + +Using [pattern matching for equality](https://openpolicyagent.org/projects/regal/rules/idiomatic/equals-pattern-matching) checks is +also allowed. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + inconsistent-args: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [equals-pattern-matching](https://openpolicyagent.org/projects/regal/rules/idiomatic/equals-pattern-matching) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/inconsistent-args/inconsistent_args.rego) diff --git a/docs/projects/regal/rules/bugs/index.md b/docs/projects/regal/rules/bugs/index.md new file mode 100644 index 0000000000..7c0538d783 --- /dev/null +++ b/docs/projects/regal/rules/bugs/index.md @@ -0,0 +1,14 @@ +--- +title: Bugs +sidebar_position: 1 +--- + + +# Bugs + +Rules that detect bugs in your code. + +import RulesTable from '@site/src/components/projects/regal/RulesTable'; + + + diff --git a/docs/projects/regal/rules/bugs/index.md.yaml b/docs/projects/regal/rules/bugs/index.md.yaml new file mode 100644 index 0000000000..31949a7f91 --- /dev/null +++ b/docs/projects/regal/rules/bugs/index.md.yaml @@ -0,0 +1,2 @@ +title: Bugs +sidebar_position: 1 diff --git a/docs/projects/regal/rules/bugs/internal-entrypoint.md b/docs/projects/regal/rules/bugs/internal-entrypoint.md new file mode 100644 index 0000000000..cc89722a5f --- /dev/null +++ b/docs/projects/regal/rules/bugs/internal-entrypoint.md @@ -0,0 +1,53 @@ +# internal-entrypoint + +**Summary**: Entrypoint can't be marked internal + +**Category**: Bugs + +**Avoid** +```rego +package policy + +# METADATA +# entrypoint: true +_authorized if { + # some conditions +} +``` + +**Prefer** +```rego +package policy + +# METADATA +# entrypoint: true +allow if _authorized + +_authorized if { + # some conditions +} +``` + +## Rationale + +Rules marked as internal using the [underscore prefix convention](https://github.com/StyraInc/rego-style-guide#optionally-use-leading-underscore-for-rules-intended-for-internal-use) +cannot be used as entrypoints, as entrypoints by definition are public. Either rename the rule to mark it as public, +or use another public rule as an entrypoint, which may reference the internal rule. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + internal-entrypoint: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Optionally, use leading underscore for rules intended for internal use](https://github.com/StyraInc/rego-style-guide#optionally-use-leading-underscore-for-rules-intended-for-internal-use) +- Regal Docs: [no-defined-entrypoint](https://openpolicyagent.org/projects/regal/rules/idiomatic/no-defined-entrypoint) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/internal-entrypoint/internal_entrypoint.rego) diff --git a/docs/projects/regal/rules/bugs/invalid-metadata-attribute.md b/docs/projects/regal/rules/bugs/invalid-metadata-attribute.md new file mode 100644 index 0000000000..3716a696a9 --- /dev/null +++ b/docs/projects/regal/rules/bugs/invalid-metadata-attribute.md @@ -0,0 +1,53 @@ +# invalid-metadata-attribute + +**Summary**: Invalid attribute in metadata annotation + +**Category**: Bugs + +**Avoid** +```rego +# METADATA +# title: Main policy routing requests to other policies based on input +# category: Routing +package router +``` + +**Prefer** +```rego +# METADATA +# title: Main policy routing requests to other policies based on input +# custom: +# category: Routing +package router +``` + +## Rationale + +Metadata comments should follow the schema expected by +[annotations](https://www.openpolicyagent.org/docs/policy-language/#annotations). Custom attributes, like +`category` above, should be placed under the `custom` key, which is a map of arbitrary key-value pairs. + +While arbitrary attributes are accepted, they will not be treated as metadata annotations but regular comments, and as +such won't be available to other tools that +[process annotations](https://www.openpolicyagent.org/docs/policy-language/#accessing-annotations). +These tools include built-in functions like +[rego.metadata.rule](https://www.openpolicyagent.org/docs/policy-reference/#builtin-rego-regometadatarule) and +[rego.metadata.chain](https://www.openpolicyagent.org/docs/policy-reference/#builtin-rego-regometadatachain). + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + invalid-metadata-attribute: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Annotations](https://www.openpolicyagent.org/docs/policy-language/#annotations) +- OPA Docs: [Accessing Annotations](https://www.openpolicyagent.org/docs/policy-language/#accessing-annotations) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/invalid-metadata-attribute/invalid_metadata_attribute.rego) diff --git a/docs/projects/regal/rules/bugs/leaked-internal-reference.md b/docs/projects/regal/rules/bugs/leaked-internal-reference.md new file mode 100644 index 0000000000..1f56e9754d --- /dev/null +++ b/docs/projects/regal/rules/bugs/leaked-internal-reference.md @@ -0,0 +1,72 @@ +# leaked-internal-reference + +**Summary**: Outside reference to internal rule or function + +**Category**: Bugs + +**Avoid** + +```rego +package policy + +# Import of rule or functions marked as internal +import data.users._all_users + +allow if { + # reference to rule or function marked as internal + some role in data.permissions._roles + # ...some conditions +} +``` + +## Rationale + +OPA doesn't have a concept of "internal", or private rules and functions — and all rules can be queried or referenced +from the outside. Despite this fact, it has become a common convention to use an underscore prefix in the name of +rules and functions to indicate that they should be considered internal to the package that they're in: + +```rego +# `allow` may be referenced from outside the package +allow if _user_is_developer + +# `_user_is_developer` should not be referenced from outside the package +_user_is_developer if "developer" in input.users.roles +``` + +While this might seem like a pointless convention if it isn't enforced by OPA, it comes with a number of benefits: + +- While OPA doesn't enforce it, other tools like linters can help with that. Like this rule does! +- It clearly communicates intent to other policy authors, and as a simple form of documentation +- Completion suggestions in editors can be filtered to exclude internal rules and functions +- Tools that render documentation from Rego policies and metadata annotations can exclude internal rules and functions +- Checking for unused rules and functions can be done much faster if they're known not to be referenced from outside + +Do note that if you disagree with this rule, you don't need to disable it unless you use underscore prefixes to mean +something else. If you don't use underscore prefixes, nothing will be reported by this rule anyway. It does however +mean that the benefits listed above won't apply to your project. + +## Exceptions + +This rule is not enabled by default for test files. In tests, it can be useful +to reference internal rules and functions to achieve good test coverage, which +would be a violation of this rule. If you want to run this rule for tests +too, you can set `include-test-files: true` in the configuration for this rule +in your Regal config file. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + leaked-internal-reference: + # one of "error", "warning", "ignore" + level: error + include-test-files: false # default is false +``` + +## Related Resources + +- Rego Style Guide: [Optionally, use leading underscore for rules intended for internal use](https://github.com/StyraInc/rego-style-guide#optionally-use-leading-underscore-for-rules-intended-for-internal-use) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/leaked-internal-reference/leaked_internal_reference.rego) diff --git a/docs/projects/regal/rules/bugs/not-equals-in-loop.md b/docs/projects/regal/rules/bugs/not-equals-in-loop.md new file mode 100644 index 0000000000..759a1247d3 --- /dev/null +++ b/docs/projects/regal/rules/bugs/not-equals-in-loop.md @@ -0,0 +1,66 @@ +# not-equals-in-loop + +**Summary**: Use of != in loop + +**Category**: Bugs + +**Avoid** +```rego +package policy + +deny if { + "admin" != input.user.roles[_] +} +``` + +**Prefer** +```rego +package policy + +deny if { + not "admin" in input.user.roles +} + +# Or as a one-liner +deny if not "admin" in input.user.roles +``` + +## Rationale + +Likely one of the most common mistakes in Rego is to use `!=` in a loop thinking it means "not in". It took some years +for the `in` keyword to be added to Rego, so perhaps it's not surprising that this mistake is a common one even to this +day. If it doesn't mean "not in", what does it mean? + +```rego +package policy + +deny if { + "admin" != input.user.roles[_] +} +``` + +The body of the `deny` rule above roughly translates to "for any item in `input.user.roles`, return true if the item is +not `admin`". This is almost never what the policy author intended. What the policy author likely intended was +"deny if `admin` is not in `input.user.roles`". The above policy would thus **not** deny a user with the roles +`["user", "admin"]` since the first item in the array is not "admin". This is almost never what the policy author +intended. + +**Note**: This linter rule currently only checks for `!=` in a non-nested comparison where iteration happens on either +side of the comparison in the same expression. This will be improved in time. Another limitation is that this rule +currently only checks for wildcard iteration (`[_]`). + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + not-equals-in-loop: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/not-equals-in-loop/not_equals_in_loop.rego) diff --git a/docs/projects/regal/rules/bugs/redundant-existence-check.md b/docs/projects/regal/rules/bugs/redundant-existence-check.md new file mode 100644 index 0000000000..9ebbb9253f --- /dev/null +++ b/docs/projects/regal/rules/bugs/redundant-existence-check.md @@ -0,0 +1,87 @@ +# redundant-existence-check + +**Summary**: Redundant existence check + +**Category**: Bugs + +**Avoid** +```rego +package policy + +employee if { + input.user.email + endswith(input.user.email, "@acmecorp.com") +} + +is_admin(user) if { + user + "admin" in user.roles +} +``` + +**Prefer** +```rego +package policy + +employee if { + endswith(input.user.email, "@acmecorp.com") +} + +# alternatively + +employee if endswith(input.user.email, "@acmecorp.com") + +is_admin(user) if { + "admin" in user.roles +} +``` + +## Rationale + +Checking that a reference (like `input.user.email`) is defined before immediately using it is redundant. If the +reference is undefined, the next expression will fail anyway, as the value will be checked before the rest of the +expression is evaluated. While an extra check doesn't "hurt", it also serves no purpose, similarly to an unused +variable. + +**Note**: This rule only applies to references that are immediately used in the next expression. If the reference is +used later in the rule, it won't be flagged. While the existence check _could_ be redundant even in that case, it could +also be used to avoid making some expensive computation, an `http.send` call, or whatnot. + +## Exceptions + +Function arguments where a boolean value is expected will be flagged as redundant existence checks, even though the +intent was to check the boolean condition. + +```rego +report(user, is_admin) if { + is_admin + + # more conditions +} +``` + +For these cases, prefer to be explicit about what the assertion is checking: + +```rego +report(user, is_admin) if { + is_admin == false # or true, != false, etc. + + # more conditions +} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + redundant-existence-check: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/redundant-existence-check/redundant_existence_check.rego) diff --git a/docs/projects/regal/rules/bugs/redundant-loop-count.md b/docs/projects/regal/rules/bugs/redundant-loop-count.md new file mode 100644 index 0000000000..4d2e74a869 --- /dev/null +++ b/docs/projects/regal/rules/bugs/redundant-loop-count.md @@ -0,0 +1,82 @@ +# redundant-loop-count + +**Summary**: Redundant count before loop + +**Category**: Bugs + +**Avoid** +```rego +package policy + +allow if { + # redundant count and > comparison + count(input.user.roles) > 0 + some role in input.user.roles + # .. do more with role .. +} +``` + +**Prefer** +```rego +package policy + +allow if { + some role in input.user.roles + # .. do more with role .. +} +``` + +## Rationale + +A loop that iterates over an empty collection evaluates to nothing, and counting the collection before the loop to +ensure it's not empty is therefore redundant. + +## Exceptions + +Note that this check is currently only performed on `some` loops, and not "ref-style" loops: + +```rego +package policy + +allow if { + # this won't be flagged + count(input.user.roles) > 0 + role := input.user.roles[_] + # .. do more with role .. +} +``` + +Another good reason to +[prefer some .. in for iteration](https://openpolicyagent.org/projects/regal/rules/style/prefer-some-in-iteration)! + +### `every` iteration + +Counting to ensure a non-empty collection is used before `every` loops may **not** be redundant, as `every` evaluates +to `true` when an empty collection is passed. + +```rego +package policy + +allow if { + # every would otherwise be `true` on empty input.user.roles + # so this may be valid, depending on the outcome you expect + count(input.user.roles) > 0 + every role in input.user.roles { + # .. do more with each role .. + } +} +``` + +If you want to have empty collections fail on `every` conditions, do make sure to use `count`! + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + redundant-loop-count: + # one of "error", "warning", "ignore" + level: error +``` diff --git a/docs/projects/regal/rules/bugs/rule-assigns-default.md b/docs/projects/regal/rules/bugs/rule-assigns-default.md new file mode 100644 index 0000000000..a5ccc38713 --- /dev/null +++ b/docs/projects/regal/rules/bugs/rule-assigns-default.md @@ -0,0 +1,51 @@ +# rule-assigns-default + +**Summary**: Rule assigned its default value + +**Category**: Bugs + +**Avoid** +```rego +package policy + +default allow := false + +# this rule assigns the same value as the default +# and the policy would work the same without it +allow := false if { + not "admin" in input.user.roles +} +``` + +**Prefer** +```rego +package policy + +default allow := false + +# or just `allow if {` as `true` is implicit +allow := true if { + "admin" in input.user.roles +} +``` + +## Rationale + +When a default value is used for a rule, assigning the same value anywhere else to that rule is pointless, as the rule +would evaluate to the same value with or without the assignment. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + rule-assigns-default: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/rule-assigns-default/rule_assigns_default.rego) diff --git a/docs/projects/regal/rules/bugs/rule-named-if.md b/docs/projects/regal/rules/bugs/rule-named-if.md new file mode 100644 index 0000000000..4860ca41ca --- /dev/null +++ b/docs/projects/regal/rules/bugs/rule-named-if.md @@ -0,0 +1,71 @@ +# rule-named-if + +**Summary**: Rule named `if` + +**Category**: Bugs + +## Notice: Rule made obsolete by OPA 1.0 + +Since Regal v0.30.0, this rule is only enabled for projects that have either been explicitly configured to target +versions of OPA before 1.0, or if no configuration is provided — where Regal is able to determine that an older version +of OPA/Rego is being targeted. Consult the documentation on Regal's +[configuration](https://openpolicyagent.org/projects/regal#configuration) for information on how to best work with older versions of +OPA and Rego. + +Since OPA v1.0, this rule is automatically disabled, as the parser itself will throw an error if a rule is named `if`, +as that is made a keyword in Rego v1.0. + +**Avoid** +```rego +package policy + +allow := true if { + authorized +} +``` + +Which actually means: + +```rego +package policy + +allow := true + +if { + authorized +} +``` + +**Prefer** +```rego +package policy + +import rego.v1 + +allow := true if { + authorized +} +``` + +## Rationale + +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 + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + rule-named-if: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/rule-named-if/rule_named_if.rego) diff --git a/docs/projects/regal/rules/bugs/rule-shadows-builtin.md b/docs/projects/regal/rules/bugs/rule-shadows-builtin.md new file mode 100644 index 0000000000..2d981044c1 --- /dev/null +++ b/docs/projects/regal/rules/bugs/rule-shadows-builtin.md @@ -0,0 +1,40 @@ +# rule-shadows-builtin + +**Summary**: Rule name shadows built-in + +**Category**: Bugs + +**Avoid** +```rego +package policy + +# `or` is an operator +or := 1 + 1 + +# `startswith` is a built-in function +startswith := indexof("rego", "r") +``` + +## Rationale + +Using the name of built-in functions or operators as rule and variable names can lead to confusion and unexpected +behavior. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + rule-shadows-builtin: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Built-in Functions](https://www.openpolicyagent.org/docs/policy-reference/#built-in-functions) +- OPA Repo: [builtin_metadata.json](https://github.com/open-policy-agent/opa/blob/main/builtin_metadata.json) +- Regal Docs: [var-shadows-builtin](https://openpolicyagent.org/projects/regal/rules/bugs/var-shadows-builtin) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/rule-shadows-builtin/rule_shadows_builtin.rego) diff --git a/docs/projects/regal/rules/bugs/sprintf-arguments-mismatch.md b/docs/projects/regal/rules/bugs/sprintf-arguments-mismatch.md new file mode 100644 index 0000000000..bd21cd71a2 --- /dev/null +++ b/docs/projects/regal/rules/bugs/sprintf-arguments-mismatch.md @@ -0,0 +1,64 @@ +# sprintf-arguments-mismatch + +**Summary**: Mismatch in `sprintf` arguments count + +**Category**: Bugs + +**Avoid** +```rego +package policy + +max_issues := 1 + +report contains warning if { + count(issues) > max_issues + + # two placeholders found in the string, but only one value in the array + warning := sprintf("number of issues (%d) must not be higher than %d", [count(issues)]) +} +``` + +**Prefer** +```rego +package policy + +max_issues := 1 + +report contains warning if { + count(issues) > max_issues + + # two placeholders found in the string, and two values in the array + warning := sprintf("number of issues (%d) must not be higher than %d", [count(issues), max_issues]) +} +``` + +## Rationale + +While the built-in `sprintf` function itself reports argument mismatches, it'll do so by returning a string containing +the error message rather than actually failing. + +```shell +> opa eval -f pretty 'sprintf("%v %d", [1])' +"1 %!d(MISSING)" +``` + +While this is normally caught in development and testing, having this issue reported at "compile time", which ideally +is [directly in your editor](https://openpolicyagent.org/projects/regal/language-server) as you work on your policy. This means less +time spent chasing down issues later, and a happier development experience. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + sprintf-arguments-mismatch: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Built-in Functions: `sprintf`](https://www.openpolicyagent.org/docs/policy-reference/#builtin-strings-sprintf) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch.rego) diff --git a/docs/projects/regal/rules/bugs/time-now-ns-twice.md b/docs/projects/regal/rules/bugs/time-now-ns-twice.md new file mode 100644 index 0000000000..128e528aa9 --- /dev/null +++ b/docs/projects/regal/rules/bugs/time-now-ns-twice.md @@ -0,0 +1,64 @@ +# time-now-ns-twice + +**Summary**: Repeated calls to `time.now_ns` + +**Category**: Bugs + +**Avoid** +```rego +package policy + +timed if { + now := time.now_ns() + + # do some work here + + # this doesn't work! result is always 0 + print("work done in:", time.now_ns() - now, "ns) +} +``` + +**Prefer** + +To use the tools OPA provides for measuring performance. + +## Rationale + +An important property of Rego is that it makes policy evaluation _predictable_. Using the same input to query OPA for a +decision multiple times should result in the same decision being made each time! A few built-in functions, like +`http.send`, or [time.now_ns](https://www.openpolicyagent.org/docs/policy-reference/#builtin-time-timenow_ns) are +however not **deterministic**. This means that repeated queries to policies where such functions are used may result in +different decisions being made. For example, a policy that validates JSON Web Tokens would normally check if the current +time is past the expiry value of the token, and deny any request where a token is found to be expired. + +But while the use of non-deterministic built-in functions may result in different outcomes across different +queries, all built-in functions are deterministic **within the scope of a single evaluation**. This means that calling +e.g. `http.send` twice in a policy using the exact same arguments never results in different values being returned. +This is equally true for `time.now_ns`. In order to ensure predictable evaluation, the time returned by `time.now_ns` is +set once at the start of the evaluation, and never changes for the course of the request. Calling `time.now_ns` several +times within a rule is thus pointless, as the same value will be returned each time. + +This mistake is most commonly observed when developers try to measure elapsed time in some parts of their policy, the +same way they'd normally do it using a traditional programming language (that is not deterministic). While this won't +work, OPA provides several tools to help measure performance, and learning how to use them well is the best way to +understand the performance characteristics of policy evaluation. + +See the [performance](https://www.openpolicyagent.org/docs/policy-performance/) section of the OPA docs for an +introduction to these tools, as well as advice on how to write performant policies. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + time-now-ns-twice: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [time.now_ns](https://www.openpolicyagent.org/docs/policy-reference/#builtin-time-timenow_ns) +- OPA Docs: [Policy Performance](https://www.openpolicyagent.org/docs/policy-performance/) diff --git a/docs/projects/regal/rules/bugs/top-level-iteration.md b/docs/projects/regal/rules/bugs/top-level-iteration.md new file mode 100644 index 0000000000..3ed6381034 --- /dev/null +++ b/docs/projects/regal/rules/bugs/top-level-iteration.md @@ -0,0 +1,42 @@ +# top-level-iteration + +**Summary**: Iteration in top-level assignment + +**Category**: Bugs + +**Avoid** +```rego +package policy + +user := input.users[_] +``` + +## Rationale + +While OPA allows this construct — it probably shouldn't. Performing iteration outside of a rule or function body +doesn't make any sense, and traversing **any** collection containing more than one item in this context will result +in an error: + +```shell +eval_conflict_error: complete rules must not produce multiple outputs +``` + +If the collection only contains a single item, the assignment will succeed, and the result will be the single element +assigned to the variable. As such, it is possible that a policy passing all tests still will fail when provided real +data. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + top-level-iteration: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/top-level-iteration/top_level_iteration.rego) diff --git a/docs/projects/regal/rules/bugs/unassigned-return-value.md b/docs/projects/regal/rules/bugs/unassigned-return-value.md new file mode 100644 index 0000000000..d694c59216 --- /dev/null +++ b/docs/projects/regal/rules/bugs/unassigned-return-value.md @@ -0,0 +1,50 @@ +# unassigned-return-value + +**Summary**: Non-boolean return value unassigned + +**Category**: Bugs + +**Avoid** +```rego +package policy + +allow if { + # return value not assigned + lower(input.user.name) + # ... +} +``` + +**Prefer** +```rego +package policy + +allow if { + # return value assigned + name_lower := lower(input.user.name) + # ... +} +``` + +## Rationale + +Calling a built-in function that returns a non-boolean value without actually assigning the returned value is almost +always a mistake. Only return of `false` or undefined will cause evaluation to halt, so a function that e.g. always +returns a string will always be evaluated as "truthy". But more importantly — not handling the return value in that case +is almost certainly a mistake. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + unassigned-return-value: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value.rego) diff --git a/docs/projects/regal/rules/bugs/unused-output-variable.md b/docs/projects/regal/rules/bugs/unused-output-variable.md new file mode 100644 index 0000000000..88d80ed4b0 --- /dev/null +++ b/docs/projects/regal/rules/bugs/unused-output-variable.md @@ -0,0 +1,98 @@ +# unused-output-variable + +**Summary**: Unused output variable + +**Category**: Bugs + +**Avoid** +```rego +package policy + +allow if { + some x + role := input.user.roles[x] + + # do something with "role", but not "x" +} +``` + +**Prefer** +```rego +package policy + +allow if { + # don't declare `x` output var as it is redundant + role := input.user.roles[_] + + # do something with "role" +} + +# or better (see prefer-some-in-iteration rule) + +allow if { + some role in input.user.roles + + # do something with "role" +} + +# or actually _use_ value bound to `x` somewhere, like in another +# reference, function call, etc + +allow if { + some x + input.user.roles[x] == data.required_roles[x] +} +``` + +## Rationale + +Output variables are variables "automatically" bound to values during evaluation, most commonly in iteration. This is +a powerful feature of Rego that when used correctly can create concise but readable policies. However, output variables +that are declared but not later referenced are _effectively_ unused and should be replaced by wildcard variables (`_`), +or the use of `some .. in` iteration. + +OPA itself has two methods for detecting and reporting unused variables as errors — one when using `some`: + +```rego +allow if { + # `x` is never used in the body — this is a compiler error + some x + input.user.roles[role] + + role == "admin" +} +``` + +And a [strict mode](https://www.openpolicyagent.org/docs/policy-language/#strict-mode) check for unused +variables defined in assignment (`:=`), or as a function arguments: + +```rego +allow(role, required) { + required_roles := data.required_roles + + role == "admin" + + # `required` never used in body, and neither is `required_roles` + # both would be errors when strict mode is enabled +} +``` + +Neither of these methods however considers an unused output variable as "unused". + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + unused-output-variable: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [prefer-some-in-iteration](https://openpolicyagent.org/projects/regal/rules/style/prefer-some-in-iteration) +- OPA Docs: [Strict Mode](https://www.openpolicyagent.org/docs/policy-language/#strict-mode) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/unused-output-variable/unused_output_variable.rego) diff --git a/docs/projects/regal/rules/bugs/unused-return-value.md b/docs/projects/regal/rules/bugs/unused-return-value.md new file mode 100644 index 0000000000..d325ea7bb8 --- /dev/null +++ b/docs/projects/regal/rules/bugs/unused-return-value.md @@ -0,0 +1,10 @@ +--- +draft: true +--- + +# unused-return-value + +## Please Note + +This rule has been renamed to *unassigned-return-value* and can be found +[here](https://openpolicyagent.org/projects/regal/rules/bugs/unassigned-return-value). diff --git a/docs/projects/regal/rules/bugs/var-shadows-builtin.md b/docs/projects/regal/rules/bugs/var-shadows-builtin.md new file mode 100644 index 0000000000..7fe7e82b5d --- /dev/null +++ b/docs/projects/regal/rules/bugs/var-shadows-builtin.md @@ -0,0 +1,52 @@ +# var-shadows-builtin + +**Summary**: Variable shadows built-in + +**Category**: Bugs + +**Avoid** +```rego +package policy + +# variable `http` shadows `http.send` built-in function +allow if { + http := startswith(input.url, "http://") + # do something with http +} +``` + +**Prefer** +```rego +package policy + +# variable `is_http` doesn't shadow any built-in function +allow if { + is_http := startswith(input.url, "http://") + # do something with is_http +} +``` + +## Rationale + +Using the name of built-in functions or operators as variable names can lead to confusion and unexpected behavior. +A variable that shadows a built-in function (or the namespace of a function, like `http` in `http.send`) prevents any +function in that namespace to be used later in the rule. Avoid this! + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + var-shadows-builtin: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Built-in Functions](https://www.openpolicyagent.org/docs/policy-reference/#built-in-functions) +- OPA Repo: [builtin_metadata.json](https://github.com/open-policy-agent/opa/blob/main/builtin_metadata.json) +- Regal Docs: [rule-shadows-builtin](https://openpolicyagent.org/projects/regal/rules/bugs/rule-shadows-builtin) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/var-shadows-builtin/var_shadows_builtin.rego) diff --git a/docs/projects/regal/rules/bugs/zero-arity-function.md b/docs/projects/regal/rules/bugs/zero-arity-function.md new file mode 100644 index 0000000000..d4b2935f9f --- /dev/null +++ b/docs/projects/regal/rules/bugs/zero-arity-function.md @@ -0,0 +1,54 @@ +# zero-arity-function + +**Summary**: Avoid functions without args + +**Category**: Bugs + +**Avoid** +```rego +package policy + +first_user() := input.users[0] +``` + +**Prefer** +```rego +package policy + +first_user := input.users[0] +``` + +## Rationale + +Zero-arity functions, or functions without arguments, aren't treated as functions by Rego, but as regular rules. For +that reason, they should also be expressed as such. One potential benefit of using functions over rules is that +functions don't contribute to the +[document](https://www.openpolicyagent.org/docs/philosophy/#the-opa-document-model) when a package is evaluated, +and as such sometimes used to "hide" information from the result of evaluation. Whether this is a good practice or not, +it importantly *doesn't work* with zero-arity functions, as they are treated as rules and *do* contribute to the +document. + +There is an [open issue](https://github.com/open-policy-agent/opa/issues/6315) in the OPA project to try and address +this in the future, and allow zero-arity functions to be treated as other functions. Until then, the recommendation +is to avoid them and just use rules in their place. + +Note that if you're using the [opa fmt](https://openpolicyagent.org/projects/regal/rules/style/opa-fmt) command to format your code, +it will remove the parentheses from a zero-arity function definition for you. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + bugs: + zero-arity-function: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [The OPA Document Model](https://www.openpolicyagent.org/docs/philosophy/#the-opa-document-model) +- OPA Issues: [Allow user-defined zero-argument functions in Rego](https://github.com/open-policy-agent/opa/issues/6315) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/zero-arity-function/zero_arity_function.rego) diff --git a/docs/projects/regal/rules/custom/forbidden-function-call.md b/docs/projects/regal/rules/custom/forbidden-function-call.md new file mode 100644 index 0000000000..42ae111fea --- /dev/null +++ b/docs/projects/regal/rules/custom/forbidden-function-call.md @@ -0,0 +1,50 @@ +# forbidden-function-call + +**Summary**: Forbidden function call + +**Category**: Custom + +## Description + +This custom rule allows providing Regal a list of +[built-in functions](https://www.openpolicyagent.org/docs/policy-reference/#built-in-functions) that should be +considered forbidden. Any call to a function in the list will be reported as a violation. + +Another, more advanced, option to achieve the same result is the +[capabilities](https://www.openpolicyagent.org/docs/deployments/#capabilities) feature in OPA. While a more +capable option, allowing things like: + +- Adding new custom built-in functions that OPA should be aware of +- Disabling certain features not necessarily being built-in functions, like "future" keywords +- List allowed hosts in network calls + +...it is also more demanding to configure and maintain. If you're already using the capabilities feature +to forbid certain functions as part of your policy development process, there's no need to enable this rule. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + custom: + forbidden-function-call: + # note that all rules in the "custom" category are disabled by default + # (i.e. level "ignore") as some configuration needs to be provided by + # the user (i.e. you!) in order for them to be useful. + # + # one of "error", "warning", "ignore" + level: error + # Just an example — no functions forbidden by default + forbidden-functions: + # Prefer to use asymmetric algorithms + - io.jwt.verify_hs256 + - io.jwt.verify_hs384 + - io.jwt.verify_hs512 +``` + +## Related Resources + +- OPA Docs: [Capabilities](https://www.openpolicyagent.org/docs/deployments/#capabilities) +- OPA Docs: [Built-in Functions](https://www.openpolicyagent.org/docs/policy-reference/#built-in-functions) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/custom/forbidden-function-call/forbidden_function_call.rego) diff --git a/docs/projects/regal/rules/custom/index.md b/docs/projects/regal/rules/custom/index.md new file mode 100644 index 0000000000..6038b46014 --- /dev/null +++ b/docs/projects/regal/rules/custom/index.md @@ -0,0 +1,27 @@ +--- +title: Custom +sidebar_position: 7 +--- + + +# Custom + +The `custom` category is a special one, as the rules in this category allow you +to enforce rules that are specific to your project, team or organization. This +typically includes things like naming conventions, where you might want to +ensure that, for example, all package names adhere to an organizational +standard, like having a prefix matching the organization name. + +:::warning +Since these rules require configuration provided by the user, or are more +opinionated than other rules, they are disabled by default. In order to enable +them, see the configuration options available for each rule for how to configure +them according to your requirements. +::: + +For more advanced requirements, see the guide on writing [custom rules](https://openpolicyagent.org/projects/regal/custom-rules) in Rego. + +import RulesTable from '@site/src/components/projects/regal/RulesTable'; + + + diff --git a/docs/projects/regal/rules/custom/index.md.yaml b/docs/projects/regal/rules/custom/index.md.yaml new file mode 100644 index 0000000000..2e4d22a201 --- /dev/null +++ b/docs/projects/regal/rules/custom/index.md.yaml @@ -0,0 +1,2 @@ +title: Custom +sidebar_position: 7 diff --git a/docs/projects/regal/rules/custom/missing-metadata.md b/docs/projects/regal/rules/custom/missing-metadata.md new file mode 100644 index 0000000000..5fddfc2ecd --- /dev/null +++ b/docs/projects/regal/rules/custom/missing-metadata.md @@ -0,0 +1,77 @@ +# missing-metadata + +**Summary**: Package or rule missing metadata + +**Category**: Custom + +**Avoid** +```rego +package acmecorp.authz + +authorized_users contains user if { + # logic to determine authorized users +} +``` + +**Prefer** +```rego +# METADATA +# description: The `acmecorp.authz` module provides authorization logic for the AcmeCorp application. +package acmecorp.authz + +# METADATA +# description: Provides a set of all authorized users given the conditions in `input`. +# scope: document +authorized_users contains user if { + # logic to determine authorized users +} +``` + +## Rationale + +Using metadata annotations is a great way to document your policies, for both yourself and others. While using metadata +annotations _everywhere_ might be overkill for many projects, it should absolutely be considered for libraries, or +policies that target a larger audience. + +## Exceptions + +Rules and functions with an underscore prefix in their name are commonly used to denote that they are intended +to be used internally (i.e. within the same file) only, and while metadata occasionally help document these, +they are not part of the "public API". The `missing-metadata` thus excludes these from the metadata requirement. + +It is also possible to configure your own exceptions for both package and rule paths. See the configuration options +below. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + custom: + missing-metadata: + # note that all rules in the "custom" category are disabled by default + # (i.e. level "ignore"), so make sure to set the level to "error" if you + # want this enabled! + # + # one of "error", "warning", "ignore" + level: error + # package path pattern(s) to exclude from the requirement + # defaults to no exclusions + except-package-path-pattern: ^internal\.* + # rule path pattern(s) to exclude from the requirement + # defaults to no exclusions + except-rule-path-pattern: \.report$ + # you might also want to exclude files based on their name, + # like e.g. tests: + ignore: + files: + - "*_test.rego" +``` + +## Related Resources + +- OPA Docs: [Metadata](https://www.openpolicyagent.org/docs/policy-language/#metadata) +- OPA Docs: [Annotations](https://www.openpolicyagent.org/docs/policy-language/#annotations) +- Rego Style Guide: [Use Metadata Annotations](https://github.com/StyraInc/rego-style-guide) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/custom/missing-metadata/missing_metadata.rego) diff --git a/docs/projects/regal/rules/custom/naming-convention.md b/docs/projects/regal/rules/custom/naming-convention.md new file mode 100644 index 0000000000..d6586b873c --- /dev/null +++ b/docs/projects/regal/rules/custom/naming-convention.md @@ -0,0 +1,55 @@ +# naming-convention + +**Summary**: Naming convention violation + +**Category**: Custom + +## Description + +This custom rule allows teams and organizations to define their own naming conventions for their Rego projects, without +having to write custom linter policies. Naming conventions are simply defined in the Regal configuration file using +regex patterns. + +Regal can enforce naming conventions for: + +- Package names +- Rule names +- Function names +- Variable names + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + custom: + naming-convention: + # note that all rules in the "custom" category are disabled by default + # (i.e. level "ignore") as some configuration needs to be provided by + # the user (i.e. you!) in order for them to be useful. + # + # one of "error", "warning", "ignore" + level: error + conventions: + # allow only "private" rules and functions, i.e. those starting with + # underscore, or rules named "deny" or "allow" + - pattern: '^_[a-z]+$|^deny$|^allow$' + # one of "package", "rule", "function", "variable" + targets: + - rule + - function + # any number of naming rules may be added + # package names must start with "acmecorp" or "system" + - pattern: '^acmecorp|^system' + targets: + - package +``` + +**Note:** In order to avoid characters accidentally getting escaped, always use single quotes to encode your regex +patterns. Additionally, you'll most often want to include anchors for the start and end of the string (`^` and `$`) in +your patterns, or else your pattern might accidentally match only parts of the name rather than the whole name. + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/custom/naming-convention/naming_convention.rego) diff --git a/docs/projects/regal/rules/custom/narrow-argument.md b/docs/projects/regal/rules/custom/narrow-argument.md new file mode 100644 index 0000000000..759ef7f0b7 --- /dev/null +++ b/docs/projects/regal/rules/custom/narrow-argument.md @@ -0,0 +1,181 @@ +# narrow-argument + +**Summary**: Function argument can be narrowed + +**Category**: Custom + +**Avoid** +```rego +package policy + +valid_user(user) if endswith(user.email, "acmecorp.com") +valid_user(user) if endswith(user.email, "acmecorp.org") +``` + +**Prefer** +```rego +package policy + +valid_email(email) if endswith(email, "acmecorp.com") +valid_email(email) if endswith(email, "acmecorp.org") +``` + +## Rationale + +**Note!** This is a highly opinionated rule with some caveats that you should be aware of before you use it. + +Accepting the most minimal types/values as functions arguments avoids unnecessary "dependencies", makes them more +likely to be reusable, and tends to make code more readable as it's often easier to predict from a call site what +a specifically named function does compared to a more generic version. It's fairly common to realize a function with +narrowed arguments would benefit from being renamed. That is however left as an exercise to you! + +This rule scans all use of arguments inside function heads and bodies, and will suggest narrowing down any argument +passed to the minimal value which the function depends on. Incrementally defined functions are scanned in their entirety +to make sure the narrowing is valid for all definitions. Example: + +```rego +package policy + +country_code(user) := 61 if user.country == "Australia" +country_code(user) := 81 if user.country == "Japan" +``` + +In the above example, the functions only depends on `user.country`, and this rule (when enabled) will thus recommend +narrowing the argument passed: + +```rego +package policy + +country_code(country) := 61 if country == "Australia" +country_code(country) := 81 if country == "Japan" +``` + +Instead of passing around potentially large `user` objects, our function now only needs to consider a `country` string, +which perhaps may prove useful for more than just users. Another benefit of this approach is that it's often possible +to simplify functions even further by moving the equality comparison directly into the function's arguments — a simple +form of [pattern matching](https://openpolicyagent.org/projects/regal/rules/idiomatic/equals-pattern-matching): + +```rego +package policy + +country_code("Australia") := 61 +country_code("Japan") := 81 +``` + +### Reference prefix narrowing + +So far we have looked only at functions using an identical reference to one of its arguments. That's not always the +case, but it doesn't mean the value passed can't be narrowed! Consider the following example: + +```rego +package policy + +internal_user(context) if endswith(context.user.email, "@acmecorp.com") +internal_user(context) if "staff" in context.user.roles +``` + +In the example above, the `narrow-argument` rule would point out that while two different references to the `context` +argument are used, they both have the `context.user` prefix in common, and the value passed could thus be narrowed to +that: + +```rego +package policy + +internal_user(user) if endswith(user.email, "@acmecorp.com") +internal_user(user) if "staff" in user.roles +``` + +## Caveats + +Narrowing the types passed as function arguments may come with unintended and/or undesired side-effects. More +specifically, the way OPA evaluates functions means that arguments are evaluated before the function is called. "Big" +objects, like a `user` tend to be less likely to be undefined than e.g. a `user.fax` attribute. Aborting evaluation only +because a user is without a fax machine is probably not what we want! But could be an unfortunate consequence of our +change unless we are careful (or better, have extensive test coverage). Consider the following example: + +```rego +package policy + +is_unreachable(user) if { + not has_phone(user) + not has_fax(user) +} + +has_phone(user) if + is_string(user.phone) + user.phone != "" +} + +has_fax(user) if { + is_string(user.fax) + user.fax != "" +} +``` + +While it's tempting to try and narrow the arguments passed to `has_phone` and `has_fax` only to what they need: + +```rego +package policy + +is_unreachable(user) if { + not has_phone(user.phone) + not has_fax(user.fax) +} + +has_phone(phone) if + is_string(phone) + phone != "" +} + +has_fax(fax) if { + is_string(fax) + fax != "" +} +``` + +We have now changed the behavior of `is_unreachable`, and a user without phone or fax will no longer be considered +unreachable. Why? Again, because OPA evaluates the function arguments before they are passed to the function, **and** +before the result is negated by `not`, an expression like: + +```rego +not has_phone(user.phone) +``` + +Will be rewritten by OPA to something like this: + +```rego +arg1 := user.phone +not has_phone(arg1) +``` + +If the `user.phone` attribute doesn't exist, evaluation will never reach the next line where the function is called! + +Before narrowing arguments, always consider the impact of undefined values, negation and how functions are evaluated. +And make sure to not rewrite any function that isn't extensively covered by unit tests! With that said, there are often +ways to deal with undefined attributes even when passing narrower argument types. In the example above, we could for +example rewrite `is_unreachable` to `is_reachable`, and then use `not` to negate *that* to answer if the user is +impossible to reach. + +Find what works best for you, and use the `exclude-args` configuration option (see below) to exclude arg names that you +commonly don't want to narrow, or [ignore directives](https://openpolicyagent.org/projects/regal#inline-ignore-directives) for single +locations. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + custom: + narrow-argument: + # note that all rules in the "custom" category are disabled by default + # (i.e. level "ignore") + # + # one of "error", "warning", "ignore" + level: error + # exclude args by name + # example below excludes any argument named 'config' or 'user' + exclude-args: + - config + - user +``` diff --git a/docs/projects/regal/rules/custom/one-liner-rule.md b/docs/projects/regal/rules/custom/one-liner-rule.md new file mode 100644 index 0000000000..01b1b99d28 --- /dev/null +++ b/docs/projects/regal/rules/custom/one-liner-rule.md @@ -0,0 +1,54 @@ +# one-liner-rule + +**Summary**: Rule body could be made a one-liner + +**Category**: Custom + +**Avoid** +```rego +package policy + +allow if { + is_admin +} + +is_admin if { + "admin" in input.user.roles +} +``` + +**Prefer** +```rego +package policy + +allow if is_admin + +is_admin if "admin" in input.user.roles +``` + +## Rationale + +Rules with only a single expression in the body may omit the curly braces around the body, and be written as a +one-liner. This makes simple rules read more like English, and will have more rules fit on the screen. + +As with other rules in the `custom` category, this is not necessarily a general recommendation, but a style preference +teams or organizations might want to standardize on. As such, it must be enabled via configuration. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + custom: + one-liner-rule: + # note that all rules in the "custom" category are disabled by default + # (i.e. level "ignore") + # + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/custom/one-line-rule/one_line_rule.rego) diff --git a/docs/projects/regal/rules/custom/prefer-value-in-head.md b/docs/projects/regal/rules/custom/prefer-value-in-head.md new file mode 100644 index 0000000000..5d1823ecd3 --- /dev/null +++ b/docs/projects/regal/rules/custom/prefer-value-in-head.md @@ -0,0 +1,85 @@ +# prefer-value-in-head + +**Summary**: Prefer value in rule head + +**Category**: Custom + +**Avoid** +```rego +package policy + +pin_as_number := val if { + is_number(input.pin_code) + val := to_number(input.pin_code) +} + +deny contains message if { + not input.user + message := "user attribute missing from input" +} +``` + +**Prefer** +```rego +package policy + +pin_as_number := to_number(input.pin_code) if is_number(input.pin_code) + +deny contains "user attribute missing from input" if not input.user +``` + +## Rationale + +Rules that return the value assigned in the last expression of the rule body may have the value, or the function +returning the value, moved directly to the rule head. This creates more succinct rules, and often allows for rules to be +expressed as "one-liners". This is not a general recommendation, but a style preference that a team or organization +might want to standardize on. As such, it is placed in the custom category, and must be explicitly enabled in +configuration. + +The `only-scalars` configuration option may be used to only suggest moving scalar values (strings, numbers, booleans, +null) to the head, and not expressions or functions returning a value. With this option set to `true`, the following +example would be flagged: + +```rego +deny contains message if { + not input.user + # value is a scalar + message := "user attribute missing from input" +} +``` + +But not: + +```rego +deny contains message if { + not input.user + # value returned from a function call, not suggested if `only-scalars` is set to `true` + message := sprintf("user attribute missing from input: %v", [input]) +} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + custom: + prefer-value-in-head: + # note that all rules in the "custom" category are disabled by default + # (i.e. level "ignore") + # + # one of "error", "warning", "ignore" + level: error + # whether to only suggest moving scalar values (strings, numbers, booleans, null) + # to the head, and not expressions or functions + only-scalars: false + # variable names to exempt from the rule (by default, none) + except-var-names: + - report + - violation +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/custom/prefer-value-in-head/prefer_value_in_head.rego) diff --git a/docs/projects/regal/rules/idiomatic/ambiguous-scope.md b/docs/projects/regal/rules/idiomatic/ambiguous-scope.md new file mode 100644 index 0000000000..c88ffcd764 --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/ambiguous-scope.md @@ -0,0 +1,86 @@ +# ambiguous-scope + +**Summary**: Ambiguous metadata scope + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +# METADATA +# description: allow is true if the user is admin, or the requested resource is public +allow if user_is_admin + +allow if public_resource +``` + +**Prefer** +```rego +package policy + +# METADATA +# description: allow is true if the user is admin, or the requested resource is public +# scope: document +allow if user_is_admin + +allow if public_resource +``` + +**Or (scope `rule` implied, but _all_ incremental definitions are annotated)** +```rego +package policy + +# METADATA +# description: allow is true if the user is admin +allow if user_is_admin + +# METADATA +# description: allow is true if the requested resource is public +allow if public_resource +``` + +**Or (scope `rule` explicit)** +```rego +package policy + +# METADATA +# description: allow is true if the user is admin +# scope: rule +allow if user_is_admin + +allow if public_resource +``` + +## Rationale + +The default scope for metadata annotating a rule is the `rule` scope, which +"[applies to the individual rule statement](https://www.openpolicyagent.org/docs/policy-language/#scope)" only. +This default is sensible for a rule defined only once, but is somewhat ambiguous for a rule defined incrementally, like +the `allow` rule in the examples above. Was the intention really to annotate that single definition, or the rule as +whole? Most likely the latter, and that's what the `document` scope is for. + +If only a single rule in a group of incremental rule definitions is annotated, it should have it's `scope` set explicitly +to either `document` or `rule`. If all incremental definitions are annotated, explicit `scope: rule` is not required. + +## Exceptions + +If a single incremental rule definition is annotated as `entrypoint: true`, this rule will allow that. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + ambiguous-scope: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Annotations](https://www.openpolicyagent.org/docs/policy-language/#annotations) +- Regal Docs: [no-defined-entrypoint](https://openpolicyagent.org/projects/regal/rules/idiomatic/no-defined-entrypoint) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/ambiguous-scope/ambiguous_scope.rego) diff --git a/docs/projects/regal/rules/idiomatic/boolean-assignment.md b/docs/projects/regal/rules/idiomatic/boolean-assignment.md new file mode 100644 index 0000000000..b4c5b8b372 --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/boolean-assignment.md @@ -0,0 +1,55 @@ +# boolean-assignment + +**Summary**: Prefer `if` over boolean assignment + +**Category**: Idiomatic + +**Avoid** + +```rego +package policy + +more_than_one_member := count(input.members) > 1 +``` + +**Prefer** +```rego +package policy + +more_than_one_member if count(input.members) > 1 +``` + +## Rationale + +Assigning the result of a boolean function is almost always redundant, as the boolean value returned by the expression +rarely is used for anything but to determine whether to continue evaluation. Moving the condition to the body following +an `if` will have the rule either evaluate to `true` or be undefined. For the few cases where `false` should be +returned, using a `default` rule assignment is preferable, as it is guaranteed to be assigned a value even on undefined +input: + +```rego +package policy + +default more_than_one_member := false + +# will be assigned `false` even if input.members is undefined +more_than_one_member if count(input.members) > 1 +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + boolean-assignment: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Styra Blog: [How to express OR in Rego](https://www.styra.com/blog/how-to-express-or-in-rego/) +- Regal Docs: [default-over-else](https://openpolicyagent.org/projects/regal/rules/style/default-over-else) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/boolean-assignment/boolean_assignment.rego) diff --git a/docs/projects/regal/rules/idiomatic/custom-has-key-construct.md b/docs/projects/regal/rules/idiomatic/custom-has-key-construct.md new file mode 100644 index 0000000000..51f9f9ca5f --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/custom-has-key-construct.md @@ -0,0 +1,46 @@ +# custom-has-key-construct + +**Summary**: Custom function may be replaced by `in` and `object.keys` + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +mfa if has_key(input.claims, "mfa") + +has_key(map, key) if { + _ = map[key] +} +``` + +**Prefer** +```rego +package policy + +mfa if "mfa" in object.keys(input.claims) +``` + +## Rationale + +Checking if a key exists in an object (regardless of the attribute's value) used to be done using custom functions. With +the introduction of the [object.keys](https://www.openpolicyagent.org/docs/policy-reference/#builtin-object-objectkeys) +(OPA [v0.47.0](https://github.com/open-policy-agent/opa/releases/tag/v0.47.0)) function, this is no longer necessary, +and using the built-in function together with `in` should be preferred. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + custom-has-key-construct: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/custom-has-key-construct/custom_has_key_construct.rego) diff --git a/docs/projects/regal/rules/idiomatic/custom-in-construct.md b/docs/projects/regal/rules/idiomatic/custom-in-construct.md new file mode 100644 index 0000000000..827475aa4b --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/custom-in-construct.md @@ -0,0 +1,49 @@ +# custom-in-construct + +**Summary**: Custom function may be replaced by `in` keyword + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +allow if has_value(input.user.roles, "admin") + +# This custom function was commonly seen before the introduction +# of the `in` keyword. Avoid it now. +has_value(arr, item) if { + item == arr[_] +} +``` + +**Prefer** +```rego +package policy + +allow if "admin" in input.user.roles +``` + +## Rationale + +The `in` keyword was introduced in OPA [v0.34.0](https://github.com/open-policy-agent/opa/releases/tag/v0.34.0). +Prior to that, it was a common practice to create a custom helper function that would iterate over values of an array in +order to check if it contained a provided value. Since the introduction of the `in` keyword, this is no longer +necessary. The `in` keyword additionally supports sets and maps as the collection type, so using it consistently is +recommended. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + custom-in-construct: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/custom-in-construct/custom_in_construct.rego) diff --git a/docs/projects/regal/rules/idiomatic/directory-package-mismatch.md b/docs/projects/regal/rules/idiomatic/directory-package-mismatch.md new file mode 100644 index 0000000000..1e7598f99e --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/directory-package-mismatch.md @@ -0,0 +1,137 @@ +# directory-package-mismatch + +**Summary**: Directory structure should mirror package + +**Category**: Idiomatic + +**Automatically fixable**: Yes + +## Rationale + +Quickly finding the package you're looking for in a policy repository is made much easier when package paths are +mirrored in the directory structure of your project. Meaning that if the name of your package is +`permissions.users.claims`, it should reside in a file under the `permissions/users/claims` directory. Note however +that any number of files can contribute to the same package! The `permissions/users/claims` directory may thus +contain several policy files that all declare `package permissions.users.claims`. + +### Example + +An example of directory structure for a project following this convention might look like this: + +```shell +# Directory structure # Package path +. +├── README.md +└── bundle + └── authorization + ├── main.rego # authorization + └── rbac + ├── data.json # authorization.rbac + ├── roles + │   └── roles.rego # authorization.rbac.roles + │   └── roles_test.rego # authorization.rbac.roles_test + └── users + ├── customers.rego # authorization.rbac.users + ├── customers_test.rego # authorization.rbac.users_test + ├── internal.rego # authorization.rbac.users + └── internal_test.rego # authorization.rbac.users_test +``` + +### Tests + +Astute observers may notice that the test files in the example above are placed in the same directory as the +policies they test. This may seem to contradict the +[test-outside-test-package](https://openpolicyagent.org/projects/regal/rules/testing/test-outside-test-package) rule, which +says that any test package should have a `_test` suffix in its package path. However, putting tests next to +the file they target arguably makes it _easier_ to find, and is a common practice in the OPA community. This +rule therefore by default ignores the `_test` suffix when determining whether the package path matches the +directory structure. + +This behavior can be changed by setting the `exclude-test-suffix` configuration option to `false`, in which +case package paths with a `_test` suffix also will be required to reside in a directory with a `_test` suffix. + +Setting the `exclude-test-suffix` option to `false` means the example from above would now look like this: + +```shell +# Directory structure # Package path +. +├── README.md +└── bundle + └── authorization + ├── main.rego # authorization + └── rbac + ├── data.json # authorization.rbac + ├── roles + │   └── roles.rego # authorization.rbac.roles + ├── roles_test + │   └── roles_test.rego # authorization.rbac.roles_test + ├── users + │   ├── customers.rego # authorization.rbac.users + │   └── internal.rego # authorization.rbac.users + └── users_test + ├── customers_test.rego # authorization.rbac.users_test + └── internal_test.rego # authorization.rbac.users_test +``` + +Whichever way you choose is up to you. Consistency is key! + +### Bundles + +While directory structure doesn't matter to OPA when parsing _policies_, directories parsed as +[bundles](https://www.openpolicyagent.org/docs/management-bundles/) will read _data_ (`data.json` or +`data.yaml`) files and insert the data in the `data` document tree based on the directory structure relative +to the bundle root. Having policies structured in the same manner provides a uniform experience, and makes it +easier to understand where both policies and data come from. + +### `regal fix` & Editor Support + +Regal's [`fix` command](https://openpolicyagent.org/projects/regal/fixing) can automatically +rename files in a project to ensure compliance with this rule. This is +particularly useful when refactoring a project with many files. + +:::info +Note that files will be renamed relative to their nearest root, see the +[documentation on roots](https://openpolicyagent.org/projects/regal#project-roots) when using +this rule with policy roots different from the project root. +::: + +Editors integrating Regal's [language server](https://openpolicyagent.org/projects/regal/language-server) will automatically display +suggestions for idiomatic package paths based on the directory structure in which a policy resides. The image below +demonstrates a new policy being created inside an `authorization/rbac/roles` directory, and the editor +([via Regal](https://openpolicyagent.org/projects/regal/language-server#code-completions)) suggesting the package path +`authorization.rbac.roles`. + + + +In addition, empty files will be be 'formatted' to have the correct package +based on the directory structure. Newly created Rego files are treated in much +the same way. When a new file is created, the server will send a series of edits +back to set the content. If `exclude-test-suffix` is set to `false`, the file +will also be moved if required to the `_test` directory for that package. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + directory-package-mismatch: + # one of "error", "warning", "ignore" + level: error + # exclude _test suffixes from package paths before comparing + # them to directory structure paths. when set to false, a + # package like authz.policy_test would need to be placed in + # an authz/policy_test directory, and if set to true (default) + # would be expected to be in authz/policy + exclude-test-suffix: true +``` + +## Related Resources + +- Rego Style Guide: [Package name should match file location](https://github.com/StyraInc/rego-style-guide#package-name-should-match-file-location) +- Regal Docs: [test-outside-test-package](https://openpolicyagent.org/projects/regal/rules/testing/test-outside-test-package) +- OPA Docs: [Bundles](https://www.openpolicyagent.org/docs/management-bundles/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/directory-package-mismatch/directory_package_mismatch.rego) diff --git a/docs/projects/regal/rules/idiomatic/equals-pattern-matching.md b/docs/projects/regal/rules/idiomatic/equals-pattern-matching.md new file mode 100644 index 0000000000..31925a9145 --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/equals-pattern-matching.md @@ -0,0 +1,96 @@ +# equals-pattern-matching + +**Summary**: Prefer pattern matching in function arguments + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +readable_number(x) := "one" if x == 1 +readable_number(x) := "two" if x == 2 +``` + +**Prefer** +```rego +package policy + +readable_number(1) := "one" +readable_number(2) := "two" +``` + +## Rationale + +Pattern matching on equality in function arguments is one of Rego's most well-kept secrets. As secret as it might be, +it's a great way to simplify custom functions performing equality checks on their arguments in the rule body, by +moving the equality check to match on the function call itself. This means that a function like the one below: + +```rego +package policy + +normalize_role(role) := "admin" if { + role == "administrator" +} + +normalize_role(role) := "admin" if { + role == "root" +} +``` + +May have the equality check moved to the function argument, and the function only evaluated in case the argument matches +the equality "pattern": + +```rego +package policy + +normalize_role("administrator") := "admin" + +normalize_role("root") := "admin" +``` + +Rules that evaluate to `true` may even have the assignment removed altogether, i.e.: + +```rego +package policy + +is_admin(role) if role == "admin" + +is_admin(role) if role == "administrator" + +is_admin(role) if role == "root" +``` + +Can be simplified to just: + +```rego +package policy + +is_admin("admin") + +is_admin("administrator") + +is_admin("root") +``` + +## Limitations + +This rule is currently limited to simple rules where the equality check is the **only** condition in the rule body. This +will be improved in future releases. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + equals-pattern-matching: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Styra Blog: [How to express OR in Rego](https://www.styra.com/blog/how-to-express-or-in-rego/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/equals-pattern-matching/equals_pattern_matching.rego) diff --git a/docs/projects/regal/rules/idiomatic/in-wildcard-key.md b/docs/projects/regal/rules/idiomatic/in-wildcard-key.md new file mode 100644 index 0000000000..61a4ae4991 --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/in-wildcard-key.md @@ -0,0 +1,65 @@ +# in-wildcard-key + +**Summary**: Unnecessary wildcard key + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +allow if { + # since only the value is used, we don't need to iterate the keys + some _, user in input.users + + # do something with each user +} +``` + +**Prefer** +```rego +package policy + +allow if { + some user in input.users + + # do something with each user +} +``` + +## Rationale + +The `some .. in` iteration form can either iterate only values: + +```rego +some value in object +``` + +Or keys and values: + +```rego +some key, value in object +``` + +Using a wildcard variable for the key in the key-value form is thus unnecessary, and: + +```rego +some _, value in object +``` + +Can simply be replaced by: + +````rego +some value in object + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + in-wildcard-key: + # one of "error", "warning", "ignore" + level: error +```` diff --git a/docs/projects/regal/rules/idiomatic/index.md b/docs/projects/regal/rules/idiomatic/index.md new file mode 100644 index 0000000000..dc360f0ee3 --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/index.md @@ -0,0 +1,14 @@ +--- +title: Idiomatic +sidebar_position: 2 +--- + + +# Idiomatic + +Rules that enforce idiomatic code. + +import RulesTable from '@site/src/components/projects/regal/RulesTable'; + + + diff --git a/docs/projects/regal/rules/idiomatic/index.md.yaml b/docs/projects/regal/rules/idiomatic/index.md.yaml new file mode 100644 index 0000000000..a8fdb6c80f --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/index.md.yaml @@ -0,0 +1,2 @@ +title: Idiomatic +sidebar_position: 2 diff --git a/docs/projects/regal/rules/idiomatic/no-defined-entrypoint.md b/docs/projects/regal/rules/idiomatic/no-defined-entrypoint.md new file mode 100644 index 0000000000..b2b6262daf --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/no-defined-entrypoint.md @@ -0,0 +1,88 @@ +# 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 + +default allow := false + +# Nothing wrong with this rule, but an +# entrypoint should be documented as such +allow if user_is_admin +allow 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" +} +``` + +**Prefer** +```rego +package policy + +default allow := false + +# METADATA +# description: Allow only admins, or reading public resources +# entrypoint: true +allow if user_is_admin +allow 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/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/policy-language/#metadata) +- OPA Docs: [Entrypoint](https://www.openpolicyagent.org/docs/policy-language/#entrypoint) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/no-defined-entrypoint/no_defined_entrypoint.rego) diff --git a/docs/projects/regal/rules/idiomatic/non-raw-regex-pattern.md b/docs/projects/regal/rules/idiomatic/non-raw-regex-pattern.md new file mode 100644 index 0000000000..d226851d1f --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/non-raw-regex-pattern.md @@ -0,0 +1,61 @@ +# non-raw-regex-pattern + +**Summary**: Use raw strings for regex patterns + +**Category**: Idiomatic + +**Automatically fixable**: [Yes](https://openpolicyagent.org/projects/regal/fixing) + +**Avoid** +```rego +all_digits if { + regex.match("[\\d]+", "12345") +} +``` + +**Prefer** +```rego +all_digits if { + regex.match(`[\d]+`, "12345") +} +``` + +## Rationale + +[Raw strings](https://www.openpolicyagent.org/docs/edge/policy-language/#strings) are interpreted literally, allowing +you to avoid having to escape special characters like `\` in your regex patterns. Using raw strings for regex patterns +additionally makes them easier to identify as such. + +## Limitations + +This rule currently only scans regex string literals in the place of the `pattern` argument of the various +[regex built-in functions](https://www.openpolicyagent.org/docs/policy-reference/#regex). It will not **not** +try to "resolve" patterns assigned to variables. The following example would as such not render a warning: + +```rego +package policy + +# Pattern assigned to variable +pattern := "[\\d]+" + +# This won't trigger a violation +allow if regex.match(pattern, "12345") +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + non-raw-regex-pattern: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Use raw strings for regex patterns](https://github.com/StyraInc/rego-style-guide#use-raw-strings-for-regex-patterns) +- OPA Docs: [Regex Functions Reference](https://www.openpolicyagent.org/docs/policy-reference/#regex) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/non-raw-regex-pattern/non_raw_regex_pattern.rego) diff --git a/docs/projects/regal/rules/idiomatic/prefer-set-or-object-rule.md b/docs/projects/regal/rules/idiomatic/prefer-set-or-object-rule.md new file mode 100644 index 0000000000..b3347f6502 --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/prefer-set-or-object-rule.md @@ -0,0 +1,149 @@ +# prefer-set-or-object-rule + +**Summary**: Prefer set or object rule over comprehension + +**Category**: Idiomatic + +**Avoid** + +```rego +package policy + +# top level set comprehension +developers := {developer | + some user in input.users + "developer" in user.roles + developer := user.name +} + +# top level object comprehension +user_roles_mapping := {user: roles | + some user in input.users + roles := user.roles +} +``` + +**Prefer** + +```rego +package policy + +# set generating rule +developers contains developer if { + some user in input.users + "developer" in user.roles + developer := user.name +} + +# object generating rule +user_roles_mapping[user] := roles if { + some user in input.users + roles := user.roles +} +``` + +## Rationale + +Comprehensions are [awesome](https://www.styra.com/blog/five-things-you-didnt-know-about-opa/), and should be part of +any policy author's toolbox. Using comprehensions inside of rule bodies allow for a wide variety of elegant solutions to +otherwise hard problems. However, when used as the value directly (and unconditionally) assigned to a rule, it is almost +always better to use a rule that generates a set or object in the rule body rather than having a comprehension do so in +the rule head. Why is that? + +### Readability + +Rules that generate objects, and sets even more so, read more natural than comprehensions, and are generally more +descriptive. While both constructs are easy to spot for a seasoned Rego author, anything that helps improve readability +is a win. + +### Extensibility + +While readability is important, the real benefit of using a rule to generate a set or object is that it allows the rule +to be _extended_. This is particularly true for set generating rules, and it's not by accident they often are referred +to as **multi-value rules**. A rule assigned the value of a set comprehension can't have its value changed later, or +more items added to the set. A set generating rule however, can easily be extended to contain more items, conditionally +or unconditionally. + +```rego +package policy + +# Getting developers from input +developers contains developer if { + some user in input.users + "developer" in user.roles + developer := user.name +} + +# *Also* getting developers from data +developers contains developer if { + some user in data.users + "developer" in user.roles + developer := user.name +} + +# Unconditionally adding a developer to the set +developers contains "Hackerman" +``` + +In the example above, all three rules contribute to the `developers` set. If we wanted to, we could even create another +policy file using the same package, and have more rules added there that would contribute to the set. This creates some +great opportunities for extensibility, and collaboration across developers and teams working on policy together. + +Objects differ somewhat from sets in that while several rules can be used to generate an object, there cannot be more +than one rule contributing to a single key-value pair. + +```rego +package policy + +novels[title] := content if { + some document in input.documents + document.type == "novel" + title := document.title + content := document.content +} + +# This works as long as "The Hobbit" is not already in the novels object +novels["The Hobbit"] := "In a hole in the ground there lived a hobbit." + +# Map and set generating objects can also be combined, in which case the +# value is extensible even for the same key! In the example above, more +# rules could help contribute titles to an author, perhaps using different +# data sources. +titles_by_author[document.author] contains document.title if { + some document in input.documents +} +``` + +## Exceptions + +Note that this rule does **not** apply to array comprehensions, as there is no equivalent tp use a rule to generate an +array. + +This rule will also ignore simple comprehensions used solely for the purpose of converting an array to a set, i.e: + +```rego +package policy + +# Convert set to array. This is fine. +my_set := {item | some item in arr} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + prefer-set-or-object-rule: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Generating Sets](https://www.openpolicyagent.org/docs/policy-language/#generating-sets) +- OPA Docs: [Generating Objects](https://www.openpolicyagent.org/docs/policy-language/#generating-objects) +- OPA Docs: [Comprehensions](https://www.openpolicyagent.org/docs/policy-language/#comprehensions) +- Styra Blog: [Five Things You Didn't Know About OPA](https://www.styra.com/blog/five-things-you-didnt-know-about-opa/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/prefer-set-or-object-rule/prefer_set_or_object_rule.rego) diff --git a/docs/projects/regal/rules/idiomatic/single-item-in.md b/docs/projects/regal/rules/idiomatic/single-item-in.md new file mode 100644 index 0000000000..97b31da3da --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/single-item-in.md @@ -0,0 +1,42 @@ +# single-item-in + +**Summary**: Avoid `in` for single item collection + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +allow if input.role in {"admin"} +``` + +**Prefer** +```rego +package policy + +allow if input.role == "admin" +``` + +## Rationale + +Using `in` on a single-item collection (array, set or object) is a convoluted way of checking for equality. Better +then to check for equality directly! Besides being more obvious, equality checks are also subject to rule indexing, +whereas `in` checks currently aren't. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + single-item-in: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Use indexed statements](https://www.openpolicyagent.org/docs/policy-performance/#use-indexed-statements) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/single-item-in/single_item_in.rego) diff --git a/docs/projects/regal/rules/idiomatic/use-contains.md b/docs/projects/regal/rules/idiomatic/use-contains.md new file mode 100644 index 0000000000..304f7c57ca --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/use-contains.md @@ -0,0 +1,80 @@ +# use-contains + +**Summary**: Use the `contains` keyword + +**Category**: Idiomatic + +## Notice: Rule made obsolete by OPA 1.0 + +Since Regal v0.30.0, this rule is only enabled for projects that have either been explicitly configured to target +versions of OPA before 1.0, or if no configuration is provided — where Regal is able to determine that an older version +of OPA/Rego is being targeted. Consult the documentation on Regal's +[configuration](https://openpolicyagent.org/projects/regal#configuration) for information on how to best work with older versions of +OPA and Rego. + +Since OPA v1.0, this rule is no longer needed as the Rego v1 syntax is now mandatory, and using `contains` is now the +de-facto way to define multi-value rules. + +**Avoid** +```rego +package policy + +import future.keywords.in + +report[item] if { + some item in input.items + startswith(item, "report") +} + +# unconditionally add an item to report +report["report1"] +``` + +**Prefer** +```rego +package policy + +import future.keywords.contains +import future.keywords.if +import future.keywords.in + +report contains item if { + some item in input.items + startswith(item, "report") +} + +# unconditionally add an item to report +report contains "report1" +``` + +## Rationale + +The `contains` keyword helps to clearly distinguish *multi-value rules* (or "partial rules") from +single-value rules ("complete rules"). Just like the `if` keyword, `contains` additionally makes the rule read the same +way in English as OPA interprets its meaning — a set that contains one or more values given some (optional) conditions. + +OPA version 1.0, which is planned for 2024, will make the `contains` keyword mandatory. This rule helps you get ahead of +the curve and start using it today. + +**Note**: don't forget to `import future.keywords.contains`! Or from OPA v0.59.0 and onwards, `import rego.v1`. + +**Tip**: When either of the imports mentioned above are found in a Rego file, the `contains` keyword will be inserted +automatically at any applicable location by the `opa fmt` tool. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + use-contains: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [use-if](https://openpolicyagent.org/projects/regal/rules/idiomatic/use-if) +- OPA Docs: [Future Keywords](https://www.openpolicyagent.org/docs/policy-language/#future-keywords) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/use-contains/use_contains.rego) diff --git a/docs/projects/regal/rules/idiomatic/use-if.md b/docs/projects/regal/rules/idiomatic/use-if.md new file mode 100644 index 0000000000..36f6b173c0 --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/use-if.md @@ -0,0 +1,76 @@ +# use-if + +**Summary**: Use the `if` keyword + +**Category**: Idiomatic + +## Notice: Rule made obsolete by OPA 1.0 + +Since Regal v0.30.0, this rule is only enabled for projects explicitly configured to target versions of OPA before 1.0. +Consult the documentation on Regal's [configuration](https://openpolicyagent.org/projects/regal#configuration) for information on how +to best work with older versions of OPA and Rego. + +Since OPA v1.0, this rule is no longer needed simply because the Rego v1 syntax is made mandatory, and the use of `if` +is now enforced before all rule bodies. + +**Avoid** +```rego +package policy + +import future.keywords.in + +is_admin { + "admin" in input.user.roles +} +``` + +**Prefer** +```rego +package policy + +import future.keywords.if +import future.keywords.in + +is_admin if { + "admin" in input.user.roles +} + +# alternatively + +is_admin if "admin" in input.user.roles +``` + +## Rationale + +The `if` keyword helps communicate what Rego rules really are — conditional assignments. Using `if` in other words makes +the rule read the same way in English as it will be interpreted by OPA, i.e: + +```rego +rule := "some value" if some_condition +``` + +OPA version 1.0, which is planned for 2024, will make the `if` keyword mandatory. This rule helps you get ahead of the +curve and start using it today. + +**Note**: don't forget to `import future.keywords.if`! Or from OPA v0.59.0 and onwards, `import rego.v1`. + +**Tip**: When either of the imports mentioned above are found in a Rego file, the `if` keyword will be inserted +automatically at any applicable location by the `opa fmt` tool. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + use-if: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [use-contains](https://openpolicyagent.org/projects/regal/rules/idiomatic/use-contains) +- OPA Docs: [Future Keywords](https://www.openpolicyagent.org/docs/policy-language/#future-keywords) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/use-if/use_if.rego) diff --git a/docs/projects/regal/rules/idiomatic/use-in-operator.md b/docs/projects/regal/rules/idiomatic/use-in-operator.md new file mode 100644 index 0000000000..c6d83f7b0d --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/use-in-operator.md @@ -0,0 +1,46 @@ +# use-in-operator + +**Summary**: Use `in` to check for membership + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +# "Old" way of checking for membership - iteration + comparison +allow if { + "admin" == input.user.roles[_] +} +``` + +**Prefer** +```rego +package policy + +allow if { + "admin" in input.user.roles +} +``` + +## Rationale + +Using `in` for membership checks clearly communicates intent, and is less prone to errors. This is especially true when +checking if something is **not** part of a collection. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + use-in-operator: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Use `in` to check for membership](https://github.com/StyraInc/rego-style-guide#use-in-to-check-for-membership) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/use-in-operator/use_in_operator.rego) diff --git a/docs/projects/regal/rules/idiomatic/use-object-keys.md b/docs/projects/regal/rules/idiomatic/use-object-keys.md new file mode 100644 index 0000000000..2c5b5ed47d --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/use-object-keys.md @@ -0,0 +1,45 @@ +# use-object-keys + +**Summary**: Prefer to use `object.keys` + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +keys := {k | some k, _ in input.object} + +# or + +keys := {k | some k; input.object[k]} +``` + +**Prefer** +```rego +package policy + +keys := object.keys(input.object) +``` + +## Rationale + +Instead of using a set comprehension to collect keys from an object, prefer to use the built-in function +[object.keys](https://www.openpolicyagent.org/docs/policy-reference/#builtin-object-objectkeys). +This option is both more declarative and better conveys the intent of the code. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + use-object-keys: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [object.keys](https://www.openpolicyagent.org/docs/policy-reference/#builtin-object-objectkeys) diff --git a/docs/projects/regal/rules/idiomatic/use-some-for-output-vars.md b/docs/projects/regal/rules/idiomatic/use-some-for-output-vars.md new file mode 100644 index 0000000000..479f5aaf0e --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/use-some-for-output-vars.md @@ -0,0 +1,111 @@ +# use-some-for-output-vars + +**Summary**: Use `some` to declare output variables + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +allow if { + userinfo := data.users[id] + # ... +} +``` + +**Prefer** +```rego +package policy + +allow if { + some id + userinfo := data.users[id] + # ... +} + +# alternatively, and arguably more idiomatic: +allow if { + some id, userinfo in data.users + # ... +} +``` + +## Rationale + +An interesting, and likely unfamiliar aspect of Rego for developers coming from other languages, is the concept of +[unification](https://en.wikipedia.org/wiki/Unification_(computer_science)). Unification happens not just explicitly via +the unification operator (`=`), but is an integral part of Rego. In the context of this rule, unification means that a +variable can either be an _input_ or an _output_. What does that mean? + +From the example above, consider that `data.users` is a map of user IDs to user objects: + +```json +{ + "jane": {"email": "jane@acmecorp.com", "firstname": "Jane", "lastname": "Doe"}, + "joe": {"email": "joe@example.com", "firstname": "Joe", "lastname": "Bloggs"}, + "john": {"email": "john@opa.org", "firstname": "John", "lastname": "Smith"} +} +``` + +```rego +usernames contains name if { + data.users[name] +} +``` + +What is the meaning of `name` in the body of the `usernames` rule? In most programming languages, evaluation would +fail unless `name` was defined elsewhere in the code. That is because `name` would be expected to be an **input** in the +expression — the result of using, say "joe", as the input in `users["joe"]` would predictably be the value associated +with that key. In Rego, however, `name` may also be an **output** — meaning that if the variable is not defined +elsewhere, OPA will attempt to _unify_ it with any value that satisfies the expression. In this case, that means that +`name` will be bound to each of the keys in the `users` object in turn, and the rule will succeed for each of them. + +This is a powerful feature of Rego, but it can also be a source of confusion. If we were to define `name` somewhere +else in the policy, perhaps by mistake: + +```rego +name := "joe" + +# hundreds of lines of Rego later.. + +usernames contains name if { + data.users[name] +} +``` + +Our `usernames` rule would no longer iterate over all the users, as the condition would be satisfied by simply mapping +the key "joe" to its value. By using `some` to locally declare `name`, we can avoid this problem: + +```rego +name := "joe" + +# hundreds of lines of Rego later.. + +usernames contains name if { + some name + data.users[name] +} +``` + +Even though `name` is defined in the global scope, the `some` keyword will ensure it's now considered as an output +variable in the local scope of the `usernames` rule. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + use-some-for-output-vars: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Don't use undeclared variables](https://github.com/StyraInc/rego-style-guide#dont-use-undeclared-variables) +- OPA Docs: [The `some` keyword](https://www.openpolicyagent.org/docs/policy-language/#some-keyword) +- Wikipedia: [Unification](https://en.wikipedia.org/wiki/Unification_(computer_science)) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/use-some-for-output-vars/use_some_for_output_vars.rego) diff --git a/docs/projects/regal/rules/idiomatic/use-strings-count.md b/docs/projects/regal/rules/idiomatic/use-strings-count.md new file mode 100644 index 0000000000..e81a7c09c7 --- /dev/null +++ b/docs/projects/regal/rules/idiomatic/use-strings-count.md @@ -0,0 +1,41 @@ +# use-strings-count + +**Summary**: Use `strings.count` where possible + +**Category**: Idiomatic + +**Avoid** +```rego +package policy + +num_as := count(indexof_n("foobarbaz", "a")) +``` + +**Prefer** +```rego +package policy + +num_as := strings.count("foobarbaz", "a") +``` + +## Rationale + +The `strings.count` function added in [OPA v0.67.0](https://github.com/open-policy-agent/opa/releases/tag/v0.67.0) +is both more readable and efficient compared to using `count(indexof_n(...))` and should therefore be preferred. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + idiomatic: + use-strings-count: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [strings.count](https://www.openpolicyagent.org/docs/policy-reference/#builtin-strings-stringscount) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/use-strings-count/use_strings_count.rego) diff --git a/docs/projects/regal/rules/imports/avoid-importing-input.md b/docs/projects/regal/rules/imports/avoid-importing-input.md new file mode 100644 index 0000000000..d03c2972f6 --- /dev/null +++ b/docs/projects/regal/rules/imports/avoid-importing-input.md @@ -0,0 +1,77 @@ +# avoid-importing-input + +**Summary**: Avoid importing `input` + +**Category**: Imports + +**Avoid** +```rego +package policy + +# This is always redundant +import input + +# This might be useful, but better to move to a local assignment +import input.user.email + +allow if "admin" in input.user.roles + +allow if { + endswith(email, "@acmecorp.com") +} +``` + +**Prefer** +```rego +package policy + +allow if "admin" in input.user.roles + +allow if { + email := input.user.email + endswith(email, "@acmecorp.com") +} +``` + +## Rationale + +Using an import for `input` is not necessary, as both `input` and `data` are globally available. + +## Exceptions + +Using an alias for `input` can sometimes be useful, e.g. when using `input` is known to represent something specific, +like a Terraform plan. Aliasing of specific input attributes should however be avoided in favor of local assignments. + +```rego +package policy + +# This is acceptable +import input as tfplan + +# But this should be avoided - use assignment instead: +# username := input.user.name +import input.user.name as username + +allow if { + some resource_change in tfplan.resource_changes + # ... +} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + avoid-importing-input: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Avoid importing `input`](https://github.com/StyraInc/rego-style-guide#avoid-importing-input) +- OPA Docs: [Terraform Tutorial](https://www.openpolicyagent.org/docs/terraform) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/avoid-importing-input/avoid_importing_input.rego) diff --git a/docs/projects/regal/rules/imports/circular-import.md b/docs/projects/regal/rules/imports/circular-import.md new file mode 100644 index 0000000000..32cf21aa45 --- /dev/null +++ b/docs/projects/regal/rules/imports/circular-import.md @@ -0,0 +1,125 @@ +# circular-import + +**Summary**: Avoid circular imports + +**Category**: Imports + +**Avoid** + +```mermaid +graph LR + authz --> shared + shared --> authz +``` + +```rego +# authz.rego +package authz + +import data.shared + +admins := { + "anna", + "bob", +} + +allow if { + input.role in shared.roles +} + +allow if { + input.user in admins +} +``` + +```rego +# shared.rego +package shared + +import data.authz # circular import! + +roles := { "admin", "editor", "viewer" } + +users := authz.admins | { + "chloe", + "dave", +} +``` + +**Prefer** + +Break out shared rules into a tree-like structure of packages. For example, one way we could refactor the above example +is to move the `admins` set into a new package. + +```mermaid +graph LR + authz --> shared + shared --> admins +``` + +```rego +# authz.rego +package authz + +import data.shared + +allow if { + input.role in shared.roles +} + +allow if { + input.user in shared.users +} +``` + +```rego +# admins.rego +package admins + +admins := { + "anna", + "bob", +} +``` + +```rego +# shared.rego +package shared + +import data.admins + +roles := {"admin", "editor", "viewer"} + +users := admins.admins | { + "chloe", + "david", +} +``` + +## Rationale + +A circular import is when a package imports itself, either by directly importing itself, +or indirectly by importing a which in turn imports a series of packages that eventually import the original package. + +As long as recursive rules definitions are avoided, circular imports are permitted in Rego. +However, such import graphs are not advisable and a signal of poorly structured policy code. + +If you have a circular import, +it's recommended that you refactor your code into different packages that do not import each other. +This will make your code easier to navigate and maintain. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + circular-import: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/circular-import/circular_import.rego) diff --git a/docs/projects/regal/rules/imports/confusing-alias.md b/docs/projects/regal/rules/imports/confusing-alias.md new file mode 100644 index 0000000000..4b6ad35fc5 --- /dev/null +++ b/docs/projects/regal/rules/imports/confusing-alias.md @@ -0,0 +1,52 @@ +# confusing-alias + +**Summary**: Confusing alias of existing import + +**Category**: Imports + +**Avoid** +```rego +package policy + +# both 'users' and 'employees' point to the same imported resource +import data.resources.users +import data.resources.users as employees +``` + +**Prefer** +```rego +package policy + +# a single import for any given resource +import data.resources.users +``` + +**or** + +```rego +package policy + +# a single aliased import for any given resource +import data.resources.users as employees +``` + +## Rationale + +Using an alias for an import occasionally helps improve intent and readability by using a name that's relevant to the +context in which the import is used. But an aliased import should never be used for a reference also imported +**without** an alias, as that's just confusing. Either use and alias or don't, but stick to one convention for any +given import. + +Using two different aliases for the same import is also likely a mistake, and is similarly flagged by this rule. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + confusing-alias: + # one of "error", "warning", "ignore" + level: error +``` diff --git a/docs/projects/regal/rules/imports/ignored-import.md b/docs/projects/regal/rules/imports/ignored-import.md new file mode 100644 index 0000000000..720e40337f --- /dev/null +++ b/docs/projects/regal/rules/imports/ignored-import.md @@ -0,0 +1,53 @@ +# ignored-import + +**Summary**: Reference ignores import + +**Category**: Imports + +**Avoid** +```rego +package policy + +import data.authz.roles + +allow if { + some role in input.user.roles + # data.authz.roles has been imported, but the import is ignored here + role in data.authz.roles.admin_roles +} +``` + +**Prefer** +```rego +package policy + +import data.authz.roles + +allow if { + some role in input.user.roles + # imported data.authz.roles used + role in roles.admin_roles +} +``` + +## Rationale + +Imports tend to make long, nested references more readable, and encourages reuse of common logic. Using a full reference +(like `data.users.permissions`) despite having previously imported the reference, or parts of it (like `data.users`) +defeats the purpose of the import, and you're better off referring to the import directly. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + ignored-import: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/ignored-import/ignored_import.rego) diff --git a/docs/projects/regal/rules/imports/implicit-future-keywords.md b/docs/projects/regal/rules/imports/implicit-future-keywords.md new file mode 100644 index 0000000000..099dd039a5 --- /dev/null +++ b/docs/projects/regal/rules/imports/implicit-future-keywords.md @@ -0,0 +1,56 @@ +# implicit-future-keywords + +**Summary**: Implicit future keywords + +**Category**: Imports + +**Avoid** +```rego +package policy + +import future.keywords + +report contains violation if { + not "developer" in input.user.roles + + violation := "Required role 'developer' missing" +} +``` + +**Prefer** +```rego +package policy + +import future.keywords.contains +import future.keywords.if +import future.keywords.in + +report contains violation if { + not "developer" in input.user.roles + + violation := "Required role 'developer' missing" +} +``` + +## Rationale + +Using the "catch all" import of `future.keywords` is convenient, but it can lead to unexpected behavior. If future +versions of OPA introduces new keywords, there's always a risk that these keywords will conflict with existing rule and +variable names in your policy. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + implicit-future-keywords: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Use explicit imports for future keywords](https://github.com/StyraInc/rego-style-guide#use-explicit-imports-for-future-keywords) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/implicit-future-keywords/implicit_future_keywords.rego) diff --git a/docs/projects/regal/rules/imports/import-after-rule.md b/docs/projects/regal/rules/imports/import-after-rule.md new file mode 100644 index 0000000000..e260ce47fd --- /dev/null +++ b/docs/projects/regal/rules/imports/import-after-rule.md @@ -0,0 +1,44 @@ +# import-after-rule + +**Summary**: Import declared after rule + +**Category**: Imports + +**Avoid** +```rego +package policy + +required_role := "developer" + +import data.identity.users +``` + +**Prefer** +```rego +package policy + +import data.identity.users + +required_role := "developer" +``` + +## Rationale + +Imports should be declared at the top of a policy, and before any rules. This makes it easy to quickly see the +dependencies imported in the policy simply by looking at the top of the file. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + import-after-rule: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/import-after-rule/import_after_rule.rego) diff --git a/docs/projects/regal/rules/imports/import-shadows-builtin.md b/docs/projects/regal/rules/imports/import-shadows-builtin.md new file mode 100644 index 0000000000..39b9f708ed --- /dev/null +++ b/docs/projects/regal/rules/imports/import-shadows-builtin.md @@ -0,0 +1,74 @@ +# import-shadows-builtin + +**Summary**: Import shadows built-in namespace + +**Category**: Imports + +**Avoid** +```rego +package policy + +# Shadows the built-in `print` function +import data.print + +# Shadows the built-in `http.send` function +import input.attributes.http +``` + +**Prefer** +To either use different names for your packages, or use import aliases to avoid shadowing built-ins. +```rego +package policy + +# Using a different package name +import data.printer + +# Using an alias +import input.attributes.http as http_attributes +``` + +## Rationale + +OPA will not complain about an import shadowing the name or the "namespace" (i.e. `array` in `array.slice`) until a +conflicting built-in function is used in the same policy. Preventing this to happen in the first place is a better +option! + +Why does this happen? The OPA compiler rewrites any import used in a policy, so that the shorthand form expands to its +longer form. Provided a simple policy like this: + +```rego +package policy + +import data.http + +allow if { + http.send({"method": "GET", "url": "https://example.com"}) +} +``` + +The compiler will go ahead and rewrite the `http.send` call using the import: + +```rego +allow if { + data.http.send({"method": "GET", "url": "https://example.com"}) +} +``` + +This is obviously not what the policy author intended. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + import-shadows-builtin: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Built-in Functions](https://www.openpolicyagent.org/docs/policy-reference/#built-in-functions) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/import-shadows-builtin/import_shadows_builtin.rego) diff --git a/docs/projects/regal/rules/imports/import-shadows-import.md b/docs/projects/regal/rules/imports/import-shadows-import.md new file mode 100644 index 0000000000..57c43484d7 --- /dev/null +++ b/docs/projects/regal/rules/imports/import-shadows-import.md @@ -0,0 +1,56 @@ +# import-shadows-import + +**Summary**: Import shadows import + +**Category**: Imports + +## Notice: Rule made obsolete by OPA 1.0 + +Since Regal v0.30.0, this rule is only enabled for projects that have either been explicitly configured to target +versions of OPA before 1.0, or if no configuration is provided — where Regal is able to determine that an older version +of OPA/Rego is being targeted. Consult the documentation on Regal's +[configuration](https://openpolicyagent.org/projects/regal#configuration) for information on how to best work with older versions of +OPA and Rego. + +Since OPA v1.0, this rule is automatically disabled as OPA itself now forbids this, and shadowed imports will result in +a parse error. + +**Avoid** +```rego +package policy + +import data.permissions +import data.users + +# Already imported +import data.permissions +``` + +**Prefer** +```rego +package policy + +import data.permissions +import data.users +``` + +## Rationale + +Duplicate imports are redundant, and while harmless, should just be removed. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + import-shadows-import: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Strict Mode](https://www.openpolicyagent.org/docs/policy-language/#strict-mode) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/import-shadows-import/import_shadows_import.rego) diff --git a/docs/projects/regal/rules/imports/index.md b/docs/projects/regal/rules/imports/index.md new file mode 100644 index 0000000000..7141879b57 --- /dev/null +++ b/docs/projects/regal/rules/imports/index.md @@ -0,0 +1,14 @@ +--- +title: Imports +sidebar_position: 3 +--- + + +# Imports + +Rules related to importing of packages and keywords. + +import RulesTable from '@site/src/components/projects/regal/RulesTable'; + + + diff --git a/docs/projects/regal/rules/imports/index.md.yaml b/docs/projects/regal/rules/imports/index.md.yaml new file mode 100644 index 0000000000..aec8b744c1 --- /dev/null +++ b/docs/projects/regal/rules/imports/index.md.yaml @@ -0,0 +1,2 @@ +title: Imports +sidebar_position: 3 diff --git a/docs/projects/regal/rules/imports/pointless-import.md b/docs/projects/regal/rules/imports/pointless-import.md new file mode 100644 index 0000000000..b22e78fe81 --- /dev/null +++ b/docs/projects/regal/rules/imports/pointless-import.md @@ -0,0 +1,50 @@ +# pointless-import + +**Summary**: Importing own package is pointless + +**Category**: Imports + +**Avoid** +```rego +package policy + +# pointless, as policy is the own package +import data.policy + +# pointless, as rules in own package can be referenced without the import +import data.policy.rule +``` + +**Prefer** +```rego +package policy +``` + +## Rationale + +There's no point importing the own package, or rules from the same package, as both can be referenced just as well +without the import. + +## Exceptions + +While it may not be the best way use a reference from the same package, longer references than the package, or the +package plus a rule, are at least not pointless, and as such not flagged by this rule. + +```rego +package policy + +# this is allowed, but consider using the reference directly rather than importing it +import data.policy.a.b.c +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + pointless-import: + # one of "error", "warning", "ignore" + level: error +``` diff --git a/docs/projects/regal/rules/imports/prefer-package-imports.md b/docs/projects/regal/rules/imports/prefer-package-imports.md new file mode 100644 index 0000000000..cdf0d5c1f0 --- /dev/null +++ b/docs/projects/regal/rules/imports/prefer-package-imports.md @@ -0,0 +1,65 @@ +# prefer-package-imports + +**Summary**: Prefer importing packages over rules + +**Category**: Imports + +**Type**: Aggregate - only runs when more than one file is provided for linting + +**Avoid** +```rego +package policy + +# Rule imported directly +import data.users.first_names + +has_waldo if { + # Not obvious where "first_names" comes from + "Waldo" in first_names +} +``` + +**Prefer** +```rego +package policy + +# Package imported rather than rule +import data.users + +has_waldo if { + # Obvious where "first_names" comes from + "Waldo" in users.first_names +} +``` + +## Rationale + +Importing packages and using the package name as a "namespace" for imported rules and functions tends to make your code +easier to follow. This is especially true for large policies, where the distance from the import to actual use may be +several hundreds of lines. + +## Exceptions + +Regal has no way of knowing whether an import points to a rule, function or some external data — only that it doesn't +point to a package. Use the `ignore-import-paths` configuration option if you want to make exceptions for e.g. imports +of external data, or use the various ignore options to ignore entire files. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + prefer-package-imports: + # one of "error", "warning", "ignore" + level: error + ignore-import-paths: + # Make an exception for some specific import paths + - data.permissions.admin.users +``` + +## Related Resources + +- Rego Style Guide: [Prefer importing packages over rules and functions](https://github.com/StyraInc/rego-style-guide#prefer-importing-packages-over-rules-and-functions) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/prefer-package-imports/prefer_package_imports.rego) diff --git a/docs/projects/regal/rules/imports/redundant-alias.md b/docs/projects/regal/rules/imports/redundant-alias.md new file mode 100644 index 0000000000..4f3292fa2f --- /dev/null +++ b/docs/projects/regal/rules/imports/redundant-alias.md @@ -0,0 +1,41 @@ +# redundant-alias + +**Summary**: Redundant alias + +**Category**: Imports + +**Avoid** +```rego +package policy + +import data.users.permissions as permissions +``` + +**Prefer** +```rego +package policy + +import data.users.permissions +``` + +## Rationale + +The last component of an import path can always be referenced by the last +component of the import path itself inside the package in which it's imported. +Using an alias with the same name is thus redundant, and should be omitted. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + redundant-alias: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/redundant-alias/redundant_alias.rego) diff --git a/docs/projects/regal/rules/imports/redundant-data-import.md b/docs/projects/regal/rules/imports/redundant-data-import.md new file mode 100644 index 0000000000..4375fac7b1 --- /dev/null +++ b/docs/projects/regal/rules/imports/redundant-data-import.md @@ -0,0 +1,32 @@ +# redundant-data-import + +**Summary**: Redundant import of data + +**Category**: Imports + +**Avoid** +```rego +package policy + +import data +``` + +## Rationale + +Just like `input`, `data` is always globally available and does not need to be imported. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + redundant-data-import: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/redundant-data-import/redundant_data_import.rego) diff --git a/docs/projects/regal/rules/imports/unresolved-import.md b/docs/projects/regal/rules/imports/unresolved-import.md new file mode 100644 index 0000000000..e095d0cdb1 --- /dev/null +++ b/docs/projects/regal/rules/imports/unresolved-import.md @@ -0,0 +1,61 @@ +# unresolved-import + +**Summary**: Unresolved import + +**Category**: Imports + +**Type**: Aggregate - only runs when more than one file is provided for linting + +**Avoid** + +Imports that can't be resolved. + +## Rationale + +OPA does no compile time checks to ensure that references in imports _resolve_ to anything, and unresolved references at +runtime are simply **undefined**. This is not a bug in OPA, but a necessary feature to allow for dynamic loading of data +and policy at runtime. The fact that it's not a bug does however not mean that it can't be +[a problem](https://github.com/open-policy-agent/opa/issues/491)! A simple typo, a refactoring, or a mistake, could +easily lead to an an import being unresolved, and as such undefined at runtime. + +This rule takes a stricter approach to imports, and will have Regal try to resolve them by scanning all the policies it +is provided for **packages**, **rules** and **functions** that may resolve the import. Note that Regal does not scan any +_data_ files. If no reference is found, the rule will flag it as unresolved. + +Since unresolved imports may be perfectly valid — for example when an import points to data — this rule provides an +option in its configuration to except certain paths from being checked. These paths may even contain a wildcard suffix +to indicate that any path past the wildcard (e.g. `data.users.*`) should be ignored. It is also possible to use a +regular [ignore directive](https://openpolicyagent.org/projects/regal#inline-ignore-directives): + +```rego +package example + +# this is provided as data! +# regal ignore:unresolved-import +import data.users +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + unresolved-import: + # one of "error", "warning", "ignore" + level: error + # list of paths that should be ignored + # these may be paths to data, or rules that may + # not be present at the time of linting + except-imports: + - data.identity.users + - data.permissions.* +``` + +## Related Resources + +- OPA Docs: [Imports](https://www.openpolicyagent.org/docs/policy-language/#imports) +- OPA Docs: [Collaboration Using Import](https://www.openpolicyagent.org/docs/faq/#collaboration-using-import) +- OPA Issues: [Missing import should create error](https://github.com/open-policy-agent/opa/issues/491) + - GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/unresolved-import/unresolved_import.rego) diff --git a/docs/projects/regal/rules/imports/unresolved-reference.md b/docs/projects/regal/rules/imports/unresolved-reference.md new file mode 100644 index 0000000000..972380a592 --- /dev/null +++ b/docs/projects/regal/rules/imports/unresolved-reference.md @@ -0,0 +1,45 @@ +# unresolved-reference + +**Summary**: Unresolved Reference + +**Category**: Imports + +**Avoid** +References to unresolved packages and rules. + +## Rationale + +This rule is similar to `unresolved-import` rule and has the same rationale: +to avoid accidentally referencing rules and data that does not exist. +As the name suggests, this rule is stricter than the `unresolved-import` rule, +and will check for references to packages and rules that may not exist throughout the entire policy, +rather than just the imports. + +This rule will have Regal try to resolve all references to external packages and rules by scanning all the policies it is +provided for **packages**, **rules** and **functions** that may resolve the reference. Note that Regal does not scan any +_data_ files. If no reference is found, the rule will flag it as unresolved. + +## Configuration Options + +This linter rule provides the following configuration options: +```yaml +rules: + imports: + unresolved-reference: + # one of "error", "warning", "ignore" + level: error + # list of paths that should be ignored + # these may be paths to data, or rules that may + # not be present at the time of linting + # using glob syntax + except-paths: + - data.identity.users + - data.permissions.* +``` + +## Related Resources + +- Unresolved Import Rule: [unresolved-import](./unresolved-import) +- OPA Docs: [Imports](https://www.openpolicyagent.org/docs/policy-language/#imports) +- OPA Docs: [Collaboration Using Import](https://www.openpolicyagent.org/docs/faq/#collaboration-using-import) +- OPA Issues: [Missing import should create error](https://github.com/open-policy-agent/opa/issues/491) diff --git a/docs/projects/regal/rules/imports/use-rego-v1.md b/docs/projects/regal/rules/imports/use-rego-v1.md new file mode 100644 index 0000000000..4af84009f0 --- /dev/null +++ b/docs/projects/regal/rules/imports/use-rego-v1.md @@ -0,0 +1,114 @@ +# use-rego-v1 + +**Summary**: Use `import rego.v1` + +**Category**: Imports + +**Automatically fixable**: [Yes](https://openpolicyagent.org/projects/regal/fixing) + +## Notice: Rule disabled by default since OPA 1.0 + +Since Regal v0.30.0, this rule is only enabled for projects that have either been explicitly configured to target +versions of OPA before 1.0, or if no configuration is provided — where Regal is able to determine that an older version +of OPA/Rego is being targeted. Consult the documentation on Regal's +[configuration](https://openpolicyagent.org/projects/regal#configuration) for information on how to best work with older versions of +OPA and Rego. + +Since OPA v1.0, the `rego.v1` import is effectively a no-op. Developers working on a **policy library**, or other +Rego polices that are expected to be used with many different OPA versions, may however benefit from enabling this rule, +as having an `import rego.v1` in the policy ensures that v1 keywords will work correctly with OPA versions both +before and after OPA v1.0. + +**Avoid** +```rego +package policy + +# before OPA v0.59.0, this was best practice +import future.keywords.contains +import future.keywords.if + +report contains item if { + # ... +} +``` + +**Prefer** +```rego +package policy + +# with OPA v0.59.0 and later, use import rego.v1 instead +# with OPA v1.0 and later, this import is unnecessary +import rego.v1 + +report contains item if { + # ... +} +``` + +## Rationale + +OPA [v0.59.0](https://github.com/open-policy-agent/opa/releases/tag/v0.59.0) introduced a new `rego.v1` import, which +allows policy authors to prepare for language changes coming in the future OPA 1.0 release. Some notable changes include: + +- All "future" keywords that currently must be imported through `import future.keywords` will be part of Rego by + default, without the need to first import them +- The `if` keyword will be required before the body of a rule +- The `contains` keyword will be required when declaring a multi-value rule (partial set rule) +- Deprecated built-in functions will be removed + +Using `import rego.v1` ensures that these requirements are met in any package including the import, and tools like +`opa check` and `opa fmt` have been updated to help users in this transition. + +See the [OPA v0.59.0 release notes](https://github.com/open-policy-agent/opa/releases/tag/v0.59.0) for more details. + +### Capabilities + +If you aren't yet using OPA v0.59.0 or later, it is recommended that you use the +[capabilities](https://openpolicyagent.org/projects/regal#capabilities) setting in your Regal configuration file to tell Regal what +version of OPA to target. This way you won't need to disable rules that require capabilities that aren't in the version +of OPA you're targeting, and allows for a smoother transition to newer versions of OPA when you're ready for that. +Another benefit of using capabilities is that Regal will include notices in the report when there are rules that have +been disabled due to missing capabilities, kindly reminding you of them, but without having the command fail. + +In the example below we're using the capabilities setting to target OPA v0.55.0 (where `import rego.v1` is not +available): + +**.regal/config.yaml** or **.regal.yaml** +```yaml +capabilities: + from: + engine: opa + version: v0.55.0 +``` + +Linting with the above configuration will exclude the `use-rego-v1` rule, but add a notice to the report reminding you +that it was disabled due to missing capabilities: + +```shell +$ regal lint bundle +131 files linted. No violations found. 1 rule skipped: +- use-rego-v1: Missing capability for `import rego.v1` +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + imports: + use-rego-v1: + # one of "error", "warning", "ignore" + level: error + +# rather than disabling this rule, use the capabilities setting +# to tell Regal which version of OPA to target: +capabilities: + from: + engine: opa + version: v0.58.0 +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/use-rego-v1/use_rego_v1.rego) diff --git a/docs/projects/regal/rules/index.md b/docs/projects/regal/rules/index.md new file mode 100644 index 0000000000..207853c527 --- /dev/null +++ b/docs/projects/regal/rules/index.md @@ -0,0 +1,19 @@ +--- +title: Rules +sidebar_position: 2 +--- + + +# Rules + +import rules from "@generated/regal/default/rules.json"; + + + +This page contains an index of all {rules.length} +Regal rules. + +import RulesTable from '@site/src/components/projects/regal/RulesTable'; + + + diff --git a/docs/projects/regal/rules/index.md.yaml b/docs/projects/regal/rules/index.md.yaml new file mode 100644 index 0000000000..f943cff953 --- /dev/null +++ b/docs/projects/regal/rules/index.md.yaml @@ -0,0 +1,2 @@ +title: Rules +sidebar_position: 2 diff --git a/docs/projects/regal/rules/performance/defer-assignment.md b/docs/projects/regal/rules/performance/defer-assignment.md new file mode 100644 index 0000000000..94fb6b224f --- /dev/null +++ b/docs/projects/regal/rules/performance/defer-assignment.md @@ -0,0 +1,69 @@ +# defer-assignment + +**Summary**: Assignment can be deferred + +**Category**: Performance + +**Avoid** +```rego +package policy + +allow if { + resp := http.send({"method": "GET", "url": "http://example.com"}) + + # this check does not depend on the response above + # and thus the resp := ... assignment can be deferred to + # after the check + input.user.name in allowed_users + + resp.status_code == 200 + + # more done with response here +} +``` + +**Prefer** +```rego +package policy + +allow if { + input.user.name in allowed_users + + # the next expression *does* depend on `resp` + resp := http.send({"method": "GET", "url": "http://example.com"}) + + resp.status_code == 200 + + # more done with response here +} +``` + +## Rationale + +Assignments are normally cheap, but certainly not always. If the right-hand side of an assignment is expensive, +deferring the assignment to where it's needed can save a considerable amount of time. Even for less expensive +assignments, code tends to be more readable when assignments are placed close to where they're used. + +This rule uses a fairly simplistic heuristic to determine if an assignment can be deferred: + +- The next expression is not an assignment +- The next expression does not depend on the assignment +- The next expression does not initialize iteration + +It is possible that the rule will be improved to cover more cases in the future. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + performance: + defer-assignment: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/performance/defer-assignment/defer_assignment.rego) diff --git a/docs/projects/regal/rules/performance/index.md b/docs/projects/regal/rules/performance/index.md new file mode 100644 index 0000000000..d1140258e1 --- /dev/null +++ b/docs/projects/regal/rules/performance/index.md @@ -0,0 +1,14 @@ +--- +title: Performance +sidebar_position: 4 +--- + + +# Performance + +Rules to help identify possible performance issues. + +import RulesTable from '@site/src/components/projects/regal/RulesTable'; + + + diff --git a/docs/projects/regal/rules/performance/index.md.yaml b/docs/projects/regal/rules/performance/index.md.yaml new file mode 100644 index 0000000000..b9df53c1a0 --- /dev/null +++ b/docs/projects/regal/rules/performance/index.md.yaml @@ -0,0 +1,2 @@ +title: Performance +sidebar_position: 4 diff --git a/docs/projects/regal/rules/performance/non-loop-expression.md b/docs/projects/regal/rules/performance/non-loop-expression.md new file mode 100644 index 0000000000..36b5885418 --- /dev/null +++ b/docs/projects/regal/rules/performance/non-loop-expression.md @@ -0,0 +1,93 @@ +# non-loop-expression + +**Summary**: Non loop expression in loop + +**Category**: Performance + +**Avoid** + +```rego +package policy + +allow if { + some email in input.emails + "admin" in input.roles # <- this is not required in the loop + endswith(email, "@example.com") +} +``` + +**Prefer** + +```rego +package policy + +allow if { + "admin" in input.roles # <- moved out of the loop + some email in input.emails + endswith(email, "@example.com") +} +``` + +## Rationale + +Expressions in loops are evaluated in each iteration of the loop. Expressions +that do not depend on the loop variable should be moved out of the loop to +save computation time. + +'Loops' in Rego refers to anywhere a rule branches, for example: + +- `some foo, bar in data.baz` +- `foo := data.baz[_]` (prefer using `some`) +- `walk(data.baz, [path, value])` +- ... + +## Exceptions + +This rule cannot yet detect the following cases. + +Expressions overly nested in more than one loop: + +```rego +package policy + +allow if { + some role in data.roles + # <--- Should be Here + some permission in data.permissions[role] + startswith(role, "admin-") # <- this is not required in the permission loop + operation.permission == permission +} +``` + +Expressions nested within comprehensions: + +```rego +package policy + +allow if { + roles := {role | + prefix := data.prefix + # <--- Should be Here + some role in data.roles + prefix != "" # <- this is not required in the role loop + startswith(role, prefix) + } +} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + performance: + non-loop-expression: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [defer-assignment](https://openpolicyagent.org/projects/regal/rules/performance/defer-assignment) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/performance/non-loop-expression/non_loop_expression.rego) diff --git a/docs/projects/regal/rules/performance/walk-no-path.md b/docs/projects/regal/rules/performance/walk-no-path.md new file mode 100644 index 0000000000..dc942ffc93 --- /dev/null +++ b/docs/projects/regal/rules/performance/walk-no-path.md @@ -0,0 +1,89 @@ +# walk-no-path + +**Summary**: Call to `walk` can be optimized + +**Category**: Performance + +**Avoid** +```rego +package policy + +allow if { + # traverse potentially nested permissions structure looking + # for an admin role, but notice how the path is never referenced + # later + walk(user.permissions, [path, value]) + + value.type == "role" + value.name == "admin" +} +``` + +**Prefer** +```rego +package policy + +allow if { + # replacing `path` with a wildcard variable tells the evaluator that it won't + # have to build the path array for each node `walk` traverses, thereby avoiding + # unnecessary allocations + walk(user.permissions, [_, value]) + + value.type == "role" + value.name == "admin" +} +``` + +## Rationale + +The primary purpose of the `walk` function is to traverse nested data structures, and often at an arbitrary depth. +Each node traversed "produces" a path/value pair, where the path is an array of keys that lead to the current node, +and the value is the current node itself. Most often, rules only need to account for the value of the node and not the +path, and when that is the case, using a wildcard variable (`_`) in place of path tells the evaluation engine that +there's no need to build the path array for each node traversed, thereby avoiding unnecessary allocations. This can +have a big impact on performance when huge data structures are traversed! + +More concretely, `walk`ing without generating the path array cuts down evaluation time by about 33%, and reduces the +number of allocations by about 40%. + +**Trivia**: this optimization was originally made in OPA to improve the performance of Regal, where `walk` is used +extensively to traverse the AST of the policy being linted. + +## Exceptions + +This rule can only optimize `walk` calls where the path/value array is provided as a second argument to `walk`, and +**not** when assigned using `:=`: + +```rego +package policy + +allow if { + # this can't be optimized, as the `walk` function can't + # "see" the array assignment on the left hand side + [path, value] := walk(user.permissions) + + value.type == "role" + value.name == "admin" +} +``` + +For this reason, and a few historic ones, using the second argument for the return value is the preferred way to use +`walk`, which is [unique](https://openpolicyagent.org/projects/regal/rules/style/function-arg-return#exceptions) for the walk built-in +function. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + performance: + walk-no-path: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [function-arg-return](https://openpolicyagent.org/projects/regal/rules/style/function-arg-return) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/performance/walk-no-path/walk_no_path.rego) diff --git a/docs/projects/regal/rules/performance/with-outside-test-context.md b/docs/projects/regal/rules/performance/with-outside-test-context.md new file mode 100644 index 0000000000..68e58d5411 --- /dev/null +++ b/docs/projects/regal/rules/performance/with-outside-test-context.md @@ -0,0 +1,89 @@ +# with-outside-test-context + +**Summary**: `with` used outside of test context + +**Category**: Performance + +**Avoid** +```rego +package policy + +allow if { + some user in data.users + + # mock input to pass data to `allowed_user` rule + allowed_user with input as {"user": user} +} + +verified := io.jwt.verify_rs256(input.token, data.keys.verification_key) + +allowed_user := input.user if { + # this expensive rule will be evaluated for each user! + verified + "admin" in input.user.roles +} +``` + +**Prefer** +```rego +package policy + +allow if { + some user in data.users + + allowed_user({"user": user}) +} + +verified := io.jwt.verify_rs256(input.token, data.keys.verification_key) + +allowed_user(user) := user if { + # this expensive rule will be evaluated only once + verified + "admin" in user.roles +} +``` + +## Rationale + +The `with` keyword exists primarily as a way to easily mock `input` or `data` in unit tests. While it's not forbidden to +use `with` in other contexts, and it's occasionally useful to do so, `with` is not optimized for performance and can +easily result in increased evaluation time if not used with care. + +One optimization that OPA does all the time is to cache the result of rule evaluation. If OPA needs to evaluate the same +rule more than once as part of evaluating a query, the result of the first evaluation is memorized and the cost of +subsequent evaluations is essentially zero. Caching however assumes that the conditions that produced the result of the +first evaluation won't _change_ — and changing the conditions (i.e. `input` or `data`) for evaluation is the very +purpose of `with`! This means that rules evaluated in the context of `with` won't be cached, and an expensive operation, +like the `io.jwt.verify_rs256` built-in function called in the examples above would be evaluated for each `user` in +`data.users`, even if the `with` clause in this case doesn't change any value that the JWT verification function depends +on. + +## Exceptions + +The obvious exception is stated already in the title of this rule: unit tests! Use `with` as much as want here, as that +is what `with` is for. + +Using `with` outside the context of unit tests is most commonly seen in policies using +[dynamic policy composition](https://www.styra.com/blog/dynamic-policy-composition-for-opa/), which typically involves +a "main" policy dispatching to a number of other policies and aggregating the result of evaluating each one. In this +scenario it's quite common to need to alter either `input` or `data` before evaluating a policy or rule, and `with` is +commonly used for this purpose. If you need to use `with` outside of tests, make sure that rules evaluated frequently +are done so outside of the scope of `with` to avoid performance issues. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + performance: + with-outside-test-context: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [With Keyword](https://www.openpolicyagent.org/docs/policy-language/#with-keyword) +- Styra Blog: [Dynamic Policy Composition for OPA](https://www.styra.com/blog/dynamic-policy-composition-for-opa/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/performance/with-outside-test-context/with_outside_test_context.rego) diff --git a/docs/projects/regal/rules/style/avoid-get-and-list-prefix.md b/docs/projects/regal/rules/style/avoid-get-and-list-prefix.md new file mode 100644 index 0000000000..a4679e1dd6 --- /dev/null +++ b/docs/projects/regal/rules/style/avoid-get-and-list-prefix.md @@ -0,0 +1,61 @@ +# avoid-get-and-list-prefix + +**Summary**: Avoid `get_` and `list_` prefix for rules and functions + +**Category**: Style + +**Avoid** +```rego +package policy + +get_first_name(user) := split(user.name, " ")[0] + +# Partial rule, so a set of users is to be expected +list_developers contains user if { + some user in data.application.users + user.type == "developer" +} +``` + +**Prefer** +```rego +package policy + +# "get" is implied +first_name(user) := split(user.name, " ")[0] + +# Partial rule, so a set of users is to be expected +developers contains user if { + some user in data.application.users + user.type == "developer" +} +``` + +## Rationale + +Since Rego evaluation is generally free of side effects, any rule or function is essentially a "getter". Adding a +`get_` prefix to a rule or function (like `get_resources`) thus adds little of value compared to just naming it +`resources`. Additionally, the type and return value of the rule should serve to tell whether a rule might return a +single value (i.e. a complete rule) or a collection (a partial rule). + +## Exceptions + +Using `is_`, or `has_` for boolean helper functions, like `is_admin(user)` may be easier to comprehend than +`admin(user)`. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + avoid-get-and-list-prefix: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Avoid prefixing rules and functions with `get_` or `list_`](https://github.com/StyraInc/rego-style-guide#avoid-prefixing-rules-and-functions-with-get_-or-list_) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/avoid-get-and-list-prefix/avoid_get_and_list_prefix.rego) diff --git a/docs/projects/regal/rules/style/chained-rule-body.md b/docs/projects/regal/rules/style/chained-rule-body.md new file mode 100644 index 0000000000..c746ffef74 --- /dev/null +++ b/docs/projects/regal/rules/style/chained-rule-body.md @@ -0,0 +1,58 @@ +# chained-rule-body + +**Summary**: Avoid chaining rule bodies + +**Category**: Style + +**Avoid** +```rego +package policy + +has_x_or_y { + input.x +} { + input.y +} +``` + +**Prefer** +```rego +package policy + +has_x_or_y { + input.x +} + +has_x_or_y { + input.y +} +``` + +## Rationale + +If the head of the rule is same, it's possible to chain multiple rule bodies together to obtain the same result. This +form was more common in the past, but is no longer recommended as it is arguably less readable, and less likely to be +understood by people new to Rego. + +## Exceptions + +The `opa fmt` command will automatically "unchain" chained rule bodies, so if you have enabled the [opa-fmt](opa-fmt) +rule, you may safely configure the level of this rule to `ignore`. While we normally don't include style rules covered +by `opa fmt`, this one is peculiar enough that we felt it was worthy of an exception. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + chained-rule-body: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Incremental Definitions](https://www.openpolicyagent.org/docs/policy-language/#incremental-definitions) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/chained-rule-body/chained_rule_body.rego) diff --git a/docs/projects/regal/rules/style/comprehension-term-assignment.md b/docs/projects/regal/rules/style/comprehension-term-assignment.md new file mode 100644 index 0000000000..8f8d5c5c58 --- /dev/null +++ b/docs/projects/regal/rules/style/comprehension-term-assignment.md @@ -0,0 +1,77 @@ +# comprehension-term-assignment + +**Summary**: Assignment can be moved to comprehension term + +**Category**: Style + +**Avoid** +```rego +package policy + +names := [name | + some user in input.users + name := user.name # redundant assignment +] +``` + +**Prefer** +```rego +package policy + +names := [user.name | + some user in input.users +] + +# which in this case can be made a one-liner +names := [user.name | some user in input.users] +``` + +## Rationale + +Adding an intermediate assignment in a comprehension body to a variable used as the comprehension term (i.e. the value +to the left side of `|` in a comprehension) is redundant, as the value can be used directly as the comprehension term. +Making code as compact as possible should never be a goal in itself, but the same is true for making code needlessly +verbose. And in cases like `names := [user.name | some user in input.users]`, adding an intermediate assignment does +nothing to improve readability. + +## Exceptions + +This rule will only flag simple assignments where the value could be moved directly into the comprehension term. More +complex assignments involving dynamic references or function calls, will not be considered as violations. + +Example: + +```rego +first_names := [first_name | + some user in input.users + first_name := capitalize(user.name.split(" ")[0]) +] +``` + +While it's possible to move the value of the `first_name` assignment directly to the comprehension term, it's arguably +less readable, as it makes it harder to see that the form is a comprehension to begin with. + +```rego +# Not recommended +first_names := [capitalize(user.name.split(" ")[0]) | + some user in input.users +] +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + comprehension-term-assignment: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Comprehensions](https://www.openpolicyagent.org/docs/policy-language/#comprehensions) +- Regal Docs: [pointless-reassignment](https://openpolicyagent.org/projects/regal/rules/style/pointless-reassignment) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/comprehension-term-assignment/comprehension_term_assignment.rego) diff --git a/docs/projects/regal/rules/style/default-over-else.md b/docs/projects/regal/rules/style/default-over-else.md new file mode 100644 index 0000000000..d2f21dcfb9 --- /dev/null +++ b/docs/projects/regal/rules/style/default-over-else.md @@ -0,0 +1,97 @@ +# default-over-else + +**Summary**: Prefer default assignment over fallback `else` + +**Category**: Style + +**Avoid** +```rego +package policy + +permissions := ["read", "write"] if { + input.user == "admin" +} else := ["read"] +``` + +**Prefer** +```rego +package policy + +default permissions := ["read"] + +permissions := ["read", "write"] if { + input.user == "admin" +} +``` + +## Rationale + +The `else` keyword has a single purpose in Rego — to allow a policy author to control the order of evaluation. Whether +several `else`-clauses are chained or not, it's common to use a last "fallback" `else` to cover all cases not covered by +the conditions in the preceding `else`-bodies. A kind of "catch all", or "default" condition. This is useful, but Rego +arguably provides a more idiomatic construct for default assignment: the +[default keyword](https://www.openpolicyagent.org/docs/policy-language/#default-keyword). + +While the end result is the same, default assignment has the benefit of more clearly — and **before** the conditional +assignments — communicating what the *safe* option is. This is particularly important for +[entrypoint](https://openpolicyagent.org/projects/regal/rules/idiomatic/no-defined-entrypoint) rules, where the +default value of a rule is a part of the rule's contract. + +## Exceptions + +OPA [v0.55.0](https://github.com/open-policy-agent/opa/releases/tag/v0.55.0) introduced support for the default keyword +for custom functions. This means that `else` fallbacks in functions may now be rewritten to use default assignment too: + +```rego +package policy + +first_name(full_name) := split(full_name, " ")[0] if { + full_name != "" +} else := "Unknown" +``` + +Could now be written as: + +```rego +package policy + +default first_name(_) := "Unknown" + +first_name(full_name) := split(full_name, " ")[0] if { + full_name != "" +} +``` + +Default value assignment for functions however come with a big caveat — the default case will only be triggered if all +arguments passed to the function evaluate to a *defined value*. Thus, calling the `first_name` function from our above +example is **not** guaranteed to return a value of `"Unknown"`: + +```rego +# undefined if `input.name` is undefined +fname := first_name(input.name) +``` + +Whether deemed acceptable or not, this differs enough from default assignment of rules to make this preference opt-in +rather than opt-out. Use the `prefer-default-functions` configuration option to control whether `default` assignment +should be preferred over `else` fallbacks also for custom functions. The default value (no pun intended!) of this config +option is `false`. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + default-over-else: + # one of "error", "warning", "ignore" + level: error + # whether to prefer default assignment over + # `else` fallbacks for custom functions + prefer-default-functions: false +``` + +## Related Resources + +- OPA Docs: [Default Keyword](https://www.openpolicyagent.org/docs/policy-language/#default-keyword) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/default-over-else/default_over_else.rego) diff --git a/docs/projects/regal/rules/style/default-over-not.md b/docs/projects/regal/rules/style/default-over-not.md new file mode 100644 index 0000000000..115303b93b --- /dev/null +++ b/docs/projects/regal/rules/style/default-over-not.md @@ -0,0 +1,47 @@ +# default-over-not + +**Summary**: Prefer default assignment over negated condition + +**Category**: Style + +**Avoid** +```rego +package policy + +username := input.user.name + +username := "anonymous" if not input.user.name +``` + +**Prefer** +```rego +package policy + +default username := "anonymous" + +username := input.user.name +``` + +## Rationale + +While both forms are valid, using the `default` keyword to assign a constant value in the fallback case better +communicates intent, avoids negation where it isn't needed, and requires less instructions to evaluate. Note that this +rule only covers simple cases where one rule assigns the "happy" path, and another rule assigns on the same condition +negated. This is by design, as using `not` and negation may very well be the right choice for more complex cases! + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + default-over-not: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Default Keyword](https://www.openpolicyagent.org/docs/policy-language/#default-keyword) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/default-over-not/default_over_not.rego) diff --git a/docs/projects/regal/rules/style/detached-metadata.md b/docs/projects/regal/rules/style/detached-metadata.md new file mode 100644 index 0000000000..8a0fe84f0f --- /dev/null +++ b/docs/projects/regal/rules/style/detached-metadata.md @@ -0,0 +1,52 @@ +# detached-metadata + +**Summary**: Detached metadata annotation + +**Category**: Style + +**Avoid** +```rego +package authz + + # METADATA + # description: allow any requests by admin users + +allow if { + "admin" in input.user.roles +} +``` + +**Prefer** +```rego +package authz + +# METADATA +# description: allow any requests by admin users +allow if { + "admin" in input.user.roles +} +``` + +## Rationale + +Metadata annotations should be placed directly above the package, rule or function they are annotating. While OPA +accepts any number of newlines between an annotation and the package/rule it applies to, this makes it difficult to +connect the two when reading the policy. Always optimize for readability! + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + detached-metadata: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Annotations](https://www.openpolicyagent.org/docs/policy-language/#annotations) +- OPA Docs: [Accessing Annotations](https://www.openpolicyagent.org/docs/policy-language/#accessing-annotations) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/detached-metadata/detached_metadata.rego) diff --git a/docs/projects/regal/rules/style/double-negative.md b/docs/projects/regal/rules/style/double-negative.md new file mode 100644 index 0000000000..05bc3024af --- /dev/null +++ b/docs/projects/regal/rules/style/double-negative.md @@ -0,0 +1,57 @@ +# double-negative + +**Summary**: Avoid double negatives + +**Category**: Style + +**Avoid** +```rego +package negative + +fine if not not_fine + +with_friends if not without_friends + +not_fine := input.fine != true + +without_friends if count(input.friends) == 0 +``` + +**Prefer** +```rego +package negative + +fine if input.fine == true + +with_friends if count(input.friends) > 0 +``` + +## Rationale + +While rules using double negatives — like `not no_funds` — occasionally make sense, it is often worth considering +whether the rule could be rewritten without the negative. For example, `not no_funds` could be rewritten as `funds` or +`has_funds`, or `funds_available`. + +Access control policy often includes rules using some form of double negatives, like `allow if not deny`. That's +considered OK, and the `double-negative` rule is limited to check for a limited list of words: + +- `not cannot_` +- `not no_` +- `not non_` +- `not not_`, + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + double-negative: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/double-negative/double_negative.rego) diff --git a/docs/projects/regal/rules/style/external-reference.md b/docs/projects/regal/rules/style/external-reference.md new file mode 100644 index 0000000000..1229372dd4 --- /dev/null +++ b/docs/projects/regal/rules/style/external-reference.md @@ -0,0 +1,93 @@ +# external-reference + +**Summary**: External reference in function + +**Category**: Style + +**Avoid** +```rego +package policy + +# Depends on both `input` and `data` +is_preferred_login_method(method) if { + preferred_login_methods := {login_method | + some login_method in data.authentication.all_login_methods + login_method in input.user.login_methods + } + method in preferred_login_methods +} +``` + +**Prefer** +```rego +package policy + +# Depends only on function arguments +is_preferred_login_method(method, user, all_login_methods) if { + preferred_login_methods := {login_method | + some login_method in all_login_methods + login_method in user.login_methods + } + method in preferred_login_methods +} +``` + +## Rationale + +What separates functions from rules is that they accept arguments. While a function also may reference anything from +`input`, `data` or other rules declared in a policy, these references create dependencies that aren't obvious simply by +checking the function signature, and it makes it harder to reuse that function in other contexts. Additionally, +functions that only depend on their arguments are easier to test standalone. + +## Exceptions + +Rego does not provide first-class functions — functions can't be passed as arguments to other functions. Therefore, this +rule allows functions to freely reference (i.e. call) _other functions_, whether built-in functions, or custom functions +defined in the same package or elsewhere, and these do not count as "external references" simply because there is not +other way to import them into the function body. + +```rego +package policy + +first_name(full_name) := capitalized { + first_name := split(full_name, " ")[0] + + # while data.utils.capitalize is an external reference, it's not flagged + # as such, since there is no way to import it via function arguments + capitalized := data.utils.capitalize(first_name) +} +``` + +### Changed default behavior since Regal v0.33.0 + +While we still consider it a best practice to pass any dependencies of a function in its arguments, the previous +(non-configurable) default of not allowing **any** external references was often considered too distracting. This led +to many disabling this rule entirely, or used inline ignore directives where this would be reported. Even in Regal's own +policies, there were quite a few locations where the latter option was used. + +From v0.33.0 and onwards, this rule's default has been relaxed to allow 2 external references in any given function +definition, and the new `max-allowed` configuration option allows changing this value to whatever feels like a +reasonable default. If you previously disabled this rule in your projects, consider enabling it again configured to +match your preference. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + external-reference: + # one of "error", "warning", "ignore" + level: error + # the number of external references to allow for any given function + # + # introduced in v0.33.0 and defaults to 2. set to 0 to revert to + # original behavior to not allow any external references + max-allowed: 2 +``` + +## Related Resources + +- Rego Style Guide: [Prefer using arguments over input, data or rule references](https://github.com/StyraInc/rego-style-guide#prefer-using-arguments-over-input-data-or-rule-references) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/external-reference/external_reference.rego) diff --git a/docs/projects/regal/rules/style/file-length.md b/docs/projects/regal/rules/style/file-length.md new file mode 100644 index 0000000000..483e4fd6bf --- /dev/null +++ b/docs/projects/regal/rules/style/file-length.md @@ -0,0 +1,48 @@ +# file-length + +**Summary**: Max file length exceeded + +**Category**: Style + +**Avoid** + +Excessively large policy files. + +**Prefer** + +Splitting large policy files into smaller ones. + +## Rationale + +Putting too much logic into a single file makes your policy harder to browse, read and maintain. Splitting logic into +several smaller files, and composing policy by proper use of packages and imports, highlights dependencies and +makes it easier to reason about. + +Note that even a single **package** may be split up across several files! This is sometimes useful when different +features or functions belong in the same "group", but are not directly related to each other. An example of this could +be having a single package for configuration split across different files for different parts or functions of that +configuration. + +As an added bonus, some tools — like Regal! — may even benefit from avoiding huge files as they process files in +parallel and thus will be able to handle many smaller files faster than a few large ones. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + file-length: + # one of "error", "warning", "ignore" + level: error + # default limit is 500 lines + max-file-length: 500 +``` + +## Related Resources + +- Styra Blog: [Dynamic Policy Composition](https://www.styra.com/blog/dynamic-policy-composition-for-opa/) +- Regal Docs: [line-length](https://openpolicyagent.org/projects/regal/rules/style/line-length) +- Regal Docs: [rule-length](https://openpolicyagent.org/projects/regal/rules/style/rule-length) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/file-length/file_length.rego) diff --git a/docs/projects/regal/rules/style/function-arg-return.md b/docs/projects/regal/rules/style/function-arg-return.md new file mode 100644 index 0000000000..55a3bda457 --- /dev/null +++ b/docs/projects/regal/rules/style/function-arg-return.md @@ -0,0 +1,70 @@ +# function-arg-return + +**Summary**: Return value assigned in function argument + +**Category**: Style + +**Avoid** +```rego +package policy + +has_email(user) if { + indexof(user.email, "@", i) + i != -1 +} +``` + +**Prefer** + +```rego +package policy + +has_email(user) if { + i := indexof(user.email, "@") + i != -1 +} +``` + +## Rationale + +Older Rego policies sometimes contain an unusual way to declare where the return value of a function call should be +stored — the last argument of the function. True to its Datalog roots, return values may be stored either using +assignment (i.e. `:=`) or by appending a variable name to the argument list of a function. While both forms are valid, +using assignment `:=` consistently is preferred. + +## Exceptions + +The `walk` built-in function is a special one, as it's the only one producing a *relation*. Therefore, it is okay to +treat it as one even in style, and: + +```rego +walk(object, [path, value]) +``` + +is arguably more idiomatic than: + +```rego +[path, value] := walk(object) +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + function-arg-return: + # one of "error", "warning", "ignore" + level: error + # list of function names to ignore + # * by default, walk is excepted from this rule + # * note that `print` is always ignored as it does not return a value + except-functions: + - walk +``` + +## Related Resources + +- Rego Style Guide: [Avoid using the last argument for the return value](https://github.com/StyraInc/rego-style-guide#avoid-using-the-last-argument-for-the-return-value) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/function-arg-return/function_arg_return.rego) diff --git a/docs/projects/regal/rules/style/index.md b/docs/projects/regal/rules/style/index.md new file mode 100644 index 0000000000..1a8910ea1a --- /dev/null +++ b/docs/projects/regal/rules/style/index.md @@ -0,0 +1,14 @@ +--- +title: Style +sidebar_position: 5 +--- + + +# Style + +Rules relating to code style. + +import RulesTable from '@site/src/components/projects/regal/RulesTable'; + + + diff --git a/docs/projects/regal/rules/style/index.md.yaml b/docs/projects/regal/rules/style/index.md.yaml new file mode 100644 index 0000000000..11a8676506 --- /dev/null +++ b/docs/projects/regal/rules/style/index.md.yaml @@ -0,0 +1,2 @@ +title: Style +sidebar_position: 5 diff --git a/docs/projects/regal/rules/style/line-length.md b/docs/projects/regal/rules/style/line-length.md new file mode 100644 index 0000000000..6390978e72 --- /dev/null +++ b/docs/projects/regal/rules/style/line-length.md @@ -0,0 +1,45 @@ +# line-length + +**Summary**: Line too long + +**Category**: Style + +**Avoid** + +Excessive line length. + +## Rationale + +Rego does not have many nested constructs, and long lines of code are thus almost never needed. If you find yourself +close to the maximum line length, consider refactoring your policy. + +The default maximum line length is 120 characters. + +## Exceptions + +On a few rare occasions, a single word — like a really long URL in a metadata annotation — can't possibly be made any +shorter. Using an ignore directive isn't an option in that context, and ignoring the whole file is rarely what you'll +want. The `non-breakable-word-threshold` configuration option allows defining a threshold length for when a single word +should be considered so long that the line length rule should ignore the line entirely. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + line-length: + # one of "error", "warning", "ignore" + level: error + # maximum line length + max-line-length: 120 + # if any single word on a line exceeds this length, ignore it + non-breakable-word-threshold: 100 +``` + +## Related Resources + +- Regal Docs: [file-length](https://openpolicyagent.org/projects/regal/rules/style/file-length) +- Regal Docs: [rule-length](https://openpolicyagent.org/projects/regal/rules/style/rule-length) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/line-length/line_length.rego) diff --git a/docs/projects/regal/rules/style/messy-rule.md b/docs/projects/regal/rules/style/messy-rule.md new file mode 100644 index 0000000000..c1c00e3061 --- /dev/null +++ b/docs/projects/regal/rules/style/messy-rule.md @@ -0,0 +1,55 @@ +# messy-rule + +**Summary**: Messy incremental rule + +**Category**: Style + +**Avoid** + +```rego +package policy + +allow if something + +unrelated_rule if { + # ... +} + +allow if something_else +``` + +**Prefer** + +```rego +package policy + +allow if something + +allow if something_else + +unrelated_rule if { + # ... +} +``` + +## Rationale + +Rules that are defined incrementally should have their definitions grouped together, as this makes the code easier to +follow. While this is mostly a style preference, having incremental rules grouped also allows editors like VS Code to +"know" that the rules belong together, allowing them to be smarter when displaying the symbols of a workspace. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + messy-rule: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/messy-rule/messy_rule.rego) diff --git a/docs/projects/regal/rules/style/mixed-iteration.md b/docs/projects/regal/rules/style/mixed-iteration.md new file mode 100644 index 0000000000..19acaa1739 --- /dev/null +++ b/docs/projects/regal/rules/style/mixed-iteration.md @@ -0,0 +1,71 @@ +# mixed-iteration + +**Summary**: Mixed iteration style + +**Category**: Style + +**Avoid** +```rego +package policy + +allow if { + # mixing 'some .. in' and reference iteration + some resource in input.assets[_] + + # do something with resource +} +``` + +**Prefer** +```rego +package policy + +allow if { + # using 'some .. in' iteration consistently + some asset in input.assets + some resource in asset + + # do something with resource +} + +# alternatively + +allow if { + # using reference iteration consistently + resource := input.assets[_][_] + + # do something with resource +} +``` + +## Rationale + +Using `some .. in` is often the [best choice](https://openpolicyagent.org/projects/regal/rules/style/prefer-some-in-iteration) for +iteration in modern Rego, as it clearly communicates what's going on and which variables will be bound in each +iteration. An alternative approach is to place variables (including the special "wildcard" variable `_`) in parts of a +references, which unless assigned elsewhere automatically will be bound to every possible value in the collection being +traversed (often called "output variables"). + +"Reference style" iteration is often preferred when deeply nested structures are traversed, as they allow +expressing that in a very concise (and sometimes, more performant) manner. + +While both forms of iteration are valid and have their place, mixing both forms in a single iteration is arguably +confusing. Feel free to choose either `some .. in` or reference style depending on your preference (and when in doubt, +use `some .. in`), but don't mix the two different styles in a single iteration expression. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + mixed-iteration: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [prefer-some-in-iteration](https://openpolicyagent.org/projects/regal/rules/style/prefer-some-in-iteration) +- OPA Docs: [Membership and iteration](https://www.openpolicyagent.org/docs/policy-language/#membership-and-iteration-in) diff --git a/docs/projects/regal/rules/style/no-whitespace-comment.md b/docs/projects/regal/rules/style/no-whitespace-comment.md new file mode 100644 index 0000000000..3a0a674698 --- /dev/null +++ b/docs/projects/regal/rules/style/no-whitespace-comment.md @@ -0,0 +1,55 @@ +# no-whitespace-comment + +**Summary**: Comment should start with whitespace + +**Category**: Style + +**Automatically fixable**: [Yes](https://openpolicyagent.org/projects/regal/fixing) + +**Avoid** + +```rego +package policy + +#Deny by default +default allow := false + +#Allow only admins +allow if "admin" in input.user.roles +``` + +**Prefer** + +```rego +package policy + +# Deny by default +default allow := false + +# Allow only admins +allow if "admin" in input.user.roles +``` + +## Rationale + +Comments should be preceded by a single space, as this makes them easier to read. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + no-whitespace-comment: + # one of "error", "warning", "ignore" + level: error + # optional pattern to except from this rule + # this example would allow comments like "#--" + # use or (`|`) to separate multiple patterns + except-pattern: "^--" +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/no-whitespace-comment/no_whitespace_comment.rego) diff --git a/docs/projects/regal/rules/style/opa-fmt.md b/docs/projects/regal/rules/style/opa-fmt.md new file mode 100644 index 0000000000..96e574fee1 --- /dev/null +++ b/docs/projects/regal/rules/style/opa-fmt.md @@ -0,0 +1,64 @@ +# opa-fmt + +**Summary**: File should be formatted with `opa fmt` + +**Category**: Style + +**Automatically fixable**: [Yes](https://openpolicyagent.org/projects/regal/fixing) + +**Avoid** + +Inconsistent style across policy files and repositories. + +## Rationale + +The `opa fmt` tool ensures consistent formatting across teams and projects. Unified formatting is a big win, and saves a +lot of time in code reviews arguing over details around style. + +A good idea could be to run `opa fmt --write` on save, which can be configured in most editors. + +**Tip**: `opa fmt` uses tabs for indentation. By default, GitHub uses 8 spaces to display tabs, which is arguably a bit +much. You can change this preference for your account in `github.com/settings/appearance`, or provide an `.editorconfig` +file in your policy repository, which will be used by GitHub (and other tools) to properly display your Rego files: + +```ini +[*.rego] +end_of_line = lf +insert_final_newline = true +charset = utf-8 +indent_style = tab +indent_size = 4 +``` + +## OPA Format with Rego v1 and v0 + +OPA 1.0 makes Rego v1 the default. This change mandated some changes to the +functionality of the `opa fmt` command and a number of new options for working +with mixed version code bases. See the +[OPA documentation](https://www.openpolicyagent.org/docs/cli/#opa-fmt) +for the command's options. + +In Regal, a v0 file will have the `opa-fmt` violation unless it's been formatted +with `opa fmt --v0-v1`. A v1 file will have the `opa-fmt` violation unless it's +been formatted with `opa fmt` (the rego.v1 keyword is permitted but not added). + +When formatting, a file expected to be v1 based on the configuration, but with +v0 syntax is still formatted as `opa fmt –-v0-v1`. Please see +[Configuring Rego Version](https://openpolicyagent.org/projects/regal#configuring-rego-version) +for more configuration help for multi version projects. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + opa-fmt: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [CLI Reference `opa fmt`](https://www.openpolicyagent.org/docs/cli/#opa-fmt) diff --git a/docs/projects/regal/rules/style/pointless-reassignment.md b/docs/projects/regal/rules/style/pointless-reassignment.md new file mode 100644 index 0000000000..dc7a78bba0 --- /dev/null +++ b/docs/projects/regal/rules/style/pointless-reassignment.md @@ -0,0 +1,60 @@ +# pointless-reassignment + +**Summary**: Pointless reassignment of variable + +**Category**: Style + +**Avoid** +```rego +package policy + +allow if { + users := all_users + any_admin(users) +} +``` + +**Prefer** +```rego +package policy + +allow if { + any_admin(all_users) +} +``` + +## Rationale + +Values and variables are immutable in Rego, so reassigning the value of one variable to another only adds noise. + +## Exceptions + +Reassigning the value of a long reference often helps readability, and especially so when it needs to be referenced +multiple times: + +```rego +package policy + +allow if { + users := input.context.permissions.users + any_admin(users) +} +``` + +This rule does not consider such assignments violations. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + pointless-reassignment: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/pointless-reassignment/pointless_reassignment.rego) diff --git a/docs/projects/regal/rules/style/prefer-snake-case.md b/docs/projects/regal/rules/style/prefer-snake-case.md new file mode 100644 index 0000000000..58f70a72b1 --- /dev/null +++ b/docs/projects/regal/rules/style/prefer-snake-case.md @@ -0,0 +1,50 @@ +# prefer-snake-case + +**Summary**: Prefer snake_case for names + +**Category**: Style + +**Avoid** +```rego +package policy + +# camelCase rule name +userIsAdmin if "admin" in input.user.roles +``` + +**Prefer** +```rego +package policy + +# snake_case rule name +user_is_admin if "admin" in input.user.roles +``` + +## Rationale + +The built-in functions use `snake_case` for naming — follow that convention for your own packages, rules, functions, +and variables, unless you have a really good reason not to. + +## Exceptions + +In many cases, you might not control the format of the `input` data — if the domain of a policy (e.g. Envoy) +mandates a different style, making an exception might seem reasonable. Adapting policy format after `input` is however +prone to inconsistencies, as you'll likely end up mixing different styles in the same policy (due to imports of common +code, etc). + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + prefer-snake-case: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Prefer snake_case for rule names and variables](https://github.com/StyraInc/rego-style-guide#prefer-snake_case-for-rule-names-and-variables) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/prefer-snake-case/prefer_snake_case.rego) diff --git a/docs/projects/regal/rules/style/prefer-some-in-iteration.md b/docs/projects/regal/rules/style/prefer-some-in-iteration.md new file mode 100644 index 0000000000..c253fd1d9c --- /dev/null +++ b/docs/projects/regal/rules/style/prefer-some-in-iteration.md @@ -0,0 +1,137 @@ +# prefer-some-in-iteration + +**Summary**: Prefer `some .. in` for iteration + +**Category**: Style + +**Avoid** +```rego +package policy + +engineering_roles := {"engineer", "dba", "developer"} + +engineers contains employee if { + employee := data.employees[_] + employee.role in engineering_roles +} +``` + +**Prefer** +```rego +package policy + +engineering_roles := {"engineer", "dba", "developer"} + +engineers contains employee if { + some employee in data.employees + employee.role in engineering_roles +} +``` + +## Rationale + +Using the `some .. in` construct for iteration removes ambiguity around iteration vs. membership checks, and is +generally more pleasant to read. Consider the following example: + +```rego +some_condition if { + other_rule[user] + # ... +} +``` + +Are we iterating users over a partial "other_rule" here, or checking if the set contains a user defined elsewhere? +Or is `other_rule` a map-generating rule, and we're checking for the existence of a key? We won't know without looking +elsewhere in the code. Using `some .. in` removes this ambiguity, and makes the intent clear without having to jump +around in the policy. + +Improved readability is not the only benefit of using `some .. in`. The `some` keyword ensures that the bindings +following the keyword are bound to the local scope, and modifications outside of e.g. a rule body won't affect how the +variables are evaluated. Consider the following simplified example to iterate over the keys of a map: + +```rego +package policy + +key_traversal if { + map[key] + # do something with key +} + + +key_traversal if { + some key in object.keys(map) + # do something with key +} +``` + +The two rules above are equivalent in that they both bind the variable `key` to the keys of `map`. The first +example would however change behavior entirely if a rule named `key` was introduced in the package, as the expression +would then mean "does map have key `key`?". While this isn't common, using `some .. in` means one less thing to worry +about. + +## Exceptions + +Deeply nested iteration is often easier to read using the more compact form. + +```rego +package policy + +# These rules are equivalent, but the more compact form is arguably easier to read + +any_user_is_admin if { + some user in input.users + some attribute in user.attributes + some role in attribute.roles + role == "admin" +} + +any_user_is_admin if { + input.users[_].attributes[_].roles[_] == "admin" +} + +# Using "if", we may even omit the brackets for single line rules +any_user_is_admin if input.users[_].attributes[_].roles[_] == "admin" +``` + +The `ignore-nesting-level` configuration option allows setting the threshold for nesting. Any level of nesting +**equal or greater than** the threshold won't be considered a violation. The default setting of `2` allows all _nested_ +iteration, but not e.g. `my_array[x]`. + +**Note:** not all nesting is _iteration_! The following example is considered to have a nesting level of `1`, as only +one of the variables (including wildcards: `_`) is an output variable bound in iteration: + +```rego +package policy + +example_users contains user if { + domain := "example.com" + user := input.sites[domain].users[_] +} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + prefer-some-in-iteration: + # one of "error", "warning", "ignore" + level: error + # except iteration if nested at or above the level i.e. setting of + # '2' will allow `input[_].users[_]` but not `input[_]` + ignore-nesting-level: 2 + # except iteration over items with sub-attributes, like + # `name := input.users[_].name` + # default is true + ignore-if-sub-attribute: true +``` + +## Related Resources + +- Rego Style Guide: [Prefer some .. in for iteration](https://github.com/StyraInc/rego-style-guide#prefer-some--in-for-iteration) +- Regal Docs: [Use `some` to declare output variables](https://openpolicyagent.org/projects/regal/rules/idiomatic/use-some-for-output-vars) +- OPA Docs: [Membership and Iteration: `in`](https://www.openpolicyagent.org/docs/policy-language/#membership-and-iteration-in) +- OPA Docs: [Some Keyword](https://www.openpolicyagent.org/docs/policy-language/#some-keyword) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/prefer-some-in-iteration/prefer_some_in_iteration.rego) diff --git a/docs/projects/regal/rules/style/rule-length.md b/docs/projects/regal/rules/style/rule-length.md new file mode 100644 index 0000000000..b3ff8ec48c --- /dev/null +++ b/docs/projects/regal/rules/style/rule-length.md @@ -0,0 +1,59 @@ +# rule-length + +**Summary**: Max rule length exceeded + +**Category**: Style + +**Avoid** + +Having too much logic placed in a single rule body. + +**Prefer** + +To use helper rules and functions to compose your rules. + +## Rationale + +Splitting up large rules into smaller ones, and liberally using helper rules and functions, makes your policy easier for +others to read and understand, and for yourself and your team to maintain. + +Note that this rule only counts the number of lines of a rule, and currently does not take into account the actual +content inside of it. Neither does it try to analyze the complexity of the code in the rule. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + rule-length: + # one of "error", "warning", "ignore" + level: error + # default limit is 30 lines + max-rule-length: 30 + # default limit is 60 lines for test rules (i.e. prefixed with 'test_') + max-test-rule-length: 60 + # whether to count comments as lines + # by default, this is set to false + count-comments: false + # except rules with empty bodies from this rule, as they're + # likely an assignment of long values rather than a "rule" + # with conditions: + # + # users := [ + # {"username": "ted"}, + # {"username": "alice"}, + # {"username": "bob"}, + # # ... many more lines + # ] + # + # the default value is true + except-empty-body: true +``` + +## Related Resources + +- Regal Docs: [file-length](https://openpolicyagent.org/projects/regal/rules/style/file-length) +- Regal Docs: [line-length](https://openpolicyagent.org/projects/regal/rules/style/line-length) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/rule-length/rule_length.rego) diff --git a/docs/projects/regal/rules/style/rule-name-repeats-package.md b/docs/projects/regal/rules/style/rule-name-repeats-package.md new file mode 100644 index 0000000000..5e01eff2ee --- /dev/null +++ b/docs/projects/regal/rules/style/rule-name-repeats-package.md @@ -0,0 +1,49 @@ +# rule-name-repeats-package + +**Summary**: Avoid repeating package path in rule names + +**Category**: Style + +**Avoid** +```rego +package policy.authz + +authz_allow if { + user.is_admin +} +``` + +**Prefer** +```rego +package policy.authz + +allow if { + user.is_admin +} +``` + +## Rationale + +When rules are referenced outside the package in which they are defined, they will be referenced using the package path. +For example, the `allow` rule in the `example` package, is available at `data.example.allow`. When rule names include +all or part of their package paths, this creates repetition in such references. For example, `authz_allow` in a package +`authz` is referenced with: `data.authz.authz_allow`. This repetition is undesirable as the reference is longer than +needed, and harder to read. + +This rule was inspired by [Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments#package-names). + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + rule-name-repeats-package: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/rule-name-repeats-package/rule_name_repeats_package.rego) diff --git a/docs/projects/regal/rules/style/todo-comment.md b/docs/projects/regal/rules/style/todo-comment.md new file mode 100644 index 0000000000..37c4e92f5d --- /dev/null +++ b/docs/projects/regal/rules/style/todo-comment.md @@ -0,0 +1,52 @@ +# todo-comment + +**Summary**: Avoid TODO Comments + +**Category**: Style + +**Avoid** +```rego +package policy + +# TODO: implementation +allow := true + +i := input.i + 1 + +# Fixme: surely there's a better way to do recursion +response := http.send({ + "url": "http://localhost:8080/v1/data/policy", + "method": "POST", + "body": { + "input": { + "i": i + } + } +}) +``` + +**Prefer** + +To fix the problem, or use an issue tracker to track it. + +## Rationale + +While TODO and FIXME comments are occasionally useful, they essentially provide a way to do issue tracking inside of +the code rather than where issues belong — in your issue tracker. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + todo-comment: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- DEV Community: [//TODO: Write a better comment](https://dev.to/adammc331/todo-write-a-better-comment-4c8c) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/todo-comment/todo_comment.rego) diff --git a/docs/projects/regal/rules/style/trailing-default-rule.md b/docs/projects/regal/rules/style/trailing-default-rule.md new file mode 100644 index 0000000000..824ad1ae99 --- /dev/null +++ b/docs/projects/regal/rules/style/trailing-default-rule.md @@ -0,0 +1,50 @@ +# trailing-default-rule + +**Summary**: Default rule should be declared first + +**Category**: Style + +**Avoid** +```rego +package policy + +allow if { + # some conditions +} + +default allow := false +``` + +**Prefer** +```rego +package policy + +default allow := false + +allow if { + # some conditions +} +``` + +## Rationale + +Presenting the default value of a rule (if one is used) before the conditional rule assignments is a common practice, +and it's often easier to to reason about conditional assignments knowing there is a default fallback value in place. +For that reason, it's recommended to follow the convention and place the default rule declaration before rules +conditionally assigning values. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + trailing-default-rule: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/trailing-default-rule/trailing_default_rule.rego) diff --git a/docs/projects/regal/rules/style/unconditional-assignment.md b/docs/projects/regal/rules/style/unconditional-assignment.md new file mode 100644 index 0000000000..dd2b109870 --- /dev/null +++ b/docs/projects/regal/rules/style/unconditional-assignment.md @@ -0,0 +1,55 @@ +# unconditional-assignment + +**Summary**: Unconditional assignment in rule body + +**Category**: Style + +**Avoid** +```rego +package policy + +full_name := name if { + name := concat(", ", [input.first_name, input.last_name]) +} + +divide_by_ten(x) := y if { + y := x / 10 +} + +names contains name if { + name := "Regal" +} +``` + +**Prefer** +```rego +package policy + +full_name := concat(", ", [input.first_name, input.last_name]) + +divide_by_ten(x) := x / 10 + +names contains "Regal" +``` + +## Rationale + +Rules that return values unconditionally should place the assignment directly in the rule head, as doing so in the rule +body adds unnecessary noise. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + unconditional-assignment: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Rego Style Guide: [Prefer unconditional assignment in rule head over rule body](https://github.com/StyraInc/rego-style-guide#prefer-unconditional-assignment-in-rule-head-over-rule-body) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/unconditional-assignment/unconditional_assignment.rego) diff --git a/docs/projects/regal/rules/style/unnecessary-some.md b/docs/projects/regal/rules/style/unnecessary-some.md new file mode 100644 index 0000000000..ad3af6a30f --- /dev/null +++ b/docs/projects/regal/rules/style/unnecessary-some.md @@ -0,0 +1,56 @@ +# unnecessary-some + +**Summary**: Unnecessary use of `some` + +**Category**: Style + +**Avoid** +```rego +package policy + +is_developer if some "developer" in input.user.roles +``` + +**Prefer** + +```rego +package policy + +is_developer if "developer" in input.user.roles +``` + +## Rationale + +Use the `some .. in` construct when you want to loop over a collection and assign variables in the iteration. If you +know the value you're looking for, just use the `in` keyword directly without using `some`. + +## Exceptions + +Note that `some .. in` iteration can be used with a limited form of pattern matching where either the key or the value +should match for the loop assignment to succeed. This is not commonly needed, but considered OK. + +```rego +package policy + +developers contains name if { + # name will only be bound when the value is "developer" + some name, "developer" in {"alice": "developer", "bob": "developer", "charlie": "manager"} +} +``` + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + unnecessary-some: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Membership and iteration: `in`](https://www.openpolicyagent.org/docs/policy-language/#membership-and-iteration-in) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/unnecessary-some/unnecessary_some.rego) diff --git a/docs/projects/regal/rules/style/use-assignment-operator.md b/docs/projects/regal/rules/style/use-assignment-operator.md new file mode 100644 index 0000000000..25b89ba39c --- /dev/null +++ b/docs/projects/regal/rules/style/use-assignment-operator.md @@ -0,0 +1,85 @@ +# use-assignment-operator + +**Summary**: Prefer `:=` over `=` for assignment + +**Category**: Style + +**Automatically fixable**: [Yes](https://openpolicyagent.org/projects/regal/fixing) + +**Avoid** +```rego +package policy + +default allow = false + +first_name(name) = split(name, " ")[0] + +allow if { + username = input.user.name + # .. more conditions .. +} +``` + +**Prefer** +```rego +package policy + +default allow := false + +first_name(full_name) := split(full_name, " ")[0] + +allow if { + username := input.user.name + # .. more conditions .. +} +``` + +## Rationale + +Rego has three operators related to assignment and equality: + +- `:=` is the assignment operator, and is only used to assign values to variables +- `==` is the equality operator, and is only used to compare values +- `=` is the unification operator, and is used both to assign values to variables **and** compare values + +While it often is "harmless" to use the unification operator (`=`) for assignment, the assignment operator (`:=`) +removes any ambiguities around intent, and prevents some hard to debug issues. Consider: + +```rego +allow if { + username = input.user.name + # .. more conditions .. +} +``` + +Using the unification operator, `username` is either assigned (if `username` isn't defined elsewhere in the +policy) or being checked for equality (if `username` is defined elsewhere in the policy). Using `:=` for assignment, +and `==` for equality comparison removes this ambiguity and make the intent obvious. + +In some cases, `=` and `:=` may be used interchangeably, as the result is the same either way: + +```rego +first_name(full_name) = split(full_name, " ")[0] +# same as +first_name(full_name) := split(full_name, " ")[0] +``` + +Even when that is the case, using `:=` consistently should be considered a best practice. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + use-assignment-operator: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Equality: Assignment, Comparison, and Unification](https://www.openpolicyagent.org/docs/policy-language/#equality-assignment-comparison-and-unification) +- Rego Style Guide: [Don't use unification operator for assignment or comparison](https://github.com/StyraInc/rego-style-guide#dont-use-unification-operator-for-assignment-or-comparison) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/use-assignment-operator/use_assignment_operator.rego) diff --git a/docs/projects/regal/rules/style/use-in-operator.md b/docs/projects/regal/rules/style/use-in-operator.md new file mode 100644 index 0000000000..e2c493d5cf --- /dev/null +++ b/docs/projects/regal/rules/style/use-in-operator.md @@ -0,0 +1,5 @@ +# use-in-operator + +## Please Note + +This rule has been moved to *idiomatic* category and can be found [here](../idiomatic/use-in-operator.md). diff --git a/docs/projects/regal/rules/style/yoda-condition.md b/docs/projects/regal/rules/style/yoda-condition.md new file mode 100644 index 0000000000..b77ca71879 --- /dev/null +++ b/docs/projects/regal/rules/style/yoda-condition.md @@ -0,0 +1,48 @@ +# yoda-condition + +**Summary**: Yoda condition, it is + +**Category**: Style + +**Avoid** +```rego +package policy + +allow if { + "GET" == input.request.method + "users" == input.request.path[0] +} +``` + +**Prefer** +```rego +package policy + +allow if { + input.request.method == "GET" + input.request.path[0] == "users" +} +``` + +## Rationale + +Yoda conditions — expressions where the constant portion of a comparison is placed on the left-hand side of the +comparison — provide no benefits in Rego. They do however add a certain amount of cognitive overhead for most policy +authors in the galaxy. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + style: + yoda-condition: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Wikipedia: [Yoda conditions](https://en.wikipedia.org/wiki/Yoda_conditions) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/style/yoda-condition/yoda_condition.rego) diff --git a/docs/projects/regal/rules/testing/dubious-print-sprintf.md b/docs/projects/regal/rules/testing/dubious-print-sprintf.md new file mode 100644 index 0000000000..f028fe7db7 --- /dev/null +++ b/docs/projects/regal/rules/testing/dubious-print-sprintf.md @@ -0,0 +1,60 @@ +# dubious-print-sprintf + +**Summary**: Dubious use of `print` and `sprintf` + +**Category**: Testing + +**Avoid** +```rego +package policy + +allow if { + # if any of input.name or input.domain are undefined, this will just print + print(sprintf("name is: %s domain is: %s", [input.name, input.domain])) + + input.name == "admin" +} +``` + +**Prefer** +```rego +package policy + +allow if { + # if any of input.name or input.domain are undefined, this will still print the whole + # sentence, with the value undefined printed as such, e.g. + # name is: admin domain is: + print("name is:", input.name, "domain is:", input.domain) + + input.name == "admin" +} +``` + +## Rationale + +Since `print` allows any number of arguments, there's rarely any benefit to using `sprintf` for formatting the output of +a `print` call. But more importantly, the `print` function is unique in that it will allow any arguments passed to be +*undefined* without terminating, but will print such values as ``. Using `sprintf` will however nullify this +benefit, and just print `` without the context. + +Note that using `print` is generally discouraged outside of development, and other rules exists to check for its use. +However, in the context of development and testing, one may choose to allow `print`, in e.g. `_test.rego` files, while +still wanting to avoid the use of `sprintf` in such cases. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + testing: + dubious-print-sprintf: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [Call to `print` or `trace` function](https://openpolicyagent.org/projects/regal/rules/testing/print-or-trace-call) +- OPA Docs: [Policy Testing](https://www.openpolicyagent.org/docs/policy-testing/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/testing/dubious-print-sprintf/dubious_print_sprintf.rego) diff --git a/docs/projects/regal/rules/testing/file-missing-test-suffix.md b/docs/projects/regal/rules/testing/file-missing-test-suffix.md new file mode 100644 index 0000000000..891c3f8e2c --- /dev/null +++ b/docs/projects/regal/rules/testing/file-missing-test-suffix.md @@ -0,0 +1,28 @@ +# file-missing-test-suffix + +**Summary**: Files containing tests should have a `_test.rego` suffix + +**Category**: Testing + +## Rationale + +In order to clearly communicate intent, and to avoid bundling tests with production policy, tests should be kept in a +separate file with a `_test.rego` suffix, and ideally prefixed with the same name as the policy the tests are targeting, +e.g. `policy.rego` and `policy_test.rego`. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + testing: + file-missing-test-suffix: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Policy Testing](https://www.openpolicyagent.org/docs/policy-testing/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/testing/file-missing-test-suffix/file_missing_test_suffix.rego) diff --git a/docs/projects/regal/rules/testing/identically-named-tests.md b/docs/projects/regal/rules/testing/identically-named-tests.md new file mode 100644 index 0000000000..9592869bde --- /dev/null +++ b/docs/projects/regal/rules/testing/identically-named-tests.md @@ -0,0 +1,59 @@ +# identically-named-tests + +**Summary**: Multiple tests with same name + +**Category**: Testing + +**Avoid** +```rego +package policy_test + +import data.policy + +test_allow_if_admin { + policy.allow with input as {"user": {"roles": ["admin"]}} +} + +test_allow_if_admin { + policy.allow with input as {"user": {"roles": ["superadmin"]}} +} +``` + +**Prefer** +```rego +package policy_test + +import data.policy + +test_allow_if_admin { + policy.allow with input as {"user": {"roles": ["admin"]}} +} + +test_allow_if_superadmin { + policy.allow with input as {"user": {"roles": ["superadmin"]}} +} +``` + +## Rationale + +While OPA allows multiple tests with the same name, using unique names for tests makes for easier to read test code, as +well as more informative test output. Since a single test may include any number of assertions, there's no need to reuse +test names within the same test package. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + testing: + identically-named-tests: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Policy Testing](https://www.openpolicyagent.org/docs/policy-testing/) +- OPA GitHub: [Support running of individual test rules sharing same name](https://github.com/open-policy-agent/opa/issues/5766) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/testing/identically-named-tests/identically_named_tests.rego) diff --git a/docs/projects/regal/rules/testing/index.md b/docs/projects/regal/rules/testing/index.md new file mode 100644 index 0000000000..d0ee6e5dbc --- /dev/null +++ b/docs/projects/regal/rules/testing/index.md @@ -0,0 +1,14 @@ +--- +title: Testing +sidebar_position: 6 +--- + + +# Testing + +Rules relading to Rego tests. + +import RulesTable from '@site/src/components/projects/regal/RulesTable'; + + + diff --git a/docs/projects/regal/rules/testing/index.md.yaml b/docs/projects/regal/rules/testing/index.md.yaml new file mode 100644 index 0000000000..9d97436f78 --- /dev/null +++ b/docs/projects/regal/rules/testing/index.md.yaml @@ -0,0 +1,2 @@ +title: Testing +sidebar_position: 6 diff --git a/docs/projects/regal/rules/testing/metasyntactic-variable.md b/docs/projects/regal/rules/testing/metasyntactic-variable.md new file mode 100644 index 0000000000..9393493270 --- /dev/null +++ b/docs/projects/regal/rules/testing/metasyntactic-variable.md @@ -0,0 +1,87 @@ +# metasyntactic-variable + +**Summary**: Metasyntactic variable name + +**Category**: Testing + +**Avoid** +```rego +package policy + +# Using metasyntactic names +foo := ["bar", "baz"] + +# ... +``` + +**Prefer** +```rego +package policy + +# Using names relevant to the context +roles := ["developer", "admin"] + +# ... +``` + +## Rationale + +Using "foo", "bar", "baz" and other [metasyntactic variables](https://en.wikipedia.org/wiki/Metasyntactic_variable) is +occasionally useful in examples, but should be avoided in production policy. + +This linter rules forbids any metasyntactic variable names, as listed by Wikipedia: + + +- foobar +- foo +- bar +- baz +- qux +- quux +- corge +- grault +- garply +- waldo +- fred +- plugh +- xyzzy +- thud + + +## Exceptions + +While there are no recommended exceptions to this rule, you could choose to allow metasyntactic variables in tests, or +perhaps code meant to be used in examples. When using a +[proper suffix](https://openpolicyagent.org/projects/regal/rules/testing/file-missing-test-suffix) for tests, like `_test.rego`, +simply configure an ignore pattern with the configuration of this rule: + +```yaml +rules: + testing: + metasyntactic-variable: + level: error + ignore: + files: + - "*_test.rego" +``` + +If you'd rather use your own list of forbidden variable names or patterns, see the +[naming convention](https://openpolicyagent.org/projects/regal/rules/custom/naming-convention) rule. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + testing: + metasyntactic-variable: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [Naming convention rule](https://openpolicyagent.org/projects/regal/rules/custom/naming-convention) +- Wikipedia: [Metasyntactic variable](https://en.wikipedia.org/wiki/Metasyntactic_variable) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/testing/metasyntactic-variable/metasyntactic_variable.rego) diff --git a/docs/projects/regal/rules/testing/print-or-trace-call.md b/docs/projects/regal/rules/testing/print-or-trace-call.md new file mode 100644 index 0000000000..1f7bfcd083 --- /dev/null +++ b/docs/projects/regal/rules/testing/print-or-trace-call.md @@ -0,0 +1,45 @@ +# print-or-trace-call + +**Summary**: Call to `print` or `trace` function + +**Category**: Testing + +**Avoid** +```rego +package policy + +reasons contains sprintf("%q is a dog!", [user.name]) if { + some user in input.users + user.species == "canine" + + # Useful for debugging, but leave out before committing + print("user:", user) +} +``` + +## Rationale + +The `print` function is really useful for development and debugging, but should normally not be included in production +policy. In order to be as useful for debugging purposes as possible, some performance optimizations are disabled when +`print` calls are encountered. Prefer decision logging in production. + +The `trace` function serves no real purpose since the introduction of `print`, and should be considered deprecated. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + testing: + print-or-trace-call: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Blog: [Introducing the OPA print function](https://blog.openpolicyagent.org/introducing-the-opa-print-function-809da6a13aee) +- OPA Docs: [Policy Reference: Debugging](https://www.openpolicyagent.org/docs/policy-reference/#debugging) +- OPA Docs: [Decision Logs](https://www.openpolicyagent.org/docs/management-decision-logs/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/testing/print-or-trace-call/print_or_trace_call.rego) diff --git a/docs/projects/regal/rules/testing/test-outside-test-package.md b/docs/projects/regal/rules/testing/test-outside-test-package.md new file mode 100644 index 0000000000..a9cc272955 --- /dev/null +++ b/docs/projects/regal/rules/testing/test-outside-test-package.md @@ -0,0 +1,54 @@ +# test-outside-test-package + +**Summary**: Test outside of test package + +**Category**: Testing + +**Avoid** +```rego +package policy + +allow if { + "admin" in input.user.roles +} + +# Tests in same package as policy +test_allow_if_admin { + allow with input as {"user": {"roles": ["admin"]}} +} +``` + +**Prefer** +```rego +# Tests in separate package with _test suffix +package policy_test + +import data.policy + +test_allow_if_admin { + policy.allow with input as {"user": {"roles": ["admin"]}} +} +``` + +## Rationale + +While OPA's test runner will evaluate any rules with a `test_` prefix, it is a good practice to clearly separate tests +from production policy. This is easily done by placing tests in a separate package with a `_test` suffix, and correctly +[naming](./file-missing-test-suffix.md) the test files. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + testing: + test-outside-test-package: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Policy Testing](https://www.openpolicyagent.org/docs/policy-testing/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/testing/test-outside-test-package/test_outside_test_package.rego) diff --git a/docs/projects/regal/rules/testing/todo-test.md b/docs/projects/regal/rules/testing/todo-test.md new file mode 100644 index 0000000000..b15ea22a87 --- /dev/null +++ b/docs/projects/regal/rules/testing/todo-test.md @@ -0,0 +1,39 @@ +# todo-test + +**Summary**: TODO test encountered + +**Category**: Testing + +**Avoid** +```rego +package policy_test + +import data.policy + +# Make sure this passes +todo_test_allow_if_admin { + policy.allow with input as {"user": {"roles": ["admin"]}} +} +``` + +## Rationale + +Writing TODO tests by prefixing `todo_` to any test is a good way to keep track of tests that need to be written while +developing policy. They are however not to be committed, and should be removed before submitting the change for review. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + testing: + todo-test: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- OPA Docs: [Policy Testing](https://www.openpolicyagent.org/docs/policy-testing/) +- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/testing/todo-test/todo_test.rego) diff --git a/docs/src/components/projects/regal/Intro/index.js b/docs/src/components/projects/regal/Intro/index.js new file mode 100644 index 0000000000..e08ef1ceb2 --- /dev/null +++ b/docs/src/components/projects/regal/Intro/index.js @@ -0,0 +1,23 @@ +import React from "react"; + +import Link from "@docusaurus/Link"; + +import styles from "./styles.module.css"; + +export default function Intro({ image }) { + return ( +
+
+ +
+
+
+ regal
+ adj : of notable excellence or magnificence : splendid +
+ --
Merriam-Webster +
+
+
+ ); +} diff --git a/docs/src/components/projects/regal/Intro/styles.module.css b/docs/src/components/projects/regal/Intro/styles.module.css new file mode 100644 index 0000000000..7b6b3cf015 --- /dev/null +++ b/docs/src/components/projects/regal/Intro/styles.module.css @@ -0,0 +1,17 @@ +.container { + display: flex; + align-items: center; +} + +.column:first-child { + width: 20%; +} + +.column:last-child { + width: 80%; +} + +.logo { + width: 10rem; + height: 10rem; +} diff --git a/docs/src/components/projects/regal/RulesTable/index.js b/docs/src/components/projects/regal/RulesTable/index.js new file mode 100644 index 0000000000..d3a0ecdcd8 --- /dev/null +++ b/docs/src/components/projects/regal/RulesTable/index.js @@ -0,0 +1,69 @@ +import React, { useState } from "react"; + +import Link from "@docusaurus/Link"; + +import rules from "@generated/regal/default/rules.json"; + +import styles from "./styles.module.css"; + +export default function RulesTable({ category }) { + const [searchQuery, setSearchQuery] = useState(""); + + let predicates = []; + + let basePath = "./rules/"; + if (category !== undefined && category !== "") { + predicates.push((rule) => rule.id.startsWith(category + "/")); + basePath = "./"; + } + + if (searchQuery !== "") { + predicates.push((rule) => rule.id.includes(searchQuery.toLowerCase())); + } + + const filteredRules = rules.filter(rule => { + if (predicates.length === 0) return true; + + return predicates.map((predicate) => predicate(rule)) + .every((e) => e == true); + }); + + return ( +
+
+ setSearchQuery(e.target.value)} + className={styles.searchInput} + /> +
+ + {filteredRules.length === 0 + ?

No matching rules

+ : ( + + + + + + + + + {filteredRules.map((rule) => { + return ( + + + + + ); + })} + +
RuleSummary
+ {rule.id} + {rule.summary}
+ )} +
+ ); +} diff --git a/docs/src/components/projects/regal/RulesTable/styles.module.css b/docs/src/components/projects/regal/RulesTable/styles.module.css new file mode 100644 index 0000000000..f4de3d6f87 --- /dev/null +++ b/docs/src/components/projects/regal/RulesTable/styles.module.css @@ -0,0 +1,24 @@ +.searchContainer { + margin: 1.5rem 0; +} + +.searchInput { + padding: 0.5rem; + width: 100%; + max-width: 40rem; + font-size: 1rem; +} + +.table { + width: 100%; + border-collapse: collapse; + table-layout: auto; +} + +.table tbody td:first-child { + width: 1; +} + +.table tbody td:last-child { + width: 100%; +} diff --git a/docs/src/lib/projects/regal/loadRules.js b/docs/src/lib/projects/regal/loadRules.js new file mode 100644 index 0000000000..64432992c3 --- /dev/null +++ b/docs/src/lib/projects/regal/loadRules.js @@ -0,0 +1,40 @@ +import fs from "fs/promises"; +import glob from "glob"; + +export async function loadRules() { + const rootPath = "projects/regal/rules"; + + const summaryMarker = "**Summary**:"; + + const files = await new Promise((resolve, reject) => { + glob(rootPath + "/*/*.md", (err, matches) => { + if (err) reject(err); + else resolve(matches); + }); + }); + + const rules = await files + .filter(file => file !== "index.md") + .reduce(async (accPromise, filePath) => { + const acc = await accPromise; + const content = await fs.readFile(filePath, "utf-8"); + + const summary = (content.split("\n") + .filter(line => line.includes(summaryMarker)) + .find(_ => true) || "").replace(summaryMarker, "").trim(); + + // index and deprecated pages + if (summary === "") return acc; + + const id = filePath.replace(rootPath + "/", "").replace(".md", ""); + + acc.push({ + summary, + id, + }); + + return acc; + }, Promise.resolve([])); + + return rules.sort((a, b) => a.id - b.id); +} diff --git a/docs/src/lib/sidebar-regal.js b/docs/src/lib/sidebar-regal.js new file mode 100644 index 0000000000..c168f607ae --- /dev/null +++ b/docs/src/lib/sidebar-regal.js @@ -0,0 +1,5 @@ +module.exports = { + regalSidebar: [ + { type: "autogenerated", dirName: "." }, + ], +};