F20-BUG1: surface swallowed format error in agentapi.FormatDisk
A failed agent format (e.g. 502 'device is mounted', ok:false, data:null) fell through FormatDisk's trailing 'return out, nil', so the web layer reported a zero-value FormatResult as ok:true — a failed DESTRUCTIVE format read as success. postWithStatus now returns the full envelope; FormatDisk returns a non-nil error on any non-2xx/ok:false that is not a recognized refusal (403/needs-confirmation). Test TestFormat_MountedFailureSurfacesError (502 → non-nil err) fails on old code.
This commit is contained in:
@@ -373,15 +373,15 @@ func (c *Client) FormatDisk(ctx context.Context, device, fstype string, confirme
|
|||||||
var out FormatResult
|
var out FormatResult
|
||||||
// Status-aware POST: the agent returns the FULL FormatResponse (incl. pending_op / durable_id)
|
// Status-aware POST: the agent returns the FULL FormatResponse (incl. pending_op / durable_id)
|
||||||
// even on the 403 refusal, so we must read the body on non-2xx rather than discarding it.
|
// even on the 403 refusal, so we must read the body on non-2xx rather than discarding it.
|
||||||
data, status, err := c.postWithStatus(ctx, "/disks/format", map[string]any{
|
env, status, err := c.postWithStatus(ctx, "/disks/format", map[string]any{
|
||||||
"device": device, "fstype": fstype, "confirmed": confirmed, "durable_id": durableID,
|
"device": device, "fstype": fstype, "confirmed": confirmed, "durable_id": durableID,
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return out, err
|
return out, err
|
||||||
}
|
}
|
||||||
// data is the envelope's {data:…} payload (present on both success and the 403 refusal).
|
// env.Data is the envelope's {data:…} payload (present on both success and the 403 refusal).
|
||||||
if len(data) > 0 {
|
if len(env.Data) > 0 {
|
||||||
_ = json.Unmarshal(data, &out) // best-effort; fields default on a missing/partial body
|
_ = json.Unmarshal(env.Data, &out) // best-effort; fields default on a missing/partial body
|
||||||
}
|
}
|
||||||
if out.Formatted {
|
if out.Formatted {
|
||||||
return out, nil
|
return out, nil
|
||||||
@@ -394,34 +394,45 @@ func (c *Client) FormatDisk(ctx context.Context, device, fstype string, confirme
|
|||||||
out.DataBearing = true
|
out.DataBearing = true
|
||||||
return out, ErrFormatRefused // system/backup: surface the opsign command
|
return out, ErrFormatRefused // system/backup: surface the opsign command
|
||||||
}
|
}
|
||||||
|
// F20-BUG1: a non-2xx response (or ok:false) that is NOT one of the recognized refusals above is a
|
||||||
|
// real failure (e.g. the agent's 502 on a mkfs error: "device is mounted"). Returning the zero-value
|
||||||
|
// result with a nil error here made a failed destructive format read as a silent SUCCESS in the web
|
||||||
|
// layer. Surface it as an error so the caller (and the dashboard) report the failure.
|
||||||
|
if status < 200 || status >= 300 || !env.OK {
|
||||||
|
msg := strings.TrimSpace(env.Error)
|
||||||
|
if msg == "" {
|
||||||
|
msg = "format failed"
|
||||||
|
}
|
||||||
|
return out, fmt.Errorf("agentapi: format: HTTP %d: %s", status, msg)
|
||||||
|
}
|
||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// postWithStatus issues an authenticated JSON POST and returns the envelope's data payload + the HTTP
|
// postWithStatus issues an authenticated JSON POST and returns the envelope's data payload + the HTTP
|
||||||
// status, even on a non-2xx (so callers like FormatDisk can read a 403 refusal body). A transport or
|
// status, even on a non-2xx (so callers like FormatDisk can read a 403 refusal body). A transport or
|
||||||
// envelope-parse failure is still an error; an `ok:false` business refusal is NOT (the data carries it).
|
// envelope-parse failure is still an error; an `ok:false` business refusal is NOT (the data carries it).
|
||||||
func (c *Client) postWithStatus(ctx context.Context, path string, body any) (json.RawMessage, int, error) {
|
func (c *Client) postWithStatus(ctx context.Context, path string, body any) (apiResponse, int, error) {
|
||||||
|
var env apiResponse
|
||||||
buf, err := json.Marshal(body)
|
buf, err := json.Marshal(body)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, 0, err
|
return env, 0, err
|
||||||
}
|
}
|
||||||
req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+path, bytes.NewReader(buf))
|
req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+path, bytes.NewReader(buf))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, 0, err
|
return env, 0, err
|
||||||
}
|
}
|
||||||
req.Header.Set("Authorization", "Bearer "+c.token)
|
req.Header.Set("Authorization", "Bearer "+c.token)
|
||||||
req.Header.Set("Content-Type", "application/json")
|
req.Header.Set("Content-Type", "application/json")
|
||||||
resp, err := c.hc.Do(req)
|
resp, err := c.hc.Do(req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, 0, fmt.Errorf("agentapi: POST %s: %w", path, err)
|
return env, 0, fmt.Errorf("agentapi: POST %s: %w", path, err)
|
||||||
}
|
}
|
||||||
defer resp.Body.Close()
|
defer resp.Body.Close()
|
||||||
raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20))
|
raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20))
|
||||||
var env apiResponse
|
|
||||||
if err := json.Unmarshal(raw, &env); err != nil {
|
if err := json.Unmarshal(raw, &env); err != nil {
|
||||||
return nil, resp.StatusCode, fmt.Errorf("agentapi: POST %s: HTTP %d, bad envelope: %w", path, resp.StatusCode, err)
|
return env, resp.StatusCode, fmt.Errorf("agentapi: POST %s: HTTP %d, bad envelope: %w", path, resp.StatusCode, err)
|
||||||
}
|
}
|
||||||
return env.Data, resp.StatusCode, nil
|
return env, resp.StatusCode, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---- slice 9: host metrics (the customer host-health view) -------------------------------
|
// ---- slice 9: host metrics (the customer host-health view) -------------------------------
|
||||||
|
|||||||
@@ -34,6 +34,9 @@ func diskStub(t *testing.T) (*httptest.Server, string) {
|
|||||||
case strings.Contains(body.Device, "data") && !body.Confirmed: // user-data, not yet confirmed
|
case strings.Contains(body.Device, "data") && !body.Confirmed: // user-data, not yet confirmed
|
||||||
w.WriteHeader(http.StatusForbidden)
|
w.WriteHeader(http.StatusForbidden)
|
||||||
_, _ = w.Write([]byte(`{"ok":false,"data":{"device":"` + body.Device + `","data_bearing":true,"role":"user-data","needs_confirmation":true,"durable_id":"byid:wwn-1"}}`))
|
_, _ = w.Write([]byte(`{"ok":false,"data":{"device":"` + body.Device + `","data_bearing":true,"role":"user-data","needs_confirmation":true,"durable_id":"byid:wwn-1"}}`))
|
||||||
|
case strings.Contains(body.Device, "mounted"): // mkfs failed (e.g. device mounted) → agent 502, data:null
|
||||||
|
w.WriteHeader(http.StatusBadGateway)
|
||||||
|
_, _ = w.Write([]byte(`{"ok":false,"error":"format failed: /dev/sdb1 is mounted; will not make a filesystem here!","data":null}`))
|
||||||
default: // blank, or user-data confirmed
|
default: // blank, or user-data confirmed
|
||||||
_, _ = w.Write([]byte(`{"ok":true,"data":{"device":"` + body.Device + `","formatted":true,"role":"user-data"}}`))
|
_, _ = w.Write([]byte(`{"ok":true,"data":{"device":"` + body.Device + `","formatted":true,"role":"user-data"}}`))
|
||||||
}
|
}
|
||||||
@@ -117,6 +120,28 @@ func TestFormat_UserDataConfirmed(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// F20-BUG1: a real mkfs failure (agent 502, ok:false, data:null) must surface as a non-nil error —
|
||||||
|
// NOT a zero-value FormatResult with nil err (which read as a silent SUCCESS in the web layer).
|
||||||
|
func TestFormat_MountedFailureSurfacesError(t *testing.T) {
|
||||||
|
s, ep := diskStub(t)
|
||||||
|
defer s.Close()
|
||||||
|
c := clientFor(t, s, ep)
|
||||||
|
res, err := c.FormatDisk(context.Background(), "/dev/sdb1-mounted", "ext4", true, "byid:wwn-1")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected a non-nil error for a failed format, got (res=%+v, err=nil) — silent success regression", res)
|
||||||
|
}
|
||||||
|
if res.Formatted {
|
||||||
|
t.Fatalf("Formatted must be false on a failed format: %+v", res)
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "502") || !strings.Contains(err.Error(), "mounted") {
|
||||||
|
t.Fatalf("error should carry the HTTP status + agent message, got: %v", err)
|
||||||
|
}
|
||||||
|
// Must not be misclassified as one of the gated refusals.
|
||||||
|
if errors.Is(err, ErrNeedsConfirmation) || errors.Is(err, ErrFormatRefused) {
|
||||||
|
t.Fatalf("a 502 mkfs failure must not be reported as a refusal: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestEject_Dependents(t *testing.T) {
|
func TestEject_Dependents(t *testing.T) {
|
||||||
s, ep := diskStub(t)
|
s, ep := diskStub(t)
|
||||||
defer s.Close()
|
defer s.Close()
|
||||||
|
|||||||
Reference in New Issue
Block a user