Skip to content

Commit

Permalink
Ensure OPA strict mode compliance
Browse files Browse the repository at this point in the history
Hi Gatekeeper friends! πŸ‘‹πŸ˜ƒ

In order to ensure the Gatekeeper library works with future
updates of OPA, and to better catch some common mistakes in
Rego policy in the future, this PR aims to fix all errors
reported from `opa check --strict src`, and to make sure
future changes are compliant with the OPA
[strict mode](https://www.openpolicyagent.org/docs/latest/policy-language/#strict-mode)
checks.

The errors raised from strict mode, and which are fixed with
this PR, include:

* `input` must not be shadowed/assigned
* Unused function args β€” use wildcards (`_`) in their place
* Unused local assignment
* Deprecated built-in functions
  * `re_match` -> `regex.match`
  * `all` -> these were not needed
  * `any` -> `strings.any_prefix_match` and `strings.any_suffix_match`
     as these were always used together with `startswith`/`endswith`

The by far most noisy fix was the first one, i.e. `input` getting
shadowed in basically all tests. Changed to use `inp` for a name
as `in` is of course another name we don't want to shadow πŸ˜…

The changes here do not change semantics of the code. I did however
also make some additional fixes where e.g. the unused function args
error showed that a function wasn't needed in the first place, and
so on. Naturally, all tests pass as before.

Two things I'm not sure about:

1. The code introduces the `strings.any_prefix_match` and
   `strings.any_suffix_match` built-in functions in one policy. These
   were introduced in OPA v0.44.0, which is quite a few OPA and
   Gatekeeper versions ago. I could not find information on what the
   minimum version of OPA or Gatekeeper must be supported in the Rego
   code in this library. If needed, that change can be reverted.
2. https://github.com/open-policy-agent/gatekeeper-library mentions
   Versioning. Should I update the versions of each policy changed
   with this PR?

To make sure the code stays compliant, I've added the
`opa check --strict` command to execute with the `test.sh` script,
before the unit tests run.

Signed-off-by: Anders Eknert <anders@styra.com>
  • Loading branch information
anderseknert committed Oct 30, 2023
1 parent 68b6d53 commit 1c8907a
Show file tree
Hide file tree
Showing 389 changed files with 8,530 additions and 2,285 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
strategy:
matrix:
os: [ "ubuntu-latest", "macos-latest" ]
opa: [ "v0.42.2" ]
opa: [ "v0.44.0" ]
name: Unit test on ${{ matrix.os }} opa ${{ matrix.opa }}
steps:
- name: Harden Runner
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: k8sallowedrepos
displayName: Allowed Repositories
createdAt: "2022-09-26T17:28:27Z"
description: Requires container images to begin with a string from the specified list.
digest: eaec68f7273f5d79ced3ed778d5c2fc26b241e5db3a0e5119650bab651f8f1ed
digest: 4f7161f8f16f238f48cd986b02404e6abaa9f6febbf22674564332771871e9b8
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/allowedrepos
keywords:
Expand Down
9 changes: 3 additions & 6 deletions artifacthub/library/general/allowedrepos/1.0.0/template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,18 @@ spec:
violation[{"msg": msg}] {
container := input.review.object.spec.containers[_]
satisfied := [good | repo = input.parameters.repos[_] ; good = startswith(container.image, repo)]
not any(satisfied)
not strings.any_prefix_match(container.image, input.parameters.repos)
msg := sprintf("container <%v> has an invalid image repo <%v>, allowed repos are %v", [container.name, container.image, input.parameters.repos])
}
violation[{"msg": msg}] {
container := input.review.object.spec.initContainers[_]
satisfied := [good | repo = input.parameters.repos[_] ; good = startswith(container.image, repo)]
not any(satisfied)
not strings.any_prefix_match(container.image, input.parameters.repos)
msg := sprintf("initContainer <%v> has an invalid image repo <%v>, allowed repos are %v", [container.name, container.image, input.parameters.repos])
}
violation[{"msg": msg}] {
container := input.review.object.spec.ephemeralContainers[_]
satisfied := [good | repo = input.parameters.repos[_] ; good = startswith(container.image, repo)]
not any(satisfied)
not strings.any_prefix_match(container.image, input.parameters.repos)
msg := sprintf("ephemeralContainer <%v> has an invalid image repo <%v>, allowed repos are %v", [container.name, container.image, input.parameters.repos])
}
22 changes: 22 additions & 0 deletions artifacthub/library/general/allowedrepos/1.0.1/artifacthub-pkg.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
version: 1.0.1
name: k8sallowedrepos
displayName: Allowed Repositories
createdAt: "2023-10-30T20:28:22Z"
description: Requires container images to begin with a string from the specified list.
digest: eaff16a982c2d3029b280b3d4061d82b55215ac648efaafa341e25c7c77b635f
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/allowedrepos
keywords:
- gatekeeper
- open-policy-agent
- policies
readme: |-
# Allowed Repositories
Requires container images to begin with a string from the specified list.
install: |-
### Usage
```shell
kubectl apply -f https://raw.githubusercontent.com/open-policy-agent/gatekeeper-library/master/artifacthub/library/general/allowedrepos/1.0.1/template.yaml
```
provider:
name: Gatekeeper Library
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
resources:
- template.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: K8sAllowedRepos
metadata:
name: repo-is-openpolicyagent
spec:
match:
kinds:
- apiGroups: [""]
kinds: ["Pod"]
namespaces:
- "default"
parameters:
repos:
- "openpolicyagent/"
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-disallowed
spec:
initContainers:
- name: nginx
image: nginx
resources:
limits:
cpu: "100m"
memory: "30Mi"
containers:
- name: nginx
image: nginx
resources:
limits:
cpu: "100m"
memory: "30Mi"
ephemeralContainers:
- name: nginx
image: nginx
resources:
limits:
cpu: "100m"
memory: "30Mi"
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: v1
kind: Pod
metadata:
name: opa-allowed
spec:
containers:
- name: opa
image: openpolicyagent/opa:0.9.2
args:
- "run"
- "--server"
- "--addr=localhost:8080"
resources:
limits:
cpu: "100m"
memory: "30Mi"
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-disallowed
spec:
initContainers:
- name: nginxinit
image: nginx
resources:
limits:
cpu: "100m"
memory: "30Mi"
containers:
- name: nginx
image: nginx
resources:
limits:
cpu: "100m"
memory: "30Mi"
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-disallowed
spec:
containers:
- name: nginx
image: nginx
resources:
limits:
cpu: "100m"
memory: "30Mi"
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-disallowed
spec:
initContainers:
- name: nginxinit
image: nginx
resources:
limits:
cpu: "100m"
memory: "30Mi"
containers:
- name: opa
image: openpolicyagent/opa:0.9.2
args:
- "run"
- "--server"
- "--addr=localhost:8080"
resources:
limits:
cpu: "100m"
memory: "30Mi"
43 changes: 43 additions & 0 deletions artifacthub/library/general/allowedrepos/1.0.1/suite.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
kind: Suite
apiVersion: test.gatekeeper.sh/v1alpha1
metadata:
name: allowedrepos
tests:
- name: allowed-repos
template: template.yaml
constraint: samples/repo-must-be-openpolicyagent/constraint.yaml
cases:
- name: example-allowed
object: samples/repo-must-be-openpolicyagent/example_allowed.yaml
assertions:
- violations: no
- name: container-disallowed
object: samples/repo-must-be-openpolicyagent/example_disallowed_container.yaml
assertions:
- violations: yes
message: container
- name: initcontainer-disallowed
object: samples/repo-must-be-openpolicyagent/example_disallowed_initcontainer.yaml
assertions:
- violations: 1
message: initContainer
- violations: 0
message: container
- name: both-disallowed
object: samples/repo-must-be-openpolicyagent/example_disallowed_both.yaml
assertions:
- violations: 2
- message: initContainer
violations: 1
- message: container
violations: 1
- name: all-disallowed
object: samples/repo-must-be-openpolicyagent/disallowed_all.yaml
assertions:
- violations: 3
- message: initContainer
violations: 1
- message: container
violations: 1
- message: ephemeralContainer
violations: 1
46 changes: 46 additions & 0 deletions artifacthub/library/general/allowedrepos/1.0.1/template.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
apiVersion: templates.gatekeeper.sh/v1
kind: ConstraintTemplate
metadata:
name: k8sallowedrepos
annotations:
metadata.gatekeeper.sh/title: "Allowed Repositories"
metadata.gatekeeper.sh/version: 1.0.1
description: >-
Requires container images to begin with a string from the specified list.
spec:
crd:
spec:
names:
kind: K8sAllowedRepos
validation:
# Schema for the `parameters` field
openAPIV3Schema:
type: object
properties:
repos:
description: The list of prefixes a container image is allowed to have.
type: array
items:
type: string
targets:
- target: admission.k8s.gatekeeper.sh
rego: |
package k8sallowedrepos
violation[{"msg": msg}] {
container := input.review.object.spec.containers[_]
not strings.any_prefix_match(container.image, input.parameters.repos)
msg := sprintf("container <%v> has an invalid image repo <%v>, allowed repos are %v", [container.name, container.image, input.parameters.repos])
}
violation[{"msg": msg}] {
container := input.review.object.spec.initContainers[_]
not strings.any_prefix_match(container.image, input.parameters.repos)
msg := sprintf("initContainer <%v> has an invalid image repo <%v>, allowed repos are %v", [container.name, container.image, input.parameters.repos])
}
violation[{"msg": msg}] {
container := input.review.object.spec.ephemeralContainers[_]
not strings.any_prefix_match(container.image, input.parameters.repos)
msg := sprintf("ephemeralContainer <%v> has an invalid image repo <%v>, allowed repos are %v", [container.name, container.image, input.parameters.repos])
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ createdAt: "2022-09-26T17:28:27Z"
description: |-
Requires containers to have memory and CPU limits set and constrains limits to be within the specified maximum values.
https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
digest: 491c7efedf4f358fd2bdd3f720c5280708dbc2472fd7d1f334ee0c9569b46e74
digest: 114d90558b4439e82686b585302f004e21fc0045c678d1b5d21002a0596e40fc
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/containerlimits
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ spec:
canonify_cpu(orig) = new {
not is_number(orig)
not endswith(orig, "m")
re_match("^[0-9]+(\\.[0-9]+)?$", orig)
regex.match("^[0-9]+(\\.[0-9]+)?$", orig)
new := to_number(orig) * 1000
}
Expand Down Expand Up @@ -162,7 +162,7 @@ spec:
not is_number(orig)
suffix := get_suffix(orig)
raw := replace(orig, suffix, "")
re_match("^[0-9]+(\\.[0-9]+)?$", raw)
regex.match("^[0-9]+(\\.[0-9]+)?$", raw)
new := to_number(raw) * mem_multiple(suffix)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
version: 1.0.1
name: k8scontainerlimits
displayName: Container Limits
createdAt: "2023-10-30T20:28:54Z"
description: |-
Requires containers to have memory and CPU limits set and constrains limits to be within the specified maximum values.
https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
digest: fa0ce11ca6bacb6320053424c65d0626ee7ef96b4d359862eea031b13c8239ce
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/containerlimits
keywords:
- gatekeeper
- open-policy-agent
- policies
readme: |-
# Container Limits
Requires containers to have memory and CPU limits set and constrains limits to be within the specified maximum values.
https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
install: |-
### Usage
```shell
kubectl apply -f https://raw.githubusercontent.com/open-policy-agent/gatekeeper-library/master/artifacthub/library/general/containerlimits/1.0.1/template.yaml
```
provider:
name: Gatekeeper Library
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
resources:
- template.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: K8sContainerLimits
metadata:
name: container-must-have-limits
spec:
match:
kinds:
- apiGroups: [""]
kinds: ["Pod"]
parameters:
cpu: "200m"
memory: "1Gi"
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: v1
kind: Pod
metadata:
name: opa-allowed
labels:
owner: me.agilebank.demo
spec:
containers:
- name: opa
image: openpolicyagent/opa:0.9.2
args:
- "run"
- "--server"
- "--addr=localhost:8080"
resources:
limits:
cpu: "100m"
memory: "1Gi"
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: v1
kind: Pod
metadata:
name: opa-disallowed
labels:
owner: me.agilebank.demo
spec:
containers:
- name: opa
image: openpolicyagent/opa:0.9.2
args:
- "run"
- "--server"
- "--addr=localhost:8080"
resources:
limits:
cpu: "100m"
memory: "2Gi"
Loading

0 comments on commit 1c8907a

Please sign in to comment.