New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add podresources DOS prevention using rate limit #116459
add podresources DOS prevention using rate limit #116459
Conversation
To enable rate limiting, needed for GA graduation, we need to pass more parameters to the already crowded `ListenAndServePodresources` function. To tidy up a bit, pack the parameters in a helper struct, with no intended changes in behavior. Signed-off-by: Francesco Romani <fromani@redhat.com>
/sig node |
/priority important-soon copied from #115852 (comment) - same reasoning applies here |
/release-note-edit
|
e2e test re-validated locally:
|
9da9acb
to
e6a2149
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a small note about the field documentation. Otherwise lgtm
/triage accepted |
Implement DOS prevention wiring a global rate limit for podresources API. The goal here is not to introduce a general ratelimiting solution for the kubelet (we need more research and discussion to get there), but rather to prevent misuse of the API. Known limitations: - the rate limits value (QPS, BurstTokens) are hardcoded to "high enough" values. Enabling user-configuration would require more discussion and sweeping changes to the other kubelet endpoints, so it is postponed for now. - the rate limiting is global. Malicious clients can starve other clients consuming the QPS quota. Add e2e test to exercise the flow, because the wiring itself is mostly boilerplate and API adaptation.
e6a2149
to
b837a0c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
LGTM label has been added. Git tree hash: 4b10493450b97c1930f3648a6e54d2a0b13c1ad1
|
rate limiting looks good. /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: derekwaynecarr, ffromani, SergeyKanzhelev 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:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Implement server-side Denial Of Service prevention using rate limit for podresources API endpoint (which is served locally on the node through unix domain socket). This is a GA graduation blocker: kubernetes/enhancements#3863
Which issue(s) this PR fixes:
related (but not sufficient to close) to k/e 3743
Special notes for your reviewer:
Alternative take to #115852
Implements suggestion made during the review: #115852 (comment)
Acknowledges some drawbacks emerged during the API review (most notably: #115852 (comment) - please see the review of #115852 for all the details)
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: