From f843178355ec32da47437f96fcf18c3dd27edee8 Mon Sep 17 00:00:00 2001 From: Nico Hinderling Date: Mon, 14 Sep 2026 10:58:29 -0700 Subject: [PATCH 1/3] feat(snapshots): Follow the server-selected objectstore usecase Request automatic usecase selection for snapshot uploads and use the returned value for both existence checks and uploads. Default to preprod when older servers omit the field. Refs getsentry/sentry#124194, getsentry/sentry#124210 --- packages/cli/src/lib/api/preprod-artifacts.ts | 4 +- packages/cli/src/lib/objectstore.ts | 7 ++-- .../test/commands/snapshots/upload.test.ts | 10 ++++- .../test/lib/api/preprod-artifacts.test.ts | 40 +++++++++++++------ packages/cli/test/lib/objectstore.test.ts | 16 +++++--- 5 files changed, 53 insertions(+), 24 deletions(-) diff --git a/packages/cli/src/lib/api/preprod-artifacts.ts b/packages/cli/src/lib/api/preprod-artifacts.ts index 11708d03aa..c6ff7c614a 100644 --- a/packages/cli/src/lib/api/preprod-artifacts.ts +++ b/packages/cli/src/lib/api/preprod-artifacts.ts @@ -23,6 +23,7 @@ import { nullish, number, object, + optional, string, tuple, } from "valibot"; @@ -398,6 +399,7 @@ export async function getLatestBaseSnapshot( /** Objectstore config within the snapshots upload-options response. */ const ObjectstoreUploadOptionsSchema = object({ url: string(), + usecase: optional(string(), "preprod"), scopes: array(tuple([string(), string()])), authToken: nullish(string()), expirationPolicy: string(), @@ -427,7 +429,7 @@ export async function fetchSnapshotsUploadOptions( const { data } = await apiRequestToRegion( regionUrl, `projects/${org}/${project}/preprodartifacts/snapshots/upload-options/`, - { schema: SnapshotsUploadOptionsSchema } + { params: { usecase: "auto" }, schema: SnapshotsUploadOptionsSchema } ); return data; } diff --git a/packages/cli/src/lib/objectstore.ts b/packages/cli/src/lib/objectstore.ts index 8ba0355f57..f7caef9d83 100644 --- a/packages/cli/src/lib/objectstore.ts +++ b/packages/cli/src/lib/objectstore.ts @@ -15,9 +15,6 @@ import { customFetch } from "./custom-ca.js"; import { ApiError } from "./errors.js"; -/** The Objectstore usecase snapshots are stored under. */ -export const OBJECTSTORE_USECASE = "preprod"; - /** Header carrying the Objectstore bearer token. */ const AUTH_HEADER = "x-os-auth"; /** Header carrying an object's expiration policy (e.g. `ttl:30d`). */ @@ -38,6 +35,8 @@ const PUT_TIMEOUT_MS = 120_000; export type ObjectstoreConfig = { /** Base service URL (may include a path prefix). */ url: string; + /** Server-selected usecase matching the upload token. */ + usecase: string; /** Ordered scope pairs (e.g. `[["org","1"],["project","2"]]`). */ scopes: [string, string][]; /** Pre-signed bearer token, or null/absent for unauthenticated stores. */ @@ -59,7 +58,7 @@ function scopeSegment(scopes: [string, string][]): string { */ export function buildObjectUrl(config: ObjectstoreConfig, key: string): string { const base = config.url.replace(TRAILING_SLASHES, ""); - return `${base}/v1/objects/${OBJECTSTORE_USECASE}/${scopeSegment( + return `${base}/v1/objects/${config.usecase}/${scopeSegment( config.scopes )}/${key}`; } diff --git a/packages/cli/test/commands/snapshots/upload.test.ts b/packages/cli/test/commands/snapshots/upload.test.ts index 5fecc2d3d2..e7e8c0c218 100644 --- a/packages/cli/test/commands/snapshots/upload.test.ts +++ b/packages/cli/test/commands/snapshots/upload.test.ts @@ -55,6 +55,7 @@ function pngBytes(width: number, height: number): Buffer { const UPLOAD_OPTIONS = { objectstore: { url: "https://os.example.com", + usecase: "snapshots", scopes: [ ["org", "1"], ["project", "2"], @@ -103,7 +104,12 @@ describe("snapshots upload", () => { return dir; } - test("uploads images and creates a snapshot with a correct manifest", async () => { + test.each([ + "preprod", + "snapshots", + ])("uploads images to %s and creates a snapshot with a correct manifest", async (usecase) => { + const config = { ...UPLOAD_OPTIONS.objectstore, usecase }; + uploadOptionsSpy.mockResolvedValue({ objectstore: config }); const dir = await writeShots(); const harness = createContext(); const func = await uploadCommand.loader(); @@ -139,6 +145,8 @@ describe("snapshots upload", () => { )?.[1] as string; expect(key).toMatch(/^1\/2\/[0-9a-f]{64}$/); expect(key.endsWith(hash)).toBe(true); + expect(existsSpy).toHaveBeenCalledWith(config, key); + expect(putSpy).toHaveBeenCalledWith(config, key, expect.any(Uint8Array)); }); test("CLI width/height/content_hash override sidecar keys", async () => { diff --git a/packages/cli/test/lib/api/preprod-artifacts.test.ts b/packages/cli/test/lib/api/preprod-artifacts.test.ts index fb95804372..8a6d400ceb 100644 --- a/packages/cli/test/lib/api/preprod-artifacts.test.ts +++ b/packages/cli/test/lib/api/preprod-artifacts.test.ts @@ -10,7 +10,7 @@ import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { safeParse } from "valibot"; +import { parse, safeParse } from "valibot"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { ApiError, ValidationError } from "../../../src/lib/errors.js"; @@ -379,23 +379,37 @@ describe("snapshots", () => { ).toBe(false); }); - test("fetchSnapshotsUploadOptions hits the upload-options endpoint", async () => { - apiRequestToRegionMock.mockResolvedValue({ - data: { - objectstore: { - url: "https://os.example.com", - scopes: [["org", "1"]], - authToken: "tok", - expirationPolicy: "ttl:30d", - }, - }, - }); + test.each([ + { usecase: "snapshots", expectedUsecase: "snapshots" }, + { usecase: "preprod", expectedUsecase: "preprod" }, + { usecase: undefined, expectedUsecase: "preprod" }, + ])("fetchSnapshotsUploadOptions negotiates auto and parses $usecase as $expectedUsecase", async ({ + usecase, + expectedUsecase, + }) => { + apiRequestToRegionMock.mockImplementation( + async (_region, _path, { schema }) => ({ + data: parse(schema, { + objectstore: { + url: "https://os.example.com", + usecase, + scopes: [["org", "1"]], + authToken: "tok", + expirationPolicy: "ttl:30d", + }, + }), + }) + ); const opts = await fetchSnapshotsUploadOptions("my-org", "my-project"); expect(opts.objectstore.url).toBe("https://os.example.com"); - const [, endpoint] = apiRequestToRegionMock.mock.calls.at(-1) ?? []; + expect(opts.objectstore.usecase).toBe(expectedUsecase); + const [region, endpoint, options] = + apiRequestToRegionMock.mock.calls.at(-1) ?? []; + expect(region).toBe("https://us.sentry.io"); expect(endpoint).toBe( "projects/my-org/my-project/preprodartifacts/snapshots/upload-options/" ); + expect(options.params).toEqual({ usecase: "auto" }); }); test("createPreprodSnapshot POSTs the manifest and parses the response", async () => { diff --git a/packages/cli/test/lib/objectstore.test.ts b/packages/cli/test/lib/objectstore.test.ts index d73077d696..f31b1d6924 100644 --- a/packages/cli/test/lib/objectstore.test.ts +++ b/packages/cli/test/lib/objectstore.test.ts @@ -20,6 +20,7 @@ import { const config: ObjectstoreConfig = { url: "https://objectstore.example.com/", + usecase: "snapshots", scopes: [ ["org", "123"], ["project", "456"], @@ -36,9 +37,12 @@ afterEach(() => { }); describe("buildObjectUrl", () => { - test("joins usecase, scope, and key (stripping a trailing slash)", () => { - expect(buildObjectUrl(config, "123/456/abc")).toBe( - "https://objectstore.example.com/v1/objects/preprod/org=123;project=456/123/456/abc" + test.each([ + "preprod", + "snapshots", + ])("joins the %s usecase, scope, and key (stripping a trailing slash)", (usecase) => { + expect(buildObjectUrl({ ...config, usecase }, "123/456/abc")).toBe( + `https://objectstore.example.com/v1/objects/${usecase}/org=123;project=456/123/456/abc` ); }); }); @@ -49,7 +53,7 @@ describe("objectExists", () => { expect(await objectExists(config, "123/456/abc")).toBe(true); const [url, init] = customFetchMock.mock.calls[0] ?? []; expect(url).toContain( - "/v1/objects/preprod/org=123;project=456/123/456/abc" + "/v1/objects/snapshots/org=123;project=456/123/456/abc" ); expect(init.method).toBe("HEAD"); expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); @@ -85,7 +89,9 @@ describe("putObject", () => { await putObject(config, "123/456/abc", body); const [url, init] = customFetchMock.mock.calls[0] ?? []; - expect(url).toContain("/123/456/abc"); + expect(url).toBe( + "https://objectstore.example.com/v1/objects/snapshots/org=123;project=456/123/456/abc" + ); expect(init.method).toBe("PUT"); expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); expect(init.headers["x-sn-expiration"]).toBe("ttl:30d"); From 16f23e2636558e1e1d7293c23f043602fa5e10d6 Mon Sep 17 00:00:00 2001 From: Nico Hinderling Date: Mon, 14 Sep 2026 11:09:30 -0700 Subject: [PATCH 2/3] test(snapshots): Use the preprod_snapshots storage namespace --- packages/cli/test/commands/snapshots/upload.test.ts | 4 ++-- packages/cli/test/lib/api/preprod-artifacts.test.ts | 2 +- packages/cli/test/lib/objectstore.test.ts | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/cli/test/commands/snapshots/upload.test.ts b/packages/cli/test/commands/snapshots/upload.test.ts index e7e8c0c218..1ffc0c5a11 100644 --- a/packages/cli/test/commands/snapshots/upload.test.ts +++ b/packages/cli/test/commands/snapshots/upload.test.ts @@ -55,7 +55,7 @@ function pngBytes(width: number, height: number): Buffer { const UPLOAD_OPTIONS = { objectstore: { url: "https://os.example.com", - usecase: "snapshots", + usecase: "preprod_snapshots", scopes: [ ["org", "1"], ["project", "2"], @@ -106,7 +106,7 @@ describe("snapshots upload", () => { test.each([ "preprod", - "snapshots", + "preprod_snapshots", ])("uploads images to %s and creates a snapshot with a correct manifest", async (usecase) => { const config = { ...UPLOAD_OPTIONS.objectstore, usecase }; uploadOptionsSpy.mockResolvedValue({ objectstore: config }); diff --git a/packages/cli/test/lib/api/preprod-artifacts.test.ts b/packages/cli/test/lib/api/preprod-artifacts.test.ts index 8a6d400ceb..128e82216a 100644 --- a/packages/cli/test/lib/api/preprod-artifacts.test.ts +++ b/packages/cli/test/lib/api/preprod-artifacts.test.ts @@ -380,7 +380,7 @@ describe("snapshots", () => { }); test.each([ - { usecase: "snapshots", expectedUsecase: "snapshots" }, + { usecase: "preprod_snapshots", expectedUsecase: "preprod_snapshots" }, { usecase: "preprod", expectedUsecase: "preprod" }, { usecase: undefined, expectedUsecase: "preprod" }, ])("fetchSnapshotsUploadOptions negotiates auto and parses $usecase as $expectedUsecase", async ({ diff --git a/packages/cli/test/lib/objectstore.test.ts b/packages/cli/test/lib/objectstore.test.ts index f31b1d6924..f38c407539 100644 --- a/packages/cli/test/lib/objectstore.test.ts +++ b/packages/cli/test/lib/objectstore.test.ts @@ -20,7 +20,7 @@ import { const config: ObjectstoreConfig = { url: "https://objectstore.example.com/", - usecase: "snapshots", + usecase: "preprod_snapshots", scopes: [ ["org", "123"], ["project", "456"], @@ -39,7 +39,7 @@ afterEach(() => { describe("buildObjectUrl", () => { test.each([ "preprod", - "snapshots", + "preprod_snapshots", ])("joins the %s usecase, scope, and key (stripping a trailing slash)", (usecase) => { expect(buildObjectUrl({ ...config, usecase }, "123/456/abc")).toBe( `https://objectstore.example.com/v1/objects/${usecase}/org=123;project=456/123/456/abc` @@ -53,7 +53,7 @@ describe("objectExists", () => { expect(await objectExists(config, "123/456/abc")).toBe(true); const [url, init] = customFetchMock.mock.calls[0] ?? []; expect(url).toContain( - "/v1/objects/snapshots/org=123;project=456/123/456/abc" + "/v1/objects/preprod_snapshots/org=123;project=456/123/456/abc" ); expect(init.method).toBe("HEAD"); expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); @@ -90,7 +90,7 @@ describe("putObject", () => { const [url, init] = customFetchMock.mock.calls[0] ?? []; expect(url).toBe( - "https://objectstore.example.com/v1/objects/snapshots/org=123;project=456/123/456/abc" + "https://objectstore.example.com/v1/objects/preprod_snapshots/org=123;project=456/123/456/abc" ); expect(init.method).toBe("PUT"); expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); From 04ae8309a8f30a2625e2ba16a0944ca92edf7dc4 Mon Sep 17 00:00:00 2001 From: Nico Hinderling Date: Mon, 14 Sep 2026 11:27:14 -0700 Subject: [PATCH 3/3] ref(snapshots): Trim review comments and redundant URL coverage --- packages/cli/src/lib/objectstore.ts | 1 - packages/cli/test/lib/objectstore.test.ts | 11 +++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/lib/objectstore.ts b/packages/cli/src/lib/objectstore.ts index f7caef9d83..2c6442c0a4 100644 --- a/packages/cli/src/lib/objectstore.ts +++ b/packages/cli/src/lib/objectstore.ts @@ -35,7 +35,6 @@ const PUT_TIMEOUT_MS = 120_000; export type ObjectstoreConfig = { /** Base service URL (may include a path prefix). */ url: string; - /** Server-selected usecase matching the upload token. */ usecase: string; /** Ordered scope pairs (e.g. `[["org","1"],["project","2"]]`). */ scopes: [string, string][]; diff --git a/packages/cli/test/lib/objectstore.test.ts b/packages/cli/test/lib/objectstore.test.ts index f38c407539..b401eb783a 100644 --- a/packages/cli/test/lib/objectstore.test.ts +++ b/packages/cli/test/lib/objectstore.test.ts @@ -37,12 +37,11 @@ afterEach(() => { }); describe("buildObjectUrl", () => { - test.each([ - "preprod", - "preprod_snapshots", - ])("joins the %s usecase, scope, and key (stripping a trailing slash)", (usecase) => { - expect(buildObjectUrl({ ...config, usecase }, "123/456/abc")).toBe( - `https://objectstore.example.com/v1/objects/${usecase}/org=123;project=456/123/456/abc` + test("joins usecase, scope, and key (stripping a trailing slash)", () => { + expect( + buildObjectUrl({ ...config, usecase: "preprod" }, "123/456/abc") + ).toBe( + "https://objectstore.example.com/v1/objects/preprod/org=123;project=456/123/456/abc" ); }); });