fix(core): Reload external secrets settings and providers from db on pubsub event (#23045)

2026-09-24 23:22:38 +08:00 · 2025-12-11 12:19:47 +01:00
parent d42654ff3f
commit 74ede64bdc
2 changed files with 99 additions and 25 deletions
@@ -210,6 +210,57 @@ describe('External Secrets Manager', () => {

 			expect(updateSecretsSpy).toHaveBeenCalledTimes(1);
 		});
+
+		test('should remove all providers when DB returns null', async () => {
+			await manager.init();
+
+			// Verify we have providers initially
+			expect(manager.getProviderNames()).toEqual(['dummy']);
+
+			const disconnectSpy = jest.spyOn(manager.getProvider('dummy')!, 'disconnect');
+
+			// Mock DB to return null (settings were deleted)
+			settingsRepo.findByKey.mockResolvedValueOnce(null);
+
+			await manager.reloadAllProviders();
+
+			// Providers should be disconnected and removed
+			expect(disconnectSpy).toHaveBeenCalledTimes(1);
+			expect(manager.getProviderNames()).toEqual([]);
+			expect(manager.hasProvider('dummy')).toBe(false);
+		});
+
+		test('should remove providers no longer in settings', async () => {
+			// Initialize with multiple providers
+			mockProvidersInstance.setProviders({
+				dummy: DummyProvider,
+				another_dummy: AnotherDummyProvider,
+			});
+
+			await manager.init();
+
+			// Verify both providers exist
+			expect(manager.getProviderNames()).toContain('dummy');
+			expect(manager.getProviderNames()).toContain('another_dummy');
+
+			const disconnectSpy = jest.spyOn(manager.getProvider('another_dummy')!, 'disconnect');
+
+			// Update settings to only include 'dummy'
+			const updatedSettings = {
+				dummy: providerSettings(),
+			};
+			settingsRepo.findByKey.mockResolvedValueOnce(
+				mock<Settings>({ value: JSON.stringify(updatedSettings) }),
+			);
+
+			await manager.reloadAllProviders();
+
+			// 'another_dummy' should be disconnected and removed
+			expect(disconnectSpy).toHaveBeenCalledTimes(1);
+			expect(manager.hasProvider('dummy')).toBe(true);
+			expect(manager.hasProvider('another_dummy')).toBe(false);
+			expect(manager.getProviderNames()).toEqual(['dummy']);
+		});
 	});

 	describe('getProviderWithSettings', () => {
@@ -47,25 +47,23 @@ export class ExternalSecretsManager implements IExternalSecretsManager {

 	async init(): Promise<void> {
 		if (this.initialized) return;
-		if (!this.initializingPromise) {
-			this.initializingPromise = (async () => {
-				try {
-					await this.internalInit();
-					this.updateInterval = setInterval(
-						async () => await this.updateSecrets(),
-						this.config.updateInterval * 1000,
-					);
-					this.initialized = true;
-				} catch (error) {
-					this.logger.error('External secrets manager failed to initialize', {
-						error: ensureError(error),
-					});
-					throw error;
-				} finally {
-					this.initializingPromise = undefined;
-				}
-			})();
-		}
+		this.initializingPromise ??= (async () => {
+			try {
+				await this.internalInit();
+				this.updateInterval = setInterval(
+					async () => await this.updateSecrets(),
+					this.config.updateInterval * 1000,
+				);
+				this.initialized = true;
+			} catch (error) {
+				this.logger.error('External secrets manager failed to initialize', {
+					error: ensureError(error),
+				});
+				throw error;
+			} finally {
+				this.initializingPromise = undefined;
+			}
+		})();
 		await this.initializingPromise;

 		this.logger.debug('External secrets manager initialized');
@@ -86,11 +84,28 @@ export class ExternalSecretsManager implements IExternalSecretsManager {
 	@OnPubSubEvent('reload-external-secrets-providers')
 	async reloadAllProviders(backoff?: number) {
 		this.logger.debug('Reloading all external secrets providers');
-		const providers = this.getProviderNames();
-		if (!providers) {
-			return;
+
+		// Refresh settings from DB to get latest changes from other instances
+		this.cachedSettings = (await this.getDecryptedSettings()) ?? {};
+
+		const newProviders = new Set(Object.keys(this.cachedSettings));
+		const existingProviders = Object.keys(this.providers);
+
+		// Disconnect and remove providers that are no longer in settings
+		for (const provider of existingProviders) {
+			if (!newProviders.has(provider)) {
+				this.logger.debug(`Removing provider ${provider} - no longer in settings`);
+				try {
+					await this.providers[provider].disconnect();
+				} catch {
+					this.logger.warn(`Error disconnecting provider ${provider} during removal`);
+				}
+				delete this.providers[provider];
+			}
 		}
-		for (const provider of providers) {
+
+		// Reload providers that are in the new settings
+		for (const provider of newProviders) {
 			await this.reloadProvider(provider, backoff);
 		}

@@ -166,7 +181,11 @@ export class ExternalSecretsManager implements IExternalSecretsManager {
 		} catch (e) {
 			try {
 				await provider.disconnect();
-			} catch {}
+			} catch {
+				this.logger.warn(
+					`Error disconnecting provider ${provider.displayName} (${provider.name}) after failed connect attempt.`,
+				);
+			}
 			this.logger.error(
 				`Error initializing secrets provider ${provider.displayName} (${provider.name}).`,
 			);
@@ -266,7 +285,11 @@ export class ExternalSecretsManager implements IExternalSecretsManager {

 	async reloadProvider(provider: string, backoff = EXTERNAL_SECRETS_INITIAL_BACKOFF) {
 		if (provider in this.providers) {
-			await this.providers[provider].disconnect();
+			try {
+				await this.providers[provider].disconnect();
+			} catch {
+				this.logger.warn(`Error disconnecting provider ${provider} during reload`);
+			}
 			delete this.providers[provider];
 		}
 		const newProvider = await this.initProvider(provider, this.cachedSettings[provider], backoff);