Merge pull request #21464 from testruction/master

[Lighthouse] Fixed crashes when customizing the schedule

.changeset/long-cycles-grow.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-lighthouse': patch
+---
+
+Updated README

.changeset/ninety-otters-report.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-lighthouse-backend': minor
+---
+
+Fixed crashes faced with custom schedule configuration. The configuration schema has been update to leverage the TaskScheduleDefinition interface. It is highly recommended to move the `lighthouse.shedule` and `lighthouse.timeout` respectively to `lighthouse.schedule.frequency` and `lighthouse.schedule.timeout`.

plugins/lighthouse-backend/README.md

@@ -19,13 +19,19 @@ import { PluginEnvironment } from '../types';
 import { CatalogClient } from '@backstage/catalog-client';
 
 export default async function createPlugin(env: PluginEnvironment) {
-  const { logger, scheduler, config } = env;
+  const { logger, scheduler, config, tokenManager } = env;
 
   const catalogClient = new CatalogClient({
     discoveryApi: env.discovery,
   });
 
-  await createScheduler({ logger, scheduler, config, catalogClient });
+  await createScheduler({
+    logger,
+    scheduler,
+    config,
+    catalogClient,
+    tokenManager,
+  });
 }
 ```
 
@@ -80,5 +86,8 @@ You can define how often and when the scheduler should run the audits:
 ```yaml
 lighthouse:
   schedule:
-    days: 1
+    frequency:
+      hours: 12 # Default: days 1
+    timeout:
+      minutes: 30 # Default: null
 ```

plugins/lighthouse-backend/config.d.ts

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2020 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { TaskScheduleDefinitionConfig } from '@backstage/backend-tasks';
+import { HumanDuration } from '@backstage/types';
+
+export interface Config {
+  /**
+   * Configuration options for the Lighthouse backend plugin.
+   */
+  lighthouse?: {
+    /**
+     * Schedule of the audit
+     */
+    schedule?: HumanDuration | TaskScheduleDefinitionConfig;
+  };
+}

plugins/lighthouse-backend/package.json

@@ -24,8 +24,10 @@
     "lighthouse"
   ],
   "files": [
-    "dist"
+    "dist",
+    "config.d.ts"
   ],
+  "configSchema": "config.d.ts",
   "scripts": {
     "build": "backstage-cli package build",
     "lint": "backstage-cli package lint",

plugins/lighthouse-backend/src/config.ts

@@ -13,56 +13,66 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import { LoggerService } from '@backstage/backend-plugin-api';
+import {
+  TaskScheduleDefinition,
+  readTaskScheduleDefinitionFromConfig,
+} from '@backstage/backend-tasks';
+import { Config, readDurationFromConfig } from '@backstage/config';
+import { HumanDuration } from '@backstage/types';
 
-import { Config } from '@backstage/config';
-import { HumanDuration as HumanDuration } from '@backstage/types';
-
-export interface LighthouseAuditScheduleConfig {
-  schedule: HumanDuration;
-  timeout: HumanDuration;
-  auditDetail: HumanDuration;
-}
-
-/** @public */
-export type LighthouseAuditSchedule = {
-  getSchedule: () => HumanDuration;
-  getTimeout: () => HumanDuration;
+type Options = {
+  logger: LoggerService;
 };
 
 /** @public */
-export class LighthouseAuditScheduleImpl implements LighthouseAuditSchedule {
-  static fromConfig(config: Config) {
-    const lighthouse = config.getOptionalConfig('lighthouse');
+export class LighthouseAuditScheduleImpl implements TaskScheduleDefinition {
+  /**
+   * Looks at the `lighthouse.schedule` section in the application configuration
+   * and returns a TaskScheduleDefinition.
+   * Defaults to `{ frequency: { days: 1 }, timeout: {}, initialDelay: { minutes; 15 } }`
+   *
+   * @returns a TaskScheduleDefinition
+   */
+  static fromConfig(config: Config, options: Options): TaskScheduleDefinition {
+    const { logger } = options;
 
-    let schedule: HumanDuration = { days: 1 };
-    let timeout: HumanDuration = {};
+    let lighthouse: TaskScheduleDefinition = {
+      frequency: { days: 1 },
+      timeout: { minutes: 10 },
+      initialDelay: { minutes: 15 },
+    };
 
-    if (lighthouse) {
-      const scheduleConfig = lighthouse.getOptionalConfig('schedule');
-      const timeoutConfig = lighthouse.getOptionalConfig('timeout');
+    if (config.has('lighthouse.schedule.frequency')) {
+      lighthouse = readTaskScheduleDefinitionFromConfig(
+        config.getConfig('lighthouse.schedule'),
+      );
+    } else if (config.has('lighthouse.schedule')) {
+      logger.warn(
+        `[Deprecation] Please migrate the schedule configuration to 'lighthouse.schedule.frequency'`,
+      );
 
-      if (scheduleConfig) {
-        schedule = scheduleConfig as HumanDuration;
-      }
-
-      if (timeoutConfig) {
-        timeout = timeoutConfig as HumanDuration;
-      }
+      lighthouse.frequency = readDurationFromConfig(
+        config.getConfig('lighthouse.schedule'),
+      );
     }
 
-    return new LighthouseAuditScheduleImpl(schedule, timeout);
+    if (config.has('lighthouse.timeout')) {
+      logger.warn(
+        `[Deprecation] Please migrate the timeout configuration to 'lighthouse.schedule.timeout'`,
+      );
+
+      lighthouse.timeout = readDurationFromConfig(
+        config.getConfig('lighthouse.timeout'),
+      );
+    }
+
+    return lighthouse;
   }
 
   constructor(
-    private schedule: HumanDuration,
-    private timeout: HumanDuration,
+    public frequency: HumanDuration,
+    public timeout: HumanDuration,
+    public initialDelay: HumanDuration,
   ) {}
-
-  getSchedule(): HumanDuration {
-    return this.schedule;
-  }
-
-  getTimeout(): HumanDuration {
-    return this.timeout;
-  }
 }

plugins/lighthouse-backend/src/service/createScheduler.ts

@@ -39,7 +39,9 @@ export async function createScheduler(
   const { logger, scheduler, catalogClient, config, tokenManager } = options;
   const lighthouseApi = LighthouseRestApi.fromConfig(config);
 
-  const lighthouseAuditConfig = LighthouseAuditScheduleImpl.fromConfig(config);
+  const lighthouseAuditConfig = LighthouseAuditScheduleImpl.fromConfig(config, {
+    logger,
+  });
   const formFactorToScreenEmulationMap = {
     // the default is mobile, so no need to override
     mobile: undefined,
@@ -56,16 +58,16 @@ export async function createScheduler(
 
   logger.info(
     `Running with Scheduler Config ${JSON.stringify(
-      lighthouseAuditConfig.getSchedule(),
-    )} and timeout ${JSON.stringify(lighthouseAuditConfig.getTimeout())}`,
+      lighthouseAuditConfig.frequency,
+    )} and timeout ${JSON.stringify(lighthouseAuditConfig.timeout)}`,
   );
 
   if (scheduler) {
     await scheduler.scheduleTask({
       id: 'lighthouse_audit',
-      frequency: lighthouseAuditConfig.getSchedule(),
-      timeout: lighthouseAuditConfig.getTimeout(),
-      initialDelay: { minutes: 15 },
+      frequency: lighthouseAuditConfig.frequency,
+      timeout: lighthouseAuditConfig.timeout,
+      initialDelay: lighthouseAuditConfig.initialDelay,
       fn: async () => {
         const filter: Record<string, symbol | string> = {
           kind: 'Component',

plugins/lighthouse/README.md

@@ -47,6 +47,10 @@ const routes = (
 Then configure the `lighthouse-audit-service` URL in your [`app-config.yaml`](https://github.com/backstage/backstage/blob/master/app-config.yaml).
 
 ```yaml
+backend:
+  csp:
+    frame-src:
+      - http://your-service-url
 lighthouse:
   baseUrl: http://your-service-url
 ```