Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webhooks intercepting exclude specific resources #3038

Closed
Markieta opened this issue Oct 5, 2023 · 8 comments · Fixed by #3343
Closed

Webhooks intercepting exclude specific resources #3038

Markieta opened this issue Oct 5, 2023 · 8 comments · Fixed by #3343
Labels
enhancement New feature or request

Comments

@Markieta
Copy link

Markieta commented Oct 5, 2023

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
Copy link
Member

ritazh commented Oct 9, 2023

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
Copy link
Author

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
Copy link
Contributor

leewoobin789 commented Oct 18, 2023

@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
Copy link
Member

ritazh commented Oct 18, 2023

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

@maxsmythe
Copy link
Contributor

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

@leewoobin789
Copy link
Contributor

leewoobin789 commented Oct 20, 2023

@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
Copy link
Contributor

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

@leewoobin789
Copy link
Contributor

leewoobin789 commented Oct 23, 2023

@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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants