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 <[email protected]>
  • Loading branch information
anderseknert committed Oct 30, 2023
1 parent 68b6d53 commit 1de0f97
Show file tree
Hide file tree
Showing 161 changed files with 2,090 additions and 2,216 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.54.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])
}
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
Expand Up @@ -5,7 +5,7 @@ createdAt: "2022-09-26T17:28:27Z"
description: |-
Requires containers to have memory and CPU requests set and constrains requests to be within the specified maximum values.
https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
digest: 05acb93b1d05819fbf80dce1bdfc13a361cbbdadcf11ce8227c113fa45fca9d2
digest: be19ed966fb2e508e498b707fa26347acfd350333e02d15e1e06b46b90a688cc
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/containerrequests
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
Expand Up @@ -5,7 +5,7 @@ createdAt: "2022-09-26T17:28:27Z"
description: |-
Sets a maximum ratio for container resource limits to requests.
https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
digest: 18a2e1bd82607bec62a8f323881d6549b7a98b91393a1ee43367a0b19c5bc7de
digest: a0580b11a241e4c5981ecaf14454ca9b0ff4685b33825c83267fc62117e80ea6
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/containerresourceratios
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@ spec:
canonify_cpu(orig) = new {
not is_number(orig)
not endswith(orig, "m")
re_match("^[0-9]+$", orig)
regex.match("^[0-9]+$", orig)
new := to_number(orig) * 1000
}
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 @@ -172,7 +172,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
Expand Up @@ -5,7 +5,7 @@ createdAt: "2022-09-26T17:28:27Z"
description: |-
Requires container images to have an image tag different from the ones in the specified list.
https://kubernetes.io/docs/concepts/containers/images/#image-names
digest: 997b327dccccafffd640617a7c26a6723c1e8cf92e127b5297e6f698eb4e9af1
digest: 68db043169a1e546e13872490673f0898ca19e581cdb1427f3d58cc9a8b5b547
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/disallowedtags
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,15 @@ spec:
violation[{"msg": msg}] {
container := input_containers[_]
not is_exempt(container)
tags := [forbid | tag = input.parameters.tags[_] ; forbid = endswith(container.image, concat(":", ["", tag]))]
any(tags)
tags := [tag_with_prefix | tag := input.parameters.tags[_]; tag_with_prefix := concat(":", ["", tag])]
strings.any_suffix_match(container.image, tags)
msg := sprintf("container <%v> uses a disallowed tag <%v>; disallowed tags are %v", [container.name, container.image, input.parameters.tags])
}
violation[{"msg": msg}] {
container := input_containers[_]
not is_exempt(container)
tag := [contains(container.image, ":")]
not all(tag)
not contains(container.image, ":")
msg := sprintf("container <%v> didn't specify an image tag <%v>", [container.name, container.image])
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ createdAt: "2023-05-23T09:47:27Z"
description: |-
Requires containers to have an ephemeral storage limit set and constrains the limit to be within the specified maximum values.
https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
digest: 84077f1dbcdcab9a7c20710e82299995e44294fccdb1a5b9de63fb5a5032a6d8
digest: b77148e0b1336583085a9caccbb9fca25985be126de651d7f31de8bb63f9b18a
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/ephemeralstoragelimit
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,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) * storage_multiple(suffix)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ createdAt: "2023-03-01T18:38:35Z"
description: |-
Requires Ingress resources to be HTTPS only. Ingress resources must include the `kubernetes.io/ingress.allow-http` annotation, set to `false`. By default a valid TLS {} configuration is required, this can be made optional by setting the `tlsOptional` parameter to `true`.
https://kubernetes.io/docs/concepts/services-networking/ingress/#tls
digest: a984590ce76c2590ae11062be433b7e7f89548a94ad7a997f6166fd5c38df469
digest: 93b5fbe1c155422349e951752a528378c01a22a4735d8579ec30f2cb658b8759
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/httpsonly
keywords:
Expand Down
20 changes: 6 additions & 14 deletions artifacthub/library/general/httpsonly/1.0.1/template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,19 @@ spec:
violation[{"msg": msg}] {
input.review.object.kind == "Ingress"
re_match("^(extensions|networking.k8s.io)/", input.review.object.apiVersion)
regex.match("^(extensions|networking.k8s.io)/", input.review.object.apiVersion)
ingress := input.review.object
not https_complete(ingress)
not tls_is_optional(ingress)
not tls_is_optional
msg := sprintf("Ingress should be https. tls configuration and allow-http=false annotation are required for %v", [ingress.metadata.name])
}
violation[{"msg": msg}] {
input.review.object.kind == "Ingress"
re_match("^(extensions|networking.k8s.io)/", input.review.object.apiVersion)
regex.match("^(extensions|networking.k8s.io)/", input.review.object.apiVersion)
ingress := input.review.object
not annotation_complete(ingress)
not tls_not_optional(ingress)
tls_is_optional
msg := sprintf("Ingress should be https. The allow-http=false annotation is required for %v", [ingress.metadata.name])
}
Expand All @@ -65,15 +65,7 @@ spec:
ingress.metadata.annotations["kubernetes.io/ingress.allow-http"] == "false"
}
tls_is_optional(ingress) = true {
tls_is_optional {
parameters := object.get(input, "parameters", {})
tlsOptional := object.get(parameters, "tlsOptional", false)
is_boolean(tlsOptional)
true == tlsOptional
}
tls_not_optional(ingress) = true {
parameters := object.get(input, "parameters", {})
tlsOptional := object.get(parameters, "tlsOptional", false)
true != tlsOptional
object.get(parameters, "tlsOptional", false) == true
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ createdAt: "2022-09-26T17:28:27Z"
description: |-
Requires container images to contain a digest.
https://kubernetes.io/docs/concepts/containers/images/
digest: 28630d3b444cc18488fff8400f04c00f550cea36325f1c74a8d035136abac78f
digest: 5d2b468c2257fe4690ba48a08e430eae82c5ecbae89675b803033c78201ca4c7
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/imagedigests
keywords:
Expand Down
9 changes: 3 additions & 6 deletions artifacthub/library/general/imagedigests/1.0.0/template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,21 @@ spec:
violation[{"msg": msg}] {
container := input.review.object.spec.containers[_]
not is_exempt(container)
satisfied := [re_match("@[a-z0-9]+([+._-][a-z0-9]+)*:[a-zA-Z0-9=_-]+", container.image)]
not all(satisfied)
not regex.match("@[a-z0-9]+([+._-][a-z0-9]+)*:[a-zA-Z0-9=_-]+", container.image)
msg := sprintf("container <%v> uses an image without a digest <%v>", [container.name, container.image])
}
violation[{"msg": msg}] {
container := input.review.object.spec.initContainers[_]
not is_exempt(container)
satisfied := [re_match("@[a-z0-9]+([+._-][a-z0-9]+)*:[a-zA-Z0-9=_-]+", container.image)]
not all(satisfied)
not regex.match("@[a-z0-9]+([+._-][a-z0-9]+)*:[a-zA-Z0-9=_-]+", container.image)
msg := sprintf("initContainer <%v> uses an image without a digest <%v>", [container.name, container.image])
}
violation[{"msg": msg}] {
container := input.review.object.spec.ephemeralContainers[_]
not is_exempt(container)
satisfied := [re_match("@[a-z0-9]+([+._-][a-z0-9]+)*:[a-zA-Z0-9=_-]+", container.image)]
not all(satisfied)
not regex.match("@[a-z0-9]+([+._-][a-z0-9]+)*:[a-zA-Z0-9=_-]+", container.image)
msg := sprintf("ephemeralContainer <%v> uses an image without a digest <%v>", [container.name, container.image])
}
libs:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: noupdateserviceaccount
displayName: Block updating Service Account
createdAt: "2022-09-26T17:28:27Z"
description: Blocks updating the service account on resources that abstract over Pods. This policy is ignored in audit mode.
digest: 33b20c359f23e7732a231c797adf543f74bf4f9218d315563f6fff75f261d5ed
digest: 177b81289249b84bd7611b54962a1cebcdbfc60abe4c1692d89da37c65affb58
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/noupdateserviceaccount
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ spec:
rego: |
package noupdateserviceaccount
privileged(userInfo, allowedUsers, allowedGroups) {
privileged(userInfo, allowedUsers, _) {
# Allow if the user is in allowedUsers.
# Use object.get so omitted parameters can't cause policy bypass by
# evaluating to undefined.
username := object.get(userInfo, "username", "")
allowedUsers[_] == username
} {
}
privileged(userInfo, _, allowedGroups) {
# Allow if the user's groups intersect allowedGroups.
# Use object.get so omitted parameters can't cause policy bypass by
# evaluating to undefined.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: k8sreplicalimits
displayName: Replica Limits
createdAt: "2023-03-31T20:34:16Z"
description: Requires that objects with the field `spec.replicas` (Deployments, ReplicaSets, etc.) specify a number of replicas within defined ranges.
digest: a8dc0e94f30cdf1f4aff2e69551744b6eaee3f4e60dba3393705f81363a249e5
digest: da23fec64fead027deed0bbbb7231e994c15d258069eb852545f77ccd3931623
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/replicalimits
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ spec:
}
input_replica_limit(spec) {
provided := input.review.object.spec.replicas
provided := spec.replicas
count(input.parameters.ranges) > 0
range := input.parameters.ranges[_]
value_within_range(range, provided)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: k8srequiredannotations
displayName: Required Annotations
createdAt: "2022-09-26T17:28:27Z"
description: Requires resources to contain specified annotations, with values matching provided regular expressions.
digest: c4074e5cb8cc492504676bb95bbb9e588745f682949911f99e0b750df0cb3b15
digest: af6b59162079d08329ae848cb6c9d145b0a730359b8dd9c7c9fc8bdf229e5403
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/requiredannotations
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ spec:
expected := input.parameters.annotations[_]
expected.key == key
expected.allowedRegex != ""
not re_match(expected.allowedRegex, value)
not regex.match(expected.allowedRegex, value)
msg := sprintf("Annotation <%v: %v> does not satisfy allowed regex: %v", [key, value, expected.allowedRegex])
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: k8srequiredlabels
displayName: Required Labels
createdAt: "2022-09-26T17:28:27Z"
description: Requires resources to contain specified labels, with values matching provided regular expressions.
digest: e0f96d5945642406b067a22f152dcf76ac038a8d4f0969668c7e70c2471731db
digest: 575da13fa9c71a4beae64469af1eaba9041e1a9ed9fc68bac37c05e097c44f15
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/requiredlabels
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,11 @@ spec:
rego: |
package k8srequiredlabels
get_message(parameters, _default) = msg {
get_message(parameters, _default) := _default {
not parameters.message
msg := _default
}
get_message(parameters, _default) = msg {
msg := parameters.message
}
get_message(parameters, _) := parameters.message
violation[{"msg": msg, "details": {"missing_labels": missing}}] {
provided := {label | input.review.object.metadata.labels[label]}
Expand All @@ -65,7 +62,7 @@ spec:
expected.key == key
# do not match if allowedRegex is not defined, or is an empty string
expected.allowedRegex != ""
not re_match(expected.allowedRegex, value)
not regex.match(expected.allowedRegex, value)
def_msg := sprintf("Label <%v: %v> does not satisfy allowed regex: %v", [key, value, expected.allowedRegex])
msg := get_message(input.parameters, def_msg)
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ createdAt: "2023-06-12T20:51:47Z"
description: |-
Requires all Ingress rule hosts to be unique.
Does not handle hostname wildcards: https://kubernetes.io/docs/concepts/services-networking/ingress/
digest: 00196c02161e0cdcfa9d71ea5e2cd620f97d37a12250cc1bccb0c1187b9056c2
digest: 0bcdae11f0afd5a52494e0e2644c5e8a5eac947b0b5cf53b965ab2d4ee687c76
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/uniqueingresshost
keywords:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ spec:
violation[{"msg": msg}] {
input.review.kind.kind == "Ingress"
re_match("^(extensions|networking.k8s.io)$", input.review.kind.group)
regex.match("^(extensions|networking.k8s.io)$", input.review.kind.group)
host := input.review.object.spec.rules[_].host
other := data.inventory.namespace[_][otherapiversion]["Ingress"][name]
re_match("^(extensions|networking.k8s.io)/.+$", otherapiversion)
regex.match("^(extensions|networking.k8s.io)/.+$", otherapiversion)
other.spec.rules[_].host == host
not identical(other, input.review)
msg := sprintf("ingress host conflicts with an existing ingress <%v>", [host])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: k8spspcapabilities
displayName: Capabilities
createdAt: "2023-05-23T09:47:31Z"
description: Controls Linux capabilities on containers. Corresponds to the `allowedCapabilities` and `requiredDropCapabilities` fields in a PodSecurityPolicy. For more information, see https://kubernetes.io/docs/concepts/policy/pod-security-policy/#capabilities
digest: 140a62f0c286b67c659beb12c38186e4071495f00d1deca606a9df54c3735c44
digest: 0f2f9bc191d2fc3bd5a7f68469a4cface3661b1b5c962d8c9a36eaa9c4eb386e
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/capabilities
keywords:
Expand Down
Loading

0 comments on commit 1de0f97

Please sign in to comment.