From 569410ebe494bdc6707e52f2934b38f50ee07c46 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:21:34 +1100 Subject: [PATCH 01/23] fix: include loggers, tags, groups and other referenced resources transitively Filtered extract and publish only followed named value, backend and policy fragment references in policies, version sets and backend pools. The filtering guide also promised an API's products and tags. Selecting an API therefore left out resources it needs: the logger its diagnostics send data to, the tags it is linked to, and others. Extract and publish now also follow: - a diagnostic's loggerId (service-level and API diagnostics), to the logger in the diagnostic's own scope, since APIM leaves the workspace out of a workspace diagnostic's loggerId; - named values in logger and backend credentials; - an API's tag links to the tag, and a product's group and tag links to the group or tag (a link recorded at service scope targets the service-level resource); - policy references to loggers (log-to-eventhub logger-id), global schemas (validate-content's content schema-id) and backends named by llm-content-safety backend-id or embeddings-backend-id on the semantic cache lookup policies. Policy references are matched with the attribute anywhere on the element (attribute by attribute, so a '>' in an earlier value does not hide it) and in either quote, so APIM's generated is found; policy expressions (@...) and {{...}} placeholders are not treated as resource names. Extraction passes API diagnostics and tag links, and the product links it wrote, to the dependency scan. In a workspace, tag links are looked up on every workspace tag the filter does not exclude by name, when transitive resolution is on and the filter narrows tags, so tags linked to the extracted APIs and products are found and pulled in. Each service-level dependency is fetched once across the service-level pass and every workspace, including one that could not be fetched, so a missing group is not retried and counted again. A workspace without a sub-filter is extracted whole and still gets no dependency pass: APIM doesn't let a workspace product link to a custom service-level group. Policies reference a named value by its display name, as APIM resolves them. On publish, a {{reference}} now resolves to the named value artifact with that display name, falling back to the resource name, so a named value whose display name differs from its name is published with the policy. Extract already resolved display names; it now matches them ignoring case, as publish does. A resource the filter excludes by name ('!name') is never pulled in, and links to it are not extracted: an API's or a product's link to an excluded tag, or a product's link to an excluded group. An exclusion names the resource, so a named value is checked again once its display name is resolved. --remove-stale still treats an API's tag links as the API's, so a link to an excluded tag left by an earlier extract is removed. Built-in groups (administrators, developers, guests) at service level are not pulled in either, since every APIM instance has them; a workspace group with one of those names is a custom group and is. The list is shared with delete-unmatched. On publish, a link to a target in the publish set is published even when the filter leaves its type out (for example tags: []); links to a target excluded by name are not, and links to built-in groups follow the filter as before. The products, gateways and subscriptions an API is linked to are still not pulled in; the filtering guide now says so and lists what is. Fixes #315 Fixes #331 Fixes #277 --- README.md | 6 +- docs/architecture/overview.md | 2 +- docs/commands/extract.md | 2 +- docs/commands/publish.md | 10 +- docs/getting-started.md | 2 +- docs/guides/filtering-resources.md | 49 ++-- docs/reference/dependency-graph.md | 9 +- src/cli/extract-command.ts | 4 +- src/models/resource-types.ts | 21 ++ src/services/api-extractor.ts | 5 +- src/services/api-publisher.ts | 8 +- src/services/delete-unmatched-service.ts | 9 +- src/services/dry-run-reporter.ts | 8 +- src/services/extract-service.ts | 68 ++++- src/services/filter-service.ts | 71 +++++ src/services/product-extractor.ts | 42 ++- src/services/product-publisher.ts | 2 +- src/services/publish-service.ts | 47 +++- src/services/resource-publisher.ts | 45 ++- src/services/transitive-extractor.ts | 57 +++- src/services/transitive-resolver.ts | 182 +++++++++--- src/services/workspace-extractor.ts | 137 ++++++++- .../services/api-product-extractor.test.ts | 59 ++++ tests/unit/services/api-publisher.test.ts | 3 +- tests/unit/services/extract-service.test.ts | 99 +++++++ tests/unit/services/filter-service.test.ts | 57 ++++ tests/unit/services/product-publisher.test.ts | 46 +++ tests/unit/services/publish-service.test.ts | 133 ++++++++- .../unit/services/resource-publisher.test.ts | 31 +++ .../services/transitive-extractor.test.ts | 35 ++- .../unit/services/transitive-resolver.test.ts | 147 +++++++++- .../unit/services/workspace-extractor.test.ts | 262 ++++++++++++++++++ 32 files changed, 1507 insertions(+), 151 deletions(-) diff --git a/README.md b/README.md index c51f9565..64a28544 100644 --- a/README.md +++ b/README.md @@ -124,8 +124,10 @@ apiops publish \ ``` The publish filter uses the same YAML file and matching rules as `apiops extract --filter`. -Referenced named values, backends (including backend pool members), policy fragments, and version -sets are included automatically. Add `--no-transitive` to publish only the exact filter matches. +The resources the selected ones depend on, such as referenced named values, backends (including +backend pool members), policy fragments, version sets, tags, groups and loggers, are included +automatically; see [Transitive Dependency Behavior](docs/guides/filtering-resources.md#transitive-dependency-behavior). +Add `--no-transitive` to publish only the exact filter matches. ### `apiops init` diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index 5e6f31f1..3a6232bd 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -124,7 +124,7 @@ flowchart TD ### Key Extract Behaviors - **Topological ordering** — Resources are extracted in dependency order across 4 tiers, ensuring parent resources are written before children. -- **Transitive dependency resolution** — Policy XML is scanned for references to named values, backends, and policy fragments. Referenced resources are included even if not explicitly listed in the filter. +- **Transitive dependency resolution** — Policy XML is scanned for references to named values, backends, policy fragments, loggers and schemas; APIs, diagnostics, loggers, backends and products are scanned for their version set, logger, credential named values, tags and groups. Referenced resources are included even if not explicitly listed in the filter. - **Dual output format** — Resource metadata is written as JSON (`*.info.json`), while policies are written as raw XML (`policy.xml`). - **LIST then GET** — Some APIM list endpoints omit fields. When `listOmitsFields` is set for a resource type, individual GET calls fetch the full representation. diff --git a/docs/commands/extract.md b/docs/commands/extract.md index c4cce83e..d67839de 100644 --- a/docs/commands/extract.md +++ b/docs/commands/extract.md @@ -199,7 +199,7 @@ Supported workspace sub-filter keys: `apis`, `backends`, `diagnostics`, ### Transitive dependencies -When extracting with a filter, transitive dependencies are **included automatically**. For example, extracting `echo-api` also extracts any backends, named values, and policy fragments that `echo-api` references in its policies. +When extracting with a filter, transitive dependencies are **included automatically**. For example, extracting `echo-api` also extracts the backends, named values and policy fragments its policies reference, its version set, its tags, and the loggers its diagnostics use. See [Transitive Dependency Behavior](../guides/filtering-resources.md#transitive-dependency-behavior) for the full list. Use `--no-transitive` to disable this behavior and extract only the explicitly listed resources. diff --git a/docs/commands/publish.md b/docs/commands/publish.md index 8d14dc32..d19bc000 100644 --- a/docs/commands/publish.md +++ b/docs/commands/publish.md @@ -53,10 +53,12 @@ apiops publish \ --filter ./configuration.extractor.yaml ``` -The filter uses the same YAML format as `apiops extract --filter`. Referenced named values, -backends (including backend pool members), policy fragments, and version sets are included by -default; use `--no-transitive` to publish only exact matches. Product, gateway, and subscription -links do not pull their API or Product targets into the publish set. +The filter uses the same YAML format as `apiops extract --filter`. The resources the selected ones +depend on — such as referenced named values, backends (including backend pool members), policy +fragments, version sets, tags, groups and loggers — are included by default; see +[Transitive Dependency Behavior](../guides/filtering-resources.md#transitive-dependency-behavior). +Use `--no-transitive` to publish only exact matches. Product, gateway, and subscription links do +not pull their API or Product targets into the publish set. ### Delete resources not in source diff --git a/docs/getting-started.md b/docs/getting-started.md index 1e4c9cd6..a9b12fbd 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -75,7 +75,7 @@ apiops extract \ --filter filter.yaml ``` -Referenced backends, named values, and policy fragments are included automatically (transitive dependencies). Use `--no-transitive` to disable this. +Referenced backends, named values, policy fragments and other dependencies are included automatically ([transitive dependencies](guides/filtering-resources.md#transitive-dependency-behavior)). Use `--no-transitive` to disable this. ## 4. Publish to a Target Environment diff --git a/docs/guides/filtering-resources.md b/docs/guides/filtering-resources.md index 4b08c976..cb1c88cb 100644 --- a/docs/guides/filtering-resources.md +++ b/docs/guides/filtering-resources.md @@ -76,7 +76,7 @@ documentations: [] workspaces: [] ``` -With transitive resolution enabled (the default), any version set, backend, named value, policy fragment, or tag directly referenced by `my-own-api` or its policies is still pulled in automatically — even though those keys are set to `[]`. Use `--no-transitive` to disable that behavior. +With transitive resolution enabled (the default), the resources `my-own-api` depends on — such as the backends, named values and policy fragments its policies reference, its version set, its tags, and its diagnostics' loggers — are still pulled in automatically, even though those keys are set to `[]`. See [Transitive Dependency Behavior](#transitive-dependency-behavior) for the full list. Use `--no-transitive` to disable that behavior. The three states for every key are: @@ -303,20 +303,28 @@ Supported workspace sub-filter keys: `apis`, `backends`, `diagnostics`, `groups` ## Transitive Dependency Behavior -When you filter by API name, `apiops extract` automatically includes resources referenced by those APIs. This is **transitive dependency resolution** — it ensures the extracted artifacts are self-contained and publishable. +When you filter resources, `apiops extract` and `apiops publish` automatically include the resources the selected ones depend on. This is **transitive dependency resolution** — it keeps the extracted artifacts self-contained and publishable. Dependencies are followed until nothing new is found, so a pulled-in resource's own dependencies are included too. ### What Gets Auto-Included -```mermaid -flowchart TD - A[Filtered API] --> B[Backends referenced in policies] - A --> C[Named Values referenced in policies] - A --> D[Policy Fragments included in policies] - A --> E[Tags attached to the API] - A --> F[Products containing the API] - A --> G[Diagnostics configured on the API] - A --> H[Version Set the API belongs to] -``` +| Selected resource | Includes | +|-------------------|----------| +| Any policy (service, API, operation, GraphQL resolver, product) and policy fragment | Named values referenced as `{{displayName}}`; backends named by ``, `` and `embeddings-backend-id` on the semantic cache lookup policies; fragments named by ``; loggers named by ``; schemas named by validate-content's `` | +| API | Its version set, and the tags it is linked to | +| Diagnostic (service-level or on an API) | The logger it sends data to (`loggerId`) | +| Logger, backend | Named values referenced in their credentials | +| Backend pool | Its member backends | +| Product | The groups and tags it is linked to | + +A reference written as a policy expression (`backend-id="@(...)"`) is not followed, because its value is only known at run time. + +A `{{...}}` reference, in a policy or in credentials, names a named value by its display name, as APIM does; when no named value has that display name, it is read as a resource name. The `namedValues` list and its exclusions (`'!name'`) use the resource name, which can differ from the display name. + +A resource the filter excludes by name (`'!name'`) is never pulled in, and links to it are not extracted: an API's or product's link to an excluded tag, or a product's link to an excluded group. Built-in groups (`administrators`, `developers`, `guests`) exist at service level in every APIM instance, so they are not pulled in either; a product's links to them are published only when the filter includes them, as for any group. + +**Not included:** the products, gateways and subscriptions an API is linked to, and the APIs a product contains. These are independent resources, not dependencies; add them to the filter to include them. On publish, links to APIs or products outside the publish set are skipped. + +> **Logger keys:** APIM stores an Application Insights logger's key as an auto-generated secret named value, named with a 24-character hex ID (its display name is `Logger-Credentials--`). It is pulled in with the logger and extracted with its value redacted. Publish skips it unless you provide an override for it: an instance that already has the logger already has its key, but on a new instance the logger can't be published without one. See [Logger credentials with auto-generated named values](environment-overrides.md#logger-credentials-with-auto-generated-named-values). ### Example @@ -325,6 +333,12 @@ Given this filter: ```yaml apis: - petstore-api +backends: [] +namedValues: [] +policyFragments: [] +loggers: [] +tags: [] +products: [] ``` If `petstore-api` has a policy that references: @@ -332,11 +346,10 @@ If `petstore-api` has a policy that references: - Named value `petstore-api-key` → **auto-included** - Policy fragment `rate-limit-fragment` → **auto-included** -And `petstore-api` is assigned to: -- Product `starter` → **auto-included** -- Tag `production` → **auto-included** - -The extract output includes all of these, even though only `apiNames` was specified in the filter. +And `petstore-api`: +- has a diagnostic that sends data to logger `appinsights` → **auto-included** +- is tagged `production` → **auto-included** +- is in product `starter` → **not included**; add `starter` to `products` to include it ### Why Transitive Matters @@ -379,7 +392,7 @@ apis: - orders-admin-api ``` -Transitive dependencies (backends, named values, policy fragments) are auto-included. +Transitive dependencies (backends, named values, policy fragments, version sets, tags and loggers) are auto-included. ### Product-Focused Extract diff --git a/docs/reference/dependency-graph.md b/docs/reference/dependency-graph.md index bb45884a..40d50ec2 100644 --- a/docs/reference/dependency-graph.md +++ b/docs/reference/dependency-graph.md @@ -163,9 +163,12 @@ If a Tier 2 resource fails to publish, its dependents in Tier 3 and 4 will also When using `filter.yaml` to include specific resources, be aware of dependencies. -**During publish**, apiops-cli does **not** auto-include dependencies at the -artifact level — your filter (and hence your extracted artifact tree) must -already contain every required parent. For example, publishing an **ApiPolicy** +**During publish** with a filter, apiops-cli includes the resources the +selected ones depend on (named values, backends, policy fragments, version +sets, tags, groups, loggers and so on) unless `--no-transitive` is passed; see +[Transitive Dependency Behavior](../guides/filtering-resources.md#transitive-dependency-behavior). +It can only include what is in the artifact tree, so the tree must already +contain every required resource. For example, publishing an **ApiPolicy** without its parent **Api** present in the artifact tree will fail. **During extract**, apiops-cli **does** auto-follow a limited set of transitive diff --git a/src/cli/extract-command.ts b/src/cli/extract-command.ts index 4dec2332..bb93f87b 100644 --- a/src/cli/extract-command.ts +++ b/src/cli/extract-command.ts @@ -13,7 +13,7 @@ import * as path from 'node:path'; import { ExtractConfig, FilterConfig } from '../models/config.js'; import { ApimServiceContext } from '../models/types.js'; import { runExtraction, ExtractionResult } from '../services/extract-service.js'; -import { shouldReconcileResource } from '../services/filter-service.js'; +import { isInRemoveStaleScope } from '../services/filter-service.js'; import { loadFilterConfig } from '../lib/config-loader.js'; import { logger, parseLogLevel } from '../lib/logger.js'; import { ApimClient } from '../clients/apim-client.js'; @@ -156,7 +156,7 @@ export async function executeExtract( await store.commitStagedExtraction( stagingDir, outputDir, - descriptor => shouldReconcileResource(descriptor, filterConfig), + descriptor => isInRemoveStaleScope(descriptor, filterConfig), shouldRemoveStaleArtifacts(result, options.removeStale ?? false) ); } diff --git a/src/models/resource-types.ts b/src/models/resource-types.ts index 6686edb1..23b9752c 100644 --- a/src/models/resource-types.ts +++ b/src/models/resource-types.ts @@ -97,6 +97,27 @@ export interface ResourceTypeMetadata { readonly workspaceLinkIdProperty?: string; } +/** Groups every APIM instance has; they are never created, changed or deleted. */ +export const BUILT_IN_GROUP_NAMES: readonly string[] = ['administrators', 'developers', 'guests']; + +/** Whether `name` is one of the built-in groups (case-insensitive). */ +export function isBuiltInGroupName(name: string): boolean { + return BUILT_IN_GROUP_NAMES.includes(name.toLowerCase()); +} + +/** + * Whether a descriptor is a built-in group. Built-in groups exist only at + * service level; a workspace group with the same name is a custom group. + */ +export function isBuiltInGroup(descriptor: { type: ResourceType; nameParts: string[]; workspace?: string }): boolean { + return ( + descriptor.type === ResourceType.Group && + !descriptor.workspace && + descriptor.nameParts.length === 1 && + isBuiltInGroupName(descriptor.nameParts[0] ?? '') + ); +} + export const RESOURCE_TYPE_METADATA: Record = { [ResourceType.NamedValue]: { armPathSuffix: 'namedValues/{0}', diff --git a/src/services/api-extractor.ts b/src/services/api-extractor.ts index 2e477385..0131f74e 100644 --- a/src/services/api-extractor.ts +++ b/src/services/api-extractor.ts @@ -810,6 +810,7 @@ async function extractGraphQLResolvers( * @param extractedApiNames - API names already extracted for this workspace * @param outputDir - Output directory * @param workspace - Workspace name + * @param onLink - Called with each ApiTag artifact written * @returns Number of ApiTag artifacts written */ export async function extractWorkspaceApiTags( @@ -819,7 +820,8 @@ export async function extractWorkspaceApiTags( extractedTagNames: string[], extractedApiNames: Set, outputDir: string, - workspace: string + workspace: string, + onLink?: (descriptor: ResourceDescriptor, json: Record) => void ): Promise { const linkProperty = RESOURCE_TYPE_METADATA[ResourceType.ApiTag].workspaceLinkIdProperty; if (!linkProperty) { @@ -866,6 +868,7 @@ export async function extractWorkspaceApiTags( await store.writeResource(outputDir, apiTagDescriptor, tagJson); logger.info(`Extracted workspace ApiTag: ${apiName}/tags/${tagName}`); + onLink?.(apiTagDescriptor, tagJson); count++; } } catch (error) { diff --git a/src/services/api-publisher.ts b/src/services/api-publisher.ts index 4fbc7fa2..36a962f0 100644 --- a/src/services/api-publisher.ts +++ b/src/services/api-publisher.ts @@ -185,7 +185,8 @@ export async function publishApi( store, context, config, - publicationPlan + publicationPlan, + allowedDescriptors )); return { @@ -718,7 +719,8 @@ async function publishApiChildren( store: IArtifactStore, context: ApimServiceContext, config: PublishConfig, - plan: ApiPublicationPlan + plan: ApiPublicationPlan, + allowedDescriptors?: ResourceDescriptor[] ): Promise { const results: ResourcePublishResult[] = []; // Group resources by publish tier for dependency ordering. @@ -738,7 +740,7 @@ async function publishApiChildren( const descriptors = tierMap.get(tier) ?? []; if (descriptors.length > 0) { const tasks = descriptors.map((descriptor) => async () => { - return publishResource(client, store, context, descriptor, config); + return publishResource(client, store, context, descriptor, config, allowedDescriptors); }); const taskResults = await runParallel(tasks, 5); for (const [index, taskResult] of (taskResults ?? []).entries()) { diff --git a/src/services/delete-unmatched-service.ts b/src/services/delete-unmatched-service.ts index bbb77691..61798324 100644 --- a/src/services/delete-unmatched-service.ts +++ b/src/services/delete-unmatched-service.ts @@ -27,7 +27,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 type { PublishConfig } from '../models/config.js'; -import { ResourceType } from '../models/resource-types.js'; +import { isBuiltInGroupName, ResourceType } from '../models/resource-types.js'; import { getTopologicalOrder } from '../lib/dependency-graph.js'; import { getNameFromNameParts, @@ -77,11 +77,6 @@ export function filterRevisionDeletesHandledByBaseApi( }); } -/** - * Built-in groups that should never be deleted - */ -const BUILT_IN_GROUPS = ['administrators', 'developers', 'guests']; - /** * System resources that should never be deleted */ @@ -393,7 +388,7 @@ function isSystemResource(descriptor: ResourceDescriptor): boolean { // Check built-in groups if (descriptor.type === ResourceType.Group) { - if (BUILT_IN_GROUPS.includes(ownName)) { + if (isBuiltInGroupName(ownName)) { return true; } } diff --git a/src/services/dry-run-reporter.ts b/src/services/dry-run-reporter.ts index 4431e810..745de3af 100644 --- a/src/services/dry-run-reporter.ts +++ b/src/services/dry-run-reporter.ts @@ -361,7 +361,9 @@ async function planDryRunPublications( } for (const child of apiPlan.childPuts) { if (child.type === ResourceType.ApiTag) { - const eligibility = await evaluateResourceEligibility(store, child, config); + const eligibility = await evaluateResourceEligibility( + store, child, config, undefined, targetDescriptors + ); if ( eligibility.eligible && !child.workspace && @@ -415,7 +417,9 @@ async function planDryRunPublications( const eligibility = await evaluateResourceEligibility( store, descriptor, - config + config, + undefined, + targetDescriptors ); if ( eligibility.eligible && diff --git a/src/services/extract-service.ts b/src/services/extract-service.ts index 8c1d87b4..9bdbb95c 100644 --- a/src/services/extract-service.ts +++ b/src/services/extract-service.ts @@ -27,8 +27,9 @@ import { extractApiResources, ApiExtractionResult } from './api-extractor.js'; import { extractProductResources, ProductExtractionResult } from './product-extractor.js'; import { extractWorkspaces, WorkspaceExtractionResult } from './workspace-extractor.js'; import { extractTransitiveDependencies } from './transitive-extractor.js'; +import { findAssociationDependencies } from './transitive-resolver.js'; import { redactAndWarnPolicySecrets } from './secret-redactor.js'; -import { shouldIncludeResource } from './filter-service.js'; +import { isExplicitlyExcluded, shouldIncludeResource } from './filter-service.js'; import { logger } from '../lib/logger.js'; import { buildResourceLabel } from '../lib/resource-uri.js'; import { EXIT_SUCCESS, EXIT_PARTIAL, EXIT_FATAL } from '../lib/exit-codes.js'; @@ -125,8 +126,9 @@ export async function runExtraction( await extractServicePolicy(client, store, service, outputDir, filter, result); // Phase 6: Transitive dependency resolution (if enabled) + let failedTransitive: ResourceDescriptor[] = []; if (config.includeTransitive && filter) { - await resolveAndExtractTransitive(client, store, service, outputDir, result); + failedTransitive = await resolveAndExtractTransitive(client, store, service, outputDir, filter, result); } // Phase 7: Extract workspace-scoped resources @@ -137,7 +139,8 @@ export async function runExtraction( outputDir, filter, config.includeTransitive, - result + result, + failedTransitive ); // Compute exit code @@ -517,8 +520,9 @@ async function resolveAndExtractTransitive( store: IArtifactStore, context: ApimServiceContext, outputDir: string, + filter: FilterConfig, result: ExtractionResult -): Promise { +): Promise { logger.info('Resolving transitive dependencies...'); // Build maps for transitive resolution @@ -535,14 +539,19 @@ async function resolveAndExtractTransitive( } } - const resources = result.typeResults.flatMap((typeResult) => - typeResult.extracted - .filter((extracted) => extracted.status === 'success') - .map((extracted) => ({ - descriptor: extracted.descriptor, - json: extracted.json, - })) - ); + // Top-level resources, plus the API children that reference other + // resources: diagnostics (their logger) and tag links (the tag). + const extractedResources = [ + ...result.typeResults.flatMap((typeResult) => typeResult.extracted), + ...result.apiResults.flatMap((apiResult) => [...apiResult.diagnostics, ...apiResult.tags]), + ]; + const resources = extractedResources + .filter((extracted) => extracted.status === 'success') + .map((extracted) => ({ + descriptor: extracted.descriptor, + json: extracted.json, + })); + const linkedDependencies = await findProductLinkDependencies(store, outputDir, result); const transitiveResult = await extractTransitiveDependencies( client, store, @@ -551,11 +560,37 @@ async function resolveAndExtractTransitive( result.collectedPolicies, apiJsonMap, resources, - result.extractedDescriptors + result.extractedDescriptors, + { + linkedDependencies, + skip: (dep) => isExplicitlyExcluded(dep, filter), + } ); result.totalExtracted += transitiveResult.extractedDescriptors.length; result.totalErrors += transitiveResult.errorCount; result.extractedDescriptors.push(...transitiveResult.extractedDescriptors); + return transitiveResult.failedDescriptors; +} + +/** + * The groups and tags that extracted products link to, read back from the + * groups.json and tags.json files written for them. + */ +async function findProductLinkDependencies( + store: IArtifactStore, + outputDir: string, + result: ExtractionResult +): Promise { + const dependencies: ResourceDescriptor[] = []; + for (const typeResult of result.typeResults.filter((r) => r.type === ResourceType.Product)) { + for (const product of typeResult.extracted.filter((r) => r.status === 'success')) { + for (const associationType of ['groups', 'tags'] as const) { + const entries = await store.readAssociation(outputDir, product.descriptor, associationType); + dependencies.push(...findAssociationDependencies(product.descriptor, associationType, entries)); + } + } + } + return dependencies; } /** @@ -568,10 +603,13 @@ async function extractWorkspaceResources( outputDir: string, filter: FilterConfig | undefined, includeTransitive: boolean, - result: ExtractionResult + result: ExtractionResult, + /** Dependencies the service-level transitive pass tried and failed to extract. */ + failedTransitive: ResourceDescriptor[] = [] ): Promise { const wsResults = await extractWorkspaces( - client, store, context, outputDir, filter, includeTransitive + client, store, context, outputDir, filter, includeTransitive, + [...result.extractedDescriptors, ...failedTransitive] ); result.workspaceResults = wsResults; diff --git a/src/services/filter-service.ts b/src/services/filter-service.ts index a73ca28b..ac18d071 100644 --- a/src/services/filter-service.ts +++ b/src/services/filter-service.ts @@ -110,6 +110,15 @@ export function shouldIncludeResource( return false; } + // An API's link to a tag the filter excludes by name is left out with + // the tag, rather than extracted and then skipped on every publish. + if ( + descriptor.type === ResourceType.ApiTag && + isExcludedByName(ResourceType.Tag, getNamePart(descriptor.nameParts, 1), filter) + ) { + return false; + } + // Second check: does the parent have sub-resource filters? if (parentField === 'apis' && filter.apiSubFilters) { return matchesApiSubFilter(descriptor, parentName, filter.apiSubFilters); @@ -172,6 +181,23 @@ function getWorkspaceFilter( } : undefined; } +/** + * Whether `extract --remove-stale` manages this artifact, removing it when + * the extract does not write it again. As shouldReconcileResource, except + * that an API's tag links belong to the API even when the filter excludes + * the tag: extract doesn't write a link to an excluded tag, so one left by + * an earlier extract is removed with the API's other stale children. + */ +export function isInRemoveStaleScope(descriptor: ResourceDescriptor, filter?: FilterConfig): boolean { + if (descriptor.type === ResourceType.ApiTag) { + return shouldReconcileResource( + { type: ResourceType.Api, nameParts: [getNamePart(descriptor.nameParts, 0)], workspace: descriptor.workspace }, + filter + ); + } + return shouldReconcileResource(descriptor, filter); +} + /** * Resolve the resource filter configured for a workspace. * Returns undefined when the workspace has no nested filter. @@ -295,6 +321,51 @@ export function extractRootApiName(name: string): string { return semiIndex >= 0 ? name.substring(0, semiIndex) : name; } +/** + * Whether the filter excludes this resource by name: its type's list has a + * `!pattern` entry that matches it. Dependency resolution respects these + * exclusions, while it still pulls in resources whose type is `[]` or that a + * list simply does not name. A workspace resource is checked against its + * workspace's sub-filter. + */ +export function isExplicitlyExcluded(descriptor: ResourceDescriptor, filter?: FilterConfig): boolean { + if (descriptor.nameParts.length === 0) { + return false; + } + const effectiveFilter = descriptor.workspace ? cachedWorkspaceFilter(descriptor.workspace, filter) : filter; + return isExcludedByName(descriptor.type, getNamePart(descriptor.nameParts, 0), effectiveFilter); +} + +/** Whether `filter` (already resolved for the resource's scope) excludes `name` of `type` by name. */ +function isExcludedByName(type: ResourceType, name: string, filter: FilterConfig | undefined): boolean { + const field = FILTER_FIELD_MAP[type]; + const list = field && filter ? (filter[field] as string[] | undefined) : undefined; + const exclusions = list?.filter((entry) => entry.startsWith('!')) ?? []; + return exclusions.length > 0 && !matchesFilter(name, exclusions); +} + +/** + * Resolved workspace filters, cached per filter object and workspace: the + * exclusion check runs for every reference during publish expansion. + */ +const workspaceFilterCache = new WeakMap>(); + +function cachedWorkspaceFilter(workspace: string, filter: FilterConfig | undefined): FilterConfig | undefined { + if (!filter) { + return undefined; + } + let byWorkspace = workspaceFilterCache.get(filter); + if (!byWorkspace) { + byWorkspace = new Map(); + workspaceFilterCache.set(filter, byWorkspace); + } + const key = workspace.toLowerCase(); + if (!byWorkspace.has(key)) { + byWorkspace.set(key, resolveWorkspaceFilter(workspace, filter)); + } + return byWorkspace.get(key); +} + /** * Check if a pattern contains wildcard characters (* or ?). */ diff --git a/src/services/product-extractor.ts b/src/services/product-extractor.ts index 97118722..c11d27f6 100644 --- a/src/services/product-extractor.ts +++ b/src/services/product-extractor.ts @@ -15,6 +15,8 @@ import { redactAndWarnPolicySecrets } from './secret-redactor.js'; import { logger } from '../lib/logger.js'; import { getNamePart } from '../lib/resource-path.js'; import { isWorkspaceScope, extractNameFromLink, extractLinkTarget } from '../lib/workspace-link.js'; +import { isExplicitlyExcluded } from './filter-service.js'; +import { associationTarget } from './transitive-resolver.js'; /** * Result of product-specific extraction for a single product. @@ -40,7 +42,11 @@ export async function extractProductResources( context: ApimServiceContext, productDescriptor: ResourceDescriptor, outputDir: string, - _filter?: FilterConfig, + /** + * The top-level filter (for a workspace product too): links to groups and + * tags it excludes by name are left out with them. + */ + filter?: FilterConfig, _workspace?: string ): Promise { const productName = getNamePart(productDescriptor.nameParts, 0); @@ -62,7 +68,7 @@ export async function extractProductResources( // Extract product group associations result.groups = await extractProductAssociations( - client, store, context, productDescriptor, outputDir, 'groups', () => result.errorCount++ + client, store, context, productDescriptor, outputDir, 'groups', () => result.errorCount++, filter ); // Extract product policy @@ -84,7 +90,7 @@ export async function extractProductResources( // in the workspace extractor. if (!isWorkspaceScope(context)) { result.tags = await extractProductTags( - client, store, context, productDescriptor, outputDir, () => result.errorCount++ + client, store, context, productDescriptor, outputDir, () => result.errorCount++, filter ); } @@ -101,9 +107,10 @@ async function extractProductAssociations( productDescriptor: ResourceDescriptor, outputDir: string, associationType: 'apis' | 'groups', - onError: () => void + onError: () => void, + filter?: FilterConfig ): Promise { - const entries: AssociationEntry[] = []; + let entries: AssociationEntry[] = []; const resourceType = associationType === 'apis' ? ResourceType.ProductApi : ResourceType.ProductGroup; @@ -137,6 +144,10 @@ async function extractProductAssociations( } } + if (associationType === 'groups') { + entries = withoutExcludedTargets(productDescriptor, 'groups', entries, filter); + } + // Write association file if (entries.length > 0) { await store.writeAssociation(outputDir, productDescriptor, associationType, entries); @@ -150,6 +161,20 @@ async function extractProductAssociations( return entries.map(entry => entry.name); } +/** + * A product's link entries without the ones to groups or tags the filter + * excludes by name: those are not extracted, so the links would only be + * skipped on every publish (as an API's links to excluded tags are left out). + */ +function withoutExcludedTargets( + product: ResourceDescriptor, + associationType: 'groups' | 'tags', + entries: AssociationEntry[], + filter: FilterConfig | undefined +): AssociationEntry[] { + return entries.filter((entry) => !isExplicitlyExcluded(associationTarget(product, associationType, entry), filter)); +} + /** * Extract product tags and write to artifact store as tags.json. * Only used in service scope; workspace scope is handled by extractWorkspaceProductTags(). @@ -160,9 +185,10 @@ async function extractProductTags( context: ApimServiceContext, productDescriptor: ResourceDescriptor, outputDir: string, - onError: () => void + onError: () => void, + filter?: FilterConfig ): Promise { - const entries: AssociationEntry[] = []; + let entries: AssociationEntry[] = []; const workspaceScoped = isWorkspaceScope(context); const linkProperty = RESOURCE_TYPE_METADATA[ResourceType.ProductTag].workspaceLinkIdProperty; @@ -188,6 +214,8 @@ async function extractProductTags( } } + entries = withoutExcludedTargets(productDescriptor, 'tags', entries, filter); + // Write tags association file if (entries.length > 0) { await store.writeAssociation(outputDir, productDescriptor, 'tags', entries); diff --git a/src/services/product-publisher.ts b/src/services/product-publisher.ts index 87b0e371..f4b06ade 100644 --- a/src/services/product-publisher.ts +++ b/src/services/product-publisher.ts @@ -291,7 +291,7 @@ export async function planProductAssociationPublications( store, targetDescriptor, config, - allowedDescriptors + { allowedDescriptors } ); const deployedDescriptor = config.envMapping ? mapDescriptor(descriptor, config.envMapping) diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 6d1e966a..a4983f97 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -29,6 +29,7 @@ import { } from '../lib/resource-path.js'; import { extractRootApiName, + isExplicitlyExcluded, resolveWorkspaceFilter, shouldIncludeResource, shouldReconcileResource, @@ -326,6 +327,18 @@ async function expandTransitivePublishTargets( const availableByKey = new Map( availableDescriptors.map((descriptor) => [getResourceDescriptorKey(descriptor), descriptor]) ); + // Policies reference named values as {{displayName}}, which can differ + // from the resource name the artifact is stored under. As in APIM (and on + // extract), the display name decides; the resource name is the fallback. + let namedValuesByDisplayName: Promise> | undefined; + const findAvailable = async (reference: ResourceDescriptor): Promise => { + const key = getResourceDescriptorKey(reference); + if (reference.type !== ResourceType.NamedValue) { + return availableByKey.get(key); + } + namedValuesByDisplayName ??= indexNamedValuesByDisplayName(store, sourceDir, availableDescriptors); + return (await namedValuesByDisplayName).get(key) ?? availableByKey.get(key); + }; const expanded = [...initialDescriptors]; const included = new Set(expanded.map(getResourceDescriptorKey)); const scanQueue = [...initialDescriptors]; @@ -385,8 +398,12 @@ async function expandTransitivePublishTargets( continue; } for (const reference of references) { - const available = availableByKey.get(getResourceDescriptorKey(reference)); - if (available && !included.has(getResourceDescriptorKey(available))) { + const available = await findAvailable(reference); + if ( + available && + !included.has(getResourceDescriptorKey(available)) && + !isExplicitlyExcluded(available, filter) + ) { included.add(getResourceDescriptorKey(available)); expanded.push(available); scanQueue.push(available); @@ -402,6 +419,32 @@ async function expandTransitivePublishTargets( return expanded; } +/** + * The named value artifacts keyed as a `{{displayName}}` reference to them + * would be (case-insensitive, like every descriptor key). + */ +async function indexNamedValuesByDisplayName( + store: IArtifactStore, + sourceDir: string, + availableDescriptors: ResourceDescriptor[] +): Promise> { + const index = new Map(); + for (const descriptor of availableDescriptors.filter((d) => d.type === ResourceType.NamedValue)) { + let json: Record | undefined; + try { + json = await store.readResource(sourceDir, descriptor); + } catch (error) { + logger.warn(`Unable to read ${buildResourceLabel(descriptor)} to match {{references}} by display name: ${String(error)}`); + continue; + } + const displayName = (json?.properties as Record | undefined)?.displayName; + if (typeof displayName === 'string') { + index.set(getResourceDescriptorKey({ ...descriptor, nameParts: [displayName] }), descriptor); + } + } + return index; +} + function filterPublishDescriptors( descriptors: ResourceDescriptor[], filter?: FilterConfig diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts index fe9f3729..81c7ddfa 100644 --- a/src/services/resource-publisher.ts +++ b/src/services/resource-publisher.ts @@ -30,7 +30,7 @@ import { REDACTION_MARKER } from './secret-redactor.js'; import { isAssociationReferenceNotFoundError, isLinkAlreadyExistsError } from '../clients/apim-client.js'; import type { OverrideConfig, OverrideSection } from '../models/config.js'; import { buildResourceLabel } from '../lib/resource-uri.js'; -import { hasExplicitTypeFilter, resolveWorkspaceFilter, shouldIncludeResource } from './filter-service.js'; +import { hasExplicitTypeFilter, isExplicitlyExcluded, resolveWorkspaceFilter, shouldIncludeResource } from './filter-service.js'; import { findSubscriptionTargets } from './transitive-resolver.js'; import { mapDescriptor, toDeployedName } from './env-mapper.js'; import type { EnvMapping } from './env-mapper.js'; @@ -259,7 +259,9 @@ export async function publishResource( const eligibility = await evaluateResourceEligibility( store, descriptor, - config + config, + undefined, + allowedDescriptors ); if (!eligibility.eligible) { logger.warn( @@ -739,7 +741,7 @@ async function buildAssociationPublicationPlan( store, target, config, - allowedDescriptors + { allowedDescriptors } ); const metadata = RESOURCE_TYPE_METADATA[type]; const workspaceScoped = !!workspace || isWorkspaceScope(context); @@ -764,16 +766,40 @@ async function buildAssociationPublicationPlan( return { descriptor, deployedDescriptor, target, payload, ...eligibility }; } +export interface AssociationEligibilityOptions { + /** + * The publish set, when the caller has one. Besides marking targets as + * published, it decides availability for targets the filter lists. + */ + allowedDescriptors?: ResourceDescriptor[]; + /** + * Resources being published, used only to accept targets in it (defaults + * to allowedDescriptors). The ApiTag check passes this without + * allowedDescriptors, so an incremental publish still accepts a tag that + * exists on disk but is not in the diff. + */ + publishSet?: ResourceDescriptor[]; +} + export async function evaluateAssociationEligibility( store: IArtifactStore, target: ResourceDescriptor, config: PublishConfig, - allowedDescriptors?: ResourceDescriptor[] + { allowedDescriptors, publishSet = allowedDescriptors }: AssociationEligibilityOptions = {} ): Promise { if (!config.filter) { return { eligible: true }; } + // A target being published is linked even when the filter leaves its type + // out, because dependency resolution pulled it in (a tag or group). + if ( + !isExplicitlyExcluded(target, config.filter) && + publishSet?.some((included) => sameResourceDescriptor(included, target)) + ) { + return { eligible: true }; + } + const effectiveFilter = target.workspace ? resolveWorkspaceFilter(target.workspace, config.filter) : config.filter; @@ -782,12 +808,6 @@ export async function evaluateAssociationEligibility( } if (allowedDescriptors) { - if ( - allowedDescriptors.some((allowed) => sameResourceDescriptor(allowed, target)) - ) { - return { eligible: true }; - } - if (config.commitId) { const exists = (await store.readResource(config.sourceDir, target)) !== undefined; return exists @@ -897,7 +917,8 @@ export async function evaluateResourceEligibility( store: IArtifactStore, descriptor: ResourceDescriptor, config: PublishConfig, - json?: Record + json?: Record, + publishSet?: ResourceDescriptor[] ): Promise { if (descriptor.type === ResourceType.ApiTag) { const target: ResourceDescriptor = { @@ -905,7 +926,7 @@ export async function evaluateResourceEligibility( nameParts: [getNamePart(descriptor.nameParts, 1)], workspace: descriptor.workspace, }; - return evaluateAssociationEligibility(store, target, config); + return evaluateAssociationEligibility(store, target, config, { publishSet }); } if (descriptor.type === ResourceType.ApiOperation) { diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index 4f59fc56..004de300 100644 --- a/src/services/transitive-extractor.ts +++ b/src/services/transitive-extractor.ts @@ -21,18 +21,24 @@ export interface TransitiveResourceArtifact { export interface TransitiveExtractionResult { extractedDescriptors: ResourceDescriptor[]; + /** Dependencies that were not found or could not be extracted. */ + failedDescriptors: ResourceDescriptor[]; errorCount: number; } interface TransitiveTaskResult { dep: ResourceDescriptor; json?: Record; - /** Resolved to a resource already extracted under its real name. */ + /** + * Resolved to a resource already extracted or claimed by another reference, + * or to one the filter excludes by name. + */ skipped?: boolean; } /** - * Per-context (keyed by baseUrl) index of named value displayName → resource name. + * Per-context (keyed by baseUrl) index of named value displayName (lower + * case, as names are compared everywhere else) → resource name. * Stored as promises so concurrent tasks share a single LIST call. */ type NamedValueNameIndex = Map>>; @@ -47,7 +53,7 @@ async function loadNamedValueNames( const name = json.name; const displayName = (json.properties as Record | undefined)?.displayName; if (typeof name === 'string' && typeof displayName === 'string') { - byDisplayName.set(displayName, name); + byDisplayName.set(displayName.toLowerCase(), name); } } } catch (error) { @@ -78,7 +84,7 @@ async function resolveNamedValueDescriptor( } const ref = dep.nameParts[0] ?? ''; - const name = (await names).get(ref); + const name = (await names).get(ref.toLowerCase()); if (!name || name === ref) { return dep; } @@ -87,6 +93,20 @@ async function resolveNamedValueDescriptor( return { ...dep, nameParts: [name] }; } +export interface TransitiveExtractionOptions { + /** Workspace being extracted; its resources are fetched through `context`. */ + workspace?: string; + /** Service-level context, for dependencies outside the workspace. */ + serviceContext?: ApimServiceContext; + /** Dependencies already known from links (a product's groups and tags). */ + linkedDependencies?: ResourceDescriptor[]; + /** + * Dependencies not to extract: ones the filter excludes by name, or ones + * the caller knows are extracted already. + */ + skip?: (dep: ResourceDescriptor) => boolean; +} + export async function extractTransitiveDependencies( client: IApimClient, store: IArtifactStore, @@ -96,29 +116,32 @@ export async function extractTransitiveDependencies( apis: Map>, resources: TransitiveResourceArtifact[], alreadyExtracted: ResourceDescriptor[], - workspace?: string, - serviceContext?: ApimServiceContext + { + workspace, + serviceContext, + linkedDependencies = [], + skip = () => false, + }: TransitiveExtractionOptions = {} ): Promise { const attempted = new Set(alreadyExtracted.map(getResourceDescriptorKey)); const claimedNamedValues = new Set(alreadyExtracted.map(getResourceDescriptorKey)); const extractedDescriptors: ResourceDescriptor[] = []; + const failedDescriptors: ResourceDescriptor[] = []; const namedValueIndex: NamedValueNameIndex = new Map(); let errorCount = 0; let foundDependencies = false; while (true) { - const newDeps = findTransitiveDependencies( - policies, - apis, - workspace, - resources - ).filter((dep) => !attempted.has(getResourceDescriptorKey(dep))); + const newDeps = [ + ...findTransitiveDependencies(policies, apis, workspace, resources), + ...linkedDependencies, + ].filter((dep) => !attempted.has(getResourceDescriptorKey(dep)) && !skip(dep)); if (newDeps.length === 0) { if (!foundDependencies) { logger.debug('No additional transitive dependencies found'); } - return { extractedDescriptors, errorCount }; + return { extractedDescriptors, failedDescriptors, errorCount }; } foundDependencies = true; @@ -135,6 +158,11 @@ export async function extractTransitiveDependencies( if (dep.type === ResourceType.NamedValue) { target = await resolveNamedValueDescriptor(client, dependencyContext, dep, namedValueIndex); const targetKey = getResourceDescriptorKey(target); + // A {{displayName}} reference is checked again under the resource + // name it resolves to, which is what an exclusion names. + if (target !== dep && skip(target)) { + return { dep: target, skipped: true }; + } if (claimedNamedValues.has(targetKey)) { return { dep: target, skipped: true }; } @@ -165,6 +193,9 @@ export async function extractTransitiveDependencies( } if (!value?.json) { errorCount++; + if (value) { + failedDescriptors.push(value.dep); + } continue; } diff --git a/src/services/transitive-resolver.ts b/src/services/transitive-resolver.ts index 1251e823..36bfe931 100644 --- a/src/services/transitive-resolver.ts +++ b/src/services/transitive-resolver.ts @@ -2,14 +2,16 @@ // Licensed under the MIT license. /** * Transitive dependency resolver - * Scan policies for named value refs, backend refs, policy fragment refs. - * Scan apiInformation.json for apiVersionSetId. + * Scan policies for named value, backend, policy fragment, logger and schema + * refs; apiInformation.json for apiVersionSetId; diagnostics for their + * logger; logger and backend credentials for named values; API and product + * links for their tags and groups. * Fixed-point expansion; --no-transitive bypass. */ import { FilterConfig } from '../models/config.js'; -import { ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.js'; -import { ResourceDescriptor } from '../models/types.js'; +import { isBuiltInGroup, ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.js'; +import { AssociationEntry, ResourceDescriptor } from '../models/types.js'; import type { IArtifactStore } from '../clients/iartifact-store.js'; import { logger } from '../lib/logger.js'; import { getResourceDescriptorKey } from '../lib/resource-path.js'; @@ -18,8 +20,35 @@ import { getResourceDescriptorKey } from '../lib/resource-path.js'; * Reference detection patterns for policy XML content. */ const NAMED_VALUE_PATTERN = /\{\{([^}]+)\}\}/g; -const BACKEND_PATTERN = /`, + * lets a value contain `>` (as policy expressions can). + */ +const XML_ATTRIBUTE = String.raw`\s+[\w:.-]+\s*=\s*(?:"[^"]*"|'[^']*')`; + +/** + * Build a pattern matching `attribute="value"` (either quote) anywhere on an + * `element` start tag; the value is capture group 2. + */ +function attributePattern(element: string, attribute: string): RegExp { + return new RegExp(`<${element}(?:${XML_ATTRIBUTE})*?\\s+${attribute}\\s*=\\s*(["'])(.*?)\\1`, 'g'); +} + +/** Policy attributes that name another resource, by the type they name. */ +const POLICY_REFERENCE_PATTERNS: ReadonlyArray<{ type: ResourceType; pattern: RegExp }> = [ + { type: ResourceType.Backend, pattern: attributePattern('set-backend-service', 'backend-id') }, + { type: ResourceType.Backend, pattern: attributePattern('llm-content-safety', 'backend-id') }, + { + type: ResourceType.Backend, + pattern: attributePattern('(?:azure-openai|llm)-semantic-cache-lookup', 'embeddings-backend-id'), + }, + { type: ResourceType.PolicyFragment, pattern: attributePattern('include-fragment', 'fragment-id') }, + { type: ResourceType.Logger, pattern: attributePattern('log-to-eventhub', 'logger-id') }, + // validate-content's names a schema added to the service. + { type: ResourceType.GlobalSchema, pattern: attributePattern('content', 'schema-id') }, +]; /** * Represents a discovered transitive dependency. @@ -38,43 +67,41 @@ const POLICY_RESOURCE_TYPES = new Set([ ]); /** - * Scan policy XML content for references to other resources. - * - * Detects: - * - Named values: {{namedValueName}} syntax - * - Backends: - * - Policy fragments: + * Scan text for named value references ({{displayName}}). */ -export function scanPolicyReferences(policyXml: string): TransitiveDependency[] { +function scanNamedValueReferences(text: string): TransitiveDependency[] { const dependencies: TransitiveDependency[] = []; - - // Named value references - for (const match of policyXml.matchAll(NAMED_VALUE_PATTERN)) { + for (const match of text.matchAll(NAMED_VALUE_PATTERN)) { if (match[1]) { - dependencies.push({ - type: ResourceType.NamedValue, - name: match[1].trim(), - }); + dependencies.push({ type: ResourceType.NamedValue, name: match[1].trim() }); } } + return dependencies; +} - // Backend references - for (const match of policyXml.matchAll(BACKEND_PATTERN)) { - if (match[1]) { - dependencies.push({ - type: ResourceType.Backend, - name: match[1].trim(), - }); - } - } +/** + * Scan policy XML content for references to other resources. + * + * Detects, with the attribute anywhere on the element and in either quote: + * - Named values: {{namedValueName}} syntax + * - Backends: , , + * and embeddings-backend-id on the semantic cache lookup policies + * - Policy fragments: + * - Loggers: + * - Schemas: validate-content's + * + * A value that is a policy expression (`@...`) or a named value reference + * (`{{...}}`, found by the named value scan) does not name a resource. + */ +export function scanPolicyReferences(policyXml: string): TransitiveDependency[] { + const dependencies = scanNamedValueReferences(policyXml); - // Policy fragment references - for (const match of policyXml.matchAll(FRAGMENT_PATTERN)) { - if (match[1]) { - dependencies.push({ - type: ResourceType.PolicyFragment, - name: match[1].trim(), - }); + for (const { type, pattern } of POLICY_REFERENCE_PATTERNS) { + for (const match of policyXml.matchAll(pattern)) { + const name = match[2]?.trim(); + if (name && !name.startsWith('@') && !name.includes('{{')) { + dependencies.push({ type, name }); + } } } @@ -228,6 +255,42 @@ export function findTransitiveDependencies( for (const { descriptor, json } of resources) { const properties = json.properties as Record | undefined; + // A diagnostic sends its data to the logger named by properties.loggerId, in the diagnostic's own + // scope. APIM returns a workspace diagnostic's loggerId without the workspace segment + // (.../service/{name}/loggers/{logger}), so the ID can't say which scope the logger is in; publish + // makes the same assumption when it rebuilds the ID. + if (descriptor.type === ResourceType.Diagnostic || descriptor.type === ResourceType.ApiDiagnostic) { + const loggerId = properties?.loggerId; + const name = typeof loggerId === 'string' ? extractResourceNameFromId(loggerId, 'loggers') : undefined; + if (name) { + dependencies.push({ + type: ResourceType.Logger, + nameParts: [name], + workspace: descriptor.workspace, + }); + } + } + + // Logger and backend credentials can hold named value references (for + // example the Application Insights key APIM stores as a named value). + if ( + (descriptor.type === ResourceType.Logger || descriptor.type === ResourceType.Backend) && + properties?.credentials !== undefined + ) { + for (const dep of scanNamedValueReferences(JSON.stringify(properties.credentials))) { + dependencies.push({ type: dep.type, nameParts: [dep.name], workspace: descriptor.workspace }); + } + } + + // An API's tag link needs the tag itself. + if (descriptor.type === ResourceType.ApiTag && descriptor.nameParts[1]) { + dependencies.push({ + type: ResourceType.Tag, + nameParts: [descriptor.nameParts[1]], + workspace: descriptor.workspace, + }); + } + if (descriptor.type === ResourceType.Backend) { const pool = isRecord(properties?.pool) ? properties.pool : undefined; const services = pool?.services; @@ -266,11 +329,47 @@ export function findTransitiveDependencies( return deduplicateDescriptors(dependencies); } +/** + * The groups or tags a product's links (groups.json, tags.json) need. A link + * recorded at service scope targets a service-level resource even when the + * product is in a workspace. Built-in groups are left out. + */ +export function findAssociationDependencies( + owner: ResourceDescriptor, + associationType: 'groups' | 'tags', + entries: ReadonlyArray +): ResourceDescriptor[] { + return deduplicateDescriptors( + entries + .map((entry) => associationTarget(owner, associationType, entry)) + // Built-in groups exist in every APIM instance, so they are not dependencies. + .filter((target) => !isBuiltInGroup(target)) + ); +} + +/** + * The group or tag a product's link entry points to. An entry recorded at + * service scope targets a service-level resource even when the product is + * in a workspace. + */ +export function associationTarget( + owner: ResourceDescriptor, + associationType: 'groups' | 'tags', + entry: AssociationEntry +): ResourceDescriptor { + return { + type: associationType === 'groups' ? ResourceType.Group : ResourceType.Tag, + nameParts: [entry.name], + workspace: entry.scope === 'service' ? undefined : owner.workspace, + }; +} + /** * Read intrinsic dependencies from one on-disk artifact. * - * Association and subscription targets are links to independently selected - * composite resources, not transitive dependencies. + * A product's group and tag links pull in those groups and tags. API, + * product and gateway targets of links and subscriptions are independently + * selected composite resources, not transitive dependencies. */ export async function scanArtifactReferences( store: IArtifactStore, @@ -308,6 +407,13 @@ export async function scanArtifactReferences( ) ); + if (descriptor.type === ResourceType.Product) { + for (const associationType of ['groups', 'tags'] as const) { + const entries = await store.readAssociation(sourceDir, descriptor, associationType); + references.push(...findAssociationDependencies(descriptor, associationType, entries)); + } + } + return deduplicateDescriptors(references); } diff --git a/src/services/workspace-extractor.ts b/src/services/workspace-extractor.ts index a91d3c14..83f3ee9c 100644 --- a/src/services/workspace-extractor.ts +++ b/src/services/workspace-extractor.ts @@ -16,6 +16,7 @@ import { extractProductResources, extractWorkspaceProductTags } from './product- import { logger } from '../lib/logger.js'; import { getNamePart } from '../lib/resource-path.js'; import { + isExplicitlyExcluded, isWildcardPattern, resolveWorkspaceFilter, shouldIncludeResource, @@ -24,6 +25,7 @@ import { extractTransitiveDependencies, type TransitiveResourceArtifact, } from './transitive-extractor.js'; +import { findAssociationDependencies } from './transitive-resolver.js'; export { resolveWorkspaceFilter }; @@ -50,6 +52,9 @@ export interface WorkspaceExtractionResult { * @param context - APIM service context * @param outputDir - Output directory * @param filter - Optional filter config (workspace names come from here) + * @param includeTransitive - Whether to pull in the resources extracted ones depend on + * @param serviceExtracted - Resources the service-level extraction already + * wrote or tried, so service-level dependencies are not fetched again * @returns Results per workspace */ export async function extractWorkspaces( @@ -58,7 +63,8 @@ export async function extractWorkspaces( context: ApimServiceContext, outputDir: string, filter?: FilterConfig, - includeTransitive: boolean = false + includeTransitive: boolean = false, + serviceExtracted: ReadonlyArray = [] ): Promise { const results: WorkspaceExtractionResult[] = []; let workspaceNames: string[]; @@ -109,6 +115,13 @@ export async function extractWorkspaces( logger.info(`Extracting ${workspaceNames.length} workspace(s)...`); + // As at service level, dependencies are only resolved when a filter + // narrows what is extracted. Service-level dependencies are shared by + // every workspace, so each is fetched once. + const transitive: WorkspaceTransitiveOptions | undefined = includeTransitive && filter + ? { serviceDependencies: serviceExtracted.filter((descriptor) => !descriptor.workspace) } + : undefined; + for (const wsName of workspaceNames) { // Persist the workspace container itself so publish can recreate it // before workspace-scoped children (named values, APIs, products, etc.). @@ -126,7 +139,8 @@ export async function extractWorkspaces( const wsResult = await extractWorkspace( client, store, context, wsName, outputDir, resolveWorkspaceFilter(wsName, filter), - includeTransitive + filter, + transitive ); wsResult.errorCount += workspaceContainerError; results.push(wsResult); @@ -135,6 +149,14 @@ export async function extractWorkspaces( return results; } +interface WorkspaceTransitiveOptions { + /** + * Service-level resources already extracted; each workspace adds the ones + * it tried, extracted or failed, so they are not tried again. + */ + serviceDependencies: ResourceDescriptor[]; +} + async function extractWorkspace( client: IApimClient, store: IArtifactStore, @@ -142,7 +164,13 @@ async function extractWorkspace( workspaceName: string, outputDir: string, filter?: FilterConfig, - includeTransitive: boolean = false + /** + * The whole filter: it decides for service-level resources and, through + * its workspace sub-filters, for workspace ones. + */ + serviceFilter?: FilterConfig, + /** Set when dependencies are resolved. */ + transitive?: WorkspaceTransitiveOptions ): Promise { logger.info(`Extracting workspace "${workspaceName}"...`); @@ -202,6 +230,12 @@ async function extractWorkspace( ); const apiName = getNamePart(api.descriptor.nameParts, 0); apis.set(apiName, api.json); + // Diagnostics reference their logger. + for (const diagnostic of apiResult.diagnostics) { + if (diagnostic.status === 'success') { + resources.push({ descriptor: diagnostic.descriptor, json: diagnostic.json }); + } + } for (let index = 0; index < apiResult.policies.length; index++) { const policy = apiResult.policies[index]; if (policy !== undefined) { @@ -226,7 +260,7 @@ async function extractWorkspace( try { const productResult = await extractProductResources( client, store, wsContext, product.descriptor, - outputDir, filter, workspaceName + outputDir, serviceFilter, workspaceName ); const productName = getNamePart(product.descriptor.nameParts, 0); for (let index = 0; index < productResult.policies.length; index++) { @@ -249,14 +283,31 @@ async function extractWorkspace( } } + // Workspace tag links can only be listed per tag (one apiLinks and one + // productLinks request each). When the filter leaves some tags out and + // transitive resolution is on, look on every workspace tag the filter does + // not exclude by name, so tags linked to the extracted APIs and products + // are found and pulled in below; otherwise the extracted tags are all + // there is to check. + const hasLinkOwners = extractedApiNames.size > 0 || extractedProducts.length > 0; + const linkTagNames = transitive && hasLinkOwners && leavesOutUnnamed(filter?.tags) + ? (await listWorkspaceTagNames(client, wsContext, workspaceName, extractedTagNames)).filter( + (name) => !isExplicitlyExcluded( + { type: ResourceType.Tag, nameParts: [name], workspace: workspaceName }, + serviceFilter + ) + ) + : extractedTagNames; + // Extract workspace API tags using the tag-centric apiLinks endpoint. // This must happen after both Tags and APIs are extracted since the // workspace scope uses `tags/{tag}/apiLinks` (inverted parent-child). - if (extractedTagNames.length > 0 && extractedApiNames.size > 0) { + if (linkTagNames.length > 0 && extractedApiNames.size > 0) { try { const apiTagCount = await extractWorkspaceApiTags( - client, store, wsContext, extractedTagNames, extractedApiNames, - outputDir, workspaceName + client, store, wsContext, linkTagNames, extractedApiNames, + outputDir, workspaceName, + (descriptor, json) => resources.push({ descriptor, json }) ); resourceCount += apiTagCount; } catch (error) { @@ -267,10 +318,10 @@ async function extractWorkspace( // Extract workspace product tags using the tag-centric productLinks endpoint. // Similar to ApiTag, workspace ProductTag uses `tags/{tag}/productLinks`. - if (extractedTagNames.length > 0 && extractedProducts.length > 0) { + if (linkTagNames.length > 0 && extractedProducts.length > 0) { try { const productTagCount = await extractWorkspaceProductTags( - client, store, wsContext, extractedTagNames, extractedProducts, + client, store, wsContext, linkTagNames, extractedProducts, outputDir, workspaceName ); resourceCount += productTagCount; @@ -280,8 +331,19 @@ async function extractWorkspace( } } - if (includeTransitive && filter) { + // Without a sub-filter the whole workspace was extracted, and its resources + // depend only on resources in it: APIM doesn't let a workspace product link + // to a custom service-level group, and the built-in ones are never pulled in. + if (transitive && filter) { logger.info(`Resolving transitive dependencies for workspace "${workspaceName}"...`); + // The groups and tags the extracted products link to. + const linkedDependencies: ResourceDescriptor[] = []; + for (const product of extractedProducts) { + for (const associationType of ['groups', 'tags'] as const) { + const entries = await store.readAssociation(outputDir, product.descriptor, associationType); + linkedDependencies.push(...findAssociationDependencies(product.descriptor, associationType, entries)); + } + } const transitiveResult = await extractTransitiveDependencies( client, store, @@ -290,9 +352,18 @@ async function extractWorkspace( policies, apis, resources, - extractedDescriptors, - workspaceName, - context + [...extractedDescriptors, ...transitive.serviceDependencies], + { + workspace: workspaceName, + serviceContext: context, + linkedDependencies, + skip: (dep) => isExplicitlyExcluded(dep, serviceFilter), + } + ); + // Later workspaces skip the service-level ones, including any that failed. + transitive.serviceDependencies.push( + ...[...transitiveResult.extractedDescriptors, ...transitiveResult.failedDescriptors] + .filter((descriptor) => !descriptor.workspace) ); resourceCount += transitiveResult.extractedDescriptors.length; errorCount += transitiveResult.errorCount; @@ -303,6 +374,46 @@ async function extractWorkspace( return { workspaceName, resourceCount, errorCount }; } +/** + * Whether a filter list leaves out resources it does not name: it is `[]` or + * names some to include. A list of exclusions only (`['!internal']`), or one + * including `*`, keeps everything else, so all it leaves out is excluded by + * name. + */ +function leavesOutUnnamed(list: string[] | undefined): boolean { + if (list === undefined) { + return false; + } + const includes = list.filter((entry) => !entry.startsWith('!')); + return list.length === 0 || (includes.length > 0 && !includes.includes('*')); +} + +/** + * Every tag in the workspace, or the extracted ones if the list fails. + */ +async function listWorkspaceTagNames( + client: IApimClient, + wsContext: ApimServiceContext, + workspaceName: string, + extractedTagNames: string[] +): Promise { + const names: string[] = []; + try { + for await (const json of client.listResources(wsContext, ResourceType.Tag)) { + if (typeof json.name === 'string') { + names.push(json.name); + } + } + return names; + } catch (error) { + logger.warn( + `Failed to list tags in workspace "${workspaceName}"; only links to extracted tags are included: ` + + `${(error as Error).message}` + ); + return extractedTagNames; + } +} + /** * Discover all workspace names from APIM. */ diff --git a/tests/unit/services/api-product-extractor.test.ts b/tests/unit/services/api-product-extractor.test.ts index 23017257..b8a2d908 100644 --- a/tests/unit/services/api-product-extractor.test.ts +++ b/tests/unit/services/api-product-extractor.test.ts @@ -1493,6 +1493,65 @@ describe('product-extractor', () => { expect(result.errorCount).toBe(1); }); + it('leaves out links to groups and tags the filter excludes by name', async () => { + const client = createMockClient(); + client.listResources = async function* (_ctx, type) { + if (type === ResourceType.ProductGroup) { + yield { name: 'partners' }; + yield { name: 'support-team' }; + } + if (type === ResourceType.ProductTag) { + yield { name: 'internal' }; + yield { name: 'public' }; + } + }; + client.getResource = vi.fn().mockResolvedValue(undefined); + const store = createMockStore(); + const filter: FilterConfig = { products: ['starter'], groups: ['!partners', '*'], tags: ['!internal'] }; + + const result = await extractProductResources( + client, store, testContext, + { type: ResourceType.Product, nameParts: ['starter'] }, + '/output', filter + ); + + expect(result.groups).toEqual(['support-team']); + expect(result.tags).toEqual(['public']); + expect(store.writeAssociation).toHaveBeenCalledWith('/output', expect.anything(), 'groups', [{ name: 'support-team' }]); + expect(store.writeAssociation).toHaveBeenCalledWith('/output', expect.anything(), 'tags', [{ name: 'public' }]); + }); + + it("decides a workspace product's links by the scope each target is in", async () => { + const SERVICE = '/subscriptions/s/resourceGroups/r/providers/Microsoft.ApiManagement/service/a'; + const client = createMockClient(); + client.listResources = async function* (_ctx, type) { + if (type === ResourceType.ProductGroup) { + yield { name: 'l1', properties: { groupId: `${SERVICE}/groups/partners` } }; + yield { name: 'l2', properties: { groupId: `${SERVICE}/workspaces/team-a/groups/partners` } }; + yield { name: 'l3', properties: { groupId: `${SERVICE}/workspaces/team-a/groups/ops` } }; + } + }; + client.getResource = vi.fn().mockResolvedValue(undefined); + const store = createMockStore(); + const filter: FilterConfig = { + groups: ['!partners', '*'], + workspaceSubFilters: { 'team-a': { groups: ['!ops', '*'] } }, + }; + + const result = await extractProductResources( + client, store, { ...testContext, baseUrl: `${testContext.baseUrl}/workspaces/team-a` }, + { type: ResourceType.Product, nameParts: ['starter'], workspace: 'team-a' }, + '/output', filter, 'team-a' + ); + + // The service-level partners group is excluded at the top level; the + // workspace's own partners group is not, but its ops group is. + expect(result.groups).toEqual(['partners']); + expect(store.writeAssociation).toHaveBeenCalledWith( + '/output', expect.anything(), 'groups', [{ name: 'partners', scope: 'workspace' }] + ); + }); + it('counts failed product associations and wiki extraction', async () => { const client = createMockClient(); // eslint-disable-next-line require-yield diff --git a/tests/unit/services/api-publisher.test.ts b/tests/unit/services/api-publisher.test.ts index 6e3592fa..3f37f1b9 100644 --- a/tests/unit/services/api-publisher.test.ts +++ b/tests/unit/services/api-publisher.test.ts @@ -771,7 +771,8 @@ describe('api-publisher', () => { store, testContext, expect.objectContaining({ type: ResourceType.ApiPolicy, nameParts: ['orders-api'] }), - testConfig + testConfig, + undefined ); }); diff --git a/tests/unit/services/extract-service.test.ts b/tests/unit/services/extract-service.test.ts index 090b8209..3b35e87f 100644 --- a/tests/unit/services/extract-service.test.ts +++ b/tests/unit/services/extract-service.test.ts @@ -273,6 +273,105 @@ describe('extract-service', () => { expect(result.collectedPolicies.get('service-policy')).toBe(redactedPolicy); }); + it("should pull in an API diagnostic's logger, an API's tags, and a product's groups and tags", async () => { + const client = createMockClient({ + [ResourceType.Api]: [{ name: 'orders', properties: {} }], + [ResourceType.ApiDiagnostic]: [{ name: 'applicationinsights', properties: { loggerId: '/loggers/app-insights' } }], + [ResourceType.ApiTag]: [{ name: 'public', properties: { displayName: 'public' } }], + [ResourceType.Product]: [{ name: 'starter', properties: {} }], + }); + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => + [ResourceType.Logger, ResourceType.Tag, ResourceType.Group].includes(descriptor.type) + ? { name: descriptor.nameParts[0], properties: {} } + : undefined + ); + const store = createMockStore(); + store.readAssociation.mockImplementation(async (_dir: string, _descriptor: ResourceDescriptor, type: string) => + type === 'groups' ? [{ name: 'support-team' }] : type === 'tags' ? [{ name: 'partner' }] : [] + ); + const filter: FilterConfig = { + apis: ['orders'], + products: ['starter'], + loggers: [], + tags: [], + groups: [], + }; + + const result = await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + filter, + includeTransitive: true, + logLevel: LogLevel.INFO, + }); + + expect(result.extractedDescriptors).toEqual(expect.arrayContaining([ + { type: ResourceType.Logger, nameParts: ['app-insights'], workspace: undefined }, + { type: ResourceType.Tag, nameParts: ['public'], workspace: undefined }, + { type: ResourceType.Group, nameParts: ['support-team'], workspace: undefined }, + { type: ResourceType.Tag, nameParts: ['partner'], workspace: undefined }, + ])); + }); + + it('should not try again in a workspace a service-level dependency the service pass failed to extract', async () => { + const client = createMockClient({ + [ResourceType.Product]: [{ name: 'starter', properties: {} }], + }); + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => + descriptor.type === ResourceType.Group ? undefined : { name: descriptor.nameParts[0], properties: {} } + ); + const store = createMockStore(); + store.readAssociation.mockImplementation(async (_dir: string, _descriptor: ResourceDescriptor, type: string) => + type === 'groups' ? [{ name: 'old-team', scope: 'service' }] : [] + ); + + const result = await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + filter: { products: ['starter'], groups: [], workspaces: ['team-a'] }, + includeTransitive: true, + logLevel: LogLevel.INFO, + }); + + const groupGets = client.getResource.mock.calls.filter(([, d]) => (d as ResourceDescriptor).type === ResourceType.Group); + expect(groupGets).toHaveLength(1); + expect(result.workspaceResults?.[0]?.errorCount).toBe(0); + }); + + it('should not pull in a tag the filter excludes by name', async () => { + const client = createMockClient({ + [ResourceType.Api]: [{ name: 'orders', properties: {} }], + [ResourceType.ApiTag]: [ + { name: 'internal', properties: { displayName: 'internal' } }, + { name: 'public', properties: { displayName: 'public' } }, + ], + }); + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => + descriptor.type === ResourceType.Tag ? { name: descriptor.nameParts[0], properties: {} } : undefined + ); + + const store = createMockStore(); + await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + filter: { apis: ['orders'], tags: ['!internal', 'none'] }, + includeTransitive: true, + logLevel: LogLevel.INFO, + }); + + const fetchedTags = client.getResource.mock.calls + .map(([, descriptor]) => descriptor as ResourceDescriptor) + .filter((descriptor) => descriptor.type === ResourceType.Tag) + .map((descriptor) => descriptor.nameParts[0]); + expect(fetchedTags).toEqual(['public']); + // The link to the excluded tag is not written either. + expect(store.writeResource).not.toHaveBeenCalledWith( + '/output', + expect.objectContaining({ type: ResourceType.ApiTag, nameParts: ['orders', 'internal'] }), + expect.anything() + ); + }); + it('should not crash during transitive dedupe when a singleton (ServicePolicy) is already extracted', async () => { // Regression: transitive resolution previously keyed the // already-extracted set on getNamePart(d.nameParts, 0), which threw diff --git a/tests/unit/services/filter-service.test.ts b/tests/unit/services/filter-service.test.ts index 169b9277..af96e35c 100644 --- a/tests/unit/services/filter-service.test.ts +++ b/tests/unit/services/filter-service.test.ts @@ -11,11 +11,13 @@ import { FilterConfig } from '../../../src/models/config.js'; import { shouldIncludeResource, shouldReconcileResource, + isInRemoveStaleScope, filterResources, extractRootApiName, isWildcardPattern, wildcardToRegex, wildcardMatch, + isExplicitlyExcluded, } from '../../../src/services/filter-service.js'; describe('filter-service', () => { @@ -49,6 +51,22 @@ describe('filter-service', () => { }); }); + describe('isInRemoveStaleScope', () => { + const internalLink: ResourceDescriptor = { type: ResourceType.ApiTag, nameParts: ['orders', 'internal'] }; + + it("should count an API's link to an excluded tag as the API's to manage", () => { + const filter: FilterConfig = { apis: ['orders'], tags: ['!internal'] }; + + expect(shouldReconcileResource(internalLink, filter)).toBe(false); + expect(isInRemoveStaleScope(internalLink, filter)).toBe(true); + expect(isInRemoveStaleScope({ type: ResourceType.Tag, nameParts: ['internal'] }, filter)).toBe(false); + }); + + it("should leave an unselected API's tag links alone", () => { + expect(isInRemoveStaleScope(internalLink, { apis: ['billing'] })).toBe(false); + }); + }); + describe('shouldIncludeResource', () => { it('should include all resources when no filter is provided', () => { const descriptor: ResourceDescriptor = { @@ -345,6 +363,45 @@ describe('filter-service', () => { }); }); + describe('isExplicitlyExcluded', () => { + it.each([ + ['a name excluded by an exclusion entry', { tags: ['!internal', '*'] }, 'internal', true], + ['a name matched by a wildcard exclusion', { tags: ['!int*'] }, 'internal', true], + ['a name the exclusions do not match', { tags: ['!internal', '*'] }, 'public', false], + ['a type set to []', { tags: [] }, 'internal', false], + ['a list that just does not name it', { tags: ['public'] }, 'internal', false], + ])('should report %s', (_label, filter, name, expected) => { + expect(isExplicitlyExcluded({ type: ResourceType.Tag, nameParts: [name] }, filter as FilterConfig)).toBe(expected); + }); + + it("should check a workspace resource against its workspace's sub-filter", () => { + const filter: FilterConfig = { + tags: ['!shared'], + workspaces: ['team-a'], + workspaceSubFilters: { 'team-a': { tags: ['!internal'] } }, + }; + + expect(isExplicitlyExcluded({ type: ResourceType.Tag, nameParts: ['internal'], workspace: 'team-a' }, filter)).toBe(true); + expect(isExplicitlyExcluded({ type: ResourceType.Tag, nameParts: ['shared'], workspace: 'team-a' }, filter)).toBe(false); + expect(isExplicitlyExcluded({ type: ResourceType.Tag, nameParts: ['shared'] }, filter)).toBe(true); + }); + }); + + describe('links to tags excluded by name', () => { + const link = (tag: string): ResourceDescriptor => ({ type: ResourceType.ApiTag, nameParts: ['orders', tag] }); + + it("should leave out an API's link to a tag the filter excludes by name", () => { + const filter: FilterConfig = { apis: ['orders'], tags: ['!internal', '*'] }; + + expect(shouldIncludeResource(link('internal'), filter)).toBe(false); + expect(shouldIncludeResource(link('public'), filter)).toBe(true); + }); + + it('should keep links to tags the filter leaves out with [] (they are pulled in as dependencies)', () => { + expect(shouldIncludeResource(link('internal'), { apis: ['orders'], tags: [] })).toBe(true); + }); + }); + describe('extractRootApiName', () => { it('should return name as-is when no revision suffix', () => { expect(extractRootApiName('my-api')).toBe('my-api'); diff --git a/tests/unit/services/product-publisher.test.ts b/tests/unit/services/product-publisher.test.ts index 27d1a833..c41b0e8d 100644 --- a/tests/unit/services/product-publisher.test.ts +++ b/tests/unit/services/product-publisher.test.ts @@ -268,6 +268,52 @@ describe('product-publisher', () => { ); }); + it('links groups and tags that were pulled into the publish set although the filter excludes them', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readAssociation.mockImplementation(async (_dir: string, _descriptor: ResourceDescriptor, type: string) => + type === 'groups' ? [{ name: 'support-team' }] : type === 'tags' ? [{ name: 'partner' }, { name: 'internal' }] : [] + ); + const config: PublishConfig = { + ...testConfig, + filter: { products: ['my-product'], groups: [], tags: [] }, + }; + + await publishProduct(client, store, testContext, productDescriptor, config, [ + productDescriptor, + { type: ResourceType.Group, nameParts: ['support-team'] }, + { type: ResourceType.Tag, nameParts: ['partner'] }, + ]); + + const links = client.putResource.mock.calls + .map(([, descriptor]) => `${descriptor.type}:${descriptor.nameParts.join('/')}`) + .filter((key) => key.startsWith('ProductGroup') || key.startsWith('ProductTag')); + expect(links).toEqual(['ProductGroup:my-product/support-team', 'ProductTag:my-product/partner']); + }); + + it('publishes links to built-in groups only when the filter includes them', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readAssociation.mockImplementation(async (_dir: string, _descriptor: ResourceDescriptor, type: string) => + type === 'groups' ? [{ name: 'developers' }] : [] + ); + const developers: ResourceDescriptor = { type: ResourceType.Group, nameParts: ['developers'] }; + const linksFor = async (groups: string[]) => { + client.putResource.mockClear(); + // A group the filter includes is in the publish set, as after extracting it. + await publishProduct(client, store, testContext, productDescriptor, { + ...testConfig, + filter: { products: ['my-product'], groups }, + }, groups.length > 0 ? [productDescriptor, developers] : [productDescriptor]); + return client.putResource.mock.calls + .map(([, descriptor]) => `${descriptor.type}:${descriptor.nameParts.join('/')}`) + .filter((key) => key.startsWith('ProductGroup')); + }; + + expect(await linksFor([])).toEqual([]); + expect(await linksFor(['developers'])).toEqual(['ProductGroup:my-product/developers']); + }); + it('does not let resolved targets override explicit API exclusions', async () => { const client = createMockClient(); const store = createMockStore(); diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 6bcac061..f4426d9e 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -8,7 +8,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { ResourceType } from '../../../src/models/resource-types.js'; import { ResourceDescriptor, ApimServiceContext } from '../../../src/models/types.js'; import { PublishConfig } from '../../../src/models/config.js'; -import { LogLevel } from '../../../src/lib/logger.js'; +import { LogLevel, logger } from '../../../src/lib/logger.js'; import { recordRetry, withRetryResource } from '../../../src/lib/retry-tracker.js'; import { getResourceDescriptorKey } from '../../../src/lib/resource-path.js'; @@ -482,6 +482,72 @@ describe('publish-service', () => { expect(publishApi).not.toHaveBeenCalled(); }); + it('should prefer the named value whose display name a reference uses over one with that resource name', async () => { + const resources = [ + { type: ResourceType.PolicyFragment, nameParts: ['auth'] }, + { type: ResourceType.NamedValue, nameParts: ['one'] }, + { type: ResourceType.NamedValue, nameParts: ['two'] }, + ]; + const client = createMockClient(); + const store = createMockStore(resources); + store.readResource.mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.PolicyFragment) { + return { properties: { value: '{{TWO}}' } }; + } + return { properties: { displayName: descriptor.nameParts[0] === 'one' ? 'two' : 'three', value: 'x' } }; + }); + + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter: { policyFragments: ['auth'], namedValues: [] }, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + + const namedValuePuts = client.putResource.mock.calls + .filter((call) => call[1].type === ResourceType.NamedValue) + .map((call) => call[1].nameParts[0]); + expect(namedValuePuts).toEqual(['one']); + }); + + it('should warn and match by resource name when a named value artifact cannot be read', async () => { + const warn = vi.spyOn(logger, 'warn').mockImplementation(() => undefined); + const resources = [ + { type: ResourceType.PolicyFragment, nameParts: ['auth'] }, + { type: ResourceType.NamedValue, nameParts: ['broken'] }, + { type: ResourceType.NamedValue, nameParts: ['plain'] }, + ]; + const client = createMockClient(); + const store = createMockStore(resources); + let namedValueReads = 0; + store.readResource.mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.PolicyFragment) { + return { properties: { value: '{{plain}}' } }; + } + if (descriptor.nameParts[0] === 'broken' && ++namedValueReads === 1) { + throw new Error('Unexpected token in JSON'); + } + return { properties: { displayName: descriptor.nameParts[0], value: 'x' } }; + }); + + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter: { policyFragments: ['auth'], namedValues: [] }, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('to match {{references}} by display name')); + expect(client.putResource.mock.calls + .filter((call) => call[1].type === ResourceType.NamedValue) + .map((call) => call[1].nameParts[0])).toEqual(['plain']); + warn.mockRestore(); + }); + it('should include backend pool members transitively', async () => { const resources = [ { type: ResourceType.Backend, nameParts: ['pool'] }, @@ -520,6 +586,71 @@ describe('publish-service', () => { )).toBe(true); }); + it("should include an API diagnostic's logger, an API's tags, and a product's groups and tags transitively", async () => { + const resources = [ + { type: ResourceType.Api, nameParts: ['orders'] }, + { type: ResourceType.ApiDiagnostic, nameParts: ['orders', 'applicationinsights'] }, + { type: ResourceType.ApiTag, nameParts: ['orders', 'public'] }, + { type: ResourceType.Product, nameParts: ['starter'] }, + { type: ResourceType.Logger, nameParts: ['app-insights'] }, + { type: ResourceType.Logger, nameParts: ['unrelated'] }, + { type: ResourceType.Tag, nameParts: ['public'] }, + { type: ResourceType.Tag, nameParts: ['partner'] }, + { type: ResourceType.Group, nameParts: ['support-team'] }, + ]; + const client = createMockClient(); + const store = createMockStore(resources); + store.readResource.mockImplementation(async (_sourceDir, descriptor) => + descriptor.type === ResourceType.ApiDiagnostic + ? { properties: { loggerId: '/loggers/app-insights' } } + : { properties: {} } + ); + store.readAssociation.mockImplementation(async (_sourceDir, _descriptor, type) => + type === 'groups' ? [{ name: 'support-team' }] : type === 'tags' ? [{ name: 'partner' }] : [] + ); + + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter: { apis: ['orders'], products: ['starter'], loggers: [], tags: [], groups: [] }, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + + const put = client.putResource.mock.calls.map((call) => `${call[1].type}:${call[1].nameParts.join('/')}`); + expect(put).toEqual(expect.arrayContaining([ + 'Logger:app-insights', + 'Tag:public', + 'Tag:partner', + 'Group:support-team', + ])); + expect(put).not.toContain('Logger:unrelated'); + }); + + it('should not pull in a tag the filter excludes by name', async () => { + const resources = [ + { type: ResourceType.Api, nameParts: ['orders'] }, + { type: ResourceType.ApiTag, nameParts: ['orders', 'internal'] }, + { type: ResourceType.Tag, nameParts: ['internal'] }, + ]; + const client = createMockClient(); + const store = createMockStore(resources); + store.readResource.mockResolvedValue({ properties: {} }); + + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter: { apis: ['orders'], tags: ['!internal'] }, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + + const put = client.putResource.mock.calls.map((call) => `${call[1].type}:${call[1].nameParts.join('/')}`); + expect(put).not.toContain('Tag:internal'); + }); + it('should not include transitive dependencies when disabled', async () => { const resources = [ { type: ResourceType.Api, nameParts: ['orders'] }, diff --git a/tests/unit/services/resource-publisher.test.ts b/tests/unit/services/resource-publisher.test.ts index 8e1154e5..923dfcf2 100644 --- a/tests/unit/services/resource-publisher.test.ts +++ b/tests/unit/services/resource-publisher.test.ts @@ -749,6 +749,37 @@ describe('resource-publisher', () => { expect(client.putResource).not.toHaveBeenCalled(); }); + it('should publish an ApiTag link whose excluded Tag was pulled into the publish set', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readResource.mockResolvedValue({ properties: { displayName: 'public' } }); + const descriptor: ResourceDescriptor = { type: ResourceType.ApiTag, nameParts: ['orders', 'public'] }; + const tag: ResourceDescriptor = { type: ResourceType.Tag, nameParts: ['public'] }; + const config: PublishConfig = { ...testConfig, filter: { apis: ['orders'], tags: [] } }; + + const pulledIn = await publishResource(client, store, testContext, descriptor, config, [descriptor, tag]); + const notPulledIn = await publishResource(client, store, testContext, descriptor, config, [descriptor]); + + expect(pulledIn.status).toBe('success'); + expect(notPulledIn.status).toBe('skipped'); + expect(client.putResource).toHaveBeenCalledTimes(1); + }); + + it('should skip a link to a target in the publish set that the filter excludes by name', async () => { + const client = createMockClient(); + const store = createMockStore(); + const descriptor: ResourceDescriptor = { type: ResourceType.ApiTag, nameParts: ['orders', 'internal'] }; + const config: PublishConfig = { ...testConfig, filter: { apis: ['orders'], tags: ['!internal', '*'] } }; + + const result = await publishResource(client, store, testContext, descriptor, config, [ + descriptor, + { type: ResourceType.Tag, nameParts: ['internal'] }, + ]); + + expect(result.status).toBe('skipped'); + expect(client.putResource).not.toHaveBeenCalled(); + }); + it('should publish a GatewayApi association when its target passes an explicit filter and was extracted', async () => { const client = createMockClient(); const store = createMockStore(); diff --git a/tests/unit/services/transitive-extractor.test.ts b/tests/unit/services/transitive-extractor.test.ts index 31ead3d4..9c12274e 100644 --- a/tests/unit/services/transitive-extractor.test.ts +++ b/tests/unit/services/transitive-extractor.test.ts @@ -90,6 +90,22 @@ describe('transitive-extractor', () => { ); }); + it('should not extract a {{displayName}} reference whose resource name is excluded', async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'my-secret', displayName: 'My Secret' }); + client.getResource.mockResolvedValue({ name: 'my-secret', properties: { displayName: 'My Secret' } }); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('My Secret'), new Map(), [], [], + { skip: (dep) => dep.nameParts[0] === 'my-secret' } + ); + + expect(result).toEqual({ extractedDescriptors: [], failedDescriptors: [], errorCount: 0 }); + expect(client.getResource).not.toHaveBeenCalled(); + expect(store.writeResource).not.toHaveBeenCalled(); + }); + 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' }); @@ -136,6 +152,22 @@ describe('transitive-extractor', () => { expect(namedValueLists).toHaveLength(1); }); + it('should match a {{displayName}} reference ignoring case', async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'nv-resource', displayName: 'NV_Display' }); + client.getResource.mockImplementation(async (_ctx, descriptor: ResourceDescriptor) => ({ + name: descriptor.nameParts[0], + properties: {}, + })); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('nv_display'), new Map(), [], [] + ); + + expect(result.extractedDescriptors.map((descriptor) => descriptor.nameParts[0])).toEqual(['nv-resource']); + }); + it('should extract each resolved resource when a resource name is also another display name', async () => { const client = createMockClient(); client.listResources = namedValueList( @@ -222,7 +254,8 @@ describe('transitive-extractor', () => { const store = createMockStore(); const result = await extractTransitiveDependencies( - client, store, wsContext, '/output', policiesWith('ws_display'), new Map(), [], [], 'team-a', testContext + client, store, wsContext, '/output', policiesWith('ws_display'), new Map(), [], [], + { workspace: 'team-a', serviceContext: testContext } ); expect(result.errorCount).toBe(0); diff --git a/tests/unit/services/transitive-resolver.test.ts b/tests/unit/services/transitive-resolver.test.ts index 8d05947e..c6305f4b 100644 --- a/tests/unit/services/transitive-resolver.test.ts +++ b/tests/unit/services/transitive-resolver.test.ts @@ -14,6 +14,7 @@ import { findTransitiveDependencies, findSubscriptionTargets, scanArtifactReferences, + findAssociationDependencies, } from '../../../src/services/transitive-resolver.js'; describe('transitive-resolver', () => { @@ -247,7 +248,7 @@ describe('transitive-resolver', () => { }, }), readContent: vi.fn().mockResolvedValue(undefined), - readAssociation: vi.fn(), + readAssociation: vi.fn().mockResolvedValue([]), }; const backendRefs = await scanArtifactReferences( @@ -272,7 +273,8 @@ describe('transitive-resolver', () => { '/source', { type: ResourceType.Product, nameParts: ['starter'] } )).resolves.toEqual([]); - expect(store.readAssociation).not.toHaveBeenCalled(); + // A product's API links are not dependencies, so they are not read. + expect(store.readAssociation.mock.calls.map((call) => call[2])).not.toContain('apis'); }); it('should identify subscription targets separately from transitive dependencies', () => { @@ -339,4 +341,145 @@ describe('transitive-resolver', () => { }); }); }); + + describe('dependency coverage', () => { + const SERVICE = '/subscriptions/s/resourceGroups/r/providers/Microsoft.ApiManagement/service/a'; + + it.each([ + ['an attribute before backend-id', '', ResourceType.Backend, 'func-backend'], + ['single quotes', "", ResourceType.Backend, 'quoted-backend'], + ['llm-content-safety', '', ResourceType.Backend, 'content-safety'], + ['azure-openai-semantic-cache-lookup', '', ResourceType.Backend, 'embeddings'], + ['llm-semantic-cache-lookup', '', ResourceType.Backend, 'llm-embeddings'], + ['include-fragment with another attribute first', '', ResourceType.PolicyFragment, 'frag-a'], + ['log-to-eventhub', '@(context.RequestId)', ResourceType.Logger, 'eventhub-logger'], + [ + 'validate-content schema-id', + '', + ResourceType.GlobalSchema, + 'order-schema', + ], + ])('should detect a reference written with %s', (_label, policy, type, name) => { + expect(scanPolicyReferences(policy)).toContainEqual({ type, name }); + }); + + it("should find a reference after an attribute whose value contains '>'", () => { + const refs = scanPolicyReferences( + '' + ); + + expect(refs).toEqual([{ type: ResourceType.Backend, name: 'content-safety' }]); + }); + + it('should not treat policy expressions or named value placeholders as resource names', () => { + const refs = scanPolicyReferences( + '' + + '' + ); + + expect(refs).toEqual([{ type: ResourceType.NamedValue, name: 'backend-name' }]); + }); + + it.each([ + [ResourceType.Diagnostic, ['applicationinsights'], `${SERVICE}/loggers/app-insights`, undefined, undefined], + [ResourceType.ApiDiagnostic, ['orders', 'azuremonitor'], '/loggers/app-insights', 'team-a', 'team-a'], + // APIM returns a workspace diagnostic's loggerId without the workspace segment. + [ResourceType.ApiDiagnostic, ['orders', 'applicationinsights'], `${SERVICE}/loggers/app-insights`, 'team-a', 'team-a'], + ])('should follow a %s loggerId to the logger in its own scope', (type, nameParts, loggerId, workspace, loggerWorkspace) => { + const deps = findTransitiveDependencies(new Map(), new Map(), workspace, [ + { descriptor: { type, nameParts, workspace }, json: { properties: { loggerId } } }, + ]); + + expect(deps).toEqual([{ type: ResourceType.Logger, nameParts: ['app-insights'], workspace: loggerWorkspace }]); + }); + + it('should follow named values in logger and backend credentials', () => { + const deps = findTransitiveDependencies(new Map(), new Map(), undefined, [ + { + descriptor: { type: ResourceType.Logger, nameParts: ['app-insights'] }, + json: { properties: { loggerType: 'applicationInsights', credentials: { instrumentationKey: '{{Logger-Credentials--abc}}' } } }, + }, + { + descriptor: { type: ResourceType.Backend, nameParts: ['orders'], workspace: 'team-a' }, + json: { + properties: { + url: 'https://orders.example.com', + credentials: { + header: { 'x-api-key': ['{{orders-key}}'] }, + query: { code: ['{{orders-code}}'] }, + authorization: { scheme: 'Bearer', parameter: '{{orders-token}}' }, + }, + }, + }, + }, + ]); + + expect(deps).toEqual([ + { type: ResourceType.NamedValue, nameParts: ['Logger-Credentials--abc'], workspace: undefined }, + { type: ResourceType.NamedValue, nameParts: ['orders-key'], workspace: 'team-a' }, + { type: ResourceType.NamedValue, nameParts: ['orders-code'], workspace: 'team-a' }, + { type: ResourceType.NamedValue, nameParts: ['orders-token'], workspace: 'team-a' }, + ]); + }); + + it("should follow an API's tag link to the tag", () => { + const deps = findTransitiveDependencies(new Map(), new Map(), 'team-a', [ + { descriptor: { type: ResourceType.ApiTag, nameParts: ['orders', 'public'], workspace: 'team-a' }, json: {} }, + ]); + + expect(deps).toEqual([{ type: ResourceType.Tag, nameParts: ['public'], workspace: 'team-a' }]); + }); + + it("should follow a product's group and tag links, keeping service-scope targets at service level", () => { + const product = { type: ResourceType.Product, nameParts: ['starter'], workspace: 'team-a' }; + + expect(findAssociationDependencies(product, 'groups', [ + { name: 'support-team', scope: 'service' }, + { name: 'partners', scope: 'workspace' }, + { name: 'partners' }, + ])).toEqual([ + { type: ResourceType.Group, nameParts: ['support-team'], workspace: undefined }, + { type: ResourceType.Group, nameParts: ['partners'], workspace: 'team-a' }, + ]); + expect(findAssociationDependencies(product, 'tags', [{ name: 'public' }])).toEqual([ + { type: ResourceType.Tag, nameParts: ['public'], workspace: 'team-a' }, + ]); + }); + + it('should leave built-in groups out of product link dependencies', () => { + expect(findAssociationDependencies({ type: ResourceType.Product, nameParts: ['starter'] }, 'groups', [ + { name: 'Developers' }, + { name: 'guests' }, + { name: 'administrators' }, + { name: 'support-team' }, + ])).toEqual([{ type: ResourceType.Group, nameParts: ['support-team'], workspace: undefined }]); + }); + + it('should treat a workspace group with a built-in name as a custom group', () => { + const product = { type: ResourceType.Product, nameParts: ['starter'], workspace: 'team-a' }; + + expect(findAssociationDependencies(product, 'groups', [ + { name: 'developers', scope: 'workspace' }, + { name: 'administrators', scope: 'service' }, + ])).toEqual([{ type: ResourceType.Group, nameParts: ['developers'], workspace: 'team-a' }]); + }); + + it("should read a product's group and tag links when scanning it for publish", async () => { + const store = { + readResource: vi.fn().mockResolvedValue({ properties: { displayName: 'Starter' } }), + readContent: vi.fn().mockResolvedValue(undefined), + readAssociation: vi.fn(async (_dir: string, _descriptor: unknown, type: string) => + type === 'groups' ? [{ name: 'support-team' }] : [{ name: 'public' }] + ), + }; + + const refs = await scanArtifactReferences(store, '/source', { type: ResourceType.Product, nameParts: ['starter'] }); + + expect(refs).toEqual([ + { type: ResourceType.Group, nameParts: ['support-team'], workspace: undefined }, + { type: ResourceType.Tag, nameParts: ['public'], workspace: undefined }, + ]); + }); + }); }); diff --git a/tests/unit/services/workspace-extractor.test.ts b/tests/unit/services/workspace-extractor.test.ts index e24f1b1f..a462c431 100644 --- a/tests/unit/services/workspace-extractor.test.ts +++ b/tests/unit/services/workspace-extractor.test.ts @@ -10,6 +10,7 @@ import { ApimServiceContext } from '../../../src/models/types.js'; import { FilterConfig } from '../../../src/models/config.js'; import { extractWorkspaces } from '../../../src/services/workspace-extractor.js'; import { resolveWorkspaceFilter } from '../../../src/services/workspace-extractor.js'; +import { logger } from '../../../src/lib/logger.js'; const testContext: ApimServiceContext = { subscriptionId: 'sub-1', @@ -563,6 +564,267 @@ describe('workspace-extractor', () => { }); }); + describe('workspace dependency coverage', () => { + const SERVICE = '/subscriptions/s/resourceGroups/r/providers/Microsoft.ApiManagement/service/a'; + const wsContext = { ...testContext, baseUrl: `${testContext.baseUrl}/workspaces/team-a` }; + + /** A store that keeps association files in memory so they can be read back. */ + function createAssociationStore() { + const store = createMockStore(); + const associations = new Map(); + const key = (descriptor: { nameParts: string[]; workspace?: string }, type: string) => + `${descriptor.workspace ?? ''}/${descriptor.nameParts.join('/')}/${type}`; + store.writeAssociation.mockImplementation(async (_dir, descriptor, type, entries) => { + associations.set(key(descriptor, type), entries); + }); + store.readAssociation.mockImplementation(async (_dir, descriptor, type) => + associations.get(key(descriptor, type)) ?? [] + ); + return store; + } + + it("should pull in an API diagnostic's logger, tags linked through any workspace tag, and a product's groups and tags", async () => { + const client = createMockClient(); + client.listResources = async function* (_ctx: ApimServiceContext, type: ResourceType, parent?: { nameParts: string[] }) { + if (type === ResourceType.Api) yield { name: 'orders', properties: {} }; + if (type === ResourceType.Product) yield { name: 'starter', properties: {} }; + if (type === ResourceType.Tag) { + yield { name: 'public', properties: {} }; + yield { name: 'internal', properties: {} }; + } + if (type === ResourceType.ApiDiagnostic) { + yield { name: 'applicationinsights', properties: { loggerId: `${SERVICE}/workspaces/team-a/loggers/ws-logger` } }; + } + if (type === ResourceType.ApiTag && parent?.nameParts[0] === 'public') { + yield { name: 'link-1', properties: { apiId: `${SERVICE}/workspaces/team-a/apis/orders` } }; + } + if (type === ResourceType.ProductTag && parent?.nameParts[0] === 'internal') { + yield { name: 'link-2', properties: { productId: `${SERVICE}/workspaces/team-a/products/starter` } }; + } + if (type === ResourceType.ProductGroup) { + yield { name: 'link-3', properties: { groupId: `${SERVICE}/groups/support-team` } }; + } + }; + client.getResource.mockImplementation(async (_ctx, descriptor) => { + if (descriptor.type === ResourceType.Workspace) return { name: 'team-a', properties: {} }; + if ([ResourceType.Logger, ResourceType.Tag, ResourceType.Group].includes(descriptor.type)) { + return { name: descriptor.nameParts[0], properties: {} }; + } + return undefined; + }); + const store = createAssociationStore(); + const filter: FilterConfig = { + workspaces: ['team-a'], + workspaceSubFilters: { + 'team-a': { apis: ['orders'], products: ['starter'], tags: [], loggers: [], groups: [] }, + }, + }; + + await extractWorkspaces(client, store, testContext, '/output', filter, true); + + const fetched = client.getResource.mock.calls.map(([ctx, d]) => + `${d.type}:${d.workspace ?? 'service'}:${d.nameParts.join('/')}@${ctx.baseUrl === wsContext.baseUrl ? 'workspace' : 'service'}` + ); + expect(fetched).toEqual(expect.arrayContaining([ + 'Logger:team-a:ws-logger@workspace', + 'Tag:team-a:public@workspace', + 'Tag:team-a:internal@workspace', + 'Group:service:support-team@service', + ])); + expect(store.writeResource).toHaveBeenCalledWith( + '/output', + expect.objectContaining({ type: ResourceType.ApiTag, nameParts: ['orders', 'public'], workspace: 'team-a' }), + expect.anything() + ); + }); + + it('should fall back to the extracted tags when listing the workspace tags fails', async () => { + const warn = vi.spyOn(logger, 'warn').mockImplementation(() => undefined); + const client = createMockClient(); + let tagListings = 0; + client.listResources = async function* (_ctx: ApimServiceContext, type: ResourceType) { + if (type === ResourceType.Api) yield { name: 'orders', properties: {} }; + if (type === ResourceType.Tag && ++tagListings > 1) throw new Error('HTTP 503'); + }; + client.getResource.mockResolvedValue({ name: 'team-a', properties: {} }); + const filter: FilterConfig = { + workspaces: ['team-a'], + workspaceSubFilters: { 'team-a': { apis: ['orders'], tags: [] } }, + }; + + const results = await extractWorkspaces(client, createAssociationStore(), testContext, '/output', filter, true); + + expect(results).toHaveLength(1); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Failed to list tags in workspace "team-a"')); + warn.mockRestore(); + }); + + it('should not pull in dependencies the workspace or top-level filter excludes by name', async () => { + const client = createMockClient(); + client.listResources = async function* (_ctx: ApimServiceContext, type: ResourceType, parent?: { nameParts: string[] }) { + if (type === ResourceType.Api) yield { name: 'orders', properties: {} }; + if (type === ResourceType.Product) yield { name: 'starter', properties: {} }; + if (type === ResourceType.Tag) { + yield { name: 'public', properties: {} }; + yield { name: 'internal', properties: {} }; + } + if (type === ResourceType.ApiTag && parent) { + yield { name: `link-${parent.nameParts[0]}`, properties: { apiId: `${SERVICE}/workspaces/team-a/apis/orders` } }; + } + if (type === ResourceType.ProductGroup) { + yield { name: 'link-a', properties: { groupId: `${SERVICE}/groups/partners` } }; + yield { name: 'link-b', properties: { groupId: `${SERVICE}/groups/support-team` } }; + yield { name: 'link-c', properties: { groupId: `${SERVICE}/workspaces/team-a/groups/developers` } }; + } + }; + client.getResource.mockImplementation(async (_ctx, descriptor) => { + if (descriptor.type === ResourceType.Workspace) return { name: 'team-a', properties: {} }; + if ([ResourceType.Tag, ResourceType.Group].includes(descriptor.type)) { + return { name: descriptor.nameParts[0], properties: {} }; + } + return undefined; + }); + const filter: FilterConfig = { + groups: ['!partners', '*'], + workspaces: ['team-a'], + workspaceSubFilters: { + 'team-a': { apis: ['orders'], products: ['starter'], tags: ['!internal', 'none'], groups: [] }, + }, + }; + + await extractWorkspaces(client, createAssociationStore(), testContext, '/output', filter, true); + + const fetched = client.getResource.mock.calls + .map(([, d]) => `${d.type}:${d.workspace ?? 'service'}:${d.nameParts.join('/')}`) + .filter((key) => key.startsWith('Tag') || key.startsWith('Group')); + expect(fetched.sort()).toEqual([ + 'Group:service:support-team', + // A workspace group named like a built-in group is a custom group. + 'Group:team-a:developers', + 'Tag:team-a:public', + ]); + }); + + /** + * A client whose workspaces each have product 'starter' linked to service + * group 'support-team'. APIM refuses that link for a custom group; it + * stands in for any service-level dependency a workspace can have. + */ + function createSharedGroupClient(workspaces: string[]) { + const client = createMockClient(); + client.listResources = async function* (ctx: ApimServiceContext, type: ResourceType) { + if (type === ResourceType.Workspace) { + for (const name of workspaces) yield { name, properties: {} }; + } + if (type === ResourceType.Product && ctx.baseUrl.includes('/workspaces/')) yield { name: 'starter', properties: {} }; + if (type === ResourceType.ProductGroup) { + yield { name: 'link-1', properties: { groupId: `${SERVICE}/groups/support-team` } }; + } + }; + client.getResource.mockImplementation(async (_ctx, descriptor) => + ({ name: descriptor.nameParts[0], properties: {} }) + ); + return client; + } + + const groupGets = (client: ReturnType) => + client.getResource.mock.calls.filter(([, d]) => d.type === ResourceType.Group).length; + + /** A filter extracting product 'starter' from each workspace through a sub-filter. */ + const starterFilter = (workspaces: string[]): FilterConfig => ({ + groups: [], + workspaces, + workspaceSubFilters: Object.fromEntries(workspaces.map((name) => [name, { products: ['starter'] }])), + }); + + it('should not resolve dependencies in a workspace extracted without a sub-filter', async () => { + const client = createSharedGroupClient(['team-a']); + const filter: FilterConfig = { groups: ['partners'], workspaces: ['team-a'] }; + + await extractWorkspaces(client, createAssociationStore(), testContext, '/output', filter, true); + + expect(groupGets(client)).toBe(0); + }); + + it('should fetch a service-level dependency once across workspaces', async () => { + const client = createSharedGroupClient(['team-a', 'team-b']); + const filter = starterFilter(['team-a', 'team-b']); + + const store = createAssociationStore(); + + await extractWorkspaces(client, store, testContext, '/output', filter, true); + + expect(groupGets(client)).toBe(1); + expect(store.writeResource.mock.calls.filter(([, d]) => d.type === ResourceType.Group)).toHaveLength(1); + }); + + it('should not fetch a service-level dependency the service extraction already wrote', async () => { + const client = createSharedGroupClient(['team-a']); + const filter = starterFilter(['team-a']); + + await extractWorkspaces( + client, createAssociationStore(), testContext, '/output', filter, true, + [{ type: ResourceType.Group, nameParts: ['support-team'] }] + ); + + expect(groupGets(client)).toBe(0); + }); + + it('should try a missing service-level dependency once across workspaces', async () => { + const client = createSharedGroupClient(['team-a', 'team-b']); + client.getResource.mockImplementation(async (_ctx, descriptor) => + descriptor.type === ResourceType.Group ? undefined : { name: descriptor.nameParts[0], properties: {} } + ); + const filter = starterFilter(['team-a', 'team-b']); + + const results = await extractWorkspaces(client, createAssociationStore(), testContext, '/output', filter, true); + + expect(groupGets(client)).toBe(1); + expect(results.map((ws) => ws.errorCount)).toEqual([1, 0]); + }); + + it.each([ + ['the tags filter only excludes by name', { apis: ['orders'], tags: ['!internal'] }], + ['no APIs or products were extracted', { apis: [], products: [], tags: [] }], + ['the tags filter includes every tag', { apis: ['orders'], tags: ['*', '!internal'] }], + ['the tags key is left out, so every tag is included', { apis: ['orders'] }], + ])('should not list the workspace tags again when %s', async (_case, subFilter) => { + const client = createMockClient(); + let tagListings = 0; + client.listResources = async function* (_ctx: ApimServiceContext, type: ResourceType) { + if (type === ResourceType.Api) yield { name: 'orders', properties: {} }; + if (type === ResourceType.Tag) { + tagListings++; + yield { name: 'public', properties: {} }; + } + }; + client.getResource.mockResolvedValue({ name: 'team-a', properties: {} }); + const filter: FilterConfig = { workspaces: ['team-a'], workspaceSubFilters: { 'team-a': subFilter } }; + + await extractWorkspaces(client, createAssociationStore(), testContext, '/output', filter, true); + + expect(tagListings).toBe(1); + }); + + it('should only use extracted tags for links without transitive resolution', async () => { + const client = createMockClient(); + const listed: ResourceType[] = []; + client.listResources = async function* (_ctx: ApimServiceContext, type: ResourceType) { + listed.push(type); + if (type === ResourceType.Api) yield { name: 'orders', properties: {} }; + }; + client.getResource.mockResolvedValue({ name: 'team-a', properties: {} }); + const filter: FilterConfig = { + workspaces: ['team-a'], + workspaceSubFilters: { 'team-a': { apis: ['orders'], tags: [] } }, + }; + + await extractWorkspaces(client, createAssociationStore(), testContext, '/output', filter, false); + + expect(listed).not.toContain(ResourceType.ApiTag); + }); + }); + describe('resolveWorkspaceFilter', () => { it('should return undefined when no filter provided', () => { expect(resolveWorkspaceFilter('ws-1')).toBeUndefined(); From f49214ac43671d017e9e3d1e18a2824dd8ec3c68 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:43:25 +1100 Subject: [PATCH 02/23] fix: ignore policy references in XML comments and Liquid templates The dependency scan read policy XML as plain text, so a commented-out or {{name}} was followed, and the {{ ... }} output expressions in a body were fetched as named values. Each missing "dependency" counted as an extraction error, so extract exited with 1. XML comments, and the bodies of Liquid set-body templates, are now removed before scanning. Only closed comments are removed, so a stray '/g; + +/** + * The body of a `` element: its `{{ ... }}` are + * Liquid output expressions, not named value references. A self-closing + * set-body has no body and is not matched. + */ +const LIQUID_BODY_PATTERN = new RegExp( + `()` + + `[\\s\\S]*?(<\\/set-body\\s*>)`, + 'gi' +); + +/** + * Policy XML with the parts that cannot reference other resources removed: + * XML comments, and the bodies of Liquid templates. + */ +function stripNonReferenceContent(policyXml: string): string { + return policyXml + .replace(XML_COMMENT_PATTERN, '') + .replace(LIQUID_BODY_PATTERN, (_match, open: string, _quote: string, close: string) => `${open}${close}`); +} + /** * Scan text for named value references ({{displayName}}). */ @@ -92,12 +120,16 @@ function scanNamedValueReferences(text: string): TransitiveDependency[] { * * A value that is a policy expression (`@...`) or a named value reference * (`{{...}}`, found by the named value scan) does not name a resource. + * References inside XML comments, and `{{ ... }}` inside a + * `` body (Liquid output, not named values), are + * ignored. */ export function scanPolicyReferences(policyXml: string): TransitiveDependency[] { - const dependencies = scanNamedValueReferences(policyXml); + const scannable = stripNonReferenceContent(policyXml); + const dependencies = scanNamedValueReferences(scannable); for (const { type, pattern } of POLICY_REFERENCE_PATTERNS) { - for (const match of policyXml.matchAll(pattern)) { + for (const match of scannable.matchAll(pattern)) { const name = match[2]?.trim(); if (name && !name.startsWith('@') && !name.includes('{{')) { dependencies.push({ type, name }); diff --git a/tests/unit/services/transitive-resolver.test.ts b/tests/unit/services/transitive-resolver.test.ts index c6305f4b..01bba12a 100644 --- a/tests/unit/services/transitive-resolver.test.ts +++ b/tests/unit/services/transitive-resolver.test.ts @@ -363,15 +363,50 @@ describe('transitive-resolver', () => { expect(scanPolicyReferences(policy)).toContainEqual({ type, name }); }); + it('should ignore references inside XML comments (#305)', () => { + const refs = scanPolicyReferences( + '' + + '' + ); + + expect(refs).toEqual([{ type: ResourceType.Backend, name: 'live-backend' }]); + }); + + it('should not let a stray "' + )).toEqual([ + { type: ResourceType.Backend, namedValue: 'Backend_Id' }, + { type: ResourceType.Backend, namedValue: 'safety' }, + ]); + }); + + it("should follow the named value's value to the resource it names, matching the display name", () => { + const deps = findTransitiveDependencies(new Map([['p', POLICY]]), new Map(), undefined, [ + namedValue({ displayName: 'backend_id', value: ' function-backend ' }), + ]); + + expect(deps).toContainEqual({ type: ResourceType.Backend, nameParts: ['function-backend'], workspace: undefined }); + }); + + it('should fall back to the resource name, and let a display name match win', () => { + const byName = findTransitiveDependencies(new Map([['p', POLICY]]), new Map(), undefined, [ + namedValue({ displayName: 'Other', value: 'by-name' }, 'Backend_Id'), + ]); + const byDisplayName = findTransitiveDependencies(new Map([['p', POLICY]]), new Map(), undefined, [ + namedValue({ displayName: 'Other', value: 'by-name' }, 'Backend_Id'), + namedValue({ displayName: 'Backend_Id', value: 'by-display-name' }, 'nv-1'), + ]); + + expect(byName).toContainEqual({ type: ResourceType.Backend, nameParts: ['by-name'], workspace: undefined }); + expect(byDisplayName.filter((dep) => dep.type === ResourceType.Backend)).toEqual([ + { type: ResourceType.Backend, nameParts: ['by-display-name'], workspace: undefined }, + ]); + }); + + it.each([ + ['a secret', { displayName: 'Backend_Id', value: '***REDACTED***', secret: true }], + ['a policy expression', { displayName: 'Backend_Id', value: '@(context.Variables["b"])' }], + ['a nested reference', { displayName: 'Backend_Id', value: '{{other}}' }], + ])('should not follow %s named value', (_kind, properties) => { + const deps = findTransitiveDependencies(new Map([['p', POLICY]]), new Map(), undefined, [namedValue(properties)]); + + expect(deps.filter((dep) => dep.type === ResourceType.Backend)).toEqual([]); + }); + + it("should only match a named value in the policy's own scope", () => { + const deps = findTransitiveDependencies(new Map([['p', POLICY]]), new Map(), 'team-a', [ + namedValue({ displayName: 'Backend_Id', value: 'service-backend' }), + namedValue({ displayName: 'Backend_Id', value: 'ws-backend' }, 'backend-id', 'team-a'), + ]); + + expect(deps.filter((dep) => dep.type === ResourceType.Backend)).toEqual([ + { type: ResourceType.Backend, nameParts: ['ws-backend'], workspace: 'team-a' }, + ]); + }); + + it('should follow one in a policy fragment', () => { + const deps = findTransitiveDependencies(new Map(), new Map(), undefined, [ + { descriptor: { type: ResourceType.PolicyFragment, nameParts: ['route'] }, json: { properties: { value: POLICY } } }, + namedValue({ displayName: 'Backend_Id', value: 'function-backend' }), + ]); + + expect(deps).toContainEqual({ type: ResourceType.Backend, nameParts: ['function-backend'], workspace: undefined }); + }); + + it('should read the named value when scanning an artifact, given a reader', async () => { + const store = { + readResource: vi.fn(), + readContent: vi.fn().mockResolvedValue({ content: POLICY, format: 'xml' }), + readAssociation: vi.fn(), + }; + const readNamedValue = vi.fn().mockResolvedValue( + namedValue({ displayName: 'Backend_Id', value: 'ws-backend' }, 'backend-id', 'team-a') + ); + const policy = { type: ResourceType.ApiPolicy, nameParts: ['orders'], workspace: 'team-a' }; + + const withReader = await scanArtifactReferences(store, '/source', policy, readNamedValue); + const withoutReader = await scanArtifactReferences(store, '/source', policy); + + expect(readNamedValue).toHaveBeenCalledWith({ type: ResourceType.NamedValue, nameParts: ['Backend_Id'], workspace: 'team-a' }); + expect(withReader).toEqual([ + { type: ResourceType.NamedValue, nameParts: ['Backend_Id'], workspace: 'team-a' }, + { type: ResourceType.Backend, nameParts: ['ws-backend'], workspace: 'team-a' }, + ]); + expect(withoutReader).toEqual([{ type: ResourceType.NamedValue, nameParts: ['Backend_Id'], workspace: 'team-a' }]); + }); + }); + it("should read a product's group and tag links when scanning it for publish", async () => { const store = { readResource: vi.fn().mockResolvedValue({ properties: { displayName: 'Starter' } }), From fd70bba62e436a492eaf5b7ecb281dcd6671a8b5 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 06:58:41 +1100 Subject: [PATCH 08/23] fix: keep a product's link to administrators when the filter narrows groups APIM links the built-in administrators group to every product, including workspace products. Publish removes the links APIM adds to a new product and then publishes the extracted ones, which follow the groups filter. A filtered publish that narrows groups therefore warned "Skipping ProductGroup association ...administrators" for every product, and left a new product without the link APIM itself would have added. The link to administrators is now published whatever the groups filter says, unless the filter excludes administrators by name. Links to the other built-in groups (developers, guests) still follow the filter. Found by the filtering integration tests. --- docs/guides/filtering-resources.md | 2 +- src/services/resource-publisher.ts | 26 ++++++++++++----- tests/unit/services/product-publisher.test.ts | 28 ++++++++++++++++++- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/docs/guides/filtering-resources.md b/docs/guides/filtering-resources.md index a7bb45d2..165c0a6b 100644 --- a/docs/guides/filtering-resources.md +++ b/docs/guides/filtering-resources.md @@ -339,7 +339,7 @@ A reference written as a policy expression (`backend-id="@(...)"`) is not follow A `{{...}}` reference, in a policy or in credentials, names a named value by its display name, as APIM does; when no named value has that display name, it is read as a resource name. The `namedValues` list and its exclusions (`'!name'`) use the resource name, which can differ from the display name. References inside XML comments are ignored, and so is a `{{...}}` inside a `` body, which is Liquid output, not a named value. -A resource the filter excludes by name (`'!name'`) is never pulled in, and links to it are not extracted: an API's or product's link to an excluded tag, or a product's link to an excluded group. Built-in groups (`administrators`, `developers`, `guests`) exist at service level in every APIM instance, so they are not pulled in either; a product's links to them are published only when the filter includes them, as for any group. +A resource the filter excludes by name (`'!name'`) is never pulled in, and links to it are not extracted: an API's or product's link to an excluded tag, or a product's link to an excluded group. Built-in groups (`administrators`, `developers`, `guests`) exist at service level in every APIM instance, so they are not pulled in either; a product's links to them are published only when the filter includes them, as for any group. The exception is the link to `administrators`, which APIM adds to every product itself: it is published whatever the `groups` filter says, unless the filter excludes `administrators` by name. **Not included:** the products, gateways and subscriptions an API is linked to, and the APIs a product contains. These are independent resources, not dependencies; add them to the filter to include them. On publish, links to APIs or products outside the publish set are skipped. diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts index dc4e2cc9..924c2b84 100644 --- a/src/services/resource-publisher.ts +++ b/src/services/resource-publisher.ts @@ -15,7 +15,7 @@ import type { ResourceDescriptor, } from '../models/types.js'; import type { PublishConfig } from '../models/config.js'; -import { ResourceType, RESOURCE_TYPE_METADATA, MANAGED_GATEWAY_NAME } from '../models/resource-types.js'; +import { isBuiltInGroup, ResourceType, RESOURCE_TYPE_METADATA, MANAGED_GATEWAY_NAME } from '../models/resource-types.js'; import { applyOverrides, resolveOverrideSection } from './override-merger.js'; import { checkKeyVaultSecretAccess } from './keyvault-checker.js'; import { @@ -738,12 +738,9 @@ async function buildAssociationPublicationPlan( nameParts: [entry.name], workspace: entry.scope === 'service' ? undefined : workspace, }; - const eligibility = await evaluateAssociationEligibility( - store, - target, - config, - { allowedDescriptors } - ); + const eligibility = isAdministratorsLink(target, config) + ? { eligible: true } + : await evaluateAssociationEligibility(store, target, config, { allowedDescriptors }); const metadata = RESOURCE_TYPE_METADATA[type]; const workspaceScoped = !!workspace || isWorkspaceScope(context); const segment = target.type === ResourceType.Group ? 'groups' : 'apis'; @@ -767,6 +764,21 @@ async function buildAssociationPublicationPlan( return { descriptor, deployedDescriptor, target, payload, ...eligibility }; } +/** + * Whether a product's group link targets the built-in `administrators` group, + * which APIM links to every product itself (publish removes the links APIM + * adds to a new product, then publishes the extracted ones). The link is kept + * whatever the groups filter says, unless the filter excludes the group by + * name, so a filter that narrows groups neither drops it nor warns about it. + */ +function isAdministratorsLink(target: ResourceDescriptor, config: PublishConfig): boolean { + return ( + isBuiltInGroup(target) && + target.nameParts[0]?.toLowerCase() === 'administrators' && + !(config.filter && isExplicitlyExcluded(target, config.filter)) + ); +} + export interface AssociationEligibilityOptions { /** * The publish set, when the caller has one. Besides marking targets as diff --git a/tests/unit/services/product-publisher.test.ts b/tests/unit/services/product-publisher.test.ts index c41b0e8d..a5edbc6f 100644 --- a/tests/unit/services/product-publisher.test.ts +++ b/tests/unit/services/product-publisher.test.ts @@ -12,7 +12,7 @@ import { import { ResourceType } from '../../../src/models/resource-types.js'; import { ApimServiceContext, ResourceDescriptor } from '../../../src/models/types.js'; import { PublishConfig } from '../../../src/models/config.js'; -import { LogLevel } from '../../../src/lib/logger.js'; +import { LogLevel, logger } from '../../../src/lib/logger.js'; import { DEFAULT_APPLIES_TO } from '../../../src/services/env-mapper.js'; // Mock resource-publisher so product-publisher tests don't run full resource-publisher logic @@ -314,6 +314,32 @@ describe('product-publisher', () => { expect(await linksFor(['developers'])).toEqual(['ProductGroup:my-product/developers']); }); + it('publishes the link APIM adds to administrators whatever the groups filter, unless it excludes the group by name', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readAssociation.mockImplementation(async (_dir: string, _descriptor: ResourceDescriptor, type: string) => + type === 'groups' ? [{ name: 'administrators' }] : [] + ); + const warn = vi.spyOn(logger, 'warn'); + const linksFor = async (groups: string[]) => { + client.putResource.mockClear(); + warn.mockClear(); + await publishProduct(client, store, testContext, productDescriptor, { + ...testConfig, + filter: { products: ['my-product'], groups }, + }, [productDescriptor]); + return client.putResource.mock.calls + .map(([, descriptor]) => `${descriptor.type}:${descriptor.nameParts.join('/')}`) + .filter((key) => key.startsWith('ProductGroup')); + }; + + expect(await linksFor([])).toEqual(['ProductGroup:my-product/administrators']); + expect(await linksFor(['support-team'])).toEqual(['ProductGroup:my-product/administrators']); + expect(warn).not.toHaveBeenCalled(); + expect(await linksFor(['!administrators'])).toEqual([]); + warn.mockRestore(); + }); + it('does not let resolved targets override explicit API exclusions', async () => { const client = createMockClient(); const store = createMockStore(); From e23d7d17d0875a69c5c2f307f54b8e7d7bd87d8d Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 12:29:20 +1100 Subject: [PATCH 09/23] fix: publish a named value after the backend its value names When a policy names its backend through a named value (backend-id="{{backend-name}}") and the named value's value changes, publish now pulls in the backend the new value names, but it published all named values before any backend. APIM checks the policies that use a named value when the named value changes, so on a target that already has such a policy it rejected the update ("set-backend-service: An expression in a property is valid, but its value breaks policies") and publish exited 1. It only worked when the policy was created later in the same run. A named value that a policy or fragment in the source uses as the whole value of a reference attribute (backend-id, logger-id, schema-id), and whose value, with --overrides applied, names a backend, logger or global schema of that kind published in the same run, now goes after those resources, pool backends included. Every policy in the source counts, not only the ones the filter selects, since APIM checks every policy on the target that uses the named value; this only changes the order, not what is published. Policy fragments then go after it, since a fragment can use it and APIM rejects a policy that uses a missing named value. One that a backend or logger in the same run uses in its credentials stays first, since that resource needs it. Every other named value still goes first, since backend and logger credentials can use them. The policies are read only when a named value's value matches the name of such a resource in the run. Found by the filtering integration tests (an override naming another backend, and an incremental publish where only the named value changed). --- docs/guides/filtering-resources.md | 2 +- docs/reference/dependency-graph.md | 2 +- src/services/publish-service.ts | 209 +++++++++++++++++--- tests/unit/services/publish-service.test.ts | 173 +++++++++++++++- 4 files changed, 359 insertions(+), 27 deletions(-) diff --git a/docs/guides/filtering-resources.md b/docs/guides/filtering-resources.md index 165c0a6b..c00a78b8 100644 --- a/docs/guides/filtering-resources.md +++ b/docs/guides/filtering-resources.md @@ -335,7 +335,7 @@ When you filter resources, `apiops extract` and `apiops publish` automatically i | Backend pool | Its member backends | | Product | The groups and tags it is linked to | -A reference written as a policy expression (`backend-id="@(...)"`) is not followed, because its value is only known at run time. One set to a named value (`backend-id="{{backend-name}}"`) pulls in the named value and the resource its value names, in the same scope (except ``, where APIM doesn't substitute named values); publish reads the value with your `--overrides` applied, so each environment gets the resource its own value names. A named value that is published also brings the resource its value names whenever any policy or fragment in the source uses it that way, even one the filter leaves out: APIM checks every policy on the target that uses a named value when its value changes. A resource the filter excludes by name, or that isn't in the source, isn't published, so it has to exist on the target already. A secret or Key Vault named value's value isn't known, so the resource it names isn't pulled in; add that resource to the filter. +A reference written as a policy expression (`backend-id="@(...)"`) is not followed, because its value is only known at run time. One set to a named value (`backend-id="{{backend-name}}"`) pulls in the named value and the resource its value names, in the same scope (except ``, where APIM doesn't substitute named values); publish reads the value with your `--overrides` applied, so each environment gets the resource its own value names. A named value that is published also brings the resource its value names whenever any policy or fragment in the source uses it that way, even one the filter leaves out: APIM checks every policy on the target that uses a named value when its value changes. A resource the filter excludes by name, or that isn't in the source, isn't published, so it has to exist on the target already. When a policy uses a named value to name a backend, logger or schema that is published in the same run, publish creates that resource before it updates the named value, and publishes policy fragments after it, since APIM rejects a named value change that would leave a policy using it pointing at a resource that doesn't exist. A secret or Key Vault named value's value isn't known, so the resource it names isn't pulled in; add that resource to the filter. A `{{...}}` reference, in a policy or in credentials, names a named value by its display name, as APIM does; when no named value has that display name, it is read as a resource name. The `namedValues` list and its exclusions (`'!name'`) use the resource name, which can differ from the display name. References inside XML comments are ignored, and so is a `{{...}}` inside a `` body, which is Liquid output, not a named value. diff --git a/docs/reference/dependency-graph.md b/docs/reference/dependency-graph.md index 40d50ec2..1f439fdc 100644 --- a/docs/reference/dependency-graph.md +++ b/docs/reference/dependency-graph.md @@ -15,7 +15,7 @@ apiops-cli solves this by organizing all 34 resource types into **4 tiers**. Eac | Tier | Resources | Rule | |------|-----------|------| -| **1** | NamedValue, Tag, Gateway, VersionSet, Backend, Logger, Group, PolicyFragment, GlobalSchema, PolicyRestriction, Documentation | No dependencies — safe to process in any order | +| **1** | NamedValue, Tag, Gateway, VersionSet, Backend, Logger, Group, PolicyFragment, GlobalSchema, PolicyRestriction, Documentation | No dependencies on other tiers. Within the tier, named values go first, since backends and loggers can use them; pool backends follow the backends they contain; and a named value a policy uses to name a backend, logger or schema published in the same run goes after that resource (unless a backend or logger in the run uses it in its credentials), with policy fragments after it in that case | | **2** | Diagnostic, ServicePolicy, Product, Api | Depend only on Tier 1 resources | | **3** | ProductPolicy, ProductGroup, ProductTag, ProductApi, ProductWiki, ApiPolicy, ApiTag, ApiDiagnostic, ApiOperation, ApiSchema, ApiRelease, ApiTagDescription, ApiWiki, McpServer, GraphQLResolver, GatewayApi, Subscription | Depend on Tier 1 and/or Tier 2 resources | | **4** | ApiOperationPolicy, GraphQLResolverPolicy | Depend on Tier 3 resources | diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 40801fed..d0b438f3 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -56,6 +56,7 @@ import { REDACTION_MARKER } from './secret-redactor.js'; import { createArtifactReferenceResolver, createNamedValueReader, + findTransitiveDependencies, indexNamedValueTargetTypes, namedValueTargetName, scanArtifactReferences, @@ -131,10 +132,15 @@ async function runPublishPipeline( // Step 0: Pre-flight validation — ensure the resource group and APIM service exist await client.validatePreFlight(config.service); + // The named values policies use as resource names, read from the source + // once for this run when the expansion or the Tier 1 order first needs it. + const namedValueTargets = createNamedValueTargetIndex(store, config); + // Step 1: Determine target descriptors based on incremental mode const { targetDescriptors, deletedDescriptors } = await determinePublishTargets( store, - config + config, + namedValueTargets ); // Step 1a: For env-mapping validation and policy ref rewriting we need @@ -227,7 +233,8 @@ async function runPublishPipeline( store, config.service, config, - targetDescriptors + targetDescriptors, + namedValueTargets ); // Step 4: Execute DELETEs in reverse dependency order (tier 4 → tier 1) if requested @@ -286,7 +293,8 @@ async function runPublishPipeline( */ async function determinePublishTargets( store: IArtifactStore, - config: PublishConfig + config: PublishConfig, + namedValueTargets: NamedValueTargetIndex ): Promise { let targetDescriptors: ResourceDescriptor[]; let deletedDescriptors: ResourceDescriptor[]; @@ -317,20 +325,46 @@ async function determinePublishTargets( targetDescriptors, availableDescriptors, config.filter, - config.overrides + config.overrides, + namedValueTargets ); } return { targetDescriptors, deletedDescriptors }; } +/** + * The resource types each named value in the source is used for as a resource + * name (indexNamedValueTargetTypes over every policy and policy fragment): + * built on first use and then shared, so a publish run reads the policies once + * for both the transitive expansion and the Tier 1 order. + */ +type NamedValueTargetIndex = () => Promise>>; + +function createNamedValueTargetIndex(store: IArtifactStore, config: PublishConfig): NamedValueTargetIndex { + let index: Promise>> | undefined; + return () => { + index ??= store.listResources(config.sourceDir).then((available) => + indexNamedValueTargetTypes( + store, + config.sourceDir, + available, + available, + createArtifactReferenceResolver(available, (descriptor) => store.readResource(config.sourceDir, descriptor)) + ) + ); + return index; + }; +} + async function expandTransitivePublishTargets( store: IArtifactStore, sourceDir: string, initialDescriptors: ResourceDescriptor[], availableDescriptors: ResourceDescriptor[], filter: FilterConfig, - overrides: OverrideConfig | undefined + overrides: OverrideConfig | undefined, + namedValueTargets: NamedValueTargetIndex ): Promise { const findAvailable = createArtifactReferenceResolver( availableDescriptors, @@ -359,17 +393,10 @@ async function expandTransitivePublishTargets( // (backend-id="{{name}}"), even one the filter leaves out: APIM checks every // policy on the target that uses a named value when the named value // changes, and rejects the change while the resource it names is missing. - // The policies are read once, on the first named value in the publish set. - let targetTypes: Promise>> | undefined; + // The policies are read once per run, on the first named value in the + // publish set. const findNamedValueTargets = async (namedValue: ResourceDescriptor): Promise => { - targetTypes ??= indexNamedValueTargetTypes( - store, - sourceDir, - availableDescriptors, - availableDescriptors, - findAvailable - ); - const types = (await targetTypes).get(getResourceDescriptorKey(namedValue)); + const types = (await namedValueTargets()).get(getResourceDescriptorKey(namedValue)); const json = types ? await readNamedValueJson(namedValue) : undefined; const name = json ? namedValueTargetName(json) : undefined; return types && name @@ -504,7 +531,8 @@ async function executePuts( store: IArtifactStore, context: ApimServiceContext, config: PublishConfig, - targetDescriptors: ResourceDescriptor[] + targetDescriptors: ResourceDescriptor[], + namedValueTargets: NamedValueTargetIndex ): Promise { const results: PublishActionResult[] = []; @@ -558,25 +586,41 @@ async function executePuts( await publishAndOutput(client, store, context, config, workspaces, targetDescriptors, results); } - // Within Tier 1 we publish in three ordered waves to satisfy implicit + // Within Tier 1 we publish in ordered waves to satisfy implicit // runtime dependencies: // - // Wave 1 — NamedValues only. + // Wave 1 — Named values, except those in wave 4. // Loggers and backends may reference named values by name in their // configuration (e.g. AppInsights instrumentation key stored as a // named value). Publishing named values first ensures they exist // before any resource that references them. // - // Wave 2 — All remaining Tier 1 resources except pool backends. - // Loggers, tags, gateways, etc. can now reference named values safely. + // Wave 2 — The remaining Tier 1 resources except pool backends (and + // except policy fragments when wave 4 has named values). // // Wave 3 — Pool backends. // Pool backends reference individual Backend resources and must be // published after the backends they aggregate (see pool backend // ordering comments in splitPoolBackends). - if (namedValues.length > 0) { - logger.debug(`Publishing ${namedValues.length} named value(s) first (wave 1 of tier 1)`); - await publishAndOutput(client, store, context, config, namedValues, targetDescriptors, results); + // + // Wave 4 — Named values a policy uses as a resource name + // (backend-id="{{name}}") whose value names a backend, logger or + // schema published here. APIM checks the policies that use a named + // value when it changes, so that resource must exist first (see + // splitNamedValuesNamingResources). + // + // Wave 5 — Policy fragments, when wave 4 has named values: a fragment + // can use them, and APIM rejects a policy that uses a missing one. + const { early: earlyNamedValues, late: lateNamedValues } = await splitNamedValuesNamingResources( + store, + config, + namedValues, + otherTier1, + namedValueTargets + ); + if (earlyNamedValues.length > 0) { + logger.debug(`Publishing ${earlyNamedValues.length} named value(s) first (wave 1 of tier 1)`); + await publishAndOutput(client, store, context, config, earlyNamedValues, targetDescriptors, results); } const { poolBackends, regularTier1 } = await splitPoolBackends( @@ -584,7 +628,18 @@ async function executePuts( config.sourceDir, otherTier1 ); - await publishAndOutput(client, store, context, config, regularTier1, targetDescriptors, results); + const fragments = lateNamedValues.length > 0 + ? regularTier1.filter((descriptor) => descriptor.type === ResourceType.PolicyFragment) + : []; + await publishAndOutput( + client, + store, + context, + config, + regularTier1.filter((descriptor) => !fragments.includes(descriptor)), + targetDescriptors, + results + ); if (poolBackends.length > 0) { logger.debug( `Publishing ${poolBackends.length} pool backend(s) after regular backends` @@ -599,6 +654,15 @@ async function executePuts( results ); } + if (lateNamedValues.length > 0) { + logger.debug( + `Publishing ${lateNamedValues.length} named value(s) after the resources their values name` + ); + await publishAndOutput(client, store, context, config, lateNamedValues, targetDescriptors, results); + } + if (fragments.length > 0) { + await publishAndOutput(client, store, context, config, fragments, targetDescriptors, results); + } } else if (tier === 2) { const tier2Descriptors = filterApiRevisionsHandledByRootApis(descriptors); if (tier2Descriptors.length === 0) continue; @@ -810,6 +874,103 @@ function splitNamedValues( return { namedValues, otherTier1 }; } +/** + * Splits named values into those published before the rest of Tier 1 and + * those published after the backends, loggers and schemas: a named value that + * a policy or policy fragment in the source uses as the whole value of a + * reference attribute (backend-id="{{name}}", logger-id, schema-id), and whose + * value (with overrides) names a resource of that type published in this + * tier. APIM checks the policies that use a named value when the named value + * changes, rejecting the change ("its value breaks policies") while the + * resource it now names doesn't exist. One that a backend or logger published + * here uses in its credentials stays first, since that resource needs it. The + * policies are read (once) only when a named value's value matches the name of + * such a resource here; a named value that can't be read goes first, and + * publishing it reports the error. + */ +async function splitNamedValuesNamingResources( + store: IArtifactStore, + config: PublishConfig, + namedValues: ResourceDescriptor[], + otherTier1: ResourceDescriptor[], + namedValueTargets: NamedValueTargetIndex +): Promise<{ early: ResourceDescriptor[]; late: ResourceDescriptor[] }> { + const resourceKey = (type: ResourceType, name: string, workspace: string | undefined) => + `${type}/${workspace ?? ''}/${name.toLowerCase()}`; + const named = new Set( + otherTier1 + .filter((descriptor) => NAMED_THROUGH_NAMED_VALUE_TYPES.has(descriptor.type)) + .map((descriptor) => resourceKey(descriptor.type, getNamePart(descriptor.nameParts, 0), descriptor.workspace)) + ); + const early: ResourceDescriptor[] = []; + const late: ResourceDescriptor[] = []; + if (named.size === 0) { + return { early: namedValues, late }; + } + // The named values policies use as resource names, and as which types. + // Every policy in the source counts, not only the ones the filter selects: + // APIM checks every policy on the target that uses the named value, and + // this only decides the order, not what is published. + const typesFor = async (namedValue: ResourceDescriptor): Promise | undefined> => + (await namedValueTargets()).get(getResourceDescriptorKey(namedValue)); + for (const descriptor of namedValues) { + let isLate = false; + try { + const json = await store.readResource(config.sourceDir, descriptor); + const name = json ? namedValueTargetName(applyOverrides(descriptor, json, config.overrides)) : undefined; + if (name && [...NAMED_THROUGH_NAMED_VALUE_TYPES].some((type) => named.has(resourceKey(type, name, descriptor.workspace)))) { + const types = await typesFor(descriptor); + isLate = [...(types ?? [])].some((type) => named.has(resourceKey(type, name, descriptor.workspace))); + } + } catch { + isLate = false; + } + (isLate ? late : early).push(descriptor); + } + if (late.length === 0) { + return { early, late }; + } + + // A backend or logger published here whose own configuration (its + // credentials) uses one of those named values needs it first, so that one + // stays early. + const available = await store.listResources(config.sourceDir); + const resolve = createArtifactReferenceResolver(available, (descriptor) => store.readResource(config.sourceDir, descriptor)); + const neededFirst = new Set(); + for (const descriptor of otherTier1) { + if (descriptor.type !== ResourceType.Backend && descriptor.type !== ResourceType.Logger) { + continue; + } + let json: Record | undefined; + try { + json = await store.readResource(config.sourceDir, descriptor); + } catch { + continue; + } + if (!json) { + continue; + } + const resource = { descriptor, json: applyOverrides(descriptor, json, config.overrides) }; + for (const dependency of findTransitiveDependencies(new Map(), new Map(), descriptor.workspace, [resource])) { + const target = dependency.type === ResourceType.NamedValue ? await resolve(dependency) : undefined; + if (target) { + neededFirst.add(getResourceDescriptorKey(target)); + } + } + } + return { + early: [...early, ...late.filter((descriptor) => neededFirst.has(getResourceDescriptorKey(descriptor)))], + late: late.filter((descriptor) => !neededFirst.has(getResourceDescriptorKey(descriptor))), + }; +} + +/** Tier 1 types a policy attribute can name through a named value. */ +const NAMED_THROUGH_NAMED_VALUE_TYPES: ReadonlySet = new Set([ + ResourceType.Backend, + ResourceType.Logger, + ResourceType.GlobalSchema, +]); + /** * Separates Backend descriptors that are pool backends (properties.type === "Pool") * from all other descriptors in the supplied list. diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 6b02643b..51f16382 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -664,9 +664,20 @@ describe('publish-service', () => { logLevel: LogLevel.INFO, }); const puts = () => client.putResource.mock.calls.map((call) => `${call[1].type}:${call[1].nameParts.join('/')}`).sort(); - return { publish, puts }; + return { publish, puts, store }; }; + it('should read that policy once, though both the expansion and the order use it', async () => { + const { publish, store } = setUp(); + + await publish({ apis: ['orders'], backends: [] }); + + const billingPolicyReads = store.readContent.mock.calls.filter( + (call) => call[1].type === ResourceType.ApiPolicy && call[1].nameParts[0] === 'billing' + ); + expect(billingPolicyReads).toHaveLength(1); + }); + it('should include the resource its value names, as APIM checks that policy on the target', async () => { const { publish, puts } = setUp(); @@ -1947,6 +1958,166 @@ describe('publish-service', () => { }); describe('named value ordering within tier 1', () => { + const orderingCase = async ( + extra: ResourceDescriptor[], + policy: string, + overrides?: PublishConfig['overrides'] + ): Promise => { + const publishOrder: string[] = []; + const resources: ResourceDescriptor[] = [ + { type: ResourceType.NamedValue, nameParts: ['backend-id'] }, + { type: ResourceType.NamedValue, nameParts: ['api-key'] }, + { type: ResourceType.Backend, nameParts: ['prod-backend'] }, + { type: ResourceType.Backend, nameParts: ['dev-backend'] }, + { type: ResourceType.PolicyFragment, nameParts: ['route'] }, + ...extra, + ]; + const client = createMockClient(); + vi.mocked(client.putResource).mockImplementation(async (_ctx, descriptor) => { + publishOrder.push(descriptor.nameParts.join('/')); + }); + const store = createMockStore(resources); + vi.mocked(store.readResource).mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.NamedValue) { + const value = { 'backend-id': 'dev-backend', 'api-key': 'prod-backend', 'schema-id': 'order-schema' }[descriptor.nameParts[0] ?? '']; + return { properties: { displayName: descriptor.nameParts[0], value } }; + } + if (descriptor.type === ResourceType.PolicyFragment) { + return { properties: { value: policy } }; + } + return { properties: { url: 'https://example.test' } }; + }); + + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + overrides, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + return publishOrder; + }; + + it('should publish a named value a policy uses as a backend after the backend its value names, and before fragments', async () => { + const order = await orderingCase( + [], + '', + { namedValues: { 'backend-id': { properties: { value: 'prod-backend' } } } } + ); + + expect(order.indexOf('backend-id')).toBeGreaterThan(order.indexOf('prod-backend')); + expect(order.indexOf('backend-id')).toBeGreaterThan(order.indexOf('dev-backend')); + // The fragment uses the named value, so it goes after it. + expect(order.indexOf('route')).toBeGreaterThan(order.indexOf('backend-id')); + // api-key's value matches a backend's name, but no policy uses it as one. + expect(order.indexOf('api-key')).toBeLessThan(order.indexOf('prod-backend')); + }); + + it('should publish a named value a policy uses as a schema after the schema its value names', async () => { + const order = await orderingCase( + [ + { type: ResourceType.NamedValue, nameParts: ['schema-id'] }, + { type: ResourceType.GlobalSchema, nameParts: ['order-schema'] }, + ], + '' + ); + + expect(order.indexOf('schema-id')).toBeGreaterThan(order.indexOf('order-schema')); + expect(order.indexOf('backend-id')).toBeLessThan(order.indexOf('dev-backend')); + }); + + const recordOrder = (resources: ResourceDescriptor[]) => { + const order: string[] = []; + const client = createMockClient(); + vi.mocked(client.putResource).mockImplementation(async (_ctx, descriptor) => { + order.push(descriptor.nameParts.join('/')); + }); + return { order, client, store: createMockStore(resources) }; + }; + const publishAll = (client: ReturnType, store: ReturnType, filter?: FilterConfig) => + runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + + it('should publish a named value naming a pool backend after the pool', async () => { + const { order, client, store } = recordOrder([ + { type: ResourceType.NamedValue, nameParts: ['pool-id'] }, + { type: ResourceType.Backend, nameParts: ['member'] }, + { type: ResourceType.Backend, nameParts: ['my-pool'] }, + { type: ResourceType.PolicyFragment, nameParts: ['route'] }, + ]); + vi.mocked(store.readResource).mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.NamedValue) { + return { properties: { displayName: 'pool-id', value: 'my-pool' } }; + } + if (descriptor.type === ResourceType.PolicyFragment) { + return { properties: { value: '' } }; + } + return descriptor.nameParts[0] === 'my-pool' + ? { properties: { type: 'Pool', pool: { services: [{ id: '/backends/member' }] } } } + : { properties: { url: 'https://example.test' } }; + }); + + await publishAll(client, store); + + expect(order.indexOf('my-pool')).toBeGreaterThan(order.indexOf('member')); + expect(order.indexOf('pool-id')).toBeGreaterThan(order.indexOf('my-pool')); + expect(order.indexOf('route')).toBeGreaterThan(order.indexOf('pool-id')); + }); + + it("should keep a named value first when a backend published with it uses it in its credentials", async () => { + const { order, client, store } = recordOrder([ + { type: ResourceType.NamedValue, nameParts: ['shared'] }, + { type: ResourceType.Backend, nameParts: ['prod-backend'] }, + { type: ResourceType.PolicyFragment, nameParts: ['route'] }, + ]); + vi.mocked(store.readResource).mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.NamedValue) { + return { properties: { displayName: 'shared', value: 'prod-backend' } }; + } + if (descriptor.type === ResourceType.PolicyFragment) { + return { properties: { value: '' } }; + } + return { properties: { url: 'https://example.test', credentials: { header: { 'x-route': ['{{shared}}'] } } } }; + }); + + await publishAll(client, store); + + expect(order.indexOf('shared')).toBeLessThan(order.indexOf('prod-backend')); + }); + + it('should order by a policy the filter leaves out, since APIM checks it on the target', async () => { + const { order, client, store } = recordOrder([ + { type: ResourceType.Api, nameParts: ['orders'] }, + { type: ResourceType.Api, nameParts: ['billing'] }, + { type: ResourceType.ApiPolicy, nameParts: ['billing'] }, + { type: ResourceType.NamedValue, nameParts: ['billing-id'] }, + { type: ResourceType.Backend, nameParts: ['new-billing'] }, + ]); + store.readContent.mockImplementation(async (_sourceDir: string, descriptor: ResourceDescriptor) => + descriptor.type === ResourceType.ApiPolicy + ? { content: '' } + : undefined + ); + vi.mocked(store.readResource).mockImplementation(async (_sourceDir, descriptor) => + descriptor.type === ResourceType.NamedValue + ? { properties: { displayName: 'billing-id', value: 'new-billing' } } + : { properties: { url: 'https://example.test' } } + ); + + // Named values and backends are left out of the filter, so all are selected. + await publishAll(client, store, { apis: ['orders'] }); + + expect(order).not.toContain('billing'); + expect(order.indexOf('billing-id')).toBeGreaterThan(order.indexOf('new-billing')); + }); + it('should publish named values before loggers within tier 1', async () => { const publishOrder: string[] = []; From 07f69bbb2882c97183e00c233469c205e55dffc2 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 12:31:39 +1100 Subject: [PATCH 10/23] fix: follow named values APIM substitutes inside Liquid bodies The dependency scan skipped every {{ ... }} inside a body, so that Liquid output expressions such as {{body.envelope.body.fault.faultcode}} weren't fetched as named values (#276). But APIM does substitute a named value there when one has that name: the integration tests got x=[function-backend] back from x=[{{demo-backend-id}}], and APIM rejects a missing {{name}} in a Liquid body when the policy is saved. A named value used only inside a Liquid body was therefore no longer pulled in, and publishing to a new instance failed with "Cannot find a property". Inside a Liquid body, a {{name}} made only of the characters a named value's name can have (letters, digits, '.', '-' and '_') is now followed when a named value with that display or resource name exists. Every other {{ ... }} there, with spaces, a filter or no such named value (like {{body.id}}), is left to Liquid and skipped without an error, so #276 stays fixed. Extract checks the names against the named values it lists, once per service or workspace extracted; publish and --remove-stale follow only those that resolve to an artifact. Extract's named value lookup now knows resource names as well as display names (display names first, as in APIM). A {{FOO}} that matches no display name used to keep its own spelling; it now takes the resource name's (foo), so the artifact is written under that name and a '!foo' exclusion is checked against it. Found by the filtering integration tests. Refs #276 --- docs/guides/filtering-resources.md | 2 +- src/services/transitive-extractor.ts | 84 ++++++++++++++++--- src/services/transitive-resolver.ts | 61 ++++++++++++-- tests/unit/services/publish-service.test.ts | 65 ++++++++++++++ .../services/stale-dependency-finder.test.ts | 15 ++++ .../services/transitive-extractor.test.ts | 81 ++++++++++++++++++ .../unit/services/transitive-resolver.test.ts | 11 +++ 7 files changed, 299 insertions(+), 20 deletions(-) diff --git a/docs/guides/filtering-resources.md b/docs/guides/filtering-resources.md index c00a78b8..b18909e1 100644 --- a/docs/guides/filtering-resources.md +++ b/docs/guides/filtering-resources.md @@ -337,7 +337,7 @@ When you filter resources, `apiops extract` and `apiops publish` automatically i A reference written as a policy expression (`backend-id="@(...)"`) is not followed, because its value is only known at run time. One set to a named value (`backend-id="{{backend-name}}"`) pulls in the named value and the resource its value names, in the same scope (except ``, where APIM doesn't substitute named values); publish reads the value with your `--overrides` applied, so each environment gets the resource its own value names. A named value that is published also brings the resource its value names whenever any policy or fragment in the source uses it that way, even one the filter leaves out: APIM checks every policy on the target that uses a named value when its value changes. A resource the filter excludes by name, or that isn't in the source, isn't published, so it has to exist on the target already. When a policy uses a named value to name a backend, logger or schema that is published in the same run, publish creates that resource before it updates the named value, and publishes policy fragments after it, since APIM rejects a named value change that would leave a policy using it pointing at a resource that doesn't exist. A secret or Key Vault named value's value isn't known, so the resource it names isn't pulled in; add that resource to the filter. -A `{{...}}` reference, in a policy or in credentials, names a named value by its display name, as APIM does; when no named value has that display name, it is read as a resource name. The `namedValues` list and its exclusions (`'!name'`) use the resource name, which can differ from the display name. References inside XML comments are ignored, and so is a `{{...}}` inside a `` body, which is Liquid output, not a named value. +A `{{...}}` reference, in a policy or in credentials, names a named value by its display name, as APIM does; when no named value has that display name, it is read as a resource name. The `namedValues` list and its exclusions (`'!name'`) use the resource name, which can differ from the display name. References inside XML comments are ignored. Inside a `` body, a `{{name}}` is followed only when a named value has that name: APIM substitutes those there and leaves every other `{{ ... }}` (`{{body.id}}`, `{{ name }}`, `{{x | upcase}}`) to Liquid, so those are skipped without an error. A resource the filter excludes by name (`'!name'`) is never pulled in, and links to it are not extracted: an API's or product's link to an excluded tag, or a product's link to an excluded group. Built-in groups (`administrators`, `developers`, `guests`) exist at service level in every APIM instance, so they are not pulled in either; a product's links to them are published only when the filter includes them, as for any group. The exception is the link to `administrators`, which APIM adds to every product itself: it is published whatever the `groups` filter says, unless the filter excludes `administrators` by name. diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index b9eab56f..8669000b 100644 --- a/src/services/transitive-extractor.ts +++ b/src/services/transitive-extractor.ts @@ -10,7 +10,7 @@ import { buildResourceLabel } from '../lib/resource-uri.js'; import { logger } from '../lib/logger.js'; import { runParallel } from '../lib/parallel-runner.js'; import { redactSecrets } from './secret-redactor.js'; -import { findTransitiveDependencies } from './transitive-resolver.js'; +import { findTransitiveDependencies, scanLiquidNamedValueCandidates } from './transitive-resolver.js'; import type { ExtractedResource } from './resource-extractor.js'; const DEFAULT_CONCURRENCY = 5; @@ -68,8 +68,9 @@ interface TransitiveTaskResult { } /** - * Per-context (keyed by baseUrl) index of named value displayName (lower - * case, as names are compared everywhere else) → resource name. + * Per-context (keyed by baseUrl) index of named value displayName, and + * resource name where no display name claims it (lower case, as names are + * compared everywhere else) → resource name. * Stored as promises so concurrent tasks share a single LIST call. */ type NamedValueNameIndex = Map>>; @@ -79,12 +80,22 @@ async function loadNamedValueNames( context: ApimServiceContext ): Promise> { const byDisplayName = new Map(); + const resourceNames: string[] = []; 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.toLowerCase(), name); + if (typeof name === 'string') { + resourceNames.push(name); + if (typeof displayName === 'string') { + byDisplayName.set(displayName.toLowerCase(), name); + } + } + } + // A reference matches a display name first, as in APIM, then a resource name. + for (const name of resourceNames) { + if (!byDisplayName.has(name.toLowerCase())) { + byDisplayName.set(name.toLowerCase(), name); } } } catch (error) { @@ -97,6 +108,20 @@ async function loadNamedValueNames( return byDisplayName; } +/** The named value name index for a context, listed once and shared. */ +function namedValueNames( + client: IApimClient, + context: ApimServiceContext, + index: NamedValueNameIndex +): Promise> { + let names = index.get(context.baseUrl); + if (!names) { + names = loadNamedValueNames(client, context); + index.set(context.baseUrl, names); + } + return names; +} + /** * 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 @@ -108,14 +133,8 @@ async function resolveNamedValueDescriptor( 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.toLowerCase()); + const name = (await namedValueNames(client, context, index)).get(ref.toLowerCase()); if (!name || name === ref) { return dep; } @@ -124,6 +143,38 @@ async function resolveNamedValueDescriptor( return { ...dep, nameParts: [name] }; } +/** The `{{name}}`s in a policy fragment's Liquid bodies (scanLiquidNamedValueCandidates). */ +function fragmentLiquidCandidates({ descriptor, json }: TransitiveResourceArtifact): string[] { + if (descriptor.type !== ResourceType.PolicyFragment) { + return []; + } + const properties = json.properties as Record | undefined; + return [properties?.value, properties?.policyContent] + .filter((text): text is string => typeof text === 'string') + .flatMap(scanLiquidNamedValueCandidates); +} + +/** + * The named values Liquid bodies reference: each `{{name}}` candidate that a + * named value in this scope has as its display or resource name. The others + * are Liquid, skipped without a warning. + */ +async function liquidNamedValueDependencies( + client: IApimClient, + context: ApimServiceContext, + candidates: ReadonlySet, + workspace: string | undefined, + index: NamedValueNameIndex +): Promise { + if (candidates.size === 0) { + return []; + } + const names = await namedValueNames(client, context, index); + return [...candidates] + .filter((candidate) => names.has(candidate.toLowerCase())) + .map((candidate) => ({ type: ResourceType.NamedValue, nameParts: [candidate], workspace })); +} + export interface TransitiveExtractionOptions { /** Workspace being extracted; its resources are fetched through `context`. */ workspace?: string; @@ -162,10 +213,19 @@ export async function extractTransitiveDependencies( let errorCount = 0; let foundDependencies = false; + // The {{name}}s in Liquid bodies: the policies' found once, and each + // fragment's as it is extracted. + const liquidCandidates = new Set([...policies.values()].flatMap(scanLiquidNamedValueCandidates)); + let resourcesScanned = 0; + while (true) { + for (; resourcesScanned < resources.length; resourcesScanned++) { + fragmentLiquidCandidates(resources[resourcesScanned]).forEach((name) => liquidCandidates.add(name)); + } const newDeps = [ ...findTransitiveDependencies(policies, apis, workspace, resources), ...linkedDependencies, + ...await liquidNamedValueDependencies(client, context, liquidCandidates, workspace, namedValueIndex), ].filter((dep) => !attempted.has(getResourceDescriptorKey(dep)) && !skip(dep)); if (newDeps.length === 0) { diff --git a/src/services/transitive-resolver.ts b/src/services/transitive-resolver.ts index bde05db5..4ff757b8 100644 --- a/src/services/transitive-resolver.ts +++ b/src/services/transitive-resolver.ts @@ -111,16 +111,23 @@ const REFERENCING_TYPES = new Set([ const XML_COMMENT_PATTERN = //g; /** - * The body of a `` element: its `{{ ... }}` are - * Liquid output expressions, not named value references. A self-closing - * set-body has no body and is not matched. + * The body of a `` element (capture group 3): its + * `{{ ... }}` are mostly Liquid output expressions, not named value + * references; see scanLiquidNamedValueCandidates. A self-closing set-body has + * no body and is not matched. */ const LIQUID_BODY_PATTERN = new RegExp( `()` + - `[\\s\\S]*?(<\\/set-body\\s*>)`, + `([\\s\\S]*?)(<\\/set-body\\s*>)`, 'gi' ); +/** + * A `{{name}}` whose name has only the characters a named value's name can: + * letters, digits, '.', '-' and '_'. Spaces or a filter (`|`) make it Liquid. + */ +const LIQUID_NAMED_VALUE_PATTERN = /\{\{([A-Za-z0-9._-]+)\}\}/g; + /** * Policy XML with the parts that cannot reference other resources removed: * XML comments, and the bodies of Liquid templates. @@ -128,7 +135,28 @@ const LIQUID_BODY_PATTERN = new RegExp( function stripNonReferenceContent(policyXml: string): string { return policyXml .replace(XML_COMMENT_PATTERN, '') - .replace(LIQUID_BODY_PATTERN, (_match, open: string, _quote: string, close: string) => `${open}${close}`); + .replace(LIQUID_BODY_PATTERN, (_match, open: string, _quote: string, _body: string, close: string) => `${open}${close}`); +} + +/** + * The names a `` body may reference named values + * by: each `{{name}}` there whose name a named value could have. APIM + * substitutes a named value inside a Liquid body when one has that name, and + * leaves every other `{{ ... }}` (with spaces, a filter, or no such named + * value, like `{{body.id}}`) to Liquid. So each is a reference only when a + * named value with that name exists; callers check before following one, and + * skip the rest without an error. XML comments are ignored. + */ +export function scanLiquidNamedValueCandidates(policyXml: string): string[] { + const names = new Set(); + for (const body of policyXml.replace(XML_COMMENT_PATTERN, '').matchAll(LIQUID_BODY_PATTERN)) { + for (const match of (body[3] ?? '').matchAll(LIQUID_NAMED_VALUE_PATTERN)) { + if (match[1]) { + names.add(match[1]); + } + } + } + return [...names]; } /** @@ -158,8 +186,10 @@ function scanNamedValueReferences(text: string): TransitiveDependency[] { * A value that is a policy expression (`@...`) or a named value reference * (`{{...}}`, found by the named value scan) does not name a resource. * References inside XML comments, and `{{ ... }}` inside a - * `` body (Liquid output, not named values), are - * ignored. + * `` body, are ignored: most of those are Liquid + * output. The bare `{{name}}`s there that APIM substitutes when a named value + * has that name are found by scanLiquidNamedValueCandidates, for callers to + * check. */ export function scanPolicyReferences(policyXml: string): TransitiveDependency[] { const scannable = stripNonReferenceContent(policyXml); @@ -601,6 +631,23 @@ export async function scanArtifactReferences( references.push(...findTransitiveDependencies(policies, apis, descriptor.workspace, resources)); + // A {{name}} in a Liquid body is a reference only when such a named value + // exists; one that resolves to no artifact is simply not followed. + const fragmentProperties = descriptor.type === ResourceType.PolicyFragment + ? (json?.properties as Record | undefined) + : undefined; + for (const text of [...policies.values(), fragmentProperties?.value, fragmentProperties?.policyContent]) { + if (typeof text === 'string') { + references.push( + ...scanLiquidNamedValueCandidates(text).map((name) => ({ + type: ResourceType.NamedValue, + nameParts: [name], + workspace: descriptor.workspace, + })) + ); + } + } + if (descriptor.type === ResourceType.Product) { references.push(...await readProductLinkDependencies(store, sourceDir, descriptor)); } diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 51f16382..1e0c23c6 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -634,6 +634,71 @@ describe('publish-service', () => { expect(puts).toEqual(['Backend:new-backend', 'NamedValue:backend-id']); }); + it("should include a named value a selected fragment's Liquid body references", async () => { + const resources = [ + { type: ResourceType.PolicyFragment, nameParts: ['route'] }, + { type: ResourceType.NamedValue, nameParts: ['frag-nv'] }, + { type: ResourceType.NamedValue, nameParts: ['unused'] }, + ]; + const client = createMockClient(); + const store = createMockStore(resources); + store.readResource.mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.PolicyFragment) { + return { properties: { value: '{{Frag_NV}} {{body.id}}' } }; + } + return { properties: { displayName: descriptor.nameParts[0] === 'frag-nv' ? 'Frag_NV' : 'unused', value: 'x' } }; + }); + + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter: { policyFragments: ['route'], namedValues: [] }, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + + const namedValuePuts = client.putResource.mock.calls + .filter((call) => call[1].type === ResourceType.NamedValue) + .map((call) => call[1].nameParts[0]); + expect(namedValuePuts).toEqual(['frag-nv']); + }); + + it('should include a named value a Liquid body references, and nothing for Liquid output', async () => { + const resources = [ + { type: ResourceType.Api, nameParts: ['orders'] }, + { type: ResourceType.ApiPolicy, nameParts: ['orders'] }, + { type: ResourceType.NamedValue, nameParts: ['backend-id'] }, + { type: ResourceType.NamedValue, nameParts: ['unused'] }, + ]; + const client = createMockClient(); + const store = createMockStore(resources); + store.readContent.mockImplementation(async (_sourceDir: string, descriptor: ResourceDescriptor) => + descriptor.type === ResourceType.ApiPolicy + ? { content: 'nv=[{{Backend_Id}}] liquid=[{{body.id}}]' } + : undefined + ); + store.readResource.mockImplementation(async (_sourceDir, descriptor) => + descriptor.type === ResourceType.NamedValue + ? { properties: { displayName: descriptor.nameParts[0] === 'backend-id' ? 'Backend_Id' : 'unused', value: 'x' } } + : { properties: {} } + ); + + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter: { apis: ['orders'], namedValues: [] }, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + + const namedValuePuts = client.putResource.mock.calls + .filter((call) => call[1].type === ResourceType.NamedValue) + .map((call) => call[1].nameParts[0]); + expect(namedValuePuts).toEqual(['backend-id']); + }); + describe('a named value a policy outside the filter uses as a resource name', () => { const setUp = (extra: ResourceDescriptor[] = [{ type: ResourceType.Backend, nameParts: ['billing-backend'] }]) => { const resources = [ diff --git a/tests/unit/services/stale-dependency-finder.test.ts b/tests/unit/services/stale-dependency-finder.test.ts index 6963bffa..eec2fa1d 100644 --- a/tests/unit/services/stale-dependency-finder.test.ts +++ b/tests/unit/services/stale-dependency-finder.test.ts @@ -121,6 +121,21 @@ describe('stale-dependency-finder', () => { expect(await findStaleDependencies(store, outputDir, stagingDir, backendFilter)).toEqual([]); }); + it('should keep a named value only a Liquid body still uses, and report it once nothing does', async () => { + const policy: ResourceDescriptor = { type: ResourceType.ApiPolicy, nameParts: ['orders'] }; + const liquid = 'nv=[{{Backend_Id}}] liquid=[{{body.id}}]'; + await write(outputDir, orders); + await store.writeContent(outputDir, policy, liquid, 'policy', 'xml'); + await write(outputDir, { type: ResourceType.NamedValue, nameParts: ['backend-id'] }, { displayName: 'Backend_Id', value: 'x' }); + await write(stagingDir, orders); + await store.writeContent(stagingDir, policy, liquid, 'policy', 'xml'); + + expect(await findStaleDependencies(store, outputDir, stagingDir, filter)).toEqual([]); + + await store.writeContent(stagingDir, policy, '', 'policy', 'xml'); + expect(names(await findStaleDependencies(store, outputDir, stagingDir, filter))).toEqual(['NamedValue:backend-id']); + }); + it("should keep a dependency's own dependency that another artifact still uses", async () => { const diagnostic: ResourceDescriptor = { type: ResourceType.ApiDiagnostic, nameParts: ['orders', 'applicationinsights'] }; await write(outputDir, orders); diff --git a/tests/unit/services/transitive-extractor.test.ts b/tests/unit/services/transitive-extractor.test.ts index fd84abc3..c30931cd 100644 --- a/tests/unit/services/transitive-extractor.test.ts +++ b/tests/unit/services/transitive-extractor.test.ts @@ -320,3 +320,84 @@ describe('transitive-extractor', () => { }); }); }); + +describe('extractTransitiveDependencies with Liquid bodies', () => { + it('should extract a named value a Liquid body references and skip Liquid output without an error', async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'backend-id', displayName: 'demo-backend-id' }); + client.getResource.mockImplementation(async (_ctx: ApimServiceContext, descriptor: ResourceDescriptor) => + descriptor.type === ResourceType.NamedValue && descriptor.nameParts[0] === 'backend-id' + ? { name: 'backend-id', properties: { displayName: 'demo-backend-id', value: 'function-backend' } } + : undefined + ); + const store = createMockStore(); + const policies = new Map([[ + 'api:liquid-test:policy:0', + 'nv=[{{demo-backend-id}}] liquid=[{{body.id}}] spaced=[{{ context.Request.Url }}]', + ]]); + + const result = await extractTransitiveDependencies(client, store, testContext, '/out', policies, new Map(), [], []); + + const fetched = client.getResource.mock.calls.map((call) => `${call[1].type}:${call[1].nameParts.join('/')}`); + expect(fetched).toEqual(['NamedValue:backend-id']); + expect(result.errorCount).toBe(0); + expect(result.extractedDescriptors.map((d) => d.nameParts[0])).toEqual(['backend-id']); + }); + + it("should follow a Liquid {{name}} in a fragment extracted in a later round", async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'frag-nv', displayName: 'frag-nv' }); + client.getResource.mockImplementation(async (_ctx: ApimServiceContext, descriptor: ResourceDescriptor) => + descriptor.type === ResourceType.PolicyFragment + ? { properties: { value: '{{frag-nv}} {{body.id}}' } } + : { properties: { value: 'x' } } + ); + const policies = new Map([['api:orders:policy:0', '']]); + + const result = await extractTransitiveDependencies(client, createMockStore(), testContext, '/out', policies, new Map(), [], []); + + expect(client.getResource.mock.calls.map((call) => `${call[1].type}:${call[1].nameParts.join('/')}`)).toEqual([ + 'PolicyFragment:route', + 'NamedValue:frag-nv', + ]); + expect(result.errorCount).toBe(0); + }); + + it('should look a Liquid {{name}} up in the workspace being extracted', async () => { + const client = createMockClient(); + const workspaceContext = { ...testContext, baseUrl: `${testContext.baseUrl}/workspaces/team-a` }; + client.listResources = vi.fn(async function* (ctx: ApimServiceContext, type: ResourceType) { + if (type === ResourceType.NamedValue && ctx.baseUrl === workspaceContext.baseUrl) { + yield { name: 'ws-key', properties: { displayName: 'ws-key' } }; + } + }); + client.getResource.mockResolvedValue({ properties: { value: 'x' } }); + const policies = new Map([['api:orders:policy:0', '{{ws-key}}']]); + + await extractTransitiveDependencies(client, createMockStore(), workspaceContext, '/out', policies, new Map(), [], [], { + workspace: 'team-a', + serviceContext: testContext, + }); + + expect(client.getResource.mock.calls.map((call) => call[1])).toEqual([ + { type: ResourceType.NamedValue, nameParts: ['ws-key'], workspace: 'team-a' }, + ]); + }); +}); + +describe('extractTransitiveDependencies named value lookup', () => { + it("should resolve a reference that matches no display name to the resource name's spelling, and check exclusions under it", async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'foo', displayName: 'Something' }); + client.getResource.mockResolvedValue({ properties: { value: 'x' } }); + + await extractTransitiveDependencies(client, createMockStore(), testContext, '/out', policiesWith('FOO'), new Map(), [], []); + expect(client.getResource.mock.calls.map((call) => call[1].nameParts)).toEqual([['foo']]); + + client.getResource.mockClear(); + await extractTransitiveDependencies(client, createMockStore(), testContext, '/out', policiesWith('FOO'), new Map(), [], [], { + skip: (dep) => dep.nameParts[0] === 'foo', + }); + expect(client.getResource).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/services/transitive-resolver.test.ts b/tests/unit/services/transitive-resolver.test.ts index 52b18680..e2f46eca 100644 --- a/tests/unit/services/transitive-resolver.test.ts +++ b/tests/unit/services/transitive-resolver.test.ts @@ -15,6 +15,7 @@ import { findAssociationDependencies, scanNamedValueTargets, createNamedValueReader, + scanLiquidNamedValueCandidates, } from '../../../src/services/transitive-resolver.js'; describe('transitive-resolver', () => { @@ -344,6 +345,16 @@ describe('transitive-resolver', () => { expect(refs).toEqual([{ type: ResourceType.Backend, name: 'content-safety' }]); }); + it('should give the {{name}}s in Liquid bodies that a named value could have, for callers to check', () => { + const policy = ` + nv=[{{demo-backend-id}}] dotted=[{{body.id}}] spaced=[{{ context.Request.Url }}] filtered=[{{x | upcase}}] + + {{outside-liquid}} + `; + + expect(scanLiquidNamedValueCandidates(policy)).toEqual(['demo-backend-id', 'body.id']); + }); + it('should ignore Liquid output in set-body templates but keep named values elsewhere (#276)', () => { const refs = scanPolicyReferences(` {{real-value}} From 5b75c26f46d6b2c015c20e0bc1bac2eda260e982 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:19:49 +1100 Subject: [PATCH 11/23] fix: read ARM IDs as collection/name pairs armNameAfter matched the first segment with the collection's name, so in /workspaces/backends/backends/member it returned the workspace name rather than the backend. The path below the service is now read as collection/name pairs, which also covers subscription scopes in a workspace named products. --- src/lib/arm-id.ts | 15 +++++++++++---- tests/unit/lib/arm-id.test.ts | 2 ++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/lib/arm-id.ts b/src/lib/arm-id.ts index 0c215ce4..91c217b4 100644 --- a/src/lib/arm-id.ts +++ b/src/lib/arm-id.ts @@ -35,12 +35,19 @@ function pathBelowService(id: string): string { } /** - * The decoded name that follows `{segment}/` in an ARM ID (case-insensitive), - * for example `armNameAfter(id, 'loggers')`. + * The decoded name of the `{segment}` collection's resource in an ARM ID + * (case-insensitive), for example `armNameAfter(id, 'loggers')`. The path + * below the service is read as collection/name pairs, so a resource named + * like a collection (a workspace named `backends`) is not read as one. */ export function armNameAfter(id: string, segment: string): string | undefined { - const match = pathBelowService(id).match(new RegExp(`(?:^|/)${segment}/([^/]+)(?:/|$)`, 'i')); - return match?.[1] ? decodeArmSegment(match[1]) : undefined; + const parts = pathBelowService(id).split('/').filter(Boolean); + for (let index = 0; index + 1 < parts.length; index += 2) { + if (parts[index].toLowerCase() === segment.toLowerCase()) { + return decodeArmSegment(parts[index + 1]); + } + } + return undefined; } /** The decoded last segment of an ARM ID, or undefined when it has none. */ diff --git a/tests/unit/lib/arm-id.test.ts b/tests/unit/lib/arm-id.test.ts index 3cd15a5c..36b23b1e 100644 --- a/tests/unit/lib/arm-id.test.ts +++ b/tests/unit/lib/arm-id.test.ts @@ -36,6 +36,8 @@ describe('arm-id', () => { [`${SERVICE}/apis/orders`, 'products', undefined], ['/subscriptions/s/resourceGroups/products/providers/Microsoft.ApiManagement/service/apim/apis/orders', 'products', undefined], ['/subscriptions/s/resourceGroups/r/providers/Microsoft.ApiManagement/service/loggers/loggers/l', 'loggers', 'l'], + [`${SERVICE}/workspaces/backends/backends/member`, 'backends', 'member'], + ['/workspaces/products/products/gold', 'products', 'gold'], ])('should read the name after a segment in %s', (id, segment, expected) => { expect(armNameAfter(id, segment)).toBe(expected); }); From e3691fd41687c76250b96fa949c2f50742290332 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:20:31 +1100 Subject: [PATCH 12/23] fix: report a failed workspace tag listing as an error When the filter leaves tags out, a workspace's tag links are found by listing every tag in it. If that listing failed, only links to the extracted tags were written, without recording an error, so --remove-stale could delete links that still exist. The failure now counts as an extraction error, which stops --remove-stale from running. --- src/services/workspace-extractor.ts | 24 ++++++++++++------- .../unit/services/workspace-extractor.test.ts | 3 ++- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/services/workspace-extractor.ts b/src/services/workspace-extractor.ts index 7cbbd4f7..9a5a269c 100644 --- a/src/services/workspace-extractor.ts +++ b/src/services/workspace-extractor.ts @@ -277,14 +277,21 @@ async function extractWorkspace( // are found and pulled in below; otherwise the extracted tags are all // there is to check. const hasLinkOwners = extractedApiNames.size > 0 || extractedProducts.length > 0; - const linkTagNames = transitive && hasLinkOwners && leavesOutUnnamed(filter?.tags) - ? (await listWorkspaceTagNames(client, wsContext, workspaceName, extractedTagNames)).filter( + let linkTagNames = extractedTagNames; + if (transitive && hasLinkOwners && leavesOutUnnamed(filter?.tags)) { + const listed = await listWorkspaceTagNames(client, wsContext, workspaceName); + if (listed) { + linkTagNames = listed.filter( (name) => !isExplicitlyExcluded( { type: ResourceType.Tag, nameParts: [name], workspace: workspaceName }, serviceFilter ) - ) - : extractedTagNames; + ); + } else { + // Links on the other tags may be missed, so --remove-stale must not run. + errorCount++; + } + } // Extract workspace API tags using the tag-centric apiLinks endpoint. // This must happen after both Tags and APIs are extracted since the @@ -373,14 +380,13 @@ function leavesOutUnnamed(list: string[] | undefined): boolean { } /** - * Every tag in the workspace, or the extracted ones if the list fails. + * Every tag in the workspace, or undefined if the list fails. */ async function listWorkspaceTagNames( client: IApimClient, wsContext: ApimServiceContext, - workspaceName: string, - extractedTagNames: string[] -): Promise { + workspaceName: string +): Promise { const names: string[] = []; try { for await (const json of client.listResources(wsContext, ResourceType.Tag)) { @@ -394,7 +400,7 @@ async function listWorkspaceTagNames( `Failed to list tags in workspace "${workspaceName}"; only links to extracted tags are included: ` + `${(error as Error).message}` ); - return extractedTagNames; + return undefined; } } diff --git a/tests/unit/services/workspace-extractor.test.ts b/tests/unit/services/workspace-extractor.test.ts index df4c8cb4..a5ba8ab9 100644 --- a/tests/unit/services/workspace-extractor.test.ts +++ b/tests/unit/services/workspace-extractor.test.ts @@ -638,7 +638,7 @@ describe('workspace-extractor', () => { ); }); - it('should fall back to the extracted tags when listing the workspace tags fails', async () => { + it('should fall back to the extracted tags, and report an error, when listing the workspace tags fails', async () => { const warn = vi.spyOn(logger, 'warn').mockImplementation(() => undefined); const client = createMockClient(); let tagListings = 0; @@ -655,6 +655,7 @@ describe('workspace-extractor', () => { const results = await extractWorkspaces(client, createAssociationStore(), testContext, '/output', filter, true); expect(results).toHaveLength(1); + expect(results[0].errorCount).toBe(1); expect(warn).toHaveBeenCalledWith(expect.stringContaining('Failed to list tags in workspace "team-a"')); warn.mockRestore(); }); From d4673f9e33e6d1186c47a793833ba3ac1bb8adb3 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:21:35 +1100 Subject: [PATCH 13/23] fix: keep stale dependencies when a named value can't be read Matching {{references}} by display name skipped a named value artifact that couldn't be read, so --remove-stale could report a backend stale that a policy left in the output still reaches through that named value. An unreadable named value now keeps every pulled-in dependency, as an artifact that can't be scanned already does. Publish keeps its best-effort fallback. --- src/services/stale-dependency-finder.ts | 16 +++++++---- src/services/transitive-resolver.ts | 11 ++++++-- .../services/stale-dependency-finder.test.ts | 28 +++++++++++++++++++ 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/services/stale-dependency-finder.ts b/src/services/stale-dependency-finder.ts index bb300de0..ff5f5e09 100644 --- a/src/services/stale-dependency-finder.ts +++ b/src/services/stale-dependency-finder.ts @@ -67,8 +67,9 @@ async function scanAll( * * Artifacts left in the output are scanned, in parallel, until every * candidate is known to be needed; this only runs when there is a candidate. - * If an artifact left in the output can't be scanned, nothing is reported - * stale, since it could reference any candidate. + * If an artifact left in the output can't be scanned, or a named value left + * there can't be read to match {{references}} by display name, nothing is + * reported stale, since it could reference any candidate. * * @param outputDir - The existing output * @param stagingDir - This extract's staged output @@ -122,7 +123,12 @@ export async function findStaleDependencies( // is still needed, and so is whatever that candidate references. const readRemaining = (descriptor: ResourceDescriptor) => store.readResource(stagedKeys.has(key(descriptor)) ? stagingDir : outputDir, descriptor); - const resolveRemaining = createArtifactReferenceResolver([...staged, ...kept], readRemaining); + // A named value that can't be read may be what a {{reference}} left in + // the output names, so its candidates can't be known to be unused. + const unreadable: ResourceDescriptor[] = []; + const resolveRemaining = createArtifactReferenceResolver([...staged, ...kept], readRemaining, (descriptor) => { + unreadable.push(descriptor); + }); const readRemainingNamedValue = createNamedValueReader(resolveRemaining, readRemaining); const stale = new Map(candidates); const referrers: ArtifactLocation[] = [ @@ -138,7 +144,7 @@ export async function findStaleDependencies( const failed: ResourceDescriptor[] = []; await runParallel( round.map(({ descriptor, dir }) => async () => { - if (stale.size === 0 || failed.length > 0) { + if (stale.size === 0 || failed.length > 0 || unreadable.length > 0) { return; } let references: ResourceDescriptor[]; @@ -158,7 +164,7 @@ export async function findStaleDependencies( }), REFERENCE_SCAN_CONCURRENCY ); - if (failed.length > 0) { + if (failed.length > 0 || unreadable.length > 0) { logger.warn('Keeping every pulled-in dependency, since not every artifact could be checked for references'); return []; } diff --git a/src/services/transitive-resolver.ts b/src/services/transitive-resolver.ts index 4ff757b8..ffadb94b 100644 --- a/src/services/transitive-resolver.ts +++ b/src/services/transitive-resolver.ts @@ -665,10 +665,13 @@ export async function scanArtifactReferences( * * @param available - The artifacts references can resolve to * @param readResource - Reads an artifact's JSON + * @param onUnreadable - Called for a named value artifact that can't be read, + * so a {{reference}} to its display name may resolve to nothing */ export function createArtifactReferenceResolver( available: ReadonlyArray, - readResource: (descriptor: ResourceDescriptor) => Promise | undefined> + readResource: (descriptor: ResourceDescriptor) => Promise | undefined>, + onUnreadable?: (descriptor: ResourceDescriptor) => void ): (reference: ResourceDescriptor) => Promise { const byKey = new Map(available.map((descriptor) => [getResourceDescriptorKey(descriptor), descriptor])); let byDisplayName: Promise> | undefined; @@ -677,7 +680,7 @@ export function createArtifactReferenceResolver( if (reference.type !== ResourceType.NamedValue) { return byKey.get(key); } - byDisplayName ??= indexNamedValuesByDisplayName(available, readResource); + byDisplayName ??= indexNamedValuesByDisplayName(available, readResource, onUnreadable); return (await byDisplayName).get(key) ?? byKey.get(key); }; } @@ -713,7 +716,8 @@ export function createNamedValueReader( */ async function indexNamedValuesByDisplayName( available: ReadonlyArray, - readResource: (descriptor: ResourceDescriptor) => Promise | undefined> + readResource: (descriptor: ResourceDescriptor) => Promise | undefined>, + onUnreadable?: (descriptor: ResourceDescriptor) => void ): Promise> { const index = new Map(); for (const descriptor of available.filter((d) => d.type === ResourceType.NamedValue)) { @@ -722,6 +726,7 @@ async function indexNamedValuesByDisplayName( json = await readResource(descriptor); } catch (error) { logger.warn(`Unable to read ${buildResourceLabel(descriptor)} to match {{references}} by display name: ${String(error)}`); + onUnreadable?.(descriptor); continue; } const displayName = (json?.properties as Record | undefined)?.displayName; diff --git a/tests/unit/services/stale-dependency-finder.test.ts b/tests/unit/services/stale-dependency-finder.test.ts index eec2fa1d..5f13f30c 100644 --- a/tests/unit/services/stale-dependency-finder.test.ts +++ b/tests/unit/services/stale-dependency-finder.test.ts @@ -225,6 +225,34 @@ describe('stale-dependency-finder', () => { } }); + it('should report nothing when a named value left in the output cannot be read to match display names', async () => { + const warn = vi.spyOn(logger, 'warn').mockImplementation(() => undefined); + const policy = (api: string): ResourceDescriptor => ({ type: ResourceType.ApiPolicy, nameParts: [api] }); + const namedValue: ResourceDescriptor = { type: ResourceType.NamedValue, nameParts: ['backend-id'] }; + await write(outputDir, orders); + await store.writeContent(outputDir, policy('orders'), '', 'policy', 'xml'); + await write(outputDir, { type: ResourceType.Backend, nameParts: ['function-backend'] }); + // billing reaches the backend through the named value's display name. + await write(outputDir, billing); + await store.writeContent(outputDir, policy('billing'), '', 'policy', 'xml'); + await write(outputDir, namedValue, { displayName: 'Backend_Id', value: 'function-backend' }); + await write(stagingDir, orders); + const failingStore: IArtifactStore = Object.create(store); + failingStore.readResource = async (dir, descriptor) => { + if (descriptor.type === ResourceType.NamedValue) { + throw new Error('Unexpected token in JSON'); + } + return store.readResource(dir, descriptor); + }; + + try { + expect(await findStaleDependencies(failingStore, outputDir, stagingDir, { ...filter, backends: [] })).toEqual([]); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Keeping every pulled-in dependency')); + } finally { + warn.mockRestore(); + } + }); + it('should report an excluded tag that only an old link from the selected API referenced', async () => { await write(outputDir, orders); await write(outputDir, apiTag('orders', 'internal')); From 7189485588cc7d42e2b775ebe074d7611fc09e52 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:22:21 +1100 Subject: [PATCH 14/23] fix: follow a re-extracted dependency's old references in --remove-stale A dependency the extract wrote again stopped the walk over what the selection used to depend on, so a named value only its old version referenced (a logger's former credential, say) was never considered and stayed in the output. Its old version is now followed too; it is still never a candidate itself. --- src/services/stale-dependency-finder.ts | 10 ++++++++-- .../unit/services/stale-dependency-finder.test.ts | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/services/stale-dependency-finder.ts b/src/services/stale-dependency-finder.ts index ff5f5e09..2e632059 100644 --- a/src/services/stale-dependency-finder.ts +++ b/src/services/stale-dependency-finder.ts @@ -99,7 +99,10 @@ export async function findStaleDependencies( const readExisting = (descriptor: ResourceDescriptor) => store.readResource(outputDir, descriptor); const resolveExisting = createArtifactReferenceResolver(existing, readExisting); const readExistingNamedValue = createNamedValueReader(resolveExisting, readExisting); + // A dependency the extract wrote again is not a candidate, but its old + // version is still followed: what it no longer references can be stale. const candidates = new Map(); + const followed = new Set(); let wave: ArtifactLocation[] = existing.filter(isSelected).map((descriptor) => ({ descriptor, dir: outputDir })); while (wave.length > 0) { // An artifact that can't be scanned contributes no candidates, which only keeps more. @@ -107,10 +110,13 @@ export async function findStaleDependencies( wave = []; for (const reference of references) { const target = await resolveExisting(reference); - if (!target || isSelected(target) || stagedKeys.has(key(target)) || candidates.has(key(target))) { + if (!target || isSelected(target) || followed.has(key(target))) { continue; } - candidates.set(key(target), target); + followed.add(key(target)); + if (!stagedKeys.has(key(target))) { + candidates.set(key(target), target); + } wave.push({ descriptor: target, dir: outputDir }); } } diff --git a/tests/unit/services/stale-dependency-finder.test.ts b/tests/unit/services/stale-dependency-finder.test.ts index 5f13f30c..a07270d5 100644 --- a/tests/unit/services/stale-dependency-finder.test.ts +++ b/tests/unit/services/stale-dependency-finder.test.ts @@ -136,6 +136,20 @@ describe('stale-dependency-finder', () => { expect(names(await findStaleDependencies(store, outputDir, stagingDir, filter))).toEqual(['NamedValue:backend-id']); }); + it('should report what a dependency the extract wrote again no longer references', async () => { + const diagnostic: ResourceDescriptor = { type: ResourceType.ApiDiagnostic, nameParts: ['orders', 'applicationinsights'] }; + const aiLogger: ResourceDescriptor = { type: ResourceType.Logger, nameParts: ['ai'] }; + await write(outputDir, orders); + await write(outputDir, diagnostic, { loggerId: '/loggers/ai' }); + await write(outputDir, aiLogger, { credentials: { instrumentationKey: '{{AI Key}}' } }); + await write(outputDir, { type: ResourceType.NamedValue, nameParts: ['ai-key'] }, { displayName: 'AI Key' }); + await write(stagingDir, orders); + await write(stagingDir, diagnostic, { loggerId: '/loggers/ai' }); + await write(stagingDir, aiLogger, { credentials: { connectionString: 'InstrumentationKey=00000000-0000-0000-0000-000000000000' } }); + + expect(names(await findStaleDependencies(store, outputDir, stagingDir, filter))).toEqual(['NamedValue:ai-key']); + }); + it("should keep a dependency's own dependency that another artifact still uses", async () => { const diagnostic: ResourceDescriptor = { type: ResourceType.ApiDiagnostic, nameParts: ['orders', 'applicationinsights'] }; await write(outputDir, orders); From 0d8fc78ee7d05a1bce135b67c4be2006bc157a3d Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:23:33 +1100 Subject: [PATCH 15/23] fix: check a named value exclusion against the name a reference resolves to A {{reference}} was dropped when an exclusion named it, before it was resolved, so {{two}} was left out under '!two' even when it is the display name of named value one, which the filter keeps. A named value reference is now resolved first and checked under the resource name it resolves to, as publish already does. --- src/services/transitive-extractor.ts | 26 ++++++++++++------- .../services/transitive-extractor.test.ts | 16 ++++++++++++ 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index 8669000b..589e90b8 100644 --- a/src/services/transitive-extractor.ts +++ b/src/services/transitive-extractor.ts @@ -210,6 +210,8 @@ export async function extractTransitiveDependencies( const extractedDescriptors: ResourceDescriptor[] = []; const failedDescriptors: ResourceDescriptor[] = []; const namedValueIndex: NamedValueNameIndex = new Map(); + const contextFor = (dep: ResourceDescriptor): ApimServiceContext => + serviceContext && dep.workspace !== workspace ? serviceContext : context; let errorCount = 0; let foundDependencies = false; @@ -222,11 +224,23 @@ export async function extractTransitiveDependencies( for (; resourcesScanned < resources.length; resourcesScanned++) { fragmentLiquidCandidates(resources[resourcesScanned]).forEach((name) => liquidCandidates.add(name)); } - const newDeps = [ + const found = [ ...findTransitiveDependencies(policies, apis, workspace, resources), ...linkedDependencies, ...await liquidNamedValueDependencies(client, context, liquidCandidates, workspace, namedValueIndex), - ].filter((dep) => !attempted.has(getResourceDescriptorKey(dep)) && !skip(dep)); + ].filter((dep) => !attempted.has(getResourceDescriptorKey(dep))); + const newDeps: ResourceDescriptor[] = []; + for (const dep of found) { + // A {{reference}} can be a display name, so a named value is checked + // under the resource name it resolves to, which is what an exclusion + // names. + const checked = dep.type === ResourceType.NamedValue + ? await resolveNamedValueDescriptor(client, contextFor(dep), dep, namedValueIndex) + : dep; + if (!skip(checked)) { + newDeps.push(dep); + } + } if (newDeps.length === 0) { if (!foundDependencies) { @@ -243,17 +257,11 @@ export async function extractTransitiveDependencies( const tasks = newDeps.map((dep) => async (): Promise => { try { - const dependencyContext = - serviceContext && dep.workspace !== workspace ? serviceContext : context; + const dependencyContext = contextFor(dep); let target = dep; if (dep.type === ResourceType.NamedValue) { target = await resolveNamedValueDescriptor(client, dependencyContext, dep, namedValueIndex); const targetKey = getResourceDescriptorKey(target); - // A {{displayName}} reference is checked again under the resource - // name it resolves to, which is what an exclusion names. - if (target !== dep && skip(target)) { - return { dep: target, skipped: true }; - } if (claimedNamedValues.has(targetKey)) { return { dep: target, skipped: true }; } diff --git a/tests/unit/services/transitive-extractor.test.ts b/tests/unit/services/transitive-extractor.test.ts index c30931cd..ec5591b0 100644 --- a/tests/unit/services/transitive-extractor.test.ts +++ b/tests/unit/services/transitive-extractor.test.ts @@ -106,6 +106,22 @@ describe('transitive-extractor', () => { expect(store.writeResource).not.toHaveBeenCalled(); }); + it('should extract a {{displayName}} reference whose display name is the name of an excluded named value', async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'one', displayName: 'two' }, { name: 'two', displayName: 'other' }); + client.getResource.mockResolvedValue({ name: 'one', properties: { displayName: 'two' } }); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('two'), new Map(), [], [], + { skip: (dep) => dep.nameParts[0] === 'two' } + ); + + expect(result.extractedDescriptors).toEqual([ + expect.objectContaining({ type: ResourceType.NamedValue, nameParts: ['one'] }), + ]); + }); + 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' }); From 3c7e7a61bd76f624f76f19a5874195f51cd0b291 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:24:39 +1100 Subject: [PATCH 16/23] fix: follow what an artifact references after overrides on publish Publish found a resource's dependencies in the source JSON, before overrides, so an override that changes a backend's credentials from {{dev-key}} to {{prod-key}} pulled in dev-key, and publishing to a target without prod-key failed. References are now read from the payload as it is published, with this environment's overrides applied. --- src/services/publish-service.ts | 7 ++++- src/services/transitive-resolver.ts | 8 ++++-- tests/unit/services/publish-service.test.ts | 30 +++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index d0b438f3..5164478e 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -454,7 +454,12 @@ async function expandTransitivePublishTargets( for (let index = 0; index < scanQueue.length; index++) { let references: ResourceDescriptor[]; try { - references = await scanArtifactReferences(store, sourceDir, scanQueue[index], readNamedValue); + // As published: an override can change what a backend's or logger's + // credentials or a diagnostic's logger reference. + references = await scanArtifactReferences( + store, sourceDir, scanQueue[index], readNamedValue, + (descriptor, json) => applyOverrides(descriptor, json, overrides) + ); if (scanQueue[index].type === ResourceType.NamedValue) { references.push(...await findNamedValueTargets(scanQueue[index])); } diff --git a/src/services/transitive-resolver.ts b/src/services/transitive-resolver.ts index ffadb94b..d6ac4b54 100644 --- a/src/services/transitive-resolver.ts +++ b/src/services/transitive-resolver.ts @@ -580,12 +580,15 @@ export type NamedValueReader = ( * @param readNamedValue - Reads the named value a `{{reference}}` resolves to, * so a policy attribute set to one (backend-id="{{backend-name}}") also * gives the resource its value names. Without it, only the named value is. + * @param prepareJson - Turns the artifact's JSON into what is scanned, such + * as the payload publish sends with overrides applied */ export async function scanArtifactReferences( store: IArtifactStore, sourceDir: string, descriptor: ResourceDescriptor, - readNamedValue?: NamedValueReader + readNamedValue?: NamedValueReader, + prepareJson?: (descriptor: ResourceDescriptor, json: Record) => Record ): Promise { if (!REFERENCING_TYPES.has(descriptor.type)) { return []; @@ -602,9 +605,10 @@ export async function scanArtifactReferences( } const infoFile = RESOURCE_TYPE_METADATA[descriptor.type]?.infoFile; - const json = POLICY_RESOURCE_TYPES.has(descriptor.type) || !infoFile?.endsWith('.json') + const stored = POLICY_RESOURCE_TYPES.has(descriptor.type) || !infoFile?.endsWith('.json') ? undefined : await store.readResource(sourceDir, descriptor); + const json = stored && prepareJson ? prepareJson(descriptor, stored) : stored; if (json && descriptor.type === ResourceType.Api) { apis.set(descriptor.nameParts.join('/'), json); } diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 1e0c23c6..42ee8dde 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -557,6 +557,36 @@ describe('publish-service', () => { expect(backendPuts).toEqual(['prod-backend']); }); + it("should pull in the named values a backend's overridden credentials use", async () => { + const resources = [ + { type: ResourceType.Backend, nameParts: ['api-backend'] }, + { type: ResourceType.NamedValue, nameParts: ['dev-key'] }, + { type: ResourceType.NamedValue, nameParts: ['prod-key'] }, + ]; + const client = createMockClient(); + const store = createMockStore(resources); + store.readResource.mockImplementation(async (_sourceDir, descriptor) => + descriptor.type === ResourceType.Backend + ? { properties: { url: 'https://example.test', credentials: { header: { 'x-key': ['{{dev-key}}'] } } } } + : { properties: { displayName: descriptor.nameParts[0], value: 'secret' } } + ); + + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter: { backends: ['api-backend'], namedValues: [] }, + overrides: { backends: { 'api-backend': { properties: { credentials: { header: { 'x-key': ['{{prod-key}}'] } } } } } }, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + + const namedValuePuts = client.putResource.mock.calls + .filter((call) => call[1].type === ResourceType.NamedValue) + .map((call) => call[1].nameParts[0]); + expect(namedValuePuts).toEqual(['prod-key']); + }); + it('should include the backend a changed named value now names when only the named value changed', async () => { const namedValue = { type: ResourceType.NamedValue, nameParts: ['backend-id'] }; const resources = [ From 00861b4ee5c2d13b85ddb3efaa27a3138e6d9b37 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:26:34 +1100 Subject: [PATCH 17/23] fix: publish a credential user after the named value and what it names A named value that one backend uses in its credentials and whose value names another backend was published before both, so APIM rejected it while a policy on the target used it as a backend name and the backend it names didn't exist yet. The order is now the backend it names, the named value, then the backend that uses it. When the user has to go first too (it is what the named value names, or a member of a pool published here), the named value stays first as before. --- src/services/publish-service.ts | 77 +++++++++++++++------ tests/unit/services/publish-service.test.ts | 51 ++++++++++++++ 2 files changed, 108 insertions(+), 20 deletions(-) diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 5164478e..62f22c4e 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -51,6 +51,7 @@ import { } from './delete-unmatched-service.js'; import { computeGitDiff } from './git-diff-service.js'; import { scanForRedactionMarkers } from './secret-redaction-guard.js'; +import { armNameAfter } from '../lib/arm-id.js'; import { applyOverrides, hasNamedValueOverride } from './override-merger.js'; import { REDACTION_MARKER } from './secret-redactor.js'; import { @@ -600,8 +601,8 @@ async function executePuts( // named value). Publishing named values first ensures they exist // before any resource that references them. // - // Wave 2 — The remaining Tier 1 resources except pool backends (and - // except policy fragments when wave 4 has named values). + // Wave 2 — The remaining Tier 1 resources except pool backends and + // wave 5 (and except policy fragments when wave 4 has named values). // // Wave 3 — Pool backends. // Pool backends reference individual Backend resources and must be @@ -614,9 +615,12 @@ async function executePuts( // value when it changes, so that resource must exist first (see // splitNamedValuesNamingResources). // - // Wave 5 — Policy fragments, when wave 4 has named values: a fragment + // Wave 5 — Backends and loggers whose credentials use a wave 4 named + // value, when they don't have to go before one. + // + // Wave 6 — Policy fragments, when wave 4 has named values: a fragment // can use them, and APIM rejects a policy that uses a missing one. - const { early: earlyNamedValues, late: lateNamedValues } = await splitNamedValuesNamingResources( + const { early: earlyNamedValues, late: lateNamedValues, after: afterLateNamedValues } = await splitNamedValuesNamingResources( store, config, namedValues, @@ -631,7 +635,7 @@ async function executePuts( const { poolBackends, regularTier1 } = await splitPoolBackends( store, config.sourceDir, - otherTier1 + otherTier1.filter((descriptor) => !afterLateNamedValues.includes(descriptor)) ); const fragments = lateNamedValues.length > 0 ? regularTier1.filter((descriptor) => descriptor.type === ResourceType.PolicyFragment) @@ -665,6 +669,9 @@ async function executePuts( ); await publishAndOutput(client, store, context, config, lateNamedValues, targetDescriptors, results); } + if (afterLateNamedValues.length > 0) { + await publishAndOutput(client, store, context, config, afterLateNamedValues, targetDescriptors, results); + } if (fragments.length > 0) { await publishAndOutput(client, store, context, config, fragments, targetDescriptors, results); } @@ -887,11 +894,13 @@ function splitNamedValues( * value (with overrides) names a resource of that type published in this * tier. APIM checks the policies that use a named value when the named value * changes, rejecting the change ("its value breaks policies") while the - * resource it now names doesn't exist. One that a backend or logger published - * here uses in its credentials stays first, since that resource needs it. The - * policies are read (once) only when a named value's value matches the name of - * such a resource here; a named value that can't be read goes first, and - * publishing it reports the error. + * resource it now names doesn't exist. A backend or logger published here + * that uses one of those in its credentials is published after it (`after`); + * when that resource has to go before a later named value too (it is what one + * names, or a member of a pool here), the named value stays first instead. + * The policies are read (once) only when a named value's value matches the + * name of such a resource here; a named value that can't be read goes first, + * and publishing it reports the error. */ async function splitNamedValuesNamingResources( store: IArtifactStore, @@ -899,7 +908,7 @@ async function splitNamedValuesNamingResources( namedValues: ResourceDescriptor[], otherTier1: ResourceDescriptor[], namedValueTargets: NamedValueTargetIndex -): Promise<{ early: ResourceDescriptor[]; late: ResourceDescriptor[] }> { +): Promise<{ early: ResourceDescriptor[]; late: ResourceDescriptor[]; after: ResourceDescriptor[] }> { const resourceKey = (type: ResourceType, name: string, workspace: string | undefined) => `${type}/${workspace ?? ''}/${name.toLowerCase()}`; const named = new Set( @@ -910,8 +919,10 @@ async function splitNamedValuesNamingResources( const early: ResourceDescriptor[] = []; const late: ResourceDescriptor[] = []; if (named.size === 0) { - return { early: namedValues, late }; + return { early: namedValues, late, after: [] }; } + // The resources the late named values name, which go before them. + const lateTargets = new Set(); // The named values policies use as resource names, and as which types. // Every policy in the source counts, not only the ones the filter selects: // APIM checks every policy on the target that uses the named value, and @@ -924,8 +935,11 @@ async function splitNamedValuesNamingResources( const json = await store.readResource(config.sourceDir, descriptor); const name = json ? namedValueTargetName(applyOverrides(descriptor, json, config.overrides)) : undefined; if (name && [...NAMED_THROUGH_NAMED_VALUE_TYPES].some((type) => named.has(resourceKey(type, name, descriptor.workspace)))) { - const types = await typesFor(descriptor); - isLate = [...(types ?? [])].some((type) => named.has(resourceKey(type, name, descriptor.workspace))); + const targets = [...(await typesFor(descriptor) ?? [])] + .map((type) => resourceKey(type, name, descriptor.workspace)) + .filter((target) => named.has(target)); + targets.forEach((target) => lateTargets.add(target)); + isLate = targets.length > 0; } } catch { isLate = false; @@ -933,15 +947,18 @@ async function splitNamedValuesNamingResources( (isLate ? late : early).push(descriptor); } if (late.length === 0) { - return { early, late }; + return { early, late, after: [] }; } // A backend or logger published here whose own configuration (its - // credentials) uses one of those named values needs it first, so that one - // stays early. + // credentials) uses one of those named values needs it first: it is + // published after the late named values, unless it has to go before them, + // in which case the named value it uses stays early. const available = await store.listResources(config.sourceDir); const resolve = createArtifactReferenceResolver(available, (descriptor) => store.readResource(config.sourceDir, descriptor)); - const neededFirst = new Set(); + const lateKeys = new Set(late.map(getResourceDescriptorKey)); + const users: Array<{ descriptor: ResourceDescriptor; uses: string[] }> = []; + const poolMembers = new Set(); for (const descriptor of otherTier1) { if (descriptor.type !== ResourceType.Backend && descriptor.type !== ResourceType.Logger) { continue; @@ -956,16 +973,36 @@ async function splitNamedValuesNamingResources( continue; } const resource = { descriptor, json: applyOverrides(descriptor, json, config.overrides) }; + const uses: string[] = []; for (const dependency of findTransitiveDependencies(new Map(), new Map(), descriptor.workspace, [resource])) { const target = dependency.type === ResourceType.NamedValue ? await resolve(dependency) : undefined; - if (target) { - neededFirst.add(getResourceDescriptorKey(target)); + if (target && lateKeys.has(getResourceDescriptorKey(target))) { + uses.push(getResourceDescriptorKey(target)); + } + } + if (uses.length > 0) { + users.push({ descriptor, uses }); + } + const pool = (resource.json.properties as Record | undefined)?.pool as Record | undefined; + for (const service of Array.isArray(pool?.services) ? pool.services : []) { + const id = (service as Record | undefined)?.id; + const member = typeof id === 'string' ? armNameAfter(id, 'backends') : undefined; + if (member) { + poolMembers.add(resourceKey(ResourceType.Backend, member, descriptor.workspace)); } } } + const mustGoFirst = users.filter(({ descriptor }) => { + const key = resourceKey(descriptor.type, getNamePart(descriptor.nameParts, 0), descriptor.workspace); + return lateTargets.has(key) || poolMembers.has(key); + }); + const neededFirst = new Set(mustGoFirst.flatMap(({ uses }) => uses)); return { early: [...early, ...late.filter((descriptor) => neededFirst.has(getResourceDescriptorKey(descriptor)))], late: late.filter((descriptor) => !neededFirst.has(getResourceDescriptorKey(descriptor))), + after: users + .filter((user) => !mustGoFirst.includes(user) && user.uses.some((use) => !neededFirst.has(use))) + .map(({ descriptor }) => descriptor), }; } diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 42ee8dde..36bcd377 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -2187,6 +2187,57 @@ describe('publish-service', () => { expect(order.indexOf('shared')).toBeLessThan(order.indexOf('prod-backend')); }); + it('should publish a backend using a named value in its credentials after the named value and the backend it names', async () => { + const { order, client, store } = recordOrder([ + { type: ResourceType.NamedValue, nameParts: ['route'] }, + { type: ResourceType.Backend, nameParts: ['new-backend'] }, + { type: ResourceType.Backend, nameParts: ['caller'] }, + { type: ResourceType.PolicyFragment, nameParts: ['frag'] }, + ]); + vi.mocked(store.readResource).mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.NamedValue) { + return { properties: { displayName: 'route', value: 'new-backend' } }; + } + if (descriptor.type === ResourceType.PolicyFragment) { + return { properties: { value: '' } }; + } + return descriptor.nameParts[0] === 'caller' + ? { properties: { url: 'https://example.test', credentials: { header: { 'x-route': ['{{route}}'] } } } } + : { properties: { url: 'https://example.test' } }; + }); + + await publishAll(client, store); + + expect(order.indexOf('route')).toBeGreaterThan(order.indexOf('new-backend')); + expect(order.indexOf('caller')).toBeGreaterThan(order.indexOf('route')); + expect(order.indexOf('frag')).toBeGreaterThan(order.indexOf('route')); + }); + + it('should keep a named value first when a pool member uses it in its credentials', async () => { + const { order, client, store } = recordOrder([ + { type: ResourceType.NamedValue, nameParts: ['route'] }, + { type: ResourceType.Backend, nameParts: ['member'] }, + { type: ResourceType.Backend, nameParts: ['my-pool'] }, + { type: ResourceType.PolicyFragment, nameParts: ['frag'] }, + ]); + vi.mocked(store.readResource).mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.NamedValue) { + return { properties: { displayName: 'route', value: 'my-pool' } }; + } + if (descriptor.type === ResourceType.PolicyFragment) { + return { properties: { value: '' } }; + } + return descriptor.nameParts[0] === 'my-pool' + ? { properties: { type: 'Pool', pool: { services: [{ id: '/backends/member' }] } } } + : { properties: { url: 'https://example.test', credentials: { header: { 'x-route': ['{{route}}'] } } } }; + }); + + await publishAll(client, store); + + expect(order.indexOf('route')).toBeLessThan(order.indexOf('member')); + expect(order.indexOf('my-pool')).toBeGreaterThan(order.indexOf('member')); + }); + it('should order by a policy the filter leaves out, since APIM checks it on the target', async () => { const { order, client, store } = recordOrder([ { type: ResourceType.Api, nameParts: ['orders'] }, From 87f5e8c41d42de9b46687140889055524978a160 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:26:57 +1100 Subject: [PATCH 18/23] docs: describe when a backend using a named value in its credentials is published --- docs/guides/filtering-resources.md | 2 +- docs/reference/dependency-graph.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/guides/filtering-resources.md b/docs/guides/filtering-resources.md index b18909e1..b5bdf039 100644 --- a/docs/guides/filtering-resources.md +++ b/docs/guides/filtering-resources.md @@ -335,7 +335,7 @@ When you filter resources, `apiops extract` and `apiops publish` automatically i | Backend pool | Its member backends | | Product | The groups and tags it is linked to | -A reference written as a policy expression (`backend-id="@(...)"`) is not followed, because its value is only known at run time. One set to a named value (`backend-id="{{backend-name}}"`) pulls in the named value and the resource its value names, in the same scope (except ``, where APIM doesn't substitute named values); publish reads the value with your `--overrides` applied, so each environment gets the resource its own value names. A named value that is published also brings the resource its value names whenever any policy or fragment in the source uses it that way, even one the filter leaves out: APIM checks every policy on the target that uses a named value when its value changes. A resource the filter excludes by name, or that isn't in the source, isn't published, so it has to exist on the target already. When a policy uses a named value to name a backend, logger or schema that is published in the same run, publish creates that resource before it updates the named value, and publishes policy fragments after it, since APIM rejects a named value change that would leave a policy using it pointing at a resource that doesn't exist. A secret or Key Vault named value's value isn't known, so the resource it names isn't pulled in; add that resource to the filter. +A reference written as a policy expression (`backend-id="@(...)"`) is not followed, because its value is only known at run time. One set to a named value (`backend-id="{{backend-name}}"`) pulls in the named value and the resource its value names, in the same scope (except ``, where APIM doesn't substitute named values); publish reads the value with your `--overrides` applied, so each environment gets the resource its own value names. A named value that is published also brings the resource its value names whenever any policy or fragment in the source uses it that way, even one the filter leaves out: APIM checks every policy on the target that uses a named value when its value changes. A resource the filter excludes by name, or that isn't in the source, isn't published, so it has to exist on the target already. When a policy uses a named value to name a backend, logger or schema that is published in the same run, publish creates that resource before it updates the named value, and publishes policy fragments, and any backend or logger using the named value in its credentials, after it, since APIM rejects a named value change that would leave a policy using it pointing at a resource that doesn't exist. A secret or Key Vault named value's value isn't known, so the resource it names isn't pulled in; add that resource to the filter. A `{{...}}` reference, in a policy or in credentials, names a named value by its display name, as APIM does; when no named value has that display name, it is read as a resource name. The `namedValues` list and its exclusions (`'!name'`) use the resource name, which can differ from the display name. References inside XML comments are ignored. Inside a `` body, a `{{name}}` is followed only when a named value has that name: APIM substitutes those there and leaves every other `{{ ... }}` (`{{body.id}}`, `{{ name }}`, `{{x | upcase}}`) to Liquid, so those are skipped without an error. diff --git a/docs/reference/dependency-graph.md b/docs/reference/dependency-graph.md index 1f439fdc..162d185d 100644 --- a/docs/reference/dependency-graph.md +++ b/docs/reference/dependency-graph.md @@ -15,7 +15,7 @@ apiops-cli solves this by organizing all 34 resource types into **4 tiers**. Eac | Tier | Resources | Rule | |------|-----------|------| -| **1** | NamedValue, Tag, Gateway, VersionSet, Backend, Logger, Group, PolicyFragment, GlobalSchema, PolicyRestriction, Documentation | No dependencies on other tiers. Within the tier, named values go first, since backends and loggers can use them; pool backends follow the backends they contain; and a named value a policy uses to name a backend, logger or schema published in the same run goes after that resource (unless a backend or logger in the run uses it in its credentials), with policy fragments after it in that case | +| **1** | NamedValue, Tag, Gateway, VersionSet, Backend, Logger, Group, PolicyFragment, GlobalSchema, PolicyRestriction, Documentation | No dependencies on other tiers. Within the tier, named values go first, since backends and loggers can use them; pool backends follow the backends they contain; and a named value a policy uses to name a backend, logger or schema published in the same run goes after that resource, with policy fragments after it in that case. A backend or logger in the run that uses such a named value in its credentials goes after the named value; when it also has to go before it (it is what the named value names, or a member of a pool in the run), the named value goes first | | **2** | Diagnostic, ServicePolicy, Product, Api | Depend only on Tier 1 resources | | **3** | ProductPolicy, ProductGroup, ProductTag, ProductApi, ProductWiki, ApiPolicy, ApiTag, ApiDiagnostic, ApiOperation, ApiSchema, ApiRelease, ApiTagDescription, ApiWiki, McpServer, GraphQLResolver, GatewayApi, Subscription | Depend on Tier 1 and/or Tier 2 resources | | **4** | ApiOperationPolicy, GraphQLResolverPolicy | Depend on Tier 3 resources | From 21e08577ff6ff05581c9c347dfcf6b9f615982d6 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:48:07 +1100 Subject: [PATCH 19/23] fix: resolve a reference to an extracted named value's name A {{reference}} was dropped when a named value with that resource name was already extracted, before it was resolved, so {{two}} was missed when it is the display name of named value one. Named value references are now compared with what is extracted only after they are resolved. --- src/services/transitive-extractor.ts | 9 +++++++-- tests/unit/services/transitive-extractor.test.ts | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index 589e90b8..43aedb42 100644 --- a/src/services/transitive-extractor.ts +++ b/src/services/transitive-extractor.ts @@ -205,7 +205,12 @@ export async function extractTransitiveDependencies( skip = () => false, }: TransitiveExtractionOptions = {} ): Promise { - const attempted = new Set(alreadyExtracted.map(getResourceDescriptorKey)); + // A {{reference}} to an extracted named value's name can be another named + // value's display name, so named value references are only compared with + // what is extracted once resolved (claimedNamedValues). + const attempted = new Set( + alreadyExtracted.filter((descriptor) => descriptor.type !== ResourceType.NamedValue).map(getResourceDescriptorKey) + ); const claimedNamedValues = new Set(alreadyExtracted.map(getResourceDescriptorKey)); const extractedDescriptors: ResourceDescriptor[] = []; const failedDescriptors: ResourceDescriptor[] = []; @@ -237,7 +242,7 @@ export async function extractTransitiveDependencies( const checked = dep.type === ResourceType.NamedValue ? await resolveNamedValueDescriptor(client, contextFor(dep), dep, namedValueIndex) : dep; - if (!skip(checked)) { + if (!skip(checked) && !claimedNamedValues.has(getResourceDescriptorKey(checked))) { newDeps.push(dep); } } diff --git a/tests/unit/services/transitive-extractor.test.ts b/tests/unit/services/transitive-extractor.test.ts index ec5591b0..e1a9284f 100644 --- a/tests/unit/services/transitive-extractor.test.ts +++ b/tests/unit/services/transitive-extractor.test.ts @@ -122,6 +122,22 @@ describe('transitive-extractor', () => { ]); }); + it('should extract a {{displayName}} reference whose display name is the name of an extracted named value', async () => { + const client = createMockClient(); + client.listResources = namedValueList({ name: 'one', displayName: 'two' }, { name: 'two', displayName: 'three' }); + client.getResource.mockResolvedValue({ name: 'one', properties: { displayName: 'two' } }); + const store = createMockStore(); + + const result = await extractTransitiveDependencies( + client, store, testContext, '/output', policiesWith('two'), new Map(), [], + [{ type: ResourceType.NamedValue, nameParts: ['two'] }] + ); + + expect(result.extractedDescriptors).toEqual([ + expect.objectContaining({ type: ResourceType.NamedValue, nameParts: ['one'] }), + ]); + }); + 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' }); From 2ecc3db2ec55146fcfd620933b6f78b036e457bf Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:49:13 +1100 Subject: [PATCH 20/23] fix: report an error when Liquid named values can't be told apart A {{name}} in a Liquid body is followed only when a named value has that name, which needs the named value list. When the list failed, every candidate was skipped with only a warning about display names, and the extract reported no errors, so --remove-stale could run on an output missing a named value. The failure now counts as an extraction error when there are Liquid candidates. Other references keep falling back to resource names. --- src/services/transitive-extractor.ts | 46 +++++++++++++------ .../services/transitive-extractor.test.ts | 20 ++++++++ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index 43aedb42..0e7fc29a 100644 --- a/src/services/transitive-extractor.ts +++ b/src/services/transitive-extractor.ts @@ -73,12 +73,18 @@ interface TransitiveTaskResult { * compared everywhere else) → resource name. * Stored as promises so concurrent tasks share a single LIST call. */ -type NamedValueNameIndex = Map>>; +type NamedValueNameIndex = Map>; + +interface NamedValueNames { + names: Map; + /** False when the named values couldn't be listed, so `names` is incomplete. */ + listed: boolean; +} async function loadNamedValueNames( client: IApimClient, context: ApimServiceContext -): Promise> { +): Promise { const byDisplayName = new Map(); const resourceNames: string[] = []; try { @@ -104,8 +110,9 @@ async function loadNamedValueNames( `Failed to list named values for display name resolution; ` + `falling back to resolving {{references}} as resource names: ${message}` ); + return { names: byDisplayName, listed: false }; } - return byDisplayName; + return { names: byDisplayName, listed: true }; } /** The named value name index for a context, listed once and shared. */ @@ -113,7 +120,7 @@ function namedValueNames( client: IApimClient, context: ApimServiceContext, index: NamedValueNameIndex -): Promise> { +): Promise { let names = index.get(context.baseUrl); if (!names) { names = loadNamedValueNames(client, context); @@ -134,7 +141,7 @@ async function resolveNamedValueDescriptor( index: NamedValueNameIndex ): Promise { const ref = dep.nameParts[0] ?? ''; - const name = (await namedValueNames(client, context, index)).get(ref.toLowerCase()); + const name = (await namedValueNames(client, context, index)).names.get(ref.toLowerCase()); if (!name || name === ref) { return dep; } @@ -157,7 +164,8 @@ function fragmentLiquidCandidates({ descriptor, json }: TransitiveResourceArtifa /** * The named values Liquid bodies reference: each `{{name}}` candidate that a * named value in this scope has as its display or resource name. The others - * are Liquid, skipped without a warning. + * are Liquid, skipped without a warning. `listed` is false when the named + * values couldn't be listed, so a candidate that is one may be missed. */ async function liquidNamedValueDependencies( client: IApimClient, @@ -165,14 +173,17 @@ async function liquidNamedValueDependencies( candidates: ReadonlySet, workspace: string | undefined, index: NamedValueNameIndex -): Promise { +): Promise<{ dependencies: ResourceDescriptor[]; listed: boolean }> { if (candidates.size === 0) { - return []; + return { dependencies: [], listed: true }; } - const names = await namedValueNames(client, context, index); - return [...candidates] - .filter((candidate) => names.has(candidate.toLowerCase())) - .map((candidate) => ({ type: ResourceType.NamedValue, nameParts: [candidate], workspace })); + const { names, listed } = await namedValueNames(client, context, index); + return { + dependencies: [...candidates] + .filter((candidate) => names.has(candidate.toLowerCase())) + .map((candidate) => ({ type: ResourceType.NamedValue, nameParts: [candidate], workspace })), + listed, + }; } export interface TransitiveExtractionOptions { @@ -219,6 +230,7 @@ export async function extractTransitiveDependencies( serviceContext && dep.workspace !== workspace ? serviceContext : context; let errorCount = 0; let foundDependencies = false; + let reportedUnlisted = false; // The {{name}}s in Liquid bodies: the policies' found once, and each // fragment's as it is extracted. @@ -229,10 +241,18 @@ export async function extractTransitiveDependencies( for (; resourcesScanned < resources.length; resourcesScanned++) { fragmentLiquidCandidates(resources[resourcesScanned]).forEach((name) => liquidCandidates.add(name)); } + const liquid = await liquidNamedValueDependencies(client, context, liquidCandidates, workspace, namedValueIndex); + if (!liquid.listed && !reportedUnlisted) { + // A named value used only in a Liquid body may be missing from the + // output, so the extract is incomplete (and --remove-stale won't run). + reportedUnlisted = true; + errorCount++; + logger.warn('Unable to tell which {{...}} in Liquid bodies are named values, since the named values could not be listed'); + } const found = [ ...findTransitiveDependencies(policies, apis, workspace, resources), ...linkedDependencies, - ...await liquidNamedValueDependencies(client, context, liquidCandidates, workspace, namedValueIndex), + ...liquid.dependencies, ].filter((dep) => !attempted.has(getResourceDescriptorKey(dep))); const newDeps: ResourceDescriptor[] = []; for (const dep of found) { diff --git a/tests/unit/services/transitive-extractor.test.ts b/tests/unit/services/transitive-extractor.test.ts index e1a9284f..99d4e0fe 100644 --- a/tests/unit/services/transitive-extractor.test.ts +++ b/tests/unit/services/transitive-extractor.test.ts @@ -8,6 +8,7 @@ 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'; +import { logger } from '../../../src/lib/logger.js'; const testContext: ApimServiceContext = { subscriptionId: 'sub-1', @@ -376,6 +377,25 @@ describe('extractTransitiveDependencies with Liquid bodies', () => { expect(result.extractedDescriptors.map((d) => d.nameParts[0])).toEqual(['backend-id']); }); + it('should report an error when the named values for a Liquid body cannot be listed', async () => { + const warn = vi.spyOn(logger, 'warn').mockImplementation(() => undefined); + const client = createMockClient(); + // eslint-disable-next-line require-yield + client.listResources = async function* () { + throw new Error('HTTP 503'); + }; + const policies = new Map([['api:orders:policy:0', '{{demo-backend-id}}']]); + + try { + const result = await extractTransitiveDependencies(client, createMockStore(), testContext, '/out', policies, new Map(), [], []); + + expect(result.errorCount).toBe(1); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Unable to tell which {{...}} in Liquid bodies are named values')); + } finally { + warn.mockRestore(); + } + }); + it("should follow a Liquid {{name}} in a fragment extracted in a later round", async () => { const client = createMockClient(); client.listResources = namedValueList({ name: 'frag-nv', displayName: 'frag-nv' }); From 5c1cc39541587f70e0ffd8a5a83795b082e117b1 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:50:57 +1100 Subject: [PATCH 21/23] fix: match logger credential references by display name first Logger credentials were rewritten from {{name}} to the display name of the named value with that resource name. APIM matches a reference by display name first, so a reference that is already another named value's display name was pointed at a different named value, one that transitive publish doesn't pull in. A reference is now matched as APIM matches it, and rewritten only to its own named value's (prefixed) display name. --- src/services/resource-publisher.ts | 34 +++++++++++++++---- .../unit/services/resource-publisher.test.ts | 29 ++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts index 924c2b84..2c3c98a3 100644 --- a/src/services/resource-publisher.ts +++ b/src/services/resource-publisher.ts @@ -1668,7 +1668,9 @@ async function publishWorkspaceApiTagLink( * rewrites the reference. * * References that already use displayName (or don't match any named value - * artifact) are left unchanged. + * artifact) are left unchanged. A reference is matched as APIM matches it, + * by display name first, so one that is another named value's display name + * stays a reference to that named value. */ async function rewriteNamedValueReferences( json: Record, @@ -1688,16 +1690,36 @@ async function rewriteNamedValueReferences( const credStr = JSON.stringify(credentials); const refPattern = /\{\{([^}]+)\}\}/g; const rewriteMap = new Map(); + // The named values in the logger's scope by display name (lower case), + // read on the first reference. + let displayNames: Promise> | undefined; + const readDisplayNames = async (): Promise> => { + const byDisplayName = new Map(); + const namedValues = (await store.listResources(sourceDir)).filter( + (candidate) => candidate.type === ResourceType.NamedValue && candidate.workspace === descriptor.workspace + ); + for (const namedValue of namedValues) { + const nvJson = await store.readResource(sourceDir, namedValue).catch(() => undefined); + const displayName = nvJson + ? (applyOverrides(namedValue, nvJson, overrides).properties as Record | undefined)?.displayName + : undefined; + if (typeof displayName === 'string') { + byDisplayName.set(displayName.toLowerCase(), getNamePart(namedValue.nameParts, 0)); + } + } + return byDisplayName; + }; let match; while ((match = refPattern.exec(credStr)) !== null) { const ref = match[1]; if (rewriteMap.has(ref)) continue; - - // Try to read the named value artifact by this ref as a resource name + // The named value APIM resolves the reference to: the one with that + // display name, otherwise the one with that resource name. + const nvName = (await (displayNames ??= readDisplayNames())).get(ref.toLowerCase()) ?? ref; const nvDescriptor: ResourceDescriptor = { type: ResourceType.NamedValue, - nameParts: [ref], + nameParts: [nvName], ...(descriptor.workspace ? { workspace: descriptor.workspace } : {}), }; let nvJson = await store.readResource(sourceDir, nvDescriptor); @@ -1711,12 +1733,12 @@ async function rewriteNamedValueReferences( // use the affixed displayName so {{token}} refs resolve on the shared APIM. if (envMapping?.appliesTo.has(ResourceType.NamedValue)) { const hasDisplayNameOverride = hasExplicitPropertyOverride( - ref, + nvName, 'displayName', resolveOverrideSection(overrides, 'namedValues', descriptor.workspace), ); if (!hasDisplayNameOverride) { - const baseDisplayName = displayName ?? ref; + const baseDisplayName = displayName ?? nvName; displayName = toDeployedName(baseDisplayName, ResourceType.NamedValue, envMapping); } } diff --git a/tests/unit/services/resource-publisher.test.ts b/tests/unit/services/resource-publisher.test.ts index 923dfcf2..aab77ef8 100644 --- a/tests/unit/services/resource-publisher.test.ts +++ b/tests/unit/services/resource-publisher.test.ts @@ -2177,6 +2177,35 @@ describe('resource-publisher', () => { expect(creds.instrumentationKey).toBe('{{Logger-Credentials--abc123}}'); }); + it("should keep a reference that is another named value's display name pointing at that named value", async () => { + const client = createMockClient(); + const store = createMockStore(); + const namedValue = (name: string): ResourceDescriptor => ({ type: ResourceType.NamedValue, nameParts: [name] }); + store.listResources.mockResolvedValue([namedValue('one'), namedValue('two')]); + store.readResource.mockImplementation(async (_dir: string, desc: ResourceDescriptor) => { + if (desc.type === ResourceType.Logger) { + return { name: 'my-logger', properties: { loggerType: 'applicationInsights', credentials: { instrumentationKey: '{{two}}' } } }; + } + if (desc.type === ResourceType.NamedValue) { + return { name: desc.nameParts[0], properties: { displayName: desc.nameParts[0] === 'one' ? 'two' : 'three' } }; + } + return null; + }); + + const loggerDescriptor: ResourceDescriptor = { type: ResourceType.Logger, nameParts: ['my-logger'] }; + const sentKey = () => + (client.putResource.mock.calls.at(-1)?.[2] as { properties: { credentials: Record } }) + .properties.credentials.instrumentationKey; + + await publishResource(client, store, testContext, loggerDescriptor, testConfig); + expect(sentKey()).toBe('{{two}}'); + + // With a prefix, the reference follows that named value's deployed display name. + const envMapping = { prefix: 'dev-', suffix: '', appliesTo: new Set([ResourceType.NamedValue]) }; + await publishResource(client, store, testContext, loggerDescriptor, { ...testConfig, envMapping }); + expect(sentKey()).toBe('{{dev-two}}'); + }); + it('should keep an explicit workspace named-value displayName unprefixed in logger credentials', async () => { const client = createMockClient(); const store = createMockStore(); From b6e4b17008530c41287888accb3705f29683fc0a Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:52:28 +1100 Subject: [PATCH 22/23] fix: follow the overridden XML of policies and fragments on publish Overrides can replace a policy's or fragment's XML, but publish read the source XML to find what policies reference and which named values they use as resource names. An override adding backend-id="{{route}}" was missed, so the backend was not pulled in, and an incremental publish changing only route did not bring the backend it now names. Both now read the XML as published, with this environment's overrides applied. --- src/services/publish-service.ts | 6 ++- src/services/transitive-resolver.ts | 37 +++++++++++++--- tests/unit/services/publish-service.test.ts | 48 +++++++++++++++++++++ 3 files changed, 83 insertions(+), 8 deletions(-) diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 62f22c4e..0f2a2f3b 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -351,7 +351,9 @@ function createNamedValueTargetIndex(store: IArtifactStore, config: PublishConfi config.sourceDir, available, available, - createArtifactReferenceResolver(available, (descriptor) => store.readResource(config.sourceDir, descriptor)) + createArtifactReferenceResolver(available, (descriptor) => store.readResource(config.sourceDir, descriptor)), + // As published: an override can replace a policy's or fragment's XML. + (descriptor, json) => applyOverrides(descriptor, json, config.overrides) ) ); return index; @@ -456,7 +458,7 @@ async function expandTransitivePublishTargets( let references: ResourceDescriptor[]; try { // As published: an override can change what a backend's or logger's - // credentials or a diagnostic's logger reference. + // credentials, a diagnostic's logger or a policy's XML reference. references = await scanArtifactReferences( store, sourceDir, scanQueue[index], readNamedValue, (descriptor, json) => applyOverrides(descriptor, json, overrides) diff --git a/src/services/transitive-resolver.ts b/src/services/transitive-resolver.ts index d6ac4b54..4e98306a 100644 --- a/src/services/transitive-resolver.ts +++ b/src/services/transitive-resolver.ts @@ -434,6 +434,24 @@ export function namedValueTargetName(json: Record): string | un return properties?.secret === true || value.startsWith('@') || value.includes('{{') ? undefined : value || undefined; } +/** + * Turns an artifact's JSON into the payload that is scanned, such as the one + * publish sends with overrides applied. + */ +export type PrepareJson = (descriptor: ResourceDescriptor, json: Record) => Record; + +/** + * A policy's XML as `prepareJson` leaves it: the payload publish sends is + * `{ properties: { value, format: 'rawxml' } }`, which overrides can change. + */ +function preparePolicyContent(descriptor: ResourceDescriptor, content: string, prepareJson?: PrepareJson): unknown { + if (!prepareJson) { + return content; + } + const payload = prepareJson(descriptor, { properties: { value: content, format: 'rawxml' } }); + return (payload.properties as Record | undefined)?.value; +} + /** * The resource types each named value is the whole value of a reference * attribute for (backend-id="{{name}}"), keyed by the named value's @@ -444,13 +462,16 @@ export function namedValueTargetName(json: Record): string | un * needs the resource its value now names. * * @param resolve - Resolves a `{{reference}}` to the named value artifact + * @param prepareJson - Turns each policy and fragment into what is scanned, + * such as the payload publish sends with overrides applied */ export async function indexNamedValueTargetTypes( store: Pick, sourceDir: string, selected: ReadonlyArray, available: ReadonlyArray, - resolve: (reference: ResourceDescriptor) => Promise + resolve: (reference: ResourceDescriptor) => Promise, + prepareJson?: PrepareJson ): Promise>> { const isPolicy = (descriptor: ResourceDescriptor) => POLICY_RESOURCE_TYPES.has(descriptor.type) || descriptor.type === ResourceType.PolicyFragment; @@ -468,9 +489,12 @@ export async function indexNamedValueTargetTypes( const results = await runParallel( policies.map((descriptor) => async (): Promise => { if (POLICY_RESOURCE_TYPES.has(descriptor.type)) { - return [(await store.readContent(sourceDir, descriptor, 'policy'))?.content]; + const content = (await store.readContent(sourceDir, descriptor, 'policy'))?.content; + return [content === undefined ? undefined : preparePolicyContent(descriptor, content, prepareJson)]; } - const properties = (await store.readResource(sourceDir, descriptor))?.properties as Record | undefined; + const json = await store.readResource(sourceDir, descriptor); + const properties = (json && prepareJson ? prepareJson(descriptor, json) : json)?.properties as + Record | undefined; return [properties?.value, properties?.policyContent]; }), REFERENCE_SCAN_CONCURRENCY @@ -588,7 +612,7 @@ export async function scanArtifactReferences( sourceDir: string, descriptor: ResourceDescriptor, readNamedValue?: NamedValueReader, - prepareJson?: (descriptor: ResourceDescriptor, json: Record) => Record + prepareJson?: PrepareJson ): Promise { if (!REFERENCING_TYPES.has(descriptor.type)) { return []; @@ -599,8 +623,9 @@ export async function scanArtifactReferences( if (POLICY_RESOURCE_TYPES.has(descriptor.type)) { const content = await store.readContent(sourceDir, descriptor, 'policy'); - if (content) { - policies.set(descriptor.nameParts.join('/'), content.content); + const text = content ? preparePolicyContent(descriptor, content.content, prepareJson) : undefined; + if (typeof text === 'string') { + policies.set(descriptor.nameParts.join('/'), text); } } diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 36bcd377..8d0adf5c 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -624,6 +624,54 @@ describe('publish-service', () => { expect(puts).toEqual(['Backend:new-backend', 'NamedValue:backend-id']); }); + describe('with a policy whose XML an override replaces', () => { + const servicePolicy: ResourceDescriptor = { type: ResourceType.ServicePolicy, nameParts: [] }; + const namedValue: ResourceDescriptor = { type: ResourceType.NamedValue, nameParts: ['backend-id'] }; + const overrides = { + policies: { policy: { properties: { value: '' } } }, + }; + const publishWith = async (filter: FilterConfig, commitId?: string) => { + const client = createMockClient(); + const store = createMockStore([servicePolicy, namedValue, { type: ResourceType.Backend, nameParts: ['new-backend'] }]); + store.readContent.mockImplementation(async (_sourceDir: string, descriptor: ResourceDescriptor) => + descriptor.type === ResourceType.ServicePolicy ? { content: '' } : undefined + ); + store.readResource.mockImplementation(async (_sourceDir, descriptor) => + descriptor.type === ResourceType.NamedValue + ? { properties: { displayName: 'Backend_Id', value: 'new-backend' } } + : { properties: {} } + ); + await runPublish(client, store, { + service: testContext, + sourceDir: '/source', + filter, + overrides, + commitId, + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }); + return client.putResource.mock.calls.map((call) => `${call[1].type}:${call[1].nameParts.join('/')}`).sort(); + }; + + it('should follow the overridden XML of a published policy', async () => { + expect(await publishWith({ namedValues: [], backends: [] })).toEqual([ + 'Backend:new-backend', + 'NamedValue:backend-id', + 'ServicePolicy:', + ]); + }); + + it('should include the backend a changed named value names in the overridden XML', async () => { + vi.mocked(computeGitDiff).mockResolvedValue({ changedDescriptors: [namedValue], deletedDescriptors: [] }); + + expect(await publishWith({ namedValues: ['backend-id'], backends: [] }, 'abc123')).toEqual([ + 'Backend:new-backend', + 'NamedValue:backend-id', + ]); + }); + }); + it('should include the backend a changed named value names in a fragment a selected policy includes', async () => { const namedValue = { type: ResourceType.NamedValue, nameParts: ['backend-id'] }; const resources = [ From 7869b61330f13d2b20ab3781b8037ca765031e5e Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:54:29 +1100 Subject: [PATCH 23/23] fix: order named values and credential users by what each needs When a backend using one late named value in its credentials was itself what another late named value names, that named value was published first, before the backend it names, even when an order existed: for route-a naming new-a, route-b naming new-b and new-a using {{route-b}}, new-b, route-b, new-a, route-a. The late named values, the backends and loggers using them in their credentials, and the pools those are members of are now published in waves where each comes after what it needs. Only the named values in a real cycle go first. --- docs/reference/dependency-graph.md | 2 +- src/services/publish-service.ts | 173 ++++++++++++-------- tests/unit/services/publish-service.test.ts | 33 ++++ 3 files changed, 142 insertions(+), 66 deletions(-) diff --git a/docs/reference/dependency-graph.md b/docs/reference/dependency-graph.md index 162d185d..73186517 100644 --- a/docs/reference/dependency-graph.md +++ b/docs/reference/dependency-graph.md @@ -15,7 +15,7 @@ apiops-cli solves this by organizing all 34 resource types into **4 tiers**. Eac | Tier | Resources | Rule | |------|-----------|------| -| **1** | NamedValue, Tag, Gateway, VersionSet, Backend, Logger, Group, PolicyFragment, GlobalSchema, PolicyRestriction, Documentation | No dependencies on other tiers. Within the tier, named values go first, since backends and loggers can use them; pool backends follow the backends they contain; and a named value a policy uses to name a backend, logger or schema published in the same run goes after that resource, with policy fragments after it in that case. A backend or logger in the run that uses such a named value in its credentials goes after the named value; when it also has to go before it (it is what the named value names, or a member of a pool in the run), the named value goes first | +| **1** | NamedValue, Tag, Gateway, VersionSet, Backend, Logger, Group, PolicyFragment, GlobalSchema, PolicyRestriction, Documentation | No dependencies on other tiers. Within the tier, named values go first, since backends and loggers can use them; pool backends follow the backends they contain; and a named value a policy uses to name a backend, logger or schema published in the same run goes after that resource, with policy fragments after it in that case. A backend or logger in the run that uses such a named value in its credentials goes after the named value, and these are ordered so each comes after what it needs; only where that is impossible (a cycle, as when a backend uses in its credentials the named value that names it, or a pool containing it), the named values in it go first | | **2** | Diagnostic, ServicePolicy, Product, Api | Depend only on Tier 1 resources | | **3** | ProductPolicy, ProductGroup, ProductTag, ProductApi, ProductWiki, ApiPolicy, ApiTag, ApiDiagnostic, ApiOperation, ApiSchema, ApiRelease, ApiTagDescription, ApiWiki, McpServer, GraphQLResolver, GatewayApi, Subscription | Depend on Tier 1 and/or Tier 2 resources | | **4** | ApiOperationPolicy, GraphQLResolverPolicy | Depend on Tier 3 resources | diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 0f2a2f3b..0f7847ff 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -604,7 +604,7 @@ async function executePuts( // before any resource that references them. // // Wave 2 — The remaining Tier 1 resources except pool backends and - // wave 5 (and except policy fragments when wave 4 has named values). + // wave 4 (and except policy fragments when wave 4 has any). // // Wave 3 — Pool backends. // Pool backends reference individual Backend resources and must be @@ -613,16 +613,17 @@ async function executePuts( // // Wave 4 — Named values a policy uses as a resource name // (backend-id="{{name}}") whose value names a backend, logger or - // schema published here. APIM checks the policies that use a named - // value when it changes, so that resource must exist first (see + // schema published here, after that resource: APIM checks the + // policies that use a named value when it changes, so that resource + // must exist first. With them, in an order where each goes after what + // it needs, the backends and loggers whose credentials use them and + // the pools those are members of (see // splitNamedValuesNamingResources). // - // Wave 5 — Backends and loggers whose credentials use a wave 4 named - // value, when they don't have to go before one. - // - // Wave 6 — Policy fragments, when wave 4 has named values: a fragment - // can use them, and APIM rejects a policy that uses a missing one. - const { early: earlyNamedValues, late: lateNamedValues, after: afterLateNamedValues } = await splitNamedValuesNamingResources( + // Wave 5 — Policy fragments, when wave 4 has resources: a fragment can + // use its named values, and APIM rejects a policy that uses a + // missing one. + const { early: earlyNamedValues, ordered } = await splitNamedValuesNamingResources( store, config, namedValues, @@ -634,12 +635,13 @@ async function executePuts( await publishAndOutput(client, store, context, config, earlyNamedValues, targetDescriptors, results); } + const inOrdered = new Set(ordered.flat()); const { poolBackends, regularTier1 } = await splitPoolBackends( store, config.sourceDir, - otherTier1.filter((descriptor) => !afterLateNamedValues.includes(descriptor)) + otherTier1.filter((descriptor) => !inOrdered.has(descriptor)) ); - const fragments = lateNamedValues.length > 0 + const fragments = inOrdered.size > 0 ? regularTier1.filter((descriptor) => descriptor.type === ResourceType.PolicyFragment) : []; await publishAndOutput( @@ -665,14 +667,13 @@ async function executePuts( results ); } - if (lateNamedValues.length > 0) { + if (inOrdered.size > 0) { logger.debug( - `Publishing ${lateNamedValues.length} named value(s) after the resources their values name` + `Publishing ${inOrdered.size} named value(s) and resource(s) in the order the named values' values need` ); - await publishAndOutput(client, store, context, config, lateNamedValues, targetDescriptors, results); } - if (afterLateNamedValues.length > 0) { - await publishAndOutput(client, store, context, config, afterLateNamedValues, targetDescriptors, results); + for (const wave of ordered) { + await publishAndOutput(client, store, context, config, wave, targetDescriptors, results); } if (fragments.length > 0) { await publishAndOutput(client, store, context, config, fragments, targetDescriptors, results); @@ -889,17 +890,23 @@ function splitNamedValues( } /** - * Splits named values into those published before the rest of Tier 1 and - * those published after the backends, loggers and schemas: a named value that - * a policy or policy fragment in the source uses as the whole value of a - * reference attribute (backend-id="{{name}}", logger-id, schema-id), and whose - * value (with overrides) names a resource of that type published in this - * tier. APIM checks the policies that use a named value when the named value - * changes, rejecting the change ("its value breaks policies") while the - * resource it now names doesn't exist. A backend or logger published here - * that uses one of those in its credentials is published after it (`after`); - * when that resource has to go before a later named value too (it is what one - * names, or a member of a pool here), the named value stays first instead. + * The Tier 1 order around named values a policy or policy fragment in the + * source uses as the whole value of a reference attribute + * (backend-id="{{name}}", logger-id, schema-id), and whose value (with + * overrides) names a resource of that type published in this tier. APIM + * checks the policies that use a named value when the named value changes, + * rejecting the change ("its value breaks policies") while the resource it + * now names doesn't exist, so such a named value goes after that resource. + * + * `early` named values go before the rest of Tier 1. `ordered` is published + * in turn after it, wave by wave: the late named values, the backends and + * loggers whose credentials use them (after them), the resources those name + * or pools those are members of, in an order where each comes after what it + * needs. Where there is none (a cycle, as when a backend uses in its + * credentials the named value that names it), the named values in it go + * early instead. `ordered` resources other than named values are not + * published in the earlier waves. + * * The policies are read (once) only when a named value's value matches the * name of such a resource here; a named value that can't be read goes first, * and publishing it reports the error. @@ -910,21 +917,22 @@ async function splitNamedValuesNamingResources( namedValues: ResourceDescriptor[], otherTier1: ResourceDescriptor[], namedValueTargets: NamedValueTargetIndex -): Promise<{ early: ResourceDescriptor[]; late: ResourceDescriptor[]; after: ResourceDescriptor[] }> { +): Promise<{ early: ResourceDescriptor[]; ordered: ResourceDescriptor[][] }> { const resourceKey = (type: ResourceType, name: string, workspace: string | undefined) => `${type}/${workspace ?? ''}/${name.toLowerCase()}`; + const keyOf = (descriptor: ResourceDescriptor) => + resourceKey(descriptor.type, getNamePart(descriptor.nameParts, 0), descriptor.workspace); + const byResourceKey = new Map(otherTier1.map((descriptor) => [keyOf(descriptor), descriptor])); const named = new Set( - otherTier1 - .filter((descriptor) => NAMED_THROUGH_NAMED_VALUE_TYPES.has(descriptor.type)) - .map((descriptor) => resourceKey(descriptor.type, getNamePart(descriptor.nameParts, 0), descriptor.workspace)) + otherTier1.filter((descriptor) => NAMED_THROUGH_NAMED_VALUE_TYPES.has(descriptor.type)).map(keyOf) ); const early: ResourceDescriptor[] = []; const late: ResourceDescriptor[] = []; if (named.size === 0) { - return { early: namedValues, late, after: [] }; + return { early: namedValues, ordered: [] }; } - // The resources the late named values name, which go before them. - const lateTargets = new Set(); + // The resources each late named value names, which go before it. + const targetsOf = new Map(); // The named values policies use as resource names, and as which types. // Every policy in the source counts, not only the ones the filter selects: // APIM checks every policy on the target that uses the named value, and @@ -932,35 +940,36 @@ async function splitNamedValuesNamingResources( const typesFor = async (namedValue: ResourceDescriptor): Promise | undefined> => (await namedValueTargets()).get(getResourceDescriptorKey(namedValue)); for (const descriptor of namedValues) { - let isLate = false; + let targets: string[] = []; try { const json = await store.readResource(config.sourceDir, descriptor); const name = json ? namedValueTargetName(applyOverrides(descriptor, json, config.overrides)) : undefined; if (name && [...NAMED_THROUGH_NAMED_VALUE_TYPES].some((type) => named.has(resourceKey(type, name, descriptor.workspace)))) { - const targets = [...(await typesFor(descriptor) ?? [])] + targets = [...(await typesFor(descriptor) ?? [])] .map((type) => resourceKey(type, name, descriptor.workspace)) .filter((target) => named.has(target)); - targets.forEach((target) => lateTargets.add(target)); - isLate = targets.length > 0; } } catch { - isLate = false; + targets = []; + } + if (targets.length > 0) { + late.push(descriptor); + targetsOf.set(descriptor, targets); + } else { + early.push(descriptor); } - (isLate ? late : early).push(descriptor); } if (late.length === 0) { - return { early, late, after: [] }; + return { early, ordered: [] }; } - // A backend or logger published here whose own configuration (its - // credentials) uses one of those named values needs it first: it is - // published after the late named values, unless it has to go before them, - // in which case the named value it uses stays early. + // The backends and loggers whose own configuration (their credentials) + // uses a late named value, and the pools each backend is a member of. const available = await store.listResources(config.sourceDir); const resolve = createArtifactReferenceResolver(available, (descriptor) => store.readResource(config.sourceDir, descriptor)); - const lateKeys = new Set(late.map(getResourceDescriptorKey)); - const users: Array<{ descriptor: ResourceDescriptor; uses: string[] }> = []; - const poolMembers = new Set(); + const lateByKey = new Map(late.map((descriptor) => [getResourceDescriptorKey(descriptor), descriptor])); + const usesOf = new Map(); + const poolsOf = new Map(); for (const descriptor of otherTier1) { if (descriptor.type !== ResourceType.Backend && descriptor.type !== ResourceType.Logger) { continue; @@ -975,37 +984,71 @@ async function splitNamedValuesNamingResources( continue; } const resource = { descriptor, json: applyOverrides(descriptor, json, config.overrides) }; - const uses: string[] = []; + const uses: ResourceDescriptor[] = []; for (const dependency of findTransitiveDependencies(new Map(), new Map(), descriptor.workspace, [resource])) { const target = dependency.type === ResourceType.NamedValue ? await resolve(dependency) : undefined; - if (target && lateKeys.has(getResourceDescriptorKey(target))) { - uses.push(getResourceDescriptorKey(target)); + const used = target ? lateByKey.get(getResourceDescriptorKey(target)) : undefined; + if (used) { + uses.push(used); } } if (uses.length > 0) { - users.push({ descriptor, uses }); + usesOf.set(descriptor, uses); } const pool = (resource.json.properties as Record | undefined)?.pool as Record | undefined; for (const service of Array.isArray(pool?.services) ? pool.services : []) { const id = (service as Record | undefined)?.id; const member = typeof id === 'string' ? armNameAfter(id, 'backends') : undefined; if (member) { - poolMembers.add(resourceKey(ResourceType.Backend, member, descriptor.workspace)); + const memberKey = resourceKey(ResourceType.Backend, member, descriptor.workspace); + poolsOf.set(memberKey, [...(poolsOf.get(memberKey) ?? []), descriptor]); } } } - const mustGoFirst = users.filter(({ descriptor }) => { - const key = resourceKey(descriptor.type, getNamePart(descriptor.nameParts, 0), descriptor.workspace); - return lateTargets.has(key) || poolMembers.has(key); - }); - const neededFirst = new Set(mustGoFirst.flatMap(({ uses }) => uses)); - return { - early: [...early, ...late.filter((descriptor) => neededFirst.has(getResourceDescriptorKey(descriptor)))], - late: late.filter((descriptor) => !neededFirst.has(getResourceDescriptorKey(descriptor))), - after: users - .filter((user) => !mustGoFirst.includes(user) && user.uses.some((use) => !neededFirst.has(use))) - .map(({ descriptor }) => descriptor), - }; + + // What is ordered: the late named values, their credential users, and the + // pools those users are members of (a pool goes after its members). + const nodes = new Set([...late, ...usesOf.keys()]); + for (const node of [...nodes]) { + if (node.type === ResourceType.Backend) { + (poolsOf.get(keyOf(node)) ?? []).forEach((pool) => nodes.add(pool)); + } + } + // Each node's predecessors among the nodes. + const before = new Map>([...nodes].map((node) => [node, new Set()])); + for (const [user, uses] of usesOf) { + uses.forEach((used) => before.get(user)?.add(used)); + } + for (const [namedValue, targets] of targetsOf) { + for (const target of targets) { + const resource = byResourceKey.get(target); + if (resource && nodes.has(resource)) { + before.get(namedValue)?.add(resource); + } + } + } + for (const node of nodes) { + if (node.type === ResourceType.Backend) { + (poolsOf.get(keyOf(node)) ?? []).forEach((pool) => before.get(pool)?.add(node)); + } + } + + const ordered: ResourceDescriptor[][] = []; + const remaining = new Set(nodes); + while (remaining.size > 0) { + const wave = [...remaining].filter((node) => [...(before.get(node) ?? [])].every((needed) => !remaining.has(needed))); + if (wave.length === 0) { + // A cycle: its named values go early, before what they name. + for (const node of [...remaining].filter((candidate) => candidate.type === ResourceType.NamedValue)) { + remaining.delete(node); + early.push(node); + } + continue; + } + wave.forEach((node) => remaining.delete(node)); + ordered.push(wave); + } + return { early, ordered }; } /** Tier 1 types a policy attribute can name through a named value. */ diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 8d0adf5c..cc441d6b 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -2261,6 +2261,39 @@ describe('publish-service', () => { expect(order.indexOf('frag')).toBeGreaterThan(order.indexOf('route')); }); + it('should order a chain of named values and the backends using them in their credentials', async () => { + const { order, client, store } = recordOrder([ + { type: ResourceType.NamedValue, nameParts: ['route-a'] }, + { type: ResourceType.NamedValue, nameParts: ['route-b'] }, + { type: ResourceType.Backend, nameParts: ['new-a'] }, + { type: ResourceType.Backend, nameParts: ['new-b'] }, + { type: ResourceType.PolicyFragment, nameParts: ['frag'] }, + ]); + vi.mocked(store.readResource).mockImplementation(async (_sourceDir, descriptor) => { + if (descriptor.type === ResourceType.NamedValue) { + const name = descriptor.nameParts[0]; + return { properties: { displayName: name, value: name === 'route-a' ? 'new-a' : 'new-b' } }; + } + if (descriptor.type === ResourceType.PolicyFragment) { + return { + properties: { + value: '', + }, + }; + } + return descriptor.nameParts[0] === 'new-a' + ? { properties: { url: 'https://example.test', credentials: { header: { 'x-route': ['{{route-b}}'] } } } } + : { properties: { url: 'https://example.test' } }; + }); + + await publishAll(client, store); + + expect(order.indexOf('route-b')).toBeGreaterThan(order.indexOf('new-b')); + expect(order.indexOf('new-a')).toBeGreaterThan(order.indexOf('route-b')); + expect(order.indexOf('route-a')).toBeGreaterThan(order.indexOf('new-a')); + expect(order.indexOf('frag')).toBeGreaterThan(order.indexOf('route-a')); + }); + it('should keep a named value first when a pool member uses it in its credentials', async () => { const { order, client, store } = recordOrder([ { type: ResourceType.NamedValue, nameParts: ['route'] },