diff --git a/.github/workflows/module-definitions.yml b/.github/workflows/module-definitions.yml index a391a097..a157684c 100644 --- a/.github/workflows/module-definitions.yml +++ b/.github/workflows/module-definitions.yml @@ -77,7 +77,7 @@ jobs: - name: Generate publish dry-run plan id: publish_plan continue-on-error: true - run: node tools/ravion-modules/dist/src/cli.js publish --format markdown --output publish-plan.md 2> publish-plan.err + run: node tools/ravion-modules/dist/src/cli.js publish --validate-remote --format markdown --output publish-plan.md 2> publish-plan.err - name: Comment publish dry-run plan uses: actions/github-script@v7 @@ -136,7 +136,7 @@ jobs: working-directory: tools/ravion-modules - name: Dry-run publish plan - run: node tools/ravion-modules/dist/src/cli.js publish + run: node tools/ravion-modules/dist/src/cli.js publish --validate-remote - name: Configure Git tag author run: | diff --git a/tools/ravion-modules/src/cli.ts b/tools/ravion-modules/src/cli.ts index 160f33ee..d1e083a6 100644 --- a/tools/ravion-modules/src/cli.ts +++ b/tools/ravion-modules/src/cli.ts @@ -91,7 +91,7 @@ if (command === "validate") { const outputPath = getArgValue(args, "--output"); let result; try { - result = await publishDefinitions(compiled, client, { dryRun: localDev ? args.includes("--dry-run") : !args.includes("--apply"), localDev, localDevForce: args.includes("--force"), localDevSourceRef, logger: (message) => console.error(`[publish] ${message}`) }); + result = await publishDefinitions(compiled, client, { dryRun: localDev ? args.includes("--dry-run") : !args.includes("--apply"), validateRemote: args.includes("--validate-remote"), localDev, localDevForce: args.includes("--force"), localDevSourceRef, logger: (message) => console.error(`[publish] ${message}`) }); } catch (error) { if (isPublishPlanError(error)) { const output = format === "markdown" ? formatPublishPlanMarkdown(error.result) : JSON.stringify(error.result, null, 2); diff --git a/tools/ravion-modules/src/generate-definitions.ts b/tools/ravion-modules/src/generate-definitions.ts index 676e82db..7d757a89 100644 --- a/tools/ravion-modules/src/generate-definitions.ts +++ b/tools/ravion-modules/src/generate-definitions.ts @@ -7,6 +7,7 @@ export interface RemoteModuleDefinition { type: string; name: string; description: string; + isGlobalPublished?: boolean; } export interface RemoteModuleVersion { diff --git a/tools/ravion-modules/src/publish.ts b/tools/ravion-modules/src/publish.ts index 38024b0d..d58eb276 100644 --- a/tools/ravion-modules/src/publish.ts +++ b/tools/ravion-modules/src/publish.ts @@ -28,6 +28,7 @@ export interface PublishResult { dryRun: boolean; items: PublishPlanItem[]; errors?: PublishPlanErrorItem[]; + validationErrors?: PublishValidationErrorItem[]; } export interface PublishPlanErrorItem { @@ -38,6 +39,12 @@ export interface PublishPlanErrorItem { diff?: string; } +export interface PublishValidationErrorItem { + type: string; + version: string; + message: string; +} + export interface ModuleDefinitionInput { type: string; name: string; @@ -66,6 +73,7 @@ export interface RavionApiClientOptions { export interface PublishOptions { dryRun?: boolean; + validateRemote?: boolean; localDev?: boolean; localDevForce?: boolean; localDevSourceRef?: string; @@ -80,6 +88,7 @@ export interface RavionModuleApiClient { patchModuleDefinition(input: ModuleDefinitionPatchInput): Promise; listModuleVersions(moduleDefinitionId: string): Promise; createModuleVersion(input: ModuleVersionInput): Promise; + validateModuleVersion(input: ModuleVersionInput): Promise; } export class PublishError extends Error { @@ -127,11 +136,15 @@ export async function publishDefinitions( inventory.definitions.map((definition) => [definition.type, definition]), ); const items: PublishPlanItem[] = []; + const validationErrors: PublishValidationErrorItem[] = []; for (const definition of [...definitionsToPublish].sort((left, right) => left.type.localeCompare(right.type), )) { let remoteDefinition = definitionsByType.get(definition.type); + const shouldPlanGlobalPublication = remoteDefinition?.isGlobalPublished === false; + const shouldPublishDefinitionAfterVersion = + !remoteDefinition || remoteDefinition.isGlobalPublished === false; if (!remoteDefinition) { items.push( createItem( @@ -153,10 +166,6 @@ export async function publishDefinitions( name: definition.name, description: definition.description, }); - remoteDefinition = await client.patchModuleDefinition({ - id: remoteDefinition.id, - isGlobalPublished: true, - }); definitionsByType.set(remoteDefinition.type, remoteDefinition); inventory.versionsByDefinitionId[remoteDefinition.id] = []; } @@ -191,12 +200,30 @@ export async function publishDefinitions( } } + const latestRemoteVersion = remoteDefinition + ? selectLatestVersion(inventory.versionsByDefinitionId[remoteDefinition.id] ?? []) + : undefined; + const globalPublicationItem = shouldPlanGlobalPublication + ? createItem( + definition, + "patch-definition", + dryRun, + `Publish module definition ${definition.type} globally.`, + "Make the module definition available globally.", + createDiff( + { isGlobalPublished: false }, + { isGlobalPublished: true }, + ), + latestRemoteVersion?.version, + ) + : undefined; + const remoteVersion = remoteDefinition ? (inventory.versionsByDefinitionId[remoteDefinition.id] ?? []).find( (version) => version.version === definition.version, ) : undefined; - if (remoteVersion) { + if (remoteVersion && remoteDefinition) { items.push( createItem( definition, @@ -208,12 +235,19 @@ export async function publishDefinitions( remoteVersion.version, ), ); + if (globalPublicationItem) { + items.push(globalPublicationItem); + } + if (!dryRun && shouldPublishDefinitionAfterVersion) { + remoteDefinition = await client.patchModuleDefinition({ + id: remoteDefinition.id, + isGlobalPublished: true, + }); + definitionsByType.set(remoteDefinition.type, remoteDefinition); + } continue; } - const latestRemoteVersion = remoteDefinition - ? selectLatestVersion(inventory.versionsByDefinitionId[remoteDefinition.id] ?? []) - : undefined; items.push( createItem( definition, @@ -225,16 +259,66 @@ export async function publishDefinitions( latestRemoteVersion?.version, ), ); + if (globalPublicationItem) { + items.push(globalPublicationItem); + } + if (dryRun && options.validateRemote) { + if (!remoteDefinition) { + options.logger?.( + `Skipping remote validation for ${definition.type}@${definition.version}; the module definition does not exist remotely yet.`, + ); + } else { + try { + await client.validateModuleVersion({ + moduleDefinitionId: remoteDefinition.id, + version: definition.version, + description: definition.releaseDescription, + config: definition.module, + }); + options.logger?.( + `Remote validation passed for ${definition.type}@${definition.version}.`, + ); + } catch (error) { + validationErrors.push({ + type: definition.type, + version: definition.version, + message: formatUnknownError(error), + }); + } + } + } if (!dryRun) { if (!remoteDefinition) { throw new PublishError( `Cannot create ${definition.type}@${definition.version}; module definition was not created.`, ); } + if (shouldPublishDefinitionAfterVersion) { + await client.validateModuleVersion({ + moduleDefinitionId: remoteDefinition.id, + version: definition.version, + description: definition.releaseDescription, + config: definition.module, + }); + } await createVersionOrConfirmDuplicate(client, remoteDefinition.id, definition); + if (shouldPublishDefinitionAfterVersion) { + remoteDefinition = await client.patchModuleDefinition({ + id: remoteDefinition.id, + isGlobalPublished: true, + }); + definitionsByType.set(remoteDefinition.type, remoteDefinition); + } } } + if (validationErrors.length > 0) { + throw new PublishPlanError( + `Remote validation failed for ${validationErrors.length} module version(s).`, + { dryRun, items, validationErrors }, + ); + } + return { dryRun, items }; } @@ -295,6 +379,23 @@ export function formatPublishPlanMarkdown(result: PublishResult): string { "", ]; + if (result.validationErrors && result.validationErrors.length > 0) { + lines.push( + "### 🚨 Remote Validation Failures 🚨", + "", + "The Ravion API rejected these module version configs during dry-run validation. Fix the module definition config before merging.", + "", + "| Module | Release Version | Error |", + "| --- | --- | --- |", + ); + for (const error of result.validationErrors) { + lines.push( + `| \`${escapeMarkdownTableCell(error.type)}\` | \`${escapeMarkdownTableCell(error.version)}\` | ${escapeMarkdownTableCell(error.message)} |`, + ); + } + lines.push(""); + } + if (result.errors && result.errors.length > 0) { lines.push( "### 🚨 Release Config Conflicts 🚨", @@ -771,6 +872,17 @@ class HttpRavionModuleApiClient implements RavionModuleApiClient { return this.call("POST", "/module-versions", { data: input }); } + async validateModuleVersion(input: ModuleVersionInput): Promise { + const { status } = await this.request("POST", "/module-versions", { + data: { ...input, dryRun: true }, + }); + if (status !== 202) { + throw new PublishError( + `POST /module-versions dry-run validation returned HTTP ${status} instead of 202. The Ravion API may not support dryRun yet, and the module version may have been created for real.`, + ); + } + } + private async list(path: string, query: Record = {}): Promise { const items: T[] = []; let cursor: string | undefined; @@ -797,6 +909,16 @@ class HttpRavionModuleApiClient implements RavionModuleApiClient { query: Record = {}, options: { unwrap?: boolean } = {}, ): Promise { + const { payload } = await this.request(method, path, input, query); + return (options.unwrap === false ? payload : unwrapApiPayload(payload)) as T; + } + + private async request( + method: string, + path: string, + input?: unknown, + query: Record = {}, + ): Promise<{ status: number; payload: unknown }> { const headers: Record = { "Content-Type": "application/json" }; if (this.token) { headers.Authorization = `Bearer ${this.token}`; @@ -823,7 +945,7 @@ class HttpRavionModuleApiClient implements RavionModuleApiClient { `${method} ${url} failed with HTTP ${response.status}: ${message}\nResponse body:\n${formatResponsePayload(payload)}`, ); } - return (options.unwrap === false ? payload : unwrapApiPayload(payload)) as T; + return { status: response.status, payload }; } } diff --git a/tools/ravion-modules/test/publish.test.ts b/tools/ravion-modules/test/publish.test.ts index d8201edf..03e94f7e 100644 --- a/tools/ravion-modules/test/publish.test.ts +++ b/tools/ravion-modules/test/publish.test.ts @@ -30,6 +30,111 @@ describe("publish", () => { assert.deepEqual(client.createdVersions.map(({ moduleDefinitionId, version, description, config }) => ({ moduleDefinitionId, version, description, config })), [ { moduleDefinitionId: "definition-1", version: "1.2.3", description: "Add subnet options.", config: { inputs: [{ id: "name", type: "string", label: "Name" }] } }, ]); + assert.deepEqual(client.validatedVersions, [ + { moduleDefinitionId: "definition-1", version: "1.2.3", description: "Add subnet options.", config: { inputs: [{ id: "name", type: "string", label: "Name" }] } }, + ]); + }); + + it("does not globally publish a new definition when its first version fails validation", async () => { + const client = new MockRavionClient(); + client.onValidateVersion = async () => { + throw new Error("Config is invalid"); + }; + + await assert.rejects( + () => publishDefinitions([createCompiledDefinition()], client, { dryRun: false }), + /Config is invalid/, + ); + + assert.equal(client.createdDefinitions.length, 1); + assert.equal(client.createdVersions.length, 0); + assert.equal(client.patchedDefinitions.length, 0); + }); + + it("does not globally publish a new definition when its first version fails creation", async () => { + const client = new MockRavionClient(); + client.onCreateVersion = async () => { + throw new Error("Version creation failed"); + }; + + await assert.rejects( + () => publishDefinitions([createCompiledDefinition()], client, { dryRun: false }), + /Version creation failed/, + ); + + assert.equal(client.validatedVersions.length, 1); + assert.equal(client.patchedDefinitions.length, 0); + }); + + it("globally publishes an existing private definition after confirming its version", async () => { + const compiled = createCompiledDefinition(); + const client = new MockRavionClient({ + definitions: [ + { + id: "vpc", + type: "ravion-aws-vpc", + name: "AWS VPC", + description: "AWS VPC and subnets.", + isGlobalPublished: false, + }, + ], + versionsByDefinitionId: { vpc: [createRemoteVersion({ config: compiled.module })] }, + }); + + const result = await publishDefinitions([compiled], client, { dryRun: false }); + + assert.deepEqual(result.items.map(({ action }) => action), ["skip-version", "patch-definition"]); + assert.deepEqual(client.patchedDefinitions, [{ id: "vpc", isGlobalPublished: true }]); + }); + + it("shows global publication of an existing private definition in the dry-run plan", async () => { + const compiled = createCompiledDefinition(); + const client = new MockRavionClient({ + definitions: [ + { + id: "vpc", + type: "ravion-aws-vpc", + name: "AWS VPC", + description: "AWS VPC and subnets.", + isGlobalPublished: false, + }, + ], + versionsByDefinitionId: { vpc: [createRemoteVersion({ config: compiled.module })] }, + }); + + const result = await publishDefinitions([compiled], client); + const markdown = formatPublishPlanMarkdown(result); + + assert.match(markdown, /\| `ravion-aws-vpc` \| `1\.2\.3` \| `1\.2\.3` \| Make the module definition available globally\. \|/); + assert.match(markdown, /-isGlobalPublished: false/); + assert.match(markdown, /\+isGlobalPublished: true/); + assert.equal(client.patchedDefinitions.length, 0); + }); + + it("shows global publication when an existing private definition needs a new version", async () => { + const compiled = createCompiledDefinition(); + const client = new MockRavionClient({ + definitions: [ + { + id: "vpc", + type: "ravion-aws-vpc", + name: "AWS VPC", + description: "AWS VPC and subnets.", + isGlobalPublished: false, + }, + ], + versionsByDefinitionId: { + vpc: [createRemoteVersion({ version: "1.2.2", config: { inputs: [] } })], + }, + }); + + const result = await publishDefinitions([compiled], client); + const markdown = formatPublishPlanMarkdown(result); + + assert.deepEqual(result.items.map(({ action }) => action), ["create-version", "patch-definition"]); + assert.match(markdown, /\| `ravion-aws-vpc` \| `1\.2\.2` \| `1\.2\.3` \| Add subnet options\. \|/); + assert.match(markdown, /-isGlobalPublished: false/); + assert.match(markdown, /\+isGlobalPublished: true/); }); it("patches metadata changes before publishing versions", async () => { @@ -53,6 +158,99 @@ describe("publish", () => { assert.equal(client.createdVersions.length, 0); }); + it("validates create-version items remotely during dry run when requested", async () => { + const client = new MockRavionClient({ + definitions: [{ id: "vpc", type: "ravion-aws-vpc", name: "AWS VPC", description: "AWS VPC and subnets." }], + }); + + const result = await publishDefinitions([createCompiledDefinition()], client, { dryRun: true, validateRemote: true }); + + assert.deepEqual(result.items.map(({ action, dryRun }) => ({ action, dryRun })), [{ action: "create-version", dryRun: true }]); + assert.deepEqual(client.validatedVersions, [ + { moduleDefinitionId: "vpc", version: "1.2.3", description: "Add subnet options.", config: { inputs: [{ id: "name", type: "string", label: "Name" }] } }, + ]); + assert.equal(client.createdVersions.length, 0); + }); + + it("fails the dry run with validation details when the API rejects a config", async () => { + const client = new MockRavionClient({ + definitions: [{ id: "vpc", type: "ravion-aws-vpc", name: "AWS VPC", description: "AWS VPC and subnets." }], + }); + client.onValidateVersion = async () => { + throw new Error('POST /module-versions failed with HTTP 422: inputs.0.required: field is not allowed'); + }; + + await assert.rejects( + publishDefinitions([createCompiledDefinition()], client, { dryRun: true, validateRemote: true }), + (error: unknown) => { + assert.ok(error instanceof PublishPlanError); + assert.deepEqual(error.result.validationErrors, [ + { type: "ravion-aws-vpc", version: "1.2.3", message: "Error: POST /module-versions failed with HTTP 422: inputs.0.required: field is not allowed" }, + ]); + const markdown = formatPublishPlanMarkdown(error.result); + assert.match(markdown, /Remote Validation Failures/); + assert.match(markdown, /field is not allowed/); + return true; + }, + ); + assert.equal(client.createdVersions.length, 0); + }); + + it("includes valid plan items and diffs alongside remote validation failures", async () => { + const client = new MockRavionClient({ + definitions: [ + { id: "invalid", type: "aaa-invalid", name: "Invalid", description: "Invalid module." }, + { id: "valid", type: "bbb-valid", name: "Valid", description: "Valid module." }, + ], + }); + client.onValidateVersion = async (input) => { + if (input.moduleDefinitionId === "invalid") { + throw new Error("Config is invalid"); + } + }; + + await assert.rejects( + publishDefinitions( + [ + createCompiledDefinition({ type: "aaa-invalid", name: "Invalid", description: "Invalid module." }), + createCompiledDefinition({ type: "bbb-valid", name: "Valid", description: "Valid module." }), + ], + client, + { dryRun: true, validateRemote: true }, + ), + (error: unknown) => { + assert.ok(error instanceof PublishPlanError); + const markdown = formatPublishPlanMarkdown(error.result); + assert.match(markdown, /Remote Validation Failures/); + assert.match(markdown, /\| `aaa-invalid` \| n\/a \| `1\.2\.3` \| Add subnet options\. \|/); + assert.match(markdown, /\| `bbb-valid` \| n\/a \| `1\.2\.3` \| Add subnet options\. \|/); + assert.match(markdown, /#### aaa-invalid n\/a -> 1\.2\.3/); + assert.match(markdown, /#### bbb-valid n\/a -> 1\.2\.3/); + return true; + }, + ); + }); + + it("skips remote validation for definitions that do not exist remotely yet", async () => { + const client = new MockRavionClient(); + + const result = await publishDefinitions([createCompiledDefinition()], client, { dryRun: true, validateRemote: true }); + + assert.deepEqual(result.items.map(({ action }) => action), ["create-definition", "create-version"]); + assert.equal(client.validatedVersions.length, 0); + assert.equal(client.createdVersions.length, 0); + }); + + it("does not validate remotely during dry run when not requested", async () => { + const client = new MockRavionClient({ + definitions: [{ id: "vpc", type: "ravion-aws-vpc", name: "AWS VPC", description: "AWS VPC and subnets." }], + }); + + await publishDefinitions([createCompiledDefinition()], client, { dryRun: true }); + + assert.equal(client.validatedVersions.length, 0); + }); + it("fails with structured conflict details when an existing version has different config", async () => { const remoteConfig = { inputs: [ @@ -366,6 +564,49 @@ describe("publish", () => { ]); }); + it("sends HTTP module-version validation as a dry-run request and requires HTTP 202", async () => { + const calls: Array<{ url: string; method: string; body?: unknown }> = []; + const originalFetch = globalThis.fetch; + globalThis.fetch = async (url, init) => { + calls.push({ url: String(url), method: init?.method ?? "GET", body: init?.body ? JSON.parse(String(init.body)) : undefined }); + return jsonResponse({}, 202); + }; + + try { + const client = await createDefaultRavionApiClient({ baseUrl: "https://api.example.test", token: "token" }); + await client.validateModuleVersion({ + moduleDefinitionId: "vpc", + version: "1.2.3", + description: "Add subnet options.", + config: { inputs: [] }, + }); + + globalThis.fetch = async () => jsonResponse({}, 200); + await assert.rejects( + () => client.validateModuleVersion({ moduleDefinitionId: "vpc", version: "1.2.3", description: "Add subnet options.", config: {} }), + /returned HTTP 200 instead of 202/, + ); + } finally { + globalThis.fetch = originalFetch; + } + + assert.deepEqual(calls, [ + { + url: "https://api.example.test/module-versions", + method: "POST", + body: { + data: { + moduleDefinitionId: "vpc", + version: "1.2.3", + description: "Add subnet options.", + config: { inputs: [] }, + dryRun: true, + }, + }, + }, + ]); + }); + it("includes REST error response details", async () => { const originalFetch = globalThis.fetch; globalThis.fetch = async () => jsonResponse({ code: "Ravion:Auth:FORBIDDEN", message: "Forbidden", description: "Missing scope", requestId: "req_123" }, 403); @@ -439,8 +680,10 @@ class MockRavionClient implements RavionModuleApiClient { createdDefinitions: ModuleDefinitionInput[] = []; patchedDefinitions: ModuleDefinitionPatchInput[] = []; createdVersions: ModuleVersionInput[] = []; + validatedVersions: ModuleVersionInput[] = []; onListModuleVersions?: (moduleDefinitionId: string) => Promise; onCreateVersion?: (input: ModuleVersionInput) => Promise; + onValidateVersion?: (input: ModuleVersionInput) => Promise; constructor(options: { definitions?: RemoteModuleDefinition[]; versionsByDefinitionId?: Record } = {}) { this.definitions = options.definitions ?? []; @@ -485,4 +728,11 @@ class MockRavionClient implements RavionModuleApiClient { this.versionsByDefinitionId[input.moduleDefinitionId] = [...(this.versionsByDefinitionId[input.moduleDefinitionId] ?? []), version]; return version; } + + async validateModuleVersion(input: ModuleVersionInput): Promise { + if (this.onValidateVersion) { + await this.onValidateVersion(input); + } + this.validatedVersions.push(input); + } } diff --git a/tools/ravion-modules/test/workflows.test.ts b/tools/ravion-modules/test/workflows.test.ts index 4b241562..c29133ad 100644 --- a/tools/ravion-modules/test/workflows.test.ts +++ b/tools/ravion-modules/test/workflows.test.ts @@ -22,7 +22,8 @@ test("module definition workflow has valid syntax and expected jobs", async () = assert.deepEqual(jobs.publish.permissions, { contents: "write" }); assert.ok(jobs.validate.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js guardrails")); assert.ok(jobs.validate.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js compile")); - assert.ok(jobs["publish-plan"].steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js publish --format markdown --output publish-plan.md 2> publish-plan.err")); + assert.ok(jobs["publish-plan"].steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js publish --validate-remote --format markdown --output publish-plan.md 2> publish-plan.err")); + assert.ok(jobs.publish.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js publish --validate-remote")); assert.ok(jobs.publish.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js tags --api --create --overwrite > release-tags.json")); assert.ok(jobs.publish.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js push-tags --plan release-tags.json")); assert.ok(jobs.publish.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js publish --apply"));