Skip to content

Commit

Permalink
refactor(core): Add external secrets log scope (#11224)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivov authored Oct 24, 2024
1 parent 475d72e commit 88c1c4a
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 11 deletions.
2 changes: 2 additions & 0 deletions packages/@n8n/config/src/configs/logging.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { StringArray } from '../utils';
/** Scopes (areas of functionality) to filter logs by. */
export const LOG_SCOPES = [
'concurrency',
'external-secrets',
'license',
'multi-main-setup',
'pubsub',
Expand Down Expand Up @@ -64,6 +65,7 @@ export class LoggingConfig {
* Supported log scopes:
*
* - `concurrency`
* - `external-secrets`
* - `license`
* - `multi-main-setup`
* - `pubsub`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
FailedProvider,
MockProviders,
} from '@test/external-secrets/utils';
import { mockInstance } from '@test/mocking';
import { mockInstance, mockLogger } from '@test/mocking';

describe('External Secrets Manager', () => {
const connectedDate = '2023-08-01T12:32:29.000Z';
Expand Down Expand Up @@ -49,7 +49,7 @@ describe('External Secrets Manager', () => {
license.isExternalSecretsEnabled.mockReturnValue(true);
settingsRepo.getEncryptedSecretsProviderSettings.mockResolvedValue(settings);
manager = new ExternalSecretsManager(
mock(),
mockLogger(),
settingsRepo,
license,
providersMock,
Expand Down
22 changes: 19 additions & 3 deletions packages/cli/src/external-secrets/external-secrets-manager.ee.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Cipher } from 'n8n-core';
import { jsonParse, type IDataObject, ApplicationError } from 'n8n-workflow';
import { jsonParse, type IDataObject, ApplicationError, ensureError } from 'n8n-workflow';
import { Service } from 'typedi';

import { SettingsRepository } from '@/databases/repositories/settings.repository';
Expand Down Expand Up @@ -39,7 +39,9 @@ export class ExternalSecretsManager {
private readonly cipher: Cipher,
private readonly eventService: EventService,
private readonly publisher: Publisher,
) {}
) {
this.logger = this.logger.scoped('external-secrets');
}

async init(): Promise<void> {
if (!this.initialized) {
Expand All @@ -57,6 +59,8 @@ export class ExternalSecretsManager {
}
return await this.initializingPromise;
}

this.logger.debug('External secrets manager initialized');
}

shutdown() {
Expand All @@ -66,6 +70,8 @@ export class ExternalSecretsManager {
void p.disconnect().catch(() => {});
});
Object.values(this.initRetryTimeouts).forEach((v) => clearTimeout(v));

this.logger.debug('External secrets manager shut down');
}

async reloadAllProviders(backoff?: number) {
Expand All @@ -77,6 +83,8 @@ export class ExternalSecretsManager {
for (const provider of providers) {
await this.reloadProvider(provider, backoff);
}

this.logger.debug('External secrets managed reloaded all providers');
}

broadcastReloadExternalSecretsProviders() {
Expand Down Expand Up @@ -191,6 +199,8 @@ export class ExternalSecretsManager {
}
}),
);

this.logger.debug('External secrets manager updated secrets');
}

getProvider(provider: string): SecretsProvider | undefined {
Expand Down Expand Up @@ -261,6 +271,8 @@ export class ExternalSecretsManager {
if (newProvider) {
this.providers[provider] = newProvider;
}

this.logger.debug(`External secrets manager reloaded provider ${provider}`);
}

async setProviderSettings(provider: string, data: IDataObject, userId?: string) {
Expand Down Expand Up @@ -382,8 +394,12 @@ export class ExternalSecretsManager {
try {
await this.providers[provider].update();
this.broadcastReloadExternalSecretsProviders();
this.logger.debug(`External secrets manager updated provider ${provider}`);
return true;
} catch {
} catch (error) {
this.logger.debug(`External secrets manager failed to update provider ${provider}`, {
error: ensureError(error),
});
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import type { INodeProperties } from 'n8n-workflow';
import Container from 'typedi';

import { UnknownAuthTypeError } from '@/errors/unknown-auth-type.error';
import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '@/external-secrets/constants';
import type { SecretsProvider, SecretsProviderState } from '@/interfaces';
import { Logger } from '@/logging/logger.service';

import { AwsSecretsClient } from './aws-secrets-client';
import type { AwsSecretsManagerContext } from './types';
Expand Down Expand Up @@ -76,20 +78,32 @@ export class AwsSecretsManager implements SecretsProvider {

private client: AwsSecretsClient;

constructor(private readonly logger = Container.get(Logger)) {
this.logger = this.logger.scoped('external-secrets');
}

async init(context: AwsSecretsManagerContext) {
this.assertAuthType(context);

this.client = new AwsSecretsClient(context.settings);

this.logger.debug('AWS Secrets Manager provider initialized');
}

async test() {
return await this.client.checkConnection();
}

async connect() {
const [wasSuccessful] = await this.test();
const [wasSuccessful, errorMsg] = await this.test();

this.state = wasSuccessful ? 'connected' : 'error';

if (wasSuccessful) {
this.logger.debug('AWS Secrets Manager provider connected');
} else {
this.logger.error('AWS Secrets Manager provider failed to connect', { errorMsg });
}
}

async disconnect() {
Expand All @@ -104,6 +118,8 @@ export class AwsSecretsManager implements SecretsProvider {
this.cachedSecrets = Object.fromEntries(
supportedSecrets.map((s) => [s.secretName, s.secretValue]),
);

this.logger.debug('AWS Secrets Manager provider secrets updated');
}

getSecret(name: string) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import type { SecretClient } from '@azure/keyvault-secrets';
import { ensureError } from 'n8n-workflow';
import type { INodeProperties } from 'n8n-workflow';
import Container from 'typedi';

import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '@/external-secrets/constants';
import type { SecretsProvider, SecretsProviderState } from '@/interfaces';
import { Logger } from '@/logging/logger.service';

import type { AzureKeyVaultContext } from './types';

Expand Down Expand Up @@ -64,8 +67,14 @@ export class AzureKeyVault implements SecretsProvider {

private settings: AzureKeyVaultContext['settings'];

constructor(private readonly logger = Container.get(Logger)) {
this.logger = this.logger.scoped('external-secrets');
}

async init(context: AzureKeyVaultContext) {
this.settings = context.settings;

this.logger.debug('Azure Key Vault provider initialized');
}

async connect() {
Expand All @@ -78,8 +87,12 @@ export class AzureKeyVault implements SecretsProvider {
const credential = new ClientSecretCredential(tenantId, clientId, clientSecret);
this.client = new SecretClient(`https://${vaultName}.vault.azure.net/`, credential);
this.state = 'connected';
} catch {
this.logger.debug('Azure Key Vault provider connected');
} catch (error) {
this.state = 'error';
this.logger.error('Azure Key Vault provider failed to connect', {
error: ensureError(error),
});
}
}

Expand Down Expand Up @@ -119,6 +132,8 @@ export class AzureKeyVault implements SecretsProvider {
acc[cur.name] = cur.value;
return acc;
}, {});

this.logger.debug('Azure Key Vault provider secrets updated');
}

getSecret(name: string) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import type { SecretManagerServiceClient as GcpClient } from '@google-cloud/secret-manager';
import { jsonParse, type INodeProperties } from 'n8n-workflow';
import { ensureError, jsonParse, type INodeProperties } from 'n8n-workflow';
import Container from 'typedi';

import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '@/external-secrets/constants';
import type { SecretsProvider, SecretsProviderState } from '@/interfaces';
import { Logger } from '@/logging/logger.service';

import type {
GcpSecretsManagerContext,
Expand Down Expand Up @@ -38,6 +40,10 @@ export class GcpSecretsManager implements SecretsProvider {

private settings: GcpSecretAccountKey;

constructor(private readonly logger = Container.get(Logger)) {
this.logger = this.logger.scoped('external-secrets');
}

async init(context: GcpSecretsManagerContext) {
this.settings = this.parseSecretAccountKey(context.settings.serviceAccountKey);
}
Expand All @@ -53,8 +59,12 @@ export class GcpSecretsManager implements SecretsProvider {
projectId,
});
this.state = 'connected';
} catch {
this.logger.debug('GCP Secrets Manager provider connected');
} catch (error) {
this.state = 'error';
this.logger.debug('GCP Secrets Manager provider failed to connect', {
error: ensureError(error),
});
}
}

Expand Down Expand Up @@ -114,6 +124,8 @@ export class GcpSecretsManager implements SecretsProvider {
if (cur) acc[cur.name] = cur.value;
return acc;
}, {});

this.logger.debug('GCP Secrets Manager provider secrets updated');
}

getSecret(name: string) {
Expand Down
7 changes: 7 additions & 0 deletions packages/cli/src/external-secrets/providers/vault.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ export class VaultProvider extends SecretsProvider {

constructor(readonly logger = Container.get(Logger)) {
super();
this.logger = this.logger.scoped('external-secrets');
}

async init(settings: SecretsProviderSettings): Promise<void> {
Expand All @@ -257,6 +258,8 @@ export class VaultProvider extends SecretsProvider {
}
return config;
});

this.logger.debug('Vault provider initialized');
}

async connect(): Promise<void> {
Expand Down Expand Up @@ -408,6 +411,7 @@ export class VaultProvider extends SecretsProvider {
kvVersion: string,
path: string,
): Promise<[string, IDataObject] | null> {
this.logger.debug(`Getting kv secrets from ${mountPath}${path} (version ${kvVersion})`);
let listPath = mountPath;
if (kvVersion === '2') {
listPath += 'metadata/';
Expand Down Expand Up @@ -441,6 +445,7 @@ export class VaultProvider extends SecretsProvider {
secretPath += path + key;
try {
const secretResp = await this.#http.get<VaultResponse<IDataObject>>(secretPath);
this.logger.debug(`Vault provider retrieved secrets from ${secretPath}`);
return [
key,
kvVersion === '2'
Expand All @@ -457,6 +462,7 @@ export class VaultProvider extends SecretsProvider {
.filter((v): v is [string, IDataObject] => v !== null),
);
const name = path.substring(0, path.length - 1);
this.logger.debug(`Vault provider retrieved kv secrets from ${name}`);
return [name, data];
}

Expand All @@ -479,6 +485,7 @@ export class VaultProvider extends SecretsProvider {
).filter((v): v is [string, IDataObject] => v !== null),
);
this.cachedSecrets = secrets;
this.logger.debug('Vault provider secrets updated');
}

async test(): Promise<[boolean] | [boolean, string]> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
MockProviders,
TestFailProvider,
} from '../../shared/external-secrets/utils';
import { mockInstance } from '../../shared/mocking';
import { mockInstance, mockLogger } from '../../shared/mocking';
import { createOwner, createUser } from '../shared/db/users';
import type { SuperAgentTest } from '../shared/types';
import { setupTestServer } from '../shared/utils';
Expand Down Expand Up @@ -52,12 +52,14 @@ async function getExternalSecretsSettings(): Promise<ExternalSecretsSettings | n

const eventService = mock<EventService>();

const logger = mockLogger();

const resetManager = async () => {
Container.get(ExternalSecretsManager).shutdown();
Container.set(
ExternalSecretsManager,
new ExternalSecretsManager(
mock(),
logger,
Container.get(SettingsRepository),
Container.get(License),
mockProvidersInstance,
Expand Down Expand Up @@ -108,6 +110,18 @@ beforeAll(async () => {
const member = await createUser();
authMemberAgent = testServer.authAgentFor(member);
config.set('userManagement.isInstanceOwnerSetUp', true);
Container.set(
ExternalSecretsManager,
new ExternalSecretsManager(
logger,
Container.get(SettingsRepository),
Container.get(License),
mockProvidersInstance,
Container.get(Cipher),
eventService,
mock(),
),
);
});

beforeEach(async () => {
Expand Down

0 comments on commit 88c1c4a

Please sign in to comment.