controller-gen feature request: support objectSelector in webhook config generation

[wbrefvem]

Currently (v0.5.0) the +kubebuilder:webhook marker does not support setting an objectSelector on a webhook. objectSelector is especially handy when mutating or validating core types. For example, I want to be able to set an environment variable on a subset of pods in a particular namespace, but not my controller manager pod, which runs in the same namespace and runs the webhook server.

I realize that the docs recommend running the webhook server in a separate namespace and scoping the validation to the target namespace to avoid deadlock, but that's not feasible for various reasons. Setting a label on the pods I want to mutate works perfectly fine but there's no way to do it (that I know of) without breaking my workflow, which involves generating the MutatingWebhookConfiguration with markers and patching it with kustomize in one shot. (I'm using a lightly modified version of what kubebuilder init provides.) I could patch the objectSelector field if it were there in the generated config.

I propose something as simple as an optional objectSelectorLabel=string that generates the following:

objectSelector:
    matchLabels:
        foo: patchMe

Where foo is the user-provided string and patchMe is some default that the user can patch.

I'm happy to put something together if there's an appetite for it.

[wbrefvem]

This is solvable with a strategic merge patch in kustomize:

webhooks:
- $patch: merge
  name: my-webhook-config
  objectSelector:
    matchLabels:
      foo: bar

But a marker parameter sure would be nice. :)

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[k8s-triage-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[pmalek]

Still unimplemented. Patches via kustomize are not always viable (when e.g. the source of truth is in *.go files).

To me it looks like it would boil down to extending the config (and parsing rules):

controller-tools/pkg/webhook/parser.go

Lines 61 to 136 in 76b24b2

	type Config struct {
	// Mutating marks this as a mutating webhook (it's validating only if false)
	//
	// Mutating webhooks are allowed to change the object in their response,
	// and are called *before* all validating webhooks. Mutating webhooks may
	// choose to reject an object, similarly to a validating webhook.
	Mutating bool
	// FailurePolicy specifies what should happen if the API server cannot reach the webhook.
	//
	// It may be either "ignore" (to skip the webhook and continue on) or "fail" (to reject
	// the object in question).
	FailurePolicy string
	// MatchPolicy defines how the "rules" list is used to match incoming requests.
	// Allowed values are "Exact" (match only if it exactly matches the specified rule)
	// or "Equivalent" (match a request if it modifies a resource listed in rules, even via another API group or version).
	MatchPolicy string `marker:",optional"`
	// SideEffects specify whether calling the webhook will have side effects.
	// This has an impact on dry runs and `kubectl diff`: if the sideEffect is "Unknown" (the default) or "Some", then
	// the API server will not call the webhook on a dry-run request and fails instead.
	// If the value is "None", then the webhook has no side effects and the API server will call it on dry-run.
	// If the value is "NoneOnDryRun", then the webhook is responsible for inspecting the "dryRun" property of the
	// AdmissionReview sent in the request, and avoiding side effects if that value is "true."
	SideEffects string `marker:",optional"`
	// TimeoutSeconds allows configuring how long the API server should wait for a webhook to respond before treating the call as a failure.
	// If the timeout expires before the webhook responds, the webhook call will be ignored or the API call will be rejected based on the failure policy.
	// The timeout value must be between 1 and 30 seconds.
	// The timeout for an admission webhook defaults to 10 seconds.
	TimeoutSeconds int `marker:",optional"`
	// Groups specifies the API groups that this webhook receives requests for.
	Groups []string
	// Resources specifies the API resources that this webhook receives requests for.
	Resources []string
	// Verbs specifies the Kubernetes API verbs that this webhook receives requests for.
	//
	// Only modification-like verbs may be specified.
	// May be "create", "update", "delete", "connect", or "*" (for all).
	Verbs []string
	// Versions specifies the API versions that this webhook receives requests for.
	Versions []string
	// Name indicates the name of this webhook configuration. Should be a domain with at least three segments separated by dots
	Name string
	// Path specifies that path that the API server should connect to this webhook on. Must be
	// prefixed with a '/validate-' or '/mutate-' depending on the type, and followed by
	// $GROUP-$VERSION-$KIND where all values are lower-cased and the periods in the group
	// are substituted for hyphens. For example, a validating webhook path for type
	// batch.tutorial.kubebuilder.io/v1,Kind=CronJob would be
	// /validate-batch-tutorial-kubebuilder-io-v1-cronjob
	Path string `marker:"path,optional"`
	// WebhookVersions specifies the target API versions of the {Mutating,Validating}WebhookConfiguration objects
	// itself to generate. The only supported value is v1. Defaults to v1.
	WebhookVersions []string `marker:"webhookVersions,optional"`
	// AdmissionReviewVersions is an ordered list of preferred `AdmissionReview`
	// versions the Webhook expects.
	AdmissionReviewVersions []string `marker:"admissionReviewVersions"`
	// ReinvocationPolicy allows mutating webhooks to request reinvocation after other mutations
	//
	// To allow mutating admission plugins to observe changes made by other plugins,
	// built-in mutating admission plugins are re-run if a mutating webhook modifies
	// an object, and mutating webhooks can specify a reinvocationPolicy to control
	// whether they are reinvoked as well.
	ReinvocationPolicy string `marker:"reinvocationPolicy,optional"`
	// URL allows mutating webhooks configuration to specify an external URL when generating
	// the manifests, instead of using the internal service communication. Should be in format of
	// https://address:port/path
	// When this option is specified, the serviceConfig.Service is removed from webhook the manifest.
	// The URL configuration should be between quotes.
	// `url` cannot be specified when `path` is specified.
	URL string `marker:"url,optional"`
	}

but we'd need to agree on the API (since the nested structure of objectSelector makes it a bit more nuanced to "get right" than other fields.

I'd be willing to contribute this if we have any idea how we'd like to API to look like.

A quick draft that I can come up with:

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mytype,...,objectSelector=matchExpressions[key=testdata.com/validation;operator=Exists]

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mytype,...,objectSelector=matchLabels[testdata.com/validation=myvalue]

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mytype,...,objectSelector=matchExpressions[key=testdata.com/validation;operator=In;values=[first;second]]

but I'm not sure if that's the way forward.

cc: @sbueringer

/reopen
/remove-lifecycle rotten

[k8s-ci-robot]

@pmalek: Reopened this issue.

In response to this:

Still unimplemented. Patches via kustomize are not always viable (when e.g. the source of truth is in *.go files).

To me it looks like it would boil down to extending the config (and parsing rules):

controller-tools/pkg/webhook/parser.go

Lines 61 to 136 in 76b24b2

	type Config struct {
	// Mutating marks this as a mutating webhook (it's validating only if false)
	//
	// Mutating webhooks are allowed to change the object in their response,
	// and are called *before* all validating webhooks. Mutating webhooks may
	// choose to reject an object, similarly to a validating webhook.
	Mutating bool
	// FailurePolicy specifies what should happen if the API server cannot reach the webhook.
	//
	// It may be either "ignore" (to skip the webhook and continue on) or "fail" (to reject
	// the object in question).
	FailurePolicy string
	// MatchPolicy defines how the "rules" list is used to match incoming requests.
	// Allowed values are "Exact" (match only if it exactly matches the specified rule)
	// or "Equivalent" (match a request if it modifies a resource listed in rules, even via another API group or version).
	MatchPolicy string `marker:",optional"`
	// SideEffects specify whether calling the webhook will have side effects.
	// This has an impact on dry runs and `kubectl diff`: if the sideEffect is "Unknown" (the default) or "Some", then
	// the API server will not call the webhook on a dry-run request and fails instead.
	// If the value is "None", then the webhook has no side effects and the API server will call it on dry-run.
	// If the value is "NoneOnDryRun", then the webhook is responsible for inspecting the "dryRun" property of the
	// AdmissionReview sent in the request, and avoiding side effects if that value is "true."
	SideEffects string `marker:",optional"`
	// TimeoutSeconds allows configuring how long the API server should wait for a webhook to respond before treating the call as a failure.
	// If the timeout expires before the webhook responds, the webhook call will be ignored or the API call will be rejected based on the failure policy.
	// The timeout value must be between 1 and 30 seconds.
	// The timeout for an admission webhook defaults to 10 seconds.
	TimeoutSeconds int `marker:",optional"`
	// Groups specifies the API groups that this webhook receives requests for.
	Groups []string
	// Resources specifies the API resources that this webhook receives requests for.
	Resources []string
	// Verbs specifies the Kubernetes API verbs that this webhook receives requests for.
	//
	// Only modification-like verbs may be specified.
	// May be "create", "update", "delete", "connect", or "*" (for all).
	Verbs []string
	// Versions specifies the API versions that this webhook receives requests for.
	Versions []string
	// Name indicates the name of this webhook configuration. Should be a domain with at least three segments separated by dots
	Name string
	// Path specifies that path that the API server should connect to this webhook on. Must be
	// prefixed with a '/validate-' or '/mutate-' depending on the type, and followed by
	// $GROUP-$VERSION-$KIND where all values are lower-cased and the periods in the group
	// are substituted for hyphens. For example, a validating webhook path for type
	// batch.tutorial.kubebuilder.io/v1,Kind=CronJob would be
	// /validate-batch-tutorial-kubebuilder-io-v1-cronjob
	Path string `marker:"path,optional"`
	// WebhookVersions specifies the target API versions of the {Mutating,Validating}WebhookConfiguration objects
	// itself to generate. The only supported value is v1. Defaults to v1.
	WebhookVersions []string `marker:"webhookVersions,optional"`
	// AdmissionReviewVersions is an ordered list of preferred `AdmissionReview`
	// versions the Webhook expects.
	AdmissionReviewVersions []string `marker:"admissionReviewVersions"`
	// ReinvocationPolicy allows mutating webhooks to request reinvocation after other mutations
	//
	// To allow mutating admission plugins to observe changes made by other plugins,
	// built-in mutating admission plugins are re-run if a mutating webhook modifies
	// an object, and mutating webhooks can specify a reinvocationPolicy to control
	// whether they are reinvoked as well.
	ReinvocationPolicy string `marker:"reinvocationPolicy,optional"`
	// URL allows mutating webhooks configuration to specify an external URL when generating
	// the manifests, instead of using the internal service communication. Should be in format of
	// https://address:port/path
	// When this option is specified, the serviceConfig.Service is removed from webhook the manifest.
	// The URL configuration should be between quotes.
	// `url` cannot be specified when `path` is specified.
	URL string `marker:"url,optional"`
	}

but we'd need to agree on the API (since the nested structure of objectSelector makes it a bit more nuanced to "get right" than other fields.

I'd be willing to contribute this if we have any idea how we'd like to API to look like.

A quick draft that I can come up with:

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mytype,...,objectSelector=matchExpressions[key=testdata.com/validation;operator=Exists]

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mytype,...,objectSelector=matchLabels[testdata.com/validation=myvalue]

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mytype,...,objectSelector=matchExpressions[key=testdata.com/validation;operator=In;values=[first;second]]

but I'm not sure if that's the way forward.

cc: @sbueringer

/reopen
/remove-lifecycle rotten

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[algo7]

Any updates on this?

[sbueringer]

@pmalek I think I would add a field named "ObjectSelector" to the Config struct the type would be a copy of metav1.LabelSelector. In that copy I would simply replace all the json markers through "marker:..." and then the resulting format should be fine.

Do you see parts of that struct where it would be necessary to diverge from that?

[pmalek]

@sbueringer That sounds good 👍 I'm not sure what will be the resulting interface but the approach is sound to me.

I can work on that but I won't have time for this in the upcoming 2 weeks (or so). If anyone beats me to it feel free to jump on this.

[sbueringer]

Sounds good!