From 2d38b678a01adcbb33694ed4e4dd21216054cd1e Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Sun, 20 Sep 2026 15:55:51 +0200 Subject: [PATCH 1/2] fix(record): prefer branch-qualified cells over legacy cells IterRecords returns cells sorted by directory name, so a legacy - cell sorts after the branch-qualified -- cell it replaces. mergeRecord keeps the last write for a given ref and arch, so on an upgraded records dir the stale legacy digest and labels won. Drop the legacy cell when a branch-qualified cell for the same app, architecture, and branch exists. --- ARCHITECTURE.md | 2 +- pkg/record/record.go | 48 +++++++++++++- pkg/record/record_test.go | 136 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+), 4 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 367b6ab..7f3cd6b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -166,7 +166,7 @@ The `StreamWithPrefix` function reads output from subprocess pipes line-by-line ### `pkg/site` * Assembles the static repository landing page. * Fetches the current production index from the active Pages hosting to seed the update. -* Merges execution records from parallel runner cells. +* Merges execution records from parallel runner cells, preferring a branch-qualified cell over a legacy `-` cell for the same app, architecture, and branch, so a stale legacy record cannot shadow a fresh one. * Reconciles the index by validating digest existence via registry `HEAD` checks (pruning entries only on definitive 404s). * Generates GPG public key material (`key.asc`), signing manifests (`signing.json`), `.flatpakrepo` configurations, and one-click `.flatpakref` installer files. Bypasses key export and GPG validation checks when `no_sign` is set to `true`, and enforces GPG key existence unless `allow_unsigned` is explicitly enabled. diff --git a/pkg/record/record.go b/pkg/record/record.go index 567e26b..a974fcb 100644 --- a/pkg/record/record.go +++ b/pkg/record/record.go @@ -65,11 +65,16 @@ func (r Record) CellDir(root string) (string, error) { return "", err } if r.Branch == "" { - return filepath.Join(root, fmt.Sprintf("%s-%s", r.AppID, r.Arch)), nil + return filepath.Join(root, legacyCellName(r)), nil } return filepath.Join(root, fmt.Sprintf("%s-%s-%s", r.AppID, r.Branch, r.Arch)), nil } +// legacyCellName is the pre-branch cell directory name (-). +func legacyCellName(r Record) string { + return fmt.Sprintf("%s-%s", r.AppID, r.Arch) +} + // WriteRecord writes the record and labels into a cell directory under root. func WriteRecord(root string, r Record, labels map[string]string) (string, error) { cellDir, err := r.CellDir(root) @@ -153,6 +158,11 @@ func IterRecords(root string) ([]RecordWithLabels, error) { sort.Strings(cellDirs) var results []RecordWithLabels + // seen maps a logical cell (app, arch, branch) to its index in results, so a + // legacy cell cannot shadow the branch-qualified cell that replaces it on + // disk. Both forms carry the same branch in JSON, so the directory shape is + // the only signal that tells them apart. + seen := make(map[cellKey]int, len(cellDirs)) for _, cellPath := range cellDirs { recPath := filepath.Join(cellPath, "record.json") lblPath := filepath.Join(cellPath, "labels.json") @@ -179,16 +189,48 @@ func IterRecords(root string) ([]RecordWithLabels, error) { return nil, fmt.Errorf("failed to parse labels JSON from %q: %w", lblPath, err) } - results = append(results, RecordWithLabels{ + rwl := RecordWithLabels{ Record: r, Labels: labels, Path: cellPath, - }) + } + + key := cellKey{AppID: r.AppID, Arch: r.Arch, Branch: r.Branch} + if idx, ok := seen[key]; ok { + // Prefer the branch-qualified cell over the legacy - + // form. On equal preference the first cell wins, which keeps the + // result deterministic. + if isLegacyCell(results[idx].Path, results[idx].Record) && !isLegacyCell(rwl.Path, rwl.Record) { + results[idx] = rwl + } + continue + } + seen[key] = len(results) + results = append(results, rwl) } return results, nil } +// cellKey identifies the logical cell a record belongs to, independent of the +// directory shape the writing version used. +type cellKey struct { + AppID string + Arch string + Branch string +} + +// isLegacyCell reports whether a cell was written to the pre-branch path form +// (-). Records written before the branch became part of the cell path +// still carry a branch in JSON, so the directory name is the only reliable +// signal. +func isLegacyCell(cellPath string, r Record) bool { + if r.Branch == "" { + return false + } + return filepath.Base(cellPath) == legacyCellName(r) +} + func fileExists(path string) bool { info, err := os.Stat(path) if err != nil { diff --git a/pkg/record/record_test.go b/pkg/record/record_test.go index 90cc4d9..543c714 100644 --- a/pkg/record/record_test.go +++ b/pkg/record/record_test.go @@ -1,6 +1,7 @@ package record import ( + "encoding/json" "os" "path/filepath" "testing" @@ -259,6 +260,141 @@ func TestIterRecordsRecursive(t *testing.T) { } } +// writeRawCell places a record and labels directly in cellDir, bypassing +// WriteRecord, so a test can reproduce the legacy - layout a +// pre-branch version of this package produced. +func writeRawCell(t *testing.T, cellDir string, rec Record, labels map[string]string) { + t.Helper() + if err := os.MkdirAll(cellDir, 0755); err != nil { + t.Fatal(err) + } + recBytes, err := json.Marshal(rec) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(cellDir, "record.json"), recBytes, 0644); err != nil { + t.Fatal(err) + } + lblBytes, err := json.Marshal(labels) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(cellDir, "labels.json"), lblBytes, 0644); err != nil { + t.Fatal(err) + } +} + +const ( + freshDigest = "sha256:1111111111111111111111111111111111111111111111111111111111111111" + legacyDigest = "sha256:2222222222222222222222222222222222222222222222222222222222222222" +) + +// TestIterRecordsPrefersBranchQualifiedCell covers both lexical orders between +// the legacy - cell and the branch-qualified -- +// cell: only the architecture decides whether the legacy cell is seen before or +// after its replacement, so both paths through the dedupe are exercised. +func TestIterRecordsPrefersBranchQualifiedCell(t *testing.T) { + const appID = "org.example.App" + const branch = "stable" + + for _, arch := range []string{"x86_64", "aarch64"} { + t.Run(arch, func(t *testing.T) { + tempDir := t.TempDir() + ref := "app/" + appID + "/" + arch + "/" + branch + + // Fresh cell written by the current code: --. + fresh := Record{ + AppID: appID, + Arch: arch, + Branch: branch, + Name: "my-org/my-app", + Registry: "ghcr.io", + Digest: freshDigest, + Ref: ref, + Tag: "fresh", + } + freshLabels := map[string]string{ + "org.flatpak.ref": ref, + "org.flatpak.commit": "fresh", + } + freshCell, err := WriteRecord(tempDir, fresh, freshLabels) + if err != nil { + t.Fatalf("failed to write fresh record: %v", err) + } + if filepath.Base(freshCell) != appID+"-"+branch+"-"+arch { + t.Fatalf("fresh cell = %q, want branch-qualified directory", freshCell) + } + + // Legacy cell for the same app/arch/branch, written to the pre-branch + // path. + legacy := fresh + legacy.Digest = legacyDigest + legacy.Tag = "legacy" + writeRawCell(t, filepath.Join(tempDir, appID+"-"+arch), legacy, map[string]string{ + "org.flatpak.ref": ref, + "org.flatpak.commit": "legacy", + }) + + records, err := IterRecords(tempDir) + if err != nil { + t.Fatalf("failed to iter records: %v", err) + } + if len(records) != 1 { + t.Fatalf("expected the legacy cell to be dropped, got %d records: %+v", len(records), records) + } + if records[0].Path != freshCell { + t.Errorf("surviving cell path = %q, want %q", records[0].Path, freshCell) + } + if records[0].Record.Digest != freshDigest { + t.Errorf("surviving digest = %q, want %q", records[0].Record.Digest, freshDigest) + } + if records[0].Labels["org.flatpak.commit"] != "fresh" { + t.Errorf("surviving labels = %v, want the branch-qualified cell's", records[0].Labels) + } + }) + } +} + +func TestIterRecordsKeepsLegacyCellWithoutCounterpart(t *testing.T) { + tempDir := t.TempDir() + + const appID = "org.example.App" + const arch = "x86_64" + const branch = "stable" + ref := "app/" + appID + "/" + arch + "/" + branch + + // A legacy cell with no branch-qualified sibling is still the only record for + // its app/arch/branch, so it must keep loading (backward compatibility). + legacyCell := filepath.Join(tempDir, appID+"-"+arch) + writeRawCell(t, legacyCell, Record{ + AppID: appID, + Arch: arch, + Branch: branch, + Name: "my-org/my-app", + Registry: "ghcr.io", + Digest: legacyDigest, + Ref: ref, + Tag: "legacy", + }, map[string]string{ + "org.flatpak.ref": ref, + "org.flatpak.commit": "legacy", + }) + + records, err := IterRecords(tempDir) + if err != nil { + t.Fatalf("failed to iter records: %v", err) + } + if len(records) != 1 { + t.Fatalf("expected 1 record, got %d", len(records)) + } + if records[0].Path != legacyCell { + t.Errorf("surviving cell path = %q, want %q", records[0].Path, legacyCell) + } + if records[0].Record.Digest != legacyDigest { + t.Errorf("surviving digest = %q, want %q", records[0].Record.Digest, legacyDigest) + } +} + func TestWriteRecordSeparatesBranches(t *testing.T) { tempDir := t.TempDir() From 457de544b51a0ee48ca3b3cf8cf8551c04ebe44e Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Sun, 20 Sep 2026 15:55:56 +0200 Subject: [PATCH 2/2] test(tests): cover one app pushed on two branches --- tests/multi_branch_push_integration_test.go | 218 ++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 tests/multi_branch_push_integration_test.go diff --git a/tests/multi_branch_push_integration_test.go b/tests/multi_branch_push_integration_test.go new file mode 100644 index 0000000..9bcfd62 --- /dev/null +++ b/tests/multi_branch_push_integration_test.go @@ -0,0 +1,218 @@ +//go:build integration + +package tests + +import ( + "bytes" + "encoding/json" + "net" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// TestE2EPushTwoBranchesSameAppArch publishes one app that has two branches on +// the same architecture in a single push-oci run, then builds the site and +// checks both branches reach the index. The dotted branch (2.54) also exercises +// the OCI tag sanitization end to end. +func TestE2EPushTwoBranchesSameAppArch(t *testing.T) { + // 1. Verify environment and tools + requiredTools := []string{"flatpak", "ostree"} + for _, tool := range requiredTools { + if _, err := exec.LookPath(tool); err != nil { + t.Skipf("Skipping integration test: missing required tool %q", tool) + } + } + + runtime, err := resolveContainerRuntime() + if err != nil { + t.Skipf("Skipping integration test: %v", err) + } + + // Find a free TCP port for the registry + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to find a free port: %v", err) + } + _, registryPort, err := net.SplitHostPort(l.Addr().String()) + if err != nil { + l.Close() + t.Fatalf("failed to parse registry port: %v", err) + } + l.Close() + + const appID = "org.example.AppBranches" + const arch = "x86_64" + branches := []string{"master", "2.54"} + + tempDir, err := os.MkdirTemp("", "aetherpak-multi-branch-*") + if err != nil { + t.Fatalf("failed to create temp directory: %v", err) + } + defer os.RemoveAll(tempDir) + + repoPath := filepath.Join(tempDir, "repo") + recordsDir := filepath.Join(tempDir, "records") + siteDir := filepath.Join(tempDir, "site") + + // 2. Spin up registry container via container runtime + t.Log("Starting local OCI registry container...") + containerName := "aetherpak-test-multi-branch-registry-" + registryPort + + _ = exec.Command(runtime, "stop", containerName).Run() + _ = exec.Command(runtime, "rm", containerName).Run() + + runCmd := exec.Command(runtime, "run", "-d", + "--name", containerName, + "-p", registryPort+":5000", + "-e", "REGISTRY_STORAGE_DELETE_ENABLED=true", + "docker.io/library/registry:2", + ) + var runStderr bytes.Buffer + runCmd.Stderr = &runStderr + if err := runCmd.Run(); err != nil { + t.Fatalf("failed to spin up registry container (%v): %s", err, runStderr.String()) + } + + t.Cleanup(func() { + t.Log("Tearing down local OCI registry...") + _ = exec.Command(runtime, "stop", containerName).Run() + _ = exec.Command(runtime, "rm", containerName).Run() + }) + + registryAddr := "127.0.0.1:" + registryPort + t.Log("Waiting for registry to accept TCP connections...") + if !waitForTCPPort(registryAddr, 30*time.Second) { + t.Fatalf("registry failed to start on address: %s", registryAddr) + } + + // 3. Compile the binary + t.Log("Compiling aetherpak binary...") + buildCmd := exec.Command("make", "build") + buildCmd.Dir = ".." + var buildStderr bytes.Buffer + buildCmd.Stderr = &buildStderr + if err := buildCmd.Run(); err != nil { + t.Fatalf("failed to compile aetherpak binary (%v): %s", err, buildStderr.String()) + } + binaryPath, err := filepath.Abs(filepath.Join("..", "bin", "aetherpak")) + if err != nil { + t.Fatal(err) + } + + // 4. One app, two branches, same architecture. + for _, branch := range branches { + createMockFlatpakAppCustom(t, repoPath, appID, arch, branch) + } + + // 5. A single push with no arch/branch filters must publish both branches. + t.Log("Pushing both branches in one push-oci run...") + pushCmd := exec.Command(binaryPath, "push-oci", + "--registry=localhost:"+registryPort, + "--oci-repository=aetherpak/test-multi-branch", + "--repo-path="+repoPath, + "--records-dir="+recordsDir, + "--allow-unsigned", + ) + pushCmd.Dir = tempDir + var pushStdout, pushStderr bytes.Buffer + pushCmd.Stdout = &pushStdout + pushCmd.Stderr = &pushStderr + if err := pushCmd.Run(); err != nil { + t.Fatalf("push-oci failed: %v\nStdout: %s\nStderr: %s", err, pushStdout.String(), pushStderr.String()) + } + + // Both branches must land in distinct branch-qualified cells, and nothing + // else (in particular no legacy - cell). + expectedTags := map[string]string{ + "master": "org_example_AppBranches-master-x86_64", + "2.54": "org_example_AppBranches-2_54-x86_64", + } + + entries, err := os.ReadDir(recordsDir) + if err != nil { + t.Fatalf("failed to read records dir: %v", err) + } + var cellDirs []string + for _, entry := range entries { + if !entry.IsDir() { + continue + } + if _, err := os.Stat(filepath.Join(recordsDir, entry.Name(), "record.json")); err == nil { + cellDirs = append(cellDirs, entry.Name()) + } + } + if len(cellDirs) != len(branches) { + t.Errorf("expected %d record cells, got %v", len(branches), cellDirs) + } + + for _, branch := range branches { + cell := filepath.Join(recordsDir, appID+"-"+branch+"-"+arch, "record.json") + if _, err := os.Stat(cell); err != nil { + t.Errorf("expected a record cell for branch %q: %v", branch, err) + continue + } + + // The dotted branch proves PR #162's tag sanitization reached the record. + var rec struct { + Tag string `json:"tag"` + } + data, err := os.ReadFile(cell) + if err != nil { + t.Fatalf("failed to read %s: %v", cell, err) + } + if err := json.Unmarshal(data, &rec); err != nil { + t.Fatalf("failed to parse %s: %v", cell, err) + } + if rec.Tag != expectedTags[branch] { + t.Errorf("record tag for branch %q = %q, want %q", branch, rec.Tag, expectedTags[branch]) + } + } + + if _, err := os.Stat(filepath.Join(recordsDir, appID+"-"+arch)); !os.IsNotExist(err) { + t.Errorf("expected no legacy %s-%s cell, stat err = %v", appID, arch, err) + } + + // 6. Build the site. Point pages-url at a local 404 to keep the run offline + // and independent of any live Pages deployment. + pagesServer := httptest.NewServer(http.NotFoundHandler()) + defer pagesServer.Close() + + t.Log("Executing build-site...") + siteCmd := exec.Command(binaryPath, "build-site", + "--pages-url="+pagesServer.URL, + "--records-dir="+recordsDir, + "--site-dir="+siteDir, + "--allow-unsigned", + ) + siteCmd.Dir = tempDir + var siteStdout, siteStderr bytes.Buffer + siteCmd.Stdout = &siteStdout + siteCmd.Stderr = &siteStderr + if err := siteCmd.Run(); err != nil { + t.Fatalf("build-site failed: %v\nStdout: %s\nStderr: %s", err, siteStdout.String(), siteStderr.String()) + } + + // 7. Both branches must reach the index and get a flatpakref file. + staticPath := filepath.Join(siteDir, "index", "static") + data, err := os.ReadFile(staticPath) + if err != nil { + t.Fatalf("failed to read %s: %v", staticPath, err) + } + for _, branch := range branches { + ref := "app/" + appID + "/" + arch + "/" + branch + if !strings.Contains(string(data), ref) { + t.Errorf("expected index/static to contain ref %q, got: %s", ref, string(data)) + } + + refFile := filepath.Join(siteDir, "refs", appID+"-"+branch+".flatpakref") + if _, err := os.Stat(refFile); err != nil { + t.Errorf("expected a flatpakref for branch %q: %v", branch, err) + } + } +}