diff --git a/.changeset/metal-fishes-grow.md b/.changeset/metal-fishes-grow.md index 7067a881bb..885bcecd1b 100644 --- a/.changeset/metal-fishes-grow.md +++ b/.changeset/metal-fishes-grow.md @@ -2,4 +2,4 @@ '@backstage/core-app-api': patch --- -Apps will now detect when a relative `backend.baseUrl` is provided and update the config accordingly. +Apps will now detect when a relative `backend.baseUrl` or `app.baseUrl` is provided and update the config to produce a full URL. diff --git a/.changeset/short-games-march.md b/.changeset/short-games-march.md deleted file mode 100644 index 54c40b943e..0000000000 --- a/.changeset/short-games-march.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/app-defaults': minor ---- - -Added support for parsing relative backend urls to the api factory. Allows relative backend urls to be specified in config options. diff --git a/.changeset/young-chairs-check.md b/.changeset/young-chairs-check.md new file mode 100644 index 0000000000..b0611af148 --- /dev/null +++ b/.changeset/young-chairs-check.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Apps will now detect when a relative `backend.baseUrl` or `app.baseUrl` is provided and update it to be a full URL. This means that you can provide relative URLs and they will be resolved as expected across the application. diff --git a/packages/cli/package.json b/packages/cli/package.json index 41b37fc7b0..661d53d897 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -154,7 +154,6 @@ "@types/terser-webpack-plugin": "^5.0.4", "@types/yarnpkg__lockfile": "^1.1.4", "del": "^6.0.0", - "get-port": "^6.1.2", "mock-fs": "^5.1.0", "msw": "^0.49.0", "nodemon": "^2.0.2", @@ -273,4 +272,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/cli/src/__fixtures__/test-project/app-config.yaml b/packages/cli/src/__fixtures__/test-project/app-config.yaml deleted file mode 100644 index 69bb98f0c7..0000000000 --- a/packages/cli/src/__fixtures__/test-project/app-config.yaml +++ /dev/null @@ -1,6 +0,0 @@ -app: - baseUrl: http://localhost:${PORT}/test - title: test - -backend: - baseUrl: http://localhost:${BACKEND_PORT} diff --git a/packages/cli/src/__fixtures__/test-project/backstage.json b/packages/cli/src/__fixtures__/test-project/backstage.json deleted file mode 100644 index 1587a66968..0000000000 --- a/packages/cli/src/__fixtures__/test-project/backstage.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "version": "1.0.0" -} diff --git a/packages/cli/src/__fixtures__/test-project/package.json b/packages/cli/src/__fixtures__/test-project/package.json deleted file mode 100644 index e1f6b310f3..0000000000 --- a/packages/cli/src/__fixtures__/test-project/package.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "name": "root", - "version": "1.0.0", - "private": true, - "engines": { - "node": "14 || 16" - }, - "workspaces": { - "packages": [ - "packages/*" - ] - }, - "devDependencies": { - "@backstage/cli": "^0.20.0-next.0", - "lerna": "^4.0.0", - "node-gyp": "^9.0.0", - "typescript": "~4.6.4" - } -} diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/package.json b/packages/cli/src/__fixtures__/test-project/packages/app/package.json deleted file mode 100644 index 95e50a6774..0000000000 --- a/packages/cli/src/__fixtures__/test-project/packages/app/package.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "name": "app", - "version": "0.0.0", - "private": true, - "bundled": true, - "backstage": { - "role": "frontend" - }, - "files": [ - "dist" - ] -} diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html b/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html deleted file mode 100644 index 93da08ba81..0000000000 --- a/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - -
- - diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx b/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx deleted file mode 100644 index 03a9b39841..0000000000 --- a/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright 2022 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. - */ -export {}; -console.log('Hello World.'); diff --git a/packages/cli/src/__fixtures__/test-project/yarn.lock b/packages/cli/src/__fixtures__/test-project/yarn.lock deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/packages/cli/src/commands/build/command.ts b/packages/cli/src/commands/build/command.ts index b5120f7ae0..fcaf58e1ee 100644 --- a/packages/cli/src/commands/build/command.ts +++ b/packages/cli/src/commands/build/command.ts @@ -29,7 +29,6 @@ export async function command(opts: OptionValues): Promise { targetDir: paths.targetDir, configPaths: opts.config as string[], writeStats: Boolean(opts.stats), - cliOptions: opts, }); } if (role === 'backend') { diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 2dd2973de1..82cb60e62f 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -126,14 +126,6 @@ export function registerScriptCommand(program: Command) { '--stats', 'If bundle stats are available, write them to the output directory. Applies to app packages only.', ) - .option( - '--public-path ', - 'Public path for hosting the website on, can be relative.', - ) - .option( - '--backend-url ', - 'Backend url, expects just the origin or sub-route. Do not include /api. Can be relative.', - ) .option( '--config ', 'Config files to load instead of app-config.yaml. Applies to app packages only.', diff --git a/packages/cli/src/commands/repo/build.ts b/packages/cli/src/commands/repo/build.ts index 55574a53b0..21b1580cb9 100644 --- a/packages/cli/src/commands/repo/build.ts +++ b/packages/cli/src/commands/repo/build.ts @@ -153,7 +153,6 @@ export async function command(opts: OptionValues, cmd: Command): Promise { return; } await buildFrontend({ - cliOptions: opts, targetDir: pkg.dir, configPaths: (buildOptions.config as string[]) ?? [], writeStats: Boolean(buildOptions.stats), diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts index bc0ff2db3b..ace60883a4 100644 --- a/packages/cli/src/lib/bundler/config.ts +++ b/packages/cli/src/lib/bundler/config.ts @@ -36,12 +36,12 @@ import { runPlain } from '../run'; import ESLintPlugin from 'eslint-webpack-plugin'; import pickBy from 'lodash/pickBy'; -const DUMMY_URL = 'http://dummyurl.org'; +const NO_OP_URL = 'http://test-site-asdf.org'; export function resolveBaseUrl(config: Config): URL { const baseUrl = config.getString('app.baseUrl'); try { - return new URL(baseUrl, DUMMY_URL); + return new URL(baseUrl, NO_OP_URL); } catch (error) { throw new Error(`Invalid app.baseUrl, ${error}`); } @@ -90,7 +90,7 @@ export async function createConfig( const externalPkgs = packages.filter(p => !isChildPath(paths.root, p.dir)); const baseUrl = frontendConfig.getString('app.baseUrl'); - const validBaseUrl = new URL(baseUrl, DUMMY_URL); + const validBaseUrl = new URL(baseUrl, NO_OP_URL); const publicPath = validBaseUrl.pathname.replace(/\/$/, ''); if (checksEnabled) { plugins.push( diff --git a/packages/cli/src/lib/config.test.ts b/packages/cli/src/lib/config.test.ts deleted file mode 100644 index 9db076e8e4..0000000000 --- a/packages/cli/src/lib/config.test.ts +++ /dev/null @@ -1,106 +0,0 @@ -/* - * 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 { readCliConfig } from './config'; - -describe('readCliConfig', () => { - it('should return empty config for empty cli', () => { - expect(readCliConfig({})).toEqual([]); - }); - - it('should return empty config for no matching keys', () => { - expect( - readCliConfig({ - test: '123', - } as any), - ).toEqual([]); - }); - - it('should return backend.baseUrl when backendUrl present in cli options', () => { - expect( - readCliConfig({ - backendUrl: '/', - }), - ).toEqual([ - { - data: { - backend: { - baseUrl: '/', - }, - }, - context: 'cli', - }, - ]); - }); - - it('should return app.baseUrl when publicPath present in cli options', () => { - expect( - readCliConfig({ - publicPath: '/', - }), - ).toEqual([ - { - data: { - app: { - baseUrl: '/', - }, - }, - context: 'cli', - }, - ]); - }); - - it('should return app.baseUrl and backend.baseUrl when publicPath and backendUrl present in cli options', () => { - expect( - readCliConfig({ - publicPath: '/', - backendUrl: '/api', - }), - ).toEqual([ - { - data: { - app: { - baseUrl: '/', - }, - backend: { - baseUrl: '/api', - }, - }, - context: 'cli', - }, - ]); - }); - - it('should throw for public paths that do NOT start with /', () => { - ['http://localhost:3000', './', '../..'].forEach(publicPath => - expect(() => { - readCliConfig({ - publicPath, - }); - }).toThrow('Public path must be relative'), - ); - }); - - it('should throw for backend urls that do NOT start with /', () => { - ['http://localhost:3000', './', '../..'].forEach(backendUrl => - expect(() => { - readCliConfig({ - backendUrl, - }); - }).toThrow('Backend URL must be relative'), - ); - }); -}); diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index 9084f72057..a76571c3f6 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -19,17 +19,11 @@ import { loadConfig, loadConfigSchema, } from '@backstage/config-loader'; -import { AppConfig, ConfigReader } from '@backstage/config'; +import { ConfigReader } from '@backstage/config'; import { paths } from './paths'; import { isValidUrl } from './urls'; import { getPackages } from '@manypkg/get-packages'; import { PackageGraph } from './monorepo'; -import { JsonObject } from '@backstage/types'; - -export type CliConfigOptions = { - publicPath?: string; - backendUrl?: string; -}; type Options = { args: string[]; @@ -38,44 +32,8 @@ type Options = { withFilteredKeys?: boolean; withDeprecatedKeys?: boolean; fullVisibility?: boolean; - cliOptions?: CliConfigOptions; }; -/** - * Read specific parameters from the CLI and add them to the build config. - * @param opts CLI passed parameters. - * @returns Array of config, empty if there is no relevant passed in cli options. - * - * @public - */ -export function readCliConfig(opts?: CliConfigOptions): AppConfig[] { - if (!opts || Object.keys(opts).length === 0) return []; - const data: JsonObject = {}; - - if (opts.publicPath?.startsWith('/')) { - data.app = { - baseUrl: opts.publicPath, - }; - } else if (opts.publicPath) { - throw new Error( - 'Public path must be relative and start with "/" when specified through CLI options. Path traversals like "./" and assumed relative endpoints like "backstage" are not supported.', - ); - } - if (opts.backendUrl?.startsWith('/')) { - data.backend = { - baseUrl: opts.backendUrl, - }; - } else if (opts.backendUrl) { - throw new Error( - 'Backend URL must be relative and start with "/" when specified through CLI options. Path traversals like "./" and assumed relative endpoint like "api" are not supported.', - ); - } - - if (Object.keys(data).length === 0) return []; - - return [{ data, context: 'cli' }]; -} - export async function loadCliConfig(options: Options) { const configTargets: ConfigTarget[] = []; options.args.forEach(arg => { @@ -114,8 +72,6 @@ export async function loadCliConfig(options: Options) { packagePaths: [paths.resolveTargetRoot('package.json')], }); - const cliConfigs = readCliConfig(options.cliOptions); - const { appConfigs } = await loadConfig({ experimentalEnvFunc: options.mockEnv ? async name => process.env[name] || 'x' @@ -124,9 +80,6 @@ export async function loadCliConfig(options: Options) { configTargets: configTargets, }); - // Add the cliConfigs to the end of the appConfigs array for final overriding. - appConfigs.push(...cliConfigs); - // printing to stderr to not clobber stdout in case the cli command // outputs structured data (e.g. as config:schema does) process.stderr.write( diff --git a/packages/cli/src/serve.test.ts b/packages/cli/src/serve.test.ts deleted file mode 100644 index 7256e7c4ad..0000000000 --- a/packages/cli/src/serve.test.ts +++ /dev/null @@ -1,163 +0,0 @@ -/* - * Copyright 2021 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 { spawn, SpawnOptionsWithoutStdio } from 'child_process'; -import EventEmitter from 'events'; -import fetch from 'node-fetch'; -import path from 'path'; -import getPort from 'get-port'; - -jest.setTimeout(150000); - -const executeCommand = ( - command: string, - args: string[], - options?: SpawnOptionsWithoutStdio, - events?: EventEmitter, - eventConfig?: { - killRegex?: RegExp; - signalRegex?: RegExp[]; - }, -): Promise<{ - exit: number; - stdout: string; - stderr: string; -}> => { - return new Promise((resolve, reject) => { - const stdout: Buffer[] = []; - const stderr: Buffer[] = []; - - const shell = process.platform === 'win32'; - const proc = spawn(command, args, { ...options, shell }); - - proc.stdout?.on('data', data => { - stdout.push(Buffer.from(data)); - }); - - proc.stderr?.on('data', data => { - stderr.push(Buffer.from(data)); - }); - - /** - * Set an interval to check if we should kill the process. - * This was the easiest way I could think of of testing across two processes. - */ - let intervalId: NodeJS.Timer | undefined = undefined; - if (eventConfig) { - intervalId = setInterval(() => { - const stdoutStr = Buffer.concat(stdout).toString('utf8'); - if (eventConfig.killRegex?.test(stdoutStr)) { - proc.kill('SIGINT'); - } else if ( - eventConfig.signalRegex?.some(regex => regex.test(stdoutStr)) - ) { - events?.emit('hit'); - } - }, 1000); - } - - const clearEventInterval = () => { - if (intervalId) { - try { - clearInterval(intervalId); - } catch (err) { - console.error(err); - } - } - }; - - /** - * Need a way to kill the process from another process. - */ - events?.on('stop', signal => { - clearEventInterval(); - proc.kill(signal); - }); - - proc.on('error', (...errorArgs) => { - clearEventInterval(); - reject(errorArgs); - }); - proc.on('exit', code => { - clearEventInterval(); - resolve({ - exit: code ?? 0, - stdout: Buffer.concat(stdout).toString('utf8'), - stderr: Buffer.concat(stderr).toString('utf8'), - }); - }); - }); -}; - -const testProjectDir = path.resolve( - __dirname, - '__fixtures__/test-project/packages/app', -); - -describe('end-to-end', () => { - const entryPoint = path.resolve(__dirname, '../bin/backstage-cli'); - - it('builds frontend with correct url overrides', async () => { - const buildProc = await executeCommand( - entryPoint, - ['package', 'build', '--public-path', '/test', '--backend-url', '/api'], - { - cwd: testProjectDir, - }, - ); - expect(buildProc.stderr).toContain( - 'Loaded config from app-config.yaml, cli', - ); - - expect(buildProc.exit).toEqual(0); - }); - - it('starts frontend on correct url', async () => { - expect.assertions(4); - - const startEmitter = new EventEmitter(); - const frontendPort = await getPort(); - startEmitter.on('hit', async () => { - const response = await fetch( - `http://localhost:${frontendPort}/test/catalog`, - ); - const text = await response.text(); - startEmitter.emit('stop', 'SIGINT'); - expect(response.status).toBe(200); - expect(text.length).toBeGreaterThan(0); - expect(text).toContain('id="root"'); - }); - const startProc = await executeCommand( - entryPoint, - ['package', 'start'], - { - cwd: testProjectDir, - env: { - ...process.env, - PORT: `${frontendPort}`, - BACKEND_PORT: `${await getPort()}`, - }, - }, - startEmitter, - { - // Need to match console colors as well, so use .* instead of just ' '. - signalRegex: [/compiled.*successfully/], - }, - ); - - expect(startProc.exit).toEqual(0); - }); -});