From 388bad21c28d7be3a54618dca84b90f8e33ec5b5 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Thu, 7 Mar 2024 08:34:14 +0000 Subject: [PATCH 1/2] fix: Always register webhooks on startup --- packages/cli/src/services/orchestration.service.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/services/orchestration.service.ts b/packages/cli/src/services/orchestration.service.ts index 45100bbe2bbb3..d80f4dee19d71 100644 --- a/packages/cli/src/services/orchestration.service.ts +++ b/packages/cli/src/services/orchestration.service.ts @@ -128,7 +128,11 @@ export class OrchestrationService { * Whether this instance may add webhooks to the `webhook_entity` table. */ shouldAddWebhooks(activationMode: WorkflowActivateMode) { - if (activationMode === 'init') return false; + // Always try to populate the webhook entity table as well as register the webhooks + // to prevent issues with users upgrading from a version < 1.15, where the webhook entity + // was cleared on shutdown to anything past 1.28.0, where we stopped populating it on init, + // causing all webhooks to break + if (activationMode === 'init') return true; if (activationMode === 'leadershipChange') return false; From faae87760a48ea4370a28015a046d55ce7b54439 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Thu, 7 Mar 2024 08:46:37 +0000 Subject: [PATCH 2/2] test: adding missing tests --- .../services/orchestration.service.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/cli/test/unit/services/orchestration.service.test.ts b/packages/cli/test/unit/services/orchestration.service.test.ts index d755c73fcb17b..35ad3a53bef57 100644 --- a/packages/cli/test/unit/services/orchestration.service.test.ts +++ b/packages/cli/test/unit/services/orchestration.service.test.ts @@ -13,6 +13,7 @@ import { Logger } from '@/Logger'; import { Push } from '@/push'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; import { mockInstance } from '../../shared/mocking'; +import type { WorkflowActivateMode } from 'n8n-workflow'; const os = Container.get(OrchestrationService); const handler = Container.get(OrchestrationHandlerMainService); @@ -149,4 +150,38 @@ describe('Orchestration Service', () => { expect(res1!.payload).toBeUndefined(); expect(res2!.payload!.result).toEqual('debounced'); }); + + describe('shouldAddWebhooks', () => { + beforeEach(() => { + config.set('multiMainSetup.instanceType', 'leader'); + }); + test('should return true for init', () => { + // We want to ensure that webhooks are populated on init + // more https://github.com/n8n-io/n8n/pull/8830 + const result = os.shouldAddWebhooks('init'); + expect(result).toBe(true); + }); + + test('should return false for leadershipChange', () => { + const result = os.shouldAddWebhooks('leadershipChange'); + expect(result).toBe(false); + }); + + test('should return true for update or activate when is leader', () => { + const modes = ['update', 'activate'] as WorkflowActivateMode[]; + for (const mode of modes) { + const result = os.shouldAddWebhooks(mode); + expect(result).toBe(true); + } + }); + + test('should return false for update or activate when not leader', () => { + config.set('multiMainSetup.instanceType', 'follower'); + const modes = ['update', 'activate'] as WorkflowActivateMode[]; + for (const mode of modes) { + const result = os.shouldAddWebhooks(mode); + expect(result).toBe(false); + } + }); + }); });