Skip to content

webhook: Admission metrics are not reported by status code #2972

Open

Description

@ahmetb

This is several bugs in one bug since it appears multiple things are broken. Apologies if it gets too complicated.

Problem 1: It appears that controller-runtime webhooks always respond with HTTP status code 200 OK even if the request is completely malformed.

Example (sending a malformed request to a webhook endpoint):

curl -k -v https://localhost:9443/validate-ship-example-org-v1beta1-frigate
< HTTP/2 200
< ... other response headers ...
<
{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"contentType=, expected application/json","code":400}}}

Problem 2: controller-runtime webhooks do not use the metav1.Status.Code and always respond with HTTP status code 200 OK.

Example code I used to register my custom handler at /test:

webhookServer.Register("/test", &webhook.Admission{Handler: &myHandler{}})
...

type myHandler struct{}
func (*myHandler) Handle(context.Context, admission.Request) admission.Response {
	return admission.Response{
		AdmissionResponse: admissionv1.AdmissionResponse{
			Allowed: false,
			Result: &metav1.Status{
				Code:    403,
				Message: "things suck",
				Status:  "Failure",
				Reason:  metav1.StatusReasonForbidden}}}
}

And here's the query I am making:

curl -XPOST -k -v -H "Content-Type: application/json" https://localhost:9443/test

< HTTP/2 200
...
<
{"kind":"AdmissionReview","apiVersion":"admission.k8s.io/v1","response":{"uid":"","allowed":false,"status":{"metadata":{},"status":"Failure","message":"things suck","reason":"Forbidden","code":403}}}

Problem 3: controller-runtime doesn't expose any metrics about rejected requests and their .status.codes. This is likely because

  1. we always report code=200 no matter the response (Problem 1 & 2)
  2. it appears we're capping controller_runtime_webhook_requests_total{code=...} dimension to only 200 and 500 here:
    lat := RequestLatency.MustCurryWith(lbl)
    cnt := RequestTotal.MustCurryWith(lbl)
    gge := RequestInFlight.With(lbl)
    // Initialize the most likely HTTP status codes.
    cnt.WithLabelValues("200")
    cnt.WithLabelValues("500")

so even if I make a faulty request, only the code=200 metric goes up:

controller_runtime_webhook_requests_total{code="200",webhook="/test"} 4

In an ideal state, I'd like to see controller_runtime_webhook_requests_total metric actually let me understand things like how many requests I'm denying (non-200 codes, such as 403, 400, 429, ...).

/kind bug

Activity

  1. troy0820 commented on Oct 10, 2024

    @troy0820
    Member

    I believe the 200 response is if the webhook can create the response. There was an issue around this with failure policy and not sure if this helps with your issue #2662.

  2. alvaroaleman commented on Oct 12, 2024

    @alvaroaleman
    Member

    Pretty much what troy said. I do agree that this is confusing and that it would be helpful to have metrics around the status code in the admission response.

  3. ahmetb commented on Oct 15, 2024

    @ahmetb
    MemberAuthor

    Indeed, after creating the issue I've realized HTTP response code must be 200 regardless of the admission response (which contains the status code the apiserver would return in its response body).

    Maybe we could still add a metric that parses the code out from admission.Response? (I understand we can't help if someone actually decides to implement their own http.Handler).

  4. ahmetb commented on Oct 16, 2024

    @ahmetb
    MemberAuthor

    /remove-kind bug
    /kind feature

  5. added
    kind/featureCategorizes issue or PR as related to a new feature.
    and removed
    kind/bugCategorizes issue or PR as related to a bug.
    on Oct 16, 2024
  6. k8s-triage-robot commented on Jan 14, 2025

    @k8s-triage-robot

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

    This bot triages un-triaged issues 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:

    • Mark this issue as fresh with /remove-lifecycle stale
    • Close this issue with /close
    • Offer to help out with Issue Triage

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

    /lifecycle stale

  7. added
    lifecycle/staleDenotes an issue or PR has remained open with no activity and has become stale.
    on Jan 14, 2025
  8. k8s-triage-robot commented on Feb 13, 2025

    @k8s-triage-robot

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

    This bot triages un-triaged issues 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:

    • Mark this issue as fresh with /remove-lifecycle rotten
    • Close this issue with /close
    • Offer to help out with Issue Triage

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

    /lifecycle rotten

  9. added
    lifecycle/rottenDenotes an issue or PR that has aged beyond stale and will be auto-closed.
    and removed
    lifecycle/staleDenotes an issue or PR has remained open with no activity and has become stale.
    on Feb 13, 2025
  10. k8s-triage-robot commented on Mar 15, 2025

    @k8s-triage-robot

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

    This bot triages issues 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 with /reopen
    • Mark this issue as fresh with /remove-lifecycle rotten
    • Offer to help out with Issue Triage

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

    /close not-planned

  11. k8s-ci-robot commented on Mar 15, 2025

    @k8s-ci-robot
    Contributor

    @k8s-triage-robot: Closing this issue, marking it as "Not Planned".

    Details

    In response to this:

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

    This bot triages issues 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 with /reopen
    • Mark this issue as fresh with /remove-lifecycle rotten
    • Offer to help out with Issue Triage

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

    /close not-planned

    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.

  12. sbueringer commented on Mar 22, 2025

    @sbueringer
    Member

    /reopen
    /remove-lifecycle rotten

    Adding a metric seems fine

  13. k8s-ci-robot commented on Mar 22, 2025

    @k8s-ci-robot
    Contributor

    @sbueringer: Reopened this issue.

    Details

    In response to this:

    /reopen
    /remove-lifecycle rotten

    Adding a metric seems fine

    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.

  14. removed
    lifecycle/rottenDenotes an issue or PR that has aged beyond stale and will be auto-closed.
    on Mar 22, 2025
  15. k8s-triage-robot commented on Jun 24, 2025

    @k8s-triage-robot

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

    This bot triages un-triaged issues 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:

    • Mark this issue as fresh with /remove-lifecycle stale
    • Close this issue with /close
    • Offer to help out with Issue Triage

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

    /lifecycle stale

  16. added
    lifecycle/staleDenotes an issue or PR has remained open with no activity and has become stale.
    on Jun 24, 2025
  17. k8s-triage-robot commented on Jul 24, 2025

    @k8s-triage-robot

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

    This bot triages un-triaged issues 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:

    • Mark this issue as fresh with /remove-lifecycle rotten
    • Close this issue with /close
    • Offer to help out with Issue Triage

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

    /lifecycle rotten

  18. added
    lifecycle/rottenDenotes an issue or PR that has aged beyond stale and will be auto-closed.
    and removed
    lifecycle/staleDenotes an issue or PR has remained open with no activity and has become stale.
    on Jul 24, 2025
  19. sbueringer commented on Jul 25, 2025

    @sbueringer
    Member

    /remove-lifecycle rotten
    /lifecycle frozen

  20. added
    lifecycle/frozenIndicates that an issue or PR should not be auto-closed due to staleness.
    and removed
    lifecycle/rottenDenotes an issue or PR that has aged beyond stale and will be auto-closed.
    on Jul 25, 2025
  21. mattsu2020 commented on May 29, 2026

    @mattsu2020
    Contributor

    Hi, I'd like to work on this issue.

    My understanding is that the HTTP response code should remain 200 when the webhook successfully returns an AdmissionReview response, even if the AdmissionResponse itself denies the request.

    So instead of changing the HTTP status behavior, I think the improvement should be around exposing the AdmissionResponse status.code in metrics.

    Would adding a separate metric for AdmissionResponse status codes be acceptable? For example:

    controller_runtime_webhook_admission_responses_total{
      webhook="...",
      allowed="false",
      admission_code="403"
    }
    
  22. mattsu2020 commented on Jun 22, 2026

    @mattsu2020
    Contributor

    /assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

kind/featureCategorizes issue or PR as related to a new feature.lifecycle/frozenIndicates that an issue or PR should not be auto-closed due to staleness.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions