diff --git a/api/src/buildconfig.ts b/api/src/buildconfig.ts index 68a44e13c..6397f024d 100644 --- a/api/src/buildconfig.ts +++ b/api/src/buildconfig.ts @@ -30,6 +30,11 @@ const FALSEY_STRINGS = ['false', '0', 'off', 'no']; export const CLUSTER_ADMIN_GROUP_NAME = 'cluster-admin'; export const NOTEBOOK_CREATOR_GROUP_NAME = 'notebook-creator'; +// Constants + +// If a URL for the conductor instance is not provided this will be used as a fall-through +const DEFAULT_CONDUCTOR_URL = 'http://localhost:8080'; + /* * This is designed to get useful commit information data from * environment variables for the testing server. While more sophisticated @@ -56,9 +61,13 @@ function commit_version(): string { function conductor_url(): string { const url = process.env.CONDUCTOR_PUBLIC_URL; if (url === '' || url === undefined) { - return 'http://localhost:8080'; + console.warn( + `No value for CONDUCTOR_PUBLIC_URL was provided in the environment. Defaulting to ${DEFAULT_CONDUCTOR_URL}.` + ); + return DEFAULT_CONDUCTOR_URL; + } else { + return url; } - return url; } function app_url(): string { diff --git a/app/src/buildconfig.test.ts b/app/src/buildconfig.test.ts new file mode 100644 index 000000000..206e2c326 --- /dev/null +++ b/app/src/buildconfig.test.ts @@ -0,0 +1,44 @@ +/* + * Copyright 2021, 2022 Macquarie University + * + * 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. + * + * Filename: buildconfig.test.ts + * Description: + * This test file checks that the parsing of conductor URLs meets specifications + */ + +// eslint-disable-next-line n/no-unpublished-import +import {expect, it, describe} from 'vitest'; +import {DEFAULT_CONDUCTOR_URL, parseConductorUrls} from './buildconfig'; + +describe('parse conductor URLs', () => { + it('defaults to the DEFAULT_CONDUCTOR_URL if empty string provided', () => { + const res = parseConductorUrls(''); + expect(res).toEqual([DEFAULT_CONDUCTOR_URL]); + }); + it('trims excess whitespace', () => { + const res = parseConductorUrls('value1,value2 , value 3 , value4'); + expect(res).toEqual(['value1', 'value2', 'value 3', 'value4']); + }); + it('removes trailing values properly', () => { + const res1 = parseConductorUrls('value1,'); + expect(res1).toEqual(['value1']); + const res2 = parseConductorUrls('value1, '); + expect(res2).toEqual(['value1']); + }); + it('handles malformed inputs such as just commas with spaces by falling through to default', () => { + const res1 = parseConductorUrls(', ,, , , ,, , '); + expect(res1).toEqual([DEFAULT_CONDUCTOR_URL]); + }); +}); diff --git a/app/src/buildconfig.ts b/app/src/buildconfig.ts index 570108120..f8d15e521 100644 --- a/app/src/buildconfig.ts +++ b/app/src/buildconfig.ts @@ -13,7 +13,7 @@ * See, the License, for the specific language governing permissions and * limitations under the License. * - * Filename: reportWebVitals.ts + * Filename: buildconfig.ts * Description: * This module exports the configuration of the build, including things like * which server to use and whether to include test data @@ -24,6 +24,11 @@ import {BUILD_VERSION, BUILD_VERSION_DEFAULT} from './version'; // need to define a local logError here since logging.tsx imports this file const logError = (err: any) => console.error(err); +// Constants + +// Default conductor URL +export const DEFAULT_CONDUCTOR_URL = 'http://localhost:8154'; + const TRUTHY_STRINGS = ['true', '1', 'on', 'yes']; const FALSEY_STRINGS = ['false', '0', 'off', 'no']; @@ -246,16 +251,50 @@ function get_bugsnag_key(): string | false { return bugsnag_key; } +/** + * Splits the input, trimming for whitespace and filtering empty strings. + * Handles cases including an empty resulting list, and warns on empty strings + * being contained. Falls through to the DEFAULT_CONDUCTOR_URL where needed. + * @returns A list of conductor URLs which can act as servers. + */ +export function parseConductorUrls(conductorUrls: string): string[] { + const urls = conductorUrls.split(','); + // Provide a warning if the split results in any empty strings + if (urls.some((url: string) => url.length === 0)) { + console.warn( + `CONDUCTOR_URL value was provided, but when split, contained entries which were empty. Value: ${conductorUrls}. After split: ${urls}.` + ); + } + // return URLs without trailing whitespace and excluding any values which are empty e.g. due to a trailing comma + + const filteredUrls = urls + .map((url: string) => url.trim()) + .filter((url: string) => url.length > 0); + + // Provide a warning if the split results in an empty list + if (!(filteredUrls.length > 0)) { + console.error( + `CONDUCTOR_URL value was provided, but when split, trimmed, and empty strings removed, had a length of zero. Value: ${conductorUrls}. After split: ${urls}. After filter: ${filteredUrls}. Returning default [${DEFAULT_CONDUCTOR_URL}]` + ); + return [DEFAULT_CONDUCTOR_URL]; + } + + return filteredUrls; +} +/** + * Hands the VITE_CONDUCTOR_URL input to the parse conductor urls method, + * returning a safely handled list of conductor URLs. + * @returns A list of conductor URLs which can act as servers. + */ function get_conductor_urls(): string[] { - const config = import.meta.env.VITE_CONDUCTOR_URL; - if (config) { - const urls = config.split(','); - return urls.map((url: string) => url.trim()); + const conductorUrls: string = import.meta.env.VITE_CONDUCTOR_URL; + if (conductorUrls) { + return parseConductorUrls(conductorUrls); } else { - console.error( - 'No CONDUCTOR URL configured, using default development server' + console.warn( + `No CONDUCTOR URL configured, using default development server at ${DEFAULT_CONDUCTOR_URL}.` ); - return ['http://localhost:8154']; + return [DEFAULT_CONDUCTOR_URL]; } } diff --git a/app/src/gui/layout/appBar.tsx b/app/src/gui/layout/appBar.tsx index 3adcafa8b..a784461e5 100644 --- a/app/src/gui/layout/appBar.tsx +++ b/app/src/gui/layout/appBar.tsx @@ -56,8 +56,12 @@ import {TokenContents} from '@faims3/data-model'; import {checkToken} from '../../utils/helpers'; import SyncStatus from '../components/sync'; import {NOTEBOOK_NAME, NOTEBOOK_NAME_CAPITALIZED} from '../../buildconfig'; + +// eslint-disable-next-line @typescript-eslint/no-unused-vars import HelpIcon from '@mui/icons-material/Help'; +// eslint-disable-next-line @typescript-eslint/no-unused-vars import SwapHorizIcon from '@mui/icons-material/SwapHoriz'; +// eslint-disable-next-line @typescript-eslint/no-unused-vars import {now} from 'lodash'; /** diff --git a/infrastructure/aws-cdk/lib/components/conductor.ts b/infrastructure/aws-cdk/lib/components/conductor.ts index e29df76d0..6b019c37d 100644 --- a/infrastructure/aws-cdk/lib/components/conductor.ts +++ b/infrastructure/aws-cdk/lib/components/conductor.ts @@ -140,7 +140,9 @@ export class FaimsConductor extends Construct { COUCHDB_EXTERNAL_PORT: `${props.couchDBPort}`, COUCHDB_PUBLIC_URL: props.couchDBEndpoint, COUCHDB_INTERNAL_URL: props.couchDBEndpoint, + // Conductor API URLs CONDUCTOR_PUBLIC_URL: this.conductorEndpoint, + CONDUCTOR_URL: this.conductorEndpoint, // TODO Setup Google auth CONDUCTOR_AUTH_PROVIDERS: 'google', GOOGLE_CLIENT_ID: 'replace-me',