fix(cli): Sync picker hides system servers; keep the two 409 refusals apart
Two defects from the live Sync drill: - The picker listed the system servers (login/lobby), which the backup API can never accept (reserved names, no servers row): the pick died in name validation with a raw "server name is reserved" error. backupPickable now filters them out; the halt picker keeps them on purpose (break-glass retains full power over system servers). - backupErrorFromResponse mapped every 409 to the stopped gate, so the new world-volume refusal would have displayed the wrong reason. The 409 arm now keys on the body's error code; a code-less body still reads as the stopped gate. Live (auditfix38): the picker shows only user servers; a world-less pick shows the API's own "no world volume yet — start it once" text; the not_stopped text is unchanged.
This commit is contained in:
3 files changed
+66
-13
No files matched your search
+33
-9
@@ -86,23 +86,31 @@ func requestBackup(ctx context.Context, hc *http.Client, baseURL, token, name, o
|
|||||||
|
|
||||||
// backupErrorFromResponse turns a non-202 into a human message. The well-known codes get
|
// backupErrorFromResponse turns a non-202 into a human message. The well-known codes get
|
||||||
// an operator-facing explanation; anything else falls back to the API's
|
// an operator-facing explanation; anything else falls back to the API's
|
||||||
// {"error":{message}} body, then the bare status code.
|
// {"error":{code,message}} body, then the bare status code.
|
||||||
func backupErrorFromResponse(resp *http.Response) error {
|
func backupErrorFromResponse(resp *http.Response) error {
|
||||||
switch resp.StatusCode {
|
|
||||||
case http.StatusConflict: // not_stopped
|
|
||||||
return fmt.Errorf("the server must be stopped before its world can be backed up — halt it first")
|
|
||||||
case http.StatusServiceUnavailable: // backup_unavailable
|
|
||||||
return fmt.Errorf("the backup subsystem is not configured on felis-api (FELIS_IMAGE / FELIS_BACKUP_PVC unset)")
|
|
||||||
case http.StatusNotFound:
|
|
||||||
return fmt.Errorf("no such server")
|
|
||||||
}
|
|
||||||
var e struct {
|
var e struct {
|
||||||
Error struct {
|
Error struct {
|
||||||
|
Code string `json:"code"`
|
||||||
Message string `json:"message"`
|
Message string `json:"message"`
|
||||||
} `json:"error"`
|
} `json:"error"`
|
||||||
}
|
}
|
||||||
raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<16))
|
raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<16))
|
||||||
_ = json.Unmarshal(raw, &e)
|
_ = json.Unmarshal(raw, &e)
|
||||||
|
|
||||||
|
switch resp.StatusCode {
|
||||||
|
case http.StatusConflict:
|
||||||
|
// Two refusals share 409: the stopped gate and the missing-world-volume
|
||||||
|
// gate. The body's code distinguishes them; a code-less body reads as the
|
||||||
|
// stopped gate (the only 409 before the volume gate existed), and any other
|
||||||
|
// coded 409 falls through to the API's own operator text.
|
||||||
|
if e.Error.Code == "" || e.Error.Code == "not_stopped" {
|
||||||
|
return fmt.Errorf("the server must be stopped before its world can be backed up — halt it first")
|
||||||
|
}
|
||||||
|
case http.StatusServiceUnavailable: // backup_unavailable
|
||||||
|
return fmt.Errorf("the backup subsystem is not configured on felis-api (FELIS_IMAGE / FELIS_BACKUP_PVC unset)")
|
||||||
|
case http.StatusNotFound:
|
||||||
|
return fmt.Errorf("no such server")
|
||||||
|
}
|
||||||
if e.Error.Message != "" {
|
if e.Error.Message != "" {
|
||||||
return fmt.Errorf("felis-api: %s", e.Error.Message)
|
return fmt.Errorf("felis-api: %s", e.Error.Message)
|
||||||
}
|
}
|
||||||
@@ -120,3 +128,19 @@ func performBackupNow(ctx context.Context, cl client.Client, controlNamespace, n
|
|||||||
hc := &http.Client{Timeout: 10 * time.Second}
|
hc := &http.Client{Timeout: 10 * time.Second}
|
||||||
return requestBackup(ctx, hc, baseURL, token, name, osUser)
|
return requestBackup(ctx, hc, baseURL, token, name, osUser)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// backupPickable narrows the backup picker to servers the backup API can accept.
|
||||||
|
// System servers (login/lobby) are excluded: they have no row in the servers
|
||||||
|
// table and carry reserved names, so every attempt dies in name validation —
|
||||||
|
// offering them would be a dead pick. The halt picker keeps them on purpose
|
||||||
|
// (break-glass retains full power over system servers); only the API-backed
|
||||||
|
// backup op cannot reach them.
|
||||||
|
func backupPickable(servers []haltableServer) []haltableServer {
|
||||||
|
out := make([]haltableServer, 0, len(servers))
|
||||||
|
for _, s := range servers {
|
||||||
|
if !s.system {
|
||||||
|
out = append(out, s)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
@@ -114,16 +114,23 @@ func TestRequestBackup(t *testing.T) {
|
|||||||
cases := []struct {
|
cases := []struct {
|
||||||
name string
|
name string
|
||||||
code int
|
code int
|
||||||
|
body string // optional JSON error body
|
||||||
expect string
|
expect string
|
||||||
}{
|
}{
|
||||||
{"409 not_stopped", http.StatusConflict, "must be stopped"},
|
{"409 not_stopped", http.StatusConflict, "", "must be stopped"},
|
||||||
{"503 backup_unavailable", http.StatusServiceUnavailable, "not configured"},
|
{"409 no_world_volume surfaces the API's own text", http.StatusConflict,
|
||||||
{"404 not found", http.StatusNotFound, "no such server"},
|
`{"error":{"code":"no_world_volume","message":"this server has no world volume yet — start it once to create it, then retry"}}`,
|
||||||
|
"no world volume yet"},
|
||||||
|
{"503 backup_unavailable", http.StatusServiceUnavailable, "", "not configured"},
|
||||||
|
{"404 not found", http.StatusNotFound, "", "no such server"},
|
||||||
}
|
}
|
||||||
for _, tc := range cases {
|
for _, tc := range cases {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
w.WriteHeader(tc.code)
|
w.WriteHeader(tc.code)
|
||||||
|
if tc.body != "" {
|
||||||
|
_, _ = io.WriteString(w, tc.body)
|
||||||
|
}
|
||||||
}))
|
}))
|
||||||
defer srv.Close()
|
defer srv.Close()
|
||||||
_, err := requestBackup(context.Background(), hc, srv.URL, "tok", "survival", "alice")
|
_, err := requestBackup(context.Background(), hc, srv.URL, "tok", "survival", "alice")
|
||||||
@@ -142,3 +149,25 @@ func TestRequestBackup(t *testing.T) {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The Sync picker must not offer system servers: the backup API validates names
|
||||||
|
// and resolves a servers-table row, so a lobby/login pick can only die in
|
||||||
|
// validation — a dead choice in an emergency console.
|
||||||
|
func TestBackupPickable(t *testing.T) {
|
||||||
|
got := backupPickable([]haltableServer{
|
||||||
|
{name: "lobby", phase: "Running", system: true},
|
||||||
|
{name: "login", phase: "Running", system: true},
|
||||||
|
{name: "test-one", phase: "Stopped"},
|
||||||
|
})
|
||||||
|
if len(got) != 1 || got[0].name != "test-one" || got[0].system {
|
||||||
|
t.Fatalf("backupPickable = %+v, want only the user server", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Survivors keep their input order (the picker's cursor math depends on it).
|
||||||
|
got = backupPickable([]haltableServer{
|
||||||
|
{name: "alpha"}, {name: "login", system: true}, {name: "beta"},
|
||||||
|
})
|
||||||
|
if len(got) != 2 || got[0].name != "alpha" || got[1].name != "beta" {
|
||||||
|
t.Fatalf("backupPickable order = %+v, want [alpha beta]", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -86,7 +86,7 @@ func (m *backupModel) loadCmd() tea.Cmd {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return backupListMsg{err: fmt.Errorf("list servers: %w", err)}
|
return backupListMsg{err: fmt.Errorf("list servers: %w", err)}
|
||||||
}
|
}
|
||||||
return backupListMsg{cl: cl, servers: servers}
|
return backupListMsg{cl: cl, servers: backupPickable(servers)}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user