-
Notifications
You must be signed in to change notification settings - Fork 10
fix: auto-uninstallation of connect api plugin #1791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
170d55d
2e95be6
4d3c810
4feb452
ca2493c
e54d889
2e98e82
84504dd
da17b69
8bc5702
a2d7f62
21f110d
dac9c17
1e1a04f
76d4f5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import { existsSync } from 'node:fs'; | ||
|
|
||
| /** | ||
| * Local filesystem and env checks stay synchronous so we can branch at module load. | ||
| * @returns True if the Connect Unraid plugin is installed, false otherwise. | ||
| */ | ||
| export const isConnectPluginInstalled = () => { | ||
| if (process.env.SKIP_CONNECT_PLUGIN_CHECK === 'true') { | ||
| return true; | ||
| } | ||
| return existsSync('/boot/config/plugins/dynamix.unraid.net.plg'); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import type { ApiConfig } from '@unraid/shared/services/api-config.js'; | |
| import { ConfigFilePersister } from '@unraid/shared/services/config-file.js'; | ||
| import { csvStringToArray } from '@unraid/shared/util/data.js'; | ||
|
|
||
| import { isConnectPluginInstalled } from '@app/connect-plugin-cleanup.js'; | ||
| import { API_VERSION, PATHS_CONFIG_MODULES } from '@app/environment.js'; | ||
|
|
||
| export { type ApiConfig }; | ||
|
|
@@ -29,6 +30,13 @@ export const loadApiConfig = async () => { | |
| const apiHandler = new ApiConfigPersistence(new ConfigService()).getFileHandler(); | ||
|
|
||
| const diskConfig: Partial<ApiConfig> = await apiHandler.loadConfig(); | ||
| // Hack: cleanup stale connect plugin entry if necessary | ||
| if (!isConnectPluginInstalled()) { | ||
| diskConfig.plugins = diskConfig.plugins?.filter( | ||
| (plugin) => plugin !== 'unraid-api-plugin-connect' | ||
| ); | ||
| await apiHandler.writeConfigFile(diskConfig as ApiConfig); | ||
| } | ||
|
Comment on lines
+33
to
+39
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling for the config write operation. The Consider adding error handling: // Hack: cleanup stale connect plugin entry if necessary
if (!isConnectPluginInstalled()) {
- diskConfig.plugins = diskConfig.plugins?.filter(
- (plugin) => plugin !== 'unraid-api-plugin-connect'
- );
- await apiHandler.writeConfigFile(diskConfig as ApiConfig);
+ const originalPlugins = diskConfig.plugins ?? [];
+ const cleanedPlugins = originalPlugins.filter(
+ (plugin) => plugin !== 'unraid-api-plugin-connect'
+ );
+ if (cleanedPlugins.length !== originalPlugins.length) {
+ diskConfig.plugins = cleanedPlugins;
+ try {
+ await apiHandler.writeConfigFile(diskConfig as ApiConfig);
+ } catch (error) {
+ console.error('Failed to cleanup stale connect plugin entry:', error);
+ // Continue with startup even if cleanup fails
+ }
+ }
}🤖 Prompt for AI AgentsOptimize: Only write config if the plugin was actually removed. The current implementation writes to disk even if the Apply this diff to optimize the write operation: // Hack: cleanup stale connect plugin entry if necessary
if (!isConnectPluginInstalled()) {
- diskConfig.plugins = diskConfig.plugins?.filter(
- (plugin) => plugin !== 'unraid-api-plugin-connect'
- );
- await apiHandler.writeConfigFile(diskConfig as ApiConfig);
+ const originalPlugins = diskConfig.plugins ?? [];
+ const cleanedPlugins = originalPlugins.filter(
+ (plugin) => plugin !== 'unraid-api-plugin-connect'
+ );
+ if (cleanedPlugins.length !== originalPlugins.length) {
+ diskConfig.plugins = cleanedPlugins;
+ await apiHandler.writeConfigFile(diskConfig as ApiConfig);
+ }
}🤖 Prompt for AI Agents |
||
|
|
||
| return { | ||
| ...defaultConfig, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,9 +21,19 @@ describe('PluginManagementService', () => { | |
| if (key === 'api.plugins') { | ||
| return configStore ?? defaultValue ?? []; | ||
| } | ||
| if (key === 'api') { | ||
| return { plugins: configStore ?? defaultValue ?? [] }; | ||
| } | ||
| return defaultValue; | ||
| }), | ||
| set: vi.fn((key: string, value: unknown) => { | ||
| if (key === 'api' && typeof value === 'object' && value !== null) { | ||
| // @ts-expect-error - value is an object | ||
| if (Array.isArray(value.plugins)) { | ||
| // @ts-expect-error - value is an object | ||
| configStore = [...value.plugins]; | ||
| } | ||
| } | ||
| if (key === 'api.plugins' && Array.isArray(value)) { | ||
| configStore = [...value]; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,8 +56,7 @@ export class PluginManagementService { | |
| } | ||
| pluginSet.add(plugin); | ||
| }); | ||
| // @ts-expect-error - This is a valid config key | ||
| this.configService.set<string[]>('api.plugins', Array.from(pluginSet)); | ||
| this.updatePluginsConfig(Array.from(pluginSet)); | ||
| return added; | ||
| } | ||
|
|
||
|
|
@@ -71,11 +70,15 @@ export class PluginManagementService { | |
| const pluginSet = new Set(this.plugins); | ||
| const removed = plugins.filter((plugin) => pluginSet.delete(plugin)); | ||
| const pluginsArray = Array.from(pluginSet); | ||
| // @ts-expect-error - This is a valid config key | ||
| this.configService.set('api.plugins', pluginsArray); | ||
| this.updatePluginsConfig(pluginsArray); | ||
| return removed; | ||
| } | ||
|
|
||
| private updatePluginsConfig(plugins: string[]) { | ||
| const apiConfig = this.configService.get<ApiConfig>('api'); | ||
| this.configService.set('api', { ...apiConfig, plugins }); | ||
| } | ||
|
|
||
| /** | ||
| * Install bundle / unbundled plugins using npm or direct with the config. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,5 @@ | ||
| import { Inject, Logger, Module } from '@nestjs/common'; | ||
| import { ConfigModule, ConfigService } from '@nestjs/config'; | ||
| import { existsSync } from 'node:fs'; | ||
|
|
||
| import { execa } from 'execa'; | ||
|
|
||
| import { ConnectConfigPersister } from './config/config.persistence.js'; | ||
| import { configFeature } from './config/connect.config.js'; | ||
|
|
@@ -30,64 +27,4 @@ class ConnectPluginModule { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Fallback module keeps the export shape intact but only warns operators. | ||
| * This makes `ApiModule` safe to import even when the plugin is absent. | ||
| */ | ||
| @Module({}) | ||
| export class DisabledConnectPluginModule { | ||
| logger = new Logger(DisabledConnectPluginModule.name); | ||
| async onModuleInit() { | ||
| const removalCommand = 'unraid-api plugins remove -b unraid-api-plugin-connect'; | ||
|
|
||
| this.logger.warn( | ||
| 'Connect plugin is not installed, but is listed as an API plugin. Attempting `%s` automatically.', | ||
| removalCommand | ||
| ); | ||
|
|
||
| try { | ||
| const { stdout, stderr } = await execa('unraid-api', [ | ||
| 'plugins', | ||
| 'remove', | ||
| '-b', | ||
| 'unraid-api-plugin-connect', | ||
| ]); | ||
|
|
||
| if (stdout?.trim()) { | ||
| this.logger.debug(stdout.trim()); | ||
| } | ||
|
|
||
| if (stderr?.trim()) { | ||
| this.logger.debug(stderr.trim()); | ||
| } | ||
|
|
||
| this.logger.log( | ||
| 'Successfully completed `%s` to prune the stale connect plugin entry.', | ||
| removalCommand | ||
| ); | ||
| } catch (error) { | ||
| const message = | ||
| error instanceof Error | ||
| ? error.message | ||
| : 'Unknown error while removing stale connect plugin entry.'; | ||
| this.logger.error('Failed to run `%s`: %s', removalCommand, message); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Local filesystem and env checks stay synchronous so we can branch at module load. | ||
| */ | ||
| const isConnectPluginInstalled = () => { | ||
| if (process.env.SKIP_CONNECT_PLUGIN_CHECK === 'true') { | ||
| return true; | ||
| } | ||
| return existsSync('/boot/config/plugins/dynamix.unraid.net.plg'); | ||
| }; | ||
|
|
||
| /** | ||
| * Downstream code always imports `ApiModule`. We swap the implementation based on availability, | ||
| * avoiding dynamic module plumbing while keeping the DI graph predictable. | ||
| * Set `SKIP_CONNECT_PLUGIN_CHECK=true` in development to force the connected path. | ||
| */ | ||
| export const ApiModule = isConnectPluginInstalled() ? ConnectPluginModule : DisabledConnectPluginModule; | ||
| export const ApiModule = ConnectPluginModule; | ||
Uh oh!
There was an error while loading. Please reload this page.