From 651e850fce57018ed35d260e4883ff6a1576c252 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Tue, 29 Sep 2026 08:39:11 -0400 Subject: [PATCH] fix(publish): keep images not yet moved to images/ in the registry Uploads moved from templates/ to images/, but only templates rebuilt since then followed. `cf publish --r2` scanned images/ alone, so the 2026-09-21 upstream check rebuilt debian-13 and published a registry with nothing else in it; the other 15 images are still under templates/ and still download. Publish now also reads the legacy prefix. Newest-per-template selection lets a rebuild under images/ supersede its old copy, and unmoved images keep their original URLs. Prune still scans only the configured prefix, so it never touches the legacy objects the registry points at. Also stop three fallbacks defaulting to templates/, and document images/. --- docs/setup.md | 15 ++++++++----- docs/usage.md | 12 +++++----- src/commands/maintenance.ts | 5 +++-- src/commands/publish.ts | 3 ++- src/config-file/upload.ts | 9 ++++++++ src/env.ts | 3 ++- src/manifest.ts | 44 +++++++++++++++++++++++++++---------- tests/manifest.test.ts | 39 ++++++++++++++++++++++++++++++++ 8 files changed, 103 insertions(+), 27 deletions(-) diff --git a/docs/setup.md b/docs/setup.md index 27e42bd..a99338e 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -149,7 +149,7 @@ the committed **`cofoundry.toml`**, which CI checks out and reads directly. It i Commit a `cofoundry.toml` (see [part 4](#4-local-development-optional), or run `cf init`). Its `[upload]` block controls the R2 layout; `layout = "grouped"` -produces `templates/{{group}}/{{recipe}}-{{arch}}/{{sha256}}{{ext}}`. See +produces `images/{{group}}/{{recipe}}-{{arch}}/{{sha256}}{{ext}}`. See [Usage → CDN upload](usage.md#cdn-upload). **Secrets** (Settings → Secrets and variables → Actions → Secrets): @@ -225,14 +225,14 @@ cpu_budget = 8 `R2_ACCESS_KEY_ID`/`R2_SECRET_ACCESS_KEY`; the S3 endpoint shown on that page is `R2_ENDPOINT`. 4. **Add a lifecycle rule** as a safety net — Settings → Object Lifecycle Rules, - prefix `templates/`, delete after 60 days. This catches orphans whose recipe + prefix `images/`, delete after 60 days. This catches orphans whose recipe was deleted; the pipeline's own `cf prune --r2 --keep 5` handles the tight per-recipe windows. With the grouped layout, published URLs look like: ``` -https://templates.example.com/templates//-/.qcow2 +https://templates.example.com/images//-/.qcow2 https://templates.example.com/registry.json ``` @@ -242,12 +242,17 @@ sidecar (`.json`), each content-addressed by its own hash. ### Path scheme -- **Images** — `templates/-/.qcow2` and, on OVMF recipes, +- **Images** — `images/-/.qcow2` and, on OVMF recipes, `…/.efivars.raw`. Content-addressed, immutable. -- **Sidecars** — `templates/-/.json`, addressed by the +- **Sidecars** — `images/-/.json`, addressed by the **system disk's** hash, so each build publishes a distinct key rather than overwriting the last. That history is what `cf publish --r2` picks newest-per-template from and what `cf prune --r2` retains N of. +- **Legacy** — `templates/…`, the prefix uploads used before `images/`. Nothing + is written there and `cf prune --r2` never touches it, but `cf publish --r2` + still reads its sidecars, so a template not rebuilt since the move stays in + the registry at its original URL. A rebuild under `images/` supersedes it. A + lifecycle rule on `templates/` deletes images the registry still points at. - **Registry** — `registry.json` at the root, short TTL (60s), one canonical pointer file. `git log registry.json` is the audit log; rollback is a `git revert` and CI re-mirrors. diff --git a/docs/usage.md b/docs/usage.md index 002ffb6..41450ba 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -193,19 +193,19 @@ and sidecar automatically. [upload] endpoint = "${R2_ENDPOINT}" # from env (contains the account id) bucket = "${R2_BUCKET}" -layout = "grouped" # templates/{{group}}/{{recipe}}-{{arch}}/{{sha256}} +layout = "grouped" # images/{{group}}/{{recipe}}-{{arch}}/{{sha256}} public_url = "https://cdn.example.com" -prefix = "templates/" # what `cf publish --r2` scans +prefix = "images/" # what `cf publish --r2` scans ``` The upload command, sidecar command, and public URL are **generated from the same key**, so they cannot drift. Both layouts are prune-safe, since each template gets its own directory: -| `layout` | object key | -| --------- | ---------------------------------------------------- | -| `grouped` | `templates/{{group}}/{{recipe}}-{{arch}}/{{sha256}}` | -| `flat` | `templates/{{recipe}}-{{arch}}/{{sha256}}` | +| `layout` | object key | +| --------- | ------------------------------------------------- | +| `grouped` | `images/{{group}}/{{recipe}}-{{arch}}/{{sha256}}` | +| `flat` | `images/{{recipe}}-{{arch}}/{{sha256}}` | For a custom path set `key` directly instead of `layout` — e.g. `key = "{{recipe}}/{{recipe}}-{{arch}}-{{sha256}}"`. For a fully hand-written diff --git a/src/commands/maintenance.ts b/src/commands/maintenance.ts index 0cf412a..cc4df08 100644 --- a/src/commands/maintenance.ts +++ b/src/commands/maintenance.ts @@ -4,6 +4,7 @@ import { runBootstrap } from '@/bootstrap.ts' import { runVerify } from '@/verify.ts' import { listRecipes, loadRecipe } from '@/config.ts' import { loadEnv, loadEnvPartial } from '@/env.ts' +import { DEFAULT_PREFIX } from '@/config-file/upload.ts' export const registerMaintenanceCommands = (program: Command): void => { program @@ -16,7 +17,7 @@ export const registerMaintenanceCommands = (program: Command): void => { .description('Reclaim space on the Proxmox node or in R2') .option('--days ', 'Treat files older than N days as stale', '30') .option('--dry-run', 'Enumerate targets without deleting', false) - .option('--r2', 'Prune R2 templates/ objects instead of node files') + .option('--r2', 'Prune R2 image objects instead of node files') .option( '--keep ', 'With --r2: keep newest N per template prefix', @@ -44,7 +45,7 @@ export const registerMaintenanceCommands = (program: Command): void => { { endpoint: env.R2_ENDPOINT, bucket: env.R2_BUCKET, - prefix: env.R2_PREFIX ?? 'templates/', + prefix: env.R2_PREFIX ?? DEFAULT_PREFIX, }, { keep: Number.parseInt(opts.keep, 10), diff --git a/src/commands/publish.ts b/src/commands/publish.ts index ff07626..4c7f260 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -2,6 +2,7 @@ import type { Command } from 'commander' import { runUpload } from '@/upload.ts' import { buildManifest, buildManifestFromR2 } from '@/manifest.ts' import { loadEnv, loadEnvPartial } from '@/env.ts' +import { DEFAULT_PREFIX } from '@/config-file/upload.ts' type UploadOptions = { remote?: boolean @@ -49,7 +50,7 @@ export const registerPublishCommands = (program: Command): void => { { endpoint: env.R2_ENDPOINT, bucket: env.R2_BUCKET, - prefix: env.R2_PREFIX ?? 'templates/', + prefix: env.R2_PREFIX ?? DEFAULT_PREFIX, }, opts.out, opts.prefix diff --git a/src/config-file/upload.ts b/src/config-file/upload.ts index cc11176..913ec3b 100644 --- a/src/config-file/upload.ts +++ b/src/config-file/upload.ts @@ -22,6 +22,15 @@ export type DerivedUpload = { */ export const DEFAULT_PREFIX = 'images/' +/** + * Prefixes uploads used to be written under. Nothing is written there any more, + * but objects already there stay put (published URLs are absolute and consumers + * store them verbatim), so `cf publish --r2` still has to read them: scanning + * only the current prefix drops every template not rebuilt since the move. + * Newest-per-template selection lets a rebuild under the current prefix win. + */ +export const LEGACY_PREFIXES: readonly string[] = ['templates/'] + /** * The part of the key below the prefix. The prefix itself is prepended from * `[upload].prefix`, which `cf publish --r2` also scans: naming it in both diff --git a/src/env.ts b/src/env.ts index fe3900c..be7134a 100644 --- a/src/env.ts +++ b/src/env.ts @@ -1,5 +1,6 @@ import { z } from 'zod' import { addSensitiveValues } from '@/util.ts' +import { DEFAULT_PREFIX } from '@/config-file/upload.ts' // CI passes unset secrets/vars as empty strings, which would defeat // `.default(...)` (Zod only applies defaults when the value is undefined). @@ -98,7 +99,7 @@ const EnvSchema = z.object({ R2_ENDPOINT: z.string().optional(), R2_BUCKET: z.string().optional(), - R2_PREFIX: z.string().default('templates/'), + R2_PREFIX: z.string().default(DEFAULT_PREFIX), }) export type Env = z.infer diff --git a/src/manifest.ts b/src/manifest.ts index d606572..737e913 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -14,6 +14,7 @@ import { type Template, } from '@/registry/schema.ts' import { renderUploadTemplate, uploadVariables } from '@/upload/template.ts' +import { LEGACY_PREFIXES } from '@/config-file/upload.ts' interface GroupDef { id: string @@ -207,6 +208,20 @@ const normalizeR2Prefix = (prefix: string): string => { return trimmed ? `${trimmed}/` : '' } +/** + * Every prefix `cf publish --r2` lists: the configured one, then each legacy + * prefix it does not already cover. An empty prefix lists the whole bucket, so + * nothing else is needed. + */ +export const publishPrefixes = (prefix: string): string[] => { + const current = normalizeR2Prefix(prefix) + if (!current) return [''] + const legacy = LEGACY_PREFIXES.map(normalizeR2Prefix).filter( + p => !p.startsWith(current) && !current.startsWith(p) + ) + return [current, ...legacy] +} + export interface R2Sidecar { key: string lastModified: string @@ -218,7 +233,7 @@ export interface R2Sidecar { * sidecar CONTENT (`name` is already `recipe-arch` and constant across * versions), never from the R2 key. Key layout is user-configurable via * `[upload].layout` / `[upload].key`, so any scheme that parses the key path - * breaks on some layout — e.g. `grouped` (templates//-/…) + * breaks on some layout — e.g. `grouped` (images//-/…) * would collapse a whole group to one entry, and a custom * `{{recipe}}/{{recipe}}-{{arch}}-{{sha256}}` would collapse archs of the same * recipe. Grouping on content is correct for every layout. @@ -237,9 +252,11 @@ export const selectNewestSidecars = (items: R2Sidecar[]): R2Sidecar[] => { /** * Aggregate sidecar JSONs from R2 into a registry, advertising the newest * artifact per template (not the full history). Every `.json` under the prefix - * is fetched and parsed: the template identity lives in the sidecar content, - * not the layout-dependent key, so we can't select newest-per-template from the - * listing alone. Object count is bounded by the retention `cf prune --r2` keeps. + * and each legacy prefix (see `publishPrefixes`) is fetched and parsed: the + * template identity lives in the sidecar content, not the layout-dependent key, + * so we can't select newest-per-template from the listing alone. Object count + * is bounded by the retention `cf prune --r2` keeps, and legacy prefixes no + * longer grow. */ export const buildManifestFromR2 = async ( location: { endpoint: string; bucket: string; prefix: string }, @@ -247,17 +264,20 @@ export const buildManifestFromR2 = async ( prefix = location.prefix ): Promise => { const { endpoint, bucket } = location - const normalizedPrefix = normalizeR2Prefix(prefix) + const prefixes = publishPrefixes(prefix) log.section( - `Publish ${pc.dim('·')} ${pc.cyan(`s3://${bucket}/${normalizedPrefix}`)}` + `Publish ${pc.dim('·')} ${prefixes.map(p => pc.cyan(`s3://${bucket}/${p}`)).join(pc.dim(' + '))}` ) - log.step(`listing objects`) - const listArgs = ['list-objects-v2', '--bucket', bucket] - if (normalizedPrefix) listArgs.push('--prefix', normalizedPrefix) - const raw = await s3api(endpoint, listArgs) - const parsed = raw.trim() ? JSON.parse(raw) : { Contents: [] } - const objects: R2Object[] = parsed.Contents ?? [] + const objects: R2Object[] = [] + for (const p of prefixes) { + log.step(`listing objects${p ? ` under ${p}` : ''}`) + const listArgs = ['list-objects-v2', '--bucket', bucket] + if (p) listArgs.push('--prefix', p) + const raw = await s3api(endpoint, listArgs) + const parsed = raw.trim() ? JSON.parse(raw) : { Contents: [] } + objects.push(...(parsed.Contents ?? [])) + } // The mirrored registry.json also ends in .json; never treat it as a sidecar. const sidecarObjects = objects.filter( diff --git a/tests/manifest.test.ts b/tests/manifest.test.ts index 76da61e..155777a 100644 --- a/tests/manifest.test.ts +++ b/tests/manifest.test.ts @@ -13,6 +13,7 @@ import { fileURLToPath } from 'node:url' import { buildManifest, disksMissingUrls, + publishPrefixes, selectNewestSidecars, withPublicUrls, type R2Sidecar, @@ -232,6 +233,26 @@ describe('buildManifest', () => { }) }) +describe('publishPrefixes', () => { + // Uploads moved from templates/ to images/ and only rebuilt templates + // followed; scanning images/ alone published a one-template registry. + test('also scans the legacy prefix the unmoved images live under', () => { + expect(publishPrefixes('images/')).toEqual(['images/', 'templates/']) + }) + + test('normalizes slashes on the configured prefix', () => { + expect(publishPrefixes('/images')).toEqual(['images/', 'templates/']) + }) + + test('does not scan a legacy prefix twice when it is the configured one', () => { + expect(publishPrefixes('templates/')).toEqual(['templates/']) + }) + + test('lists the whole bucket once when the prefix is empty', () => { + expect(publishPrefixes('')).toEqual(['']) + }) +}) + describe('selectNewestSidecars', () => { const names = (items: R2Sidecar[]): string[] => items.map(i => i.sidecar.name).sort() @@ -274,6 +295,24 @@ describe('selectNewestSidecars', () => { expect(names(newest)).toEqual(['debian-12-amd64', 'debian-12-arm64']) }) + test('a rebuild under the new prefix supersedes the legacy copy', () => { + const newest = selectNewestSidecars([ + { + ...sc('debian-13-amd64', '2026-09-04T05:40:11.000Z'), + key: 'templates/debian/debian-13-amd64/old.json', + }, + sc('debian-13-amd64', '2026-09-21T11:24:17.000Z'), + { + ...sc('debian-12-amd64', '2026-09-04T05:42:35.000Z'), + key: 'templates/debian/debian-12-amd64/old.json', + }, + ]) + expect(newest.map(i => i.key).sort()).toEqual([ + 'images/debian-13-amd64/2026-09-21T11:24:17.000Z.json', + 'templates/debian/debian-12-amd64/old.json', + ]) + }) + test('ignores sidecars with no name', () => { const newest = selectNewestSidecars([ { ...sc('debian-12-amd64', '2026-01-01T00:00:00.000Z') },