fix(core): Make senderId required for all command messages (#7252)

all commands sent between main instance and workers need to contain a
server id to prevent senders from reacting to their own messages,
causing loops

this PR makes sure all sent messages contain a sender id by default as
part of constructing a sending redis client.

---------

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
This commit is contained in:
Michael Auerswald
2023-09-26 13:58:06 +02:00
committed by GitHub
parent 77d6e3fc07
commit 4b014286cf
23 changed files with 231 additions and 203 deletions

View File

@@ -4,16 +4,16 @@ import { LoggerProxy } from 'n8n-workflow';
import { getLogger } from '@/Logger';
import { OrchestrationService } from '@/services/orchestration.service';
import type { RedisServiceWorkerResponseObject } from '@/services/redis/RedisServiceCommands';
import { EventMessageWorkflow } from '@/eventbus/EventMessageClasses/EventMessageWorkflow';
import { eventBus } from '@/eventbus';
import { RedisService } from '@/services/redis.service';
import { mockInstance } from '../../integration/shared/utils';
import { handleWorkerResponseMessage } from '../../../src/services/orchestration/handleWorkerResponseMessage';
import { handleCommandMessage } from '../../../src/services/orchestration/handleCommandMessage';
import { License } from '../../../src/License';
const os = Container.get(OrchestrationService);
let queueModeId: string;
function setDefaultConfig() {
config.set('executions.mode', 'queue');
}
@@ -27,15 +27,6 @@ const workerRestartEventbusResponse: RedisServiceWorkerResponseObject = {
},
};
const eventBusMessage = new EventMessageWorkflow({
eventName: 'n8n.workflow.success',
id: 'test',
message: 'test',
payload: {
test: 'test',
},
});
describe('Orchestration Service', () => {
beforeAll(async () => {
mockInstance(RedisService);
@@ -74,6 +65,7 @@ describe('Orchestration Service', () => {
});
});
setDefaultConfig();
queueModeId = config.get('redis.queueModeId');
});
afterAll(async () => {
@@ -83,10 +75,10 @@ describe('Orchestration Service', () => {
});
test('should initialize', async () => {
await os.init('test-orchestration-service');
await os.init();
expect(os.redisPublisher).toBeDefined();
expect(os.redisSubscriber).toBeDefined();
expect(os.uniqueInstanceId).toBeDefined();
expect(queueModeId).toBeDefined();
});
test('should handle worker responses', async () => {
@@ -97,32 +89,28 @@ describe('Orchestration Service', () => {
});
test('should handle command messages from others', async () => {
const license = Container.get(License);
license.instanceId = 'test';
jest.spyOn(license, 'reload');
jest.spyOn(LoggerProxy, 'warn');
const responseFalseId = await handleCommandMessage(
JSON.stringify({
senderId: 'test',
command: 'reloadLicense',
}),
os.uniqueInstanceId,
);
expect(responseFalseId).toBeDefined();
expect(responseFalseId!.command).toEqual('reloadLicense');
expect(responseFalseId!.senderId).toEqual('test');
expect(license.reload).toHaveBeenCalled();
jest.spyOn(license, 'reload').mockRestore();
expect(LoggerProxy.warn).toHaveBeenCalled();
jest.spyOn(LoggerProxy, 'warn').mockRestore();
});
test('should reject command messages from iteslf', async () => {
jest.spyOn(eventBus, 'restart');
const response = await handleCommandMessage(
JSON.stringify({ ...workerRestartEventbusResponse, senderId: os.uniqueInstanceId }),
os.uniqueInstanceId,
JSON.stringify({ ...workerRestartEventbusResponse, senderId: queueModeId }),
);
expect(response).toBeDefined();
expect(response!.command).toEqual('restartEventBus');
expect(response!.senderId).toEqual(os.uniqueInstanceId);
expect(response!.senderId).toEqual(queueModeId);
expect(eventBus.restart).not.toHaveBeenCalled();
jest.spyOn(eventBus, 'restart').mockRestore();
});