apf: exempt probes /healthz /livez /readyz

[tkashem]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Add a FlowSchema that exempts the following probes from any user:

• /readyz
• /livez
• /healthz

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fixes a regression in 1.20+ default configurations in the priority and fairness API server filter by exempting all probes (/readyz, /healthz, /livez) to prevent restarting of "healthy" kube-apiserver instance(s) by kubelet.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[tkashem]

/assign @deads2k @MikeSpreitzer

[lavalamp]

Is this the right thing to do? Isn't it common for unathenticated users to have access to these endpoints?

[fejta-bot]

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

[tkashem]

Is this the right thing to do? Isn't it common for unauthenticated users to have access to these endpoints?

kubelet liveness probe gets a 429 response (the priority level is saturated due to load or degradation) and this may cause kubelet to restart the apiserver instance.

Is your concern that a bad actor can drown the apiserver with probe requests and bring it down? I think the health checks in the apiserver are designed to be asynchronous and these are very cheap. But yeah, making it exempt does leave a window open. An alternative is to use a dedicated non-exempt priority level (maybe with a small concurrency share)

[MikeSpreitzer]

@kubernetes/sig-api-machinery-bugs

[MikeSpreitzer]

/triage accepted

[MikeSpreitzer]

/priority important-soon

[MikeSpreitzer]

@lavalamp : we discussed it in the meeting on Monday. Our thinking was that we do not like making this available to bad actors, but having kubelets kill loaded apiservers is worse.

@tkashem : what you suggested sounds interesting. Have you tried it?

[MikeSpreitzer]

The choice of flow distinguisher method is not obvious. Since these requests are exempt and thus not queued, there is no wrong answer from a functional point of view. Why not go with the simplest choice, the one that makes the flow distinguisher always be the empty string? Specifying something more specific suggests there is a reason, which would be misleading to readers if there is not a reason. Is there thinking that this will produce better metrics or debug output or something? If there is a positive reason, it would be best to comment it, since it is not obvious.

[lavalamp]

I guess the right thing to do is to figure out what user kubelet identifies as and make only kubelet exempt.

I have conflicting feelings about that, however, like imagine we get into a situation where all the queues go into a deadlock, but exempt requests are getting handled fine. Then the proper thing would be for kubelet to restart apiserver, but it will appear as if apiserver is perfectly happy. I guess maybe instead of relying on the health check itself going through the system, we should make them exempt and somehow continuously test the queues ourselves, and report a health failure if they are stuck for a long time?

E.g., we could periodically insert sentry requests in each PL and verify that they actually eventually get handled or canceled.

Anyway I agree restarting healthy apiservers is quite bad and we should prevent that. But failing to restart unhealthy apiservers is also bad!

[tkashem]

I guess the right thing to do is to figure out what user kubelet identifies as and make only kubelet exempt.

So I grabbed the kubelet probes for /healthz from audit.

{
  "kind": "Event",
  "apiVersion": "audit.k8s.io/v1",
  "level": "RequestResponse",
  "auditID": "af13ff6f-fcd7-444d-8224-0429d8a52b43",
  "stage": "RequestReceived",
  "requestURI": "/healthz",
  "verb": "get",
  "user": {
    "username": "system:anonymous",
    "groups": [
      "system:unauthenticated"
    ]
  },
  "sourceIPs": [
    "10.0.132.119"
  ],
  "userAgent": "kube-probe/1.20",
  "requestReceivedTimestamp": "2021-03-16T20:17:36.069405Z",
  "stageTimestamp": "2021-03-16T20:17:36.069405Z"
}

It appears as system:anonymous to kube-apiserver, there is no information in the user to identify whether it's coming from kubelet. The user agent is kube-probe/{version}. So it appears we can't exempt kubelet probes exclusively.

The audit is taken from an OpenShift cluster, and I assume it will be the same for other clusters as well.
Thoughts?

[lavalamp]

Long term, we need to make sure kubelet has its own account.

We probably can't block on that.

People can always change this back on individual clusters if it gets abused, I guess.

[lavalamp]

/approve

(with the understanding that @tkashem is going to start a conversation with SIG Auth about them getting kubelet its own identity somehow)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavalamp, MikeSpreitzer, tkashem

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• staging/src/k8s.io/apiserver/pkg/apis/OWNERS [lavalamp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

[tkashem]

Long term, we need to make sure kubelet has its own account.

opened an issue #100844 to have a discussion on this

[wojtek-t]

@lavalamp @deads2k - should we consider cherrypicking back to 1.21 and 1.20 (where APF is Beta already)?

[deads2k]

@lavalamp @deads2k - should we consider cherrypicking back to 1.21 and 1.20 (where APF is Beta already)?

Yes. I'm willing to go back to all supported versions.