🐛 My previous change has revive/stylecheck warnings because I made the `warn` return value `[]error` and it wants the `error` last, which is kind of silly.
This mutes the linter on this signature, since it hits both revive and stylecheck
/kind bug
🧹 Previously we lacked a public method for turning our `apis.FieldError` multi-error into a list of constituent error messages, so when we turned things into a webhook warning we simply used the combined serialization of all of the warnings. Thanks to Nghia's recent change, we can now access the list of warnings to return as a list of errors.
/kind cleanup
* Preserve webhook namespaceSelector.matchLabels
I have a webhook with this definition and the reconciler is
removing the matchLabels field:
Current resource:
```
namespaceSelector:
matchExpressions:
- key: webhooks.knative.dev/exclude
operator: DoesNotExist
objectSelector:
matchLabels:
app.kubernetes.io/component: kafka-dispatcher
```
Applied resource:
```
namespaceSelector:
matchExpressions: [ ]
matchLabels:
app.kubernetes.io/name: knative-eventing
objectSelector:
matchLabels:
app.kubernetes.io/component: kafka-dispatcher
```
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* Optimize cases that don't need ensureLabelSelectorRequirements
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
When a callback is registered for a gvk that isn't part of the
handlers that callback is never called since there is no webhook
rule associated with that GVK.
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* WIP: just one option exploration.
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
* Another option. New signature and new type for configuration.
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
* Use interfaces and type assertions.
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
* log custom config with debug.
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
* Address PR feedback.
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
* Add support for admission webhook warnings.
This extends `apis.FieldError` to support designating certain FieldErrors as "warnings" (or explicitly as "errors", however, this is the default for back-compat).
You can turn an `apis.FieldError` into a warning using: `fe.At(apis.WarningLevel)` or force it into an error using: `fe.At(apis.ErrorLevel)`.
You can get the errors at a particular diagnostic level using: `fe.Filter(apis.WarningLevel)`.
This change also hooks this into the admission webhook infrastructure to support surfacing the "warning" level `apis.FieldError`s via the `Warnings` section of the `AdmissionResponse`.
Fixes: #2497
* Add a comment about the use of defer.
When the admission request is for a resource with an empty string as
group, which happens on core resources, the `creator` or `lastModifier`
annotations are invalid since they become `/creator` or
`/lastModifier`.
This patch removes the `/` when group = `""`.
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* Add support for callback defaults
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
* Put unstr object in ctx and set user info
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
* Move get callback at the top
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
* Panic when using delete verb
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
* Split tests and add callback ctx tests
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
* Set user info annotations
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
* Register Webhook Rules from callbacks
Signed-off-by: Pierangelo Di Pilato <pdipilat@redhat.com>
* Adapt unstructured objects to apis.HasSpec
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* Change json tag name to match struct field name
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* Introduce `NewContext`, deprecate `NewImplFull`.
Our generated `NewImpl` methods have long taken `context.Context`, but despite many iterations the forms we expose from our `controller` package never have. This change contains several elements:
1. Expose a new `NewContext` method that takes `context.Context` in addition to the current `NewImplFull` signature.
2. Call `NewContext` instead of the deprecated `NewImpl` from our generated controller code.
3. Call `NewContext` from all our webhook reconcilers.
* Add a Tracker to controller.Impl to cut down on downstream boilerplate.
This change introduces a new `controller.NewSkipKey` method to designate certain reconciliations as "skipped".
The primary motivation for this is to squelch useless logging on non-leader replicas, which currently report success with trivial latency.
I have plumbed this through existing reconcilers and the code-gen so most things downstream should get this for free. In places where a key is observed, I do not mark the reconcile as skipped as the reconciler did some processing for which the awareness of side-effects and reported latency may be interesting.
* Add smart handling of selectors in webhooks
This is an alternative fix for #1590. Instead of arbitrarily adding a label from a different project to avoid the reconcilers racing, this adds "smart" handling of the selectors in that labels not inside the knative.dev domain are plainly ignored and our own selectors are added additively.
* Fix formatting
* Fix missing variable usage
* Enable golint and exclude some other generated or additional dirs
Also remove `test` ignore, since it's covered by path ignore rule.
* meh
* fixes
* more
* progressing
* further
* like a boss
* Various cleanups around the codebase
- unindent the else after return
- make things private that are not used anywhere
- rearrange params
- etc
* add
* include a filter on control plane namespaces for defaulting and validation webhooks from knative/pkg
* Update unit tests to include control-plane
* adding a comment to explain why we are adding 'control-plane' to the webhook config
* Use two lane queue instead of the regular workqueue
- we need to poll for len in the webhook tests because we have async propagation now, and check at the wrong time will be not correct.
- otherwise just a drop in replacement.
* update test
* cmt
* tests hardened
* Allows for webhooks to exclude certain namspaces
Added a namespaces selector to the mutating webhook configuration which
allows for excluding namespaces from the webhook
Fixes#1379
* Updated skipWebhooks key to skip-webhooks for defaulting and validating
webhooks
* Updated table tests with new label
* Updated key name to webhooks.knative.dev/exclude
* Add the key of the object to the log context
We don't log _what_ we convert, but only _what type_ it is.
And it's not very useful
So log all the stuff
* issues
* redo
* Remove unused code.
* Use raw strings to avoid escaping.
* Remove unneeded type conversions.
* Preallocate slices where possible.
* Use semantic equality in psbinding reconciler.
Mathew Wicks
Jonathan Innis
Dave Protasowski
Prashant Rewar
cola
Hector Fernandez
Matt Moore
Pierangelo Di Pilato
Ville Aikas
Paul Schweigert
Pierangelo Di Pilato
knative-automation
Matt Moore
Markus Thömmes
NovaHe
Matt Moore
Victor Agababov
Lionel Villard
Josh Soref
Zbynek Roubalik
Scott Nichols
Mike Petersen