Exclude UPDATE operations in constraints for immutable fields

[ordovicia]

What this PR does / why we need it: This PR will fix a problem described in #346, by excluding UPDATE operations in constraint templates for immutable fields.

Which issue(s) does this PR fix (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #346

Special notes for your reviewer: None.

[ordovicia]

Hi @apeabody @maxsmythe , could you take a look? Thank you!

[apeabody]

Thanks for the contribution @ordovicia!

A few thoughts/questions:

• Just confirming we want/need to exclude PATCH? The default validating webhook verbs are CREATE and UPDATE: https://github.com/open-policy-agent/gatekeeper/blob/v3.12.0/deploy/gatekeeper.yaml#L3747 | @maxsmythe, any caveats to PATCH?
• Thanks for the rego/opa test for lib.exclude_update_patch! We should probably also include some tests for the templates (and maybe even constraints):
  • For the templates you can add "operation": "UPDATE" to the "review": {}, for example: https://github.com/open-policy-agent/gatekeeper-library/blob/master/src/general/automount-serviceaccount-token/src_test.rego#L10
  • For the constraints you could use a AdmissionReview object to include request.operation: UPDATE for the test suite.

[maxsmythe]

PATCH is probably irrelevant. From the K8s docs:

Operations is the operations the admission hook cares about - CREATE, UPDATE, DELETE, CONNECT or * for all of those operations and any future admission operations that are added. If '*' is present, the length of the slice must be one. Required.

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#rulewithoperations-v1-admissionregistration-k8s-io

[ordovicia]

Thank you for the comments!

Fixed to exclude UPDATE operation only.
Also added tests for constraint templates.
But I could not find how to add UPDATE operation tests for constraints.

[apeabody]

Thank you for the comments!

Fixed to exclude UPDATE operation only. Also added tests for constraint templates. But I could not find how to add UPDATE operation tests for constraints.

Thanks @ordovicia! You should be able to use a AdmissionReview object in the test suite to include the operation type. For example with K8sPSPAutomountServiceAccountTokenPod you could add a new allowed similar to the existing disallowed but allowed for the UPDATE operation test. Instead of the using the Pod object directly, for object: you would use an AdmissionReview object similar to:

kind: AdmissionReview
apiVersion: admission.k8s.io/v1beta1
request:
  operation: "UPDATE"
  object:
    apiVersion: v1
    kind: Pod
    metadata:
      name: nginx-automountserviceaccounttoken-update-allowed
      labels:
        app: nginx-automountserviceaccounttoken
    spec:
      automountServiceAccountToken: true
      containers:
        - name: nginx
          image: nginx

The idea being this Pod should normally raise a violation, but not if the operation is "UPDATE".

[ordovicia]

Thank you @apeabody !
I added tests for constraints.

[ordovicia]

Hi @apeabody @maxsmythe , could you take a look again?

[apeabody]

Thanks @ordovicia!

One question/comment on lib.exclude_update.

Cheers!

[ordovicia]

Hi @apeabody please merge this if it's OK. Thank you!

[apeabody]

Thanks for the contribution @ordovicia!

Another maintainer will need to review as well.

[ritazh]

LGTM

Thanks for the fix @ordovicia! 🎉 I updated the PR description to remove patch.