From fb829724bacbb9de0e7a66bf4b75c067ddea1259 Mon Sep 17 00:00:00 2001 From: Alexey Zheltov Date: Fri, 9 Oct 2026 19:27:35 +0000 Subject: [PATCH 1/2] fix(extract): resolve {{displayName}} named value references to resource names Policies reference named values by displayName ({{displayName}}), but the ARM GET endpoint is keyed by resource name. When the two differ, transitive extraction issued GET namedValues/, got a silent 404, counted an error and never wrote the artifact. extractTransitiveDependencies now builds a per-context displayName -> name index (one LIST per context, shared across concurrent tasks) and resolves each named value reference through it before the GET. References that do not match any displayName still fall back to the reference as the resource name, so existing behaviour is unchanged for name == displayName. A reference that resolves to an already-extracted resource is skipped instead of being re-extracted or counted as an error, and a 404 on a transitive dependency now logs a warning instead of failing silently. Verified on a live APIM instance: a named value with name != displayName referenced from an API policy was previously reported as "1 errors" with no artifact; it is now extracted under its resource name with 0 errors. Closes #291 --- src/services/transitive-extractor.ts | 92 +++++++- .../services/transitive-extractor.test.ts | 223 ++++++++++++++++++ 2 files changed, 309 insertions(+), 6 deletions(-) create mode 100644 tests/unit/services/transitive-extractor.test.ts diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index 4717d95f..70d1f819 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, @@ -37,6 +101,7 @@ export async function extractTransitiveDependencies( ): Promise { const attempted = new Set(alreadyExtracted.map(getResourceDescriptorKey)); const extractedDescriptors: ResourceDescriptor[] = []; + const namedValueIndex: NamedValueNameIndex = new Map(); let errorCount = 0; let foundDependencies = false; @@ -61,17 +126,29 @@ 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); + if (target !== dep) { + const targetKey = getResourceDescriptorKey(target); + if (attempted.has(targetKey)) { + return { dep: target, skipped: true }; + } + attempted.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 +161,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..8b9f9204 --- /dev/null +++ b/tests/unit/services/transitive-extractor.test.ts @@ -0,0 +1,223 @@ +// 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 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([]); + }); + }); +}); From cbdf72e34ef703f36ef55de13681c2e2559cbfc9 Mon Sep 17 00:00:00 2001 From: Alexander Zaslonov Date: Fri, 9 Oct 2026 16:04:17 -0700 Subject: [PATCH 2/2] fix: separate named value target claims Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/services/transitive-extractor.ts | 11 ++++----- .../services/transitive-extractor.test.ts | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index 70d1f819..4f59fc56 100644 --- a/src/services/transitive-extractor.ts +++ b/src/services/transitive-extractor.ts @@ -100,6 +100,7 @@ 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; @@ -133,13 +134,11 @@ export async function extractTransitiveDependencies( let target = dep; if (dep.type === ResourceType.NamedValue) { target = await resolveNamedValueDescriptor(client, dependencyContext, dep, namedValueIndex); - if (target !== dep) { - const targetKey = getResourceDescriptorKey(target); - if (attempted.has(targetKey)) { - return { dep: target, skipped: true }; - } - attempted.add(targetKey); + 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) { diff --git a/tests/unit/services/transitive-extractor.test.ts b/tests/unit/services/transitive-extractor.test.ts index 8b9f9204..31ead3d4 100644 --- a/tests/unit/services/transitive-extractor.test.ts +++ b/tests/unit/services/transitive-extractor.test.ts @@ -136,6 +136,30 @@ describe('transitive-extractor', () => { 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' });