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,