diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e85e7ca..238fadec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,12 @@ hub answers 400 to both and nothing else changes. A third review finding: a link TARGET through a link already in the folder („x" → „evil/y", evil pointing out). The rule is now strict and simple: a restored link points only DOWNWARD — relative, no „..", no link on its path or in its target; everything else is refused and listed (`safeDownwardLink`; `TestR1187b_NoLinkTargetThroughALink`, red-proved). + **Found live on 9202 the same night (grimmory), fixed:** the import ran `compose` without the controller's own path + variables (`USERDATA_PATH`, `IMPORT_PATH` — `stacks.policyEnv` adds them at every install and start); compose printed + „The USERDATA_PATH variable is not set" and the DB step read that WARNING as the container id, waited 60 s and left the + app removed and not started. Now `withImportPathVars` gives the import's compose runs the same derivation (never a + bundle's value) and `containerIDFrom` takes only a line that IS an id. Tests `TestR1187_TheContainerIDIsNeverAComposeWarning`, + `TestR1187_TheImportEnvCarriesTheControllersPathVars`, red-proved. - **R-1298:** the hold report skips an install's hold (`unhealthy_stop`, `update_failed`, `putback_pending`) of an app that is not installed (`holdreport.Reporter.SetInstalled`: the app's `app.yaml` exists — an unreadable one still counts, R-1220) and reports it cleared if it was reported; a restore hold is always reported (R-379). 9202's four orphan holds diff --git a/controller/internal/appexport/r1187_import_env_test.go b/controller/internal/appexport/r1187_import_env_test.go new file mode 100644 index 00000000..68ba587e --- /dev/null +++ b/controller/internal/appexport/r1187_import_env_test.go @@ -0,0 +1,40 @@ +package appexport + +import ( + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" +) + +// R-1187, found live on 9202 2026-10-26 night (grimmory): the import ran `compose` WITHOUT the two path variables the +// controller adds at every install and start (USERDATA_PATH, IMPORT_PATH — stacks.policyEnv). Compose then printed +// „The USERDATA_PATH variable is not set" on its combined output, the DB step read that WARNING as the container id, +// waited 60 s for a container that does not exist, and left the app removed and not started. Both halves pinned here. +// RED-PROOF: with the old helpers (the first output line; the env unchanged) both tests fail. +func TestR1187_TheContainerIDIsNeverAComposeWarning(t *testing.T) { + out := []byte("time=\"2026-10-26T20:18:06Z\" level=warning msg=\"The \\\"USERDATA_PATH\\\" variable is not set. Defaulting to a blank string.\"\n" + + "1047c698c778ab12cd34ef56ab78cd90ef12ab34cd56ef78ab90cd12ef34ab56\n") + if got := containerIDFrom(out); got != "1047c698c778ab12cd34ef56ab78cd90ef12ab34cd56ef78ab90cd12ef34ab56" { + t.Fatalf("container id = %q", got) + } + if got := containerIDFrom([]byte("time=x level=warning msg=y\n")); got != "" { + t.Fatalf("a warning alone must give no id, got %q", got) + } +} + +func TestR1187_TheImportEnvCarriesTheControllersPathVars(t *testing.T) { + env := map[string]string{"HDD_PATH": "/mnt/felhom-drives/d", "USERDATA_PATH": "/mnt/felhom-drives/d/appdata/other", "X": "1"} + got := withImportPathVars(env, "/mnt/felhom-drives/d", "/mnt/sys_drive/felhom-data/userdata/import") + if got["USERDATA_PATH"] != appbackup.UserdataDir("/mnt/felhom-drives/d") { + t.Fatalf("USERDATA_PATH = %q — must be the controller's own derivation, never the bundle's", got["USERDATA_PATH"]) + } + if got["IMPORT_PATH"] != "/mnt/sys_drive/felhom-data/userdata/import" || got["X"] != "1" || got["HDD_PATH"] != "/mnt/felhom-drives/d" { + t.Fatalf("env = %v", got) + } + if env["USERDATA_PATH"] != "/mnt/felhom-drives/d/appdata/other" { + t.Fatal("the caller's map was changed") + } + if g := withImportPathVars(map[string]string{"IMPORT_PATH": "/x"}, "", ""); g["USERDATA_PATH"] != "" || g["IMPORT_PATH"] != "" { + t.Fatalf("no namespace / no import root must leave neither variable: %v", g) + } +} diff --git a/controller/internal/appexport/restore.go b/controller/internal/appexport/restore.go index fa44be3c..682c626c 100644 --- a/controller/internal/appexport/restore.go +++ b/controller/internal/appexport/restore.go @@ -442,6 +442,8 @@ func (e *Exporter) executeImport(req ImportRequest, job *Job) { e.failJob(job, step, fmt.Sprintf("Konfiguráció visszaállítása sikertelen: %v", err)) return } + // R-1187 (found live): compose runs below see the controller's path variables, as at every install and start. + env = withImportPathVars(env, e.provider.GetStackNamespaceRoot(manifest.AppName), e.provider.GetImportRoot()) e.debugf("config restored: %d env vars", len(env)) if e.debug { for k := range env { @@ -1029,17 +1031,45 @@ func getComposeContainerID(stackDir string, env map[string]string, service strin if err != nil { return "", fmt.Errorf("compose ps: %w", err) } - id := strings.TrimSpace(string(out)) + id := containerIDFrom(out) if id == "" { return "", fmt.Errorf("no container found for service %s", service) } - // Take the first line in case of multiple - if idx := strings.Index(id, "\n"); idx > 0 { - id = id[:idx] - } return id, nil } +// containerIDFrom picks the container id out of `compose ps -q` output: the first line that IS an id (12–64 hex). The +// output is combined, so a compose WARNING can come first — it was read as the id once (R-1187, found live on 9202). +func containerIDFrom(out []byte) string { + for _, line := range strings.Split(string(out), "\n") { + line = strings.TrimSpace(line) + if n := len(line); n >= 12 && n <= 64 && strings.Trim(line, "0123456789abcdef") == "" { + return line + } + } + return "" +} + +// withImportPathVars is the import's env as `compose` must see it: the bundle's values plus the two path variables the +// controller itself adds at every install and start (stacks.policyEnv) — USERDATA_PATH always the controller's own +// derivation from the namespace root (never a bundle's value), IMPORT_PATH the drop-zone root. Without them compose +// printed a warning the DB step read as the container id (R-1187, live on 9202). Returns a new map. +func withImportPathVars(env map[string]string, nsRoot, importRoot string) map[string]string { + out := make(map[string]string, len(env)+2) + for k, v := range env { + out[k] = v + } + delete(out, "USERDATA_PATH") + delete(out, "IMPORT_PATH") + if nsRoot != "" { + out["USERDATA_PATH"] = appbackup.UserdataDir(nsRoot) + } + if importRoot != "" { + out["IMPORT_PATH"] = importRoot + } + return out +} + // waitForDB polls until the database is ready to accept connections. func waitForDB(containerID, dbType string, env map[string]string) error { timeout := 60 * time.Second