Webhooks intercepting exclude specific resources

[Markieta]

GKE clusters warn about Intercepting cluster-scoped system resources.

Google recommends excluding nodes, tokenreviews, subjectaccessreviews, and certificatesigningrequests on webhooks intercepting those as they consider it unsafe.

I believe matchConditions (FEATURE STATE: Kubernetes v1.28 [beta]) can be used here to exclude those cluster-scoped (or any other) resources on gatekeeper-validating-webhook-configuration & gatekeeper-mutating-webhook-configuration.

[ritazh]

Thanks for raising this @Markieta. Does configure your own validatingWebhookCustomRules work for your need?

validating webhook: https://github.com/open-policy-agent/gatekeeper/blob/master/charts/gatekeeper/templates/gatekeeper-validating-webhook-configuration-validatingwebhookconfiguration.yaml#L49

mutating webhook https://github.com/open-policy-agent/gatekeeper/blob/master/charts/gatekeeper/templates/gatekeeper-mutating-webhook-configuration-mutatingwebhookconfiguration.yaml#L50

Note: matchConditions (FEATURE STATE: Kubernetes v1.28 [beta]) is still beta in 1.28, most people are either not on 1.28 yet or they might not have this feature enabled for their cluster yet.

[Markieta]

It would be a bit tedious to explicitly state each resource type to intercept, but I agree it is probably the most feasible approach for now. Thanks for your input!

[leewoobin789]

@ritazh @Markieta
The Addition of matchConditions would be a great help for our use case, where we do not want to explicitly set all resources in the validatingCustomRules, rather denying one specific resource based on more advanced condition (via CEL)

what about adding matchConditions based on Capabilities.KubeVersion.Minor which is one of build-in objects of helm.
e.g.

{{- if ge (int .Capabilities.KubeVersion.Minor) 28 }}
   matchConditions: {{ toYaml .Values.validatingWebhookMatchConditions | nindent 4 }}
{{- end }}

if it is fine for you @ritazh , I would prepare another PR to contribute to this enhancement 😄

[ritazh]

I think that’s reasonable and valuable. WDYT @sozercan @maxsmythe ?

[maxsmythe]

SGTM, though we probably want to put the match conditions on their own line, and support multiple entries.

[leewoobin789]

@maxsmythe yes. just like the value of validatingWebhookAnnotations. setting the default value to be {} should be sufficient for multiple entries.
furthermore as there is also no objectSelector for the check-ignore-label.gatekeeper.sh, I would set the scope of applying matchConditions only for validation.gatekeeper.sh.
this should apply to the MutatingWebhookConfiguration
WDYT?

[maxsmythe]

Do we want to allow for separate exclusions for mutation vs. validation?

[leewoobin789]

@maxsmythe I would recommend & advocate using separate ones to align with the way how it is implemented for objectSelector. mainly for the sake of consistency.

PR is in place 👍

[tuananhnguyen-ct]

We used the following in helm manifest

validatingWebhookMatchConditions:
  - expression: '!(request.resource.resource in ["nodes", "tokenreviews", "subjectaccessreviews", "certificatesigningrequests"])'
    name: exclude-system-resources
mutatingWebhookMatchConditions:
  - expression: '!(request.resource.resource in ["nodes", "tokenreviews", "subjectaccessreviews", "certificatesigningrequests"])'
    name: exclude-system-resources

but it doesn't seem to work. Is it necessary to modify the resources list and remove nodes/proxy instead?

[mcariatm]

Hi @tuananhnguyen-ct, did you manage to make it work?

I'm using the same pattern but looks like the GKE still warn and doesn't understand the matchConditions. I can see them in MutatingWebhookConfiguration.

Update:
I think it was a mistake as I used "subjectacessreviews" instead of "subjectaccessreviews":

Looks like the google documentation is wrong: