* 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
* Implement a new shared "Drainer" handler.
This implements a new `http.Handler` called `Drainer`, which is intended to wrap some inner `http.Handler` business logic with a new outer handler that can respond to Kubelet probes (successfully until told to "Drain()").
This takes over the webhook's relatively new probe handling and lame duck logic with one key difference. Previously the webhook waited for a fixed period after SIGTERM before exitting, but the new logic waits for this same grace period AFTER THE LAST REQUEST. So if the handler keeps getting (non-probe) requests, the timer will continually reset, and once it stops receiving requests for the configured grace period, "Drain()" will return and the webhook will exit.
The goal of this work is to try to better cope with what we believe to be high tail latencies of the API server seeing that a webhook replica is shutting down.
Related: https://github.com/knative/pkg/issues/1509
* Switch to RWLock
* 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
* Added common name to cert tmpl
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
* Added a common name test to TestCreateCert function
* Update webhook/certificates/resources/certs_test.go
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Update webhook/certificates/resources/certs_test.go
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Applied changes to improve formatting and style
* Update webhook/certificates/resources/certs_test.go
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* Update webhook/certificates/resources/certs_test.go
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* Added Subject to caParsedCert CommonName check
* Fixed failure message to remove reference to diff
* Update webhook/certificates/resources/certs_test.go
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Update webhook/certificates/resources/certs_test.go
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Update webhook/certificates/resources/certs.go
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* Update webhook/certificates/resources/certs.go
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* Update webhook/certificates/resources/certs.go
Co-authored-by: Matt Moore <mattmoor@vmware.com>
Co-authored-by: Victor Agababov <vagababov@gmail.com>
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* Check example-hash label for consistency in ConfigMap webhook.
We've seen users try to edit our configuration and falling into the trap of editing the '_example' block a lot. This attempts at guiding the users to do "the right thing" by checking the ConfigMap's '_example' value (if present) against a precomputed hash of the same value (if present). The idea is that we precompute this has using the tool herein in code generation and thus allow us to easily change the example block automatically while making it hard to change it on ppurpose. If the hashes don't match on an upgrade, the webhook will return an error synchronously, guiding the user to the correct behavior.
* Add tests.
* Add hash check to test utilities.
* Add a bit of coverage.
* Rewrite into a table test.
* Rename.
* Reduce test surface.
* Godoc.
* Code nits.
* Docs.
* Use CRC32.
* Nits.
* Kubelet probes would result in the webhook writing the HTTP status twice
Doesn't seem like it affected anything - just writes out some extra
log messages
* nits
* nits
* nits
* nits
* 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
* Add helper function to read port number from env var
* Add check for port == 0
* Add the unit test
* Update error message
Co-Authored-By: savitaashture <sashture@redhat.com>
* Panic when env var is set but invalid
* Rename the func to be PortFromEnv.
* Apply suggestions from code review on error message
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Join defers
Co-authored-by: savitaashture <sashture@redhat.com>
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Implement the K8s lifecycle in webhook.
The webhook never properly implemented the Kubernetes SIGTERM/SIGKILL
lifecycle, and doesn't even really support readiness probes today. This
change enables folks to use a block like this on their webhook container:
```yaml
readinessProbe: &probe
periodSeconds: 1
httpGet:
scheme: HTTPS
port: 8443
httpHeaders:
- name: k-kubelet-probe
value: "webhook"
livenessProbe: *probe
```
With this, the webhook won't report as `Ready` until a probe has succeeded,
and when the SIGTERM is received, we will start failing probes for a grace
period (so our Endpoint drops) before shutting down the webhook's HTTP Server.
This was uncovered by running the webhook across 10 replicas in Serving with
the "Goose" (https://github.com/knative/pkg/pull/1316) enabled for the e2e
tests. The failure mode I saw was conversion webhook requests failing across
random tests.
This also moves the Serving probe-detection function into PKG.
* Increase the log level when we start to fail probes
* Wait for go routines to terminate on all paths.
* do not record for empty metric config
* Revert "do not record for empty metric config"
This reverts commit 539a5e4dbb.
* add a comment
* fix typo
* fix tests
* revert
* revert tests
* revert
* fix conflicts
* one more test file
* Remove unused code.
* Use raw strings to avoid escaping.
* Remove unneeded type conversions.
* Preallocate slices where possible.
* Use semantic equality in psbinding reconciler.
* First hack at getting the namespaces of interest to the ps
* Have the webhook label the namespace for inclusion
* Fix unit tests
* Update with feedback
* Add namespace lister, use constants
* Add one more err logging statement
* Create IsInDelete context
* Set up context with WithinDelete
* Test for callback delete
* fix subresource update
* Return oldObj for deletes
* include delete in webhook config
* include delete in unit test
* don't log on delete
* Add new callback pattern to pkg
* include the context
* typo
* Remove the empty instance of unstructured
* initialize the unstructured var
* Eliminate the unneeded pointer
* Pass a pointer to unstructured callback
* Create a validation specific context struct
* Move callback tests to own unit test case
* Switch from converting to decoding
* Update webhook/resourcesemantics/validation/validation.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* don't wrap context and include params
* split validation files
* include 2020 copyright
* include unit test for WithKubeClient
* Don't bother updating copyright date
* Inclue a unit test for panic
* Move dryRun to context
* Include context dry run unit test
* put the request operation in the context
* eliminate circular dep
* move kubeclient test out of context_test
* dont bother iterating callback map
* Callback takes a list of supported verbs
* Remove extra type
* Ensure Callback interface is public
* Alias Operation into validation
* alias Operation right in Webhook
* Update webhook/resourcesemantics/validation/validation_admit.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit_test.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit_test.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit_test.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* correct parens
* minor style fixes
* Rename Callback to Func
* Fix build error
* Switch callback to take a list with a factory
* keep descriptive names
* update comment
* Drop pointer, correct comments
* Add a unit test to disallow duplicate verbs
* fix comments, struct{} for set
* switch to variadic arg for NewCallback
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Start the webhook before informers sync.
Some webhooks (e.g. conversion) are required to list resources, so by delaying those until after informers have synced, we create a deadlock when they run in the same process. This change has two key parts:
1. Start the webhook immediately when our process starts, and issue a callback from sharedmain when the informers have synced.
2. Block `Admit` calls until informers have synced (all conversions are exempt), unless they have been designated by implementing `webhook.StatelessAdmissionController`.
Our built-in admission controllers (defaulting, validation, configmap validation) have all been marked as stateless, the main case where we want to block `Admit` calls is when we require the informer to have synchronized to populate indices for Bindings.
* Add missing err declaration
In general, imo, it's prettier to return things as is, rather than the pattern that was
there.
Also handle one more error case, which was kind of ignored before (probably in practice
impossible, but idk)
* Renews a webhook cert before it expires
* Moved checks out of webhook and into certificate
* Updated error messages and others from review
* Updated error messages and changed time comparison
* Added two tests cases for checking expiration
* Fixed issue with missing "." in webhook.go
* ConversionController implementation
This controller will reconcile target CRDs with the correct
conversion webhook configuration. Specifically, the HTTP path and
CA bundle will be updated.
Additionally, the conversion controller will perform the given
conversions through a hub and spoke model utilizing the
apis.Convertible interface.
* Webhook now can host ConversionControllers
* injection/sharedmain now supports webhook.ConversionControllers
These conversion controllers will be hosted by the webhook that
the sharedmain will start
* support defaulting & include godoc
Victor Agababov
Scott Nichols
Matt Moore
Dave Protasowski
Weston Haught
Mike Petersen
Markus Thömmes
jpower432
Julian Friedman
Jihui Nie
Yanwei Guo
Shashwathi
Lukas Berk
whaught
Cong Liu
Mike Petersen
cshou
Timur Zununbekov