Skip to content

Commit

Permalink
Add Regal for linting Rego
Browse files Browse the repository at this point in the history
This PR adds [Regal](https://github.com/styraInc/regal) for linting the Rego
contained in this repo.

Practically, this PR adds a Regal configuration file adapted to the project,
where some of the most reported style issues are ignoed for the time being.
Additionally, we're adding a linter job to the CI pipeline to ensure future
updates to policy if compliant as well.

A few (hopefully) uncontroversial issues reported have also been fixed:

* [constant-condition](https://docs.styra.com/regal/rules/bugs/constant-condition)
* [use-assignment-operator](https://docs.styra.com/regal/rules/style/use-assignment-operator)
* [use-in-operator](https://docs.styra.com/regal/rules/idiomatic/use-in-operator)
* [use-some-for-output-vars](https://docs.styra.com/regal/rules/idiomatic/use-some-for-output-vars)
* [non-raw-regex-pattern](https://docs.styra.com/regal/rules/idiomatic/non-raw-regex-pattern)

While the changes are non-intrusive, and should have no impact on evaluation,
I naturally wanted to run the tests included in this repo and followed the instructions
in TESTING.md. However, even after having installed all of the dependencies listed as
required, the test command fails due to a missing `oc` command. I tried to find that
and possibly update the TESTING.md file to include it, but the instructions I followed
suggested logging in to a "RedHat Customer Portal" to get it. If being a customer is required
to run the tests, that requirement would be good to add to the docs as well.

Signed-off-by: Anders Eknert <[email protected]>
  • Loading branch information
anderseknert committed Oct 15, 2023
1 parent 090de51 commit ddc427e
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 74 deletions.
18 changes: 18 additions & 0 deletions .github/workflows/regal-lint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
name: Lint policies with Regal

on: [push, pull_request]

jobs:
lint-policy:
runs-on: ubuntu-latest
steps:
- name: Check out code
uses: actions/checkout@v4

- name: Setup Regal
uses: StyraInc/[email protected]
with:
version: v0.10.1

- name: Run Regal lint
run: regal lint --format github policy
22 changes: 22 additions & 0 deletions .regal/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
rules:
idiomatic:
no-defined-entrypoint:
level: ignore
style:
avoid-get-and-list-prefix:
level: ignore
external-reference:
level: ignore
line-length:
level: ignore
no-whitespace-comment:
level: ignore
opa-fmt:
level: ignore
prefer-some-in-iteration:
level: ignore
prefer-snake-case:
# 13 violations in the repo seems easy to fix
level: ignore
todo-comment:
level: ignore
43 changes: 23 additions & 20 deletions policy/lib/konstraint/core.rego
Original file line number Diff line number Diff line change
@@ -1,41 +1,44 @@
package lib.konstraint.core

default is_gatekeeper = false
default is_gatekeeper := false

is_gatekeeper {
has_field(input, "review")
has_field(input.review, "object")
}

resource = input.review.object {
resource := input.review.object {
is_gatekeeper
}

resource = input {
# This is a bug in Regal causing a false positive
# to be reported. It's been fixed, and this can
# safely be removed following the next (above
# v0.10.1) release.
# https://github.com/StyraInc/regal/issues/401
#
# regal ignore:top-level-iteration
resource := input {
not is_gatekeeper
}

format(msg) = {"msg": msg} {
true
}
format(msg) := {"msg": msg}

format_with_id(msg, id) = msg_fmt {
msg_fmt := {
"msg": sprintf("%s: %s", [id, msg]),
"details": {"policyID": id}
}
format_with_id(msg, id) := {
"msg": sprintf("%s: %s", [id, msg]),
"details": {"policyID": id}
}

apiVersion = resource.apiVersion
name = resource.metadata.name
kind = resource.kind
labels = resource.metadata.labels
annotations = resource.metadata.annotations
apiVersion := resource.apiVersion
name := resource.metadata.name
kind := resource.kind
labels := resource.metadata.labels
annotations := resource.metadata.annotations
gv := split(apiVersion, "/")
group = gv[0] {
group := gv[0] {
contains(apiVersion, "/")
}
group = "core" {
group := "core" {
not contains(apiVersion, "/")
}
version := gv[count(gv) - 1]
Expand All @@ -52,10 +55,10 @@ has_field(obj, field) {
not object.get(obj, field, "N_DEFINED") == "N_DEFINED"
}

missing_field(obj, field) = true {
missing_field(obj, field) := true {
obj[field] == ""
}

missing_field(obj, field) = true {
missing_field(obj, field) := true {
not has_field(obj, field)
}
54 changes: 27 additions & 27 deletions policy/lib/konstraint/measurements.rego
Original file line number Diff line number Diff line change
@@ -1,127 +1,127 @@
package lib.konstraint.measurements

# 10 ** 21
mem_multiple("E") = 1000000000000000000000 { true }
mem_multiple("E") := 1000000000000000000000

# 10 ** 18
mem_multiple("P") = 1000000000000000000 { true }
mem_multiple("P") := 1000000000000000000

# 10 ** 15
mem_multiple("T") = 1000000000000000 { true }
mem_multiple("T") := 1000000000000000

# 10 ** 12
mem_multiple("G") = 1000000000000 { true }
mem_multiple("G") := 1000000000000

# 10 ** 9
mem_multiple("M") = 1000000000 { true }
mem_multiple("M") := 1000000000

# 10 ** 6
mem_multiple("k") = 1000000 { true }
mem_multiple("k") := 1000000

# 10 ** 3
mem_multiple("") = 1000 { true }
mem_multiple("") := 1000

# Kubernetes accepts millibyte precision when it probably shouldn't.
# https://github.com/kubernetes/kubernetes/issues/28741
# 10 ** 0
mem_multiple("m") = 1 { true }
mem_multiple("m") := 1

# 1000 * 2 ** 10
mem_multiple("Ki") = 1024000 { true }
mem_multiple("Ki") := 1024000

# 1000 * 2 ** 20
mem_multiple("Mi") = 1048576000 { true }
mem_multiple("Mi") := 1048576000

# 1000 * 2 ** 30
mem_multiple("Gi") = 1073741824000 { true }
mem_multiple("Gi") := 1073741824000

# 1000 * 2 ** 40
mem_multiple("Ti") = 1099511627776000 { true }
mem_multiple("Ti") := 1099511627776000

# 1000 * 2 ** 50
mem_multiple("Pi") = 1125899906842624000 { true }
mem_multiple("Pi") := 1125899906842624000

# 1000 * 2 ** 60
mem_multiple("Ei") = 1152921504606846976000 { true }
mem_multiple("Ei") := 1152921504606846976000

get_suffix(mem) = suffix {
get_suffix(mem) := suffix {
not is_string(mem)
suffix := ""
}

get_suffix(mem) = suffix {
get_suffix(mem) := suffix {
is_string(mem)
count(mem) > 0
suffix := substring(mem, count(mem) - 1, -1)
mem_multiple(suffix)
}

get_suffix(mem) = suffix {
get_suffix(mem) := suffix {
is_string(mem)
count(mem) > 1
suffix := substring(mem, count(mem) - 2, -1)
mem_multiple(suffix)
}

get_suffix(mem) = suffix {
get_suffix(mem) := suffix {
is_string(mem)
count(mem) > 1
not mem_multiple(substring(mem, count(mem) - 1, -1))
not mem_multiple(substring(mem, count(mem) - 2, -1))
suffix := ""
}

get_suffix(mem) = suffix {
get_suffix(mem) := suffix {
is_string(mem)
count(mem) == 1
not mem_multiple(substring(mem, count(mem) - 1, -1))
suffix := ""
}

get_suffix(mem) = suffix {
get_suffix(mem) := suffix {
is_string(mem)
count(mem) == 0
suffix := ""
}

canonify_mem(orig) = new {
canonify_mem(orig) := new {
is_number(orig)
new := orig * 1000
}

canonify_mem(orig) = new {
canonify_mem(orig) := new {
not is_number(orig)
suffix := get_suffix(orig)
raw := replace(orig, suffix, "")
re_match("^[0-9]+$", raw)
new := to_number(raw) * mem_multiple(suffix)
}

canonify_storage(orig) = new {
canonify_storage(orig) := new {
is_number(orig)
new := orig
}

canonify_storage(orig) = new {
canonify_storage(orig) := new {
not is_number(orig)
suffix := get_suffix(orig)
raw := replace(orig, suffix, "")
re_match("^[0-9]+$", raw)
new := to_number(raw) * mem_multiple(suffix)
}

canonify_cpu(orig) = new {
canonify_cpu(orig) := new {
is_number(orig)
new := orig * 1000
}

canonify_cpu(orig) = new {
canonify_cpu(orig) := new {
not is_number(orig)
endswith(orig, "m")
new := to_number(replace(orig, "m", ""))
}

canonify_cpu(orig) = new {
canonify_cpu(orig) := new {
not is_number(orig)
not endswith(orig, "m")
re_match("^[0-9]+$", orig)
Expand Down
16 changes: 8 additions & 8 deletions policy/lib/konstraint/pods.rego
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,27 @@ package lib.konstraint.pods

import data.lib.konstraint.core

default pod = false
default pod := false

pod = core.resource.spec.template {
pod := core.resource.spec.template {
pod_templates := ["daemonset","deployment","job","replicaset","replicationcontroller","statefulset"]
lower(core.kind) == pod_templates[_]
}

pod = core.resource {
pod := core.resource {
lower(core.kind) == "pod"
}

pod = core.resource.spec.jobTemplate.spec.template {
pod := core.resource.spec.jobTemplate.spec.template {
lower(core.kind) == "cronjob"
}

containers[container] {
keys = {"containers", "initContainers"}
all_containers = [c | keys[k]; c = pod.spec[k][_]]
container = all_containers[_]
keys := {"containers", "initContainers"}
all_containers = [c | some k; keys[k]; c := pod.spec[k][_]]
container := all_containers[_]
}

volumes[volume] {
volume = pod.spec.volumes[_]
volume := pod.spec.volumes[_]
}
4 changes: 3 additions & 1 deletion policy/lib/konstraint/rbac.rego
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package lib.konstraint.rbac

import future.keywords.in

import data.lib.konstraint.core

rule_has_verb(rule, verb) {
Expand All @@ -13,7 +15,7 @@ rule_has_resource_type(rule, type) {
}

rule_has_resource_name(rule, name) {
name == rule.resourceNames[_]
name in rule.resourceNames
}

rule_has_resource_name(rule, name) {
Expand Down
18 changes: 10 additions & 8 deletions policy/lib/openshift.rego
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
package lib.openshift

import future.keywords.in

import data.lib.konstraint.core as konstraint_core
import data.lib.konstraint.pods as konstraint_pods
import data.lib.kubernetes

pod = konstraint_pods.pod {
pod := konstraint_pods.pod {
konstraint_pods.pod
}

pod = konstraint_core.resource.spec.template {
pod := konstraint_core.resource.spec.template {
is_deploymentconfig
}

containers[container] {
keys = {"containers", "initContainers"}
all_containers = [c | keys[k]; c = pod.spec[k][_]]
container = all_containers[_]
keys := {"containers", "initContainers"}
all_containers := [c | some k; keys[k]; c := pod.spec[k][_]]
container := all_containers[_]
}

is_deploymentconfig {
Expand Down Expand Up @@ -52,16 +54,16 @@ is_policy_active(policyId) {
}

label_contains(disabledpolicies, policyId) {
policyId == disabledpolicies[_]
policyId in disabledpolicies
}

namespace_disabled_policies_label = disabledpolicies {
namespace_disabled_policies_label := disabledpolicies {
namepace := data.inventory.cluster["v1"].Namespace[konstraint_core.resource.metadata.namespace]
label := namepace.metadata.labels["redhat-cop.github.com/gatekeeper-disabled-policies"]
disabledpolicies := split(label, ",")
}

namespace_disabled_policies_label = [""] {
namespace_disabled_policies_label := [""] {
namepace := data.inventory.cluster["v1"].Namespace[konstraint_core.resource.metadata.namespace]
not namepace.metadata.labels["redhat-cop.github.com/gatekeeper-disabled-policies"]
}
Loading

0 comments on commit ddc427e

Please sign in to comment.