From fa6fcc12aaf1a8e68c9292bb47ea7dcb352c0ed3 Mon Sep 17 00:00:00 2001 From: Andrey Sobolev Date: Sun, 13 Jul 2025 22:04:08 +0700 Subject: [PATCH] UBERF-12445: Fix adding second Github integration for same user (#9531) Signed-off-by: Andrey Sobolev --- .vscode/launch.json | 4 +- services/github/pod-github/src/platform.ts | 172 ++++++++++++++------- services/github/pod-github/src/server.ts | 3 +- services/github/pod-github/src/types.ts | 2 +- services/github/pod-github/src/worker.ts | 16 +- 5 files changed, 135 insertions(+), 62 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 9b79971e53..f3ac42a14f 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -606,9 +606,7 @@ "CLIENT_SECRET": "${env:POD_GITHUB_CLIENT_SECRET}", "PRIVATE_KEY": "${env:POD_GITHUB_PRIVATE_KEY}", "COLLABORATOR_URL": "ws://huly.local:3078", - "MINIO_ENDPOINT": "localhost:9002", - "MINIO_ACCESS_KEY": "minioadmin", - "MINIO_SECRET_KEY": "minioadmin", + "STORAGE_CONFIG": "datalake|http://huly.local:4030", "PLATFORM_OPERATION_LOGGING": "true", "FRONT_URL": "http://localhost:8080", "PORT": "3500", diff --git a/services/github/pod-github/src/platform.ts b/services/github/pod-github/src/platform.ts index b15ac74136..301f79036e 100644 --- a/services/github/pod-github/src/platform.ts +++ b/services/github/pod-github/src/platform.ts @@ -58,6 +58,10 @@ export interface InstallationRecord { suspended: boolean } +interface IntegrationDataValue { + installationId: number | number[] +} + export class PlatformWorker { private readonly clients = new Map() @@ -123,12 +127,12 @@ export class PlatformWorker { if (i.workspaceUuid == null) { continue } - const installationId = i.data?.installationId + const installationId = (i.data as IntegrationDataValue)?.installationId if (installationId !== undefined) { this.integrations.push({ accountId: i.socialId, workspace: i.workspaceUuid, - installationId + installationId: Array.isArray(installationId) ? installationId : [installationId] }) } } @@ -137,19 +141,32 @@ export class PlatformWorker { for (const integr of [...this.integrations]) { // We need to check and remove integrations without a real integration's - if (!this.installations.has(integr.installationId)) { - ctx.warn('Installation was deleted during service shutdown', { - installationId: integr.installationId, + const ids = integr.installationId + + const missing = ids.filter((id) => !this.installations.has(id)) + if (missing.length > 0) { + const has = ids.filter((id) => this.installations.has(id)) + ctx.warn('Few Installation was deleted during service shutdown', { + installationId: missing, workspace: integr.workspace }) - await accountsClient.deleteIntegration({ - kind: 'github', - workspaceUuid: integr.workspace, - socialId: integr.accountId - }) - this.integrations = this.integrations.filter((it) => it.installationId !== integr.installationId) + if (has.length > 0) { + await accountsClient.updateIntegration({ + kind: 'github', + workspaceUuid: integr.workspace, + socialId: integr.accountId, + data: { installationId: has } satisfies IntegrationDataValue + }) + } else { + await accountsClient.deleteIntegration({ + kind: 'github', + workspaceUuid: integr.workspace, + socialId: integr.accountId + }) + } } } + this.integrations = this.integrations.filter((it) => it.installationId.length > 0) void this.doSyncWorkspaces().catch((err) => { ctx.error('error during sync workspaces', { err }) @@ -227,29 +244,56 @@ export class PlatformWorker { const sysToken = generateToken(systemAccountUuid, undefined, { service: 'github' }) const accountsClient = getAccountClient(config.AccountsURL, sysToken) - const oldInstallation = this.integrations.find((it) => it.installationId === installationId) - if (oldInstallation != null) { - ctx.info('update integration', { workspace, installationId, accountId }) + const oldInstallation = this.integrations.filter((it) => it.installationId.includes(installationId)) + if (oldInstallation.length > 0) { + ctx.info('update integrations', { workspace, installationId, accountId }) // What to do with installation in different workspace? // Let's remove it and sync to new one. - if (oldInstallation.workspace !== workspace) { - // - const oldWorkspace = oldInstallation.workspace + const oldWorkspaces = oldInstallation.filter((it) => it.workspace !== workspace) + if (oldWorkspaces.length > 0) { + const oldWorkspace = oldWorkspaces[0].workspace - await accountsClient.createIntegration({ - kind: 'github', - workspaceUuid: workspace, - socialId: accountId, - data: { installationId: oldInstallation.installationId } - }) + for (const oldInstallation of oldWorkspaces) { + const has = oldInstallation.installationId.filter((it) => it !== installationId) + if (has.length > 0) { + oldInstallation.installationId = has + await accountsClient.updateIntegration({ + kind: 'github', + workspaceUuid: oldWorkspace, + socialId: accountId + }) + } else { + await accountsClient.deleteIntegration({ + kind: 'github', + workspaceUuid: oldWorkspace, + socialId: accountId + }) + } + } - await accountsClient.deleteIntegration({ - kind: 'github', - workspaceUuid: oldWorkspace, - socialId: accountId - }) - - oldInstallation.workspace = workspace + // We need new integeration to be added to new workspace + const existingRecord = this.integrations.find((it) => it.workspace === workspace && it.accountId === accountId) + if (existingRecord !== undefined) { + existingRecord.installationId.push(installationId) + await accountsClient.updateIntegration({ + kind: 'github', + workspaceUuid: workspace, + socialId: accountId, + data: { installationId: existingRecord.installationId } satisfies IntegrationDataValue + }) + } else { + await accountsClient.createIntegration({ + kind: 'github', + workspaceUuid: workspace, + socialId: accountId, + data: { installationId } satisfies IntegrationDataValue + }) + this.integrations.push({ + workspace, + installationId: [installationId], + accountId + }) + } const oldWorker = this.clients.get(oldWorkspace) as GithubWorker if (oldWorker !== undefined) { @@ -276,25 +320,35 @@ export class PlatformWorker { this.triggerCheckWorkspaces() return } - const record: GithubIntegrationRecord = { - workspace, - installationId, - accountId - } ctx.info('add integration', { workspace, installationId, accountId }) await ctx.with( 'add integration', {}, async (ctx) => { - await accountsClient.createIntegration({ - kind: 'github', - workspaceUuid: record.workspace, - socialId: record.accountId, - data: { installationId: record.installationId } - }) - - this.integrations.push(record) + const existing = this.integrations.find((it) => it.workspace === workspace && it.accountId === accountId) + if (existing !== undefined) { + existing.installationId.push(installationId) + await accountsClient.updateIntegration({ + kind: 'github', + workspaceUuid: existing.workspace, + socialId: existing.accountId, + data: { installationId: existing.installationId } satisfies IntegrationDataValue + }) + } else { + const record: GithubIntegrationRecord = { + workspace, + installationId: [installationId], + accountId + } + await accountsClient.createIntegration({ + kind: 'github', + workspaceUuid: record.workspace, + socialId: record.accountId, + data: { installationId: record.installationId } + }) + this.integrations.push(record) + } }, { workspace, installationId, accountId } ) @@ -782,10 +836,10 @@ export class PlatformWorker { this.installations.delete(installId) this.ctx.info('handle integration delete', { installId, name: existing?.installationName }) - const interg = this.integrations.find((it) => it.installationId === installId) + const interg = this.integrations.filter((it) => it.installationId.includes(installId)) // We already have, worker we need to update it. - const worker = this.getWorker(installId) ?? (interg !== undefined ? this.clients.get(interg.workspace) : undefined) + const worker = this.getWorker(installId) ?? (interg.length > 0 ? this.clients.get(interg[0].workspace) : undefined) if (worker !== undefined) { const integeration = worker.integrations.get(installId) if (integeration !== undefined) { @@ -802,16 +856,30 @@ export class PlatformWorker { this.ctx.info('No worker for removed installation', { installId, name: existing?.installationName }) // No worker } - this.integrations = this.integrations.filter((it) => it.installationId !== installId) - if (interg !== undefined) { + if (interg.length > 0) { const sysToken = generateToken(systemAccountUuid, undefined, { service: 'github' }) const sysAccountClient = getAccountClient(config.AccountsURL, sysToken) - await sysAccountClient.deleteIntegration({ - kind: 'github', - workspaceUuid: interg.workspace, - socialId: interg.accountId - }) + + for (const intgr of interg) { + const has = intgr.installationId.filter((it) => it !== installId) + if (has.length > 0) { + await sysAccountClient.updateIntegration({ + kind: 'github', + workspaceUuid: intgr.workspace, + socialId: intgr.accountId, + data: { installationId: has } satisfies IntegrationDataValue + }) + } else { + intgr.installationId = [] + await sysAccountClient.deleteIntegration({ + kind: 'github', + workspaceUuid: intgr.workspace, + socialId: intgr.accountId + }) + } + } } + this.integrations = this.integrations.filter((it) => it.installationId.length > 0) this.triggerCheckWorkspaces() } diff --git a/services/github/pod-github/src/server.ts b/services/github/pod-github/src/server.ts index 883c1dfc97..1567a744a7 100644 --- a/services/github/pod-github/src/server.ts +++ b/services/github/pod-github/src/server.ts @@ -97,7 +97,8 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro ctx.error('failed to map-installation', { workspace: tok.workspace, installationid: payloadData.installationId, - email: tok?.account + email: tok?.account, + error: err.message }) res.status(401) res.json({ error: err.message }) diff --git a/services/github/pod-github/src/types.ts b/services/github/pod-github/src/types.ts index 20838ded22..8341354c27 100644 --- a/services/github/pod-github/src/types.ts +++ b/services/github/pod-github/src/types.ts @@ -188,7 +188,7 @@ export interface DocSyncManager { * @public */ export interface GithubIntegrationRecord { - installationId: number + installationId: number[] workspace: WorkspaceUuid accountId: PersonId } diff --git a/services/github/pod-github/src/worker.ts b/services/github/pod-github/src/worker.ts index ace10fd165..5ff72fd636 100644 --- a/services/github/pod-github/src/worker.ts +++ b/services/github/pod-github/src/worker.ts @@ -1330,18 +1330,24 @@ export class GithubWorker implements IntegrationManager { } async checkMapping (): Promise { - for (const intgr of this.platform.integrations.filter((it) => it.workspace === this.workspace.uuid)) { + const installations = new Map() + for (const integeration of this.platform.integrations.filter((it) => it.workspace === this.workspace.uuid)) { + for (const installationId of integeration.installationId) { + installations.set(installationId, integeration.accountId) + } + } + for (const [installationId, accountId] of installations.entries()) { const integration = await this._client.findOne(github.class.GithubIntegration, { - installationId: intgr.installationId + installationId }) - const installation = this.installations.get(intgr.installationId) as InstallationRecord + const installation = this.installations.get(installationId) as InstallationRecord if (integration === undefined && installation !== undefined) { await this._client.createDoc( github.class.GithubIntegration, core.space.Configuration, { alive: !installation.suspended, - installationId: intgr.installationId, + installationId, clientId: config.ClientID, name: installation.installationName, nodeId: installation.loginNodeId, @@ -1349,7 +1355,7 @@ export class GithubWorker implements IntegrationManager { }, generateId(), Date.now(), - intgr.accountId + accountId ) this.triggerUpdate() } else if (integration !== undefined) {