Skip to content

Commit 7bc583b

Browse files
pujitmpujitm
authored
fix: remote access lifecycle during boot & shutdown (#1422)
## Summary - **New Features** - Introduced a comprehensive Nginx control script for Unraid OS, enabling advanced server management, SSL certificate handling, and dynamic configuration based on system state. unraid/webgui#2269 - Added a utility function to safely execute code with error handling support. - **Improvements** - Enhanced logging across remote access, WAN access, and settings services for improved traceability. - Added initialization and cleanup hooks to remote access and UPnP services for better lifecycle management. - Optimized configuration persistence by batching rapid changes for more efficient updates. - Refined URL resolution logic for improved configuration retrieval and error handling. - Broadened pattern matching for domain keys in Nginx state parsing. - Updated remote access settings to reload the network stack after changes. - Simplified remote access and WAN port configuration logic for clarity and accuracy. - Improved port mapping logic with explicit error handling in UPnP service. - Updated UI and form controls for remote access settings to reflect SSL requirements and access type restrictions. - **Configuration** - Updated the default path for module configuration files. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent b8035c2 commit 7bc583b

File tree

14 files changed

+227
-141
lines changed

14 files changed

+227
-141
lines changed

‎api/src/environment.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,4 @@ export const PM2_PATH = join(import.meta.dirname, '../../', 'node_modules', 'pm2
101101
export const ECOSYSTEM_PATH = join(import.meta.dirname, '../../', 'ecosystem.config.json');
102102

103103
export const PATHS_CONFIG_MODULES =
104-
process.env.PATHS_CONFIG_MODULES ?? '/usr/local/unraid-api/config/modules';
104+
process.env.PATHS_CONFIG_MODULES ?? '/boot/config/plugins/dynamix.my.servers/configs';

‎api/src/graphql/resolvers/mutation/connect/setup-remote-access.ts‎

Whitespace-only changes.

‎api/src/store/state-parsers/nginx.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import type { IniStringBooleanOrAuto } from '@app/core/types/ini.js';
22
import type { StateFileToIniParserMap } from '@app/store/types.js';
33
import { type FqdnEntry } from '@app/core/types/states/nginx.js';
44

5-
// Allow upper or lowercase FQDN6
6-
const fqdnRegex = /^nginx(.*?)fqdn6?$/i;
5+
// Allow upper or lowercase FQDN6, with optional separators
6+
const fqdnRegex = /^nginx[_-]?(.+?)fqdn6?$/i;
77

88
export type NginxIni = {
99
nginxCertname: string;

‎api/src/unraid-api/graph/resolvers/settings/settings.resolver.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ export class UnifiedSettingsResolver {
8989
): Promise<UpdateSettingsResponse> {
9090
this.logger.verbose('Updating Settings %O', input);
9191
const { restartRequired, values } = await this.userSettings.updateNamespacedValues(input);
92+
this.logger.verbose('Updated Setting Values %O', values);
9293
if (restartRequired) {
93-
this.logger.verbose('Will restart %O', values);
9494
// hack: allow time for pending writes to flush
9595
this.lifecycleService.restartApi({ delayMs: 300 });
9696
}

‎packages/unraid-api-plugin-connect/src/event-handler/wan-access.handler.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,30 @@
1-
import { Injectable, OnModuleDestroy } from '@nestjs/common';
1+
import { Injectable, Logger } from '@nestjs/common';
22
import { ConfigService } from '@nestjs/config';
33
import { OnEvent } from '@nestjs/event-emitter';
44

55
import { EVENTS } from '../helper/nest-tokens.js';
66
import { ConfigType } from '../model/connect-config.model.js';
77
import { NetworkService } from '../service/network.service.js';
8-
import { UrlResolverService } from '../service/url-resolver.service.js';
98

109
@Injectable()
11-
export class WanAccessEventHandler implements OnModuleDestroy {
10+
export class WanAccessEventHandler {
11+
private readonly logger = new Logger(WanAccessEventHandler.name);
12+
1213
constructor(
1314
private readonly configService: ConfigService<ConfigType>,
1415
private readonly networkService: NetworkService
1516
) {}
1617

17-
async onModuleDestroy() {
18-
await this.disableWanAccess();
19-
}
20-
2118
@OnEvent(EVENTS.ENABLE_WAN_ACCESS, { async: true })
2219
async enableWanAccess() {
20+
this.logger.log('Enabling WAN Access');
2321
this.configService.set('connect.config.wanaccess', true);
2422
await this.networkService.reloadNetworkStack();
2523
}
2624

2725
@OnEvent(EVENTS.DISABLE_WAN_ACCESS, { async: true })
2826
async disableWanAccess() {
27+
this.logger.log('Disabling WAN Access');
2928
this.configService.set('connect.config.wanaccess', false);
3029
await this.networkService.reloadNetworkStack();
3130
}

‎packages/unraid-api-plugin-connect/src/service/config.persistence.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@ import { plainToInstance } from 'class-transformer';
99
import { validateOrReject } from 'class-validator';
1010
import { parse as parseIni } from 'ini';
1111
import { isEqual } from 'lodash-es';
12-
import { debounceTime } from 'rxjs/operators';
12+
import { bufferTime } from 'rxjs/operators';
1313

1414
import type { MyServersConfig as LegacyConfig } from '../model/my-servers-config.model.js';
1515
import { ConfigType, MyServersConfig } from '../model/connect-config.model.js';
1616

1717
@Injectable()
1818
export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy {
19-
constructor(private readonly configService: ConfigService<ConfigType>) {}
19+
constructor(private readonly configService: ConfigService<ConfigType, true>) {}
2020

2121
private logger = new Logger(ConnectConfigPersister.name);
2222
get configPath() {
@@ -33,10 +33,10 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy {
3333
this.logger.verbose(`Config path: ${this.configPath}`);
3434
await this.loadOrMigrateConfig();
3535
// Persist changes to the config.
36-
this.configService.changes$.pipe(debounceTime(25)).subscribe({
37-
next: async ({ newValue, oldValue, path }) => {
38-
if (path.startsWith('connect.config')) {
39-
this.logger.verbose(`Config changed: ${path} from ${oldValue} to ${newValue}`);
36+
this.configService.changes$.pipe(bufferTime(25)).subscribe({
37+
next: async (changes) => {
38+
const connectConfigChanged = changes.some(({ path }) => path.startsWith('connect.config'));
39+
if (connectConfigChanged) {
4040
await this.persist();
4141
}
4242
},
@@ -60,7 +60,7 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy {
6060
return false;
6161
}
6262
} catch (error) {
63-
this.logger.error(`Error loading config (will overwrite file):`, error);
63+
this.logger.error(error, `Error loading config (will overwrite file)`);
6464
}
6565
const data = JSON.stringify(config, null, 2);
6666
this.logger.verbose(`Persisting config to ${this.configPath}: ${data}`);
@@ -69,7 +69,7 @@ export class ConnectConfigPersister implements OnModuleInit, OnModuleDestroy {
6969
this.logger.verbose(`Config persisted to ${this.configPath}`);
7070
return true;
7171
} catch (error) {
72-
this.logger.error(`Error persisting config to '${this.configPath}':`, error);
72+
this.logger.error(error, `Error persisting config to '${this.configPath}'`);
7373
return false;
7474
}
7575
}

‎packages/unraid-api-plugin-connect/src/service/connect-settings.service.ts‎

Lines changed: 84 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Injectable, Logger } from '@nestjs/common';
22
import { ConfigService } from '@nestjs/config';
33
import { EventEmitter2 } from '@nestjs/event-emitter';
44

5-
import type { SchemaBasedCondition } from '@jsonforms/core';
5+
import type { JsonSchema7, SchemaBasedCondition } from '@jsonforms/core';
66
import type { DataSlice, SettingSlice, UIElement } from '@unraid/shared/jsonforms/settings.js';
77
import { RuleEffect } from '@jsonforms/core';
88
import { createLabeledControl } from '@unraid/shared/jsonforms/control.js';
@@ -26,6 +26,7 @@ import { ConfigType, MyServersConfig } from '../model/connect-config.model.js';
2626
import { DynamicRemoteAccessType, WAN_ACCESS_TYPE, WAN_FORWARD_TYPE } from '../model/connect.model.js';
2727
import { ConnectApiKeyService } from './connect-api-key.service.js';
2828
import { DynamicRemoteAccessService } from './dynamic-remote-access.service.js';
29+
import { NetworkService } from './network.service.js';
2930

3031
declare module '@unraid/shared/services/user-settings.js' {
3132
interface UserSettings {
@@ -40,7 +41,8 @@ export class ConnectSettingsService {
4041
private readonly remoteAccess: DynamicRemoteAccessService,
4142
private readonly apiKeyService: ConnectApiKeyService,
4243
private readonly eventEmitter: EventEmitter2,
43-
private readonly userSettings: UserSettingsService
44+
private readonly userSettings: UserSettingsService,
45+
private readonly networkService: NetworkService
4446
) {
4547
this.userSettings.register('remote-access', {
4648
buildSlice: async () => this.buildRemoteAccessSlice(),
@@ -215,7 +217,7 @@ export class ConnectSettingsService {
215217
forwardType?: WAN_FORWARD_TYPE | undefined | null
216218
): DynamicRemoteAccessType {
217219
// If access is disabled or always, DRA is disabled
218-
if (accessType === WAN_ACCESS_TYPE.DISABLED || accessType === WAN_ACCESS_TYPE.ALWAYS) {
220+
if (accessType === WAN_ACCESS_TYPE.DISABLED) {
219221
return DynamicRemoteAccessType.DISABLED;
220222
}
221223
// if access is enabled and forward type is UPNP, DRA is UPNP, otherwise it is static
@@ -231,10 +233,10 @@ export class ConnectSettingsService {
231233
);
232234

233235
this.configService.set('connect.config.wanaccess', input.accessType === WAN_ACCESS_TYPE.ALWAYS);
234-
this.configService.set(
235-
'connect.config.wanport',
236-
input.forwardType === WAN_FORWARD_TYPE.STATIC ? input.port : null
237-
);
236+
if (input.forwardType === WAN_FORWARD_TYPE.STATIC) {
237+
this.configService.set('connect.config.wanport', input.port);
238+
// when forwarding with upnp, the upnp service will clear & set the wanport as necessary
239+
}
238240
this.configService.set(
239241
'connect.config.upnpEnabled',
240242
input.forwardType === WAN_FORWARD_TYPE.UPNP
@@ -251,17 +253,15 @@ export class ConnectSettingsService {
251253
},
252254
});
253255

256+
await this.networkService.reloadNetworkStack();
257+
254258
return true;
255259
}
256260

257261
public async dynamicRemoteAccessSettings(): Promise<RemoteAccess> {
258262
const config = this.configService.getOrThrow<MyServersConfig>('connect.config');
259263
return {
260-
accessType: config.wanaccess
261-
? config.dynamicRemoteAccessType !== DynamicRemoteAccessType.DISABLED
262-
? WAN_ACCESS_TYPE.DYNAMIC
263-
: WAN_ACCESS_TYPE.ALWAYS
264-
: WAN_ACCESS_TYPE.DISABLED,
264+
accessType: config.wanaccess ? WAN_ACCESS_TYPE.ALWAYS : WAN_ACCESS_TYPE.DISABLED,
265265
forwardType: config.upnpEnabled ? WAN_FORWARD_TYPE.UPNP : WAN_FORWARD_TYPE.STATIC,
266266
port: config.wanport ? Number(config.wanport) : null,
267267
};
@@ -272,9 +272,48 @@ export class ConnectSettingsService {
272272
*------------------------------------------------------------------------**/
273273

274274
async buildRemoteAccessSlice(): Promise<SettingSlice> {
275-
return mergeSettingSlices([await this.remoteAccessSlice()], {
276-
as: 'remote-access',
277-
});
275+
const slice = await this.remoteAccessSlice();
276+
/**------------------------------------------------------------------------
277+
* UX: Only validate 'port' when relevant
278+
*
279+
* 'port' will be null when remote access is disabled, and it's irrelevant
280+
* when using upnp (because it becomes read-only for the end-user).
281+
*
282+
* In these cases, we should omit type and range validation for 'port'
283+
* to avoid confusing end-users.
284+
*
285+
* But, when using static port forwarding, 'port' is required, so we validate it.
286+
*------------------------------------------------------------------------**/
287+
return {
288+
properties: {
289+
'remote-access': {
290+
type: 'object',
291+
properties: slice.properties as JsonSchema7['properties'],
292+
allOf: [
293+
{
294+
if: {
295+
properties: {
296+
forwardType: { const: WAN_FORWARD_TYPE.STATIC },
297+
accessType: { const: WAN_ACCESS_TYPE.ALWAYS },
298+
},
299+
required: ['forwardType', 'accessType'],
300+
},
301+
then: {
302+
required: ['port'],
303+
properties: {
304+
port: {
305+
type: 'number',
306+
minimum: 1,
307+
maximum: 65535,
308+
},
309+
},
310+
},
311+
},
312+
],
313+
},
314+
},
315+
elements: slice.elements,
316+
};
278317
}
279318

280319
buildFlashBackupSlice(): SettingSlice {
@@ -289,7 +328,8 @@ export class ConnectSettingsService {
289328
async remoteAccessSlice(): Promise<SettingSlice> {
290329
const isSignedIn = await this.isSignedIn();
291330
const isSSLCertProvisioned = await this.isSSLCertProvisioned();
292-
const precondition = isSignedIn && isSSLCertProvisioned;
331+
const { sslEnabled } = this.configService.getOrThrow('store.emhttp.nginx');
332+
const precondition = isSignedIn && isSSLCertProvisioned && sslEnabled;
293333

294334
/** shown when preconditions are not met */
295335
const requirements: UIElement[] = [
@@ -315,6 +355,10 @@ export class ConnectSettingsService {
315355
text: 'You have provisioned a valid SSL certificate',
316356
status: isSSLCertProvisioned,
317357
},
358+
{
359+
text: 'SSL is enabled',
360+
status: sslEnabled,
361+
},
318362
],
319363
},
320364
},
@@ -353,19 +397,30 @@ export class ConnectSettingsService {
353397
},
354398
},
355399
rule: {
356-
effect: RuleEffect.SHOW,
400+
effect: RuleEffect.DISABLE,
357401
condition: {
402+
scope: '#/properties/remote-access',
358403
schema: {
359-
properties: {
360-
forwardType: {
361-
enum: [WAN_FORWARD_TYPE.STATIC],
404+
anyOf: [
405+
{
406+
properties: {
407+
accessType: {
408+
const: WAN_ACCESS_TYPE.DISABLED,
409+
},
410+
},
411+
required: ['accessType'],
362412
},
363-
accessType: {
364-
enum: [WAN_ACCESS_TYPE.DYNAMIC, WAN_ACCESS_TYPE.ALWAYS],
413+
{
414+
properties: {
415+
forwardType: {
416+
const: WAN_FORWARD_TYPE.UPNP,
417+
},
418+
},
419+
required: ['forwardType'],
365420
},
366-
},
421+
],
367422
},
368-
} as Omit<SchemaBasedCondition, 'scope'>,
423+
},
369424
},
370425
}),
371426
];
@@ -374,22 +429,22 @@ export class ConnectSettingsService {
374429
const properties: DataSlice = {
375430
accessType: {
376431
type: 'string',
377-
enum: Object.values(WAN_ACCESS_TYPE),
432+
enum: [WAN_ACCESS_TYPE.DISABLED, WAN_ACCESS_TYPE.ALWAYS],
378433
title: 'Allow Remote Access',
379-
default: 'DISABLED',
434+
default: WAN_ACCESS_TYPE.DISABLED,
380435
},
381436
forwardType: {
382437
type: 'string',
383438
enum: Object.values(WAN_FORWARD_TYPE),
384439
title: 'Forward Type',
385-
default: 'STATIC',
440+
default: WAN_FORWARD_TYPE.STATIC,
386441
},
387442
port: {
388-
type: 'number',
443+
// 'port' is null when remote access is disabled.
444+
type: ['number', 'null'],
389445
title: 'WAN Port',
390446
minimum: 0,
391447
maximum: 65535,
392-
default: 0,
393448
},
394449
};
395450

0 commit comments

Comments
 (0)