From 4128c85623406e2c602b53f422c888f2be08fcfd Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Fri, 6 Nov 2026 17:27:22 +1100 Subject: [PATCH 1/6] fix: leave APIM's built-in master subscription alone on extract, publish and delete APIM's built-in all-access subscription, master, is scoped to the service root. APIM manages it: a subscription's scope has to be an API or a product, so it can't be created or updated through the API, and once deleted it can't be recreated. - --delete-unmatched deleted it whenever the artifacts didn't contain it. SYSTEM_RESOURCES listed 'master' as a product ("Master product"), but isSystemResource only checks that list for products and APIs, so the subscription was never protected; APIM accepts the DELETE. Confirmed on a StandardV2 instance, where the deleted master couldn't be put back. An incremental publish with --delete-unmatched did the same when its artifact was removed from the source. - Extract wrote it, and every publish then warned "Skipping subscription "master": root-scoped subscriptions are managed by APIM", although the docs already said publish skips it. isBuiltInSubscription now names it, and extract leaves it out, publish leaves it out when choosing what to publish (quietly, so no warning on every run), and full and incremental deletes, real and dry-run, never delete it. Publish's separate root-scope check is gone: master is the only root-scoped subscription APIM has. There is no master product, so a product named master is now deleted by --delete-unmatched like any other. --- docs/guides/environment-overrides.md | 5 ++- src/models/resource-types.ts | 16 ++++++++++ src/services/delete-unmatched-service.ts | 8 +++-- src/services/dry-run-reporter.ts | 5 +-- src/services/publish-service.ts | 10 ++++-- src/services/resource-extractor.ts | 6 +++- src/services/resource-publisher.ts | 14 ++------- tests/unit/models/resource-types.test.ts | 12 ++++++- .../services/delete-unmatched-service.test.ts | 15 +++++++-- tests/unit/services/dry-run-reporter.test.ts | 13 ++------ tests/unit/services/publish-service.test.ts | 26 +++++++++++++++- .../unit/services/resource-extractor.test.ts | 11 +++++++ .../unit/services/resource-publisher.test.ts | 31 ------------------- 13 files changed, 106 insertions(+), 66 deletions(-) diff --git a/docs/guides/environment-overrides.md b/docs/guides/environment-overrides.md index b6be22fa..77f8bc1a 100644 --- a/docs/guides/environment-overrides.md +++ b/docs/guides/environment-overrides.md @@ -307,7 +307,10 @@ subscriptions: scope: "/apis/petstore-api" ``` -> **Note:** The built-in `master` subscription is automatically skipped during publish. +> **Note:** APIM's built-in `master` subscription (the all-access one, scoped to the whole instance) is left alone: +> extract doesn't write it, publish doesn't publish it, and neither `--delete-unmatched` nor an incremental publish +> deletes it, since APIM can't recreate it. An artifact for it from an earlier extract is ignored, and +> `--remove-stale` removes it when subscriptions are in its scope. > When publish creates a new product, the subscriptions APIM automatically creates for it are deleted, > so only the extracted product subscriptions (including ones with 24-character hex IDs) are published. > Deletion is best-effort: a failed delete is logged as a warning and does not fail the publish. diff --git a/src/models/resource-types.ts b/src/models/resource-types.ts index 6686edb1..215f38bf 100644 --- a/src/models/resource-types.ts +++ b/src/models/resource-types.ts @@ -97,6 +97,22 @@ export interface ResourceTypeMetadata { readonly workspaceLinkIdProperty?: string; } +/** + * Whether a descriptor is APIM's built-in all-access subscription: `master`, + * scoped to the service root. APIM creates it and manages it: it can't be + * created or updated through the API (a subscription's scope has to be an API + * or a product), and one that is deleted can't be recreated. Extract, publish + * and deletes leave it alone. + */ +export function isBuiltInSubscription(descriptor: { type: ResourceType; nameParts: string[]; workspace?: string }): boolean { + return ( + descriptor.type === ResourceType.Subscription && + !descriptor.workspace && + descriptor.nameParts.length === 1 && + descriptor.nameParts[0]?.toLowerCase() === 'master' + ); +} + export const RESOURCE_TYPE_METADATA: Record = { [ResourceType.NamedValue]: { armPathSuffix: 'namedValues/{0}', diff --git a/src/services/delete-unmatched-service.ts b/src/services/delete-unmatched-service.ts index bbb77691..2ae2adac 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 { isBuiltInSubscription, ResourceType } from '../models/resource-types.js'; import { getTopologicalOrder } from '../lib/dependency-graph.js'; import { getNameFromNameParts, @@ -86,7 +86,6 @@ const BUILT_IN_GROUPS = ['administrators', 'developers', 'guests']; * System resources that should never be deleted */ const SYSTEM_RESOURCES = new Set([ - 'master', // Master product 'unlimited', // Unlimited product 'starter', // Starter product 'echo-api', // Echo API (system test API) @@ -398,6 +397,11 @@ function isSystemResource(descriptor: ResourceDescriptor): boolean { } } + // APIM's built-in all-access subscription can't be recreated once deleted. + if (isBuiltInSubscription(descriptor)) { + return true; + } + // Check system products and APIs if ( descriptor.type === ResourceType.Product || diff --git a/src/services/dry-run-reporter.ts b/src/services/dry-run-reporter.ts index 4431e810..d98ea238 100644 --- a/src/services/dry-run-reporter.ts +++ b/src/services/dry-run-reporter.ts @@ -10,7 +10,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, RESOURCE_TYPE_METADATA } from '../models/resource-types.js'; +import { isBuiltInSubscription, ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.js'; import { getTopologicalOrder } from '../lib/dependency-graph.js'; import { buildResourceLabel } from '../lib/resource-uri.js'; import { getResourceDescriptorKey } from '../lib/resource-path.js'; @@ -162,7 +162,8 @@ export async function generateDryRunReport( // Apply the same ;rev=N filtering as the real delete path so the preview // matches what publish would actually delete. if (incrementalDeletedDescriptors.length > 0) { - for (const descriptor of filterRevisionDeletesHandledByBaseApi(incrementalDeletedDescriptors)) { + for (const descriptor of filterRevisionDeletesHandledByBaseApi(incrementalDeletedDescriptors) + .filter((deleted) => !isBuiltInSubscription(deleted))) { try { const deployedDescriptor = config.envMapping ? mapDescriptor(descriptor, config.envMapping) diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 6d1e966a..157a4c52 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -10,7 +10,7 @@ import { IApimClient } from '../clients/iapim-client.js'; import { IArtifactStore } from '../clients/iartifact-store.js'; import { FilterConfig, PublishConfig, OverrideConfig } from '../models/config.js'; import { ApimServiceContext, ResourceDescriptor } from '../models/types.js'; -import { ResourceType } from '../models/resource-types.js'; +import { isBuiltInSubscription, ResourceType } from '../models/resource-types.js'; import { getResourceTier, TIER_LABELS } from '../lib/dependency-graph.js'; import { runParallel } from '../lib/parallel-runner.js'; import { logger } from '../lib/logger.js'; @@ -299,6 +299,8 @@ async function determinePublishTargets( deletedDescriptors = []; } + // APIM's built-in `master` subscription can't be published (isBuiltInSubscription). + targetDescriptors = targetDescriptors.filter((descriptor) => !isBuiltInSubscription(descriptor)); targetDescriptors = filterPublishDescriptors(targetDescriptors, config.filter); deletedDescriptors = filterPublishDescriptors(deletedDescriptors, config.filter); @@ -988,7 +990,11 @@ async function executeDeletesForDescriptors( // removes all of its revisions in one call. Drop individual ;rev=N deletes // whose base API is also in the delete set to avoid redundant/racy deletes and // the "Cannot delete the current revision of an API" error. - const effectiveDescriptors = filterRevisionDeletesHandledByBaseApi(deleteDescriptors); + // APIM's built-in `master` subscription is never deleted: one that is can't + // be recreated (isBuiltInSubscription). An incremental publish lists it when + // its artifact is removed from the source. + const effectiveDescriptors = filterRevisionDeletesHandledByBaseApi(deleteDescriptors) + .filter((descriptor) => !isBuiltInSubscription(descriptor)); // Group by tier const tierGroups = new Map(); diff --git a/src/services/resource-extractor.ts b/src/services/resource-extractor.ts index c62c8c6a..24b3d70a 100644 --- a/src/services/resource-extractor.ts +++ b/src/services/resource-extractor.ts @@ -9,7 +9,7 @@ import { IApimClient } from '../clients/iapim-client.js'; import { IArtifactStore } from '../clients/iartifact-store.js'; import { ApimServiceContext, ResourceDescriptor } from '../models/types.js'; -import { ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.js'; +import { isBuiltInSubscription, ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.js'; import { redactSecrets } from './secret-redactor.js'; import { shouldIncludeResource } from './filter-service.js'; import { FilterConfig } from '../models/config.js'; @@ -104,6 +104,10 @@ export async function extractResourceType( logger.debug(`Filtered out ${buildResourceLabel(descriptor)}`); continue; } + if (isBuiltInSubscription(descriptor)) { + logger.debug(`Skipping ${buildResourceLabel(descriptor)}: APIM manages the built-in subscription`); + continue; + } // Some APIM list endpoints return a shallow response that omits the // heavyweight payload we need for round-trip publish. ApiSchema list diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts index fe9f3729..055bb240 100644 --- a/src/services/resource-publisher.ts +++ b/src/services/resource-publisher.ts @@ -417,9 +417,8 @@ export async function publishResource( // but the PUT endpoint requires a relative APIM path (e.g. "/apis" or // "/apis/{apiId}" or "/products/{productId}"). Strip the service base path // prefix when it is present so that the publish round-trip works correctly. - // HOWEVER: Skip publishing subscriptions with root-level scope (e.g. the built-in - // "master" subscription), as APIM treats these as read-only system resources - // and returns ValidationError when attempting to update them. + // The built-in root-scoped `master` subscription never gets here: publish + // leaves it out (isBuiltInSubscription). if (descriptor.type === ResourceType.Subscription) { const subscriptionName = getNamePart(descriptor.nameParts, 0); const eligibility = await evaluateResourceEligibility( @@ -921,15 +920,6 @@ export async function evaluateResourceEligibility( return { eligible: true }; } - const properties = json.properties as Record | undefined; - const scope = properties?.scope as string | undefined; - if ( - scope && - (scope.endsWith('/') || (!scope.includes('/apis') && !scope.includes('/products'))) - ) { - return { eligible: false, reason: 'root-scoped subscriptions are managed by APIM' }; - } - for (const target of findSubscriptionTargets(json, descriptor.workspace)) { const eligibility = await evaluateAssociationEligibility( store, diff --git a/tests/unit/models/resource-types.test.ts b/tests/unit/models/resource-types.test.ts index f28b6698..b928aa3a 100644 --- a/tests/unit/models/resource-types.test.ts +++ b/tests/unit/models/resource-types.test.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. import { describe, it, expect } from 'vitest'; -import { ResourceType, RESOURCE_TYPE_METADATA } from '../../../src/models/resource-types.js'; +import { ResourceType, RESOURCE_TYPE_METADATA, isBuiltInSubscription } from '../../../src/models/resource-types.js'; describe('ResourceType enum', () => { it('should have exactly 35 resource types', () => { @@ -146,3 +146,13 @@ describe('RESOURCE_TYPE_METADATA', () => { } }); }); + +describe('isBuiltInSubscription', () => { + it("should recognise only the service-level master subscription", () => { + expect(isBuiltInSubscription({ type: ResourceType.Subscription, nameParts: ['master'] })).toBe(true); + expect(isBuiltInSubscription({ type: ResourceType.Subscription, nameParts: ['MASTER'] })).toBe(true); + expect(isBuiltInSubscription({ type: ResourceType.Subscription, nameParts: ['master'], workspace: 'team-a' })).toBe(false); + expect(isBuiltInSubscription({ type: ResourceType.Subscription, nameParts: ['orders-sub'] })).toBe(false); + expect(isBuiltInSubscription({ type: ResourceType.Product, nameParts: ['master'] })).toBe(false); + }); +}); diff --git a/tests/unit/services/delete-unmatched-service.test.ts b/tests/unit/services/delete-unmatched-service.test.ts index 615b567e..8dc124e4 100644 --- a/tests/unit/services/delete-unmatched-service.test.ts +++ b/tests/unit/services/delete-unmatched-service.test.ts @@ -130,7 +130,6 @@ describe('delete-unmatched-service', () => { it('should skip system products', async () => { const apimResources = new Map[]>([ [ResourceType.Product, [ - { name: 'master', id: '/products/master' }, { name: 'unlimited', id: '/products/unlimited' }, { name: 'starter', id: '/products/starter' }, { name: 'custom-product', id: '/products/custom-product' }, @@ -143,12 +142,24 @@ describe('delete-unmatched-service', () => { const result = await computeDeleteActions(client, store, testContext, testConfig); const products = result.filter((d) => d.type === ResourceType.Product).map((d) => d.nameParts[0]); - expect(products).not.toContain('master'); expect(products).not.toContain('unlimited'); expect(products).not.toContain('starter'); expect(products).toContain('custom-product'); }); + it("should never delete APIM's built-in master subscription", async () => { + const apimResources = new Map[]>([ + [ResourceType.Subscription, [ + { name: 'master', id: '/subscriptions/master', properties: { scope: '/' } }, + { name: 'old-sub', id: '/subscriptions/old-sub', properties: { scope: '/apis/orders' } }, + ]], + ]); + + const result = await computeDeleteActions(createMockClient(apimResources), createMockStore([]), testContext, testConfig); + + expect(result.filter((d) => d.type === ResourceType.Subscription).map((d) => d.nameParts[0])).toEqual(['old-sub']); + }); + it('should skip echo-api system API', async () => { const apimResources = new Map[]>([ [ResourceType.Api, [ diff --git a/tests/unit/services/dry-run-reporter.test.ts b/tests/unit/services/dry-run-reporter.test.ts index dc530e9e..a78cc608 100644 --- a/tests/unit/services/dry-run-reporter.test.ts +++ b/tests/unit/services/dry-run-reporter.test.ts @@ -750,9 +750,6 @@ describe('dry-run-reporter', () => { ['Subscription:team:workspace-product-sub', { properties: { scope: '/products/team-store' }, }], - ['Subscription::root-sub', { - properties: { scope: armPrefix }, - }], ['Product::store', { properties: {} }], ['Api:team:orders', { properties: {} }], ['Product:team:team-store', { properties: {} }], @@ -766,7 +763,7 @@ describe('dry-run-reporter', () => { const config: PublishConfig = { ...testConfig, filter: { - subscriptions: ['product-sub', 'excluded-api-sub', 'missing-product-sub', 'root-sub'], + subscriptions: ['product-sub', 'excluded-api-sub', 'missing-product-sub'], products: ['store', 'missing'], apis: ['!legacy', '*'], workspaces: ['team'], @@ -799,7 +796,6 @@ describe('dry-run-reporter', () => { nameParts: ['workspace-product-sub'], workspace: 'team', }, - { type: ResourceType.Subscription, nameParts: ['root-sub'] }, ] ); @@ -809,13 +805,8 @@ describe('dry-run-reporter', () => { { operation: 'SKIP', name: 'missing-product-sub' }, { operation: 'PUT', name: 'workspace-api-sub' }, { operation: 'PUT', name: 'workspace-product-sub' }, - { - operation: 'SKIP', - name: 'root-sub', - reason: 'root-scoped subscriptions are managed by APIM', - }, ]); - expect(report.summary).toEqual({ creates: 3, patches: 0, deletes: 0, skips: 3 }); + expect(report.summary).toEqual({ creates: 3, patches: 0, deletes: 0, skips: 2 }); expect(client.putResource).not.toHaveBeenCalled(); expect(client.patchResource).not.toHaveBeenCalled(); expect(client.deleteResource).not.toHaveBeenCalled(); diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 6bcac061..629cccd9 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'; @@ -1138,6 +1138,30 @@ describe('publish-service', () => { expect(computeGitDiff).toHaveBeenCalledWith('/source', 'abc123'); }); + it("should neither publish nor delete APIM's built-in master subscription, without a warning", async () => { + const client = createMockClient(); + const master = { type: ResourceType.Subscription, nameParts: ['master'] }; + const store = createMockStore([master, { type: ResourceType.Subscription, nameParts: ['orders-sub'] }]); + const warn = vi.spyOn(logger, 'warn'); + const base: PublishConfig = { + service: testContext, + sourceDir: '/source', + dryRun: false, + deleteUnmatched: false, + logLevel: LogLevel.INFO, + }; + + await runPublish(client, store, base); + expect(client.putResource.mock.calls.map(([, descriptor]) => descriptor.nameParts[0])).toEqual(['orders-sub']); + expect(warn.mock.calls.map(([message]) => String(message)).filter((message) => message.includes('master'))).toEqual([]); + + // An incremental publish lists it as deleted when its artifact is removed. + vi.mocked(computeGitDiff).mockResolvedValue({ changedDescriptors: [], deletedDescriptors: [master] }); + await runPublish(client, store, { ...base, deleteUnmatched: true, commitId: 'abc123' }); + expect(client.deleteResource).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + it('should delete descriptors removed in the commit when explicitly enabled', async () => { const client = createMockClient(); const store = createMockStore([]); diff --git a/tests/unit/services/resource-extractor.test.ts b/tests/unit/services/resource-extractor.test.ts index 9c828123..3611f808 100644 --- a/tests/unit/services/resource-extractor.test.ts +++ b/tests/unit/services/resource-extractor.test.ts @@ -107,6 +107,17 @@ describe('resource-extractor', () => { expect(result.extracted[0]?.descriptor.nameParts[0]).toBe('nv-1'); }); + it("should leave out APIM's built-in master subscription", async () => { + const client = createMockClient([ + { name: 'master', properties: { scope: '/subscriptions/s/resourceGroups/rg/providers/Microsoft.ApiManagement/service/apim-1' } }, + { name: 'orders-sub', properties: { scope: '/apis/orders' } }, + ]); + + const result = await extractResourceType(client, createMockStore(), testContext, ResourceType.Subscription, '/output'); + + expect(result.extracted.map((extracted) => extracted.descriptor.nameParts[0])).toEqual(['orders-sub']); + }); + it('should redact secret named values', async () => { const client = createMockClient([ { name: 'secret-nv', properties: { secret: true, value: 'hidden' } }, diff --git a/tests/unit/services/resource-publisher.test.ts b/tests/unit/services/resource-publisher.test.ts index 8e1154e5..81f99339 100644 --- a/tests/unit/services/resource-publisher.test.ts +++ b/tests/unit/services/resource-publisher.test.ts @@ -1366,37 +1366,6 @@ describe('resource-publisher', () => { expect(client.putResource).toHaveBeenCalledOnce(); }); - it('should skip subscription with root scope (master subscription)', async () => { - // The master subscription has scope as the service root, which results in - // "/" after ARM path stripping. This is invalid and the subscription should be skipped. - const client = createMockClient(); - const store = createMockStore(); - - const armScopePrefix = - '/subscriptions/sub-1/resourceGroups/rg-1/providers/Microsoft.ApiManagement/service/apim-1'; - - const subscriptionJson = { - name: 'master', - properties: { - displayName: 'Master subscription', - scope: armScopePrefix, // service root, results in "/" after stripping - state: 'active', - }, - }; - store.readResource.mockResolvedValue(subscriptionJson); - - const descriptor: ResourceDescriptor = { - type: ResourceType.Subscription, - nameParts: ['master'], - }; - - const result = await publishResource(client, store, testContext, descriptor, testConfig); - - expect(result.status).toBe('skipped'); - expect(result.action).toBe('noop'); - expect(client.putResource).not.toHaveBeenCalled(); - }); - it('should publish 24-hex product subscription with empty displayName', async () => { const client = createMockClient(); const store = createMockStore(); From 048aa083194e45fb4f3270be11d05b2fb7cba0fc Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Fri, 6 Nov 2026 17:07:01 +1100 Subject: [PATCH 2/6] fix: delete resources that others being deleted still use once those are gone Publish deletes tier by tier, each tier's deletes in parallel. Resources in one tier can use each other: a backend pool its members, a backend or logger the named value in its credentials, a policy fragment a named value. APIM rejects deleting one that something still uses ("The Api Management Property 'x' is used by the following entities: /loggers/y", "Backend Entity 'x' is referenced in Backend 'pool'"), so emptying an instance with --delete-unmatched took three runs, and an incremental publish (--commit-id) left the resource behind for good, since the next commit doesn't list its deletion again. #142's log shows the same failure for a named value its logger used. A tier's deletes now run in rounds: a delete APIM rejects because another resource still uses it (isResourceInUseError) is tried again once the rest of the round is done, for as long as each round deletes something. That covers every such dependency without modelling it, including resources only the target knows about, whose artifacts a full --delete-unmatched doesn't have. Any other failure isn't retried, and a delete something outside the run still blocks fails after one more try. Failures are logged once, at the end. Refs #142 --- docs/commands/publish.md | 2 + src/clients/apim-client.ts | 16 ++++++ src/services/publish-service.ts | 61 +++++++++++++++------ tests/unit/services/publish-service.test.ts | 58 ++++++++++++++++++++ 4 files changed, 121 insertions(+), 16 deletions(-) diff --git a/docs/commands/publish.md b/docs/commands/publish.md index 8d14dc32..fe86c28e 100644 --- a/docs/commands/publish.md +++ b/docs/commands/publish.md @@ -203,6 +203,8 @@ The output lists each resource and the planned action (create, update, or delete For a full publish, `--delete-unmatched` deletes APIM resources that are **not** present in the source artifacts. With `--commit-id`, it deletes only resources and Product associations removed by the selected commit. +Deletes run from the most dependent resources to the least (tier 4 to tier 1). Within a tier, resources can use each other — a backend pool its members, a backend or logger the named value in its credentials, a policy fragment a named value — and APIM rejects deleting one that something still uses. Such a delete is tried again once the rest of the tier is deleted, so one run removes them all. APIM's built-in resources (the built-in groups, the `unlimited` and `starter` products, `echo-api`, and the `master` subscription) are never deleted. + > **Warning:** Use with caution. Resources created manually in the Azure portal that are not in your artifact directory will be removed. ## Exit codes diff --git a/src/clients/apim-client.ts b/src/clients/apim-client.ts index 06adbf5e..658daae2 100644 --- a/src/clients/apim-client.ts +++ b/src/clients/apim-client.ts @@ -44,6 +44,22 @@ export function isLinkAlreadyExistsError(error: unknown): boolean { return error instanceof HttpError && error.status === 409; } +/** + * Returns true when APIM rejects a DELETE because another resource still uses + * the one being deleted, for example "The Api Management Property 'x' is used + * by the following entities: /backends/y", "Backend Entity 'x' is referenced + * in Backend 'pool'" or "The Policy Fragment 'x' is used by the following + * entities". The message can be the error's own or one of its details, so the + * whole response text is matched. + */ +export function isResourceInUseError(error: unknown): boolean { + return ( + error instanceof HttpError && + error.status === 400 && + /is used by the following entities|is referenced in/i.test(error.message) + ); +} + /** Returns true when APIM reports that an association's referenced API/group is absent. */ export function isAssociationReferenceNotFoundError(error: unknown): boolean { if ( diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 157a4c52..06b94efc 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -7,6 +7,7 @@ */ import { IApimClient } from '../clients/iapim-client.js'; +import { isResourceInUseError } from '../clients/apim-client.js'; import { IArtifactStore } from '../clients/iartifact-store.js'; import { FilterConfig, PublishConfig, OverrideConfig } from '../models/config.js'; import { ApimServiceContext, ResourceDescriptor } from '../models/types.js'; @@ -1070,10 +1071,6 @@ async function deleteTier( status: deleted ? ('success' as const) : ('skipped' as const), }; } catch (error) { - logger.error( - `Failed to delete ${buildResourceLabel(descriptor)}:`, - error - ); return { descriptor, action: 'delete' as const, @@ -1083,18 +1080,17 @@ async function deleteTier( } }; - const tasks = descriptors.map((descriptor) => async () => { - const { value, retries } = await trackRetries(() => deleteDescriptor(descriptor)); - return attachRetries([value], retries)[0] ?? value; - }); - - const taskResults = await runParallel(tasks, 5); - - return taskResults.map((tr, index) => { - if (tr.status === 'fulfilled' && tr.value) { - return tr.value; - } else { - const descriptor = descriptors[index]; + const deleteAll = async (pending: ResourceDescriptor[]): Promise => { + const tasks = pending.map((descriptor) => async () => { + const { value, retries } = await trackRetries(() => deleteDescriptor(descriptor)); + return attachRetries([value], retries)[0] ?? value; + }); + const taskResults = await runParallel(tasks, 5); + return taskResults.map((tr, index) => { + if (tr.status === 'fulfilled' && tr.value) { + return tr.value; + } + const descriptor = pending[index]; if (!descriptor) { throw new Error('No descriptor found for failed task'); } @@ -1104,7 +1100,40 @@ async function deleteTier( status: 'failed' as const, error: tr.reason || new Error('Unknown error'), }; + }); + }; + + // Resources in one tier can use each other (a backend pool its members, a + // backend or logger the named value in its credentials, a fragment a named + // value), and APIM rejects deleting one that something still uses. Such a + // delete is tried again once the rest of the tier is gone, for as long as + // each round deletes something. + const finalResults = new Map(); + let pending = descriptors; + while (pending.length > 0) { + const roundResults = await deleteAll(pending); + const inUse = roundResults.filter((result) => result.status === 'failed' && isResourceInUseError(result.error)); + const progressed = roundResults.some((result) => result.status !== 'failed'); + for (const result of roundResults) { + if (!inUse.includes(result) || !progressed) { + finalResults.set(result.descriptor, result); + } + } + if (!progressed) { + break; + } + pending = inUse.map((result) => result.descriptor); + if (pending.length > 0) { + logger.debug(`Deleting ${pending.length} resource(s) again that others being deleted were still using`); + } + } + + return descriptors.map((descriptor) => { + const result = finalResults.get(descriptor)!; + if (result.status === 'failed') { + logger.error(`Failed to delete ${buildResourceLabel(descriptor)}:`, result.error); } + return result; }); } diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 629cccd9..125b9c37 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -9,6 +9,7 @@ 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, logger } from '../../../src/lib/logger.js'; +import { HttpError } from '../../../src/clients/apim-client.js'; import { recordRetry, withRetryResource } from '../../../src/lib/retry-tracker.js'; import { getResourceDescriptorKey } from '../../../src/lib/resource-path.js'; @@ -1162,6 +1163,63 @@ describe('publish-service', () => { warn.mockRestore(); }); + describe('deleting resources that use each other', () => { + const inUse = (name: string, user: string) => new HttpError( + 400, + `HTTP 400: {"error":{"code":"ValidationError","message":"The Api Management Property '${name}' is used by the following entities: /${user}"}}`, + 'ValidationError' + ); + const descriptor = (type: ResourceType, name: string): ResourceDescriptor => ({ type, nameParts: [name] }); + const run = async (deleted: ResourceDescriptor[], deleteResource: (name: string) => Promise) => { + const client = createMockClient(); + client.deleteResource.mockImplementation(async (_context: ApimServiceContext, d: ResourceDescriptor) => deleteResource(d.nameParts[0] ?? '')); + vi.mocked(computeGitDiff).mockResolvedValue({ changedDescriptors: [], deletedDescriptors: deleted }); + const result = await runPublish(client, createMockStore([]), { + service: testContext, + sourceDir: '/source', + dryRun: false, + deleteUnmatched: true, + commitId: 'abc123', + logLevel: LogLevel.INFO, + }); + const calls = client.deleteResource.mock.calls.map(([, d]) => (d as ResourceDescriptor).nameParts[0]); + return { result, calls }; + }; + + it('should delete a pool and its member, and a logger and the named value in its credentials, in one run', async () => { + const existing = new Set(['pool', 'member', 'key', 'ai']); + const { result, calls } = await run( + [descriptor(ResourceType.Backend, 'member'), descriptor(ResourceType.NamedValue, 'key'), + descriptor(ResourceType.Backend, 'pool'), descriptor(ResourceType.Logger, 'ai')], + async (name) => { + if (name === 'member' && existing.has('pool')) throw inUse('member', 'backends/pool'); + if (name === 'key' && existing.has('ai')) throw inUse('key', 'loggers/ai'); + existing.delete(name); + return true; + } + ); + + expect(existing.size).toBe(0); + expect(result.exitCode).toBe(0); + expect(calls.filter((name) => name === 'member')).toHaveLength(2); + }); + + it('should not try again a delete that failed for another reason, or one nothing in the run frees', async () => { + const { result, calls } = await run( + [descriptor(ResourceType.Tag, 'broken'), descriptor(ResourceType.Tag, 'busy'), descriptor(ResourceType.Tag, 'free')], + async (name) => { + if (name === 'broken') throw new HttpError(500, 'HTTP 500: boom'); + if (name === 'busy') throw inUse('busy', 'apis/elsewhere'); + return true; + } + ); + + expect(calls.filter((name) => name === 'broken')).toHaveLength(1); + expect(calls.filter((name) => name === 'busy')).toHaveLength(2); + expect(result.exitCode).toBe(1); + }); + }); + it('should delete descriptors removed in the commit when explicitly enabled', async () => { const client = createMockClient(); const store = createMockStore([]); From e70159ace903818fb91392694c688468bb246893 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Fri, 6 Nov 2026 17:57:33 +1100 Subject: [PATCH 3/6] fix: check the master subscription under its name on the instance The deletes guarded `master` by its source name, before environment mapping. Subscriptions are in the default appliesTo, so with prefix `m` a source subscription `aster` deploys as `master`, and its deletion sent a DELETE for the built-in subscription; with prefix `dev-`, a user's own `dev-master` read as `master` and was never deleted. Full and incremental deletes, real and dry-run, now check the name on the instance. --- src/services/delete-unmatched-service.ts | 11 ++++---- src/services/dry-run-reporter.ts | 7 +++-- src/services/publish-service.ts | 8 +++--- .../services/delete-unmatched-service.test.ts | 27 +++++++++++++++++++ tests/unit/services/publish-service.test.ts | 20 ++++++++++++++ 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/services/delete-unmatched-service.ts b/src/services/delete-unmatched-service.ts index 2ae2adac..ccfe275b 100644 --- a/src/services/delete-unmatched-service.ts +++ b/src/services/delete-unmatched-service.ts @@ -134,7 +134,11 @@ export async function computeDeleteActions( for await (const resource of apimResources) { const deployedDescriptor = parseResourceDescriptor(resource, resourceType); - if (!deployedDescriptor) { + // APIM's built-in `master` subscription can't be recreated once deleted. + // It is checked under its name on the instance: with an environment + // affix, a source name can deploy as `master`, and `master` can read as + // another source name. + if (!deployedDescriptor || isBuiltInSubscription(deployedDescriptor)) { continue; } @@ -397,11 +401,6 @@ function isSystemResource(descriptor: ResourceDescriptor): boolean { } } - // APIM's built-in all-access subscription can't be recreated once deleted. - if (isBuiltInSubscription(descriptor)) { - return true; - } - // Check system products and APIs if ( descriptor.type === ResourceType.Product || diff --git a/src/services/dry-run-reporter.ts b/src/services/dry-run-reporter.ts index d98ea238..4b6e8008 100644 --- a/src/services/dry-run-reporter.ts +++ b/src/services/dry-run-reporter.ts @@ -162,12 +162,15 @@ export async function generateDryRunReport( // Apply the same ;rev=N filtering as the real delete path so the preview // matches what publish would actually delete. if (incrementalDeletedDescriptors.length > 0) { - for (const descriptor of filterRevisionDeletesHandledByBaseApi(incrementalDeletedDescriptors) - .filter((deleted) => !isBuiltInSubscription(deleted))) { + for (const descriptor of filterRevisionDeletesHandledByBaseApi(incrementalDeletedDescriptors)) { try { const deployedDescriptor = config.envMapping ? mapDescriptor(descriptor, config.envMapping) : descriptor; + // As publish: never the built-in `master` subscription, by its name on the instance. + if (isBuiltInSubscription(deployedDescriptor)) { + continue; + } const resolvedDescriptor = await resolveAssociationDeleteDescriptor( client, context, diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 06b94efc..31fe6742 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -992,10 +992,12 @@ async function executeDeletesForDescriptors( // whose base API is also in the delete set to avoid redundant/racy deletes and // the "Cannot delete the current revision of an API" error. // APIM's built-in `master` subscription is never deleted: one that is can't - // be recreated (isBuiltInSubscription). An incremental publish lists it when - // its artifact is removed from the source. + // be recreated (isBuiltInSubscription). It is checked under the name on the + // instance, which an environment affix can make `master`. const effectiveDescriptors = filterRevisionDeletesHandledByBaseApi(deleteDescriptors) - .filter((descriptor) => !isBuiltInSubscription(descriptor)); + .filter((descriptor) => !isBuiltInSubscription( + config.envMapping && !descriptorsAreDeployed ? mapDescriptor(descriptor, config.envMapping) : descriptor + )); // Group by tier const tierGroups = new Map(); diff --git a/tests/unit/services/delete-unmatched-service.test.ts b/tests/unit/services/delete-unmatched-service.test.ts index 8dc124e4..16af1f90 100644 --- a/tests/unit/services/delete-unmatched-service.test.ts +++ b/tests/unit/services/delete-unmatched-service.test.ts @@ -160,6 +160,33 @@ describe('delete-unmatched-service', () => { expect(result.filter((d) => d.type === ResourceType.Subscription).map((d) => d.nameParts[0])).toEqual(['old-sub']); }); + it("should decide master by its name on the instance when names are affixed", async () => { + const apimResources = new Map[]>([ + [ResourceType.Subscription, [ + { name: 'master', id: '/subscriptions/master', properties: { scope: '/' } }, + { name: 'mold-sub', id: '/subscriptions/mold-sub', properties: { scope: '/apis/orders' } }, + ]], + ]); + const subscriptions = async (namePrefix: string) => (await computeDeleteActions( + createMockClient(apimResources), createMockStore([]), testContext, { ...testConfig, envMapping: buildEnvMapping({ namePrefix }) } + )).filter((d) => d.type === ResourceType.Subscription).map((d) => d.nameParts[0]); + + // With prefix `m`, master reads as source name `aster`; it is still never deleted. + expect(await subscriptions('m')).toEqual(['mold-sub']); + }); + + it("should delete a user subscription whose source name is master", async () => { + const apimResources = new Map[]>([ + [ResourceType.Subscription, [{ name: 'dev-master', id: '/subscriptions/dev-master', properties: { scope: '/apis/orders' } }]], + ]); + + const result = await computeDeleteActions( + createMockClient(apimResources), createMockStore([]), testContext, { ...testConfig, envMapping: buildEnvMapping({ namePrefix: 'dev-' }) } + ); + + expect(result.map((d) => d.nameParts[0])).toEqual(['dev-master']); + }); + it('should skip echo-api system API', async () => { const apimResources = new Map[]>([ [ResourceType.Api, [ diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 125b9c37..293b594d 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -1139,6 +1139,26 @@ describe('publish-service', () => { expect(computeGitDiff).toHaveBeenCalledWith('/source', 'abc123'); }); + it("should not delete the built-in master subscription that an affixed source name deploys as", async () => { + const client = createMockClient(); + vi.mocked(computeGitDiff).mockResolvedValue({ + changedDescriptors: [], + deletedDescriptors: [{ type: ResourceType.Subscription, nameParts: ['aster'] }], + }); + + await runPublish(client, createMockStore([]), { + service: testContext, + sourceDir: '/source', + dryRun: false, + deleteUnmatched: true, + commitId: 'abc123', + overrides: { environment: { namePrefix: 'm' } }, + logLevel: LogLevel.INFO, + }); + + expect(client.deleteResource).not.toHaveBeenCalled(); + }); + it("should neither publish nor delete APIM's built-in master subscription, without a warning", async () => { const client = createMockClient(); const master = { type: ResourceType.Subscription, nameParts: ['master'] }; From 4af759f2d5cd2204cd93f2ea2bfb8c4a7eb94dd2 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Fri, 6 Nov 2026 17:57:33 +1100 Subject: [PATCH 4/6] fix: report the HTTP retries a delete needed across rounds A delete retried before APIM rejected it as in use, then deleted in a later round, reported only the last round's retries. Each resource's retries now add up across rounds. --- src/services/publish-service.ts | 9 ++++++++- tests/unit/services/publish-service.test.ts | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 31fe6742..e1e9310b 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -1111,9 +1111,15 @@ async function deleteTier( // delete is tried again once the rest of the tier is gone, for as long as // each round deletes something. const finalResults = new Map(); + // HTTP retries add up across rounds: a delete can be retried before APIM + // rejects it as in use, then succeed in a later round. + const retries = new Map(); let pending = descriptors; while (pending.length > 0) { const roundResults = await deleteAll(pending); + for (const result of roundResults) { + retries.set(result.descriptor, (retries.get(result.descriptor) ?? 0) + (result.retries ?? 0)); + } const inUse = roundResults.filter((result) => result.status === 'failed' && isResourceInUseError(result.error)); const progressed = roundResults.some((result) => result.status !== 'failed'); for (const result of roundResults) { @@ -1131,7 +1137,8 @@ async function deleteTier( } return descriptors.map((descriptor) => { - const result = finalResults.get(descriptor)!; + const total = retries.get(descriptor) ?? 0; + const result = { ...finalResults.get(descriptor)!, ...(total > 0 ? { retries: total } : {}) }; if (result.status === 'failed') { logger.error(`Failed to delete ${buildResourceLabel(descriptor)}:`, result.error); } diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 293b594d..de7c3566 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -1224,6 +1224,24 @@ describe('publish-service', () => { expect(calls.filter((name) => name === 'member')).toHaveLength(2); }); + it('should report the HTTP retries a delete needed across rounds', async () => { + const existing = new Set(['pool', 'member']); + const { result } = await run( + [descriptor(ResourceType.Backend, 'member'), descriptor(ResourceType.Backend, 'pool')], + async (name) => { + if (name === 'member' && existing.has('pool')) { + recordRetry(); + throw inUse('member', 'backends/pool'); + } + existing.delete(name); + return true; + } + ); + + expect(result.actions.find((action) => action.descriptor.nameParts[0] === 'member')?.retries).toBe(1); + expect(result.totalRetries).toBe(1); + }); + it('should not try again a delete that failed for another reason, or one nothing in the run frees', async () => { const { result, calls } = await run( [descriptor(ResourceType.Tag, 'broken'), descriptor(ResourceType.Tag, 'busy'), descriptor(ResourceType.Tag, 'free')], From a31ea45049795add40ef9bcc93d90d4e22ef1733 Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:26:39 +1100 Subject: [PATCH 5/6] fix: leave out a subscription that an environment affix publishes as master Publish left out the built-in `master` by its source name, before the environment mapping is built, so a subscription `aster` with `namePrefix: 'm'` was published, and planned in dry-run, as `master`, writing to APIM's built-in subscription. Once the mapping is built, a subscription whose name on the instance is `master` is left out with a warning, as deletes already check it. --- src/services/publish-service.ts | 23 ++++++++++++++++-- tests/unit/services/publish-service.test.ts | 27 +++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index e1e9310b..90402064 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -126,7 +126,7 @@ async function runPublishPipeline( await client.validatePreFlight(config.service); // Step 1: Determine target descriptors based on incremental mode - const { targetDescriptors, deletedDescriptors } = await determinePublishTargets( + const { targetDescriptors: selectedDescriptors, deletedDescriptors } = await determinePublishTargets( store, config ); @@ -140,10 +140,13 @@ async function runPublishPipeline( // every unchanged artifact in incremental runs. const allDescriptors = config.commitId ? await store.listResources(config.sourceDir) - : targetDescriptors; + : selectedDescriptors; // Step 1b: Validate env mapping and store it on config for downstream use validateAndBuildEnvMapping(config.overrides, allDescriptors, config); + const targetDescriptors = selectedDescriptors.filter( + (descriptor) => !deploysAsBuiltInSubscription(descriptor, config) + ); // Step 1c: Build known artifact sets for policy ref rewriting config.knownArtifactSets = buildKnownArtifactSets(allDescriptors); @@ -319,6 +322,22 @@ async function determinePublishTargets( return { targetDescriptors, deletedDescriptors }; } +/** + * Whether an environment affix makes a subscription's name on the instance + * `master`: publishing it would write to APIM's built-in subscription + * (isBuiltInSubscription), so it is left out with a warning. The source's + * own `master` is left out quietly when choosing what to publish. + */ +function deploysAsBuiltInSubscription(descriptor: ResourceDescriptor, config: PublishConfig): boolean { + if (!config.envMapping || !isBuiltInSubscription(mapDescriptor(descriptor, config.envMapping))) { + return false; + } + logger.warn( + `Skipping subscription "${descriptor.nameParts.join('/')}": the environment affix would publish it as APIM's built-in "master" subscription` + ); + return true; +} + async function expandTransitivePublishTargets( store: IArtifactStore, sourceDir: string, diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index de7c3566..51ad68ae 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -1159,6 +1159,33 @@ describe('publish-service', () => { expect(client.deleteResource).not.toHaveBeenCalled(); }); + it('should not publish, or plan in dry-run, a subscription that an affix deploys as the built-in master', async () => { + const store = createMockStore([ + { type: ResourceType.Subscription, nameParts: ['aster'] }, + { type: ResourceType.Subscription, nameParts: ['orders-sub'] }, + ]); + const warn = vi.spyOn(logger, 'warn'); + const base: PublishConfig = { + service: testContext, + sourceDir: '/source', + dryRun: false, + deleteUnmatched: false, + overrides: { environment: { namePrefix: 'm' } }, + logLevel: LogLevel.INFO, + }; + + const client = createMockClient(); + await runPublish(client, store, base); + expect(client.putResource.mock.calls.map(([, descriptor]) => descriptor.nameParts[0])).toEqual(['morders-sub']); + expect(warn.mock.calls.map(([message]) => String(message)).filter((message) => message.includes('"aster"'))).toHaveLength(1); + + await runPublish(client, store, { ...base, dryRun: true }); + expect(vi.mocked(generateDryRunReport).mock.calls.at(-1)?.[4]).toEqual([ + { type: ResourceType.Subscription, nameParts: ['orders-sub'] }, + ]); + warn.mockRestore(); + }); + it("should neither publish nor delete APIM's built-in master subscription, without a warning", async () => { const client = createMockClient(); const master = { type: ResourceType.Subscription, nameParts: ['master'] }; From 20d964afffe0d9715b5aa2ee0cf8da10882287bc Mon Sep 17 00:00:00 2001 From: Chris Dayne <1642847+cdayne@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:26:39 +1100 Subject: [PATCH 6/6] docs: say built-in resources other than master are protected only from full deletes --- docs/commands/publish.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/commands/publish.md b/docs/commands/publish.md index fe86c28e..5a0eb6bf 100644 --- a/docs/commands/publish.md +++ b/docs/commands/publish.md @@ -203,7 +203,7 @@ The output lists each resource and the planned action (create, update, or delete For a full publish, `--delete-unmatched` deletes APIM resources that are **not** present in the source artifacts. With `--commit-id`, it deletes only resources and Product associations removed by the selected commit. -Deletes run from the most dependent resources to the least (tier 4 to tier 1). Within a tier, resources can use each other — a backend pool its members, a backend or logger the named value in its credentials, a policy fragment a named value — and APIM rejects deleting one that something still uses. Such a delete is tried again once the rest of the tier is deleted, so one run removes them all. APIM's built-in resources (the built-in groups, the `unlimited` and `starter` products, `echo-api`, and the `master` subscription) are never deleted. +Deletes run from the most dependent resources to the least (tier 4 to tier 1). Within a tier, resources can use each other — a backend pool its members, a backend or logger the named value in its credentials, a policy fragment a named value — and APIM rejects deleting one that something still uses. Such a delete is tried again once the rest of the tier is deleted, so one run removes them all. A full `--delete-unmatched` leaves APIM's built-in resources alone (the built-in groups, the `unlimited` and `starter` products, and `echo-api`); with `--commit-id`, a commit that removes one of their artifacts deletes it. The built-in `master` subscription is never deleted. > **Warning:** Use with caution. Resources created manually in the Azure portal that are not in your artifact directory will be removed.