Repository: ioBroker/ioBroker.js-controller (ts)

Issue:
Allow "randomization of schedules" for schedule adapters
We should find a way to randomize the defaut schedule for schedule adapters, if the start is not required to be exactly at a special time; and especially when external systems are queried (see yr and tankerkoenig case).

We could allow placeholders like "X * * * *" in the io-package config where X is then on instance creation being replaced by a random number. We also could be able to find solutions to allow "every 5 minutes" schedules.

We could also allow (or by default?) delay the start within the minute by some random seconds on start to make sure the run is also spread in the respective minute.
[Feature] randomize scheduled adapters per default
Scheduled adapters using cloud access should have renadomized start times. Currently this is implemented per adapter (or not at all).

Please evaluate the following feature:

If an adapter is scheduled, randomize the starttime within one minute (+0 to +59 seconds) unless an adapter explicitly states "do not randomize".

This functionality could eventually limited to adapters specifiying cloud access.
Disabling this feature could be done either by an new option inside io-package oder by applying the randomization only if no seconds are specified at cron data.

In addition (gold version) randomization could be extended to a range explicitly specified at cron value (i.e +/-0:0:5)

In general - the goal of this feature request is to support users with randomization and to change the default to randamization as this is a repeating issue / feedback at adapter review.

Pull request:
[feature]: implemented random delay for schedule adapters
**Link the feature issue which is closed by this PR**
<!--
If the PR closes an issue add a `closes #issue-no`. If no issue exists yet, please create an issue first to discuss with the core team if this feature is desirable.
-->
- closes #2513
- closes #1839

**Implementation details**
<!--
    What has been changed?
-->

If an `schedule` adapter is scheduled and it has not specified a specific argument for `seconds` in the cron tab the contoller will automatically add seconds for the next execution with a value between 0 - 59.

**Tests**
- [x] I have added tests to test this feature
- [ ] It is not possible to test this feature

**Documentation**
<!--
    New features should be documented in the `README.md` file under `Feature Overview`. If a host message was added, please document it under `Feature Overview/js-controller Host Messages`.
-->
- [x] I have documented the new feature

Patch:
diff --git a/README.md b/README.md
index 473901500..7b0c98a3b 100644
--- a/README.md
+++ b/README.md
@@ -665,6 +665,10 @@ ioBroker supports multiple Adapter modes. These are:
 * `once`:      The adapter ist started only once after it's object got modified. No restarting happens after the adapter stops.
 * `none`:      The adapter is officially not having any process, but could be a webExtension (so iis included by a web instance on the same host or is only running client side and so offering www files)
 
+> **_NOTE:_** Up from controller v6 for adapters of type `schedule` and if `connectionType` is set to `cloud`
+>
+> If the schedule consists of a CRON tab without specifically specifying seconds, the execution will be delayed randomly up to 60 seconds. This is to prevent unwanted DDoS attacks. If your adapter needs to exactly run at a specified second, please specify the second in your CRON tab so no delay will be introduced by the controller.
+
 #### Start adapter instances as normal processes
 **Feature status:** stable
 
diff --git a/package.json b/package.json
index 1e504b4b1..c6f3d0b75 100644
--- a/package.json
+++ b/package.json
@@ -93,7 +93,7 @@
     "test-jsonl": "mocha packages/controller/test/jsonl/*.ts --exit",
     "test-redis-socket": "mocha packages/controller/test/redis-socket/*.ts --exit",
     "test-redis-sentinel": "mocha packages/controller/test/redis-sentinel/*.ts --exit",
-    "test-types-check": "tsc --project packages/controller/test/tsconfig.json",
+    "test-types-check": "tsc --project packages/controller/tsconfig.check.json",
     "test-types": "npm run test-types-check && npm run test --workspace=@iobroker/types"
   }
 }
diff --git a/packages/controller/src/lib/utils.ts b/packages/controller/src/lib/utils.ts
index c3d44fa68..48a2c3f6c 100644
--- a/packages/controller/src/lib/utils.ts
+++ b/packages/controller/src/lib/utils.ts
@@ -15,3 +15,30 @@ export function getDiskWarningLevel(state: ioBroker.State): number {
 
 /** Default value for disk warning level */
 export const DEFAULT_DISK_WARNING_LEVEL = 5;
+
+interface GetCronExpressionOptions {
+    /** The cron expression in the schedule */
+    cronExpression: string;
+    /** The connection type of the instance */
+    connectionType?: ioBroker.ConnectionType;
+}
+
+/** Max time in seconds to delay an instance start if no seconds specified, has to be below 60 */
+const MAX_SCHEDULE_DELAY = 59;
+
+/**
+ * Delay the instance start if the expression has no seconds, and it is a cloud connection
+ *
+ * @param options information about the cron expression and connection type
+ */
+export function getCronExpression(options: GetCronExpressionOptions): string {
+    const { cronExpression, connectionType } = options;
+    const cronHasSeconds = cronExpression.split(' ').length > 5;
+
+    if (cronHasSeconds || connectionType !== 'cloud') {
+        return cronExpression;
+    }
+
+    const randomSeconds = Math.round(Math.random() * MAX_SCHEDULE_DELAY);
+    return `${randomSeconds} ${cronExpression}`;
+}
diff --git a/packages/controller/src/main.ts b/packages/controller/src/main.ts
index 26aa250c1..a938f4b92 100644
--- a/packages/controller/src/main.ts
+++ b/packages/controller/src/main.ts
@@ -38,7 +38,7 @@ import decache from 'decache';
 import cronParser from 'cron-parser';
 import type { PluginHandlerSettings } from '@iobroker/plugin-base/types';
 import type { GetDiskInfoResponse } from '@iobroker/js-controller-common/tools';
-import { DEFAULT_DISK_WARNING_LEVEL, getDiskWarningLevel } from '@/lib/utils.js';
+import { DEFAULT_DISK_WARNING_LEVEL, getCronExpression, getDiskWarningLevel } from '@/lib/utils.js';
 import { AdapterAutoUpgradeManager } from '@/lib/adapterAutoUpgradeManager.js';
 import {
     getHostObject,
@@ -3601,8 +3601,9 @@ function cleanErrors(procObj: Process, now: number | null, doOutput?: boolean):
 }
 
 /**
+ * Start an instance of type schedule right now
  *
- * @param callback
+ * @param callback optional callback function
  */
 async function startScheduledInstance(callback?: () => void): Promise<void> {
     const idsToStart = Object.keys(scheduledInstances);
@@ -4638,15 +4639,21 @@ async function startInstance(id: ioBroker.ObjectIDs.Instance, wakeUp = false): P
                 break;
             }
 
-            proc.schedule = schedule.scheduleJob(instance.common.schedule, () => {
-                // queue up, but only if not already queued
-                scheduledInstances[id] = {
-                    fileNameFull: adapterMainFile,
-                    adapterDir,
-                    wakeUp
-                };
-                Object.keys(scheduledInstances).length === 1 && startScheduledInstance();
-            });
+            proc.schedule = schedule.scheduleJob(
+                getCronExpression({
+                    cronExpression: instance.common.schedule,
+                    connectionType: instance.common.connectionType
+                }),
+                () => {
+                    // queue up, but only if not already queued
+                    scheduledInstances[id] = {
+                        fileNameFull: adapterMainFile,
+                        adapterDir,
+                        wakeUp
+                    };
+                    Object.keys(scheduledInstances).length === 1 && startScheduledInstance();
+                }
+            );
             logger.info(`${hostLogPrefix} instance scheduled ${instance._id} ${instance.common.schedule}`);
             // Start one time adapter by start or if configuration changed
             if (instance.common.allowInit) {
diff --git a/packages/controller/tsconfig.build.json b/packages/controller/tsconfig.build.json
index 418b71383..ebb555c53 100644
--- a/packages/controller/tsconfig.build.json
+++ b/packages/controller/tsconfig.build.json
@@ -2,6 +2,7 @@
 {
   "extends": "./tsconfig.json",
   "compilerOptions": {
+    "rootDir": "src",
     "allowJs": true,
     "checkJs": false,
     "noEmit": false
@@ -43,6 +44,7 @@
     "src/**/*.js" // currently, we have js too
   ],
   "exclude": [
+    "test/**",
     "src/**/*.test.ts",
     "build/**",
     "node_modules/**"
diff --git a/packages/controller/tsconfig.check.json b/packages/controller/tsconfig.check.json
new file mode 100644
index 000000000..7ed1afcc8
--- /dev/null
+++ b/packages/controller/tsconfig.check.json
@@ -0,0 +1,4 @@
+// specialized config for type checking (test folder included)
+{
+  "extends": "./tsconfig.json",
+}
\ No newline at end of file
diff --git a/packages/controller/tsconfig.json b/packages/controller/tsconfig.json
index a872118ae..6ee8eaffd 100644
--- a/packages/controller/tsconfig.json
+++ b/packages/controller/tsconfig.json
@@ -2,7 +2,6 @@
   "extends": "../../tsconfig.json",
   "compilerOptions": {
     "outDir": "build/esm",
-    "rootDir": "src",
     "module": "NodeNext",
     "paths": {
       "@/*": [
@@ -43,6 +42,7 @@
     }
   ],
   "include": [
+    "test/**/*.ts",
     "src/**/*.ts",
     "src/**/*.js"
   ],
diff --git a/packages/types-dev/objects.d.ts b/packages/types-dev/objects.d.ts
index 3a8cb6f12..edbbd9d83 100644
--- a/packages/types-dev/objects.d.ts
+++ b/packages/types-dev/objects.d.ts
@@ -563,6 +563,8 @@ declare global {
             align?: 'left' | 'center' | 'right';
         }
 
+        type ConnectionType = 'local' | 'cloud';
+
         interface AdapterCommon extends ObjectCommon {
             /** Custom attributes to be shown in admin in the object browser */
             adminColumns?: string | (string | CustomAdminColumn)[];
@@ -588,7 +590,7 @@ declare global {
             /** Whether this adapter includes custom blocks for Blockly. If true, `admin/blockly.js` must exist. */
             blockly?: boolean;
             /** Where the adapter will get its data from. Set this together with @see dataSource */
-            connectionType?: 'local' | 'cloud';
+            connectionType?: ConnectionType;
             /** If true, this adapter can be started in compact mode (in the same process as other adpaters) */
             compact?: boolean;
             /** The directory relative to iobroker-data where the adapter stores the data. Supports the placeholder `%INSTANCE%`. This folder will be backed up and restored automatically. */


Test patch:
diff --git a/packages/controller/test/testInternalUtilities.ts b/packages/controller/test/testInternalUtilities.ts
new file mode 100644
index 000000000..3b984df16
--- /dev/null
+++ b/packages/controller/test/testInternalUtilities.ts
@@ -0,0 +1,23 @@
+import { getCronExpression } from '../src/lib/utils.js';
+import { expect } from 'chai';
+
+describe('test internal helpers', () => {
+    it('getCronExpression', () => {
+        const cronWithoutSeconds = '15 * * * *';
+        const cronWithSeconds = '3 15 * * * *';
+
+        const cronSecondsAdded = getCronExpression({ cronExpression: cronWithoutSeconds, connectionType: 'cloud' });
+        expect(cronSecondsAdded).to.be.not.equal(cronWithoutSeconds);
+        expect(cronSecondsAdded.split(' ').length).to.be.equal(6);
+
+        const cronNothingAdded = getCronExpression({ cronExpression: cronWithSeconds, connectionType: 'cloud' });
+        expect(cronNothingAdded).to.be.equal(cronWithSeconds);
+
+        // no delay for connection types different from cloud
+        const cronSecondsWrongConnectionType = getCronExpression({
+            cronExpression: cronWithoutSeconds,
+            connectionType: 'local'
+        });
+        expect(cronSecondsWrongConnectionType).to.be.equal(cronWithoutSeconds);
+    });
+});
diff --git a/packages/controller/test/tsconfig.json b/packages/controller/test/tsconfig.json
deleted file mode 100644
index 6dfa78640..000000000
--- a/packages/controller/test/tsconfig.json
+++ /dev/null
@@ -1,12 +0,0 @@
-{
-  // own json config, for test files root is different, just using ts-node no build
-  "extends": "../tsconfig.json",
-  "compilerOptions": {
-    "rootDir": "."
-  },
-  "include": [
-    "**/*.ts"
-  ],
-  "exclude": [
-  ]
-}