api: validate labels in store-limit requests - #11235
Conversation
Reject malformed label objects and non-string values before updating store limits, returning HTTP 400 instead of panicking. Cover rejection without configuration changes and valid label filtering in API integration tests. Signed-off-by: JmPotato <github@ipotato.me>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe store-limit API now returns HTTP 400 for malformed label selectors instead of panicking. Tests cover invalid selector shapes, unchanged configuration after rejection, no-op selectors, and matching selectors across supported limit types. ChangesStore limit label handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Malformed label requests now return HTTP 400 without changing store-limit configuration, and valid label-filtered updates retain their expected behavior. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai[bot]: adding LGTM is restricted to approvers and reviewers in OWNERS files. 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11235 +/- ##
==========================================
+ Coverage 79.79% 79.80% +0.01%
==========================================
Files 544 545 +1
Lines 78879 78898 +19
==========================================
+ Hits 62942 62967 +25
- Misses 11570 11572 +2
+ Partials 4367 4359 -8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@YuhaoZhang00: adding LGTM is restricted to approvers and reviewers in OWNERS files. 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bufferflies, coderabbitai[bot], lhy1024, YuhaoZhang00 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
|
/retest |
1 similar comment
|
/retest |
|
In response to a cherrypick label: new pull request created to branch |
|
In response to a cherrypick label: new pull request created to branch |
What problem does this PR solve?
Issue Number: Close #11234
Malformed
labelsinPOST /pd/api/v1/stores/limit, such as{"rate":25,"type":"add-peer","labels":null}, trigger unchecked type assertions and return HTTP 500 with a panic stack.What is changed and how does it work?
Check List
Tests
make basic-test BASIC_TEST_PKGS=github.com/tikv/pd/server/apimake checkCode changes
Related changes
Release note
Summary by CodeRabbit
Bug Fixes
400 Bad Requestresponse instead of causing an error.Tests