Merge pull request #513 from snyk/feat/ED-215_conditional-preflight-c…

…hecks-loading

feat: load rest-api-status preflight check only if high availability mode is enabled [ED-251]

lib/client/checks/http/http-checks.ts

@@ -1,5 +1,6 @@
 import { Config } from '../../config';
 import type { Check } from '../types';
+import { highAvailabilityModeEnabled } from '../../dispatcher';
 
 const defaultBrokerServerUrl = 'https://broker.snyk.io';
 const defaultApiBaseUrl = 'https://api.snyk.io';
@@ -10,16 +11,22 @@ export function createBrokerServerHealthcheck(config: Config): Check {
   return {
     checkId: 'broker-server-status',
     checkName: 'Broker Server Healthcheck',
+    active: true,
     url: `${url}/healthcheck`,
     timeoutMs: defaultTimeoutMs,
   };
 }
 
 export function createRestApiHealthcheck(config: Config): Check {
   const url = config.API_BASE_URL || defaultApiBaseUrl;
+
+  // check is enabled only for high availability mode
+  const enabled = highAvailabilityModeEnabled(config);
+
   return {
     checkId: 'rest-api-status',
     checkName: 'REST API Healthcheck',
+    active: enabled,
     url: `${url}/rest/openapi`,
     timeoutMs: defaultTimeoutMs,
   };

lib/client/checks/preflight-check-store.ts

@@ -12,10 +12,11 @@ export class PreflightCheckStore implements CheckStore {
 
   async get(checkId: CheckId): Promise<Check | null> {
     const entry = this.preflightChecks.find((c) => c.checkId === checkId);
-    return Promise.resolve(entry !== undefined ? entry : null);
+    return Promise.resolve(entry !== undefined && entry.active ? entry : null);
   }
 
   async getAll(): Promise<Check[]> {
-    return [...this.preflightChecks];
+    const activeChecks = this.preflightChecks.filter((entry) => entry.active);
+    return [...activeChecks];
   }
 }

lib/client/checks/types.ts

@@ -6,6 +6,7 @@ export interface Check {
   checkName: string;
   checkStatus?: CheckStatus;
 
+  active: boolean;
   output?: string;
   timeoutMs: number;
   url: string;

lib/client/config.ts

@@ -10,4 +10,15 @@ interface BrokerServer {
   BROKER_SERVER_URL: string;
 }
 
-export type Config = BackendAPI & BrokerClient & BrokerServer;
+/**
+ * Configuration options for HA (high-availability) mode.
+ */
+interface HighAvailabilityMode {
+  BROKER_DISPATCHER_BASE_URL: string;
+  BROKER_HA_MODE_ENABLED: string;
+}
+
+export type Config = BackendAPI &
+  BrokerClient &
+  BrokerServer &
+  HighAvailabilityMode;

lib/client/dispatcher/index.ts

@@ -1,6 +1,6 @@
 import crypto = require('crypto');
 import logger = require('../../log');
-import { HAConfiguration } from './config';
+import { Config } from '../config';
 import { HttpDispatcherServiceClient } from './client/api';
 import { ServerId, getServerIdFromDispatcher } from './dispatcher-service';
 
@@ -10,7 +10,7 @@ export function highAvailabilityModeEnabled(config: any): boolean {
   // high availability mode is disabled per default
   let highAvailabilityModeEnabled = false;
 
-  const highAvailabilityModeEnabledValue = (config as HAConfiguration)
+  const highAvailabilityModeEnabledValue = (config as Config)
     .BROKER_HA_MODE_ENABLED;
 
   if (typeof highAvailabilityModeEnabledValue !== 'undefined') {
@@ -63,8 +63,8 @@ export async function getServerId(
   return null;
 }
 
-function getHAConfig(config: any): HAConfiguration {
-  return config as HAConfiguration;
+function getHAConfig(config: any): Config {
+  return config as Config;
 }
 
 function hash(token: string): string {

test/helpers/test-factories.ts

@@ -1,12 +1,28 @@
 import { Check } from '../../lib/client/checks/types';
+import { Config } from '../../lib/client/config';
 
 export const aCheck = (fields: Partial<Check>): Check => {
   const id = `check_${Date.now()}`;
   return {
     checkId: id,
     checkName: id,
     url: 'http://localhost:8080/check-url',
+    active: true,
     timeoutMs: 500,
     ...fields,
   };
 };
+
+/**
+ * Config with all features disabled.
+ */
+export const aConfig = (fields: Partial<Config>): Config => {
+  return {
+    API_BASE_URL: 'http://api:8080',
+    BROKER_DISPATCHER_BASE_URL: 'http://dispatcher:8080',
+    BROKER_HA_MODE_ENABLED: 'false',
+    BROKER_SERVER_URL: 'http://broker-server:8080',
+    PREFLIGHT_CHECKS_ENABLED: 'false',
+    ...fields,
+  };
+};

test/unit/client/checks/config.test.ts

@@ -0,0 +1,29 @@
+import { aConfig } from '../../../helpers/test-factories';
+import { checksConfig } from '../../../../lib/client/checks';
+import { createRestApiHealthcheck } from '../../../../lib/client/checks/http/http-checks';
+
+describe('preflight-checks-config', () => {
+  it('should contain rest-api-status check if high availability mode is enabled', async () => {
+    const config = aConfig({
+      BROKER_HA_MODE_ENABLED: 'true',
+    });
+    const restApiCheck = createRestApiHealthcheck(config);
+    const { preflightCheckStore } = await checksConfig(config);
+
+    const actualChecks = await preflightCheckStore.getAll();
+
+    expect(actualChecks).toContainEqual(restApiCheck);
+  });
+
+  it('should not contain rest-api-status check if high availability mode is disabled', async () => {
+    const config = aConfig({
+      BROKER_HA_MODE_ENABLED: 'false',
+    });
+    const restApiCheck = createRestApiHealthcheck(config);
+    const { preflightCheckStore } = await checksConfig(config);
+
+    const actualChecks = await preflightCheckStore.getAll();
+
+    expect(actualChecks).not.toContainEqual(restApiCheck);
+  });
+});

test/unit/client/checks/prefilght-check-store.test.ts → test/unit/client/checks/preflight-check-store.test.ts

@@ -1,5 +1,5 @@
-import { Check } from '../../../../lib/client/checks/types';
 import { PreflightCheckStore } from '../../../../lib/client/checks/preflight-check-store';
+import { aCheck } from '../../../helpers/test-factories';
 
 let preflightCheckStore: PreflightCheckStore;
 
@@ -9,19 +9,9 @@ describe('preflight-check-store', () => {
   });
 
   it('returns all preflight checks', async () => {
-    const checkOne: Check = {
-      checkId: 'check-one',
-      checkName: 'check one',
-      url: 'https://some-url',
-      timeoutMs: 10,
-    };
+    const checkOne = aCheck({ checkId: 'check-one', checkName: 'check-one' });
+    const checkTwo = aCheck({ checkId: 'check-two', checkName: 'check-two' });
     await preflightCheckStore.add(checkOne);
-    const checkTwo: Check = {
-      checkId: 'check-two',
-      checkName: 'check two',
-      url: 'https://some-url',
-      timeoutMs: 10,
-    };
     await preflightCheckStore.add(checkTwo);
 
     const foundChecks = await preflightCheckStore.getAll();
@@ -31,13 +21,23 @@ describe('preflight-check-store', () => {
     expect(foundChecks[1]).toStrictEqual(checkTwo);
   });
 
+  it('returns only active preflight checks', async () => {
+    const checkActive = aCheck({ checkId: 'check-active' });
+    const checkNotActive = aCheck({
+      checkId: 'check-not-active',
+      active: false,
+    });
+    await preflightCheckStore.add(checkActive);
+    await preflightCheckStore.add(checkNotActive);
+
+    const foundChecks = await preflightCheckStore.getAll();
+
+    expect(foundChecks.length).toEqual(1);
+    expect(foundChecks[0]).toStrictEqual(checkActive);
+  });
+
   it('returns the preflight check by checkId', async () => {
-    const check: Check = {
-      checkId: 'check-one',
-      checkName: 'check one',
-      url: 'https://some-url',
-      timeoutMs: 1_000,
-    };
+    const check = aCheck({ checkId: 'check-one', checkName: 'check-one' });
     await preflightCheckStore.add(check);
 
     const foundCheck = await preflightCheckStore.get('check-one');
@@ -46,13 +46,23 @@ describe('preflight-check-store', () => {
     expect(foundCheck?.checkId).toBe('check-one');
   });
 
+  it('returns null for inactive check by checkId', async () => {
+    const checkNotActive = aCheck({
+      checkId: 'check-not-active',
+      active: false,
+    });
+    await preflightCheckStore.add(checkNotActive);
+
+    const foundCheck = await preflightCheckStore.get('check-not-active');
+
+    expect(foundCheck).toBeNull();
+  });
+
   it('returns null if no check found', async () => {
-    const check: Check = {
+    const check = aCheck({
       checkId: 'check-random',
-      checkName: 'check random',
-      url: 'https://some-random-url',
-      timeoutMs: 10,
-    };
+      checkName: 'check-random',
+    });
     await preflightCheckStore.add(check);
 
     const foundCheck = await preflightCheckStore.get('non-existing-checkId');
@@ -61,12 +71,10 @@ describe('preflight-check-store', () => {
   });
 
   it('adds only unique preflight checks', async () => {
-    const check: Check = {
+    const check = aCheck({
       checkId: 'unique-check',
       checkName: 'unique-check',
-      url: 'https://some-unique-url',
-      timeoutMs: 100,
-    };
+    });
     await preflightCheckStore.add(check);
     await preflightCheckStore.add(check);