From 782d5ebadc67854298f3b2355255983024d2a225 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Wed, 30 Jul 2025 08:04:54 -0400 Subject: [PATCH] fix: remove connect api plugin upon removal of Connect Unraid plugin (#1548) ## Summary by CodeRabbit * **Bug Fixes** * Improved plugin removal process on Unraid 7.2 and above by ensuring the associated API plugin component is actively uninstalled during plugin removal. * **Enhancements** * API version is now consistently set during application startup and configuration migration. * Configuration file writing logs now include detailed file paths for better traceability. * File operations now use atomic writes for increased reliability. * **Chores** * Updated dependencies to include atomic file writing support. * Removed redundant configuration persistence calls after plugin changes. --- api/package.json | 1 + .../unraid-api/config/api-config.module.ts | 12 +++- .../plugin/plugin-management.service.ts | 12 +--- packages/unraid-shared/package.json | 1 + .../src/util/config-file-handler.ts | 4 +- packages/unraid-shared/src/util/file.ts | 57 +++++++++---------- plugin/plugins/dynamix.unraid.net.plg | 5 +- pnpm-lock.yaml | 26 ++++++++- 8 files changed, 72 insertions(+), 46 deletions(-) diff --git a/api/package.json b/api/package.json index be96c9471..2abea259b 100644 --- a/api/package.json +++ b/api/package.json @@ -79,6 +79,7 @@ "@unraid/libvirt": "2.1.0", "@unraid/shared": "workspace:*", "accesscontrol": "2.2.1", + "atomically": "2.0.3", "bycontract": "2.0.11", "bytes": "3.1.2", "cache-manager": "7.0.1", diff --git a/api/src/unraid-api/config/api-config.module.ts b/api/src/unraid-api/config/api-config.module.ts index 642ebef2c..affeb2e4a 100644 --- a/api/src/unraid-api/config/api-config.module.ts +++ b/api/src/unraid-api/config/api-config.module.ts @@ -1,4 +1,4 @@ -import { Injectable, Logger, Module } from '@nestjs/common'; +import { Injectable, Logger, Module, OnApplicationBootstrap } from '@nestjs/common'; import { ConfigService, registerAs } from '@nestjs/config'; import path from 'path'; @@ -50,7 +50,10 @@ export const loadApiConfig = async () => { export const apiConfig = registerAs('api', loadApiConfig); @Injectable() -export class ApiConfigPersistence extends ConfigFilePersister { +export class ApiConfigPersistence + extends ConfigFilePersister + implements OnApplicationBootstrap +{ constructor(configService: ConfigService) { super(configService); } @@ -79,12 +82,17 @@ export class ApiConfigPersistence extends ConfigFilePersister { return createDefaultConfig(); } + async onApplicationBootstrap() { + this.configService.set('api.version', API_VERSION); + } + async migrateConfig(): Promise { const legacyConfig = this.configService.get('store.config', {}); const migrated = this.convertLegacyConfig(legacyConfig); return { ...this.defaultConfig(), ...migrated, + version: API_VERSION, }; } diff --git a/api/src/unraid-api/plugin/plugin-management.service.ts b/api/src/unraid-api/plugin/plugin-management.service.ts index d1c20db71..95e7d5c50 100644 --- a/api/src/unraid-api/plugin/plugin-management.service.ts +++ b/api/src/unraid-api/plugin/plugin-management.service.ts @@ -4,14 +4,12 @@ import { ConfigService } from '@nestjs/config'; import { ApiConfig } from '@unraid/shared/services/api-config.js'; import { DependencyService } from '@app/unraid-api/app/dependency.service.js'; -import { ApiConfigPersistence } from '@app/unraid-api/config/api-config.module.js'; @Injectable() export class PluginManagementService { constructor( private readonly configService: ConfigService<{ api: ApiConfig }, true>, - private readonly dependencyService: DependencyService, - private readonly apiConfigPersistence: ApiConfigPersistence + private readonly dependencyService: DependencyService ) {} get plugins() { @@ -20,14 +18,12 @@ export class PluginManagementService { async addPlugin(...plugins: string[]) { const added = this.addPluginToConfig(...plugins); - await this.persistConfig(); await this.installPlugins(...added); await this.dependencyService.rebuildVendorArchive(); } async removePlugin(...plugins: string[]) { const removed = this.removePluginFromConfig(...plugins); - await this.persistConfig(); await this.uninstallPlugins(...removed); await this.dependencyService.rebuildVendorArchive(); } @@ -101,17 +97,11 @@ export class PluginManagementService { async addBundledPlugin(...plugins: string[]) { const added = this.addPluginToConfig(...plugins); - await this.persistConfig(); return added; } async removeBundledPlugin(...plugins: string[]) { const removed = this.removePluginFromConfig(...plugins); - await this.persistConfig(); return removed; } - - private async persistConfig() { - return await this.apiConfigPersistence.persist(); - } } diff --git a/packages/unraid-shared/package.json b/packages/unraid-shared/package.json index 4b9ae703a..8f220c01a 100644 --- a/packages/unraid-shared/package.json +++ b/packages/unraid-shared/package.json @@ -49,6 +49,7 @@ "@nestjs/common": "11.1.5", "@nestjs/config": "4.0.2", "@nestjs/graphql": "13.1.0", + "atomically": "2.0.3", "class-validator": "0.14.2", "graphql": "16.11.0", "graphql-scalars": "1.24.2", diff --git a/packages/unraid-shared/src/util/config-file-handler.ts b/packages/unraid-shared/src/util/config-file-handler.ts index 29946e0f3..729eccc17 100644 --- a/packages/unraid-shared/src/util/config-file-handler.ts +++ b/packages/unraid-shared/src/util/config-file-handler.ts @@ -1,5 +1,5 @@ import { Logger } from "@nestjs/common"; -import { readFile, writeFile } from "node:fs/promises"; +import { readFile, writeFile } from "atomically"; import { isEqual } from "lodash-es"; import { ConfigDefinition } from "./config-definition.js"; import { fileExists } from "./file.js"; @@ -122,7 +122,7 @@ export class ConfigFileHandler { try { const data = JSON.stringify(config, null, 2); - this.logger.verbose("Writing config"); + this.logger.verbose(`Writing config to ${this.definition.configPath()}`); await writeFile(this.definition.configPath(), data); return true; } catch (error) { diff --git a/packages/unraid-shared/src/util/file.ts b/packages/unraid-shared/src/util/file.ts index 57d96b3e7..53368a153 100644 --- a/packages/unraid-shared/src/util/file.ts +++ b/packages/unraid-shared/src/util/file.ts @@ -1,8 +1,8 @@ -import { accessSync } from 'fs'; -import { access, mkdir, writeFile } from 'fs/promises'; -import { mkdirSync, writeFileSync } from 'fs'; -import { F_OK } from 'node:constants'; -import { dirname } from 'path'; +import { accessSync } from "fs"; +import { access, mkdir, writeFile } from "fs/promises"; +import { mkdirSync, writeFileSync } from "fs"; +import { F_OK } from "node:constants"; +import { dirname } from "path"; /** * Checks if a file exists asynchronously. @@ -10,9 +10,9 @@ import { dirname } from 'path'; * @returns Promise that resolves to true if file exists, false otherwise */ export const fileExists = async (path: string) => - access(path, F_OK) - .then(() => true) - .catch(() => false); + access(path, F_OK) + .then(() => true) + .catch(() => false); /** * Checks if a file exists synchronously. @@ -20,51 +20,50 @@ export const fileExists = async (path: string) => * @returns true if file exists, false otherwise */ export const fileExistsSync = (path: string) => { - try { - accessSync(path, F_OK); - return true; - } catch (error: unknown) { - return false; - } + try { + accessSync(path, F_OK); + return true; + } catch (error: unknown) { + return false; + } }; /** * Writes data to a file, creating parent directories if they don't exist. - * + * * This function ensures the directory structure exists before writing the file, * equivalent to `mkdir -p` followed by file writing. - * + * * @param path - The file path to write to * @param data - The data to write (string or Buffer) * @throws {Error} If path is invalid (null, empty, or not a string) * @throws {Error} For any file system errors (EACCES, EPERM, ENOSPC, EISDIR, etc.) */ export const ensureWrite = async (path: string, data: string | Buffer) => { - if (!path || typeof path !== 'string') { - throw new Error(`Invalid path provided: ${path}`); - } + if (!path || typeof path !== "string") { + throw new Error(`Invalid path provided: ${path}`); + } - await mkdir(dirname(path), { recursive: true }); - return await writeFile(path, data); + await mkdir(dirname(path), { recursive: true }); + return await writeFile(path, data); }; /** * Writes data to a file synchronously, creating parent directories if they don't exist. - * + * * This function ensures the directory structure exists before writing the file, * equivalent to `mkdir -p` followed by file writing. - * + * * @param path - The file path to write to * @param data - The data to write (string or Buffer) * @throws {Error} If path is invalid (null, empty, or not a string) * @throws {Error} For any file system errors (EACCES, EPERM, ENOSPC, EISDIR, etc.) */ export const ensureWriteSync = (path: string, data: string | Buffer) => { - if (!path || typeof path !== 'string') { - throw new Error(`Invalid path provided: ${path}`); - } + if (!path || typeof path !== "string") { + throw new Error(`Invalid path provided: ${path}`); + } - mkdirSync(dirname(path), { recursive: true }); - return writeFileSync(path, data); + mkdirSync(dirname(path), { recursive: true }); + return writeFileSync(path, data); }; - diff --git a/plugin/plugins/dynamix.unraid.net.plg b/plugin/plugins/dynamix.unraid.net.plg index cae025d48..6ebddb974 100755 --- a/plugin/plugins/dynamix.unraid.net.plg +++ b/plugin/plugins/dynamix.unraid.net.plg @@ -198,7 +198,10 @@ exit 0 if [ "$is_7_2_or_higher" = true ]; then echo "Unraid 7.2+ detected. Using safe removal method." - + if ! /etc/rc.d/rc.unraid-api plugins remove unraid-api-plugin-connect -b; then + echo "Warning: Failed to remove connect API plugin" + fi + # Send notification to user /usr/local/emhttp/webGui/scripts/notify \ -e "Unraid Connect" \ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 67b1f05e9..b8c916cfd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -118,6 +118,9 @@ importers: accesscontrol: specifier: 2.2.1 version: 2.2.1 + atomically: + specifier: 2.0.3 + version: 2.0.3 bycontract: specifier: 2.0.11 version: 2.0.11 @@ -721,6 +724,9 @@ importers: '@nestjs/config': specifier: 4.0.2 version: 4.0.2(@nestjs/common@11.1.5(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.1.14)(rxjs@7.8.2))(rxjs@7.8.2) + atomically: + specifier: 2.0.3 + version: 2.0.3 rxjs: specifier: 7.8.2 version: 7.8.2 @@ -6051,6 +6057,9 @@ packages: resolution: {integrity: sha512-kNOjDqAh7px0XWNI+4QbzoiR/nTkHAWNud2uvnJquD1/x5a7EQZMJT0AczqK0Qn67oY/TTQ1LbUKajZpp3I9tQ==} engines: {node: '>=8.0.0'} + atomically@2.0.3: + resolution: {integrity: sha512-kU6FmrwZ3Lx7/7y3hPS5QnbJfaohcIul5fGqf7ok+4KklIEk9tJ0C2IQPdacSbVUWv6zVHXEBWoWd6NrVMT7Cw==} + auto-bind@4.0.0: resolution: {integrity: sha512-Hdw8qdNiqdJ8LqT0iK0sVzkFbzg6fhnQqqfWhBDxcHZvU75+B+ayzTy8x+k5Ix0Y92XOhOUlx74ps+bA6BeYMQ==} engines: {node: '>=8'} @@ -12290,6 +12299,9 @@ packages: structured-clone-es@1.0.0: resolution: {integrity: sha512-FL8EeKFFyNQv5cMnXI31CIMCsFarSVI2bF0U0ImeNE3g/F1IvJQyqzOXxPBRXiwQfyBTlbNe88jh1jFW0O/jiQ==} + stubborn-fs@1.2.5: + resolution: {integrity: sha512-H2N9c26eXjzL/S/K+i/RHHcFanE74dptvvjM8iwzwbVcWY/zjBbgRqF3K0DY4+OD+uTTASTBvDoxPDaPN02D7g==} + stylehacks@7.0.5: resolution: {integrity: sha512-5kNb7V37BNf0Q3w+1pxfa+oiNPS++/b4Jil9e/kPDgrk1zjEd6uR7SZeJiYaLYH6RRSC1XX2/37OTeU/4FvuIA==} engines: {node: ^18.12.0 || ^20.9.0 || >=22.0} @@ -13516,6 +13528,9 @@ packages: resolution: {integrity: sha512-f+Gy33Oa5Z14XY9679Zze+7VFhbsQfBFXodnU2x589l4kxGM9L5Y8zETTmcMR5pWOPQyRv4Z0lNax6xCO0NSlA==} engines: {node: '>=18'} + when-exit@2.1.4: + resolution: {integrity: sha512-4rnvd3A1t16PWzrBUcSDZqcAmsUIy4minDXT/CZ8F2mVDgd65i4Aalimgz1aQkRGU0iH5eT5+6Rx2TK8o443Pg==} + which-boxed-primitive@1.1.1: resolution: {integrity: sha512-TbX3mj8n0odCBFVlY8AxkqcHASw3L60jIuF8jFP78az3C2YhmGvqbHBpAjTRH2/xqYunrJ9g1jSyjCjpoWzIAA==} engines: {node: '>= 0.4'} @@ -19249,6 +19264,11 @@ snapshots: atomic-sleep@1.0.0: {} + atomically@2.0.3: + dependencies: + stubborn-fs: 1.2.5 + when-exit: 2.1.4 + auto-bind@4.0.0: {} autoprefixer@10.4.21(postcss@8.5.6): @@ -26570,6 +26590,8 @@ snapshots: structured-clone-es@1.0.0: {} + stubborn-fs@1.2.5: {} + stylehacks@7.0.5(postcss@8.5.6): dependencies: browserslist: 4.25.0 @@ -27909,7 +27931,7 @@ snapshots: '@webassemblyjs/wasm-edit': 1.14.1 '@webassemblyjs/wasm-parser': 1.14.1 acorn: 8.15.0 - browserslist: 4.25.0 + browserslist: 4.25.1 chrome-trace-event: 1.0.4 enhanced-resolve: 5.18.1 es-module-lexer: 1.7.0 @@ -27956,6 +27978,8 @@ snapshots: wheel-gestures@2.2.48: {} + when-exit@2.1.4: {} + which-boxed-primitive@1.1.1: dependencies: is-bigint: 1.1.0