From c6d8bc82a28fd075f040d3af6623e0376e404e8d Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 14 Jul 2026 15:18:21 +0200 Subject: [PATCH] =?UTF-8?q?C6B-F1=20cause=201=20+=20=C2=A78:=20additive=20?= =?UTF-8?q?.fab=20export=20(HDD=20binds=20AND=20named=20volumes)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit executeExport no longer either/or gates user data on needs_hdd — a needs_hdd app bundles BOTH its HDD mounts and its named volumes (sonarr_config = the whole app DB was silently dropped pre-fix). exportHDDData returns error and fails LOUDLY on a basename collision between mounts (the manifest keys tars by basename; the old code silently overwrote the first tar). EstimateExport made additive to match, so the fits-on-dest gate counts both. Round-trip placement test proves a userdata tar restores to /userdata through the untouched import mapping. Red-proofs recorded: either/or revert fails scenario A; collision-check removal fails the collision test. --- controller/internal/appexport/estimate.go | 30 +- .../appexport/estimate_volsize_test.go | 9 +- controller/internal/appexport/export.go | 48 +++- .../appexport/export_additive_test.go | 270 ++++++++++++++++++ 4 files changed, 326 insertions(+), 31 deletions(-) create mode 100644 controller/internal/appexport/export_additive_test.go diff --git a/controller/internal/appexport/estimate.go b/controller/internal/appexport/estimate.go index ba94008..51fd469 100644 --- a/controller/internal/appexport/estimate.go +++ b/controller/internal/appexport/estimate.go @@ -44,7 +44,8 @@ func (e *Exporter) EstimateExport(stackName, destDrive string) (*ExportEstimate, est.ConfigSizeHuman = humanizeBytes(est.ConfigSizeBytes) e.debugf("EstimateExport: configSize=%s (%d bytes)", est.ConfigSizeHuman, est.ConfigSizeBytes) - // Data size: HDD bind mounts or Docker volumes + // Data size: HDD bind mounts PLUS Docker volumes. v0.130.0 (C6B-F1): additive, mirroring the + // export itself — a needs_hdd app bundles BOTH, so the fits-on-dest gate must count both. if e.provider.GetStackNeedsHDD(stackName) { mounts := e.provider.GetStackHDDMounts(stackName) e.debugf("EstimateExport: HDD mounts: %v", mounts) @@ -53,21 +54,20 @@ func (e *Exporter) EstimateExport(stackName, destDrive string) (*ExportEstimate, e.debugf("EstimateExport: mount %s = %s", mount, humanizeBytes(mountSize)) est.DataSizeBytes += mountSize } - } else { - volumes := e.provider.GetDockerVolumes(stackName) - e.debugf("EstimateExport: Docker volumes: %v", volumes) - for _, vol := range volumes { - volSize, err := volumeSizer(vol) - if err != nil { - // F-A: the controller runs containerized, so a failed helper read must not - // silently become 0-that-reads-as-fits. Mark unknown and keep going. - e.logger.Printf("[WARN] appexport: volume size unknown for %s: %v", vol, err) - est.SizeUnknown = true - continue - } - e.debugf("EstimateExport: volume %s = %s", vol, humanizeBytes(volSize)) - est.DataSizeBytes += volSize + } + volumes := e.provider.GetDockerVolumes(stackName) + e.debugf("EstimateExport: Docker volumes: %v", volumes) + for _, vol := range volumes { + volSize, err := volumeSizer(vol) + if err != nil { + // F-A: the controller runs containerized, so a failed helper read must not + // silently become 0-that-reads-as-fits. Mark unknown and keep going. + e.logger.Printf("[WARN] appexport: volume size unknown for %s: %v", vol, err) + est.SizeUnknown = true + continue } + e.debugf("EstimateExport: volume %s = %s", vol, humanizeBytes(volSize)) + est.DataSizeBytes += volSize } if est.SizeUnknown { est.DataSizeHuman = "ismeretlen méret" diff --git a/controller/internal/appexport/estimate_volsize_test.go b/controller/internal/appexport/estimate_volsize_test.go index 091f9a2..386a766 100644 --- a/controller/internal/appexport/estimate_volsize_test.go +++ b/controller/internal/appexport/estimate_volsize_test.go @@ -8,14 +8,17 @@ import ( "testing" ) -// hddProvider is an rtProvider that reports an HDD-backed stack (for the regression scenario H). +// hddProvider is an rtProvider that reports an HDD-backed stack (estimate scenario H + the +// v0.130.0 additive-export tests in export_additive_test.go). type hddProvider struct { *rtProvider - mounts []string + mounts []string + hddPath string } -func (p *hddProvider) GetStackNeedsHDD(string) bool { return true } +func (p *hddProvider) GetStackNeedsHDD(string) bool { return true } func (p *hddProvider) GetStackHDDMounts(string) []string { return p.mounts } +func (p *hddProvider) GetStackHDDPath(string) string { return p.hddPath } func newEstimator(t *testing.T, provider ExportStackProvider) *Exporter { t.Helper() diff --git a/controller/internal/appexport/export.go b/controller/internal/appexport/export.go index 0484c88..be9691f 100644 --- a/controller/internal/appexport/export.go +++ b/controller/internal/appexport/export.go @@ -324,18 +324,24 @@ func (e *Exporter) executeExport(req ExportRequest, job *Job) { dataDir := filepath.Join(tmpDir, "data") os.MkdirAll(dataDir, 0755) + // C6B-F1 cause 1 (v0.130.0): user-data capture is ADDITIVE, not either/or. A needs_hdd app + // can hold state in BOTH its HDD binds and its named volumes (sonarr: ${USERDATA_PATH} media + // binds + the sonarr_config volume with the entire app DB) — the old else-branch silently + // dropped every named volume of every needs_hdd app. if e.provider.GetStackNeedsHDD(req.StackName) { e.debugf("exporting HDD data for %s", req.StackName) - e.exportHDDData(req.StackName, dataDir, manifest) - e.debugf("HDD data exported: subdirs=%v hasData=%v", manifest.HDDSubdirs, manifest.HasHDDData) - } else { - e.debugf("exporting Docker volumes for %s", req.StackName) - if err := e.exportVolumeData(req.StackName, dataDir, manifest); err != nil { - e.failJob(job, step, fmt.Sprintf("Kötet mentése sikertelen: %v", err)) + if err := e.exportHDDData(req.StackName, dataDir, manifest); err != nil { + e.failJob(job, step, fmt.Sprintf("Felhasználói adatok mentése sikertelen: %v", err)) return } - e.debugf("volume data exported: volumes=%v hasData=%v", manifest.VolumeNames, manifest.HasVolumeData) + e.debugf("HDD data exported: subdirs=%v hasData=%v", manifest.HDDSubdirs, manifest.HasHDDData) } + e.debugf("exporting Docker volumes for %s", req.StackName) + if err := e.exportVolumeData(req.StackName, dataDir, manifest); err != nil { + e.failJob(job, step, fmt.Sprintf("Kötet mentése sikertelen: %v", err)) + return + } + e.debugf("volume data exported: volumes=%v hasData=%v", manifest.VolumeNames, manifest.HasVolumeData) e.debugf("step 3 (user data) done in %v", time.Since(stepStart)) job.setStep(step, "done", "") @@ -581,8 +587,11 @@ func (e *Exporter) dumpDatabase(stackName, dbDir string, manifest *Manifest) boo return true } -// exportHDDData copies HDD bind mount data for the export. -func (e *Exporter) exportHDDData(stackName, dataDir string, manifest *Manifest) { +// exportHDDData copies HDD bind mount data for the export. v0.130.0 (C6B-F1 §8): a basename +// collision between two mounts is a FATAL error (the manifest keys tars by basename; the old +// code silently overwrote the first tar). A non-existent mount is still soft-skipped (honestly +// absent from the manifest — the anti-hollow guard catches total emptiness). +func (e *Exporter) exportHDDData(stackName, dataDir string, manifest *Manifest) error { hddDir := filepath.Join(dataDir, "hdd") os.MkdirAll(hddDir, 0755) @@ -590,15 +599,25 @@ func (e *Exporter) exportHDDData(stackName, dataDir string, manifest *Manifest) e.debugf("HDD mounts for %s: %v (%d total)", stackName, mounts, len(mounts)) if len(mounts) == 0 { e.debugf("no HDD mounts — skipping HDD data export") - return + return nil } + claimed := make(map[string]string) // subdir → mount that claimed it for _, mount := range mounts { if _, err := os.Stat(mount); os.IsNotExist(err) { e.debugf("HDD mount %s does not exist — skipping", mount) continue } subdir := filepath.Base(mount) + // C6B-F1 §8 (v0.130.0): the manifest keys HDD tars by BASENAME (the import side maps a + // basename back to a path), so two mounts sharing a basename cannot round-trip — the old + // code silently overwrote the first tar with the second (silent partial data loss). + // Renaming can't help either (the import couldn't map the new name), so the only honest + // outcome is a loud failure. + if prev, dup := claimed[subdir]; dup { + return fmt.Errorf("két adatkönyvtár azonos névvel végződik (%q: %s és %s) — a csomag nem tudná megkülönböztetni őket", subdir, prev, mount) + } + claimed[subdir] = mount tarPath := filepath.Join(hddDir, subdir+".tar") e.debugf("tarring HDD mount: %s → %s", mount, tarPath) @@ -616,6 +635,7 @@ func (e *Exporter) exportHDDData(stackName, dataDir string, manifest *Manifest) } } manifest.HasHDDData = len(manifest.HDDSubdirs) > 0 + return nil } // dockerExec is the docker-CLI seam (v0.125.0): runs `docker args...` with optional @@ -684,9 +704,11 @@ func (e *Exporter) exportVolumeTar(volName, tarPath string) error { }) } -// exportVolumeData exports Docker named volumes for apps without HDD storage. v0.125.0: a -// failed volume export is FATAL (export must never report success on a hollow bundle — the -// pre-fix WARN+continue is exactly how the data-loss bundles were born). +// exportVolumeData exports the app's Docker named volumes. v0.130.0 (C6B-F1): runs for EVERY +// app — needs_hdd apps hold state in named volumes too (sonarr_config = the whole app DB); the +// pre-fix else-branch silently dropped them. v0.125.0: a failed volume export is FATAL (export +// must never report success on a hollow bundle — the pre-fix WARN+continue is exactly how the +// data-loss bundles were born). func (e *Exporter) exportVolumeData(stackName, dataDir string, manifest *Manifest) error { volDir := filepath.Join(dataDir, "volumes") os.MkdirAll(volDir, 0755) diff --git a/controller/internal/appexport/export_additive_test.go b/controller/internal/appexport/export_additive_test.go new file mode 100644 index 0000000..061a5c4 --- /dev/null +++ b/controller/internal/appexport/export_additive_test.go @@ -0,0 +1,270 @@ +package appexport + +import ( + "archive/tar" + "compress/gzip" + "fmt" + "io" + "log" + "os" + "path/filepath" + "strings" + "testing" +) + +// C6B-F1 (v0.130.0) — the additive-export tests. A needs_hdd app bundles BOTH its HDD mounts +// and its named volumes (the old either/or dropped every needs_hdd app's volumes); a basename +// collision between mounts fails LOUDLY (the old code silently overwrote the first tar); the +// HDD round-trip places a "userdata" tar back at /userdata through the untouched +// import mapping. + +// listFabEntries returns the entry names inside an unencrypted .fab (tar.gz). +func listFabEntries(t *testing.T, fabPath string) map[string]int64 { + t.Helper() + f, err := os.Open(fabPath) + if err != nil { + t.Fatal(err) + } + defer f.Close() + gz, err := gzip.NewReader(f) + if err != nil { + t.Fatal(err) + } + tr := tar.NewReader(gz) + entries := map[string]int64{} + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatal(err) + } + entries[filepath.ToSlash(hdr.Name)] = hdr.Size + } + return entries +} + +func findFab(t *testing.T, drive string) string { + t.Helper() + entries, _ := os.ReadDir(ExportDir(drive)) + for _, en := range entries { + if strings.HasSuffix(en.Name(), ".fab") { + return filepath.Join(ExportDir(drive), en.Name()) + } + } + t.Fatal("no .fab produced") + return "" +} + +// Scenario A (§7): the sonarr shape — a needs_hdd app with a populated userdata mount AND a +// named volume. The bundle must contain BOTH tars; the manifest must claim both. +// RED-PROOF (either/or): revert executeExport to the else-only volume branch → the volume tar +// is absent and has_volume_data=false → this test fails. +// RED-PROOF (discovery): with the pre-fix ${HDD_PATH}-only adapter the mount list is empty → +// has_hdd_data=false → this test fails (proven at the adapter level in stacks/export_mounts_test.go). +func TestExport_NeedsHDDBundlesBothUserdataAndVolumes(t *testing.T) { + swapDockerExec(t, func(c dockerCall, stdin io.Reader, stdout io.Writer) (string, error) { + switch c.args[0] { + case "create": + fmt.Fprint(stdout, "cid-123\n") + return "", nil + case "cp": + // stream a plausible non-empty tar for the volume + fmt.Fprint(stdout, strings.Repeat("VOLTAR", 100)) + return "", nil + case "rm": + return "", nil + } + return "", fmt.Errorf("unexpected docker call: %v", c.args) + }) + + srcStack := t.TempDir() + os.WriteFile(filepath.Join(srcStack, "docker-compose.yml"), + []byte("services:\n hdd-app:\n image: alpine\n"), 0644) + + hdd := t.TempDir() + ud := filepath.Join(hdd, "userdata") + os.MkdirAll(filepath.Join(ud, "media", "tv"), 0755) + os.WriteFile(filepath.Join(ud, "media", "tv", "marker.bin"), []byte("USERDATA-MARKER-7"), 0644) + + prov := &hddProvider{ + rtProvider: &rtProvider{stackDir: srcStack, stacksDir: t.TempDir(), deployed: true, + volumes: []string{"hdd-app_config"}}, + mounts: []string{ud}, hddPath: hdd, + } + drive := t.TempDir() + e := NewExporter(prov, log.New(io.Discard, "", 0), "test") + + if err := e.StartExport(ExportRequest{StackName: "hdd-app", DestDrive: drive}); err != nil { + t.Fatalf("StartExport: %v", err) + } + job := waitJob(t, e) + if msg := jobErr(job); msg != "" { + t.Fatalf("export failed: %s", msg) + } + + fabPath := findFab(t, drive) + man, err := ReadManifestFromFAB(fabPath) + if err != nil { + t.Fatalf("manifest: %v", err) + } + if !man.HasHDDData { + t.Error("has_hdd_data=false — the userdata mount was dropped (C6B-F1 cause 2)") + } + if !man.HasVolumeData { + t.Error("has_volume_data=false — the named volume was dropped (C6B-F1 cause 1, the either/or)") + } + + entries := listFabEntries(t, fabPath) + if sz, ok := entries["data/hdd/userdata.tar"]; !ok || sz == 0 { + t.Errorf("bundle is missing a non-empty data/hdd/userdata.tar (entries: %v)", entries) + } + if sz, ok := entries["data/volumes/hdd-app_config.tar"]; !ok || sz == 0 { + t.Errorf("bundle is missing a non-empty data/volumes/hdd-app_config.tar (entries: %v)", entries) + } +} + +// Scenario E (§7): a needs_hdd app whose volume export strands (cp writes nothing) must FAIL the +// whole job loudly — never a partial-success bundle with userdata but silently-missing volumes. +func TestExport_NeedsHDDVolumeStrandFailsLoud(t *testing.T) { + swapDockerExec(t, func(c dockerCall, stdin io.Reader, stdout io.Writer) (string, error) { + switch c.args[0] { + case "create": + fmt.Fprint(stdout, "cid-123\n") + return "", nil + case "cp": + return "", nil // stranded: nothing written + case "rm": + return "", nil + } + return "", fmt.Errorf("unexpected docker call: %v", c.args) + }) + + srcStack := t.TempDir() + os.WriteFile(filepath.Join(srcStack, "docker-compose.yml"), + []byte("services:\n hdd-app:\n image: alpine\n"), 0644) + hdd := t.TempDir() + ud := filepath.Join(hdd, "userdata") + os.MkdirAll(ud, 0755) + os.WriteFile(filepath.Join(ud, "f.bin"), []byte("x"), 0644) + + prov := &hddProvider{ + rtProvider: &rtProvider{stackDir: srcStack, stacksDir: t.TempDir(), deployed: true, + volumes: []string{"vol1"}}, + mounts: []string{ud}, hddPath: hdd, + } + drive := t.TempDir() + e := NewExporter(prov, log.New(io.Discard, "", 0), "test") + if err := e.StartExport(ExportRequest{StackName: "hdd-app", DestDrive: drive}); err != nil { + t.Fatalf("StartExport: %v", err) + } + job := waitJob(t, e) + if msg := jobErr(job); msg == "" { + t.Fatal("a stranded volume tar must FAIL a needs_hdd export too — got success") + } + entries, _ := os.ReadDir(ExportDir(drive)) + for _, en := range entries { + if strings.HasSuffix(en.Name(), ".fab") { + t.Fatalf("a bundle was produced despite the stranded volume: %s", en.Name()) + } + } +} + +// §8: two mounts sharing a basename cannot round-trip through the basename-keyed manifest — +// the export must fail loudly instead of silently overwriting the first tar (the pre-fix +// behavior). RED-PROOF: drop the collision check in exportHDDData → this test fails. +func TestExport_HDDMountBasenameCollisionFailsLoud(t *testing.T) { + srcStack := t.TempDir() + os.WriteFile(filepath.Join(srcStack, "docker-compose.yml"), + []byte("services:\n hdd-app:\n image: alpine\n"), 0644) + + hdd := t.TempDir() + a := filepath.Join(hdd, "a", "config") + b := filepath.Join(hdd, "b", "config") + os.MkdirAll(a, 0755) + os.MkdirAll(b, 0755) + os.WriteFile(filepath.Join(a, "one.txt"), []byte("A"), 0644) + os.WriteFile(filepath.Join(b, "two.txt"), []byte("B"), 0644) + + prov := &hddProvider{ + rtProvider: &rtProvider{stackDir: srcStack, stacksDir: t.TempDir(), deployed: true}, + mounts: []string{a, b}, hddPath: hdd, + } + drive := t.TempDir() + e := NewExporter(prov, log.New(io.Discard, "", 0), "test") + if err := e.StartExport(ExportRequest{StackName: "hdd-app", DestDrive: drive}); err != nil { + t.Fatalf("StartExport: %v", err) + } + job := waitJob(t, e) + msg := jobErr(job) + if msg == "" { + t.Fatal("a basename collision must FAIL the export — got success (silent overwrite)") + } + if !strings.Contains(msg, "config") { + t.Errorf("the error must name the colliding basename, got %q", msg) + } +} + +// Scenario A' — the round-trip placement proof: an exported "userdata" tar restores back to +// /userdata through the UNTOUCHED import mapping (basename → / +// fallback). This is the property that dictated capturing the userdata ROOT rather than +// per-bind subpaths. +func TestFabRoundTrip_UserdataPlacement(t *testing.T) { + const stack = "ud-app" + lg := log.New(io.Discard, "", 0) + + hdd := t.TempDir() + ud := filepath.Join(hdd, "userdata") + os.MkdirAll(filepath.Join(ud, "media", "tv"), 0755) + marker := "ROUNDTRIP-MARKER-99" + os.WriteFile(filepath.Join(ud, "media", "tv", "show.bin"), []byte(marker), 0644) + + srcStack := t.TempDir() + os.WriteFile(filepath.Join(srcStack, "docker-compose.yml"), + []byte("services:\n ud-app:\n image: alpine\n volumes:\n - ${USERDATA_PATH}/media/tv:/tv\n"), 0644) + // app.yaml carries HDD_PATH into the bundle — the import derives every restore path from it. + os.WriteFile(filepath.Join(srcStack, "app.yaml"), + []byte("deployed: true\nenv:\n HDD_PATH: "+hdd+"\n"), 0644) + + prov := &hddProvider{ + rtProvider: &rtProvider{stackDir: srcStack, stacksDir: t.TempDir(), deployed: true}, + mounts: []string{ud}, hddPath: hdd, + } + drive := t.TempDir() + e := NewExporter(prov, lg, "test") + if err := e.StartExport(ExportRequest{StackName: stack, DestDrive: drive}); err != nil { + t.Fatalf("StartExport: %v", err) + } + job := waitJob(t, e) + if msg := jobErr(job); msg != "" { + t.Fatalf("export failed: %s", msg) + } + fabPath := findFab(t, drive) + + // wipe the source userdata — the import must bring it back to the same place + if err := os.RemoveAll(ud); err != nil { + t.Fatal(err) + } + + prov2 := &hddProvider{ + rtProvider: &rtProvider{stackDir: srcStack, stacksDir: t.TempDir(), deployed: false}, + hddPath: hdd, + } + e2 := NewExporter(prov2, lg, "test") + if err := e2.StartImport(ImportRequest{FABPath: fabPath}); err != nil { + t.Fatalf("StartImport: %v", err) + } + job = waitJob(t, e2) + if msg := jobErr(job); msg != "" { + t.Fatalf("import failed: %s", msg) + } + + got, err := os.ReadFile(filepath.Join(hdd, "userdata", "media", "tv", "show.bin")) + if err != nil { + t.Fatalf("restored userdata not at /userdata/media/tv/show.bin: %v", err) + } + if string(got) != marker { + t.Fatalf("restored content differs: got %q want %q", got, marker) + } +}