open-policy-agent / gatekeeper

🐊 Gatekeeper - Policy Controller for Kubernetes
https://open-policy-agent.github.io/gatekeeper/
Apache License 2.0
3.7k stars 759 forks source link

Gatekeeper violation msg requirements different from standard OPA #1168

Open rbkaspr opened 3 years ago

rbkaspr commented 3 years ago

What steps did you take and what happened:

apiVersion: templates.gatekeeper.sh/v1beta1
kind: ConstraintTemplate
metadata:
  creationTimestamp: null
  name: namespacerequireowner
spec:
  crd:
    spec:
      names:
        kind: NamespaceRequireOwner
  targets:
  - libs:
    - |-
      package lib.core
      default is_gatekeeper = false
      is_gatekeeper {
        has_field(input, "review")
        has_field(input.review, "object")
      }
      resource = input.review.object {
        is_gatekeeper
      }
      resource = input {
        not is_gatekeeper
      }
      apiVersion = resource.apiVersion
      name = resource.metadata.name
      kind = resource.kind
      labels = resource.metadata.labels
      annotations = resource.metadata.annotations
      gv := split(apiVersion, "/")
      group = gv[0] {
        contains(apiVersion, "/")
      }
      group = "core" {
        not contains(apiVersion, "/")
      }
      version := gv[count(gv) -1]
      has_field(obj, field) {
        not object.get(obj, field, "N_DEFINED") == "N_DEFINED"
      }
      missing_field(obj, field) = true {
        obj[field] == ""
      }
     missing_field(obj, field) = true {
        not has_field(obj, field)
      }
    rego: |-
      package namespace_require_owner
      import data.lib.core
      violation[msg] {
        missing_label(core.labels)
        msg = sprintf("%s: Namespace is missing the owner label", [core.name])
      }
      missing_label(labels) {
        not labels.owner
      }
    target: admission.k8s.gatekeeper.sh
status: {}
kubectl get constrainttemplate.templates.gatekeeper.sh/namespacerequireowner -ojsonpath='{.status}' | jq
{
  "byPod": [
    {
      "errors": [
        {
          "code": "ingest_error",
          "message": "Could not ingest Rego: 2 errors occurred:\nhooks[\"admission.k8s.gatekeeper.sh\"].hooks_builtin:32: rego_type_error: undefined ref: r.msg\n\tr.msg\n\t^\n\thave: string\nhooks[\"admission.k8s.gatekeeper.sh\"].hooks_builtin:53: rego_type_error: undefined ref: r.msg\n\tr.msg\n\t^\n\thave: string"
        }
      ],
      "id": "gatekeeper-audit-84964f86f-rhcnt",
      "observedGeneration": 1,
      "operations": [
        "audit",
        "status"
      ],
      "templateUID": "1b5c2d6a-447f-4dff-9566-53a7ce22b8b1"
    },
    {
      "errors": [
        {
          "code": "ingest_error",
          "message": "Could not ingest Rego: 2 errors occurred:\nhooks[\"admission.k8s.gatekeeper.sh\"].hooks_builtin:32: rego_type_error: undefined ref: r.msg\n\tr.msg\n\t^\n\thave: string\nhooks[\"admission.k8s.gatekeeper.sh\"].hooks_builtin:53: rego_type_error: undefined ref: r.msg\n\tr.msg\n\t^\n\thave: string"
        }
      ],
      "id": "gatekeeper-controller-manager-5bb5f9b4dd-8sk6q",
      "observedGeneration": 1,
      "operations": [
        "webhook"
      ],
      "templateUID": "1b5c2d6a-447f-4dff-9566-53a7ce22b8b1"
    },
    {
      "errors": [
        {
          "code": "ingest_error",
          "message": "Could not ingest Rego: 2 errors occurred:\nhooks[\"admission.k8s.gatekeeper.sh\"].hooks_builtin:32: rego_type_error: undefined ref: r.msg\n\tr.msg\n\t^\n\thave: string\nhooks[\"admission.k8s.gatekeeper.sh\"].hooks_builtin:53: rego_type_error: undefined ref: r.msg\n\tr.msg\n\t^\n\thave: string"
        }
      ],
      "id": "gatekeeper-controller-manager-5bb5f9b4dd-9t7j6",
      "observedGeneration": 1,
      "operations": [
        "webhook"
      ],
      "templateUID": "1b5c2d6a-447f-4dff-9566-53a7ce22b8b1"
    },
    {
      "errors": [
        {
          "code": "ingest_error",
          "message": "Could not ingest Rego: 2 errors occurred:\nhooks[\"admission.k8s.gatekeeper.sh\"].hooks_builtin:32: rego_type_error: undefined ref: r.msg\n\tr.msg\n\t^\n\thave: string\nhooks[\"admission.k8s.gatekeeper.sh\"].hooks_builtin:53: rego_type_error: undefined ref: r.msg\n\tr.msg\n\t^\n\thave: string"
        }
      ],
      "id": "gatekeeper-controller-manager-5bb5f9b4dd-d7hvt",
      "observedGeneration": 1,
      "operations": [
        "webhook"
      ],
      "templateUID": "1b5c2d6a-447f-4dff-9566-53a7ce22b8b1"
    }
  ],
  "created": true
}

What did you expect to happen: Based on just looking at standard OPA, I would expect violation[msg] to be a perfectly functional rule signature, but Gatekeeper requires a struct containing, at minimum, and msg key. Making that requirement much more explicit in the Gatekeeper docs would help, but it would be nice if Gatekeeper was able to handle normal OPA syntax rule signatures, if only to ease the transition as more people convert their existing OPA policy libraries over to running on Gatekeeper

Environment:

maxsmythe commented 3 years ago

Thanks for the feedback!

IIRC standard OPA policies use rule headers like:

deny[msg]

would keeping that same rule header but adding the Gatekeeper-specific:

violation[{"msg": msg}] {
   deny[msg]
}

work for migrating without the need to refactor?

The reason for using an object-based return value was to leave room for returning machine-readable data so that we left open the possibility for things like automated remediation in the future.

rbkaspr commented 3 years ago

Thanks for the response!

As it turns out, your suggested modification works like a charm. If it's important for planned future features of Gatekeeper, then I'm totally on board with the response structure as it stands, I would just like to see it called out a little more clearly on the documentation page for the benefit of those migrating from using standard OPA to Gatekeeper.
Same with the requirement for violation instead of deny, it's a convention you can pick up from the examples, but having it be called out more explicitly will probably help prevent confusion from those who may expect that they can just pick up their existing policy libraries from OPA and drop them into Gatekeeper without modification.
I'd be happy to contribute back a docs page to that effect, if need be.

sozercan commented 3 years ago

This is documented in https://github.com/open-policy-agent/frameworks/tree/master/constraint#rule-schema but it's a very obscure location. This should be reflected in Gatekeeper docs too.

rbkaspr commented 3 years ago

Yeah, buried in the docs pages of a completely different repo isn't exactly easy to find for people just getting started with Gatekeeper, especially if they don't also have a ton of experience with OPA (like a certain me).

If we could get that page replicated to the Gatekeeper repo, that would close this issue nicely.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

maxsmythe commented 2 years ago

This still seems relevant

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.