diff --git a/src/clients/apim-client.ts b/src/clients/apim-client.ts index 9166cf0..15a4ca3 100644 --- a/src/clients/apim-client.ts +++ b/src/clients/apim-client.ts @@ -42,6 +42,17 @@ export class ApimClient implements IApimClient { private static readonly MAX_DELAY_MS = 30000; private static readonly MAX_POLLING_ATTEMPTS = 30; private static readonly POLL_INTERVAL_MS = 2000; + /** Deadline for async (LRO) operation polling — 7.5 minutes. */ + private static readonly ASYNC_POLL_TIMEOUT_MS = 7.5 * 60 * 1000; + /** Default interval between async operation polls when no Retry-After header. */ + private static readonly ASYNC_POLL_INTERVAL_MS = 5000; + /** Known ARM management plane host suffixes for URL validation. */ + private static readonly ARM_HOSTS = [ + 'management.azure.com', + 'management.chinacloudapi.cn', + 'management.usgovcloudapi.net', + 'management.microsoftazure.de', + ]; /** Stable ARM API version for Resource Group existence checks */ private static readonly RESOURCE_GROUP_API_VERSION = '2021-04-01'; @@ -291,6 +302,14 @@ export class ApimClient implements IApimClient { context: ApimServiceContext, descriptor: ResourceDescriptor ): Promise | undefined> { + // Some association resources (ProductGroup, ProductApi, GatewayApi) only + // support PUT/DELETE. Short-circuit before making a network call. + const metadata = RESOURCE_TYPE_METADATA[descriptor.type]; + if (!metadata.supportsGet) { + logger.debug(`Resource type ${descriptor.type} does not support GET, returning undefined`); + return undefined; + } + const url = buildArmUri(context, descriptor); // Azure APIM returns HTTP 500 (not 404) when an API or product has no wiki. // Suppress retries for wiki types so the extractor silently skips them. @@ -353,6 +372,15 @@ export class ApimClient implements IApimClient { logger.debug(`Skipping provisioning poll for association resource: ${buildResourceLabel(descriptor)}`); return {}; } + + // Prefer ARM async operation polling when the service provides an + // Azure-AsyncOperation or Location header — these long-running operations + // (e.g. large API spec imports) may take minutes to complete. + const asyncUrl = this.extractAsyncOperationUrl(response); + if (asyncUrl) { + return await this.pollAsyncOperation(asyncUrl, context, descriptor); + } + return await this.pollProvisioningState(context, descriptor); } @@ -414,9 +442,14 @@ export class ApimClient implements IApimClient { // Poll for long-running operations if (response.status === 202) { - await this.pollProvisioningState(context, descriptor, { - treatMissingAsSuccess: true, - }); + const asyncUrl = this.extractAsyncOperationUrl(response); + if (asyncUrl) { + await this.pollAsyncOperation(asyncUrl, context, descriptor, { treatMissingAsSuccess: true }); + } else { + await this.pollProvisioningState(context, descriptor, { + treatMissingAsSuccess: true, + }); + } } return true; @@ -719,6 +752,132 @@ export class ApimClient implements IApimClient { return 'yaml'; } + /** + * Extract the async operation URL from ARM LRO response headers. + * Prefers Azure-AsyncOperation over Location. + * Validates the URL points to a known ARM management endpoint to prevent + * leaking the bearer token to an unexpected host. + */ + private extractAsyncOperationUrl(response: Response): string | undefined { + const asyncOpUrl = response.headers.get('Azure-AsyncOperation') + ?? response.headers.get('Operation-Location') + ?? response.headers.get('Location'); + + if (!asyncOpUrl) return undefined; + + // Validate URL host is a known ARM management endpoint + try { + const parsed = new URL(asyncOpUrl); + const isArmHost = ApimClient.ARM_HOSTS.some( + (host) => parsed.hostname === host || parsed.hostname.endsWith(`.${host}`) + ); + if (!isArmHost) { + logger.warn( + `Ignoring async operation URL with unexpected host: ${parsed.hostname}` + ); + return undefined; + } + } catch { + logger.warn(`Ignoring malformed async operation URL: ${asyncOpUrl}`); + return undefined; + } + + return asyncOpUrl; + } + + /** + * Poll an ARM async operation URL until terminal state. + * Used for long-running operations like large API spec imports. + * + * The operation status endpoint returns: + * { "status": "InProgress" | "Succeeded" | "Failed" | "Canceled", ... } + * + * On success, GETs the original resource to return the final state. + */ + private async pollAsyncOperation( + operationUrl: string, + context: ApimServiceContext, + descriptor: ResourceDescriptor, + options: { treatMissingAsSuccess?: boolean } = {} + ): Promise> { + const { treatMissingAsSuccess = false } = options; + const label = buildResourceLabel(descriptor); + const deadline = Date.now() + ApimClient.ASYNC_POLL_TIMEOUT_MS; + let pollInterval = ApimClient.ASYNC_POLL_INTERVAL_MS; + + logger.debug(`Polling async operation for ${label}: ${operationUrl}`); + + while (Date.now() < deadline) { + await this.delay(pollInterval); + + const response = await this.request(operationUrl); + + // Honour Retry-After if the service provides it + const retryAfter = response.headers.get('Retry-After'); + if (retryAfter) { + const parsed = parseInt(retryAfter, 10); + if (!isNaN(parsed) && parsed > 0) { + pollInterval = parsed * 1000; + } + } + + // Some Location-style polls respond with 202 (still in progress) + // and may not have a JSON body — keep polling. + if (response.status === 202) { + logger.debug(`Async operation still in progress (HTTP 202) for ${label}`); + continue; + } + + // Try to parse the status body + const text = await response.text(); + if (!text.trim()) { + // Empty body on 200/204 — treat as complete + if (response.ok) { + return treatMissingAsSuccess + ? {} + : await this.getResource(context, descriptor) ?? {}; + } + throw new Error(`Async operation returned empty body with status ${response.status} for ${label}`); + } + + let body: Record; + try { + body = JSON.parse(text) as Record; + } catch { + // Non-JSON on an OK status — treat as complete + if (response.ok) { + return treatMissingAsSuccess + ? {} + : await this.getResource(context, descriptor) ?? {}; + } + throw new Error(`Async operation returned non-JSON with status ${response.status} for ${label}`); + } + + const status = (body.status as string | undefined)?.toLowerCase(); + + if (status === 'succeeded') { + logger.debug(`Async operation succeeded for ${label}`); + if (treatMissingAsSuccess) return {}; + // GET the final resource state + return await this.getResource(context, descriptor) ?? {}; + } + + if (status === 'failed' || status === 'canceled' || status === 'cancelled') { + const error = body.error as Record | undefined; + const code = (error?.code as string) ?? 'UnknownError'; + const message = (error?.message as string) ?? JSON.stringify(body); + throw new Error(`Async operation ${status} for ${label}: [${code}] ${message}`); + } + + // InProgress or other intermediate status — keep polling + logger.debug(`Async operation status: ${status ?? 'unknown'} for ${label}`); + } + + throw new Error( + `Async operation polling timed out after ${ApimClient.ASYNC_POLL_TIMEOUT_MS / 1000}s for ${label}` + ); + } + private async pollProvisioningState( context: ApimServiceContext, descriptor: ResourceDescriptor, diff --git a/src/services/api-publisher.ts b/src/services/api-publisher.ts index e7a23c3..c696743 100644 --- a/src/services/api-publisher.ts +++ b/src/services/api-publisher.ts @@ -200,13 +200,16 @@ async function publishRootApi( cleanProps.apiType = apiType; } + // Sanitize the spec before import (e.g. inject missing path parameters) + const sanitizedContent = sanitizeOpenApiSpec(specResult.content, specResult.format, descriptor); + // Inject the spec into the PUT body for APIM to import json = { ...json, properties: { ...cleanProps, format: importFormat, - value: specResult.content, + value: sanitizedContent, }, }; @@ -557,3 +560,84 @@ function getOpenApiOperationIdsWithNullDescription( return []; } } + +/** + * Sanitize an OpenAPI spec before importing into APIM. + * Currently handles: + * - Missing path parameter declarations: APIM rejects specs where a URL + * template contains `{param}` placeholders that are not declared in the + * operation's `parameters` array. We auto-inject the missing declarations + * as `{ name, in: "path", required: true, schema: { type: "string" } }` + * and log a warning for each one. + */ +function sanitizeOpenApiSpec( + content: string, + format: string | undefined, + descriptor: ResourceDescriptor +): string { + // Only process JSON-based OpenAPI specs (openapi+json, json) + const isJson = + !format || + format.toLowerCase().includes('json') || + content.trimStart().startsWith('{'); + if (!isJson) return content; + + let spec: Record; + try { + spec = JSON.parse(content) as Record; + } catch { + return content; // Not valid JSON — return as-is + } + + const paths = spec.paths as Record> | undefined; + if (!paths) return content; + + const apiLabel = getNamePart(descriptor.nameParts, 0); + let modified = false; + + for (const [pathTemplate, pathItem] of Object.entries(paths)) { + // Extract placeholders from the path template + const placeholders = pathTemplate.match(/\{([^}]+)\}/g); + if (!placeholders) continue; + const placeholderNames = placeholders.map((p) => p.slice(1, -1)); + + for (const [method, operation] of Object.entries(pathItem)) { + // Skip non-operation keys (e.g. "summary", "parameters", "servers") + if (typeof operation !== 'object' || operation === null) continue; + if (!['get', 'put', 'post', 'delete', 'patch', 'head', 'options', 'trace'].includes(method)) continue; + + const op = operation as Record; + const params = (op.parameters ?? []) as Array>; + const declaredPathParams = new Set( + params + .filter((p) => p.in === 'path') + .map((p) => p.name as string) + ); + + for (const name of placeholderNames) { + if (!declaredPathParams.has(name)) { + // Inject the missing parameter declaration + const newParam: Record = { + name, + in: 'path', + required: true, + schema: { type: 'string' }, + }; + const updatedParams = [...params, newParam]; + op.parameters = updatedParams; + // Update params reference for subsequent placeholder checks on this operation + params.push(newParam); + declaredPathParams.add(name); + modified = true; + + const opSummary = (op.summary ?? op.operationId ?? `${method.toUpperCase()} ${pathTemplate}`) as string; + logger.warn( + `API "${apiLabel}": auto-injected missing path parameter "{${name}}" for operation "${opSummary}"` + ); + } + } + } + } + + return modified ? JSON.stringify(spec) : content; +} diff --git a/tests/integration/package-build/package-build.test.ts b/tests/integration/package-build/package-build.test.ts index da1c20d..06071e1 100644 --- a/tests/integration/package-build/package-build.test.ts +++ b/tests/integration/package-build/package-build.test.ts @@ -2,7 +2,7 @@ // Licensed under the MIT license. /// -import { execFile } from 'node:child_process'; +import { exec as execCb } from 'node:child_process'; import * as fs from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -10,8 +10,7 @@ import { fileURLToPath } from 'node:url'; import { promisify } from 'node:util'; import { describe, expect, it } from 'vitest'; -const exec = promisify(execFile); -const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm'; +const exec = promisify(execCb); const currentFileDir = path.dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(currentFileDir, '../../..'); @@ -33,10 +32,14 @@ function normalizePath(filePath: string): string { } async function runNpm(args: string[]): Promise { - const result = await exec(npmCommand, args, { + // Use 'npm' with shell so .cmd resolution works on Windows and + // plain 'npm' works on Linux/macOS. Pass args as a single joined string + // to avoid the Node.js v24 DEP0190 deprecation warning about passing + // array args with shell option. + const command = `npm ${args.join(' ')}`; + const result = await exec(command, { cwd: repoRoot, timeout: 90_000, - env: process.env, }); return result.stdout;