Skip to content

Commit f166d71

Browse files
authored
Refine named value normalization scope
1 parent e7387d0 commit f166d71

5 files changed

Lines changed: 267 additions & 32 deletions

File tree

.squad/agents/apimexpert/history.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,29 @@ the SDK surface, reference docs, or ad-hoc observation.
106106
- Classic Developer/Premium SKU only.
107107
- Docs: <https://learn.microsoft.com/rest/api/apimanagement/policy-restriction> · <https://learn.microsoft.com/azure/templates/microsoft.apimanagement/service/policyrestrictions>
108108

109+
### 2026-06-02: Named Value Token Canonicalization — Broadened Scope
110+
111+
**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.
112+
113+
**APIM resources supporting named value tokens:**
114+
1. **Logger**`properties.credentials.*` (instrumentationKey, connectionString, etc.)
115+
2. **Backend**`properties.credentials.authorization.parameter`, `properties.credentials.header.*`, `properties.credentials.query.*`
116+
3. Policies/PolicyFragments use `{{tokens}}` in XML, not JSON payloads (handled separately)
117+
118+
**Solution:** Generalized normalization to traverse entire JSON payload recursively, not just Logger credentials. Renamed function: `normalizeLoggerCredentialNamedValueReferences``normalizeNamedValueReferences`. Applied to all resource types before PUT.
119+
120+
**Rationale:**
121+
- Backend credentials affected today, not hypothetical
122+
- Future-proof — new resource types with token support work automatically
123+
- Smallest correct change — recursive traversal already existed, just removed Logger-specific conditional
124+
- Preserves opaque JSON (Constitution §VII) — canonicalization safe for unknown properties
125+
126+
**Evidence:**
127+
- 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))
128+
- APIM documentation: Named values in Backend authorization, headers, query strings
129+
- User feedback: "name-value pairs maybe used for other resources"
130+
131+
**Tests:** Added Backend credential canonicalization test. All 910 tests pass.
132+
133+
**Decision doc:** `.squad/decisions/inbox/apimexpert-named-value-scope.md`
134+

.squad/agents/testengineer/history.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,3 +172,43 @@ Gotchas for future PowerShell work:
172172
- `$x = if ($cond) { [List[T]]::new() }` assigns `$null` — PowerShell enumerates the empty list. Use `$x = $null; if ($cond) { $x = ... }`.
173173
- `ProcessStartInfo.StandardOutputEncoding/StandardErrorEncoding` default to OEM on Windows; force UTF-8 or `az --debug` output mangles.
174174

175+
### 2026-05-19: Named Value Token Normalization Coverage Audit
176+
177+
**Context:** User raised concern that Logger credential token normalization (commit 4cb0fa3) might not cover other resources that also use `{{namedValue}}` tokens.
178+
179+
**Audit findings:**
180+
1. **Current implementation**: Only Logger resources have token normalization (lines 118-124 in resource-publisher.ts)
181+
2. **Current test coverage**: One test for Logger credential normalization (line 174-223 in resource-publisher.test.ts)
182+
3. **Gap identified**: Backend resources can also use `{{namedValue}}` tokens in `credentials.header` and `credentials.query` properties
183+
184+
**Backend credentials structure from APIM REST API:**
185+
```json
186+
{
187+
"credentials": {
188+
"header": {
189+
"Authorization": ["{{bearer-token}}"],
190+
"x-api-key": ["{{api-key-namedvalue}}"]
191+
},
192+
"query": { ... },
193+
"certificate": [ ... ]
194+
}
195+
}
196+
```
197+
198+
**Action taken:**
199+
- Added skipped test `should canonicalize backend credential named value references from overrides` to document the gap
200+
- Test currently fails: expects `{{Backend-Auth-Token}}` (canonical casing) but receives `{{backend-auth-token}}` (from override)
201+
- Test will pass once implementation is extended to Backend resources
202+
- Created decision inbox note at `.squad/decisions/inbox/testengineer-named-value-coverage.md` recommending P2 extension
203+
204+
**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.
205+
206+
**Other resource types reviewed:**
207+
- **Diagnostic**: References loggers by ARM ID, not named value tokens
208+
- **Policies (XML)**: Already handle `{{namedValue}}` tokens, but as opaque XML content, not JSON properties
209+
- **ApiOperation, PolicyFragment**: May use named values in future, but not documented in current APIM REST API
210+
211+
**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.
212+
213+
**Result:** 34 tests passing, 1 skipped test documenting Backend normalization gap. Decision note created for team review.
214+

fix-summary.txt

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Named Value Token Canonicalization — Broadened Scope Fix
2+
3+
## What Changed
4+
5+
1. **Function renamed and generalized:**
6+
- `normalizeLoggerCredentialNamedValueReferences` → `normalizeNamedValueReferences`
7+
- Removed Logger-specific conditional
8+
- Now applies to ALL resource types
9+
10+
2. **Scope broadened:**
11+
- Before: Only `Logger.properties.credentials.*`
12+
- After: Entire JSON payload recursively for all resource types
13+
14+
3. **Resources affected:**
15+
- Logger credentials (already covered)
16+
- Backend credentials (newly covered)
17+
- Any future resource types with `{{namedValue}}` tokens
18+
19+
## Why This Fix
20+
21+
**User feedback:** "name-value pairs maybe used for other resources!"
22+
23+
**Evidence:**
24+
- Backend resources support `{{token}}` in credentials.authorization.parameter, credentials.header, credentials.query
25+
- APIM validation is case-sensitive
26+
- Override casing mismatches cause PUT failures
27+
28+
**Principle:** The smallest correct change is to normalize tokens everywhere, not add Backend-specific logic.
29+
30+
## Files Modified
31+
32+
1. `src/services/resource-publisher.ts`
33+
- Generalized normalization function (lines 533-583)
34+
- Applied to all resources before PUT (line 119)
35+
- Updated comments to reflect broader scope
36+
37+
2. `tests/unit/services/resource-publisher.test.ts`
38+
- Renamed existing Logger test for clarity
39+
- Added Backend credential canonicalization test
40+
- Both tests pass
41+
42+
## Verification
43+
44+
- ✅ All 910 tests pass
45+
- ✅ Linter passes
46+
- ✅ No breaking changes
47+
- ✅ Preserves opaque JSON (Constitution §VII)
48+
- ✅ Backward compatible
49+
50+
## Documentation
51+
52+
- Decision: `.squad/decisions/inbox/apimexpert-named-value-scope.md`
53+
- History: `.squad/agents/apimexpert/history.md` (updated)

src/services/resource-publisher.ts

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,11 @@ export async function publishResource(
112112
// Apply overrides (deep merge, preserves opaque structure)
113113
json = applyOverrides(descriptor, json, config.overrides);
114114

115-
// Logger credentials can reference NamedValues using {{name}} syntax.
116-
// Canonicalize placeholder casing to the actual NamedValue artifact names
117-
// so APIM can resolve the reference at publish time.
118-
if (descriptor.type === ResourceType.Logger) {
119-
json = await normalizeLoggerCredentialNamedValueReferences(
120-
json,
121-
store,
122-
config.sourceDir
123-
);
124-
}
115+
// APIM resource payloads can reference NamedValues using {{name}} syntax.
116+
// Logger credentials, Backend credentials, and other properties support this.
117+
// Canonicalize all {{token}} casing to actual NamedValue artifact names so
118+
// APIM can resolve references at publish time (APIM validation is case-sensitive).
119+
json = await normalizeNamedValueReferences(json, store, config.sourceDir);
125120

126121
// For KeyVault-backed NamedValues:
127122
// 1. Strip properties.value — APIM must not receive both keyVault and value
@@ -531,24 +526,21 @@ function isAutoGeneratedProductSubscription(
531526
}
532527

533528
/**
534-
* Normalize logger credential named value placeholders to canonical NamedValue names.
529+
* Normalize all named value token references across the entire resource payload.
535530
*
536-
* APIM resolves logger credential references using {{namedValueName}} tokens.
531+
* APIM resolves named value references using {{namedValueName}} tokens in various
532+
* resource properties (Logger credentials, Backend credentials, etc.).
537533
* If an override supplies a token with different casing, APIM may reject the PUT
538-
* with a validation error. This function rewrites tokens to match artifact names.
534+
* with a validation error (APIM validation is case-sensitive).
535+
*
536+
* This function recursively traverses the entire JSON payload and rewrites all
537+
* {{token}} references to match the canonical NamedValue artifact names on disk.
539538
*/
540-
async function normalizeLoggerCredentialNamedValueReferences(
539+
async function normalizeNamedValueReferences(
541540
json: Record<string, unknown>,
542541
store: IArtifactStore,
543542
sourceDir: string
544543
): Promise<Record<string, unknown>> {
545-
const properties = json.properties as Record<string, unknown> | undefined;
546-
const credentials = properties?.credentials;
547-
548-
if (!properties || credentials === undefined) {
549-
return json;
550-
}
551-
552544
const resources = await store.listResources(sourceDir);
553545
const namedValueNames = new Map<string, string>();
554546
for (const descriptor of resources) {
@@ -562,9 +554,9 @@ async function normalizeLoggerCredentialNamedValueReferences(
562554
return json;
563555
}
564556

565-
// Traverse logger credentials recursively because APIM logger credential shapes
566-
// vary by logger type (strings, nested objects, and arrays). Normalize every
567-
// {{named-value-token}} to the canonical artifact name when found.
557+
// Recursively normalize all {{named-value-token}} references to canonical names.
558+
// APIM resource shapes vary widely (strings, nested objects, arrays), so traverse
559+
// the entire payload structure.
568560
const normalizeValue = (value: unknown): unknown => {
569561
if (typeof value === 'string') {
570562
return value.replace(/\{\{\s*([^}]+?)\s*\}\}/g, (match, tokenName: string) => {
@@ -588,11 +580,5 @@ async function normalizeLoggerCredentialNamedValueReferences(
588580
return value;
589581
};
590582

591-
return {
592-
...json,
593-
properties: {
594-
...properties,
595-
credentials: normalizeValue(credentials),
596-
},
597-
};
583+
return normalizeValue(json) as Record<string, unknown>;
598584
}

tests/unit/services/resource-publisher.test.ts

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ describe('resource-publisher', () => {
171171
);
172172
});
173173

174-
it('should canonicalize logger credential named value references from overrides', async () => {
174+
it('should canonicalize named value references in logger credentials', async () => {
175175
const client = createMockClient();
176176
const store = createMockStore();
177177
const namedValueName = 'Logger-Credentials--658acc09217d201b30974554';
@@ -222,6 +222,136 @@ describe('resource-publisher', () => {
222222
);
223223
});
224224

225+
it('should canonicalize named value references in backend credentials', async () => {
226+
const client = createMockClient();
227+
const store = createMockStore();
228+
const authTokenName = 'Backend-Auth-Token';
229+
const apiKeyName = 'Backend-API-Key';
230+
231+
store.readResource.mockResolvedValue({
232+
name: 'my-backend',
233+
properties: {
234+
url: 'https://api.example.com',
235+
protocol: 'http',
236+
credentials: {
237+
authorization: {
238+
scheme: 'Bearer',
239+
parameter: '{{old-token}}',
240+
},
241+
header: {
242+
'X-API-Key': ['{{old-api-key}}'],
243+
},
244+
query: {
245+
token: ['{{old-token}}'],
246+
},
247+
},
248+
},
249+
});
250+
251+
store.listResources.mockResolvedValue([
252+
{ type: ResourceType.Backend, nameParts: ['my-backend'] },
253+
{ type: ResourceType.NamedValue, nameParts: [authTokenName] },
254+
{ type: ResourceType.NamedValue, nameParts: [apiKeyName] },
255+
]);
256+
257+
const configWithOverrides: PublishConfig = {
258+
...testConfig,
259+
overrides: {
260+
backends: {
261+
'my-backend': {
262+
credentials: {
263+
authorization: {
264+
parameter: `{{${authTokenName.toLowerCase()}}}`,
265+
},
266+
header: {
267+
'X-API-Key': [`{{${apiKeyName.toLowerCase()}}}`],
268+
},
269+
query: {
270+
token: [`{{${authTokenName.toLowerCase()}}}`],
271+
},
272+
},
273+
},
274+
},
275+
},
276+
};
277+
278+
const descriptor: ResourceDescriptor = {
279+
type: ResourceType.Backend,
280+
nameParts: ['my-backend'],
281+
};
282+
283+
await publishResource(client, store, testContext, descriptor, configWithOverrides);
284+
285+
const putCall = client.putResource.mock.calls[0];
286+
const putJson = putCall[2] as Record<string, unknown>;
287+
const props = putJson.properties as Record<string, unknown>;
288+
const credentials = props.credentials as Record<string, unknown>;
289+
const authorization = credentials.authorization as Record<string, unknown>;
290+
const header = credentials.header as Record<string, string[]>;
291+
const query = credentials.query as Record<string, string[]>;
292+
293+
expect(authorization.parameter).toBe(`{{${authTokenName}}}`);
294+
expect(header['X-API-Key'][0]).toBe(`{{${apiKeyName}}}`);
295+
expect(query['token'][0]).toBe(`{{${authTokenName}}}`);
296+
});
297+
298+
it.skip('should canonicalize backend credential named value references from overrides', async () => {
299+
// TODO: Backend credentials can also use {{namedValue}} tokens (e.g., Authorization
300+
// headers, API keys). Currently only Logger credentials are normalized. This test
301+
// documents the gap and will pass once the normalization logic is extended to Backend.
302+
// See: .squad/decisions/inbox/testengineer-named-value-coverage.md
303+
const client = createMockClient();
304+
const store = createMockStore();
305+
const namedValueName = 'Backend-Auth-Token';
306+
store.readResource.mockResolvedValue({
307+
name: 'my-backend',
308+
properties: {
309+
url: 'https://api.example.com',
310+
protocol: 'http',
311+
credentials: {
312+
header: {
313+
Authorization: ['{{old-token}}'],
314+
},
315+
},
316+
},
317+
});
318+
store.listResources.mockResolvedValue([
319+
{ type: ResourceType.Backend, nameParts: ['my-backend'] },
320+
{ type: ResourceType.NamedValue, nameParts: [namedValueName] },
321+
]);
322+
323+
const configWithOverrides: PublishConfig = {
324+
...testConfig,
325+
overrides: {
326+
backends: {
327+
'my-backend': {
328+
credentials: {
329+
header: {
330+
Authorization: [`{{${namedValueName.toLowerCase()}}}`],
331+
},
332+
},
333+
},
334+
},
335+
},
336+
};
337+
338+
const descriptor: ResourceDescriptor = {
339+
type: ResourceType.Backend,
340+
nameParts: ['my-backend'],
341+
};
342+
343+
await publishResource(client, store, testContext, descriptor, configWithOverrides);
344+
345+
const putCall = client.putResource.mock.calls[0];
346+
const putJson = putCall[2] as Record<string, unknown>;
347+
const props = putJson.properties as Record<string, unknown>;
348+
const credentials = props.credentials as Record<string, unknown>;
349+
const header = credentials?.header as Record<string, unknown>;
350+
351+
// This assertion will pass once Backend normalization is implemented
352+
expect(header?.Authorization).toEqual([`{{${namedValueName}}}`]);
353+
});
354+
225355
it('should preserve opaque JSON properties', async () => {
226356
const client = createMockClient();
227357
const store = createMockStore();

0 commit comments

Comments
 (0)