diff --git a/pkg/rancher-desktop/assets/specs/command-api.yaml b/pkg/rancher-desktop/assets/specs/command-api.yaml index 92f9ce4f793..bd2d3399e30 100644 --- a/pkg/rancher-desktop/assets/specs/command-api.yaml +++ b/pkg/rancher-desktop/assets/specs/command-api.yaml @@ -101,7 +101,7 @@ paths: summary: >- Return a list of the check IDs for the Diagnostics category, or 404 if there is no such `category`. - Specifying an exiting category with no checks + Specifying an existing category with no checks will return status code 200 and an empty array. parameters: - in: query diff --git a/pkg/rancher-desktop/integrations/manageLinesInFile.ts b/pkg/rancher-desktop/integrations/manageLinesInFile.ts index f88237d1a44..635217e3fb5 100644 --- a/pkg/rancher-desktop/integrations/manageLinesInFile.ts +++ b/pkg/rancher-desktop/integrations/manageLinesInFile.ts @@ -6,11 +6,60 @@ export const START_LINE = '### MANAGED BY RANCHER DESKTOP START (DO NOT EDIT)'; export const END_LINE = '### MANAGED BY RANCHER DESKTOP END (DO NOT EDIT)'; const DEFAULT_FILE_MODE = 0o644; +/** + * `newErrorWithPath` returns a dynamically constructed subclass of `Error` that + * has a constructor that constructs a message using the `messageTemplate` + * function, and also sets any inputs to the function as properties on the + * resulting object. + * @param messageTemplate A function used to generate an error message, based on + * any arguments passed in as properties of an object. + * @returns A subclass of Error. + */ +function newErrorWithPath>(messageTemplate: (input: T) => string) { + const result = class extends Error { + constructor(input: T, options?: ErrorOptions) { + super(messageTemplate(input), options); + Object.assign(this, input); + } + }; + + return result as unknown as new(...args: ConstructorParameters) => (InstanceType & T); +} + +/** + * `ErrorDeterminingExtendedAttributes` signifies that we failed to determine if + * the given path contains extended attributes; to be safe, we are not managing + * this file. + */ +export const ErrorDeterminingExtendedAttributes = + newErrorWithPath(({ path }: {path: string}) => `Failed to determine if \`${ path }\` contains extended attributes`); +/** + * `ErrorHasExtendedAttributes` signifies that we were unable to process a file + * because it has extended attributes that would not have been preserved had we + * tried to edit it. + */ +export const ErrorHasExtendedAttributes = + newErrorWithPath(({ path }: {path: string}) => `Refusing to manage \`${ path }\` which has extended attributes`); +/** + * `ErrorNotRegularFile` signifies that we were unable to process a file because + * it is not a regular file (e.g. a named pipe or a device). + */ +export const ErrorNotRegularFile = + newErrorWithPath(({ path }: {path: string}) => `Refusing to manage \`${ path }\` which is neither a regular file nor a symbolic link`); +/** + * `ErrorWritingFile` signifies that we attempted to process a file but writing + * to it resulted in unexpected contents. + */ +export const ErrorWritingFile = + newErrorWithPath(({ path, backupPath }: {path: string, backupPath: string}) => `Error writing to \`${ path }\`: written contents are unexpected; see backup in \`${ backupPath }\``); + /** * Inserts/removes fenced lines into/from a file. Idempotent. * @param path The path to the file to work on. * @param desiredManagedLines The lines to insert into the file. * @param desiredPresent Whether the lines should be present. + * @throws If the file could not be managed; for example, if it has extended + * attributes, is not a regular file, or a backup exists. */ export default async function manageLinesInFile(path: string, desiredManagedLines: string[], desiredPresent: boolean): Promise { const desired = getDesiredLines(desiredManagedLines, desiredPresent); @@ -35,7 +84,7 @@ export default async function manageLinesInFile(path: string, desiredManagedLine if (fileStats.isFile()) { if (await fileHasExtendedAttributes(path)) { - throw new Error(`Refusing to manage ${ path } which has extended attributes`); + throw new ErrorHasExtendedAttributes({ path }); } const tempName = `${ path }.rd-temp`; @@ -88,13 +137,13 @@ export default async function manageLinesInFile(path: string, desiredManagedLine const actualContents = await fs.promises.readFile(path, 'utf-8'); if (!isEqual(targetContents, actualContents)) { - throw new Error(`Error writing to ${ path }: written contents are unexpected; see backup in ${ backupPath }`); + throw new ErrorWritingFile({ path, backupPath }); } await fs.promises.unlink(backupPath); } else { // Target exists, and is neither a normal file nor a symbolic link. // Return with an error. - throw new Error(`Refusing to manage ${ path } which is neither a regular file nor a symbolic link`); + throw new ErrorNotRegularFile({ path }); } } @@ -112,15 +161,15 @@ async function fileHasExtendedAttributes(filePath: string): Promise { const { list } = await import('fs-xattr'); return (await list(filePath)).length > 0; - } catch { + } catch (cause) { if (process.env.NODE_ENV === 'test' && process.env.RD_TEST !== 'e2e') { // When running unit tests, assume they do not have extended attributes. return false; } - console.error(`Failed to import fs-xattr, cannot check for extended attributes on ${ filePath }; assuming it exists.`); + console.error(`Failed to import fs-xattr, cannot check for extended attributes on ${ filePath }`); - return true; + throw new ErrorDeterminingExtendedAttributes({ path: filePath }, { cause }); } } diff --git a/pkg/rancher-desktop/integrations/pathManager.ts b/pkg/rancher-desktop/integrations/pathManager.ts index aa32e53d86c..681f9e134ee 100644 --- a/pkg/rancher-desktop/integrations/pathManager.ts +++ b/pkg/rancher-desktop/integrations/pathManager.ts @@ -7,9 +7,15 @@ export interface PathManager { /** The PathManagementStrategy that corresponds to the implementation. */ readonly strategy: PathManagementStrategy - /** Makes real any changes to the system. Should be idempotent. */ + /** + * Applies changes to the system. Should be idempotent, and should not throw + * any exceptions. + */ enforce(): Promise - /** Removes any changes that the PathManager may have made. Should be idempotent. */ + /** + * Removes any changes that the PathManager may have made. Should be + * idempotent, and should not throw any exceptions. + */ remove(): Promise } diff --git a/pkg/rancher-desktop/integrations/pathManagerImpl.ts b/pkg/rancher-desktop/integrations/pathManagerImpl.ts index 3fbbea09cf3..6e1dce43f29 100644 --- a/pkg/rancher-desktop/integrations/pathManagerImpl.ts +++ b/pkg/rancher-desktop/integrations/pathManagerImpl.ts @@ -7,8 +7,11 @@ import { Mutex } from 'async-mutex'; import manageLinesInFile from '@pkg/integrations/manageLinesInFile'; import { ManualPathManager, PathManagementStrategy, PathManager } from '@pkg/integrations/pathManager'; import mainEvents from '@pkg/main/mainEvents'; +import Logging from '@pkg/utils/logging'; import paths from '@pkg/utils/paths'; +const console = Logging['path-management']; + /** * RcFilePathManager is for when the user wants Rancher Desktop to * make changes to their PATH by putting lines that change it in their @@ -32,15 +35,36 @@ export class RcFilePathManager implements PathManager { } async enforce(): Promise { - await this.managePosix(true); - await this.manageCsh(true); - await this.manageFish(true); + try { + await this.managePosix(true); + await this.manageCsh(true); + await this.manageFish(true); + } catch (error) { + console.error(error); + } } async remove(): Promise { - await this.managePosix(false); - await this.manageCsh(false); - await this.manageFish(false); + try { + await this.managePosix(false); + await this.manageCsh(false); + await this.manageFish(false); + } catch (error) { + console.error(error); + } + } + + /** + * Call manageFilesInLine, wrapped in calls to trigger diagnostics updates. + */ + protected async manageLinesInFile(fileName: string, filePath: string, lines: string[], desiredPresent: boolean) { + try { + await manageLinesInFile(filePath, lines, desiredPresent); + mainEvents.emit('diagnostics-event', 'path-management', { fileName, error: undefined }); + } catch (error: any) { + mainEvents.emit('diagnostics-event', 'path-management', { fileName, error }); + throw error; + } } /** @@ -51,7 +75,8 @@ export class RcFilePathManager implements PathManager { protected async managePosix(desiredPresent: boolean): Promise { await this.posixMutex.runExclusive(async() => { const pathLine = `export PATH="${ paths.integration }:$PATH"`; - // Note: order is important here. Only the first one that is present is modified. + // Note: order is important here. Only the first one has the PATH added; + // all others have it removed. const bashLoginShellFiles = [ '.bash_profile', '.bash_login', @@ -70,35 +95,38 @@ export class RcFilePathManager implements PathManager { await fs.promises.stat(filePath); } catch (error: any) { if (error.code === 'ENOENT') { + // If the file does not exist, it is not an error. + mainEvents.emit('diagnostics-event', 'path-management', { fileName, error: undefined }); continue; } + mainEvents.emit('diagnostics-event', 'path-management', { fileName, error }); throw error; } - await manageLinesInFile(filePath, [pathLine], desiredPresent); + await this.manageLinesInFile(fileName, filePath, [pathLine], !linesAdded); linesAdded = true; - break; } // If none of the files exist, write .bash_profile if (!linesAdded) { - const filePath = path.join(os.homedir(), bashLoginShellFiles[0]); + const fileName = bashLoginShellFiles[0]; + const filePath = path.join(os.homedir(), fileName); - await manageLinesInFile(filePath, [pathLine], desiredPresent); + await this.manageLinesInFile(fileName, filePath, [pathLine], true); } } else { // Ensure lines are not present in any of the files - await Promise.all(bashLoginShellFiles.map((fileName) => { + await Promise.all(bashLoginShellFiles.map(async(fileName) => { const filePath = path.join(os.homedir(), fileName); - return manageLinesInFile(filePath, [], desiredPresent); + await this.manageLinesInFile(fileName, filePath, [], false); })); } // Handle other shells' rc files and .bashrc - await Promise.all(['.bashrc', '.zshrc'].map((rcName) => { - const rcPath = path.join(os.homedir(), rcName); + await Promise.all(['.bashrc', '.zshrc'].map((fileName) => { + const rcPath = path.join(os.homedir(), fileName); - return manageLinesInFile(rcPath, [pathLine], desiredPresent); + return this.manageLinesInFile(fileName, rcPath, [pathLine], desiredPresent); })); mainEvents.invoke('diagnostics-trigger', 'RD_BIN_IN_BASH_PATH'); @@ -110,10 +138,10 @@ export class RcFilePathManager implements PathManager { await this.cshMutex.runExclusive(async() => { const pathLine = `setenv PATH "${ paths.integration }"\\:"$PATH"`; - await Promise.all(['.cshrc', '.tcshrc'].map((rcName) => { - const rcPath = path.join(os.homedir(), rcName); + await Promise.all(['.cshrc', '.tcshrc'].map((fileName) => { + const rcPath = path.join(os.homedir(), fileName); - return manageLinesInFile(rcPath, [pathLine], desiredPresent); + return this.manageLinesInFile(fileName, rcPath, [pathLine], desiredPresent); })); }); } @@ -122,11 +150,12 @@ export class RcFilePathManager implements PathManager { await this.fishMutex.runExclusive(async() => { const pathLine = `set --export --prepend PATH "${ paths.integration }"`; const configHome = process.env['XDG_CONFIG_HOME'] || path.join(os.homedir(), '.config'); + const fileName = 'config.fish'; const fishConfigDir = path.join(configHome, 'fish'); - const fishConfigPath = path.join(fishConfigDir, 'config.fish'); + const fishConfigPath = path.join(fishConfigDir, fileName); await fs.promises.mkdir(fishConfigDir, { recursive: true, mode: 0o700 }); - await manageLinesInFile(fishConfigPath, [pathLine], desiredPresent); + await this.manageLinesInFile(fileName, fishConfigPath, [pathLine], desiredPresent); }); } } diff --git a/pkg/rancher-desktop/main/diagnostics/diagnostics.ts b/pkg/rancher-desktop/main/diagnostics/diagnostics.ts index ef5402520bd..d492f733130 100644 --- a/pkg/rancher-desktop/main/diagnostics/diagnostics.ts +++ b/pkg/rancher-desktop/main/diagnostics/diagnostics.ts @@ -1,4 +1,4 @@ -import { DiagnosticsCategory, DiagnosticsChecker, DiagnosticsCheckerResult } from './types'; +import { DiagnosticsCategory, DiagnosticsChecker, DiagnosticsCheckerResult, DiagnosticsCheckerSingleResult } from './types'; import mainEvents from '@pkg/main/mainEvents'; import Logging from '@pkg/utils/logging'; @@ -38,7 +38,7 @@ export class DiagnosticsManager { lastUpdate = new Date(0); /** Last known check results, indexed by the checker id. */ - results: Record = {}; + results: Record = {}; /** Mapping of category name to diagnostic ids */ readonly checkerIdByCategory: Partial> = {}; @@ -52,6 +52,7 @@ export class DiagnosticsManager { import('./kubeContext'), import('./limaDarwin'), import('./mockForScreenshots'), + import('./pathManagement'), import('./rdBinInShell'), import('./testCheckers'), import('./wslFromStore'), @@ -92,9 +93,10 @@ export class DiagnosticsManager { } protected async applicableCheckers(categoryName: string | null, id: string | null): Promise { + const checkerId = id?.split(':', 1)[0]; const checkers = (await this.checkers) .filter(checker => categoryName ? checker.category === categoryName : true) - .filter(checker => id ? checker.id === id : true); + .filter(checker => checkerId ? checker.id === checkerId : true); return (await Promise.all(checkers.map(async(checker) => { try { @@ -124,12 +126,25 @@ export class DiagnosticsManager { return { last_update: this.lastUpdate.toISOString(), checks: checkers - .map(checker => ({ - ...this.results[checker.id], - id: checker.id, - category: checker.category, - mute: false, - })), + .flatMap((checker) => { + const result = this.results[checker.id]; + + if (Array.isArray(result)) { + return result.map(result => ({ + ...result, + id: `${ checker.id }:${ result.id }`, + category: checker.category, + mute: false, + })); + } else { + return { + ...result, + id: checker.id, + category: checker.category, + mute: false, + }; + } + }), }; } @@ -139,8 +154,16 @@ export class DiagnosticsManager { protected async runChecker(checker: DiagnosticsChecker) { console.debug(`Running check ${ checker.id }`); try { - this.results[checker.id] = await checker.check(); - console.debug(`Check ${ checker.id } result: ${ JSON.stringify(this.results[checker.id]) }`); + const result = await checker.check(); + + this.results[checker.id] = result; + if (Array.isArray(result)) { + for (const singleResult of result) { + console.debug(`Check ${ checker.id }:${ singleResult.id } result: ${ JSON.stringify(singleResult) }`); + } + } else { + console.debug(`Check ${ checker.id } result: ${ JSON.stringify(result) }`); + } } catch (e) { console.error(`ERROR checking ${ checker.id }`, { e }); } diff --git a/pkg/rancher-desktop/main/diagnostics/kubeConfigSymlink.ts b/pkg/rancher-desktop/main/diagnostics/kubeConfigSymlink.ts index 883897d8c51..10b67ee7845 100644 --- a/pkg/rancher-desktop/main/diagnostics/kubeConfigSymlink.ts +++ b/pkg/rancher-desktop/main/diagnostics/kubeConfigSymlink.ts @@ -27,7 +27,7 @@ const CheckKubeConfigSymlink: DiagnosticsChecker = { id: 'VERIFY_WSL_INTEGRATION_KUBECONFIG', category: DiagnosticsCategory.Kubernetes, applicable() { - return Promise.resolve(true); + return Promise.resolve(process.platform === 'win32'); }, async check() { return Promise.resolve({ diff --git a/pkg/rancher-desktop/main/diagnostics/pathManagement.ts b/pkg/rancher-desktop/main/diagnostics/pathManagement.ts new file mode 100644 index 00000000000..89c9eadf709 --- /dev/null +++ b/pkg/rancher-desktop/main/diagnostics/pathManagement.ts @@ -0,0 +1,61 @@ +import { DiagnosticsCategory, DiagnosticsChecker, DiagnosticsCheckerResult, DiagnosticsCheckerSingleResult } from './types'; + +import { ErrorHasExtendedAttributes, ErrorNotRegularFile, ErrorWritingFile } from '@pkg/integrations/manageLinesInFile'; +import mainEvents from '@pkg/main/mainEvents'; + +const cachedResults: Record = {}; + +/** + * Check for any errors raised from handling path management (i.e. handling of + * ~/.bashrc and related files) and report them to the user. + */ +const CheckPathManagement: DiagnosticsChecker = { + id: 'PATH_MANAGEMENT', + category: DiagnosticsCategory.Utilities, + applicable() { + return Promise.resolve(['darwin', 'linux'].includes(process.platform)); + }, + check(): Promise { + return Promise.resolve(Object.entries(cachedResults).map(([id, result]) => { + return ({ + ...result, + id, + }); + })); + }, +}; + +mainEvents.on('diagnostics-event', (id, state) => { + if (id !== 'path-management') { + return; + } + const typedState: { fileName: string, error: Error | undefined } = state; + const { fileName, error } = typedState; + + cachedResults[fileName] = { + description: error?.message ?? error?.toString() ?? `Unknown error managing ${ fileName }`, + passed: false, + fixes: [], + ...(() => { + if (!error) { + return { passed: true, description: `\`${ fileName }\` is managed` }; + } + + if (error instanceof ErrorHasExtendedAttributes) { + return { fixes: [{ description: `Remove extended attributes from \`${ fileName }\`` }] }; + } + + if (error instanceof ErrorNotRegularFile) { + return { fixes: [{ description: `Replace \`${ fileName }\` with a regular file` }] }; + } + + if (error instanceof ErrorWritingFile) { + return { fixes: [{ description: `Restore \`${ fileName }\` from backup file \`${ error.backupPath }\`` }] }; + } + + return {}; + })(), + }; +}); + +export default CheckPathManagement; diff --git a/pkg/rancher-desktop/main/diagnostics/types.ts b/pkg/rancher-desktop/main/diagnostics/types.ts index 0b311bb3392..cf677c015e8 100644 --- a/pkg/rancher-desktop/main/diagnostics/types.ts +++ b/pkg/rancher-desktop/main/diagnostics/types.ts @@ -16,9 +16,9 @@ type DiagnosticsFix = { * checker. */ export type DiagnosticsCheckerResult = { - /* Link to documentation about this check. */ + /** Link to documentation about this check. */ documentation?: string, - /* User-visible markdown description about this check. */ + /** User-visible markdown description about this check. */ description: string, /** If true, the check succeeded (no fixes need to be applied). */ passed: boolean, @@ -26,17 +26,31 @@ export type DiagnosticsCheckerResult = { fixes: DiagnosticsFix[], }; +export type DiagnosticsCheckerSingleResult = DiagnosticsCheckerResult & { + /** + * For checkers returning multiple results, each result must have its own + * identifier that is consistent over checker runs. + */ + id: string; +}; + /** - * DiagnosticsChecker describes an implementation of a single diagnostics check. + * DiagnosticsChecker describes an implementation of a diagnostics checker. + * The checker may return one or more results. */ export interface DiagnosticsChecker { /** Unique identifier for this check. */ id: string; category: DiagnosticsCategory, - /** Whether this checker should be used on this system. */ + /** + * Whether any of the checks this checker supports should be used on this + * system. + */ applicable(): Promise, /** - * Perform the check. + * Perform the check. If this checker does multiple checks, any checks that + * are not applicable on this system should be skipped (rather than returning + * a failing result). */ - check(): Promise; + check(): Promise; } diff --git a/pkg/rancher-desktop/main/mainEvents.ts b/pkg/rancher-desktop/main/mainEvents.ts index 9f8c81d81c5..f33c3742729 100644 --- a/pkg/rancher-desktop/main/mainEvents.ts +++ b/pkg/rancher-desktop/main/mainEvents.ts @@ -98,7 +98,14 @@ interface MainEventNames { * @note This does not update the last run time (since it only runs a single * checker). */ - 'diagnostics-trigger'(id: string): DiagnosticsCheckerResult | undefined; + 'diagnostics-trigger'(id: string): DiagnosticsCheckerResult|DiagnosticsCheckerResult[] | undefined; + + /** + * Generically signify that a diagnostic should be updated. + * @param id The diagnostic identifier. + * @param state The new state for the diagnostic. + */ + 'diagnostics-event'(id: K, state: DiagnosticsEventPayload[K]): void; /** * Emitted when an extension is uninstalled via the extension manager. @@ -137,6 +144,14 @@ interface MainEventNames { 'dialog-info'(args: Record): void; } +/** + * DiagnosticsEventPayload defines the data that will be passed on a + * 'diagnostics-event' event. + */ +type DiagnosticsEventPayload = { + 'path-management': { fileName: string; error: Error | undefined }; +}; + /** * Helper type definition to check if the given event name is a handler (i.e. * has a return value) instead of an event (i.e. returns void). diff --git a/pkg/rancher-desktop/main/tray.ts b/pkg/rancher-desktop/main/tray.ts index 024b10fa0df..0a284d7ae3e 100644 --- a/pkg/rancher-desktop/main/tray.ts +++ b/pkg/rancher-desktop/main/tray.ts @@ -203,8 +203,11 @@ export class Tray { // updates the network status in the tray if there's a change in the network // state. this.networkInterval = setInterval(async() => { - const networkDiagnostic = await mainEvents.invoke('diagnostics-trigger', 'CONNECTED_TO_INTERNET'); + let networkDiagnostic = await mainEvents.invoke('diagnostics-trigger', 'CONNECTED_TO_INTERNET'); + if (Array.isArray(networkDiagnostic)) { + networkDiagnostic = networkDiagnostic.shift(); + } if (this.networkState === networkDiagnostic?.passed) { return; // network state hasn't changed since last check }