diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index 4717d95f..4f59fc56 100644 --- a/src/services/transitive-extractor.ts +++ b/src/services/transitive-extractor.ts @@ -4,6 +4,7 @@ import type { IApimClient } from '../clients/iapim-client.js'; import type { IArtifactStore } from '../clients/iartifact-store.js'; import type { ApimServiceContext, ResourceDescriptor } from '../models/types.js'; +import { ResourceType } from '../models/resource-types.js'; import { getResourceDescriptorKey } from '../lib/resource-path.js'; import { buildResourceLabel } from '../lib/resource-uri.js'; import { logger } from '../lib/logger.js'; @@ -23,6 +24,69 @@ export interface TransitiveExtractionResult { errorCount: number; } +interface TransitiveTaskResult { + dep: ResourceDescriptor; + json?: Record; + /** Resolved to a resource already extracted under its real name. */ + skipped?: boolean; +} + +/** + * Per-context (keyed by baseUrl) index of named value displayName → resource name. + * Stored as promises so concurrent tasks share a single LIST call. + */ +type NamedValueNameIndex = Map>>; + +async function loadNamedValueNames( + client: IApimClient, + context: ApimServiceContext +): Promise> { + const byDisplayName = new Map(); + try { + for await (const json of client.listResources(context, ResourceType.NamedValue)) { + const name = json.name; + const displayName = (json.properties as Record | undefined)?.displayName; + if (typeof name === 'string' && typeof displayName === 'string') { + byDisplayName.set(displayName, name); + } + } + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.warn( + `Failed to list named values for display name resolution; ` + + `falling back to resolving {{references}} as resource names: ${message}` + ); + } + return byDisplayName; +} + +/** + * Policies reference named values as {{displayName}}, but the ARM GET endpoint + * is keyed by resource name. Map the reference to the resource name when they + * differ; otherwise return the descriptor unchanged. + */ +async function resolveNamedValueDescriptor( + client: IApimClient, + context: ApimServiceContext, + dep: ResourceDescriptor, + index: NamedValueNameIndex +): Promise { + let names = index.get(context.baseUrl); + if (!names) { + names = loadNamedValueNames(client, context); + index.set(context.baseUrl, names); + } + + const ref = dep.nameParts[0] ?? ''; + const name = (await names).get(ref); + if (!name || name === ref) { + return dep; + } + + logger.debug(`Resolved named value reference "{{${ref}}}" to resource name "${name}"`); + return { ...dep, nameParts: [name] }; +} + export async function extractTransitiveDependencies( client: IApimClient, store: IArtifactStore, @@ -36,7 +100,9 @@ export async function extractTransitiveDependencies( serviceContext?: ApimServiceContext ): Promise { const attempted = new Set(alreadyExtracted.map(getResourceDescriptorKey)); + const claimedNamedValues = new Set(alreadyExtracted.map(getResourceDescriptorKey)); const extractedDescriptors: ResourceDescriptor[] = []; + const namedValueIndex: NamedValueNameIndex = new Map(); let errorCount = 0; let foundDependencies = false; @@ -61,17 +127,27 @@ export async function extractTransitiveDependencies( attempted.add(getResourceDescriptorKey(dep)); } - const tasks = newDeps.map((dep) => async () => { + const tasks = newDeps.map((dep) => async (): Promise => { try { const dependencyContext = serviceContext && dep.workspace !== workspace ? serviceContext : context; - const json = await client.getResource(dependencyContext, dep); + let target = dep; + if (dep.type === ResourceType.NamedValue) { + target = await resolveNamedValueDescriptor(client, dependencyContext, dep, namedValueIndex); + const targetKey = getResourceDescriptorKey(target); + if (claimedNamedValues.has(targetKey)) { + return { dep: target, skipped: true }; + } + claimedNamedValues.add(targetKey); + } + const json = await client.getResource(dependencyContext, target); if (json) { - const safeJson = redactSecrets(dep, json); - await store.writeResource(outputDir, dep, safeJson); - logger.info(`Extracted transitive dependency ${buildResourceLabel(dep)}`); - return { dep, json: safeJson }; + const safeJson = redactSecrets(target, json); + await store.writeResource(outputDir, target, safeJson); + logger.info(`Extracted transitive dependency ${buildResourceLabel(target)}`); + return { dep: target, json: safeJson }; } + logger.warn(`Transitive dependency ${buildResourceLabel(target)} was not found`); } catch (error) { const message = error instanceof Error ? error.message : String(error); logger.warn( @@ -84,6 +160,9 @@ export async function extractTransitiveDependencies( const taskResults = await runParallel(tasks, DEFAULT_CONCURRENCY); for (const taskResult of taskResults) { const value = taskResult.status === 'fulfilled' ? taskResult.value : undefined; + if (value?.skipped) { + continue; + } if (!value?.json) { errorCount++; continue; diff --git a/tests/unit/services/transitive-extractor.test.ts b/tests/unit/services/transitive-extractor.test.ts new file mode 100644 index 00000000..31ead3d4 --- /dev/null +++ b/tests/unit/services/transitive-extractor.test.ts @@ -0,0 +1,247 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +/** + * Unit tests for transitive dependency extraction + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ResourceType } from '../../../src/models/resource-types.js'; +import { ApimServiceContext, ResourceDescriptor } from '../../../src/models/types.js'; +import { extractTransitiveDependencies } from '../../../src/services/transitive-extractor.js'; + +const testContext: ApimServiceContext = { + subscriptionId: 'sub-1', + resourceGroup: 'rg-1', + serviceName: 'apim-1', + apiVersion: '2024-05-01', + baseUrl: 'https://management.azure.com/subscriptions/sub-1/resourceGroups/rg-1/providers/Microsoft.ApiManagement/service/apim-1', +}; + +function createMockClient() { + return { + listResources: vi.fn(async function* (_ctx: ApimServiceContext, _type: ResourceType): AsyncGenerator> {}), + getResource: vi.fn().mockResolvedValue(undefined), + putResource: vi.fn(), + patchResource: vi.fn(), + deleteResource: vi.fn(), + listApiRevisions: async function* () {}, + getApiSpecification: vi.fn().mockResolvedValue(undefined), + validatePreFlight: vi.fn().mockResolvedValue(undefined), + }; +} + +function createMockStore() { + return { + writeResource: vi.fn().mockResolvedValue(undefined), + writeContent: vi.fn().mockResolvedValue(undefined), + writeAssociation: vi.fn().mockResolvedValue(undefined), + readResource: vi.fn().mockResolvedValue(undefined), + readContent: vi.fn().mockResolvedValue(undefined), + readAssociation: vi.fn().mockResolvedValue([]), + listResources: vi.fn().mockResolvedValue([]), + deleteResource: vi.fn().mockResolvedValue(undefined), + }; +} + +function policiesWith(...refs: string[]): Map { + const xml = refs.map((ref) => ``).join(''); + return new Map([['api-1', `${xml}`]]); +} + +function namedValueList(...entries: Array<{ name: string; displayName: string }>) { + return vi.fn(async function* (_ctx: ApimServiceContext, type: ResourceType) { + if (type === ResourceType.NamedValue) { + for (const entry of entries) { + yield { name: entry.name, properties: { displayName: entry.displayName } }; + } + } + }); +} + +describe('transitive-extractor', () => { + describe('named value displayName resolution (#291)', () => { + it('should resolve {{displayName}} to the resource name and extract under that name', async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'nv-resource', displayName: 'nv_display' }); + client.getResource.mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => { + if (descriptor.type === ResourceType.NamedValue && descriptor.nameParts[0] === 'nv-resource') { + return { name: 'nv-resource', properties: { displayName: 'nv_display', secret: false, value: 'x' } }; + } + return undefined; + }); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('nv_display'), new Map(), [], [] + ); + + expect(result.errorCount).toBe(0); + expect(result.extractedDescriptors).toEqual([ + expect.objectContaining({ type: ResourceType.NamedValue, nameParts: ['nv-resource'] }), + ]); + expect(client.getResource).not.toHaveBeenCalledWith( + testContext, + expect.objectContaining({ nameParts: ['nv_display'] }) + ); + expect(store.writeResource).toHaveBeenCalledWith( + '/output', + expect.objectContaining({ type: ResourceType.NamedValue, nameParts: ['nv-resource'] }), + expect.objectContaining({ name: 'nv-resource' }) + ); + }); + + it('should fall back to the reference as resource name when no displayName matches', async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'other', displayName: 'other' }); + client.getResource.mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => { + if (descriptor.type === ResourceType.NamedValue && descriptor.nameParts[0] === 'plain-ref') { + return { name: 'plain-ref', properties: { displayName: 'plain-ref' } }; + } + return undefined; + }); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('plain-ref'), new Map(), [], [] + ); + + expect(result.errorCount).toBe(0); + expect(result.extractedDescriptors).toEqual([ + expect.objectContaining({ type: ResourceType.NamedValue, nameParts: ['plain-ref'] }), + ]); + }); + + it('should list named values once per context across multiple references', async () => { + const client = createMockClient(); + client.listResources = namedValueList( + { name: 'nv-a', displayName: 'A' }, + { name: 'nv-b', displayName: 'B' }, + { name: 'nv-c', displayName: 'C' }, + ); + client.getResource.mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => ({ + name: descriptor.nameParts[0], + properties: {}, + })); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('A', 'B', 'C'), new Map(), [], [] + ); + + expect(result.errorCount).toBe(0); + expect(result.extractedDescriptors.map((d) => d.nameParts[0]).sort()).toEqual(['nv-a', 'nv-b', 'nv-c']); + const namedValueLists = client.listResources.mock.calls.filter( + ([, type]) => type === ResourceType.NamedValue + ); + expect(namedValueLists).toHaveLength(1); + }); + + it('should extract each resolved resource when a resource name is also another display name', async () => { + const client = createMockClient(); + client.listResources = namedValueList( + { name: 'one', displayName: 'two' }, + { name: 'two', displayName: 'three' }, + ); + client.getResource.mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => ({ + name: descriptor.nameParts[0], + properties: {}, + })); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('two', 'three'), new Map(), [], [] + ); + + expect(result.errorCount).toBe(0); + expect(result.extractedDescriptors.map((descriptor) => descriptor.nameParts[0]).sort()).toEqual([ + 'one', + 'two', + ]); + expect(client.getResource).toHaveBeenCalledTimes(2); + }); + + it('should not re-extract or count an error when the resolved name was already extracted', async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'nv-resource', displayName: 'nv_display' }); + const store = createMockStore(); + const alreadyExtracted: ResourceDescriptor[] = [ + { type: ResourceType.NamedValue, nameParts: ['nv-resource'] }, + ]; + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('nv_display'), new Map(), [], alreadyExtracted + ); + + expect(result.errorCount).toBe(0); + expect(result.extractedDescriptors).toEqual([]); + expect(client.getResource).not.toHaveBeenCalled(); + expect(store.writeResource).not.toHaveBeenCalled(); + }); + + it('should fall back to the reference when listing named values fails', async () => { + const client = createMockClient(); + // eslint-disable-next-line require-yield + client.listResources = vi.fn(async function* (): AsyncGenerator> { + throw new Error('403 Forbidden'); + }); + client.getResource.mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => { + if (descriptor.nameParts[0] === 'nv_display') { + return { name: 'nv_display', properties: {} }; + } + return undefined; + }); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('nv_display'), new Map(), [], [] + ); + + expect(result.errorCount).toBe(0); + expect(result.extractedDescriptors).toEqual([ + expect.objectContaining({ type: ResourceType.NamedValue, nameParts: ['nv_display'] }), + ]); + }); + + it('should resolve workspace-scoped references against the workspace context', async () => { + const wsContext: ApimServiceContext = { + ...testContext, + baseUrl: `${testContext.baseUrl}/workspaces/team-a`, + }; + const client = createMockClient(); + client.listResources = vi.fn(async function* (ctx: ApimServiceContext, type: ResourceType) { + if (type === ResourceType.NamedValue && ctx.baseUrl === wsContext.baseUrl) { + yield { name: 'ws-nv', properties: { displayName: 'ws_display' } }; + } + }); + client.getResource.mockImplementation(async (ctx: ApimServiceContext, descriptor: ResourceDescriptor) => { + if (ctx.baseUrl === wsContext.baseUrl && descriptor.nameParts[0] === 'ws-nv') { + return { name: 'ws-nv', properties: {} }; + } + return undefined; + }); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, wsContext, '/output', policiesWith('ws_display'), new Map(), [], [], 'team-a', testContext + ); + + expect(result.errorCount).toBe(0); + expect(result.extractedDescriptors).toEqual([ + expect.objectContaining({ type: ResourceType.NamedValue, nameParts: ['ws-nv'], workspace: 'team-a' }), + ]); + expect(client.listResources).not.toHaveBeenCalledWith(testContext, ResourceType.NamedValue); + }); + + it('should count a missing dependency as an error', async () => { + const client = createMockClient(); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('missing'), new Map(), [], [] + ); + + expect(result.errorCount).toBe(1); + expect(result.extractedDescriptors).toEqual([]); + }); + }); +});