fix: reject negative replica counts in handleScaleHTTP#3840
fix: reject negative replica counts in handleScaleHTTP#3840clubanderson merged 1 commit intomainfrom
Conversation
handleScaleHTTP checked for int32 overflow but allowed negative replica values through. Add validation to return HTTP 400 when replicas < 0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits. 📝 Please follow instructions in the contributing guide to update your commits with the DCO Full details of the Developer Certificate of Origin can be found at developercertificate.org. The list of commits missing DCO signoff:
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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
There was a problem hiding this comment.
Pull request overview
This PR hardens the agent’s /scale HTTP endpoint by ensuring negative replica counts are rejected early (instead of being forwarded to Kubernetes), aligning behavior with issue #3831.
Changes:
- Adds validation to reject
replicas < 0after parsing (covers both POST body and GET query-param flows). - Returns an HTTP 400 with a clear JSON error payload when a negative replica value is provided.
| if replicas < 0 { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| json.NewEncoder(w).Encode(map[string]interface{}{ | ||
| "success": false, | ||
| "error": "replicas must be a non-negative integer", | ||
| }) | ||
| return |
There was a problem hiding this comment.
This new negative-replicas branch returns HTTP 400, but other client/input validation failures in handleScaleHTTP (invalid JSON body, invalid replicas query value, missing cluster/namespace/name) still return HTTP 200 with an error payload. Consider making the status-code behavior consistent across all validation errors (e.g., always StatusBadRequest for malformed input) so clients can reliably use res.ok/status checks.
| if replicas < 0 { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| json.NewEncoder(w).Encode(map[string]interface{}{ | ||
| "success": false, | ||
| "error": "replicas must be a non-negative integer", | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
There are extensive HTTP handler tests in pkg/agent/server_test.go, but none appear to cover /scale. Since this change introduces new request validation behavior (negative replicas -> 400) for both POST and GET paths, add unit tests that assert status code + error payload for replicas < 0 and that replicas=0 remains allowed.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 0 code suggestion(s) and 2 general comment(s). Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
Closes #3831
Summary
handleScaleHTTPchecked for int32 overflow on the GET query-param path but allowed negative replica values through on both POST and GET pathsreplicas < 0Test plan
POST /scalewith{"replicas": -1}and verify 400 responseGET /scale?replicas=-5&cluster=x&namespace=y&name=zand verify 400 responsePOST /scalewith{"replicas": 0}(scale to zero) and verify it still worksPOST /scalewith{"replicas": 3}and verify normal scaling worksCo-Authored-By: Claude Opus 4.6 noreply@anthropic.com