From 4b755fb0b441a37eb804c9e70d4b071a341f7155 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Wed, 28 Jun 2023 11:06:40 +0200 Subject: [PATCH] fix(core): Use owners file to export wf owners (#6547) * remove owner from exported workflow * use owners file to export wf owners * update sharedworkflow owners * fix logic * further update logic * add updatetAt to local changes * additional filter for cred export * optimize query * remove transactions and optimize query * reduce array size and add updated at to tags status --- .../environments/sourceControl/constants.ts | 1 + .../sourceControl/sourceControl.service.ee.ts | 53 ++- .../sourceControlExport.service.ee.ts | 16 +- .../sourceControlImport.service.ee.ts | 316 ++++++++++-------- .../sourceControl/types/exportableWorkflow.ts | 1 - .../types/sourceControlledFile.ts | 1 + 6 files changed, 235 insertions(+), 153 deletions(-) diff --git a/packages/cli/src/environments/sourceControl/constants.ts b/packages/cli/src/environments/sourceControl/constants.ts index ab74559ce7..3ef023f349 100644 --- a/packages/cli/src/environments/sourceControl/constants.ts +++ b/packages/cli/src/environments/sourceControl/constants.ts @@ -5,6 +5,7 @@ export const SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER = 'workflows'; export const SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER = 'credential_stubs'; export const SOURCE_CONTROL_VARIABLES_EXPORT_FILE = 'variable_stubs.json'; export const SOURCE_CONTROL_TAGS_EXPORT_FILE = 'tags.json'; +export const SOURCE_CONTROL_OWNERS_EXPORT_FILE = 'owners.json'; export const SOURCE_CONTROL_SSH_FOLDER = 'ssh'; export const SOURCE_CONTROL_SSH_KEY_NAME = 'key'; export const SOURCE_CONTROL_DEFAULT_BRANCH = 'main'; diff --git a/packages/cli/src/environments/sourceControl/sourceControl.service.ee.ts b/packages/cli/src/environments/sourceControl/sourceControl.service.ee.ts index d1c5a3ccf6..35125ecc96 100644 --- a/packages/cli/src/environments/sourceControl/sourceControl.service.ee.ts +++ b/packages/cli/src/environments/sourceControl/sourceControl.service.ee.ts @@ -32,6 +32,8 @@ import type { import { SourceControlPreferencesService } from './sourceControlPreferences.service.ee'; import { writeFileSync } from 'fs'; import { SourceControlImportService } from './sourceControlImport.service.ee'; +import type { WorkflowEntity } from '../../databases/entities/WorkflowEntity'; +import type { CredentialsEntity } from '../../databases/entities/CredentialsEntity'; @Service() export class SourceControlService { private sshKeyName: string; @@ -252,6 +254,7 @@ export class SourceControlService { ...status.modified, ]); } + mergedFileNames.add(this.sourceControlExportService.getOwnersPath()); const deletedFiles = new Set(status.deleted); deletedFiles.forEach((e) => mergedFileNames.delete(e)); await this.unstage(); @@ -285,6 +288,20 @@ export class SourceControlService { let conflict = false; let status: SourceControlledFileStatus = 'unknown'; let type: SourceControlledFileType = 'file'; + let updatedAt = ''; + + const allWorkflows: Map = new Map(); + (await Db.collections.Workflow.find({ select: ['id', 'name', 'updatedAt'] })).forEach( + (workflow) => { + allWorkflows.set(workflow.id, workflow); + }, + ); + const allCredentials: Map = new Map(); + (await Db.collections.Credentials.find({ select: ['id', 'name', 'updatedAt'] })).forEach( + (credential) => { + allCredentials.set(credential.id, credential); + }, + ); // initialize status from git status result if (statusResult.not_added.find((e) => e === fileName)) status = 'new'; @@ -303,14 +320,14 @@ export class SourceControlService { .replace(/[\/,\\]/, '') .replace('.json', ''); if (location === 'remote') { - const existingWorkflow = await Db.collections.Workflow.find({ - where: { id }, - }); - if (existingWorkflow?.length > 0) { - name = existingWorkflow[0].name; + const existingWorkflow = allWorkflows.get(id); + if (existingWorkflow) { + name = existingWorkflow.name; + updatedAt = existingWorkflow.updatedAt.toISOString(); } } else { name = '(deleted)'; + // todo: once we have audit log, this deletion date could be looked up } } else { const workflow = await this.sourceControlExportService.getWorkflowFromFile(fileName); @@ -326,6 +343,11 @@ export class SourceControlService { id = workflow.id; name = workflow.name; } + const existingWorkflow = allWorkflows.get(id); + if (existingWorkflow) { + name = existingWorkflow.name; + updatedAt = existingWorkflow.updatedAt.toISOString(); + } } } if (fileName.startsWith(SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER)) { @@ -336,11 +358,10 @@ export class SourceControlService { .replace(/[\/,\\]/, '') .replace('.json', ''); if (location === 'remote') { - const existingCredential = await Db.collections.Credentials.find({ - where: { id }, - }); - if (existingCredential?.length > 0) { - name = existingCredential[0].name; + const existingCredential = allCredentials.get(id); + if (existingCredential) { + name = existingCredential.name; + updatedAt = existingCredential.updatedAt.toISOString(); } } else { name = '(deleted)'; @@ -359,6 +380,11 @@ export class SourceControlService { id = credential.id; name = credential.name; } + const existingCredential = allCredentials.get(id); + if (existingCredential) { + name = existingCredential.name; + updatedAt = existingCredential.updatedAt.toISOString(); + } } } @@ -369,9 +395,15 @@ export class SourceControlService { } if (fileName.startsWith(SOURCE_CONTROL_TAGS_EXPORT_FILE)) { + const lastUpdatedTag = await Db.collections.Tag.find({ + order: { updatedAt: 'DESC' }, + take: 1, + select: ['updatedAt'], + }); id = 'tags'; name = 'tags'; type = 'tags'; + updatedAt = lastUpdatedTag[0]?.updatedAt.toISOString(); } if (!id) return; @@ -384,6 +416,7 @@ export class SourceControlService { status, location, conflict, + updatedAt, }; } diff --git a/packages/cli/src/environments/sourceControl/sourceControlExport.service.ee.ts b/packages/cli/src/environments/sourceControl/sourceControlExport.service.ee.ts index 1352c8c7b8..b8f24e03e0 100644 --- a/packages/cli/src/environments/sourceControl/sourceControlExport.service.ee.ts +++ b/packages/cli/src/environments/sourceControl/sourceControlExport.service.ee.ts @@ -3,6 +3,7 @@ import path from 'path'; import { SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER, SOURCE_CONTROL_GIT_FOLDER, + SOURCE_CONTROL_OWNERS_EXPORT_FILE, SOURCE_CONTROL_TAGS_EXPORT_FILE, SOURCE_CONTROL_VARIABLES_EXPORT_FILE, SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER, @@ -50,6 +51,10 @@ export class SourceControlExportService { return path.join(this.gitFolder, SOURCE_CONTROL_TAGS_EXPORT_FILE); } + getOwnersPath(): string { + return path.join(this.gitFolder, SOURCE_CONTROL_OWNERS_EXPORT_FILE); + } + getVariablesPath(): string { return path.join(this.gitFolder, SOURCE_CONTROL_VARIABLES_EXPORT_FILE); } @@ -160,7 +165,6 @@ export class SourceControlExportService { connections: e.workflow?.connections, settings: e.workflow?.settings, triggerCount: e.workflow?.triggerCount, - owner: e.user.email, versionId: e.workflow?.versionId, }; LoggerProxy.debug(`Writing workflow ${e.workflowId} to ${fileName}`); @@ -186,6 +190,11 @@ export class SourceControlExportService { const removedFiles = await this.rmDeletedWorkflowsFromExportFolder(sharedWorkflows); // write the workflows to the export folder as json files await this.writeExportableWorkflowsToExportFolder(sharedWorkflows); + // write list of owners to file + const ownersFileName = this.getOwnersPath(); + const owners: Record = {}; + sharedWorkflows.forEach((e) => (owners[e.workflowId] = e.user.email)); + await fsWriteFile(ownersFileName, JSON.stringify(owners, null, 2)); return { count: sharedWorkflows.length, folder: this.workflowExportFolder, @@ -280,7 +289,10 @@ export class SourceControlExportService { } else if (typeof data[key] === 'object') { data[key] = this.replaceCredentialData(data[key] as ICredentialDataDecryptedObject); } else if (typeof data[key] === 'string') { - data[key] = (data[key] as string)?.startsWith('={{') ? data[key] : ''; + data[key] = + (data[key] as string)?.startsWith('={{') && (data[key] as string)?.includes('$secret') + ? data[key] + : ''; } else if (typeof data[key] === 'number') { // TODO: leaving numbers in for now, but maybe we should remove them continue; diff --git a/packages/cli/src/environments/sourceControl/sourceControlImport.service.ee.ts b/packages/cli/src/environments/sourceControl/sourceControlImport.service.ee.ts index af4d5946d8..e1e680b338 100644 --- a/packages/cli/src/environments/sourceControl/sourceControlImport.service.ee.ts +++ b/packages/cli/src/environments/sourceControl/sourceControlImport.service.ee.ts @@ -3,6 +3,7 @@ import path from 'path'; import { SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER, SOURCE_CONTROL_GIT_FOLDER, + SOURCE_CONTROL_OWNERS_EXPORT_FILE, SOURCE_CONTROL_TAGS_EXPORT_FILE, SOURCE_CONTROL_VARIABLES_EXPORT_FILE, SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER, @@ -14,15 +15,12 @@ import { readFile as fsReadFile } from 'fs/promises'; import { Credentials, UserSettings } from 'n8n-core'; import type { IWorkflowToImport } from '@/Interfaces'; import type { ExportableCredential } from './types/exportableCredential'; -import { SharedWorkflow } from '@/databases/entities/SharedWorkflow'; -import { CredentialsEntity } from '@/databases/entities/CredentialsEntity'; import { Variables } from '@/databases/entities/Variables'; import type { ImportResult } from './types/importResult'; import { UM_FIX_INSTRUCTION } from '@/commands/BaseCommand'; import { SharedCredentials } from '@/databases/entities/SharedCredentials'; -import { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; -import { WorkflowTagMapping } from '@/databases/entities/WorkflowTagMapping'; -import { TagEntity } from '@/databases/entities/TagEntity'; +import type { WorkflowTagMapping } from '@/databases/entities/WorkflowTagMapping'; +import type { TagEntity } from '@/databases/entities/TagEntity'; import { ActiveWorkflowRunner } from '../../ActiveWorkflowRunner'; import type { SourceControllPullOptions } from './types/sourceControlPullWorkFolder'; import { In } from 'typeorm'; @@ -94,56 +92,54 @@ export class SourceControlImportService { const ownerGlobalRole = await this.getOwnerGlobalRole(); const encryptionKey = await UserSettings.getEncryptionKey(); let importCredentialsResult: Array<{ id: string; name: string; type: string }> = []; - await Db.transaction(async (transactionManager) => { - importCredentialsResult = await Promise.all( - credentialFiles.map(async (file) => { - LoggerProxy.debug(`Importing credentials file ${file}`); - const credential = jsonParse( - await fsReadFile(file, { encoding: 'utf8' }), - ); - const existingCredential = existingCredentials.find( - (e) => e.id === credential.id && e.type === credential.type, - ); - const sharedOwner = await Db.collections.SharedCredentials.findOne({ - select: ['userId'], - where: { - credentialsId: credential.id, - roleId: In([ownerCredentialRole.id, ownerGlobalRole.id]), - }, - }); + importCredentialsResult = await Promise.all( + credentialFiles.map(async (file) => { + LoggerProxy.debug(`Importing credentials file ${file}`); + const credential = jsonParse( + await fsReadFile(file, { encoding: 'utf8' }), + ); + const existingCredential = existingCredentials.find( + (e) => e.id === credential.id && e.type === credential.type, + ); + const sharedOwner = await Db.collections.SharedCredentials.findOne({ + select: ['userId'], + where: { + credentialsId: credential.id, + roleId: In([ownerCredentialRole.id, ownerGlobalRole.id]), + }, + }); - const { name, type, data, id, nodesAccess } = credential; - const newCredentialObject = new Credentials({ id, name }, type, []); - if (existingCredential?.data) { - newCredentialObject.data = existingCredential.data; - } else { - newCredentialObject.setData(data, encryptionKey); - } - newCredentialObject.nodesAccess = nodesAccess || existingCredential?.nodesAccess || []; + const { name, type, data, id, nodesAccess } = credential; + const newCredentialObject = new Credentials({ id, name }, type, []); + if (existingCredential?.data) { + newCredentialObject.data = existingCredential.data; + } else { + newCredentialObject.setData(data, encryptionKey); + } + newCredentialObject.nodesAccess = nodesAccess || existingCredential?.nodesAccess || []; - LoggerProxy.debug(`Updating credential id ${newCredentialObject.id as string}`); - await transactionManager.upsert(CredentialsEntity, newCredentialObject, ['id']); + LoggerProxy.debug(`Updating credential id ${newCredentialObject.id as string}`); + await Db.collections.Credentials.upsert(newCredentialObject, ['id']); - if (!sharedOwner) { - const newSharedCredential = new SharedCredentials(); - newSharedCredential.credentialsId = newCredentialObject.id as string; - newSharedCredential.userId = userId; - newSharedCredential.roleId = ownerGlobalRole.id; + if (!sharedOwner) { + const newSharedCredential = new SharedCredentials(); + newSharedCredential.credentialsId = newCredentialObject.id as string; + newSharedCredential.userId = userId; + newSharedCredential.roleId = ownerGlobalRole.id; - await transactionManager.upsert(SharedCredentials, { ...newSharedCredential }, [ - 'credentialsId', - 'userId', - ]); - } + await Db.collections.SharedCredentials.upsert({ ...newSharedCredential }, [ + 'credentialsId', + 'userId', + ]); + } - return { - id: newCredentialObject.id as string, - name: newCredentialObject.name, - type: newCredentialObject.type, - }; - }), - ); - }); + return { + id: newCredentialObject.id as string, + name: newCredentialObject.name, + type: newCredentialObject.type, + }; + }), + ); return importCredentialsResult.filter((e) => e !== undefined); } @@ -224,35 +220,31 @@ export class SourceControlImportService { ).map((e) => e.id), ); - await Db.transaction(async (transactionManager) => { - await Promise.all( - mappedTags.tags.map(async (tag) => { - await transactionManager.upsert( - TagEntity, - { - ...tag, - }, - { - skipUpdateIfNoValuesChanged: true, - conflictPaths: { id: true }, - }, - ); - }), - ); - await Promise.all( - mappedTags.mappings.map(async (mapping) => { - if (!existingWorkflowIds.has(String(mapping.workflowId))) return; - await transactionManager.upsert( - WorkflowTagMapping, - { tagId: String(mapping.tagId), workflowId: String(mapping.workflowId) }, - { - skipUpdateIfNoValuesChanged: true, - conflictPaths: { tagId: true, workflowId: true }, - }, - ); - }), - ); - }); + await Promise.all( + mappedTags.tags.map(async (tag) => { + await Db.collections.Tag.upsert( + { + ...tag, + }, + { + skipUpdateIfNoValuesChanged: true, + conflictPaths: { id: true }, + }, + ); + }), + ); + await Promise.all( + mappedTags.mappings.map(async (mapping) => { + if (!existingWorkflowIds.has(String(mapping.workflowId))) return; + await Db.collections.WorkflowTagMapping.upsert( + { tagId: String(mapping.tagId), workflowId: String(mapping.workflowId) }, + { + skipUpdateIfNoValuesChanged: true, + conflictPaths: { tagId: true, workflowId: true }, + }, + ); + }), + ); return mappedTags; } return { tags: [], mappings: [] }; @@ -273,74 +265,118 @@ export class SourceControlImportService { const ownerWorkflowRole = await this.getOwnerWorkflowRole(); const workflowRunner = Container.get(ActiveWorkflowRunner); - let importWorkflowsResult = new Array<{ id: string; name: string }>(); - await Db.transaction(async (transactionManager) => { - importWorkflowsResult = await Promise.all( - workflowFiles.map(async (file) => { - LoggerProxy.debug(`Parsing workflow file ${file}`); - const importedWorkflow = jsonParse( - await fsReadFile(file, { encoding: 'utf8' }), + // read owner file if it exists and map workflow ids to owner emails + // then find existing users with those emails or fallback to passed in userId + const ownerRecords: Record = {}; + const ownersFile = await glob(SOURCE_CONTROL_OWNERS_EXPORT_FILE, { + cwd: this.gitFolder, + absolute: true, + }); + if (ownersFile.length > 0) { + LoggerProxy.debug(`Reading workflow owners from file ${ownersFile[0]}`); + const ownerEmails = jsonParse>( + await fsReadFile(ownersFile[0], { encoding: 'utf8' }), + { fallbackValue: {} }, + ); + if (ownerEmails) { + const uniqueOwnerEmails = new Set(Object.values(ownerEmails)); + const existingUsers = await Db.collections.User.find({ + where: { email: In([...uniqueOwnerEmails]) }, + }); + Object.keys(ownerEmails).forEach((workflowId) => { + ownerRecords[workflowId] = + existingUsers.find((e) => e.email === ownerEmails[workflowId])?.id ?? userId; + }); + } + } + + let importWorkflowsResult = new Array<{ id: string; name: string } | undefined>(); + + const allSharedWorkflows = await Db.collections.SharedWorkflow.find({ + select: ['workflowId', 'roleId', 'userId'], + }); + + importWorkflowsResult = await Promise.all( + workflowFiles.map(async (file) => { + LoggerProxy.debug(`Parsing workflow file ${file}`); + const importedWorkflow = jsonParse( + await fsReadFile(file, { encoding: 'utf8' }), + ); + if (!importedWorkflow?.id) { + return; + } + const existingWorkflow = existingWorkflows.find((e) => e.id === importedWorkflow.id); + if (existingWorkflow?.versionId === importedWorkflow.versionId) { + LoggerProxy.debug( + `Skipping import of workflow ${importedWorkflow.id ?? 'n/a'} - versionId is up to date`, ); - const existingWorkflow = existingWorkflows.find((e) => e.id === importedWorkflow.id); - if (existingWorkflow?.versionId === importedWorkflow.versionId) { - LoggerProxy.debug( - `Skipping import of workflow ${ - importedWorkflow.id ?? 'n/a' - } - versionId is up to date`, - ); - return { - id: importedWorkflow.id ?? 'n/a', - name: 'skipped', - }; - } - LoggerProxy.debug(`Importing workflow ${importedWorkflow.id ?? 'n/a'}`); - importedWorkflow.active = existingWorkflow?.active ?? false; - LoggerProxy.debug(`Updating workflow id ${importedWorkflow.id ?? 'new'}`); - const upsertResult = await transactionManager.upsert( - WorkflowEntity, - { ...importedWorkflow }, - ['id'], - ); - if (upsertResult?.identifiers?.length !== 1) { - throw new Error(`Failed to upsert workflow ${importedWorkflow.id ?? 'new'}`); - } - // due to sequential Ids, this may have changed during the insert - // TODO: once IDs are unique and we removed autoincrement, remove this - const upsertedWorkflowId = upsertResult.identifiers[0].id as string; - await transactionManager.upsert( - SharedWorkflow, + return { + id: importedWorkflow.id ?? 'n/a', + name: 'skipped', + }; + } + LoggerProxy.debug(`Importing workflow ${importedWorkflow.id ?? 'n/a'}`); + importedWorkflow.active = existingWorkflow?.active ?? false; + LoggerProxy.debug(`Updating workflow id ${importedWorkflow.id ?? 'new'}`); + const upsertResult = await Db.collections.Workflow.upsert({ ...importedWorkflow }, ['id']); + if (upsertResult?.identifiers?.length !== 1) { + throw new Error(`Failed to upsert workflow ${importedWorkflow.id ?? 'new'}`); + } + // Update workflow owner to the user who exported the workflow, if that user exists + // in the instance, and the workflow doesn't already have an owner + const workflowOwnerId = ownerRecords[importedWorkflow.id] ?? userId; + const existingSharedWorkflowOwnerByRoleId = allSharedWorkflows.find( + (e) => e.workflowId === importedWorkflow.id && e.roleId === ownerWorkflowRole.id, + ); + const existingSharedWorkflowOwnerByUserId = allSharedWorkflows.find( + (e) => e.workflowId === importedWorkflow.id && e.userId === workflowOwnerId, + ); + if (!existingSharedWorkflowOwnerByUserId && !existingSharedWorkflowOwnerByRoleId) { + // no owner exists yet, so create one + await Db.collections.SharedWorkflow.insert({ + workflowId: importedWorkflow.id, + userId: workflowOwnerId, + roleId: ownerWorkflowRole.id, + }); + } else if (existingSharedWorkflowOwnerByRoleId) { + // skip, because the workflow already has a global owner + } else if (existingSharedWorkflowOwnerByUserId && !existingSharedWorkflowOwnerByRoleId) { + // if the worklflow has a non-global owner that is referenced by the owner file, + // and no existing global owner, update the owner to the user referenced in the owner file + await Db.collections.SharedWorkflow.update( + { + workflowId: importedWorkflow.id, + userId: workflowOwnerId, + }, { - workflowId: upsertedWorkflowId, - userId, roleId: ownerWorkflowRole.id, }, - ['workflowId', 'userId'], ); - - if (existingWorkflow?.active) { - try { - // remove active pre-import workflow - LoggerProxy.debug(`Deactivating workflow id ${existingWorkflow.id}`); - await workflowRunner.remove(existingWorkflow.id); - // try activating the imported workflow - LoggerProxy.debug(`Reactivating workflow id ${existingWorkflow.id}`); - await workflowRunner.add(existingWorkflow.id, 'activate'); - } catch (error) { - LoggerProxy.error( - `Failed to activate workflow ${existingWorkflow.id}`, - error as Error, - ); - } + } + if (existingWorkflow?.active) { + try { + // remove active pre-import workflow + LoggerProxy.debug(`Deactivating workflow id ${existingWorkflow.id}`); + await workflowRunner.remove(existingWorkflow.id); + // try activating the imported workflow + LoggerProxy.debug(`Reactivating workflow id ${existingWorkflow.id}`); + await workflowRunner.add(existingWorkflow.id, 'activate'); + } catch (error) { + LoggerProxy.error(`Failed to activate workflow ${existingWorkflow.id}`, error as Error); } + } - return { - id: importedWorkflow.id ?? 'unknown', - name: file, - }; - }), - ); - }); - return importWorkflowsResult; + return { + id: importedWorkflow.id ?? 'unknown', + name: file, + }; + }), + ); + + return importWorkflowsResult.filter((e) => e !== undefined) as Array<{ + id: string; + name: string; + }>; } async importFromWorkFolder(options: SourceControllPullOptions): Promise { diff --git a/packages/cli/src/environments/sourceControl/types/exportableWorkflow.ts b/packages/cli/src/environments/sourceControl/types/exportableWorkflow.ts index ca0f7087f9..15d405fbb7 100644 --- a/packages/cli/src/environments/sourceControl/types/exportableWorkflow.ts +++ b/packages/cli/src/environments/sourceControl/types/exportableWorkflow.ts @@ -8,6 +8,5 @@ export interface ExportableWorkflow { connections: IConnections; settings?: IWorkflowSettings; triggerCount: number; - owner: string; versionId: string; } diff --git a/packages/cli/src/environments/sourceControl/types/sourceControlledFile.ts b/packages/cli/src/environments/sourceControl/types/sourceControlledFile.ts index 12b99457b7..165621ebc6 100644 --- a/packages/cli/src/environments/sourceControl/types/sourceControlledFile.ts +++ b/packages/cli/src/environments/sourceControl/types/sourceControlledFile.ts @@ -16,4 +16,5 @@ export type SourceControlledFile = { status: SourceControlledFileStatus; location: SourceControlledFileLocation; conflict: boolean; + updatedAt: string; };