diff --git a/src/clients/apim-client.ts b/src/clients/apim-client.ts index abd2622b..09f4d6bb 100644 --- a/src/clients/apim-client.ts +++ b/src/clients/apim-client.ts @@ -383,27 +383,23 @@ export class ApimClient implements IApimClient { body: JSON.stringify(payload), }); - // Poll for long-running operations (201/202 responses). + // Poll for long-running operations signaled by ARM response headers, including + // updates that return 200 while provisioning continues asynchronously. // Skip polling for association resources that don't support GET (supportsGet: false in metadata). - // Check status BEFORE reading the body so the body stream is not consumed - // unnecessarily — and to avoid JSON-parsing failures when the 201/202 body - // is XML (e.g. policy endpoints that echo back raw XML on creation). + const metadata = RESOURCE_TYPE_METADATA[descriptor.type]; + const asyncUrl = this.extractAsyncOperationUrl(response, response.status !== 200); + if (asyncUrl && metadata.supportsGet) { + return await this.pollAsyncOperation(asyncUrl, context, descriptor); + } + + // Headerless creates may still provision asynchronously. Check status before + // reading the body to avoid consuming XML returned by policy endpoints. if (response.status === 201 || response.status === 202) { - const metadata = RESOURCE_TYPE_METADATA[descriptor.type]; if (!metadata.supportsGet) { - // Association resources don't support GET - return empty on success 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); } @@ -788,16 +784,23 @@ export class ApimClient implements IApimClient { * 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 { + private extractAsyncOperationUrl( + response: Response, + includeLocation = true + ): string | undefined { const asyncOpUrl = response.headers.get('Azure-AsyncOperation') ?? response.headers.get('Operation-Location') - ?? response.headers.get('Location'); + ?? (includeLocation ? response.headers.get('Location') : undefined); if (!asyncOpUrl) return undefined; // Validate URL host is a known ARM management endpoint try { const parsed = new URL(asyncOpUrl); + if (parsed.protocol !== 'https:') { + logger.warn(`Ignoring non-HTTPS async operation URL: ${asyncOpUrl}`); + return undefined; + } const isArmHost = ApimClient.ARM_HOSTS.some( (host) => parsed.hostname === host || parsed.hostname.endsWith(`.${host}`) ); diff --git a/tests/unit/clients/apim-client.test.ts b/tests/unit/clients/apim-client.test.ts index 50832269..9864fac5 100644 --- a/tests/unit/clients/apim-client.test.ts +++ b/tests/unit/clients/apim-client.test.ts @@ -507,6 +507,103 @@ describe('ApimClient.putResource provisioning polling', () => { const descriptor = { type: ResourceType.NamedValue, nameParts: ['my-nv'] }; const succeededResource = { name: 'my-nv', properties: { provisioningState: 'Succeeded' } }; + it('should poll Azure-AsyncOperation when an update returns 200', async () => { + const operationUrl = 'https://management.azure.com/operations/update-named-value'; + fetchSpy + .mockResolvedValueOnce( + new Response(JSON.stringify({ properties: { provisioningState: 'InProgress' } }), { + status: 200, + headers: { + 'Azure-AsyncOperation': operationUrl, + 'Content-Type': 'application/json', + }, + }) + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ status: 'Succeeded' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ) + .mockResolvedValueOnce( + new Response(JSON.stringify(succeededResource), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ); + + const result = await client.putResource(testContext, descriptor, { name: 'my-nv' }); + + expect(result).toEqual(succeededResource); + expect(fetchSpy).toHaveBeenCalledTimes(3); + expect(fetchSpy.mock.calls[1]?.[0]).toBe(operationUrl); + }); + + it('should surface a failed Azure-AsyncOperation when an update returns 200', async () => { + const operationUrl = 'https://management.azure.com/operations/update-named-value'; + fetchSpy + .mockResolvedValueOnce( + new Response(JSON.stringify({ properties: { provisioningState: 'InProgress' } }), { + status: 200, + headers: { 'Azure-AsyncOperation': operationUrl }, + }) + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ + status: 'Failed', + error: { code: 'InvalidKeyVault', message: 'The referenced Key Vault does not exist.' }, + }), { status: 200 }) + ); + + await expect(client.putResource(testContext, descriptor, { name: 'my-nv' })) + .rejects.toThrow('InvalidKeyVault'); + expect(fetchSpy).toHaveBeenCalledTimes(2); + }); + + it('should parse a normal 200 response with only a Location header without polling', async () => { + const responseBody = { name: 'my-nv', properties: { provisioningState: 'Succeeded' } }; + fetchSpy.mockResolvedValueOnce( + new Response(JSON.stringify(responseBody), { + status: 200, + headers: { Location: 'https://management.azure.com/resources/my-nv' }, + }) + ); + + await expect(client.putResource(testContext, descriptor, {})).resolves.toEqual(responseBody); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it('should not poll a non-HTTPS async operation URL', async () => { + const responseBody = { name: 'my-nv', properties: { provisioningState: 'Succeeded' } }; + fetchSpy.mockResolvedValueOnce( + new Response(JSON.stringify(responseBody), { + status: 200, + headers: { 'Azure-AsyncOperation': 'http://management.azure.com/operations/unsafe' }, + }) + ); + + await expect(client.putResource(testContext, descriptor, {})).resolves.toEqual(responseBody); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it('should not poll async headers for association resources that do not support GET', async () => { + const association = { + type: ResourceType.ProductApi, + nameParts: ['starter', 'my-api'], + }; + fetchSpy.mockResolvedValueOnce( + new Response('', { + status: 201, + headers: { + 'Azure-AsyncOperation': 'https://management.azure.com/operations/association', + }, + }) + ); + + await expect(client.putResource(testContext, association, {})).resolves.toEqual({}); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + it('should poll when PUT returns 201 and eventually return the resource', async () => { fetchSpy // Initial PUT → 201