Skip to content

fix: rm short-circuit in rc.unraid-api if plugin config dir is absent #1515

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 15, 2025
Merged

fix: rm short-circuit in rc.unraid-api if plugin config dir is absent #1515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d08f875
fix: rm short-circuit in `rc.unraid-api` if plugin config dir is absent
pujitm Jul 14, 2025
fc7bce7
refactor: remove dependency on env file from flash drive
pujitm Jul 14, 2025
c410fc0
refactor: `switch-env` to update `.env` contents
pujitm Jul 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion api/src/__test__/store/modules/__snapshots__/paths.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ exports[`Returns paths 1`] = `
"myservers-base",
"myservers-config",
"myservers-config-states",
"myservers-env",
"myservers-keepalive",
"keyfile-base",
"machine-id",
Expand Down
1 change: 0 additions & 1 deletion api/src/__test__/store/modules/paths.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ test('Returns paths', async () => {
'myservers-base': '/boot/config/plugins/dynamix.my.servers/',
'myservers-config': expect.stringContaining('api/dev/Unraid.net/myservers.cfg'),
'myservers-config-states': expect.stringContaining('api/dev/states/myservers.cfg'),
'myservers-env': '/boot/config/plugins/dynamix.my.servers/env',
'myservers-keepalive': './dev/Unraid.net/fb_keepalive',
'keyfile-base': expect.stringContaining('api/dev/Unraid.net'),
'machine-id': expect.stringContaining('api/dev/data/machine-id'),
Expand Down
2 changes: 2 additions & 0 deletions api/src/environment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export const getPackageJsonDependencies = (): string[] | undefined => {

export const API_VERSION = process.env.npm_package_version ?? getPackageJson().version;

/** Controls how the app is built/run (i.e. in terms of optimization) */
export const NODE_ENV =
(process.env.NODE_ENV as 'development' | 'test' | 'staging' | 'production') ?? 'production';
export const environment = {
Expand All @@ -76,6 +77,7 @@ export const CHOKIDAR_USEPOLLING = process.env.CHOKIDAR_USEPOLLING === 'true';
export const IS_DOCKER = process.env.IS_DOCKER === 'true';
export const DEBUG = process.env.DEBUG === 'true';
export const INTROSPECTION = process.env.INTROSPECTION === 'true';
/** Determines the app-level & business logic environment (i.e. what data & infrastructure is used) */
export const ENVIRONMENT = process.env.ENVIRONMENT
? (process.env.ENVIRONMENT as 'production' | 'staging' | 'development')
: 'production';
Expand Down
1 change: 0 additions & 1 deletion api/src/store/modules/paths.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const initialState = {
resolvePath(process.env.PATHS_STATES ?? ('/usr/local/emhttp/state/' as const)),
'myservers.cfg' as const
),
'myservers-env': '/boot/config/plugins/dynamix.my.servers/env' as const,
'myservers-keepalive':
process.env.PATHS_MY_SERVERS_FB ??
('/boot/config/plugins/dynamix.my.servers/fb_keepalive' as const),
Expand Down
77 changes: 30 additions & 47 deletions api/src/unraid-api/cli/switch-env.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { copyFile, readFile, writeFile } from 'fs/promises';
import { copyFile } from 'fs/promises';
import { join } from 'path';

import { Command, CommandRunner, Option } from 'nest-commander';

import { cliLogger } from '@app/core/log.js';
import { fileExistsSync } from '@app/core/utils/files/file-exists.js';
import { ENVIRONMENT } from '@app/environment.js';
import { getters } from '@app/store/index.js';
import { LogService } from '@app/unraid-api/cli/log.service.js';
import { StartCommand } from '@app/unraid-api/cli/start.command.js';
import { StopCommand } from '@app/unraid-api/cli/stop.command.js';
import { RestartCommand } from '@app/unraid-api/cli/restart.command.js';

interface SwitchEnvOptions {
environment?: 'staging' | 'production';
Expand All @@ -31,60 +31,43 @@ export class SwitchEnvCommand extends CommandRunner {

constructor(
private readonly logger: LogService,
private readonly stopCommand: StopCommand,
private readonly startCommand: StartCommand
private readonly restartCommand: RestartCommand
) {
super();
}

private async getEnvironmentFromFile(path: string): Promise<'production' | 'staging'> {
const envFile = await readFile(path, 'utf-8').catch(() => '');
this.logger.debug(`Checking ${path} for current ENV, found ${envFile}`);

// Match the env file env="production" which would be [0] = env="production", [1] = env and [2] = production
const matchArray = /([a-zA-Z]+)=["]*([a-zA-Z]+)["]*/.exec(envFile);
// Get item from index 2 of the regex match or return production
const [, , currentEnvInFile] = matchArray && matchArray.length === 3 ? matchArray : [];
return this.parseStringToEnv(currentEnvInFile);
}

private switchToOtherEnv(environment: 'production' | 'staging'): 'production' | 'staging' {
if (environment === 'production') {
return 'staging';
}
return 'production';
}

async run(_, options: SwitchEnvOptions): Promise<void> {
const paths = getters.paths();
const basePath = paths['unraid-api-base'];
const envFlashFilePath = paths['myservers-env'];
const currentEnvPath = join(basePath, '.env');

this.logger.warn('Stopping the Unraid API');
try {
await this.stopCommand.run([], { delete: false });
} catch (err) {
this.logger.warn('Failed to stop the Unraid API (maybe already stopped?)');
}

const newEnv =
options.environment ??
this.switchToOtherEnv(await this.getEnvironmentFromFile(envFlashFilePath));
this.logger.info(`Setting environment to ${newEnv}`);
// Determine target environment
const currentEnv = ENVIRONMENT;
const targetEnv = options.environment ?? 'production';
Comment on lines +44 to +46
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify the default environment behavior.

The command now defaults to 'production' when no environment is specified. Ensure this aligns with user expectations, as the previous implementation may have toggled between environments.

🏁 Script executed:

#!/bin/bash
# Description: Check for documentation or tests that describe the expected default behavior

# Search for documentation about switch-env command behavior
fd -e md -e txt | xargs rg -A 5 -B 5 "switch-env"

# Check for tests that validate the default behavior
fd -e test.ts -e spec.ts | xargs rg -A 10 "switch-env.*default|switch-env.*no.*argument"

Length of output: 10821

Ensure switch-env toggles by default when no environment is specified

The CLI docs and developer guide show that running
unraid-api switch-env without -e should toggle between production and staging, not always set to production. Update the default assignment in api/src/unraid-api/cli/switch-env.command.ts:

• File: api/src/unraid-api/cli/switch-env.command.ts
Lines: 44–46

Replace:

// Determine target environment
const currentEnv = ENVIRONMENT;
const targetEnv = options.environment ?? 'production';

With:

// Determine target environment (toggle if none provided)
const currentEnv = ENVIRONMENT;
const targetEnv = options.environment
  ?? (currentEnv === 'production' ? 'staging' : 'production');

This aligns the behavior with:

  • api/docs/public/cli.md: “Switch between production and staging environments”
  • api/docs/developer/development.md: step 2 runs switch-env without args to switch environments.
🤖 Prompt for AI Agents 
In api/src/unraid-api/cli/switch-env.command.ts around lines 44 to 46, the code
currently defaults the target environment to 'production' when no environment
option is provided, but it should toggle between 'production' and 'staging'
instead. Update the assignment of targetEnv to check if options.environment is
set; if not, set targetEnv to 'staging' when currentEnv is 'production',
otherwise set it to 'production'. This change will make the CLI toggle
environments by default as documented.


// Write new env to flash
const newEnvLine = `env="${newEnv}"`;
this.logger.debug('Writing %s to %s', newEnvLine, envFlashFilePath);
await writeFile(envFlashFilePath, newEnvLine);
this.logger.info(`Switching environment from ${currentEnv} to ${targetEnv}`);

// Copy the new env over to live location before restarting
const source = join(basePath, `.env.${newEnv}`);
const destination = join(basePath, '.env');
// Check if target environment file exists
const sourceEnvPath = join(basePath, `.env.${targetEnv}`);
if (!fileExistsSync(sourceEnvPath)) {
this.logger.error(
`Environment file ${sourceEnvPath} does not exist. Cannot switch to ${targetEnv} environment.`
);
process.exit(1);
}

cliLogger.debug('Copying %s to %s', source, destination);
await copyFile(source, destination);
// Copy the target environment file to .env
this.logger.debug(`Copying ${sourceEnvPath} to ${currentEnvPath}`);
try {
await copyFile(sourceEnvPath, currentEnvPath);
this.logger.info(`Successfully switched to ${targetEnv} environment`);
} catch (error) {
this.logger.error(`Failed to copy environment file: ${error}`);
process.exit(1);
}

cliLogger.info('Now using %s', newEnv);
await this.startCommand.run([], {});
// Restart the API to pick up the new environment
this.logger.info('Restarting Unraid API to apply environment changes...');
await this.restartCommand.run();
}
}
2 changes: 1 addition & 1 deletion ...ile-modifier/modifications/__test__/__fixtures__/downloaded/.login.php.last-download-time
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1751630630443
1752524464371
2 changes: 1 addition & 1 deletion ...r/modifications/__test__/__fixtures__/downloaded/DefaultPageLayout.php.last-download-time
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1751630630198
1752524464066
2 changes: 1 addition & 1 deletion ...fier/modifications/__test__/__fixtures__/downloaded/Notifications.page.last-download-time
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1751630630343
1752524464213
2 changes: 1 addition & 1 deletion ...difier/modifications/__test__/__fixtures__/downloaded/auth-request.php.last-download-time
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1751630630571
1752524464631
2 changes: 1 addition & 1 deletion ...-file-modifier/modifications/__test__/__fixtures__/downloaded/rc.nginx.last-download-time
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1751630630810
1752524464761
3 changes: 0 additions & 3 deletions plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
# shellcheck source=/dev/null
source /etc/profile

flash="/boot/config/plugins/dynamix.my.servers"
[[ ! -d "${flash}" ]] && echo "Please reinstall the Unraid Connect plugin" && exit 1
[[ ! -f "${flash}/env" ]] && echo 'env=production' >"${flash}/env"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elibosley @ljm42 is it preferable to still write this env file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for flash backup? I think we probably should nuke it if possible and just have flash backup work off production by default.

unraid_binary_path="/usr/local/bin/unraid-api"
api_base_dir="/usr/local/unraid-api"
scripts_dir="/usr/local/share/dynamix.unraid.net/scripts"
Expand Down