Use Regal for linting Rego (#7752)

Closing the circle here, or something.

Not a lot of Rego used in OPA yet, but some in examples and tests. The little
there is should be linted though, and it'd be good if any new addition of policies got
linted by default. But more than anything, the "ignore configuration" provided here
avoids having developers seeing thousands of issues reported by Regal when they
open the OPA project in VS Code or their editor of choice.

Someone might want to look into un-ignoring the doc directory at some point, as it's
probably a good idea to have the docs follow best practices.

Signed-off-by: Anders Eknert <anders@styra.com>

.github/workflows/pull-request.yaml

@@ -439,24 +439,29 @@ jobs:
         version: edge
 
     - name: Test policies
-      run: opa test --v0-compatible build/policy
+      run: opa test --schema build/policy/schema --bundle build/policy
 
     - name: Ensure proper formatting
-      run: opa fmt --v0-compatible --list --fail build/policy
+      run: opa fmt --list --fail build/policy
 
     - name: Run file policy checks on changed files
       run: |
         curl --silent --fail --header 'Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' -o files.json \
           https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files
 
-        opa eval --v0-compatible -d build/policy/files.rego -d build/policy/helpers.rego  --format values --input files.json \
-          --fail-defined 'data.files.deny[message]'
+        opa eval --bundle build/policy --format values --input files.json  --fail-defined 'data.files.deny[message]'
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
-    - name: Show input on failure
-      run: opa eval --v0-compatible --input files.json --format pretty input
-      if: ${{ failure() }}
+    - name: Download Regal
+      uses: StyraInc/setup-regal@33a142b1189004e0f14bf42b15972c67eecce776 #v1.0.0
+      with:
+        version: latest
+
+    - name: Run Regal lint
+      # Current configuration ensures anything but build/policy is ignored. While this could point Regal only at that
+      # directory, this will serve as a reminder when more Rego policies are added, as they should be linted by default.
+      run: regal lint --format github .
 
   docs-build:
     name: Build Docs
@@ -504,18 +509,18 @@ jobs:
       run: |
         # Create the input file with all job results
         echo '${{ toJSON(needs) }}' > input.json
-        
+
         # Find failed or cancelled jobs using OPA
         opa eval -d .github/workflows/pull-request.yaml \
         --input=input.json \
         '{job|some _, job in data.jobs["pr-check-summary"].needs} & {job | input[job].result in {"failure", "cancelled"}}' \
         --format=raw > failed_jobs.json
-        
+
         # Check for failures and display a nice message
         if [ "$(cat failed_jobs.json)" != "[]" ]; then
           echo "The following required jobs did not complete successfully:"
           jq -r '.[]' failed_jobs.json | sed 's/^/- /'
           exit 1
         fi
-        
+
         echo "All jobs completed successfully or were skipped"

.regal/config.yaml

@@ -0,0 +1,24 @@
+ignore:
+  files:
+    - "docs/*"
+    - "**/test/*"
+    - "**/testdata/*"
+    - "**/testfiles/*"
+
+rules:
+  idiomatic:
+    directory-package-mismatch:
+      exclude-test-suffix: true
+      ignore:
+        files:
+          - "docs"
+          - "internal/wasm/sdk/examples/*"
+  style:
+    line-length:
+      ignore:
+        files:
+          - "docs"
+
+project:
+  roots:
+    - build/policy

build/policy/files/files.rego

@@ -1,46 +1,17 @@
-# Expects policy input as provided by:
-# https://api.github.com/repos/open-policy-agent/opa/pulls/${PR_ID}/files
+# METADATA
+# description: |
+#   Expects policy input as provided by:
+#   https://api.github.com/repos/open-policy-agent/opa/pulls/${PR_ID}/files
 #
-# Note that the "filename" here refers to the full path of the file, like
-# docs/foo/bar.yaml - since that's how it's named in the
-# input we'll use the same convention here.
-
+#   Note that the "filename" here refers to the full, relative path of the file,
+#   like docs/foo/bar.yaml - since that's how it's named in the input we'll use
+#   the same convention here.
+# schemas:
+#   - input: schema.files
 package files
 
-import rego.v1
-
-import data.helpers.basename
-import data.helpers.directory
-import data.helpers.extension
-
-filenames := {f.filename | some f in input}
-
-changes[filename] := attributes if {
-	some change in input
-	filename := change.filename
-	attributes := object.remove(change, ["filename"])
-}
-
-http_error(response) if response.status_code == 0
-
-http_error(response) if response.status_code >= 400
-
-dump_response_on_error(response) := response if {
-	http_error(response)
-	print("unexpected error in response", response)
-}
-
-dump_response_on_error(response) := response if not http_error(response)
-
-get_file_in_pr(filename) := dump_response_on_error(http.send({
-	"url": changes[filename].raw_url,
-	"method": "GET",
-	"headers": {"Authorization": sprintf("Bearer %v", [opa.runtime().env.GITHUB_TOKEN])},
-	"cache": true,
-	"enable_redirect": true,
-	"raise_error": false,
-})).raw_body
-
+# METADATA
+# entrypoint: true
 deny contains sprintf("%s is an invalid YAML file: %s", [filename, content]) if {
 	some filename, content in yaml_file_contents
 	changes[filename].status in {"added", "modified"}
@@ -53,12 +24,45 @@ deny contains sprintf("%s is an invalid JSON file: %s", [filename, content]) if
 	not json.is_valid(content)
 }
 
-yaml_file_contents[filename] := get_file_in_pr(filename) if {
+yaml_file_contents[filename] := file_in_pr(filename) if {
 	some filename in filenames
 	extension(filename) in {"yml", "yaml"}
 }
 
-json_file_contents[filename] := get_file_in_pr(filename) if {
+json_file_contents[filename] := file_in_pr(filename) if {
 	some filename in filenames
 	extension(filename) == "json"
 }
+
+filenames contains f.filename if some f in input
+
+changes[filename] := attributes if {
+	some change in input
+	filename := change.filename
+	attributes := object.remove(change, ["filename"])
+}
+
+http_error(response) if response.status_code == 0
+http_error(response) if response.status_code >= 400
+
+dump_response_on_error(response) := response if {
+	http_error(response)
+	print("unexpected error in response", response) # regal ignore:print-or-trace-call
+}
+
+dump_response_on_error(response) := response if not http_error(response)
+
+file_in_pr(filename) := dump_response_on_error(http.send({
+	"url": changes[filename].raw_url,
+	"method": "GET",
+	"headers": {"Authorization": sprintf("Bearer %v", [opa.runtime().env.GITHUB_TOKEN])},
+	"cache": true,
+	"enable_redirect": true,
+	"raise_error": false,
+})).raw_body
+
+default last_indexof(_, _) := -1
+
+last_indexof(string, search) := indices[count(indices) - 1] if indices := indexof_n(string, search)
+
+extension(filename) := substring(filename, last_indexof(filename, ".") + 1, count(filename) - 1)

build/policy/files/files_test.rego

@@ -0,0 +1,25 @@
+package files_test
+
+import data.files
+
+test_deny_invalid_yaml_file if {
+	expected := "invalid.yaml is an invalid YAML file: {null{}}"
+	expected in files.deny with files.yaml_file_contents as {"invalid.yaml": "{null{}}"}
+		with files.changes as {"invalid.yaml": {"status": "modified"}}
+}
+
+test_allow_valid_yaml_file if {
+	count(files.deny) == 0 with files.yaml_file_contents as {"valid.yaml": "foo: bar"}
+		with files.changes as {"valid.yaml": {"status": "modified"}}
+}
+
+test_deny_invalid_json_file if {
+	expected := "invalid.json is an invalid JSON file: }}}"
+	expected in files.deny with files.json_file_contents as {"invalid.json": "}}}"}
+		with files.changes as {"invalid.json": {"status": "modified"}}
+}
+
+test_allow_valid_json_file if {
+	count(files.deny) == 0 with files.json_file_contents as {"valid.json": "{\"foo\": \"bar\"}"}
+		with files.changes as {"valid.json": {"status": "modified"}}
+}

build/policy/files_test.rego

@@ -1,27 +0,0 @@
-package files_test
-
-import rego.v1
-
-import data.files.deny
-
-test_deny_invalid_yaml_file if {
-	expected := "invalid.yaml is an invalid YAML file: {null{}}"
-	expected in deny with data.files.yaml_file_contents as {"invalid.yaml": "{null{}}"}
-		with data.files.changes as {"invalid.yaml": {"status": "modified"}}
-}
-
-test_allow_valid_yaml_file if {
-	count(deny) == 0 with data.files.yaml_file_contents as {"valid.yaml": "foo: bar"}
-		with data.files.changes as {"valid.yaml": {"status": "modified"}}
-}
-
-test_deny_invalid_json_file if {
-	expected := "invalid.json is an invalid JSON file: }}}"
-	expected in deny with data.files.json_file_contents as {"invalid.json": "}}}"}
-		with data.files.changes as {"invalid.json": {"status": "modified"}}
-}
-
-test_allow_valid_json_file if {
-	count(deny) == 0 with data.files.json_file_contents as {"valid.json": "{\"foo\": \"bar\"}"}
-		with data.files.changes as {"valid.json": {"status": "modified"}}
-}

build/policy/helpers.rego

@@ -1,15 +0,0 @@
-package helpers
-
-import rego.v1
-
-last_indexof(string, search) := i if {
-	all := [i | chars := split(string, ""); chars[i] == search]
-	count(all) > 0
-	i := all[count(all) - 1]
-} else := -1
-
-basename(filename) := substring(filename, last_indexof(filename, "/") + 1, count(filename) - 1)
-
-extension(filename) := substring(filename, last_indexof(filename, ".") + 1, count(filename) - 1)
-
-directory(filename) := substring(filename, 0, last_indexof(filename, "/"))

build/policy/schema/files.json

@@ -0,0 +1,84 @@
+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "$id": "files",
+  "$ref": "#/$defs/files",
+  "$defs": {
+    "files": {
+      "type": "array",
+      "items": {
+        "title": "Diff Entry",
+        "description": "Diff Entry",
+        "type": "object",
+        "properties": {
+          "sha": {
+            "type": "string",
+            "example": "bbcd538c8e72b8c175046e27cc8f907076331401"
+          },
+          "filename": {
+            "type": "string",
+            "example": "file1.txt"
+          },
+          "status": {
+            "type": "string",
+            "enum": [
+              "added",
+              "removed",
+              "modified",
+              "renamed",
+              "copied",
+              "changed",
+              "unchanged"
+            ],
+            "example": "added"
+          },
+          "additions": {
+            "type": "integer",
+            "example": 103
+          },
+          "deletions": {
+            "type": "integer",
+            "example": 21
+          },
+          "changes": {
+            "type": "integer",
+            "example": 124
+          },
+          "blob_url": {
+            "type": "string",
+            "format": "uri",
+            "example": "https://github.com/octocat/Hello-World/blob/6dcb09b5b57875f334f61aebed695e2e4193db5e/file1.txt"
+          },
+          "raw_url": {
+            "type": "string",
+            "format": "uri",
+            "example": "https://github.com/octocat/Hello-World/raw/6dcb09b5b57875f334f61aebed695e2e4193db5e/file1.txt"
+          },
+          "contents_url": {
+            "type": "string",
+            "format": "uri",
+            "example": "https://api.github.com/repos/octocat/Hello-World/contents/file1.txt?ref=6dcb09b5b57875f334f61aebed695e2e4193db5e"
+          },
+          "patch": {
+            "type": "string",
+            "example": "@@ -132,7 +132,7 @@ module Test @@ -1000,7 +1000,7 @@ module Test"
+          },
+          "previous_filename": {
+            "type": "string",
+            "example": "file.txt"
+          }
+        },
+        "required": [
+          "additions",
+          "blob_url",
+          "changes",
+          "contents_url",
+          "deletions",
+          "filename",
+          "raw_url",
+          "sha",
+          "status"
+        ]
+      }
+    }
+  }
+}

internal/wasm/sdk/examples/basic/example-1.rego

@@ -1,3 +1,3 @@
 package example
 
-allow = input.foo
+allow := input.foo

internal/wasm/sdk/examples/basic/example-2.rego

@@ -1,3 +1,3 @@
 package example
 
-allow = input.bar
+allow := input.bar

internal/wasm/sdk/examples/loaders/example.rego

@@ -1,3 +1,3 @@
 package example
 
-allow = input.foo
+allow := input.foo

v1/loader/loader_test.go

@@ -1011,7 +1011,7 @@ func TestLoadRooted(t *testing.T) {
 	})
 }
 
-//go:embed internal/embedtest
+//go:embed testdata/embedtest
 var embedTestFS embed.FS
 
 func TestLoadFS(t *testing.T) {
@@ -1021,7 +1021,7 @@ func TestLoadFS(t *testing.T) {
 		"three:baz",
 	}
 
-	fsys, err := fs.Sub(embedTestFS, "internal/embedtest")
+	fsys, err := fs.Sub(embedTestFS, "testdata/embedtest")
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
@@ -1060,7 +1060,7 @@ func TestLoadWithJSONOptions(t *testing.T) {
 		"three:baz",
 	}
 
-	fsys, err := fs.Sub(embedTestFS, "internal/embedtest")
+	fsys, err := fs.Sub(embedTestFS, "testdata/embedtest")
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}