diff --git a/.squad/agents/apimexpert/history.md b/.squad/agents/apimexpert/history.md index 8a2d744..3f319a9 100644 --- a/.squad/agents/apimexpert/history.md +++ b/.squad/agents/apimexpert/history.md @@ -106,3 +106,48 @@ the SDK surface, reference docs, or ad-hoc observation. - Classic Developer/Premium SKU only. - Docs: · +### 2026-06-02: Named Value Token Canonicalization — Broadened Scope + +**Problem:** Initial fix only canonicalized `{{namedValue}}` tokens in Logger credentials. Backend credentials also contain these tokens and fail validation when override casing differs from artifact names. + +**APIM resources supporting named value tokens:** +1. **Logger** — `properties.credentials.*` (instrumentationKey, connectionString, etc.) +2. **Backend** — `properties.credentials.authorization.parameter`, `properties.credentials.header.*`, `properties.credentials.query.*` +3. Policies/PolicyFragments use `{{tokens}}` in XML, not JSON payloads (handled separately) + +**Solution:** Generalized normalization to traverse entire JSON payload recursively, not just Logger credentials. Renamed function: `normalizeLoggerCredentialNamedValueReferences` → `normalizeNamedValueReferences`. Applied to all resource types before PUT. + +**Rationale:** +- Backend credentials affected today, not hypothetical +- Future-proof — new resource types with token support work automatically +- Smallest correct change — recursive traversal already existed, just removed Logger-specific conditional +- Preserves opaque JSON (Constitution §VII) — canonicalization safe for unknown properties + +**Evidence:** +- Azure REST API spec: Backend credentials support `{{namedValue}}` references ([azure-rest-api-specs](https://github.com/Azure/azure-rest-api-specs/tree/main/specification/apimanagement)) +- APIM documentation: Named values in Backend authorization, headers, query strings +- User feedback: "name-value pairs maybe used for other resources" + +**Tests:** Added Backend credential canonicalization test. All 910 tests pass. + +**Decision doc:** `.squad/decisions/inbox/apimexpert-named-value-scope.md` + + +### 2026-06-02: Named Value Normalization Generalization — Complete + +**Recommendation adopted:** TestEngineer's audit identified Backend resources as affected by the original Logger-only normalization. Generalized approach implemented as recommended. + +**Changes:** +- Renamed: `normalizeLoggerCredentialNamedValueReferences()` → `normalizeNamedValueReferences()` +- Scope: Now applies to all resource types (not just Logger) +- Logic: Recursive JSON traversal, entire payload canonicalization +- New test: Backend credential canonicalization (passing) + +**Verification:** +- All 910 unit tests pass +- Backend test covers credentials.header, credentials.query, credentials.authorization.parameter +- Recursive approach handles unknown properties (Constitution §VII) + +**Decision merged:** Inbox notes moved to `.squad/decisions/decisions.md` (2026-06-02 entries). + +**Future-proofing:** Any new APIM resource type with `{{token}}` references will be automatically normalized without additional code changes. diff --git a/.squad/agents/testengineer/history.md b/.squad/agents/testengineer/history.md index cb3f08e..ad67571 100644 --- a/.squad/agents/testengineer/history.md +++ b/.squad/agents/testengineer/history.md @@ -172,3 +172,53 @@ Gotchas for future PowerShell work: - `$x = if ($cond) { [List[T]]::new() }` assigns `$null` — PowerShell enumerates the empty list. Use `$x = $null; if ($cond) { $x = ... }`. - `ProcessStartInfo.StandardOutputEncoding/StandardErrorEncoding` default to OEM on Windows; force UTF-8 or `az --debug` output mangles. +### 2026-05-19: Named Value Token Normalization Coverage Audit + +**Context:** User raised concern that Logger credential token normalization (commit 4cb0fa3) might not cover other resources that also use `{{namedValue}}` tokens. + +**Audit findings:** +1. **Current implementation**: Only Logger resources have token normalization (lines 118-124 in resource-publisher.ts) +2. **Current test coverage**: One test for Logger credential normalization (line 174-223 in resource-publisher.test.ts) +3. **Gap identified**: Backend resources can also use `{{namedValue}}` tokens in `credentials.header` and `credentials.query` properties + +**Backend credentials structure from APIM REST API:** +```json +{ + "credentials": { + "header": { + "Authorization": ["{{bearer-token}}"], + "x-api-key": ["{{api-key-namedvalue}}"] + }, + "query": { ... }, + "certificate": [ ... ] + } +} +``` + +**Action taken:** +- Added skipped test `should canonicalize backend credential named value references from overrides` to document the gap +- Test currently fails: expects `{{Backend-Auth-Token}}` (canonical casing) but receives `{{backend-auth-token}}` (from override) +- Test will pass once implementation is extended to Backend resources +- Created decision inbox note at `.squad/decisions/inbox/testengineer-named-value-coverage.md` recommending P2 extension + +**Recommendation:** Generalize `normalizeLoggerCredentialNamedValueReferences()` to `normalizeCredentialNamedValueReferences()` and apply to both Logger and Backend resource types. The existing recursive logic already handles nested objects/arrays, so minimal code change required. + +**Other resource types reviewed:** +- **Diagnostic**: References loggers by ARM ID, not named value tokens +- **Policies (XML)**: Already handle `{{namedValue}}` tokens, but as opaque XML content, not JSON properties +- **ApiOperation, PolicyFragment**: May use named values in future, but not documented in current APIM REST API + +**Pattern reinforced:** When implementing normalization/canonicalization logic, audit all resource types that share similar property patterns. Skipped tests document known gaps and serve as regression coverage when gaps are closed. + +**Result:** 34 tests passing, 1 skipped test documenting Backend normalization gap. Decision note created for team review. + + +### 2026-06-02: Named Value Coverage Audit — Merged to Decisions + +**Team decision:** ApimExpert implemented the generalized approach, renaming `normalizeLoggerCredentialNamedValueReferences` → `normalizeNamedValueReferences` and applying it to all resource types (not just Logger). Recursive traversal now handles Backend credentials, future resource types, and preserves Constitution §VII (opaque JSON). + +**Test status:** Skipped test `should canonicalize backend credential named value references from overrides` now passes. All 910 tests passing. + +**Decision merged:** Inbox note moved to `.squad/decisions/decisions.md` (2026-06-02 entry). + +**Impact:** Backend resources with `{{namedValue}}` tokens in credentials now work correctly regardless of casing mismatch between override and artifact names. diff --git a/.squad/decisions/decisions.md b/.squad/decisions/decisions.md index bc00654..5d3ae12 100644 --- a/.squad/decisions/decisions.md +++ b/.squad/decisions/decisions.md @@ -67,3 +67,39 @@ All new `.ts` files in `src/` and `tests/` must include: - [Microsoft Open Source Program](https://opensource.microsoft.com/program) --- + +### 2026-06-02: Named Value Token Canonicalization — Broadened Scope +**By:** ApimExpert +**Status:** Implemented + +The initial fix for named value token canonicalization only applied to Logger resources. Backend resources also contain `{{namedValue}}` tokens in `credentials.header`, `credentials.query`, and `credentials.authorization.parameter`, and require the same casing-sensitive normalization. + +**Decision:** Generalize the normalization logic to canonicalize ALL `{{token}}` references across ANY resource type's entire JSON payload. +- Renamed function: `normalizeLoggerCredentialNamedValueReferences` → `normalizeNamedValueReferences` +- Removed Logger-specific resource type check; now applies to all resources +- Recursive traversal handles unknown properties (Constitution §VII) +- Implementation: `src/services/resource-publisher.ts` +- Tests: Backend credential test added + all 910 tests pass + +**Rationale:** Backend credentials are affected today, not hypothetical. Future-proof for any new APIM resource types that support named value tokens. + +**Evidence:** Azure APIM REST API spec shows Backend credentials support `{{namedValue}}` references in authorization parameters, headers, and query strings. User report confirmed: "Named value pairs maybe used for other resources!" + +--- + +### 2026-06-02: Extend Named Value Token Normalization to Backend Resources +**By:** TestEngineer +**Status:** Audit Complete + +Audit identified that named value token normalization only covered Logger resources, leaving Backend credentials vulnerable to APIM validation failures when override casing differs from artifact naming. + +**Findings:** +- Backend resources contain `{{namedValue}}` tokens in `credentials.header`, `credentials.query`, and `credentials.authorization.parameter` +- Example: override supplies `{{Bearer-Token}}` but artifact is named `bearer-token` → APIM rejects PUT +- Skipped test added: `should canonicalize backend credential named value references from overrides` (test code in resource-publisher.test.ts) + +**Recommendation:** Extend normalization logic to Backend resources. Generalized approach (apply to all resource types) preferred over Backend-specific implementation. + +**Priority:** P2 (Medium) — Valid production use case but workaround exists (manual casing in overrides). + +--- diff --git a/src/services/extract-service.ts b/src/services/extract-service.ts index 0167a2d..3b7a233 100644 --- a/src/services/extract-service.ts +++ b/src/services/extract-service.ts @@ -473,6 +473,7 @@ async function resolveAndExtractTransitive( // Build maps for transitive resolution const apiJsonMap = new Map>(); + const namedValueJsonMap = new Map>(); for (const apiResult of result.apiResults) { // Find the API's JSON from type results const apiTypeResults = result.typeResults.filter((r) => r.type === ResourceType.Api); @@ -485,10 +486,21 @@ async function resolveAndExtractTransitive( } } + const namedValueTypeResults = result.typeResults.filter((r) => r.type === ResourceType.NamedValue); + for (const ntr of namedValueTypeResults) { + for (const extracted of ntr.extracted) { + if (extracted.status === 'success') { + const name = getNamePart(extracted.descriptor.nameParts, 0); + namedValueJsonMap.set(name, extracted.json); + } + } + } + // Find transitive dependencies const transitiveDeps = findTransitiveDependencies( result.collectedPolicies, - apiJsonMap + apiJsonMap, + namedValueJsonMap ); // Filter out already-extracted resources diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts index da3d69e..6151fa5 100644 --- a/src/services/resource-publisher.ts +++ b/src/services/resource-publisher.ts @@ -119,6 +119,12 @@ export async function publishResource( json = normalizeApiOperationTextFields(json); } + // APIM resource payloads can reference NamedValues using {{name}} syntax. + // Logger credentials, Backend credentials, and other properties support this. + // Canonicalize all {{token}} casing to actual NamedValue artifact names so + // APIM can resolve references at publish time (APIM validation is case-sensitive). + json = await normalizeNamedValueReferences(json, store, config.sourceDir); + // For KeyVault-backed NamedValues: // 1. Strip properties.value — APIM must not receive both keyVault and value // in the same PUT body, as it causes indefinite provisioning or rejection. @@ -551,3 +557,94 @@ function isAutoGeneratedProductSubscription( return scope.includes('/products/'); } + +/** + * Normalize all named value token references across the entire resource payload. + * + * APIM resolves named value references using {{namedValueName}} tokens in various + * resource properties (Logger credentials, Backend credentials, etc.). + * If an override supplies a token with different casing, APIM may reject the PUT + * with a validation error (APIM validation is case-sensitive). + * + * This function recursively traverses the entire JSON payload and rewrites all + * {{token}} references to match the canonical NamedValue artifact names on disk. + */ +async function normalizeNamedValueReferences( + json: Record, + store: IArtifactStore, + sourceDir: string +): Promise> { + const resources = await store.listResources(sourceDir); + const namedValueNames = new Map(); + for (const descriptor of resources) { + if (descriptor.type === ResourceType.NamedValue) { + const name = getNamePart(descriptor.nameParts, 0); + namedValueNames.set(name.toLowerCase(), name); + } + } + + if (namedValueNames.size === 0) { + return json; + } + + const normalizeTokenString = (value: string): string => + value.replace(/\{\{\s*([^}]+?)\s*\}\}/g, (match, tokenName: string) => { + const canonicalName = namedValueNames.get(tokenName.toLowerCase()); + return canonicalName ? `{{${canonicalName}}}` : match; + }); + + const normalizedRoot: Record = {}; + const stack: Array<{ source: unknown; assign: (value: unknown) => void }> = []; + + for (const [key, value] of Object.entries(json)) { + stack.push({ + source: value, + assign: (normalizedValue: unknown) => { + normalizedRoot[key] = normalizedValue; + }, + }); + } + + while (stack.length > 0) { + const frame = stack.pop()!; + const { source, assign } = frame; + + if (typeof source === 'string') { + assign(normalizeTokenString(source)); + continue; + } + + if (Array.isArray(source)) { + const out: unknown[] = new Array(source.length); + assign(out); + // Push in reverse so LIFO processing assigns array elements in natural order. + for (let i = source.length - 1; i >= 0; i--) { + stack.push({ + source: source[i], + assign: (normalizedValue: unknown) => { + out[i] = normalizedValue; + }, + }); + } + continue; + } + + if (source && typeof source === 'object') { + const out: Record = {}; + assign(out); + for (const [key, child] of Object.entries(source as Record)) { + stack.push({ + source: child, + assign: (normalizedValue: unknown) => { + out[key] = normalizedValue; + }, + }); + } + continue; + } + + assign(source); + } + + return normalizedRoot; +} diff --git a/src/services/transitive-resolver.ts b/src/services/transitive-resolver.ts index 4abab24..35974b5 100644 --- a/src/services/transitive-resolver.ts +++ b/src/services/transitive-resolver.ts @@ -113,7 +113,8 @@ export function scanApiVersionSetReference( export function resolveTransitiveDependencies( extractedPolicies: Map, extractedApis: Map>, - currentFilter: FilterConfig + currentFilter: FilterConfig, + extractedNamedValues: Map> = new Map() ): FilterConfig { const expanded = { ...currentFilter }; let changed = true; @@ -142,6 +143,23 @@ export function resolveTransitiveDependencies( changed = true; } } + + // Scan extracted named values for references to other named values. + for (const [name, namedValueJson] of extractedNamedValues) { + const currentNamedValues = expanded.namedValueNames; + if (currentNamedValues !== undefined) { + const isIncluded = currentNamedValues.some((n) => n.toLowerCase() === name.toLowerCase()); + if (!isIncluded) { + continue; + } + } + + for (const ref of scanNamedValueReferences(namedValueJson)) { + if (addToFilter(expanded, ref)) { + changed = true; + } + } + } } if (iterations > 1) { @@ -195,18 +213,35 @@ function addToFilter( */ export function findTransitiveDependencies( policies: Map, - apis: Map> + apis: Map>, + namedValues: Map> = new Map() ): ResourceDescriptor[] { const dependencies: ResourceDescriptor[] = []; const seen = new Set(); + const namedValueKeyToName = new Map(); + + const addDependency = (dep: TransitiveDependency): boolean => { + const key = `${dep.type}:${dep.name.toLowerCase()}`; + if (seen.has(key)) { + return false; + } + seen.add(key); + dependencies.push({ type: dep.type, nameParts: [dep.name] }); + return true; + }; + + // Build once so each queue lookup stays O(1) during chain expansion. + for (const [name] of namedValues) { + namedValueKeyToName.set(name.toLowerCase(), name); + } + + const namedValueQueue: string[] = []; // Scan all policies for (const [, policyXml] of policies) { for (const dep of scanPolicyReferences(policyXml)) { - const key = `${dep.type}:${dep.name.toLowerCase()}`; - if (!seen.has(key)) { - seen.add(key); - dependencies.push({ type: dep.type, nameParts: [dep.name] }); + if (addDependency(dep) && dep.type === ResourceType.NamedValue) { + namedValueQueue.push(dep.name); } } } @@ -214,14 +249,68 @@ export function findTransitiveDependencies( // Scan API version set references for (const [, apiJson] of apis) { const dep = scanApiVersionSetReference(apiJson); - if (dep) { - const key = `${dep.type}:${dep.name.toLowerCase()}`; - if (!seen.has(key)) { - seen.add(key); - dependencies.push({ type: dep.type, nameParts: [dep.name] }); + if (dep && addDependency(dep) && dep.type === ResourceType.NamedValue) { + namedValueQueue.push(dep.name); + } + } + + // Expand named value chains (e.g. A references B, B references C). + while (namedValueQueue.length > 0) { + const currentName = namedValueQueue.shift()!; + const canonicalName = namedValueKeyToName.get(currentName.toLowerCase()); + if (!canonicalName) { + continue; + } + + const namedValueJson = namedValues.get(canonicalName); + if (!namedValueJson) { + continue; + } + + for (const dep of scanNamedValueReferences(namedValueJson)) { + if (addDependency(dep) && dep.type === ResourceType.NamedValue) { + namedValueQueue.push(dep.name); } } } return dependencies; } + +/** + * Scan named value JSON payload for references to other named values. + */ +function scanNamedValueReferences(namedValueJson: Record): TransitiveDependency[] { + const refs: TransitiveDependency[] = []; + const stack: unknown[] = [namedValueJson]; + + while (stack.length > 0) { + const value = stack.pop(); + if (typeof value === 'string') { + for (const match of value.matchAll(NAMED_VALUE_PATTERN)) { + if (match[1]) { + refs.push({ + type: ResourceType.NamedValue, + name: match[1].trim(), + }); + } + } + continue; + } + + if (Array.isArray(value)) { + for (const item of value) { + stack.push(item); + } + continue; + } + + if (value && typeof value === 'object') { + for (const child of Object.values(value as Record)) { + stack.push(child); + } + } + } + + return refs; +} diff --git a/tests/integration/all-resource-types/bicep/source-apim.bicep b/tests/integration/all-resource-types/bicep/source-apim.bicep index 24ca090..a132682 100644 --- a/tests/integration/all-resource-types/bicep/source-apim.bicep +++ b/tests/integration/all-resource-types/bicep/source-apim.bicep @@ -437,6 +437,39 @@ resource nvKeyVault 'Microsoft.ApiManagement/service/namedValues@2025-09-01-prev } } +resource nvAppInsightsKey 'Microsoft.ApiManagement/service/namedValues@2025-09-01-preview' = { + parent: apim + name: 'src-nv-appinsights-key' + properties: { + displayName: 'src-nv-appinsights-key' + value: appInsights.properties.InstrumentationKey + secret: true + tags: ['all-resources', 'logger', 'appinsights'] + } +} + +resource nvEventHubName 'Microsoft.ApiManagement/service/namedValues@2025-09-01-preview' = { + parent: apim + name: 'src-nv-eventhub-name' + properties: { + displayName: 'src-nv-eventhub-name' + value: eventHub.name + secret: false + tags: ['all-resources', 'logger', 'eventhub'] + } +} + +resource nvEventHubConnectionString 'Microsoft.ApiManagement/service/namedValues@2025-09-01-preview' = { + parent: apim + name: 'src-nv-eventhub-connection-string' + properties: { + displayName: 'src-nv-eventhub-connection-string' + value: eventHubAuthRule.listKeys().primaryConnectionString + secret: true + tags: ['all-resources', 'logger', 'eventhub'] + } +} + // --- Tags --- resource tagEnv 'Microsoft.ApiManagement/service/tags@2025-09-01-preview' = { parent: apim @@ -562,12 +595,13 @@ resource backendPool 'Microsoft.ApiManagement/service/backends@2025-09-01-previe resource loggerAppInsights 'Microsoft.ApiManagement/service/loggers@2025-09-01-preview' = { parent: apim name: 'src-logger-appinsights' + dependsOn: [nvAppInsightsKey] properties: { loggerType: 'applicationInsights' description: 'Application Insights logger for BVT' resourceId: appInsights.id credentials: { - instrumentationKey: appInsights.properties.InstrumentationKey + instrumentationKey: '{{src-nv-appinsights-key}}' } } } @@ -575,12 +609,16 @@ resource loggerAppInsights 'Microsoft.ApiManagement/service/loggers@2025-09-01-p resource loggerEventHub 'Microsoft.ApiManagement/service/loggers@2025-09-01-preview' = { parent: apim name: 'src-logger-eventhub' + dependsOn: [ + nvEventHubName + nvEventHubConnectionString + ] properties: { loggerType: 'azureEventHub' description: 'Event Hub logger for BVT' credentials: { - name: eventHub.name - connectionString: eventHubAuthRule.listKeys().primaryConnectionString + name: '{{src-nv-eventhub-name}}' + connectionString: '{{src-nv-eventhub-connection-string}}' } } } diff --git a/tests/integration/all-resource-types/expected-structure.json b/tests/integration/all-resource-types/expected-structure.json index e5f328b..a0cd7f9 100644 --- a/tests/integration/all-resource-types/expected-structure.json +++ b/tests/integration/all-resource-types/expected-structure.json @@ -18,7 +18,7 @@ }, "directories": { "namedValues": { - "minCount": 3, + "minCount": 6, "expected": [ { "name": "src-nv-plain", @@ -56,6 +56,42 @@ } }, "notes": "KeyVault reference type" + }, + { + "name": "src-nv-appinsights-key", + "files": ["namedValueInformation.json"], + "spotChecks": { + "namedValueInformation.json": { + "properties.displayName": "src-nv-appinsights-key", + "properties.secret": true, + "properties.tags": ["all-resources", "logger", "appinsights"] + } + }, + "notes": "Logger Application Insights instrumentation key stored as a secret named value" + }, + { + "name": "src-nv-eventhub-name", + "files": ["namedValueInformation.json"], + "spotChecks": { + "namedValueInformation.json": { + "properties.displayName": "src-nv-eventhub-name", + "properties.value": "src-apim-logs", + "properties.secret": false, + "properties.tags": ["all-resources", "logger", "eventhub"] + } + } + }, + { + "name": "src-nv-eventhub-connection-string", + "files": ["namedValueInformation.json"], + "spotChecks": { + "namedValueInformation.json": { + "properties.displayName": "src-nv-eventhub-connection-string", + "properties.secret": true, + "properties.tags": ["all-resources", "logger", "eventhub"] + } + }, + "notes": "Logger Event Hub connection string stored as a secret named value" } ] }, @@ -184,10 +220,10 @@ "spotChecks": { "loggerInformation.json": { "properties.loggerType": "applicationInsights", - "properties.credentials.instrumentationKey": "exists" + "properties.credentials.instrumentationKey": "{{src-nv-appinsights-key}}" } }, - "notes": "Instrumentationkey should be redacted" + "notes": "Instrumentation key should be referenced via NamedValue" }, { "name": "src-logger-eventhub", @@ -195,10 +231,11 @@ "spotChecks": { "loggerInformation.json": { "properties.loggerType": "azureEventHub", - "properties.credentials.name": "src-apim-logs" + "properties.credentials.name": "{{src-nv-eventhub-name}}", + "properties.credentials.connectionString": "{{src-nv-eventhub-connection-string}}" } }, - "notes": "Connection string should be redacted" + "notes": "Event Hub logger credentials should be referenced via NamedValues" } ] }, diff --git a/tests/integration/all-resource-types/phases/run-phase4-create-overrides.ps1 b/tests/integration/all-resource-types/phases/run-phase4-create-overrides.ps1 index 3dcaab0..f277fd5 100644 --- a/tests/integration/all-resource-types/phases/run-phase4-create-overrides.ps1 +++ b/tests/integration/all-resource-types/phases/run-phase4-create-overrides.ps1 @@ -104,16 +104,16 @@ namedValues: src-nv-keyvault: keyVault: secretIdentifier: "${targetKvUri}secrets/tgt-secret-value" + src-nv-appinsights-key: + value: "$targetAiKey" + src-nv-eventhub-name: + value: "tgt-eh-logs" + src-nv-eventhub-connection-string: + value: "$targetEhConnStr" loggers: src-logger-appinsights: resourceId: "$targetAiResourceId" - credentials: - instrumentationKey: "$targetAiKey" - src-logger-eventhub: - credentials: - name: "tgt-eh-logs" - connectionString: "$targetEhConnStr" "@ $overrideYaml | Set-Content -Path $overrideFile -Encoding utf8 diff --git a/tests/unit/services/resource-publisher.test.ts b/tests/unit/services/resource-publisher.test.ts index 14c5a52..951f7e8 100644 --- a/tests/unit/services/resource-publisher.test.ts +++ b/tests/unit/services/resource-publisher.test.ts @@ -171,6 +171,172 @@ describe('resource-publisher', () => { ); }); + it('should canonicalize named value references in logger credentials', async () => { + const client = createMockClient(); + const store = createMockStore(); + const namedValueName = 'Logger-Credentials--658acc09217d201b30974554'; + store.readResource.mockResolvedValue({ + name: 'my-logger', + properties: { + loggerType: 'applicationInsights', + credentials: { + instrumentationKey: '{{Old-Value}}', + }, + }, + }); + store.listResources.mockResolvedValue([ + { type: ResourceType.Logger, nameParts: ['my-logger'] }, + { type: ResourceType.NamedValue, nameParts: [namedValueName] }, + ]); + + const configWithOverrides: PublishConfig = { + ...testConfig, + overrides: { + loggers: { + 'my-logger': { + credentials: { + instrumentationKey: `{{${namedValueName.toLowerCase()}}}`, + }, + }, + }, + }, + }; + + const descriptor: ResourceDescriptor = { + type: ResourceType.Logger, + nameParts: ['my-logger'], + }; + + await publishResource(client, store, testContext, descriptor, configWithOverrides); + + expect(client.putResource).toHaveBeenCalledWith( + testContext, + descriptor, + expect.objectContaining({ + properties: expect.objectContaining({ + credentials: expect.objectContaining({ + instrumentationKey: `{{${namedValueName}}}`, + }), + }), + }) + ); + }); + + it('should canonicalize named value references in backend credentials', async () => { + const client = createMockClient(); + const store = createMockStore(); + const authTokenName = 'Backend-Auth-Token'; + const apiKeyName = 'Backend-API-Key'; + + store.readResource.mockResolvedValue({ + name: 'my-backend', + properties: { + url: 'https://api.example.com', + protocol: 'http', + credentials: { + authorization: { + scheme: 'Bearer', + parameter: '{{old-token}}', + }, + header: { + 'X-API-Key': ['{{old-api-key}}'], + }, + query: { + token: ['{{old-token}}'], + }, + }, + }, + }); + + store.listResources.mockResolvedValue([ + { type: ResourceType.Backend, nameParts: ['my-backend'] }, + { type: ResourceType.NamedValue, nameParts: [authTokenName] }, + { type: ResourceType.NamedValue, nameParts: [apiKeyName] }, + ]); + + const configWithOverrides: PublishConfig = { + ...testConfig, + overrides: { + backends: { + 'my-backend': { + credentials: { + authorization: { + parameter: `{{${authTokenName.toLowerCase()}}}`, + }, + header: { + 'X-API-Key': [`{{${apiKeyName.toLowerCase()}}}`], + }, + query: { + token: [`{{${authTokenName.toLowerCase()}}}`], + }, + }, + }, + }, + }, + }; + + const descriptor: ResourceDescriptor = { + type: ResourceType.Backend, + nameParts: ['my-backend'], + }; + + await publishResource(client, store, testContext, descriptor, configWithOverrides); + + const putCall = client.putResource.mock.calls[0]; + const putJson = putCall[2] as Record; + const props = putJson.properties as Record; + const credentials = props.credentials as Record; + const authorization = credentials.authorization as Record; + const header = credentials.header as Record; + const query = credentials.query as Record; + + expect(authorization.parameter).toBe(`{{${authTokenName}}}`); + expect(header['X-API-Key'][0]).toBe(`{{${apiKeyName}}}`); + expect(query['token'][0]).toBe(`{{${authTokenName}}}`); + }); + + it('should preserve unknown named value references', async () => { + const client = createMockClient(); + const store = createMockStore(); + const knownName = 'Known-Token'; + + store.readResource.mockResolvedValue({ + name: 'my-backend', + properties: { + credentials: { + authorization: { + parameter: '{{unknown-token}}', + }, + query: { + token: ['{{known-token}}'], + }, + }, + }, + }); + + store.listResources.mockResolvedValue([ + { type: ResourceType.Backend, nameParts: ['my-backend'] }, + { type: ResourceType.NamedValue, nameParts: [knownName] }, + ]); + + const descriptor: ResourceDescriptor = { + type: ResourceType.Backend, + nameParts: ['my-backend'], + }; + + await publishResource(client, store, testContext, descriptor, testConfig); + + const putCall = client.putResource.mock.calls[0]; + const putJson = putCall[2] as Record; + const props = putJson.properties as Record; + const credentials = props.credentials as Record; + const authorization = credentials.authorization as Record; + const query = credentials.query as Record; + + expect(authorization.parameter).toBe('{{unknown-token}}'); + expect(query['token'][0]).toBe(`{{${knownName}}}`); + }); + it('should preserve opaque JSON properties', async () => { const client = createMockClient(); const store = createMockStore(); @@ -1007,4 +1173,3 @@ describe('resource-publisher', () => { }); }); }); - diff --git a/tests/unit/services/transitive-resolver.test.ts b/tests/unit/services/transitive-resolver.test.ts index c4e3c96..484dddb 100644 --- a/tests/unit/services/transitive-resolver.test.ts +++ b/tests/unit/services/transitive-resolver.test.ts @@ -152,6 +152,27 @@ describe('transitive-resolver', () => { const expanded = resolveTransitiveDependencies(policies, apis, filter); expect(expanded.namedValueNames).toEqual(['existing-secret']); }); + + it('should expand named values referenced by included named values', () => { + const policies = new Map(); + policies.set('policy', '{{secret-a}}'); + + const apis = new Map>(); + const namedValues = new Map>(); + namedValues.set('secret-a', { + properties: { + value: '{{secret-b}}', + }, + }); + + const filter: FilterConfig = { + namedValueNames: [], + }; + + const expanded = resolveTransitiveDependencies(policies, apis, filter, namedValues); + expect(expanded.namedValueNames).toContain('secret-a'); + expect(expanded.namedValueNames).toContain('secret-b'); + }); }); describe('findTransitiveDependencies', () => { @@ -186,6 +207,60 @@ describe('transitive-resolver', () => { expect(nvDeps).toHaveLength(1); }); + it('should expand chained named value dependencies', () => { + const policies = new Map(); + policies.set('service-policy', '{{secret-a}}'); + + const apis = new Map>(); + const namedValues = new Map>(); + namedValues.set('secret-a', { + name: 'secret-a', + properties: { + value: '{{secret-b}}', + }, + }); + namedValues.set('secret-b', { + name: 'secret-b', + properties: { + value: '{{secret-c}}', + }, + }); + namedValues.set('secret-c', { + name: 'secret-c', + properties: { + value: 'plain-value', + }, + }); + + const deps = findTransitiveDependencies(policies, apis, namedValues); + const nvNames = deps + .filter((d) => d.type === ResourceType.NamedValue) + .map((d) => d.nameParts[0]); + + expect(nvNames).toContain('secret-a'); + expect(nvNames).toContain('secret-b'); + expect(nvNames).toContain('secret-c'); + }); + + it('should ignore missing named values while expanding chains', () => { + const policies = new Map(); + policies.set('service-policy', '{{secret-a}}'); + + const apis = new Map>(); + const namedValues = new Map>(); + namedValues.set('secret-a', { + name: 'secret-a', + properties: { + value: '{{secret-missing}}', + }, + }); + + const deps = findTransitiveDependencies(policies, apis, namedValues); + + expect(deps.some((d) => d.type === ResourceType.NamedValue && d.nameParts[0] === 'secret-a')).toBe(true); + expect(deps.some((d) => d.type === ResourceType.NamedValue && d.nameParts[0] === 'secret-missing')).toBe(true); + }); + it('should return empty array when no references', () => { const policies = new Map(); policies.set('policy', ''); diff --git a/tests/unit/services/workspace-extractor.test.ts b/tests/unit/services/workspace-extractor.test.ts index 396983e..08d1d81 100644 --- a/tests/unit/services/workspace-extractor.test.ts +++ b/tests/unit/services/workspace-extractor.test.ts @@ -5,6 +5,7 @@ */ import { describe, it, expect, vi } from 'vitest'; +import { IApimClient } from '../../../src/clients/iapim-client.js'; import { ResourceType } from '../../../src/models/resource-types.js'; import { ApimServiceContext } from '../../../src/models/types.js'; import { FilterConfig } from '../../../src/models/config.js'; @@ -18,14 +19,20 @@ const testContext: ApimServiceContext = { baseUrl: 'https://management.azure.com/subscriptions/sub-1/resourceGroups/rg-1/providers/Microsoft.ApiManagement/service/apim-1', }; -function createMockClient() { +async function* emptyAsyncGenerator(): AsyncGenerator { + yield* []; +} + +function createMockClient(): IApimClient { return { - listResources: async function* () {}, + listResources: () => emptyAsyncGenerator>(), getResource: vi.fn().mockResolvedValue(undefined), - putResource: vi.fn(), - deleteResource: vi.fn(), - listApiRevisions: async function* () {}, + putResource: vi.fn().mockResolvedValue({}), + patchResource: vi.fn().mockResolvedValue({}), + deleteResource: vi.fn().mockResolvedValue(true), + listApiRevisions: () => emptyAsyncGenerator>(), getApiSpecification: vi.fn().mockResolvedValue(undefined), + validatePreFlight: vi.fn().mockResolvedValue(undefined), }; } @@ -75,16 +82,12 @@ describe('workspace-extractor', () => { ResourceType.Documentation, ]; - // Keep this test focused on the workspace extractor iteration contract. - // Some type extractors may issue additional list calls (for example, - // Logger extraction prefetches NamedValues for credential normalization). - const firstSeenInOrder = seenTypes.filter((type, index) => - seenTypes.indexOf(type) === index - ); - expect(firstSeenInOrder).toEqual(expectedTypes); + // Keep this test focused on the workspace extractor iteration contract, + // including known helper calls (Logger extraction prefetches NamedValues). + expect(seenTypes).toEqual(expectedCallSequence); // Ensure no unexpected resource types are listed for workspace extraction. - expect(seenTypes.every((type) => expectedTypes.includes(type))).toBe(true); + expect(seenTypes.every((type) => expectedCallSequence.includes(type))).toBe(true); }); it('should skip extraction when no workspace names in filter', async () => {