From 9fefe7a4700fb8cbc0df2058acfbaf83a21b54f8 Mon Sep 17 00:00:00 2001 From: ikethecoder Date: Wed, 5 Aug 2026 14:21:36 -0400 Subject: [PATCH] SP136: make subsystem gateway registration resumable after partial failure PUT .../subsystems/{name}/gateway always targets the same fixed, pre-allocated namespace ID for a given subsystem, but the registration sequence (CreateNamespace/createSDXNamespace) was create-only: checkNamespaceAvailable hard-rejected any second call, and createResourceSet/createUmaPolicy had no idempotency, so a call interrupted partway (timeout, restart, dropped connection) could never be retried or completed - only failed forever with "Namespace already exists". CreateNamespace now accepts allowResume, under which an existing namespace is treated as an in-progress registration to resume: the existing UMA resource set is reused (findResourceByName) instead of duplicated, permission tickets are upserted (createOrUpdatePermission) instead of blindly created, and group attributes are reconciled unconditionally instead of only on first create. createSDXNamespace opts into this and swaps createUmaPolicy for the new createUmaPolicyIfMissing, which skips creating a duplicate policy for a client that already has one on the resource. Adds a failing-before/passing-after e2e test under tests/99-sp136 that registers a subsystem's gateway twice and asserts the retry recovers with the same gatewayId instead of erroring. --- e2e/cypress.config.ts | 1 + ...subsystem-gateway-registration-recovery.ts | 63 +++++++++++++++++++ src/services/org-groups/namespace.ts | 4 ++ src/services/workflow/create-namespace-sdx.ts | 8 ++- src/services/workflow/create-namespace.ts | 33 ++++++++-- src/services/workflow/ns-uma-policy-access.ts | 31 +++++++++ 6 files changed, 133 insertions(+), 7 deletions(-) create mode 100644 e2e/cypress/tests/99-sp136/01-subsystem-gateway-registration-recovery.ts diff --git a/e2e/cypress.config.ts b/e2e/cypress.config.ts index 9734727f6..6308578b3 100644 --- a/e2e/cypress.config.ts +++ b/e2e/cypress.config.ts @@ -46,6 +46,7 @@ export default defineConfig({ './cypress/tests/21-*/**/*.ts', './cypress/tests/22-*/*.ts', './cypress/tests/23-*/*.ts', + './cypress/tests/99-*/*.ts', ] return config }, diff --git a/e2e/cypress/tests/99-sp136/01-subsystem-gateway-registration-recovery.ts b/e2e/cypress/tests/99-sp136/01-subsystem-gateway-registration-recovery.ts new file mode 100644 index 000000000..8c8b8cc0d --- /dev/null +++ b/e2e/cypress/tests/99-sp136/01-subsystem-gateway-registration-recovery.ts @@ -0,0 +1,63 @@ +import { v4 as uuidv4 } from 'uuid' + +import { createRuntimeGroup, createSubsystem, uniqueSubsystemName } from '../../support/sdx-commands' + +describe('SDX Subsystem Gateway Registration Recovery', () => { + let workingData: any + + before(() => { + cy.buildOrgGatewayDatasetAndProduct().then((data) => { + workingData = data + + const rg = uuidv4().replace(/-/g, '').toUpperCase().substring(0, 6) + workingData['runtimeGroupId'] = rg.toLowerCase() + + createRuntimeGroup(workingData.org, workingData.runtimeGroupId, 'dev') + }) + }) + + it('PUT /organizations/{org}/subsystems/{name}/gateway - retrying a registration for the same subsystem recovers instead of failing', () => { + const { org, runtimeGroupId } = workingData + const subsystemName = uniqueSubsystemName() + + createSubsystem(org, subsystemName, () => { + cy.setRequestBody({ runtimeGroupName: runtimeGroupId }) + cy.callAPI( + `ds/api/sdx/v1/organizations/${org.name}/subsystems/${subsystemName}/gateway`, + 'PUT' + ).then(({ apiRes: { status, body } }: any) => { + expect(status, body.message).to.be.equal(200) + expect(body).to.have.property('gatewayId') + const gatewayId = body.gatewayId + + // Simulate a caller retrying the same registration request - e.g. + // because it never saw the first response (timeout, dropped + // connection, portal restart). The subsystem's gateway/namespace ID + // is fixed and immutable, so this always targets the exact same + // namespace as the first call. + cy.setRequestBody({ runtimeGroupName: runtimeGroupId }) + cy.callAPI( + `ds/api/sdx/v1/organizations/${org.name}/subsystems/${subsystemName}/gateway`, + 'PUT' + ).then(({ apiRes: { status, body } }: any) => { + expect(status, body.message).to.be.equal(200) + expect(body).to.have.property('gatewayId') + expect(body.gatewayId).to.be.equal(gatewayId) + + // The subsystem catalog should reflect a single, consistent + // registration - no duplicate/corrupted state left behind by the + // repeated registration attempt. + cy.callAPI( + `ds/api/sdx/v1/organizations/${org.name}/subsystems`, + 'GET' + ).then(({ apiRes: { status, body } }: any) => { + expect(status).to.be.equal(200) + const matches = body.filter((s: any) => s.name === subsystemName) + expect(matches.length).to.be.equal(1) + expect(matches[0].gatewayId).to.be.equal(gatewayId) + }) + }) + }) + }) + }) +}) diff --git a/src/services/org-groups/namespace.ts b/src/services/org-groups/namespace.ts index 70e460909..f0f271e19 100644 --- a/src/services/org-groups/namespace.ts +++ b/src/services/org-groups/namespace.ts @@ -150,6 +150,10 @@ export class NamespaceService { assert.strictEqual(groupExists, false, 'Namespace already exists'); } + async namespaceExists(ns: string): Promise { + return this.groupService.hasGroup('ns', ns); + } + async listAssignedNamespacesByOrg(org: string): Promise { const groups = await this.groupService.getGroups('ns', false); assert.strictEqual( diff --git a/src/services/workflow/create-namespace-sdx.ts b/src/services/workflow/create-namespace-sdx.ts index 4565fb36f..9a456730e 100644 --- a/src/services/workflow/create-namespace-sdx.ts +++ b/src/services/workflow/create-namespace-sdx.ts @@ -12,7 +12,10 @@ import { CreateNamespace, CreateNamespaceArgs } from './create-namespace'; import assert from '../user-assert'; import { EnvironmentContext, getEnvironmentContext } from './get-namespaces'; import { lookupProductEnvironmentServicesBySlug } from '../keystone'; -import { createUmaPolicy, updateUmaPolicy } from './ns-uma-policy-access'; +import { + createUmaPolicyIfMissing, + updateUmaPolicy, +} from './ns-uma-policy-access'; import { SysGroupAccessService } from '../org-groups/sys-group-access'; import { GroupAccessService } from '../org-groups'; @@ -296,6 +299,7 @@ async function createSDXNamespace( 'GatewayPattern.Publish', ]; args.includeSDXScopes = true; + args.allowResume = true; const resourceSet = await CreateNamespace(context, args); @@ -308,7 +312,7 @@ async function createSDXNamespace( scopes: ['GatewayConfig.Publish', 'Namespace.Manage'], }; - const umaResult = await createUmaPolicy( + const umaResult = await createUmaPolicyIfMissing( context, envCtx, resourceSet.id, diff --git a/src/services/workflow/create-namespace.ts b/src/services/workflow/create-namespace.ts index b6513dcca..0b3fc6bfc 100644 --- a/src/services/workflow/create-namespace.ts +++ b/src/services/workflow/create-namespace.ts @@ -39,6 +39,14 @@ export interface CreateNamespaceArgs { routePaths?: string[]; assignedScopes?: string[]; includeSDXScopes?: boolean; + /** + * When true, an existing namespace with this exact name is treated as an + * in-progress/partially-completed registration to resume rather than a + * conflict to reject. Only safe when `name` is a stable, pre-allocated + * identifier (e.g. a Subsystem's own `namespace`) that can never + * legitimately collide with a different owner's namespace. + */ + allowResume?: boolean; } export async function CreateNamespace( @@ -67,7 +75,15 @@ export async function CreateNamespace( envCtx.issuerEnvConfig.clientId, envCtx.issuerEnvConfig.clientSecret ); - await nsService.checkNamespaceAvailable(newNS); + const resuming = args.allowResume && (await nsService.namespaceExists(newNS)); + if (!resuming) { + await nsService.checkNamespaceAvailable(newNS); + } else { + logger.info( + '[CreateNamespace] Namespace %s already exists, resuming registration', + newNS + ); + } // This function gets all resources but also sets the accessToken in envCtx // which we need to create the resource set @@ -98,7 +114,10 @@ export async function CreateNamespace( ownerManagedAccess: true, }; - const rset = await resourceApi.createResourceSet(res); + const existingRset = resuming + ? await resourceApi.findResourceByName(newNS) + : undefined; + const rset = existingRset || (await resourceApi.createResourceSet(res)); if (isUserBasedResourceOwners(envCtx) == false) { const permissionApi = new KeycloakPermissionTicketService( @@ -111,7 +130,7 @@ export async function CreateNamespace( 'GatewayConfig.Publish', 'Access.Manage', ]) { - await permissionApi.createPermission( + await permissionApi.createOrUpdatePermission( rset.id, envCtx.subjectUuid, true, @@ -128,9 +147,13 @@ export async function CreateNamespace( envCtx.issuerEnvConfig.clientSecret ); - const { id, created } = await kcGroupService.createIfMissing('ns', newNS); + const { id } = await kcGroupService.createIfMissing('ns', newNS); - if (created) { + { + // Reconcile attributes unconditionally (not just on first create) so a + // resumed registration still lands on the fully-configured end state, + // even if an earlier attempt died after the group was created but + // before attributes were applied. const gwGroup = await kcGroupService.getGroupById(id); if (args.org) { gwGroup.attributes['org'] = [args.org]; diff --git a/src/services/workflow/ns-uma-policy-access.ts b/src/services/workflow/ns-uma-policy-access.ts index ee6ce8d31..75c2167ba 100644 --- a/src/services/workflow/ns-uma-policy-access.ts +++ b/src/services/workflow/ns-uma-policy-access.ts @@ -39,6 +39,37 @@ export async function createUmaPolicy( return umaPolicy; } +export async function createUmaPolicyIfMissing( + context: any, + envCtx: EnvironmentContext, + resourceId: string, + policy: Policy +) { + logger.debug('[createUmaPolicyIfMissing] %s %j', resourceId, policy); + + await enforceAccessToResource(envCtx, resourceId); + + const policyApi = new UMAPolicyService( + envCtx.uma2.policy_endpoint, + envCtx.accessToken + ); + + const existing = (await policyApi.listPolicies({ resource: resourceId })) + .filter((p) => policy.clients?.every((c) => p.clients?.includes(c))) + .pop(); + + if (existing) { + logger.debug( + '[createUmaPolicyIfMissing] %s already has a policy for %j, skipping create', + resourceId, + policy.clients + ); + return existing; + } + + return createUmaPolicy(context, envCtx, resourceId, policy); +} + export async function updateUmaPolicy( context: any, envCtx: EnvironmentContext,