refactor(core): More decouplings from internal hooks (no-changelog) (#10099)

This commit is contained in:
Iván Ovejero 2024-07-19 10:33:13 +02:00 committed by GitHub
parent 11db5a5b51
commit 5eca7c8e28
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 106 additions and 52 deletions

View file

@ -13,7 +13,7 @@ import { Logger } from '@/Logger';
import { jsonParse, type IDataObject, ApplicationError } from 'n8n-workflow';
import { EXTERNAL_SECRETS_INITIAL_BACKOFF, EXTERNAL_SECRETS_MAX_BACKOFF } from './constants';
import { License } from '@/License';
import { InternalHooks } from '@/InternalHooks';
import { EventRelay } from '@/eventbus/event-relay.service';
import { updateIntervalTime } from './externalSecretsHelper.ee';
import { ExternalSecretsProviders } from './ExternalSecretsProviders.ee';
import { OrchestrationService } from '@/services/orchestration.service';
@ -38,6 +38,7 @@ export class ExternalSecretsManager {
private readonly license: License,
private readonly secretsProviders: ExternalSecretsProviders,
private readonly cipher: Cipher,
private readonly eventRelay: EventRelay,
) {}
async init(): Promise<void> {
@ -308,12 +309,12 @@ export class ExternalSecretsManager {
try {
testResult = await this.getProvider(vaultType)?.test();
} catch {}
void Container.get(InternalHooks).onExternalSecretsProviderSettingsSaved({
user_id: userId,
vault_type: vaultType,
is_new: isNew,
is_valid: testResult?.[0] ?? false,
error_message: testResult?.[1],
this.eventRelay.emit('external-secrets-provider-settings-saved', {
userId,
vaultType,
isNew,
isValid: testResult?.[0] ?? false,
errorMessage: testResult?.[1],
});
}

View file

@ -756,32 +756,4 @@ export class InternalHooks {
}): Promise<void> {
return await this.telemetry.track('Workflow first data fetched', data);
}
/**
* License
*/
async onLicenseRenewAttempt(data: { success: boolean }): Promise<void> {
await this.telemetry.track('Instance attempted to refresh license', data);
}
/**
* Audit
*/
async onAuditGeneratedViaCli() {
return await this.telemetry.track('Instance generated security audit via CLI command');
}
async onVariableCreated(createData: { variable_type: string }): Promise<void> {
return await this.telemetry.track('User created variable', createData);
}
async onExternalSecretsProviderSettingsSaved(saveData: {
user_id?: string | undefined;
vault_type: string;
is_valid: boolean;
is_new: boolean;
error_message?: string | undefined;
}): Promise<void> {
return await this.telemetry.track('User updated external secrets settings', saveData);
}
}

View file

@ -7,7 +7,7 @@ import { RISK_CATEGORIES } from '@/security-audit/constants';
import config from '@/config';
import type { Risk } from '@/security-audit/types';
import { BaseCommand } from './BaseCommand';
import { InternalHooks } from '@/InternalHooks';
import { EventRelay } from '@/eventbus/event-relay.service';
export class SecurityAudit extends BaseCommand {
static description = 'Generate a security audit report for this n8n instance';
@ -62,7 +62,7 @@ export class SecurityAudit extends BaseCommand {
process.stdout.write(JSON.stringify(result, null, 2));
}
void Container.get(InternalHooks).onAuditGeneratedViaCli();
Container.get(EventRelay).emit('security-audit-generated-via-cli');
}
async catch(error: Error) {

View file

@ -1,18 +1,19 @@
import { Container, Service } from 'typedi';
import type { Variables } from '@db/entities/Variables';
import { InternalHooks } from '@/InternalHooks';
import { generateNanoId } from '@db/utils/generators';
import { canCreateNewVariable } from './environmentHelpers';
import { CacheService } from '@/services/cache/cache.service';
import { VariablesRepository } from '@db/repositories/variables.repository';
import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error';
import { VariableValidationError } from '@/errors/variable-validation.error';
import { EventRelay } from '@/eventbus/event-relay.service';
@Service()
export class VariablesService {
constructor(
protected cacheService: CacheService,
protected variablesRepository: VariablesRepository,
private readonly eventRelay: EventRelay,
) {}
async getAllCached(): Promise<Variables[]> {
@ -70,7 +71,7 @@ export class VariablesService {
}
this.validateVariable(variable);
void Container.get(InternalHooks).onVariableCreated({ variable_type: variable.type });
this.eventRelay.emit('variable-created', { variableType: variable.type });
const saveResult = await this.variablesRepository.save(
{
...variable,

View file

@ -4,7 +4,7 @@ import type { Event } from './event.types';
@Service()
export class EventRelay extends EventEmitter {
emit<K extends keyof Event>(eventName: K, arg: Event[K]) {
emit<K extends keyof Event>(eventName: K, arg?: Event[K]) {
super.emit(eventName, arg);
return true;
}

View file

@ -252,4 +252,24 @@ export type Event = {
credsPushed: number;
variablesPushed: number;
};
'license-renewal-attempted': {
success: boolean;
};
'security-audit-generated-via-cli': {
// no payload
};
'variable-created': {
variableType: string;
};
'external-secrets-provider-settings-saved': {
userId?: string;
vaultType: string;
isValid: boolean;
isNew: boolean;
errorMessage?: string;
};
};

View file

@ -3,7 +3,7 @@ import axios from 'axios';
import { Logger } from '@/Logger';
import { License } from '@/License';
import { InternalHooks } from '@/InternalHooks';
import { EventRelay } from '@/eventbus/event-relay.service';
import type { User } from '@db/entities/User';
import { WorkflowRepository } from '@db/repositories/workflow.repository';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
@ -26,9 +26,9 @@ export class LicenseService {
constructor(
private readonly logger: Logger,
private readonly license: License,
private readonly internalHooks: InternalHooks,
private readonly workflowRepository: WorkflowRepository,
private readonly urlService: UrlService,
private readonly eventRelay: EventRelay,
) {}
async getLicenseData() {
@ -78,13 +78,12 @@ export class LicenseService {
await this.license.renew();
} catch (e) {
const message = this.mapErrorMessage(e as LicenseError, 'renew');
// not awaiting so as not to make the endpoint hang
void this.internalHooks.onLicenseRenewAttempt({ success: false });
this.eventRelay.emit('license-renewal-attempted', { success: false });
throw new BadRequestError(message);
}
// not awaiting so as not to make the endpoint hang
void this.internalHooks.onLicenseRenewAttempt({ success: true });
this.eventRelay.emit('license-renewal-attempted', { success: true });
}
private mapErrorMessage(error: LicenseError, action: 'activate' | 'renew') {

View file

@ -41,6 +41,18 @@ export class TelemetryEventRelay {
this.eventRelay.on('source-control-user-finished-push-ui', (event) =>
this.sourceControlUserFinishedPushUi(event),
);
this.eventRelay.on('license-renewal-attempted', (event) => {
this.licenseRenewalAttempted(event);
});
this.eventRelay.on('security-audit-generated-via-cli', () => {
this.securityAuditGeneratedViaCli();
});
this.eventRelay.on('variable-created', (event) => {
this.variableCreated(event);
});
this.eventRelay.on('external-secrets-provider-settings-saved', (event) => {
this.externalSecretsProviderSettingsSaved(event);
});
}
private teamProjectUpdated({ userId, role, members, projectId }: Event['team-project-updated']) {
@ -153,4 +165,36 @@ export class TelemetryEventRelay {
variables_pushed: variablesPushed,
});
}
private licenseRenewalAttempted({ success }: Event['license-renewal-attempted']) {
void this.telemetry.track('Instance attempted to refresh license', {
success,
});
}
private securityAuditGeneratedViaCli() {
void this.telemetry.track('Instance generated security audit via CLI command');
}
private variableCreated({ variableType }: Event['variable-created']) {
void this.telemetry.track('User created variable', {
variable_type: variableType,
});
}
private externalSecretsProviderSettingsSaved({
userId,
vaultType,
isValid,
isNew,
errorMessage,
}: Event['external-secrets-provider-settings-saved']) {
void this.telemetry.track('User updated external secrets settings', {
user_id: userId,
vault_type: vaultType,
is_valid: isValid,
is_new: isNew,
error_message: errorMessage,
});
}
}

View file

@ -21,6 +21,7 @@ import {
TestFailProvider,
} from '../../shared/ExternalSecrets/utils';
import type { SuperAgentTest } from '../shared/types';
import type { EventRelay } from '@/eventbus/event-relay.service';
let authOwnerAgent: SuperAgentTest;
let authMemberAgent: SuperAgentTest;
@ -49,6 +50,8 @@ async function getExternalSecretsSettings(): Promise<ExternalSecretsSettings | n
return await jsonParse(Container.get(Cipher).decrypt(encSettings));
}
const eventRelay = mock<EventRelay>();
const resetManager = async () => {
Container.get(ExternalSecretsManager).shutdown();
Container.set(
@ -59,6 +62,7 @@ const resetManager = async () => {
Container.get(License),
mockProvidersInstance,
Container.get(Cipher),
eventRelay,
),
);

View file

@ -49,7 +49,14 @@ describe('External Secrets Manager', () => {
});
license.isExternalSecretsEnabled.mockReturnValue(true);
settingsRepo.getEncryptedSecretsProviderSettings.mockResolvedValue(settings);
manager = new ExternalSecretsManager(mock(), settingsRepo, license, providersMock, cipher);
manager = new ExternalSecretsManager(
mock(),
settingsRepo,
license,
providersMock,
cipher,
mock(),
);
});
afterEach(() => {

View file

@ -1,6 +1,6 @@
import { LicenseErrors, LicenseService } from '@/license/license.service';
import type { License } from '@/License';
import type { InternalHooks } from '@/InternalHooks';
import type { EventRelay } from '@/eventbus/event-relay.service';
import type { WorkflowRepository } from '@db/repositories/workflow.repository';
import type { TEntitlement } from '@n8n_io/license-sdk';
import { mock } from 'jest-mock-extended';
@ -8,10 +8,16 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error';
describe('LicenseService', () => {
const license = mock<License>();
const internalHooks = mock<InternalHooks>();
const workflowRepository = mock<WorkflowRepository>();
const entitlement = mock<TEntitlement>({ productId: '123' });
const licenseService = new LicenseService(mock(), license, internalHooks, workflowRepository);
const eventRelay = mock<EventRelay>();
const licenseService = new LicenseService(
mock(),
license,
workflowRepository,
mock(),
eventRelay,
);
license.getMainPlan.mockReturnValue(entitlement);
license.getTriggerLimit.mockReturnValue(400);
@ -61,7 +67,7 @@ describe('LicenseService', () => {
license.renew.mockResolvedValueOnce();
await licenseService.renewLicense();
expect(internalHooks.onLicenseRenewAttempt).toHaveBeenCalledWith({ success: true });
expect(eventRelay.emit).toHaveBeenCalledWith('license-renewal-attempted', { success: true });
});
test('on failure', async () => {
@ -70,7 +76,7 @@ describe('LicenseService', () => {
new BadRequestError('Activation key has expired'),
);
expect(internalHooks.onLicenseRenewAttempt).toHaveBeenCalledWith({ success: false });
expect(eventRelay.emit).toHaveBeenCalledWith('license-renewal-attempted', { success: false });
});
});
});