From b810344311e8dd75dbc120153d569fb166f57be4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Wed, 2 Aug 2023 15:23:46 +0200
Subject: [PATCH 1/2] Make readTaskScheduleDefinitionFromConfig properly handle
 bad inputs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fredrik Adelöw <freben@gmail.com>
---
 .changeset/twelve-seahorses-perform.md        |  5 +++
 ...adTaskScheduleDefinitionFromConfig.test.ts | 15 +++++++-
 .../readTaskScheduleDefinitionFromConfig.ts   | 37 ++++++++++++++-----
 3 files changed, 47 insertions(+), 10 deletions(-)
 create mode 100644 .changeset/twelve-seahorses-perform.md

diff --git a/.changeset/twelve-seahorses-perform.md b/.changeset/twelve-seahorses-perform.md
new file mode 100644
index 0000000000..e3c761b2a5
--- /dev/null
+++ b/.changeset/twelve-seahorses-perform.md
@@ -0,0 +1,5 @@
+---
+'@backstage/backend-plugin-api': patch
+---
+
+Make `readTaskScheduleDefinitionFromConfig` properly handle bad inputs
diff --git a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.test.ts b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.test.ts
index fc2a045614..21f51954b5 100644
--- a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.test.ts
+++ b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.test.ts
@@ -76,7 +76,7 @@ describe('readTaskScheduleDefinitionFromConfig', () => {
     );
   });
 
-  it('invalid frequency value', () => {
+  it('invalid frequency key', () => {
     const config = new ConfigReader({
       frequency: {
         invalid: 'value',
@@ -89,6 +89,19 @@ describe('readTaskScheduleDefinitionFromConfig', () => {
     );
   });
 
+  it('invalid frequency value', () => {
+    const config = new ConfigReader({
+      frequency: {
+        minutes: 'value',
+      },
+      timeout: 'PT3M',
+    });
+
+    expect(() => readTaskScheduleDefinitionFromConfig(config)).toThrow(
+      "Unable to convert config value for key 'frequency.minutes' in 'mock-config' to a number",
+    );
+  });
+
   it('frequency value with additional invalid prop', () => {
     const config = new ConfigReader({
       frequency: {
diff --git a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts
index 1448a328f8..5d5682450e 100644
--- a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts
+++ b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts
@@ -31,29 +31,48 @@ const propsOfHumanDuration = [
 ];
 
 function convertToHumanDuration(config: Config, key: string): HumanDuration {
-  const props = config.getConfig(key).keys();
-  if (!props.find(prop => propsOfHumanDuration.includes(prop))) {
+  // Ensures that the root is an object
+  const root = config.getConfig(key);
+
+  const result: Record<string, number> = {};
+  let found = false;
+  for (const prop of propsOfHumanDuration) {
+    const value = root.getOptionalNumber(prop);
+    if (value !== undefined) {
+      result[prop] = value;
+      found = true;
+    }
+  }
+
+  if (!found) {
     throw new Error(
       `HumanDuration needs at least one of: ${propsOfHumanDuration}`,
     );
   }
 
-  const invalidProps = props.filter(
-    prop => !propsOfHumanDuration.includes(prop),
-  );
+  const invalidProps = root
+    .keys()
+    .filter(prop => !propsOfHumanDuration.includes(prop));
   if (invalidProps.length > 0) {
     throw new Error(
       `HumanDuration does not contain properties: ${invalidProps}`,
     );
   }
 
-  return config.get<JsonObject>(key) as HumanDuration;
+  return result as HumanDuration;
 }
 
 function readDuration(config: Config, key: string): Duration | HumanDuration {
-  return typeof config.get(key) === 'string'
-    ? Duration.fromISO(config.getString(key))
-    : convertToHumanDuration(config, key);
+  if (typeof config.get(key) === 'string') {
+    const value = config.getString(key);
+    const duration = Duration.fromISO(value);
+    if (!duration.isValid) {
+      throw new Error(`Invalid duration: ${value}`);
+    }
+    return duration;
+  }
+
+  return convertToHumanDuration(config, key);
 }
 
 function readCronOrDuration(

From dfd1b6b2fc33c317a00f0cc95530b3b8d5f85052 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Wed, 2 Aug 2023 15:27:35 +0200
Subject: [PATCH 2/2] changeset
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fredrik Adelöw <freben@gmail.com>
---
 .../{twelve-seahorses-perform.md => seven-cougars-smash.md}     | 2 +-
 .../src/tasks/readTaskScheduleDefinitionFromConfig.ts           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename .changeset/{twelve-seahorses-perform.md => seven-cougars-smash.md} (67%)

diff --git a/.changeset/twelve-seahorses-perform.md b/.changeset/seven-cougars-smash.md
similarity index 67%
rename from .changeset/twelve-seahorses-perform.md
rename to .changeset/seven-cougars-smash.md
index e3c761b2a5..5493a724d7 100644
--- a/.changeset/twelve-seahorses-perform.md
+++ b/.changeset/seven-cougars-smash.md
@@ -1,5 +1,5 @@
 ---
-'@backstage/backend-plugin-api': patch
+'@backstage/backend-tasks': patch
 ---
 
 Make `readTaskScheduleDefinitionFromConfig` properly handle bad inputs
diff --git a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts
index 5d5682450e..b31cf68f74 100644
--- a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts
+++ b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts
@@ -15,7 +15,7 @@
  */
 
 import { Config } from '@backstage/config';
-import { HumanDuration, JsonObject } from '@backstage/types';
+import { HumanDuration } from '@backstage/types';
 import { TaskScheduleDefinition } from './types';
 import { Duration } from 'luxon';