✨ WIP KEP-6060: Webhook Auth integration - #3554
Open
benjaminapetersen wants to merge 6 commits into
Open
Conversation
Implements the controller-runtime portion of KEP-6060 (API Server Authentication to Admission Webhooks): an opt-in, webhook-side library that verifies the bound, APIGroup-scoped service-account JWT presented by kube-apiserver / aggregated API servers to admission webhooks. Phase 1 (admission integration seam): - Add admission.Authenticator interface and optional Webhook.Authenticator field, invoked in ServeHTTP after AdmissionReview decode and before the user handler. Nil authenticator preserves existing behavior exactly. Auth failures short-circuit and fail closed (401 unauthenticated / 403 unauthorized). Phase 2 (offline JWT verifier): - Add pkg/webhook/admission/webhookauth: offline verification via go-jose/v4 with OIDC discovery + JWKS caching and fail-closed refresh, an explicit RS256 algorithm allow-list (rejects "none"/HS* per RFC 8725, audience-bound per RFC 9700), and issuer/exp/nbf/iat, binding, and allowedAPIGroup claim checks. No TokenReview round-trips (KEP mandate). - Typed unauthenticated/unauthorized errors with no token material leakage. - Adapter (NewAuthenticator) maps the Verifier onto admission.Authenticator. Backward compatible: webhooks that do not adopt verification are unaffected and silently ignore the Authorization header. NOTE: upstream KEP-6060 wire format is not yet settled (k/k#140113 is WIP); KEP-specific claim keys/field names are isolated as provisional constants pending upstream api-review reconciliation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the controller-runtime-local JWT verifier with a thin adapter over the k8s.io/webhook-auth library, so controller-runtime demonstrates a simple opt-in to KEP-6060 API server authentication rather than duplicating verification logic. pkg/webhook/admission/webhookauth is now a ~3-file adapter: - NewAuthenticator(v *verify.Verifier) wraps the library verifier onto the admission.Authenticator seam, reusing controller-runtime's single AdmissionReview decode (admissionhttp.VerifyAdmissionReview) and the library's BearerToken helper. It fails closed and logs only verify.Reason(err). - Deleted the local verifier / RemoteKeySet / OIDC re-implementation and its typed errors, dropping the direct github.com/go-jose/go-jose/v4 dependency. The constructor intentionally takes a caller-built *verify.Verifier so no JOSE/OIDC dependency enters controller-runtime. The library's zero-config oidckeyset.NewInClusterVerifier is documented as a future optional helper (pending SIG-Auth review) rather than wired here. The Phase 1 admission.Authenticator seam (Webhook.Authenticator, invoked after decode and before the handler) is unchanged; nil preserves existing behavior. NOTE: depends on k8s.io/webhook-auth via a local replace; the module is not yet published, so this branch is a preview pending upstream publication. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
…gthen tests The k8s.io/webhook-auth library moved to a single-arg verify.NewVerifier(TokenAuthenticator) seam (iss/aud/exp now live in the authenticator; oidckeyset renamed to oidc). Update the adapter doc comments to the v2 surface (comments only, no behavior change) and rewrite the adapter tests to inject a fake verify.TokenAuthenticator instead of the removed v1 KeySet. Tests: add mutating-bound and empty-bearer adapter cases; lock the core seam's denied-response default to 403 (was an uncovered fail-open branch). All pkg/webhook/admission tests green.
…webhookauth v3 Replace the pkg/webhook/admission/webhookauth/ subpackage adapter with a native adapter in pkg/webhook/admission (verifierAuthenticator / NewAuthenticator + fluent WithInClusterAuthenticator / WithRemoteAuthenticator). Migrate to the renamed no-dash k8s.io/webhookauth v3 API: - Verify is now error-only - admissionhttp.VerifyAdmissionRequest for admission-request verification - incluster.InCluster for the in-cluster authenticator The interim local `replace k8s.io/webhookauth => <local staging>` plus the v0.0.0 pseudo-version is deliberate and load-bearing: the module is still unpublished, so the local staging checkout is required to build. Comprehensive authenticator_ext_test.go binding-violation coverage (bothBound / noBound / mutatingApps) is deferred to a follow-up commit; TODO markers are left in-file. Signed-off-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
…ticator Restores the authenticator test coverage deferred from b027da3 (commit 1 left MINIMAL-compile TODO markers). Exercises controller-runtime's OWN adapter fail-closed contract: verify-error deny (which at the CR layer subsumes the old bothBound/noBound/mutatingApps binding violations), missing/malformed/empty/ wrong-scheme bearer token, nil verifier, and the 401 + HTTP-200 deny shape through the ServeHTTP seam so a failurePolicy:Ignore webhook cannot flip a deny into an admit. Also asserts no token material leaks into the deny response body. The v3 exactly-one-of-(validating|mutating) webhook-binding checks moved INTO k8s.io/webhookauth's authenticator; controller-runtime's offline verifier fake cannot drive that internal binding logic, so those checks are covered by the library's own test suite (documented via an in-file NOTE rather than a hollow CR test). Signed-off-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
Surface the library verifier's HealthCheck() through Webhook.HealthCheck, a healthz.Checker-compatible method (raw func(*http.Request) error, so the admission package takes no healthz dependency). It is directly assignable to mgr.AddReadyzCheck and safe to register unconditionally: when no verifier-backed authenticator is configured it always reports ready. When an in-cluster/remote authenticator is configured, readiness delegates to the library verifier and reports not-ready until the audience is bound. This prevents a pod that can never derive its audience from silently denying all traffic — it fails readiness and is restarted instead. Addresses McManus OBS-1 / cr-4 7.1. Signed-off-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
|
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: benjaminapetersen The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
|
PR needs rebase. DetailsInstructions 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. |
kubernetes-prow
Bot
added
needs-rebase
Contributor
|
@benjaminapetersen: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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. I understand the commands that are listed here. |
kubernetes-prow
Bot
added
the
size/XXL
Author
|
/hold |
kubernetes-prow
Bot
added
the
do-not-merge/hold
Author
|
/sig auth |
kubernetes-prow
Bot
added
the
sig/auth
Contributor
DetailsIn response to this:
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. |
kubernetes-prow
Bot
added
the
kind/feature
Author
|
/wip |
20 tasks
Member
|
/retitle ✨ WIP KEP-6060: Webhook Auth integration Also, please disclose AI when submitting a pull-request |
kubernetes-prow
Bot
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.