mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
Defer guest commands after incomplete HTTP error responses
The HTTP error wrapper hid truncated response bodies from guest admission. Preserve the bounded read failure as a private wrapping type so error statuses also cool down the whole guest. Cover both 200 and 500 truncation, and retain complete permission refusals plus subsequent healthy reads. Contract-Neutral: Completes the existing full-response/incomplete-body guest safety contract; error text and ordinary API errors retain compatibility, with no new interface or surface. Change-source: pulse-maintainer
This commit is contained in:
@@ -52,13 +52,20 @@ func classifyAPIErrorLog(status int, path string) (apiErrorLogLevel, string) {
|
||||
return apiErrorLogNone, ""
|
||||
}
|
||||
|
||||
// Preserve an incomplete body through the HTTP error-response wrapper too.
|
||||
// An error status alone is not a completed refusal if its body was truncated.
|
||||
type responseBodyReadError struct{ cause error }
|
||||
|
||||
func (e *responseBodyReadError) Error() string { return e.cause.Error() }
|
||||
func (e *responseBodyReadError) Unwrap() error { return e.cause }
|
||||
|
||||
func readResponseBodyLimited(r io.Reader) ([]byte, error) {
|
||||
body, err := io.ReadAll(io.LimitReader(r, maxResponseBodyBytes+1))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, &responseBodyReadError{cause: err}
|
||||
}
|
||||
if int64(len(body)) > maxResponseBodyBytes {
|
||||
return nil, fmt.Errorf("response body exceeds %d bytes", maxResponseBodyBytes)
|
||||
return nil, &responseBodyReadError{cause: fmt.Errorf("response body exceeds %d bytes", maxResponseBodyBytes)}
|
||||
}
|
||||
return body, nil
|
||||
}
|
||||
|
||||
@@ -176,6 +176,11 @@ func (c *Client) getGuestAgent(ctx context.Context, path, node string, vmid int)
|
||||
// own their usual session recovery; this guest admission is single-attempt.
|
||||
resp, err := c.requestWithRetry(ctx, http.MethodGet, path, nil, true)
|
||||
if err != nil {
|
||||
var incomplete *responseBodyReadError
|
||||
if errors.As(err, &incomplete) {
|
||||
uncertain = true
|
||||
return nil, &guestAgentDeferredError{reason: "agent-response-incomplete", cause: err}
|
||||
}
|
||||
// Explicit refusals/unsupported commands remain ordinary errors. An
|
||||
// uncertain completion blocks every command, not just this method.
|
||||
lower := strings.ToLower(err.Error())
|
||||
|
||||
@@ -101,6 +101,14 @@ func TestGuestAgentLockAppearingInFlightDiscardsPayload(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestGuestAgentIncompleteBodyBlocksWholeVM(t *testing.T) {
|
||||
for _, status := range []int{http.StatusOK, http.StatusInternalServerError} {
|
||||
t.Run(fmt.Sprintf("status-%d", status), func(t *testing.T) {
|
||||
testGuestAgentIncompleteBodyBlocksWholeVM(t, status)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func testGuestAgentIncompleteBodyBlocksWholeVM(t *testing.T, status int) {
|
||||
var calls atomic.Int32
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if strings.HasSuffix(r.URL.Path, "/config") {
|
||||
@@ -109,12 +117,13 @@ func TestGuestAgentIncompleteBodyBlocksWholeVM(t *testing.T) {
|
||||
}
|
||||
calls.Add(1)
|
||||
w.Header().Set("Content-Length", "1000")
|
||||
w.WriteHeader(status)
|
||||
fmt.Fprint(w, `{"data":{"result":[]}}`)
|
||||
}))
|
||||
defer server.Close()
|
||||
c := backupTestClient(t, server.URL)
|
||||
if _, err := c.GetVMFSInfo(context.Background(), "node", 105); GuestAgentDeferredReason(err) != "agent-response-incomplete" {
|
||||
t.Fatalf("incomplete body not deferred: %v", err)
|
||||
t.Errorf("incomplete body not deferred: %v", err)
|
||||
}
|
||||
for name, read := range backupAgentReads() {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
@@ -128,6 +137,33 @@ func TestGuestAgentIncompleteBodyBlocksWholeVM(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestGuestAgentDeferralDoesNotHideCompletedRefusal(t *testing.T) {
|
||||
var calls atomic.Int32
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if strings.HasSuffix(r.URL.Path, "/config") {
|
||||
fmt.Fprint(w, `{"data":{}}`)
|
||||
return
|
||||
}
|
||||
calls.Add(1)
|
||||
if strings.HasSuffix(r.URL.Path, "/get-fsinfo") {
|
||||
http.Error(w, "permission denied", http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
backupAgentPayload(w, r)
|
||||
}))
|
||||
defer server.Close()
|
||||
c := backupTestClient(t, server.URL)
|
||||
if _, err := c.GetVMFSInfo(context.Background(), "node", 105); err == nil || errors.Is(err, ErrGuestAgentDeferred) || !strings.Contains(err.Error(), "403") {
|
||||
t.Fatalf("completed refusal hidden or lost: %v", err)
|
||||
}
|
||||
if _, err := c.GetVMAgentInfo(context.Background(), "node", 105); err != nil {
|
||||
t.Fatalf("completed refusal poisoned other healthy reads: %v", err)
|
||||
}
|
||||
if calls.Load() != 2 {
|
||||
t.Fatalf("refused/healthy command count = %d", calls.Load())
|
||||
}
|
||||
}
|
||||
|
||||
func TestGuestAgentClusterDoesNotReplayUncertainCall(t *testing.T) {
|
||||
var calls atomic.Int32
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
Reference in New Issue
Block a user