From ee318017e71fea9f2ee6999065dba3c5e98cae00 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 Sep 2023 19:21:30 +0200 Subject: [PATCH 01/42] backend-test-utils: initial directory mock implementation Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/package.json | 1 + .../src/files/DirectoryMocker.ts | 83 +++++++++++++++++++ .../backend-test-utils/src/files/index.ts | 17 ++++ packages/backend-test-utils/src/index.ts | 1 + yarn.lock | 1 + 5 files changed, 103 insertions(+) create mode 100644 packages/backend-test-utils/src/files/DirectoryMocker.ts create mode 100644 packages/backend-test-utils/src/files/index.ts diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index 5274b49ce2..bc78e2b4e0 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -50,6 +50,7 @@ "@backstage/types": "workspace:^", "better-sqlite3": "^8.0.0", "express": "^4.17.1", + "fs-extra": "^10.0.1", "knex": "^2.0.0", "msw": "^1.0.0", "mysql2": "^2.2.5", diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts new file mode 100644 index 0000000000..6b67f8f9c0 --- /dev/null +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -0,0 +1,83 @@ +/* + * Copyright 2023 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 { resolveSafeChildPath } from '@backstage/backend-common'; +import fs from 'fs-extra'; +import { tmpdir as getTmpDir } from 'os'; +import { join as joinPath } from 'path'; + +type FileNode = string | Buffer; + +type DirectoryNode = { + [name in string]: MockDirectoryNode; +}; + +type MockDirectoryNode = DirectoryNode | FileNode; + +export class DirectoryMocker { + static create() { + const root = fs.mkdtempSync( + joinPath(getTmpDir(), 'backstage-tmp-test-dir-'), + ); + + const mocker = new DirectoryMocker(root); + + process.on('beforeExit', mocker.#cleanupSync); + + afterAll(async () => { + await mocker.cleanup(); + }); + + return mocker; + } + + readonly #root: string; + + private constructor(root: string) { + this.#root = root; + } + + get dir() { + return this.#root; + } + + async setContent(root: MockDirectoryNode) { + await this.cleanup(); + + async function createFiles(node: MockDirectoryNode, path: string) { + if (typeof node === 'string' || node instanceof Buffer) { + await fs.writeFile(path, node, 'utf8'); + return; + } + + await fs.ensureDir(path); + + for (const [name, child] of Object.entries(node)) { + await createFiles(child, resolveSafeChildPath(path, name)); + } + } + + await createFiles(root, this.#root); + } + + async cleanup() { + await fs.rm(this.#root, { recursive: true, force: true }); + } + + #cleanupSync() { + fs.rmSync(this.#root, { recursive: true, force: true }); + } +} diff --git a/packages/backend-test-utils/src/files/index.ts b/packages/backend-test-utils/src/files/index.ts new file mode 100644 index 0000000000..f6ed111069 --- /dev/null +++ b/packages/backend-test-utils/src/files/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 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 { DirectoryMocker } from './DirectoryMocker'; diff --git a/packages/backend-test-utils/src/index.ts b/packages/backend-test-utils/src/index.ts index ae937d2fc8..5609dbbc2b 100644 --- a/packages/backend-test-utils/src/index.ts +++ b/packages/backend-test-utils/src/index.ts @@ -22,5 +22,6 @@ export * from './database'; export * from './msw'; +export * from './files'; export * from './next'; export * from './util'; diff --git a/yarn.lock b/yarn.lock index 9419064a2e..83d9128534 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3649,6 +3649,7 @@ __metadata: "@types/supertest": ^2.0.8 better-sqlite3: ^8.0.0 express: ^4.17.1 + fs-extra: ^10.0.1 knex: ^2.0.0 msw: ^1.0.0 mysql2: ^2.2.5 From cee06b595291411383658612ee24f0cd059b5841 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 Sep 2023 19:31:00 +0200 Subject: [PATCH 02/42] backend-test-utils: added DirectoryMocker root option Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 6b67f8f9c0..7fe7a1d1f5 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -27,19 +27,33 @@ type DirectoryNode = { type MockDirectoryNode = DirectoryNode | FileNode; +interface DirectoryMockerOptions { + /** + * The root path to create the directory in. Defaults to a temporary directory. + * + * If an existing directory is provided, it will not be cleaned up after the test. + */ + root?: string; +} + export class DirectoryMocker { - static create() { - const root = fs.mkdtempSync( - joinPath(getTmpDir(), 'backstage-tmp-test-dir-'), - ); + static create(options?: DirectoryMockerOptions) { + const root = + options?.root ?? + fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); const mocker = new DirectoryMocker(root); - process.on('beforeExit', mocker.#cleanupSync); + const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); + if (shouldCleanup) { + process.on('beforeExit', mocker.#cleanupSync); - afterAll(async () => { - await mocker.cleanup(); - }); + try { + afterAll(mocker.cleanup); + } catch { + /* ignore */ + } + } return mocker; } @@ -73,11 +87,11 @@ export class DirectoryMocker { await createFiles(root, this.#root); } - async cleanup() { + cleanup = async () => { await fs.rm(this.#root, { recursive: true, force: true }); - } + }; - #cleanupSync() { + #cleanupSync = () => { fs.rmSync(this.#root, { recursive: true, force: true }); - } + }; } From c09cd27904f3dc077d4e7590d893841d3c7f69d1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 11:35:56 +0200 Subject: [PATCH 03/42] backend-test-utils: support key path expansion in DirectoryMocker Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 7fe7a1d1f5..7e99f30086 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -17,15 +17,16 @@ import { resolveSafeChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import { tmpdir as getTmpDir } from 'os'; -import { join as joinPath } from 'path'; +import { dirname, join as joinPath, resolve as resolvePath } from 'path'; -type FileNode = string | Buffer; - -type DirectoryNode = { - [name in string]: MockDirectoryNode; +type MockEntry = { + path: string; + content: Buffer; }; -type MockDirectoryNode = DirectoryNode | FileNode; +export type MockDirectory = { + [name in string]: MockDirectory | string | Buffer; +}; interface DirectoryMockerOptions { /** @@ -68,23 +69,36 @@ export class DirectoryMocker { return this.#root; } - async setContent(root: MockDirectoryNode) { + async setContent(root: MockDirectory) { + const entries = this.#transformInput(root); + await this.cleanup(); - async function createFiles(node: MockDirectoryNode, path: string) { - if (typeof node === 'string' || node instanceof Buffer) { - await fs.writeFile(path, node, 'utf8'); - return; - } + for (const { path, content } of entries) { + const fullPath = resolveSafeChildPath(this.#root, path.slice(1)); // trim leading slash + await fs.ensureDir(dirname(fullPath)); + await fs.writeFile(fullPath, content); + } + } - await fs.ensureDir(path); + #transformInput(input: MockDirectory[string]): MockEntry[] { + const entries: MockEntry[] = []; - for (const [name, child] of Object.entries(node)) { - await createFiles(child, resolveSafeChildPath(path, name)); + function traverse(node: MockDirectory[string], path: string) { + if (typeof node === 'string') { + entries.push({ path, content: Buffer.from(node, 'utf8') }); + } else if (node instanceof Buffer) { + entries.push({ path, content: node }); + } else { + for (const [name, child] of Object.entries(node)) { + traverse(child, `${path}/${name}`); + } } } - await createFiles(root, this.#root); + traverse(input, ''); + + return entries; } cleanup = async () => { From 8f6194ff18a6d3dc163621028478eaafd79aec8d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 11:55:49 +0200 Subject: [PATCH 04/42] backend-test-utils: add directoryMocker.getContent() Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 7e99f30086..66a6823a2f 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -81,6 +81,32 @@ export class DirectoryMocker { } } + async getContent(): Promise { + async function read(path: string): Promise { + if (!(await fs.pathExists(path))) { + return undefined; + } + const entries = await fs.readdir(path); + + return Object.fromEntries( + await Promise.all( + entries.map(async entry => { + const fullPath = resolvePath(path, entry); + const stat = await fs.stat(fullPath); + + if (stat.isDirectory()) { + return [entry, await read(fullPath)]; + } + const content = await fs.readFile(fullPath); + return [entry, content.toString('utf8')]; + }), + ), + ); + } + + return read(this.#root); + } + #transformInput(input: MockDirectory[string]): MockEntry[] { const entries: MockEntry[] = []; From 8a1d570218a8c91002571ae32cfa1062f58983b6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 15:06:23 +0200 Subject: [PATCH 05/42] backend-test-utils: add DirectoryMocker shouldReadAsText option Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/package.json | 1 + .../src/files/DirectoryMocker.ts | 32 +++++++++++++++++-- yarn.lock | 9 +++--- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index bc78e2b4e0..da626d3786 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -56,6 +56,7 @@ "mysql2": "^2.2.5", "pg": "^8.3.0", "testcontainers": "^8.1.2", + "textextensions": "^5.16.0", "uuid": "^8.0.0" }, "peerDependencies": { diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 66a6823a2f..7bf23aa2de 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -16,8 +16,14 @@ import { resolveSafeChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; +import textextensions from 'textextensions'; import { tmpdir as getTmpDir } from 'os'; -import { dirname, join as joinPath, resolve as resolvePath } from 'path'; +import { + dirname, + extname, + join as joinPath, + resolve as resolvePath, +} from 'path'; type MockEntry = { path: string; @@ -37,6 +43,15 @@ interface DirectoryMockerOptions { root?: string; } +interface DirectoryMockerGetContentOptions { + /** + * Whether or not to return files as text rather than buffers. + * + * Defaults to checking the file extension against a list of known text extensions. + */ + shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); +} + export class DirectoryMocker { static create(options?: DirectoryMockerOptions) { const root = @@ -81,7 +96,14 @@ export class DirectoryMocker { } } - async getContent(): Promise { + async getContent( + options?: DirectoryMockerGetContentOptions, + ): Promise { + const shouldReadAsText = + (typeof options?.shouldReadAsText === 'boolean' + ? () => options?.shouldReadAsText + : options?.shouldReadAsText) ?? + ((path: string) => textextensions.includes(extname(path).slice(1))); async function read(path: string): Promise { if (!(await fs.pathExists(path))) { return undefined; @@ -98,7 +120,11 @@ export class DirectoryMocker { return [entry, await read(fullPath)]; } const content = await fs.readFile(fullPath); - return [entry, content.toString('utf8')]; + + if (shouldReadAsText(fullPath, content)) { + return [entry, content.toString('utf8')]; + } + return [entry, content]; }), ), ); diff --git a/yarn.lock b/yarn.lock index 83d9128534..86a011100e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3656,6 +3656,7 @@ __metadata: pg: ^8.3.0 supertest: ^6.1.3 testcontainers: ^8.1.2 + textextensions: ^5.16.0 uuid: ^8.0.0 peerDependencies: "@types/jest": "*" @@ -40458,10 +40459,10 @@ __metadata: languageName: node linkType: hard -"textextensions@npm:^5.12.0, textextensions@npm:^5.13.0": - version: 5.14.0 - resolution: "textextensions@npm:5.14.0" - checksum: 1f610ccf2a2c1445fb7156c23b5c8defd608c74e8047df18abd4b9ee44c74d29b453ba104d14c53c91f9026670f7c923114cd12200ee5ec458cc518fd0798c74 +"textextensions@npm:^5.12.0, textextensions@npm:^5.13.0, textextensions@npm:^5.16.0": + version: 5.16.0 + resolution: "textextensions@npm:5.16.0" + checksum: d2abd5c962760046aa85d9ca542bd8bdb451370fc0a5e5f807aa80dd2f50175ec10d5ce9d28ae96968aaf6a1b1bea254cf4715f24852d0dcf29c6a60af7f793c languageName: node linkType: hard From c6e5eec85c2f3f032a8a020745f45a81d419b71b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 15:06:39 +0200 Subject: [PATCH 06/42] backend-test-utils: initial tests for DirectoryMocker Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.test.ts | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 packages/backend-test-utils/src/files/DirectoryMocker.test.ts diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts new file mode 100644 index 0000000000..cb0e52125c --- /dev/null +++ b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts @@ -0,0 +1,83 @@ +/* + * Copyright 2023 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 { DirectoryMocker } from './DirectoryMocker'; + +describe('DirectoryMocker', () => { + const mocker = DirectoryMocker.create(); + + it('should populate a directory with text files', async () => { + await mocker.setContent({ + 'a.txt': 'a', + 'a/b.txt': 'b', + 'a/b/c.txt': 'c', + 'a/b/d.txt': 'd', + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + a: { + 'b.txt': 'b', + b: { + 'c.txt': 'c', + 'd.txt': 'd', + }, + }, + }); + }); + + it('should mix text and binary files', async () => { + await mocker.setContent({ + 'a.txt': 'a', + 'a/b.txt': 'b', + 'a/b/c.bin': Buffer.from([0xc]), + 'a/b/d.bin': Buffer.from([0xd]), + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + a: { + 'b.txt': 'b', + b: { + 'c.bin': Buffer.from([0xc]), + 'd.bin': Buffer.from([0xd]), + }, + }, + }); + }); + + describe('cleanup', () => { + let cleanupMocker: DirectoryMocker; + + describe('inner', () => { + cleanupMocker = DirectoryMocker.create(); + + it('should populate a directory', async () => { + await cleanupMocker.setContent({ + 'a.txt': 'a', + }); + + await expect(cleanupMocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + }); + }); + }); + + it('should clean up after itself automatically', async () => { + await expect(cleanupMocker.getContent()).resolves.toBeUndefined(); + }); + }); +}); From dbad03ac47aeacb7d621f8e4312e6b00cccf3c5e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 20 Sep 2023 19:12:19 +0200 Subject: [PATCH 07/42] backend-test-utils: added directoryMocker.addContent Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.test.ts | 60 +++++++++++++++++ .../src/files/DirectoryMocker.ts | 66 ++++++++++++------- 2 files changed, 103 insertions(+), 23 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts index cb0e52125c..1a50e38b47 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts @@ -59,6 +59,66 @@ describe('DirectoryMocker', () => { }); }); + it('should be able to add content', async () => { + await mocker.setContent({ + 'a.txt': 'a', + b: {}, + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + b: {}, + }); + + await mocker.addContent({ + 'b.txt': 'b', + b: { + 'c.txt': 'c', + }, + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a', + 'b.txt': 'b', + b: { + 'c.txt': 'c', + }, + }); + }); + + it('should replace existing files', async () => { + await mocker.setContent({ + 'a.txt': 'a', + }); + + await mocker.addContent({ + 'a.txt': 'a2', + }); + + await expect(mocker.getContent()).resolves.toEqual({ + 'a.txt': 'a2', + }); + }); + + it('should not override directories', async () => { + await mocker.setContent({ + 'a.txt': 'a', + b: {}, + }); + + await expect( + mocker.addContent({ + 'a.txt': {}, + }), + ).rejects.toThrow('EEXIST'); + + await expect( + mocker.addContent({ + b: 'b', + }), + ).rejects.toThrow('EISDIR'); + }); + describe('cleanup', () => { let cleanupMocker: DirectoryMocker; diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 7bf23aa2de..87538183ec 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -25,10 +25,16 @@ import { resolve as resolvePath, } from 'path'; -type MockEntry = { - path: string; - content: Buffer; -}; +type MockEntry = + | { + type: 'file'; + path: string; + content: Buffer; + } + | { + type: 'dir'; + path: string; + }; export type MockDirectory = { [name in string]: MockDirectory | string | Buffer; @@ -65,7 +71,7 @@ export class DirectoryMocker { process.on('beforeExit', mocker.#cleanupSync); try { - afterAll(mocker.cleanup); + afterAll(mocker.removeContent); } catch { /* ignore */ } @@ -85,14 +91,23 @@ export class DirectoryMocker { } async setContent(root: MockDirectory) { + await this.removeContent(); + + return this.addContent(root); + } + + async addContent(root: MockDirectory) { const entries = this.#transformInput(root); - await this.cleanup(); + for (const entry of entries) { + const fullPath = resolveSafeChildPath(this.#root, entry.path.slice(1)); // trim leading slash - for (const { path, content } of entries) { - const fullPath = resolveSafeChildPath(this.#root, path.slice(1)); // trim leading slash - await fs.ensureDir(dirname(fullPath)); - await fs.writeFile(fullPath, content); + if (entry.type === 'dir') { + await fs.ensureDir(fullPath); + } else if (entry.type === 'file') { + await fs.ensureDir(dirname(fullPath)); + await fs.writeFile(fullPath, entry.content); + } } } @@ -104,27 +119,27 @@ export class DirectoryMocker { ? () => options?.shouldReadAsText : options?.shouldReadAsText) ?? ((path: string) => textextensions.includes(extname(path).slice(1))); + async function read(path: string): Promise { if (!(await fs.pathExists(path))) { return undefined; } - const entries = await fs.readdir(path); + const entries = await fs.readdir(path, { withFileTypes: true }); return Object.fromEntries( await Promise.all( entries.map(async entry => { - const fullPath = resolvePath(path, entry); - const stat = await fs.stat(fullPath); + const fullPath = resolvePath(path, entry.name); - if (stat.isDirectory()) { - return [entry, await read(fullPath)]; + if (entry.isDirectory()) { + return [entry.name, await read(fullPath)]; } const content = await fs.readFile(fullPath); if (shouldReadAsText(fullPath, content)) { - return [entry, content.toString('utf8')]; + return [entry.name, content.toString('utf8')]; } - return [entry, content]; + return [entry.name, content]; }), ), ); @@ -133,15 +148,24 @@ export class DirectoryMocker { return read(this.#root); } + removeContent = async () => { + await fs.rm(this.#root, { recursive: true, force: true }); + }; + #transformInput(input: MockDirectory[string]): MockEntry[] { const entries: MockEntry[] = []; function traverse(node: MockDirectory[string], path: string) { if (typeof node === 'string') { - entries.push({ path, content: Buffer.from(node, 'utf8') }); + entries.push({ + type: 'file', + path, + content: Buffer.from(node, 'utf8'), + }); } else if (node instanceof Buffer) { - entries.push({ path, content: node }); + entries.push({ type: 'file', path, content: node }); } else { + entries.push({ type: 'dir', path }); for (const [name, child] of Object.entries(node)) { traverse(child, `${path}/${name}`); } @@ -153,10 +177,6 @@ export class DirectoryMocker { return entries; } - cleanup = async () => { - await fs.rm(this.#root, { recursive: true, force: true }); - }; - #cleanupSync = () => { fs.rmSync(this.#root, { recursive: true, force: true }); }; From 24f1dfa58fb1a13991ec3e541c60feb85acfb5f2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 10:45:13 +0200 Subject: [PATCH 08/42] backend-test-utils: path option for DirectoryMocker.getContent + better child path check Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 87538183ec..593b2790c4 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { resolveSafeChildPath } from '@backstage/backend-common'; +import { isChildPath, resolveSafeChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import textextensions from 'textextensions'; import { tmpdir as getTmpDir } from 'os'; @@ -23,6 +23,7 @@ import { extname, join as joinPath, resolve as resolvePath, + relative as relativePath, } from 'path'; type MockEntry = @@ -50,6 +51,8 @@ interface DirectoryMockerOptions { } interface DirectoryMockerGetContentOptions { + path?: string; + /** * Whether or not to return files as text rather than buffers. * @@ -100,7 +103,12 @@ export class DirectoryMocker { const entries = this.#transformInput(root); for (const entry of entries) { - const fullPath = resolveSafeChildPath(this.#root, entry.path.slice(1)); // trim leading slash + const fullPath = resolveSafeChildPath(this.#root, entry.path); + if (!isChildPath(this.#root, fullPath)) { + throw new Error( + `Provided path must resolve to a child path of the mock directory, got ${entry.path}`, + ); + } if (entry.type === 'dir') { await fs.ensureDir(fullPath); @@ -120,6 +128,16 @@ export class DirectoryMocker { : options?.shouldReadAsText) ?? ((path: string) => textextensions.includes(extname(path).slice(1))); + const root = resolvePath(this.#root, options?.path ?? ''); + if (!isChildPath(this.#root, root)) { + throw new Error( + `Provided path must resolve to a child path of the mock directory, got ${relativePath( + this.#root, + root, + )}`, + ); + } + async function read(path: string): Promise { if (!(await fs.pathExists(path))) { return undefined; @@ -145,7 +163,7 @@ export class DirectoryMocker { ); } - return read(this.#root); + return read(root); } removeContent = async () => { @@ -156,18 +174,19 @@ export class DirectoryMocker { const entries: MockEntry[] = []; function traverse(node: MockDirectory[string], path: string) { + const trimmedPath = path.startsWith('/') ? path.slice(1) : path; // trim leading slash if (typeof node === 'string') { entries.push({ type: 'file', - path, + path: trimmedPath, content: Buffer.from(node, 'utf8'), }); } else if (node instanceof Buffer) { - entries.push({ type: 'file', path, content: node }); + entries.push({ type: 'file', path: trimmedPath, content: node }); } else { - entries.push({ type: 'dir', path }); + entries.push({ type: 'dir', path: trimmedPath }); for (const [name, child] of Object.entries(node)) { - traverse(child, `${path}/${name}`); + traverse(child, `${trimmedPath}/${name}`); } } } From d96fb0642fa89991d75ef685db3d9baacd653366 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 10:53:23 +0200 Subject: [PATCH 09/42] backend-test-utils: add DirectoryMocker.mockOsTmpDir Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 593b2790c4..0422969762 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import os from 'os'; import { isChildPath, resolveSafeChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import textextensions from 'textextensions'; @@ -83,6 +84,21 @@ export class DirectoryMocker { return mocker; } + static mockOsTmpDir() { + const mocker = DirectoryMocker.create(); + const origTmpdir = os.tmpdir; + os.tmpdir = () => mocker.path; + + try { + afterAll(() => { + os.tmpdir = origTmpdir; + }); + } catch { + /* ignore */ + } + return mocker; + } + readonly #root: string; private constructor(root: string) { From 4c5d3940cb86e743a5e0c6f8be49324a8e8edce4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 10:55:14 +0200 Subject: [PATCH 10/42] backend-test-utils: add resolve and clear for DirectoryMocker + renames Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 0422969762..473504b6c2 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -75,7 +75,7 @@ export class DirectoryMocker { process.on('beforeExit', mocker.#cleanupSync); try { - afterAll(mocker.removeContent); + afterAll(mocker.cleanup); } catch { /* ignore */ } @@ -105,12 +105,16 @@ export class DirectoryMocker { this.#root = root; } - get dir() { + get path() { return this.#root; } + resolve(...paths: string[]) { + return resolvePath(this.#root, ...paths); + } + async setContent(root: MockDirectory) { - await this.removeContent(); + await this.cleanup(); return this.addContent(root); } @@ -182,7 +186,11 @@ export class DirectoryMocker { return read(root); } - removeContent = async () => { + clear = async () => { + await this.setContent({}); + }; + + cleanup = async () => { await fs.rm(this.#root, { recursive: true, force: true }); }; From f3bd2700ccbb7db71eee60cb4c945510bab5f494 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 10:57:03 +0200 Subject: [PATCH 11/42] backend-test-utils: rename DirectoryMocker.getContent -> .content Signed-off-by: Patrik Oldsberg --- .../src/files/DirectoryMocker.test.ts | 14 +++++++------- .../src/files/DirectoryMocker.ts | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts index 1a50e38b47..6ebf74b2a4 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.test.ts @@ -27,7 +27,7 @@ describe('DirectoryMocker', () => { 'a/b/d.txt': 'd', }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -47,7 +47,7 @@ describe('DirectoryMocker', () => { 'a/b/d.bin': Buffer.from([0xd]), }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -65,7 +65,7 @@ describe('DirectoryMocker', () => { b: {}, }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a', b: {}, }); @@ -77,7 +77,7 @@ describe('DirectoryMocker', () => { }, }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a', 'b.txt': 'b', b: { @@ -95,7 +95,7 @@ describe('DirectoryMocker', () => { 'a.txt': 'a2', }); - await expect(mocker.getContent()).resolves.toEqual({ + await expect(mocker.content()).resolves.toEqual({ 'a.txt': 'a2', }); }); @@ -130,14 +130,14 @@ describe('DirectoryMocker', () => { 'a.txt': 'a', }); - await expect(cleanupMocker.getContent()).resolves.toEqual({ + await expect(cleanupMocker.content()).resolves.toEqual({ 'a.txt': 'a', }); }); }); it('should clean up after itself automatically', async () => { - await expect(cleanupMocker.getContent()).resolves.toBeUndefined(); + await expect(cleanupMocker.content()).resolves.toBeUndefined(); }); }); }); diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/files/DirectoryMocker.ts index 473504b6c2..bf741f6ce2 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/files/DirectoryMocker.ts @@ -51,7 +51,7 @@ interface DirectoryMockerOptions { root?: string; } -interface DirectoryMockerGetContentOptions { +interface DirectoryMockerContentOptions { path?: string; /** @@ -139,8 +139,8 @@ export class DirectoryMocker { } } - async getContent( - options?: DirectoryMockerGetContentOptions, + async content( + options?: DirectoryMockerContentOptions, ): Promise { const shouldReadAsText = (typeof options?.shouldReadAsText === 'boolean' From a51c1f46aa60df4042881caf621e8141cd96dc77 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 11:11:48 +0200 Subject: [PATCH 12/42] backend-test-utils: rename DirectoryMocker -> MockDirectory Signed-off-by: Patrik Oldsberg --- .../MockDirectory.test.ts} | 10 ++++---- .../MockDirectory.ts} | 24 ++++++++++--------- .../src/{files => filesystem}/index.ts | 2 +- packages/backend-test-utils/src/index.ts | 2 +- 4 files changed, 20 insertions(+), 18 deletions(-) rename packages/backend-test-utils/src/{files/DirectoryMocker.test.ts => filesystem/MockDirectory.test.ts} (93%) rename packages/backend-test-utils/src/{files/DirectoryMocker.ts => filesystem/MockDirectory.ts} (90%) rename packages/backend-test-utils/src/{files => filesystem}/index.ts (91%) diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts similarity index 93% rename from packages/backend-test-utils/src/files/DirectoryMocker.test.ts rename to packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 6ebf74b2a4..48b8dcd83d 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -14,10 +14,10 @@ * limitations under the License. */ -import { DirectoryMocker } from './DirectoryMocker'; +import { MockDirectory } from './MockDirectory'; -describe('DirectoryMocker', () => { - const mocker = DirectoryMocker.create(); +describe('MockDirectory', () => { + const mocker = MockDirectory.create(); it('should populate a directory with text files', async () => { await mocker.setContent({ @@ -120,10 +120,10 @@ describe('DirectoryMocker', () => { }); describe('cleanup', () => { - let cleanupMocker: DirectoryMocker; + let cleanupMocker: MockDirectory; describe('inner', () => { - cleanupMocker = DirectoryMocker.create(); + cleanupMocker = MockDirectory.create(); it('should populate a directory', async () => { await cleanupMocker.setContent({ diff --git a/packages/backend-test-utils/src/files/DirectoryMocker.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts similarity index 90% rename from packages/backend-test-utils/src/files/DirectoryMocker.ts rename to packages/backend-test-utils/src/filesystem/MockDirectory.ts index bf741f6ce2..d7e7d2c2e1 100644 --- a/packages/backend-test-utils/src/files/DirectoryMocker.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -38,8 +38,8 @@ type MockEntry = path: string; }; -export type MockDirectory = { - [name in string]: MockDirectory | string | Buffer; +export type MockDirectoryContent = { + [name in string]: MockDirectoryContent | string | Buffer; }; interface DirectoryMockerOptions { @@ -62,13 +62,13 @@ interface DirectoryMockerContentOptions { shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); } -export class DirectoryMocker { +export class MockDirectory { static create(options?: DirectoryMockerOptions) { const root = options?.root ?? fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); - const mocker = new DirectoryMocker(root); + const mocker = new MockDirectory(root); const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); if (shouldCleanup) { @@ -85,7 +85,7 @@ export class DirectoryMocker { } static mockOsTmpDir() { - const mocker = DirectoryMocker.create(); + const mocker = MockDirectory.create(); const origTmpdir = os.tmpdir; os.tmpdir = () => mocker.path; @@ -113,13 +113,13 @@ export class DirectoryMocker { return resolvePath(this.#root, ...paths); } - async setContent(root: MockDirectory) { + async setContent(root: MockDirectoryContent) { await this.cleanup(); return this.addContent(root); } - async addContent(root: MockDirectory) { + async addContent(root: MockDirectoryContent) { const entries = this.#transformInput(root); for (const entry of entries) { @@ -141,7 +141,7 @@ export class DirectoryMocker { async content( options?: DirectoryMockerContentOptions, - ): Promise { + ): Promise { const shouldReadAsText = (typeof options?.shouldReadAsText === 'boolean' ? () => options?.shouldReadAsText @@ -158,7 +158,9 @@ export class DirectoryMocker { ); } - async function read(path: string): Promise { + async function read( + path: string, + ): Promise { if (!(await fs.pathExists(path))) { return undefined; } @@ -194,10 +196,10 @@ export class DirectoryMocker { await fs.rm(this.#root, { recursive: true, force: true }); }; - #transformInput(input: MockDirectory[string]): MockEntry[] { + #transformInput(input: MockDirectoryContent[string]): MockEntry[] { const entries: MockEntry[] = []; - function traverse(node: MockDirectory[string], path: string) { + function traverse(node: MockDirectoryContent[string], path: string) { const trimmedPath = path.startsWith('/') ? path.slice(1) : path; // trim leading slash if (typeof node === 'string') { entries.push({ diff --git a/packages/backend-test-utils/src/files/index.ts b/packages/backend-test-utils/src/filesystem/index.ts similarity index 91% rename from packages/backend-test-utils/src/files/index.ts rename to packages/backend-test-utils/src/filesystem/index.ts index f6ed111069..f0b05641f5 100644 --- a/packages/backend-test-utils/src/files/index.ts +++ b/packages/backend-test-utils/src/filesystem/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { DirectoryMocker } from './DirectoryMocker'; +export { MockDirectory } from './MockDirectory'; diff --git a/packages/backend-test-utils/src/index.ts b/packages/backend-test-utils/src/index.ts index 5609dbbc2b..ff1f2ee460 100644 --- a/packages/backend-test-utils/src/index.ts +++ b/packages/backend-test-utils/src/index.ts @@ -22,6 +22,6 @@ export * from './database'; export * from './msw'; -export * from './files'; +export * from './filesystem'; export * from './next'; export * from './util'; From 3eab720cd210b0dc41f731c48e9e2fd2eff77263 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 11:18:05 +0200 Subject: [PATCH 13/42] backend-common: remove usage of mock-fs, use MockDirectory instead Signed-off-by: Patrik Oldsberg --- packages/backend-common/package.json | 2 - .../src/reading/AzureUrlReader.test.ts | 23 +-- .../reading/BitbucketCloudUrlReader.test.ts | 25 ++- .../reading/BitbucketServerUrlReader.test.ts | 21 +-- .../src/reading/BitbucketUrlReader.test.ts | 27 ++-- .../src/reading/GerritUrlReader.test.ts | 27 ++-- .../src/reading/GithubUrlReader.test.ts | 25 ++- .../src/reading/GitlabUrlReader.test.ts | 25 ++- .../tree/ReadableArrayResponse.test.ts | 45 +++--- .../reading/tree/TarArchiveResponse.test.ts | 151 +++++++++++------- .../reading/tree/ZipArchiveResponse.test.ts | 116 ++++++++------ .../src/reading/tree/ZipArchiveResponse.ts | 11 +- .../src/util/DockerContainerRunner.test.ts | 30 ++-- yarn.lock | 2 - 14 files changed, 267 insertions(+), 263 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index a887bdc8e7..ee43b74c16 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -125,7 +125,6 @@ "@types/fs-extra": "^9.0.3", "@types/http-errors": "^2.0.0", "@types/minimist": "^1.2.0", - "@types/mock-fs": "^4.13.0", "@types/morgan": "^1.9.0", "@types/node-forge": "^1.3.0", "@types/pg": "^8.6.6", @@ -138,7 +137,6 @@ "aws-sdk-client-mock": "^2.0.0", "better-sqlite3": "^8.0.0", "http-errors": "^2.0.0", - "mock-fs": "^5.2.0", "msw": "^1.0.0", "mysql2": "^2.2.5", "recursive-readdir": "^2.2.2", diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index 6b2f41a2bf..b6338a1c70 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -23,12 +23,13 @@ import { AzureDevOpsCredentialLike, AzureIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import * as os from 'os'; import path from 'path'; import { NotModifiedError } from '@backstage/errors'; import { getVoidLogger } from '../logging'; @@ -43,6 +44,8 @@ type AzureIntegrationConfigLike = Partial< const logger = getVoidLogger(); +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -69,18 +72,8 @@ const urlReaderFactory = (azureIntegration: AzureIntegrationConfigLike) => { ); }; -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('AzureUrlReader', () => { - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); + beforeEach(() => mockDir.clear()); const worker = setupServer(); setupRequestMockHandlers(worker); @@ -270,7 +263,7 @@ describe('AzureUrlReader', () => { 'https://dev.azure.com/organization/project/_git/repository', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), diff --git a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts index c00aeba124..18a186f793 100644 --- a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts @@ -19,18 +19,21 @@ import { BitbucketCloudIntegration, readBitbucketCloudIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { NotModifiedError } from '@backstage/errors'; import { BitbucketCloudUrlReader } from './BitbucketCloudUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; import getRawBody from 'raw-body'; +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -49,18 +52,8 @@ const reader = new BitbucketCloudUrlReader( { treeResponseFactory }, ); -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('BitbucketCloudUrlReader', () => { - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); + beforeEach(() => mockDir.clear()); const worker = setupServer(); setupRequestMockHandlers(worker); @@ -316,7 +309,7 @@ describe('BitbucketCloudUrlReader', () => { 'https://bitbucket.org/backstage/mock', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), @@ -346,7 +339,7 @@ describe('BitbucketCloudUrlReader', () => { 'https://bitbucket.org/backstage/mock/src/master/docs', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'index.md'), 'utf8'), diff --git a/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts index 9f231e2ba7..82b467f70f 100644 --- a/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts @@ -19,17 +19,20 @@ import { BitbucketServerIntegration, readBitbucketServerIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { NotModifiedError } from '@backstage/errors'; import { BitbucketServerUrlReader } from './BitbucketServerUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; +MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -46,19 +49,7 @@ const reader = new BitbucketServerUrlReader( { treeResponseFactory }, ); -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('BitbucketServerUrlReader', () => { - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); - const worker = setupServer(); setupRequestMockHandlers(worker); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 15e007d516..986619d161 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -19,12 +19,13 @@ import { BitbucketIntegration, readBitbucketIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { NotModifiedError } from '@backstage/errors'; import { BitbucketUrlReader } from './BitbucketUrlReader'; @@ -68,6 +69,10 @@ describe('BitbucketUrlReader.factory', () => { }); describe('BitbucketUrlReader', () => { + const mockDir = MockDirectory.mockOsTmpDir(); + + beforeEach(() => mockDir.clear()); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -98,18 +103,6 @@ describe('BitbucketUrlReader', () => { { treeResponseFactory }, ); - const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); - const worker = setupServer(); setupRequestMockHandlers(worker); @@ -391,7 +384,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.org/backstage/mock', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), @@ -436,7 +429,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.org/backstage/mock/src/master/docs', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'index.md'), 'utf8'), diff --git a/packages/backend-common/src/reading/GerritUrlReader.test.ts b/packages/backend-common/src/reading/GerritUrlReader.test.ts index 19c67ffd56..d3c336ee93 100644 --- a/packages/backend-common/src/reading/GerritUrlReader.test.ts +++ b/packages/backend-common/src/reading/GerritUrlReader.test.ts @@ -14,7 +14,10 @@ * limitations under the License. */ -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; import { NotModifiedError, NotFoundError } from '@backstage/errors'; import { @@ -24,7 +27,6 @@ import { import { JsonObject } from '@backstage/types'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import mockFs from 'mock-fs'; import fs from 'fs-extra'; import path from 'path'; import { getVoidLogger } from '../logging'; @@ -33,6 +35,8 @@ import { DefaultReadTreeResponseFactory } from './tree'; import { GerritUrlReader } from './GerritUrlReader'; import getRawBody from 'raw-body'; +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -82,13 +86,12 @@ const createReader = (config: JsonObject): UrlReaderPredicateTuple[] => { }); }; -// TODO(Rugvip): These tests seem to be a direct or indirect cause of the TaskWorker test flakiness -// We're not sure why at this point, but while investigating these tests are disabled -// eslint-disable-next-line jest/no-disabled-tests -describe.skip('GerritUrlReader', () => { +describe('GerritUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); + beforeEach(() => mockDir.clear()); + afterAll(() => { jest.clearAllMocks(); }); @@ -249,14 +252,13 @@ describe.skip('GerritUrlReader', () => { path.resolve(__dirname, '__fixtures__/gerrit/gerrit-master-docs.tar.gz'), ); - beforeEach(() => { - mockFs({ - '/tmp/': mockFs.directory(), - '/tmp/gerrit-clone-123abc/repo/mkdocs.yml': mkdocsContent, - '/tmp/gerrit-clone-123abc/repo/docs/first.md': mdContent, + beforeEach(async () => { + await mockDir.setContent({ + 'repo/mkdocs.yml': mkdocsContent, + 'repo/docs/first.md': mdContent, }); const spy = jest.spyOn(fs, 'mkdtemp'); - spy.mockImplementation(() => '/tmp/gerrit-clone-123abc'); + spy.mockImplementation(() => mockDir.path); worker.use( rest.get( @@ -289,7 +291,6 @@ describe.skip('GerritUrlReader', () => { }); afterEach(() => { - mockFs.restore(); jest.clearAllMocks(); }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 499f97d6ec..326d3632a4 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -20,12 +20,13 @@ import { GithubIntegration, readGithubIntegrationConfig, } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { NotFoundError, NotModifiedError } from '@backstage/errors'; import { @@ -37,6 +38,8 @@ import { } from './GithubUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -69,21 +72,11 @@ const gheProcessor = new GithubUrlReader( { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, ); -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('GithubUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); + beforeEach(() => mockDir.clear()); beforeEach(() => { jest.clearAllMocks(); @@ -420,7 +413,7 @@ describe('GithubUrlReader', () => { 'https://github.com/backstage/mock', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), @@ -501,7 +494,7 @@ describe('GithubUrlReader', () => { 'https://github.com/backstage/mock/tree/main/docs', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'index.md'), 'utf8'), diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 5af4faf7e5..6500f207f0 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -15,12 +15,13 @@ */ import { ConfigReader } from '@backstage/config'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { + MockDirectory, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import os from 'os'; import path from 'path'; import { getVoidLogger } from '../logging'; import { GitlabUrlReader } from './GitlabUrlReader'; @@ -33,6 +34,8 @@ import { const logger = getVoidLogger(); +const mockDir = MockDirectory.mockOsTmpDir(); + const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), }); @@ -65,18 +68,8 @@ const hostedGitlabProcessor = new GitlabUrlReader( { treeResponseFactory }, ); -const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; - describe('GitlabUrlReader', () => { - beforeEach(() => { - mockFs({ - [tmpDir]: mockFs.directory(), - }); - }); - - afterEach(() => { - mockFs.restore(); - }); + beforeEach(() => mockDir.clear()); const worker = setupServer(); setupRequestMockHandlers(worker); @@ -400,7 +393,7 @@ describe('GitlabUrlReader', () => { 'https://gitlab.com/backstage/mock', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'), @@ -457,7 +450,7 @@ describe('GitlabUrlReader', () => { 'https://gitlab.com/backstage/mock/tree/main/docs', ); - const dir = await response.dir({ targetDir: tmpDir }); + const dir = await response.dir({ targetDir: mockDir.path }); await expect( fs.readFile(path.join(dir, 'index.md'), 'utf8'), diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index a5728a6169..9f0fd5377e 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -15,33 +15,35 @@ */ import fs from 'fs-extra'; -import mockFs from 'mock-fs'; -import path, { resolve as resolvePath } from 'path'; +import path from 'path'; import { FromReadableArrayOptions } from '../types'; import { ReadableArrayResponse } from './ReadableArrayResponse'; +import { MockDirectory } from '@backstage/backend-test-utils'; -const path1 = '/file1.yaml'; +const name1 = 'file1.yaml'; const file1 = fs.readFileSync( path.resolve(__filename, '../../__fixtures__/awsS3/awsS3-mock-object.yaml'), ); -const path2 = '/file2.yaml'; +const name2 = 'file2.yaml'; const file2 = fs.readFileSync( path.resolve(__filename, '../../__fixtures__/awsS3/awsS3-mock-object2.yaml'), ); describe('ReadableArrayResponse', () => { - beforeEach(() => { - mockFs({ - [path1]: file1, - [path2]: file2, - '/tmp': mockFs.directory(), + const sourceDir = MockDirectory.create(); + const targetDir = MockDirectory.create(); + + beforeEach(async () => { + await sourceDir.setContent({ + [name1]: file1, + [name2]: file2, }); + await targetDir.clear(); }); - afterEach(() => { - mockFs.restore(); - }); + const path1 = sourceDir.resolve(name1); + const path2 = sourceDir.resolve(name2); it('should read files', async () => { const arr: FromReadableArrayOptions = [ @@ -49,7 +51,7 @@ describe('ReadableArrayResponse', () => { { data: fs.createReadStream(path2), path: path2 }, ]; - const res = new ReadableArrayResponse(arr, '/tmp', 'etag'); + const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -57,10 +59,7 @@ describe('ReadableArrayResponse', () => { { path: path2, content: expect.any(Function) }, ]); const contents = await Promise.all(files.map(f => f.content())); - expect(contents.map(c => c.toString('utf8').trim())).toEqual([ - 'site_name: Test', - 'site_name: Test2', - ]); + expect(contents).toEqual([file1, file2]); }); it('should extract entire archive into directory', async () => { @@ -69,14 +68,12 @@ describe('ReadableArrayResponse', () => { { data: fs.createReadStream(path2), path: path2 }, ]; - const res = new ReadableArrayResponse(arr, '/tmp', 'etag'); + const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); const dir = await res.dir(); - await expect( - fs.readFile(resolvePath(dir, 'file1.yaml'), 'utf8'), - ).resolves.toMatch(/site_name: Test/); - await expect( - fs.readFile(resolvePath(dir, 'file2.yaml'), 'utf8'), - ).resolves.toMatch(/site_name: Test2/); + await expect(targetDir.content({ path: dir })).resolves.toEqual({ + [name1]: file1.toString('utf8'), + [name2]: file2.toString('utf8'), + }); }); }); diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index 1765a41156..ce126eb345 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -15,30 +15,31 @@ */ import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { resolve as resolvePath } from 'path'; import { TarArchiveResponse } from './TarArchiveResponse'; +import { MockDirectory } from '@backstage/backend-test-utils'; const archiveData = fs.readFileSync( resolvePath(__filename, '../../__fixtures__/mock-main.tar.gz'), ); describe('TarArchiveResponse', () => { - beforeEach(() => { - mockFs({ - '/test-archive.tar.gz': archiveData, - '/tmp': mockFs.directory(), - }); - }); + const sourceDir = MockDirectory.create(); + const targetDir = MockDirectory.create(); - afterEach(() => { - mockFs.restore(); + beforeAll(async () => { + await sourceDir.setContent({ 'test-archive.tar.gz': archiveData }); + }); + beforeEach(async () => { + await targetDir.clear(); }); it('should read files', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new TarArchiveResponse(stream, '', targetDir.path, 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -61,10 +62,16 @@ describe('TarArchiveResponse', () => { }); it('should read files with filter', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', path => - path.endsWith('.yml'), + const res = new TarArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + path => path.endsWith('.yml'), ); const files = await res.files(); @@ -80,16 +87,18 @@ describe('TarArchiveResponse', () => { }); it('should read as archive and files', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new TarArchiveResponse(stream, '', targetDir.path, 'etag'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( 'Response has already been read', ); - const res2 = new TarArchiveResponse(buffer, '', '/tmp', 'etag'); + const res2 = new TarArchiveResponse(buffer, '', targetDir.path, 'etag'); const files = await res2.files(); expect(files).toEqual([ @@ -112,9 +121,11 @@ describe('TarArchiveResponse', () => { }); it('should extract entire archive into directory', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new TarArchiveResponse(stream, '', targetDir.path, 'etag'); const dir = await res.dir(); await expect( fs.readFile(resolvePath(dir, 'mkdocs.yml'), 'utf8'), @@ -125,67 +136,91 @@ describe('TarArchiveResponse', () => { }); it('should extract archive into directory with a subpath', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, 'docs', '/tmp', 'etag'); + const res = new TarArchiveResponse(stream, 'docs', targetDir.path, 'etag'); const dir = await res.dir(); - expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); - await expect( - fs.readFile(resolvePath(dir, 'index.md'), 'utf8'), - ).resolves.toBe('# Test\n'); + await expect(targetDir.content({ path: dir })).resolves.toEqual({ + 'index.md': '# Test\n', + }); }); it('should extract archive into directory with a subpath and filter', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); - - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', path => - path.endsWith('.yml'), + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), ); - const dir = await res.dir({ targetDir: '/tmp' }); - expect(dir).toBe('/tmp'); - await expect(fs.pathExists(resolvePath(dir, 'mkdocs.yml'))).resolves.toBe( - true, + const res = new TarArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + path => path.endsWith('.yml'), ); - await expect( - fs.pathExists(resolvePath(dir, 'docs/index.md')), - ).resolves.toBe(false); + + await targetDir.addContent({ sub: {} }); + const dir = await res.dir({ targetDir: targetDir.resolve('sub') }); + + expect(dir).toBe(targetDir.resolve('sub')); + await expect(targetDir.content()).resolves.toEqual({ + sub: { + 'mkdocs.yml': 'site_name: Test\n', + }, + }); }); - it('should leave temporary directories in place in the case of an error', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + it('should clean up temporary directories in place in the case of an error', async () => { + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', () => { - throw new Error('NOPE'); - }); + const res = new TarArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + () => { + throw new Error('NOPE'); + }, + ); - const tmpDir = await fs.mkdtemp('/tmp/test'); - // selects the wrong overload by default - const mkdtemp = jest.spyOn(fs, 'mkdtemp') as unknown as jest.SpyInstance< - Promise, - [] - >; - mkdtemp.mockResolvedValue(tmpDir); + await targetDir.addContent({ sub: {} }); + const sub = targetDir.resolve('sub'); - await expect(fs.pathExists(tmpDir)).resolves.toBe(true); + const mkdtemp = jest + .spyOn(fs, 'mkdtemp') + .mockImplementation(async () => sub); + + await expect(fs.pathExists(sub)).resolves.toBe(true); await expect(res.dir()).rejects.toThrow('NOPE'); - await expect(fs.pathExists(tmpDir)).resolves.toBe(false); + await expect(fs.pathExists(sub)).resolves.toBe(false); mkdtemp.mockRestore(); }); it('should leave directory in place if provided in the case of an error', async () => { - const stream = fs.createReadStream('/test-archive.tar.gz'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive.tar.gz'), + ); - const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', () => { - throw new Error('NOPE'); - }); + const res = new TarArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + () => { + throw new Error('NOPE'); + }, + ); - const tmpDir = await fs.mkdtemp('/tmp/test'); + await targetDir.addContent({ sub: {} }); + const sub = targetDir.resolve('sub'); - await expect(fs.pathExists(tmpDir)).resolves.toBe(true); - await expect(res.dir({ targetDir: tmpDir })).rejects.toThrow('NOPE'); - await expect(fs.pathExists(tmpDir)).resolves.toBe(true); + await expect(fs.pathExists(sub)).resolves.toBe(true); + await expect(res.dir({ targetDir: sub })).rejects.toThrow('NOPE'); + await expect(fs.pathExists(sub)).resolves.toBe(true); }); }); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 0b010565dd..20cf33241b 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -15,11 +15,11 @@ */ import fs from 'fs-extra'; -import mockFs from 'mock-fs'; import { Readable } from 'stream'; import { create as createArchive } from 'archiver'; import { resolve as resolvePath } from 'path'; import { ZipArchiveResponse } from './ZipArchiveResponse'; +import { MockDirectory } from '@backstage/backend-test-utils'; const archiveData = fs.readFileSync( resolvePath(__filename, '../../__fixtures__/mock-main.zip'), @@ -35,24 +35,25 @@ const archiveWithMaliciousEntry = fs.readFileSync( ); describe('ZipArchiveResponse', () => { - beforeEach(() => { - mockFs({ - '/test-archive.zip': archiveData, - '/test-archive-with-extra-root-dir.zip': archiveDataWithExtraDir, - '/test-archive-corrupted.zip': archiveDataCorrupted, - '/test-archive-malicious.zip': archiveWithMaliciousEntry, - '/tmp': mockFs.directory(), + const sourceDir = MockDirectory.create(); + const targetDir = MockDirectory.create(); + + beforeAll(async () => { + await sourceDir.setContent({ + 'test-archive.zip': archiveData, + 'test-archive-with-extra-root-dir.zip': archiveDataWithExtraDir, + 'test-archive-corrupted.zip': archiveDataCorrupted, + 'test-archive-malicious.zip': archiveWithMaliciousEntry, }); }); - - afterEach(() => { - mockFs.restore(); + beforeEach(async () => { + await targetDir.clear(); }); it('should read files', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -76,10 +77,14 @@ describe('ZipArchiveResponse', () => { }); it('should read files with filter', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag', path => - path.endsWith('.yml'), + const res = new ZipArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + path => path.endsWith('.yml'), ); const files = await res.files(); @@ -95,16 +100,16 @@ describe('ZipArchiveResponse', () => { }); it('should read as archive and files', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( 'Response has already been read', ); - const res2 = new ZipArchiveResponse(buffer, '', '/tmp', 'etag'); + const res2 = new ZipArchiveResponse(buffer, '', targetDir.path, 'etag'); const files = await res2.files(); expect(files).toEqual([ @@ -127,9 +132,9 @@ describe('ZipArchiveResponse', () => { }); it('should extract entire archive into directory', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const dir = await res.dir(); await expect( @@ -141,38 +146,45 @@ describe('ZipArchiveResponse', () => { }); it('should extract archive into directory with a subpath', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + + const res = new ZipArchiveResponse(stream, 'docs/', targetDir.path, 'etag'); - const res = new ZipArchiveResponse(stream, 'docs/', '/tmp', 'etag'); const dir = await res.dir(); - expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); - await expect( - fs.readFile(resolvePath(dir, 'index.md'), 'utf8'), - ).resolves.toBe('# Test\n'); + await expect(targetDir.content({ path: dir })).resolves.toEqual({ + 'index.md': '# Test\n', + }); }); it('should extract archive into directory with a subpath and filter', async () => { - const stream = fs.createReadStream('/test-archive.zip'); + const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag', path => - path.endsWith('.yml'), + const res = new ZipArchiveResponse( + stream, + '', + targetDir.path, + 'etag', + path => path.endsWith('.yml'), ); - const dir = await res.dir({ targetDir: '/tmp' }); - expect(dir).toBe('/tmp'); - await expect(fs.pathExists(resolvePath(dir, 'mkdocs.yml'))).resolves.toBe( - true, - ); - await expect( - fs.pathExists(resolvePath(dir, 'docs/index.md')), - ).resolves.toBe(false); + await targetDir.addContent({ sub: {} }); + const sub = targetDir.resolve('sub'); + const dir = await res.dir({ targetDir: sub }); + + expect(dir).toBe(sub); + + await expect(targetDir.content()).resolves.toEqual({ + sub: { + 'mkdocs.yml': 'site_name: Test\n', + }, + }); }); it('should extract a large archive', async () => { const fileCount = 10; const fileSize = 1000 * 1000; const filePath = await new Promise((resolve, reject) => { - const outFile = '/large-archive.zip'; + const outFile = targetDir.resolve('large-archive.zip'); const archive = createArchive('zip'); archive.on('error', reject); @@ -195,12 +207,14 @@ describe('ZipArchiveResponse', () => { const stream = fs.createReadStream(filePath); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); + + await targetDir.addContent({ sub: {} }); + const sub = targetDir.resolve('sub'); const dir = await res.dir({ - targetDir: '/out', + targetDir: sub, }); - expect(dir).toBe('/out'); const files = await fs.readdir(dir); expect(files).toHaveLength(fileCount); @@ -211,9 +225,11 @@ describe('ZipArchiveResponse', () => { }); it('should throw on invalid archive', async () => { - const stream = fs.createReadStream('/test-archive-corrupted.zip'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive-corrupted.zip'), + ); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const filesPromise = res.files(); await expect(filesPromise).rejects.toThrow( @@ -222,18 +238,22 @@ describe('ZipArchiveResponse', () => { }); it('should throw on entries with a path outside the destination dir', async () => { - const stream = fs.createReadStream('/test-archive-malicious.zip'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive-malicious.zip'), + ); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); await expect(res.files()).rejects.toThrow( 'invalid relative path: ../side.txt', ); }); it('should throw on entries that attempt to write outside destination dir', async () => { - const stream = fs.createReadStream('/test-archive-malicious.zip'); + const stream = fs.createReadStream( + sourceDir.resolve('test-archive-malicious.zip'), + ); - const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); + const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); await expect(res.dir()).rejects.toThrow( 'invalid relative path: ../side.txt', ); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index 1e55150310..cd57788743 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -94,7 +94,10 @@ export class ZipArchiveResponse implements ReadTreeResponse { return new Promise((resolve, reject) => { writeStream.on('error', reject); writeStream.on('finish', () => - resolve({ fileName: tmpFile, cleanup: () => fs.remove(tmpFile) }), + resolve({ + fileName: tmpFile, + cleanup: () => fs.rm(tmpDir, { recursive: true }), + }), ); stream.pipe(writeStream); }); @@ -152,7 +155,7 @@ export class ZipArchiveResponse implements ReadTreeResponse { }); }); - temporary.cleanup(); + await temporary.cleanup(); return files; } @@ -175,7 +178,7 @@ export class ZipArchiveResponse implements ReadTreeResponse { archive.finalize(); - temporary.cleanup(); + await temporary.cleanup(); return archive; } @@ -204,7 +207,7 @@ export class ZipArchiveResponse implements ReadTreeResponse { }); }); - temporary.cleanup(); + await temporary.cleanup(); return dir; } diff --git a/packages/backend-common/src/util/DockerContainerRunner.test.ts b/packages/backend-common/src/util/DockerContainerRunner.test.ts index 3208679d1f..6b4150540a 100644 --- a/packages/backend-common/src/util/DockerContainerRunner.test.ts +++ b/packages/backend-common/src/util/DockerContainerRunner.test.ts @@ -14,27 +14,24 @@ * limitations under the License. */ +import fs from 'fs-extra'; import Docker from 'dockerode'; -import mockFs from 'mock-fs'; -import os from 'os'; -import path from 'path'; import Stream, { PassThrough } from 'stream'; import { ContainerRunner } from './ContainerRunner'; import { DockerContainerRunner, UserOptions } from './DockerContainerRunner'; +import { MockDirectory } from '@backstage/backend-test-utils'; const mockDocker = new Docker() as jest.Mocked; -const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; describe('DockerContainerRunner', () => { let containerTaskApi: ContainerRunner; - beforeEach(() => { - mockFs({ - [rootDir]: { - input: mockFs.directory(), - output: mockFs.directory(), - }, - }); + const inputDir = MockDirectory.create(); + const outputDir = MockDirectory.create(); + + beforeEach(async () => { + await inputDir.clear(); + await outputDir.clear(); jest.spyOn(mockDocker, 'pull').mockImplementation((async ( _image: string, @@ -59,16 +56,15 @@ describe('DockerContainerRunner', () => { afterEach(() => { jest.clearAllMocks(); - mockFs.restore(); }); const imageName = 'dockerOrg/image'; const args = ['bash', '-c', 'echo test']; const mountDirs = { - [path.join(rootDir, 'input')]: '/input', - [path.join(rootDir, 'output')]: '/output', + [inputDir.path]: '/input', + [outputDir.path]: '/output', }; - const workingDir = path.join(rootDir, 'input'); + const workingDir = inputDir.path; const envVars = { HOME: '/tmp', LOG_LEVEL: 'debug' }; const envVarsArray = ['HOME=/tmp', 'LOG_LEVEL=debug']; @@ -117,8 +113,8 @@ describe('DockerContainerRunner', () => { HostConfig: { AutoRemove: true, Binds: expect.arrayContaining([ - `${path.join(rootDir, 'input')}:/input`, - `${path.join(rootDir, 'output')}:/output`, + `${await fs.realpath(inputDir.path)}:/input`, + `${await fs.realpath(outputDir.path)}:/output`, ]), }, Volumes: { diff --git a/yarn.lock b/yarn.lock index 86a011100e..dc8e58da96 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3464,7 +3464,6 @@ __metadata: "@types/http-errors": ^2.0.0 "@types/luxon": ^3.0.0 "@types/minimist": ^1.2.0 - "@types/mock-fs": ^4.13.0 "@types/morgan": ^1.9.0 "@types/node-forge": ^1.3.0 "@types/pg": ^8.6.6 @@ -3497,7 +3496,6 @@ __metadata: luxon: ^3.0.0 minimatch: ^5.0.0 minimist: ^1.2.5 - mock-fs: ^5.2.0 morgan: ^1.10.0 msw: ^1.0.0 mysql2: ^2.2.5 From 5b20107206650412798013c976ba5b533267812f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 12:12:36 +0200 Subject: [PATCH 14/42] backend-test-utils: document MockDirectory + more naming tweaks Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 74 +++++-- .../src/filesystem/MockDirectory.ts | 191 +++++++++++++++--- 2 files changed, 219 insertions(+), 46 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 48b8dcd83d..22563b7a59 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -17,17 +17,17 @@ import { MockDirectory } from './MockDirectory'; describe('MockDirectory', () => { - const mocker = MockDirectory.create(); + const mockDir = MockDirectory.create(); it('should populate a directory with text files', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', 'a/b.txt': 'b', 'a/b/c.txt': 'c', 'a/b/d.txt': 'd', }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -40,14 +40,14 @@ describe('MockDirectory', () => { }); it('should mix text and binary files', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', 'a/b.txt': 'b', 'a/b/c.bin': Buffer.from([0xc]), 'a/b/d.bin': Buffer.from([0xd]), }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -60,24 +60,24 @@ describe('MockDirectory', () => { }); it('should be able to add content', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', b: {}, }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a', b: {}, }); - await mocker.addContent({ + await mockDir.addContent({ 'b.txt': 'b', b: { 'c.txt': 'c', }, }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a', 'b.txt': 'b', b: { @@ -87,57 +87,91 @@ describe('MockDirectory', () => { }); it('should replace existing files', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', }); - await mocker.addContent({ + await mockDir.addContent({ 'a.txt': 'a2', }); - await expect(mocker.content()).resolves.toEqual({ + await expect(mockDir.content()).resolves.toEqual({ 'a.txt': 'a2', }); }); it('should not override directories', async () => { - await mocker.setContent({ + await mockDir.setContent({ 'a.txt': 'a', b: {}, }); await expect( - mocker.addContent({ + mockDir.addContent({ 'a.txt': {}, }), ).rejects.toThrow('EEXIST'); await expect( - mocker.addContent({ + mockDir.addContent({ b: 'b', }), ).rejects.toThrow('EISDIR'); }); + it('examples should work', async () => { + await mockDir.setContent({ + 'test.txt': 'content', + 'sub-dir': { + 'file.txt': 'content', + 'nested-dir/file.txt': 'content', + }, + 'empty-dir': {}, + 'binary-file': Buffer.from([0, 1, 2]), + }); + + await mockDir.addContent({ + 'test.txt': 'content', + 'sub-dir': { + 'file.txt': 'content', + 'nested-dir/file.txt': 'content', + }, + 'empty-dir': {}, + 'binary-file': Buffer.from([0, 1, 2]), + }); + + await expect(mockDir.content()).resolves.toEqual({ + 'test.txt': 'content', + 'sub-dir': { + 'file.txt': 'content', + 'nested-dir': { + 'file.txt': 'content', + }, + }, + 'empty-dir': {}, + 'binary-file': Buffer.from([0, 1, 2]), + }); + }); + describe('cleanup', () => { - let cleanupMocker: MockDirectory; + let cleanupMockDir: MockDirectory; describe('inner', () => { - cleanupMocker = MockDirectory.create(); + cleanupMockDir = MockDirectory.create(); it('should populate a directory', async () => { - await cleanupMocker.setContent({ + await cleanupMockDir.setContent({ 'a.txt': 'a', }); - await expect(cleanupMocker.content()).resolves.toEqual({ + await expect(cleanupMockDir.content()).resolves.toEqual({ 'a.txt': 'a', }); }); }); it('should clean up after itself automatically', async () => { - await expect(cleanupMocker.content()).resolves.toBeUndefined(); + await expect(cleanupMockDir.content()).resolves.toBeUndefined(); }); }); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index d7e7d2c2e1..2ef61b3d0c 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -27,6 +27,69 @@ import { relative as relativePath, } from 'path'; +/** + * The content of a mock directory represented by a nested object structure. + * + * @remarks + * + * When used as input, the keys may contain forward slashes to indicate nested directories. + * Then returned as output, each directory will always be represented as a separate object. + * + * @example + * ```ts + * { + * 'test.txt': 'content', + * 'sub-dir': { + * 'file.txt': 'content', + * 'nested-dir/file.txt': 'content', + * }, + * 'empty-dir': {}, + * 'binary-file': Buffer.from([0, 1, 2]), + * } + * ``` + * + * @public + */ +export type MockDirectoryContent = { + [name in string]: MockDirectoryContent | string | Buffer; +}; + +/** + * Options for {@link MockDirectory.create}. + * + * @public + */ +export interface MockDirectoryCreateOptions { + /** + * The root path to create the directory in. Defaults to a temporary directory. + * + * If an existing directory is provided, it will not be cleaned up after the test. + */ + root?: string; +} + +/** + * Options for {@link MockDirectory.content}. + * + * @public + */ +export interface MockDirectoryContentOptions { + /** + * The path to read content from. Defaults to the root of the mock directory. + * + * An absolute path can also be provided, as long as it is a child path of the mock directory. + */ + path?: string; + + /** + * Whether or not to return files as text rather than buffers. + * + * Defaults to checking the file extension against a list of known text extensions. + */ + shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); +} + +/** @internal */ type MockEntry = | { type: 'file'; @@ -38,32 +101,35 @@ type MockEntry = path: string; }; -export type MockDirectoryContent = { - [name in string]: MockDirectoryContent | string | Buffer; -}; - -interface DirectoryMockerOptions { - /** - * The root path to create the directory in. Defaults to a temporary directory. - * - * If an existing directory is provided, it will not be cleaned up after the test. - */ - root?: string; -} - -interface DirectoryMockerContentOptions { - path?: string; - - /** - * Whether or not to return files as text rather than buffers. - * - * Defaults to checking the file extension against a list of known text extensions. - */ - shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); -} - +/** + * A utility for creating a mock directory that is automatically cleaned up. + * + * @public + */ export class MockDirectory { - static create(options?: DirectoryMockerOptions) { + /** + * Creates a new temporary mock directory that will be removed after the tests have completed. + * + * @remarks + * + * This method is intended to be called outside of any test, either at top-level or + * within a `describe` block. It will call `afterAll` to make sure that the mock directory + * is removed after the tests have run. + * + * @example + * ```ts + * describe('MySubject', () => { + * const mockDir = MockDirectory.create(); + * + * beforeEach(() => mockDir.clear()); + * + * it('should work', async () => { + * // ... use mockDir + * }) + * }) + * ``` + */ + static create(options?: MockDirectoryCreateOptions) { const root = options?.root ?? fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); @@ -84,6 +150,12 @@ export class MockDirectory { return mocker; } + /** + * Like {@link MockDirectory.create}, but also mocks `os.tmpdir()` to return the + * mock directory path until the end of the test suite. + * + * @returns + */ static mockOsTmpDir() { const mocker = MockDirectory.create(); const origTmpdir = os.tmpdir; @@ -105,20 +177,58 @@ export class MockDirectory { this.#root = root; } + /** + * The path to the root of the mock directory + */ get path() { return this.#root; } + /** + * Resolves a path relative to the root of the mock directory. + */ resolve(...paths: string[]) { return resolvePath(this.#root, ...paths); } + /** + * Sets the content of the mock directory. This will remove any existing content. + * + * @example + * ```ts + * await mockDir.setContent({ + * 'test.txt': 'content', + * 'sub-dir': { + * 'file.txt': 'content', + * 'nested-dir/file.txt': 'content', + * }, + * 'empty-dir': {}, + * 'binary-file': Buffer.from([0, 1, 2]), + * }); + * ``` + */ async setContent(root: MockDirectoryContent) { await this.cleanup(); return this.addContent(root); } + /** + * Adds content of the mock directory. This will overwrite existing files. + * + * @example + * ```ts + * await mockDir.addContent({ + * 'test.txt': 'content', + * 'sub-dir': { + * 'file.txt': 'content', + * 'nested-dir/file.txt': 'content', + * }, + * 'empty-dir': {}, + * 'binary-file': Buffer.from([0, 1, 2]), + * }); + * ``` + */ async addContent(root: MockDirectoryContent) { const entries = this.#transformInput(root); @@ -139,8 +249,31 @@ export class MockDirectory { } } + /** + * Reads the content of the mock directory. + * + * @remarks + * + * Text files will be returned as strings, while binary files will be returned as buffers. + * By default the file extension is used to determine whether a file should be read as text. + * + * @example + * ```ts + * await expect(mockDir.content()).resolves.toEqual({ + * 'test.txt': 'content', + * 'sub-dir': { + * 'file.txt': 'content', + * 'nested-dir': { + * 'file.txt': 'content', + * }, + * }, + * 'empty-dir': {}, + * 'binary-file': Buffer.from([0, 1, 2]), + * }); + * ``` + */ async content( - options?: DirectoryMockerContentOptions, + options?: MockDirectoryContentOptions, ): Promise { const shouldReadAsText = (typeof options?.shouldReadAsText === 'boolean' @@ -188,10 +321,16 @@ export class MockDirectory { return read(root); } + /** + * Clears the content of the mock directory, ensuring that the directory itself exists. + */ clear = async () => { await this.setContent({}); }; + /** + * Removes the mock directory and all its contents. + */ cleanup = async () => { await fs.rm(this.#root, { recursive: true, force: true }); }; From 4606abef326ec47ceed07cd2e284e3f8712288f7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 12:17:37 +0200 Subject: [PATCH 15/42] backend-test-utils: MockDirectory API report updates Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 32 +++++++++++++++++++ .../src/filesystem/MockDirectory.ts | 16 +++++----- .../src/filesystem/index.ts | 7 +++- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index 4a27ac21a9..03df9de264 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -4,6 +4,7 @@ ```ts /// +/// import { Backend } from '@backstage/backend-app-api'; import { BackendFeature } from '@backstage/backend-plugin-api'; @@ -33,6 +34,37 @@ import { UrlReaderService } from '@backstage/backend-plugin-api'; // @public (undocumented) export function isDockerDisabledForTests(): boolean; +// @public +export class MockDirectory { + addContent(root: MockDirectoryContent): Promise; + cleanup: () => Promise; + clear: () => Promise; + content( + options?: MockDirectoryContentOptions, + ): Promise; + static create(options?: MockDirectoryCreateOptions): MockDirectory; + static mockOsTmpDir(): MockDirectory; + get path(): string; + resolve(...paths: string[]): string; + setContent(root: MockDirectoryContent): Promise; +} + +// @public +export type MockDirectoryContent = { + [name in string]: MockDirectoryContent | string | Buffer; +}; + +// @public +export interface MockDirectoryContentOptions { + path?: string; + shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); +} + +// @public +export interface MockDirectoryCreateOptions { + root?: string; +} + // @public (undocumented) export namespace mockServices { // (undocumented) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 2ef61b3d0c..81a65783fb 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -129,7 +129,7 @@ export class MockDirectory { * }) * ``` */ - static create(options?: MockDirectoryCreateOptions) { + static create(options?: MockDirectoryCreateOptions): MockDirectory { const root = options?.root ?? fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); @@ -156,7 +156,7 @@ export class MockDirectory { * * @returns */ - static mockOsTmpDir() { + static mockOsTmpDir(): MockDirectory { const mocker = MockDirectory.create(); const origTmpdir = os.tmpdir; os.tmpdir = () => mocker.path; @@ -180,14 +180,14 @@ export class MockDirectory { /** * The path to the root of the mock directory */ - get path() { + get path(): string { return this.#root; } /** * Resolves a path relative to the root of the mock directory. */ - resolve(...paths: string[]) { + resolve(...paths: string[]): string { return resolvePath(this.#root, ...paths); } @@ -207,7 +207,7 @@ export class MockDirectory { * }); * ``` */ - async setContent(root: MockDirectoryContent) { + async setContent(root: MockDirectoryContent): Promise { await this.cleanup(); return this.addContent(root); @@ -229,7 +229,7 @@ export class MockDirectory { * }); * ``` */ - async addContent(root: MockDirectoryContent) { + async addContent(root: MockDirectoryContent): Promise { const entries = this.#transformInput(root); for (const entry of entries) { @@ -324,14 +324,14 @@ export class MockDirectory { /** * Clears the content of the mock directory, ensuring that the directory itself exists. */ - clear = async () => { + clear = async (): Promise => { await this.setContent({}); }; /** * Removes the mock directory and all its contents. */ - cleanup = async () => { + cleanup = async (): Promise => { await fs.rm(this.#root, { recursive: true, force: true }); }; diff --git a/packages/backend-test-utils/src/filesystem/index.ts b/packages/backend-test-utils/src/filesystem/index.ts index f0b05641f5..d1077db980 100644 --- a/packages/backend-test-utils/src/filesystem/index.ts +++ b/packages/backend-test-utils/src/filesystem/index.ts @@ -14,4 +14,9 @@ * limitations under the License. */ -export { MockDirectory } from './MockDirectory'; +export { + MockDirectory, + type MockDirectoryContent, + type MockDirectoryContentOptions, + type MockDirectoryCreateOptions, +} from './MockDirectory'; From a250ad775f0bd48eaf1ce09b959664f1562cee09 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 12:55:02 +0200 Subject: [PATCH 16/42] changesets: add changeset for MockDirectory Signed-off-by: Patrik Oldsberg --- .changeset/flat-pandas-tan.md | 5 +++++ .changeset/short-terms-check.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/flat-pandas-tan.md create mode 100644 .changeset/short-terms-check.md diff --git a/.changeset/flat-pandas-tan.md b/.changeset/flat-pandas-tan.md new file mode 100644 index 0000000000..8461c9beb8 --- /dev/null +++ b/.changeset/flat-pandas-tan.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-test-utils': patch +--- + +Added `MockDirectory` to help out with file system mocking in tests. diff --git a/.changeset/short-terms-check.md b/.changeset/short-terms-check.md new file mode 100644 index 0000000000..69d85fcb9c --- /dev/null +++ b/.changeset/short-terms-check.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Removed `mock-fs` dev dependency. From 3f3714ab4170913c51a5c6b77bfb04d503b3cbad Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 12:55:27 +0200 Subject: [PATCH 17/42] backend-test-utils: rename MockDirectory.cleanup -> .remove Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 2 +- .../backend-test-utils/src/filesystem/MockDirectory.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index 03df9de264..0262ef339d 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -37,7 +37,6 @@ export function isDockerDisabledForTests(): boolean; // @public export class MockDirectory { addContent(root: MockDirectoryContent): Promise; - cleanup: () => Promise; clear: () => Promise; content( options?: MockDirectoryContentOptions, @@ -45,6 +44,7 @@ export class MockDirectory { static create(options?: MockDirectoryCreateOptions): MockDirectory; static mockOsTmpDir(): MockDirectory; get path(): string; + remove: () => Promise; resolve(...paths: string[]): string; setContent(root: MockDirectoryContent): Promise; } diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 81a65783fb..cd85a4c347 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -138,10 +138,10 @@ export class MockDirectory { const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); if (shouldCleanup) { - process.on('beforeExit', mocker.#cleanupSync); + process.on('beforeExit', mocker.#removeSync); try { - afterAll(mocker.cleanup); + afterAll(mocker.remove); } catch { /* ignore */ } @@ -208,7 +208,7 @@ export class MockDirectory { * ``` */ async setContent(root: MockDirectoryContent): Promise { - await this.cleanup(); + await this.remove(); return this.addContent(root); } @@ -331,7 +331,7 @@ export class MockDirectory { /** * Removes the mock directory and all its contents. */ - cleanup = async (): Promise => { + remove = async (): Promise => { await fs.rm(this.#root, { recursive: true, force: true }); }; @@ -361,7 +361,7 @@ export class MockDirectory { return entries; } - #cleanupSync = () => { + #removeSync = () => { fs.rmSync(this.#root, { recursive: true, force: true }); }; } From b516ebc8e70919ffdf208ad9c41b82cacb5384fd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 13:17:22 +0200 Subject: [PATCH 18/42] backend-test-utils: MockDirectory tests + fixes Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 159 ++++++++++++++++++ .../src/filesystem/MockDirectory.ts | 18 +- 2 files changed, 169 insertions(+), 8 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 22563b7a59..562acb3a8a 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -14,11 +14,28 @@ * limitations under the License. */ +import fs from 'fs-extra'; +import os from 'os'; +import { relative as relativePath } from 'path'; import { MockDirectory } from './MockDirectory'; describe('MockDirectory', () => { const mockDir = MockDirectory.create(); + beforeEach(mockDir.clear); + + it('should resolve paths', () => { + expect(mockDir.path).toEqual(expect.any(String)); + expect(relativePath(mockDir.path, mockDir.resolve('a'))).toBe('a'); + expect(relativePath(mockDir.path, mockDir.resolve('a/b/c'))).toBe('a/b/c'); + }); + + it('should remove itself', async () => { + await expect(fs.pathExists(mockDir.path)).resolves.toBe(true); + await mockDir.remove(); + await expect(fs.pathExists(mockDir.path)).resolves.toBe(false); + }); + it('should populate a directory with text files', async () => { await mockDir.setContent({ 'a.txt': 'a', @@ -100,6 +117,102 @@ describe('MockDirectory', () => { }); }); + it('should read content from sub dirs', async () => { + await mockDir.setContent({ + 'a.txt': 'a', + 'b/b.txt': 'b', + 'b/c/c.txt': 'c', + }); + + const expected = { + 'a.txt': 'a', + b: { + 'b.txt': 'b', + c: { + 'c.txt': 'c', + }, + }, + }; + + await expect(mockDir.content()).resolves.toEqual(expected); + await expect(mockDir.content({ path: mockDir.path })).resolves.toEqual( + expected, + ); + await expect( + mockDir.content({ path: mockDir.resolve('.') }), + ).resolves.toEqual(expected); + await expect(mockDir.content({ path: 'b' })).resolves.toEqual(expected.b); + await expect(mockDir.content({ path: './b' })).resolves.toEqual(expected.b); + await expect( + mockDir.content({ path: mockDir.resolve('b') }), + ).resolves.toEqual(expected.b); + await expect(mockDir.content({ path: 'b/c' })).resolves.toEqual( + expected.b.c, + ); + await expect(mockDir.content({ path: './b/c' })).resolves.toEqual( + expected.b.c, + ); + await expect( + mockDir.content({ path: mockDir.resolve('b/c') }), + ).resolves.toEqual(expected.b.c); + await expect( + mockDir.content({ path: mockDir.resolve('b', 'c') }), + ).resolves.toEqual(expected.b.c); + }); + + it('should allow text reading to be configured', async () => { + const text = 'a'; + const binary = Buffer.from('a', 'utf8'); + + await mockDir.setContent({ + a: binary, + 'a.txt': text, + 'a.bin': binary, + }); + + await expect(mockDir.content()).resolves.toEqual({ + a: binary, + 'a.txt': text, + 'a.bin': binary, + }); + + await expect(mockDir.content({ shouldReadAsText: false })).resolves.toEqual( + { + a: binary, + 'a.txt': binary, + 'a.bin': binary, + }, + ); + + await expect(mockDir.content({ shouldReadAsText: true })).resolves.toEqual({ + a: text, + 'a.txt': text, + 'a.bin': text, + }); + + await expect( + mockDir.content({ shouldReadAsText: path => path.length > 3 }), + ).resolves.toEqual({ + a: binary, + 'a.txt': text, + 'a.bin': text, + }); + }); + + it('should provide a posix path to shouldReadAsText', async () => { + const shouldReadAsText = jest.fn().mockReturnValue(true); + + await mockDir.setContent({ 'a/b/c': 'c' }); + + await expect(mockDir.content({ shouldReadAsText })).resolves.toEqual({ + a: { b: { c: 'c' } }, + }); + expect(shouldReadAsText).toHaveBeenCalledWith( + 'a/b/c', + Buffer.from('c', 'utf8'), + ); + }); + it('should not override directories', async () => { await mockDir.setContent({ 'a.txt': 'a', @@ -153,6 +266,18 @@ describe('MockDirectory', () => { }); }); + it('should reject non-child paths', async () => { + await expect(mockDir.setContent({ '/root/a.txt': 'a' })).rejects.toThrow( + "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + ); + await expect(mockDir.addContent({ '/root/a.txt': 'a' })).rejects.toThrow( + "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + ); + await expect(mockDir.content({ path: '/root/a.txt' })).rejects.toThrow( + "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + ); + }); + describe('cleanup', () => { let cleanupMockDir: MockDirectory; @@ -174,4 +299,38 @@ describe('MockDirectory', () => { await expect(cleanupMockDir.content()).resolves.toBeUndefined(); }); }); + + describe('tmpdir mock', () => { + let tmpDirMock: MockDirectory; + + describe('inner', () => { + tmpDirMock = MockDirectory.mockOsTmpDir(); + + it('should mock os.tmpdir()', async () => { + expect(os.tmpdir()).toBe(tmpDirMock.path); + }); + }); + + it('should restore os.tmpdir()', async () => { + expect(os.tmpdir()).not.toBe(tmpDirMock.path); + }); + }); + + describe('existing directory', () => { + let existingMockDir: MockDirectory; + + describe('inner', () => { + existingMockDir = MockDirectory.create({ root: __dirname }); // hardcore mode + + it('should read existing directory', async () => { + await expect(existingMockDir.content()).resolves.toMatchObject({ + 'index.ts': expect.any(String), + }); + }); + }); + + it('should remove existing directory', async () => { + await expect(fs.pathExists(__dirname)).resolves.toBe(true); + }); + }); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index cd85a4c347..5553982484 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -15,7 +15,7 @@ */ import os from 'os'; -import { isChildPath, resolveSafeChildPath } from '@backstage/backend-common'; +import { isChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import textextensions from 'textextensions'; import { tmpdir as getTmpDir } from 'os'; @@ -25,6 +25,8 @@ import { join as joinPath, resolve as resolvePath, relative as relativePath, + win32, + posix, } from 'path'; /** @@ -233,10 +235,10 @@ export class MockDirectory { const entries = this.#transformInput(root); for (const entry of entries) { - const fullPath = resolveSafeChildPath(this.#root, entry.path); + const fullPath = resolvePath(this.#root, entry.path); if (!isChildPath(this.#root, fullPath)) { throw new Error( - `Provided path must resolve to a child path of the mock directory, got ${entry.path}`, + `Provided path must resolve to a child path of the mock directory, got '${entry.path}'`, ); } @@ -284,10 +286,7 @@ export class MockDirectory { const root = resolvePath(this.#root, options?.path ?? ''); if (!isChildPath(this.#root, root)) { throw new Error( - `Provided path must resolve to a child path of the mock directory, got ${relativePath( - this.#root, - root, - )}`, + `Provided path must resolve to a child path of the mock directory, got '${root}'`, ); } @@ -308,8 +307,11 @@ export class MockDirectory { return [entry.name, await read(fullPath)]; } const content = await fs.readFile(fullPath); + const relativePosixPath = relativePath(root, fullPath) + .split(win32.sep) + .join(posix.sep); - if (shouldReadAsText(fullPath, content)) { + if (shouldReadAsText(relativePosixPath, content)) { return [entry.name, content.toString('utf8')]; } return [entry.name, content]; From 71b63f6ad129fd9bcef56b0fec8e976f99ca6a86 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 17:33:21 +0200 Subject: [PATCH 19/42] backend-test-utils: MockDirectory fixes for windows Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/package.json | 1 + .../src/filesystem/MockDirectory.test.ts | 17 ++++++++++++----- .../src/filesystem/MockDirectory.ts | 12 +++++++++++- yarn.lock | 1 + 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index da626d3786..0a91508400 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -46,6 +46,7 @@ "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/config": "workspace:^", + "@backstage/errors": "workspace:^", "@backstage/plugin-auth-node": "workspace:^", "@backstage/types": "workspace:^", "better-sqlite3": "^8.0.0", diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 562acb3a8a..a7fe1815b2 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -16,7 +16,11 @@ import fs from 'fs-extra'; import os from 'os'; -import { relative as relativePath } from 'path'; +import { + join as joinPath, + resolve as resolvePath, + relative as relativePath, +} from 'path'; import { MockDirectory } from './MockDirectory'; describe('MockDirectory', () => { @@ -27,7 +31,9 @@ describe('MockDirectory', () => { it('should resolve paths', () => { expect(mockDir.path).toEqual(expect.any(String)); expect(relativePath(mockDir.path, mockDir.resolve('a'))).toBe('a'); - expect(relativePath(mockDir.path, mockDir.resolve('a/b/c'))).toBe('a/b/c'); + expect(relativePath(mockDir.path, mockDir.resolve('a/b/c'))).toBe( + joinPath('a', 'b', 'c'), + ); }); it('should remove itself', async () => { @@ -267,14 +273,15 @@ describe('MockDirectory', () => { }); it('should reject non-child paths', async () => { + const path = resolvePath('/root/a.txt'); await expect(mockDir.setContent({ '/root/a.txt': 'a' })).rejects.toThrow( - "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); await expect(mockDir.addContent({ '/root/a.txt': 'a' })).rejects.toThrow( - "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); await expect(mockDir.content({ path: '/root/a.txt' })).rejects.toThrow( - "Provided path must resolve to a child path of the mock directory, got '/root/a.txt'", + `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 5553982484..371a315f22 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -28,6 +28,7 @@ import { win32, posix, } from 'path'; +import { isError } from '@backstage/errors'; /** * The content of a mock directory represented by a nested object structure. @@ -334,7 +335,16 @@ export class MockDirectory { * Removes the mock directory and all its contents. */ remove = async (): Promise => { - await fs.rm(this.#root, { recursive: true, force: true }); + try { + await fs.rm(this.#root, { recursive: true, force: true }); + } catch (error: unknown) { + if (isError(error) && error.code === 'ENOTEMPTY') { + // Windows can be a bit flaky, give it another go + await fs.rm(this.#root, { recursive: true, force: true }); + } else { + throw error; + } + } }; #transformInput(input: MockDirectoryContent[string]): MockEntry[] { diff --git a/yarn.lock b/yarn.lock index dc8e58da96..20922a9959 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3642,6 +3642,7 @@ __metadata: "@backstage/backend-plugin-api": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" + "@backstage/errors": "workspace:^" "@backstage/plugin-auth-node": "workspace:^" "@backstage/types": "workspace:^" "@types/supertest": ^2.0.8 From 95dab5827e50c25f78d81a4d5aaaaed06674f229 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Sep 2023 17:36:53 +0200 Subject: [PATCH 20/42] backend-common: disable GerritUrlReader tests again Signed-off-by: Patrik Oldsberg --- packages/backend-common/src/reading/GerritUrlReader.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/src/reading/GerritUrlReader.test.ts b/packages/backend-common/src/reading/GerritUrlReader.test.ts index d3c336ee93..e5a299d0b8 100644 --- a/packages/backend-common/src/reading/GerritUrlReader.test.ts +++ b/packages/backend-common/src/reading/GerritUrlReader.test.ts @@ -86,7 +86,10 @@ const createReader = (config: JsonObject): UrlReaderPredicateTuple[] => { }); }; -describe('GerritUrlReader', () => { +// TODO(Rugvip): These tests seem to be a direct or indirect cause of the TaskWorker test flakiness +// We're not sure why at this point, but while investigating these tests are disabled +// eslint-disable-next-line jest/no-disabled-tests +describe.skip('GerritUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); From 7d275f4587a641e13d8a9d9c503a55c6cdc25078 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 Sep 2023 11:48:12 +0200 Subject: [PATCH 21/42] backend-test-utils: refactor MockDirectory to be synchronous Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 10 +- .../src/filesystem/MockDirectory.test.ts | 146 ++++++++---------- .../src/filesystem/MockDirectory.ts | 80 +++++----- 3 files changed, 108 insertions(+), 128 deletions(-) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index 0262ef339d..dff218edb1 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -36,17 +36,17 @@ export function isDockerDisabledForTests(): boolean; // @public export class MockDirectory { - addContent(root: MockDirectoryContent): Promise; - clear: () => Promise; + addContent(root: MockDirectoryContent): void; + clear: () => void; content( options?: MockDirectoryContentOptions, - ): Promise; + ): MockDirectoryContent | undefined; static create(options?: MockDirectoryCreateOptions): MockDirectory; static mockOsTmpDir(): MockDirectory; get path(): string; - remove: () => Promise; + remove: () => void; resolve(...paths: string[]): string; - setContent(root: MockDirectoryContent): Promise; + setContent(root: MockDirectoryContent): void; } // @public diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index a7fe1815b2..19c56119f0 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -38,19 +38,19 @@ describe('MockDirectory', () => { it('should remove itself', async () => { await expect(fs.pathExists(mockDir.path)).resolves.toBe(true); - await mockDir.remove(); + mockDir.remove(); await expect(fs.pathExists(mockDir.path)).resolves.toBe(false); }); - it('should populate a directory with text files', async () => { - await mockDir.setContent({ + it('should populate a directory with text files', () => { + mockDir.setContent({ 'a.txt': 'a', 'a/b.txt': 'b', 'a/b/c.txt': 'c', 'a/b/d.txt': 'd', }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -62,15 +62,15 @@ describe('MockDirectory', () => { }); }); - it('should mix text and binary files', async () => { - await mockDir.setContent({ + it('should mix text and binary files', () => { + mockDir.setContent({ 'a.txt': 'a', 'a/b.txt': 'b', 'a/b/c.bin': Buffer.from([0xc]), 'a/b/d.bin': Buffer.from([0xd]), }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a', a: { 'b.txt': 'b', @@ -82,25 +82,25 @@ describe('MockDirectory', () => { }); }); - it('should be able to add content', async () => { - await mockDir.setContent({ + it('should be able to add content', () => { + mockDir.setContent({ 'a.txt': 'a', b: {}, }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a', b: {}, }); - await mockDir.addContent({ + mockDir.addContent({ 'b.txt': 'b', b: { 'c.txt': 'c', }, }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a', 'b.txt': 'b', b: { @@ -109,22 +109,22 @@ describe('MockDirectory', () => { }); }); - it('should replace existing files', async () => { - await mockDir.setContent({ + it('should replace existing files', () => { + mockDir.setContent({ 'a.txt': 'a', }); - await mockDir.addContent({ + mockDir.addContent({ 'a.txt': 'a2', }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'a.txt': 'a2', }); }); - it('should read content from sub dirs', async () => { - await mockDir.setContent({ + it('should read content from sub dirs', () => { + mockDir.setContent({ 'a.txt': 'a', 'b/b.txt': 'b', 'b/c/c.txt': 'c', @@ -140,77 +140,65 @@ describe('MockDirectory', () => { }, }; - await expect(mockDir.content()).resolves.toEqual(expected); - await expect(mockDir.content({ path: mockDir.path })).resolves.toEqual( - expected, - ); - await expect( - mockDir.content({ path: mockDir.resolve('.') }), - ).resolves.toEqual(expected); - await expect(mockDir.content({ path: 'b' })).resolves.toEqual(expected.b); - await expect(mockDir.content({ path: './b' })).resolves.toEqual(expected.b); - await expect( - mockDir.content({ path: mockDir.resolve('b') }), - ).resolves.toEqual(expected.b); - await expect(mockDir.content({ path: 'b/c' })).resolves.toEqual( + expect(mockDir.content()).toEqual(expected); + expect(mockDir.content({ path: mockDir.path })).toEqual(expected); + expect(mockDir.content({ path: mockDir.resolve('.') })).toEqual(expected); + expect(mockDir.content({ path: 'b' })).toEqual(expected.b); + expect(mockDir.content({ path: './b' })).toEqual(expected.b); + expect(mockDir.content({ path: mockDir.resolve('b') })).toEqual(expected.b); + expect(mockDir.content({ path: 'b/c' })).toEqual(expected.b.c); + expect(mockDir.content({ path: './b/c' })).toEqual(expected.b.c); + expect(mockDir.content({ path: mockDir.resolve('b/c') })).toEqual( expected.b.c, ); - await expect(mockDir.content({ path: './b/c' })).resolves.toEqual( + expect(mockDir.content({ path: mockDir.resolve('b', 'c') })).toEqual( expected.b.c, ); - await expect( - mockDir.content({ path: mockDir.resolve('b/c') }), - ).resolves.toEqual(expected.b.c); - await expect( - mockDir.content({ path: mockDir.resolve('b', 'c') }), - ).resolves.toEqual(expected.b.c); }); - it('should allow text reading to be configured', async () => { + it('should allow text reading to be configured', () => { const text = 'a'; const binary = Buffer.from('a', 'utf8'); - await mockDir.setContent({ + mockDir.setContent({ a: binary, 'a.txt': text, 'a.bin': binary, }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ a: binary, 'a.txt': text, 'a.bin': binary, }); - await expect(mockDir.content({ shouldReadAsText: false })).resolves.toEqual( - { - a: binary, - 'a.txt': binary, - 'a.bin': binary, - }, - ); + expect(mockDir.content({ shouldReadAsText: false })).toEqual({ + a: binary, + 'a.txt': binary, + 'a.bin': binary, + }); - await expect(mockDir.content({ shouldReadAsText: true })).resolves.toEqual({ + expect(mockDir.content({ shouldReadAsText: true })).toEqual({ a: text, 'a.txt': text, 'a.bin': text, }); - await expect( + expect( mockDir.content({ shouldReadAsText: path => path.length > 3 }), - ).resolves.toEqual({ + ).toEqual({ a: binary, 'a.txt': text, 'a.bin': text, }); }); - it('should provide a posix path to shouldReadAsText', async () => { + it('should provide a posix path to shouldReadAsText', () => { const shouldReadAsText = jest.fn().mockReturnValue(true); - await mockDir.setContent({ 'a/b/c': 'c' }); + mockDir.setContent({ 'a/b/c': 'c' }); - await expect(mockDir.content({ shouldReadAsText })).resolves.toEqual({ + expect(mockDir.content({ shouldReadAsText })).toEqual({ a: { b: { c: 'c' } }, }); expect(shouldReadAsText).toHaveBeenCalledWith( @@ -219,27 +207,27 @@ describe('MockDirectory', () => { ); }); - it('should not override directories', async () => { - await mockDir.setContent({ + it('should not override directories', () => { + mockDir.setContent({ 'a.txt': 'a', b: {}, }); - await expect( + expect(() => mockDir.addContent({ 'a.txt': {}, }), - ).rejects.toThrow('EEXIST'); + ).toThrow('EEXIST'); - await expect( + expect(() => mockDir.addContent({ b: 'b', }), - ).rejects.toThrow('EISDIR'); + ).toThrow('EISDIR'); }); - it('examples should work', async () => { - await mockDir.setContent({ + it('examples should work', () => { + mockDir.setContent({ 'test.txt': 'content', 'sub-dir': { 'file.txt': 'content', @@ -249,7 +237,7 @@ describe('MockDirectory', () => { 'binary-file': Buffer.from([0, 1, 2]), }); - await mockDir.addContent({ + mockDir.addContent({ 'test.txt': 'content', 'sub-dir': { 'file.txt': 'content', @@ -259,7 +247,7 @@ describe('MockDirectory', () => { 'binary-file': Buffer.from([0, 1, 2]), }); - await expect(mockDir.content()).resolves.toEqual({ + expect(mockDir.content()).toEqual({ 'test.txt': 'content', 'sub-dir': { 'file.txt': 'content', @@ -272,15 +260,15 @@ describe('MockDirectory', () => { }); }); - it('should reject non-child paths', async () => { + it('should reject non-child paths', () => { const path = resolvePath('/root/a.txt'); - await expect(mockDir.setContent({ '/root/a.txt': 'a' })).rejects.toThrow( + expect(() => mockDir.setContent({ '/root/a.txt': 'a' })).toThrow( `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); - await expect(mockDir.addContent({ '/root/a.txt': 'a' })).rejects.toThrow( + expect(() => mockDir.addContent({ '/root/a.txt': 'a' })).toThrow( `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); - await expect(mockDir.content({ path: '/root/a.txt' })).rejects.toThrow( + expect(() => mockDir.content({ path: '/root/a.txt' })).toThrow( `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); }); @@ -291,19 +279,19 @@ describe('MockDirectory', () => { describe('inner', () => { cleanupMockDir = MockDirectory.create(); - it('should populate a directory', async () => { - await cleanupMockDir.setContent({ + it('should populate a directory', () => { + cleanupMockDir.setContent({ 'a.txt': 'a', }); - await expect(cleanupMockDir.content()).resolves.toEqual({ + expect(cleanupMockDir.content()).toEqual({ 'a.txt': 'a', }); }); }); - it('should clean up after itself automatically', async () => { - await expect(cleanupMockDir.content()).resolves.toBeUndefined(); + it('should clean up after itself automatically', () => { + expect(cleanupMockDir.content()).toBeUndefined(); }); }); @@ -313,12 +301,12 @@ describe('MockDirectory', () => { describe('inner', () => { tmpDirMock = MockDirectory.mockOsTmpDir(); - it('should mock os.tmpdir()', async () => { + it('should mock os.tmpdir()', () => { expect(os.tmpdir()).toBe(tmpDirMock.path); }); }); - it('should restore os.tmpdir()', async () => { + it('should restore os.tmpdir()', () => { expect(os.tmpdir()).not.toBe(tmpDirMock.path); }); }); @@ -329,15 +317,15 @@ describe('MockDirectory', () => { describe('inner', () => { existingMockDir = MockDirectory.create({ root: __dirname }); // hardcore mode - it('should read existing directory', async () => { - await expect(existingMockDir.content()).resolves.toMatchObject({ + it('should read existing directory', () => { + expect(existingMockDir.content()).toMatchObject({ 'index.ts': expect.any(String), }); }); }); - it('should remove existing directory', async () => { - await expect(fs.pathExists(__dirname)).resolves.toBe(true); + it('should remove existing directory', () => { + expect(fs.pathExistsSync(__dirname)).toBe(true); }); }); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 371a315f22..b452fdcc08 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -124,9 +124,9 @@ export class MockDirectory { * describe('MySubject', () => { * const mockDir = MockDirectory.create(); * - * beforeEach(() => mockDir.clear()); + * beforeEach(mockDir.clear); * - * it('should work', async () => { + * it('should work', () => { * // ... use mockDir * }) * }) @@ -141,7 +141,7 @@ export class MockDirectory { const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); if (shouldCleanup) { - process.on('beforeExit', mocker.#removeSync); + process.on('beforeExit', mocker.remove); try { afterAll(mocker.remove); @@ -199,7 +199,7 @@ export class MockDirectory { * * @example * ```ts - * await mockDir.setContent({ + * mockDir.setContent({ * 'test.txt': 'content', * 'sub-dir': { * 'file.txt': 'content', @@ -210,8 +210,8 @@ export class MockDirectory { * }); * ``` */ - async setContent(root: MockDirectoryContent): Promise { - await this.remove(); + setContent(root: MockDirectoryContent): void { + this.remove(); return this.addContent(root); } @@ -221,7 +221,7 @@ export class MockDirectory { * * @example * ```ts - * await mockDir.addContent({ + * mockDir.addContent({ * 'test.txt': 'content', * 'sub-dir': { * 'file.txt': 'content', @@ -232,7 +232,7 @@ export class MockDirectory { * }); * ``` */ - async addContent(root: MockDirectoryContent): Promise { + addContent(root: MockDirectoryContent): void { const entries = this.#transformInput(root); for (const entry of entries) { @@ -244,10 +244,10 @@ export class MockDirectory { } if (entry.type === 'dir') { - await fs.ensureDir(fullPath); + fs.ensureDirSync(fullPath); } else if (entry.type === 'file') { - await fs.ensureDir(dirname(fullPath)); - await fs.writeFile(fullPath, entry.content); + fs.ensureDirSync(dirname(fullPath)); + fs.writeFileSync(fullPath, entry.content); } } } @@ -262,7 +262,7 @@ export class MockDirectory { * * @example * ```ts - * await expect(mockDir.content()).resolves.toEqual({ + * expect(mockDir.content()).toEqual({ * 'test.txt': 'content', * 'sub-dir': { * 'file.txt': 'content', @@ -275,9 +275,9 @@ export class MockDirectory { * }); * ``` */ - async content( + content( options?: MockDirectoryContentOptions, - ): Promise { + ): MockDirectoryContent | undefined { const shouldReadAsText = (typeof options?.shouldReadAsText === 'boolean' ? () => options?.shouldReadAsText @@ -291,33 +291,29 @@ export class MockDirectory { ); } - async function read( - path: string, - ): Promise { - if (!(await fs.pathExists(path))) { + function read(path: string): MockDirectoryContent | undefined { + if (!fs.pathExistsSync(path)) { return undefined; } - const entries = await fs.readdir(path, { withFileTypes: true }); + const entries = fs.readdirSync(path, { withFileTypes: true }); return Object.fromEntries( - await Promise.all( - entries.map(async entry => { - const fullPath = resolvePath(path, entry.name); + entries.map(entry => { + const fullPath = resolvePath(path, entry.name); - if (entry.isDirectory()) { - return [entry.name, await read(fullPath)]; - } - const content = await fs.readFile(fullPath); - const relativePosixPath = relativePath(root, fullPath) - .split(win32.sep) - .join(posix.sep); + if (entry.isDirectory()) { + return [entry.name, read(fullPath)]; + } + const content = fs.readFileSync(fullPath); + const relativePosixPath = relativePath(root, fullPath) + .split(win32.sep) + .join(posix.sep); - if (shouldReadAsText(relativePosixPath, content)) { - return [entry.name, content.toString('utf8')]; - } - return [entry.name, content]; - }), - ), + if (shouldReadAsText(relativePosixPath, content)) { + return [entry.name, content.toString('utf8')]; + } + return [entry.name, content]; + }), ); } @@ -327,20 +323,20 @@ export class MockDirectory { /** * Clears the content of the mock directory, ensuring that the directory itself exists. */ - clear = async (): Promise => { - await this.setContent({}); + clear = (): void => { + this.setContent({}); }; /** * Removes the mock directory and all its contents. */ - remove = async (): Promise => { + remove = (): void => { try { - await fs.rm(this.#root, { recursive: true, force: true }); + fs.rmSync(this.#root, { recursive: true, force: true }); } catch (error: unknown) { if (isError(error) && error.code === 'ENOTEMPTY') { // Windows can be a bit flaky, give it another go - await fs.rm(this.#root, { recursive: true, force: true }); + fs.rmSync(this.#root, { recursive: true, force: true }); } else { throw error; } @@ -372,8 +368,4 @@ export class MockDirectory { return entries; } - - #removeSync = () => { - fs.rmSync(this.#root, { recursive: true, force: true }); - }; } From 587644c044b938f2ead04af945faa76aa88e9109 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 Sep 2023 11:53:57 +0200 Subject: [PATCH 22/42] backend-common: update MockDirectory usage Signed-off-by: Patrik Oldsberg --- .../src/reading/AzureUrlReader.test.ts | 2 +- .../reading/BitbucketCloudUrlReader.test.ts | 2 +- .../src/reading/BitbucketUrlReader.test.ts | 2 +- .../src/reading/GerritUrlReader.test.ts | 4 ++-- .../src/reading/GithubUrlReader.test.ts | 2 +- .../src/reading/GitlabUrlReader.test.ts | 2 +- .../reading/tree/ReadableArrayResponse.test.ts | 8 ++++---- .../reading/tree/TarArchiveResponse.test.ts | 18 +++++++++--------- .../reading/tree/ZipArchiveResponse.test.ts | 16 ++++++++-------- .../src/util/DockerContainerRunner.test.ts | 6 +++--- 10 files changed, 31 insertions(+), 31 deletions(-) diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index b6338a1c70..45d64c5b96 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -73,7 +73,7 @@ const urlReaderFactory = (azureIntegration: AzureIntegrationConfigLike) => { }; describe('AzureUrlReader', () => { - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); const worker = setupServer(); setupRequestMockHandlers(worker); diff --git a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts index 18a186f793..7b1e0f9fb3 100644 --- a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts @@ -53,7 +53,7 @@ const reader = new BitbucketCloudUrlReader( ); describe('BitbucketCloudUrlReader', () => { - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); const worker = setupServer(); setupRequestMockHandlers(worker); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 986619d161..5e3884cf8e 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -71,7 +71,7 @@ describe('BitbucketUrlReader.factory', () => { describe('BitbucketUrlReader', () => { const mockDir = MockDirectory.mockOsTmpDir(); - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/GerritUrlReader.test.ts b/packages/backend-common/src/reading/GerritUrlReader.test.ts index e5a299d0b8..d91e49e533 100644 --- a/packages/backend-common/src/reading/GerritUrlReader.test.ts +++ b/packages/backend-common/src/reading/GerritUrlReader.test.ts @@ -93,7 +93,7 @@ describe.skip('GerritUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); afterAll(() => { jest.clearAllMocks(); @@ -256,7 +256,7 @@ describe.skip('GerritUrlReader', () => { ); beforeEach(async () => { - await mockDir.setContent({ + mockDir.setContent({ 'repo/mkdocs.yml': mkdocsContent, 'repo/docs/first.md': mdContent, }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 326d3632a4..82ce07a3a9 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -76,7 +76,7 @@ describe('GithubUrlReader', () => { const worker = setupServer(); setupRequestMockHandlers(worker); - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); beforeEach(() => { jest.clearAllMocks(); diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 6500f207f0..098d48458d 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -69,7 +69,7 @@ const hostedGitlabProcessor = new GitlabUrlReader( ); describe('GitlabUrlReader', () => { - beforeEach(() => mockDir.clear()); + beforeEach(mockDir.clear); const worker = setupServer(); setupRequestMockHandlers(worker); diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index 9f0fd5377e..1c95f653c1 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -34,12 +34,12 @@ describe('ReadableArrayResponse', () => { const sourceDir = MockDirectory.create(); const targetDir = MockDirectory.create(); - beforeEach(async () => { - await sourceDir.setContent({ + beforeEach(() => { + sourceDir.setContent({ [name1]: file1, [name2]: file2, }); - await targetDir.clear(); + targetDir.clear(); }); const path1 = sourceDir.resolve(name1); @@ -71,7 +71,7 @@ describe('ReadableArrayResponse', () => { const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); const dir = await res.dir(); - await expect(targetDir.content({ path: dir })).resolves.toEqual({ + expect(targetDir.content({ path: dir })).toEqual({ [name1]: file1.toString('utf8'), [name2]: file2.toString('utf8'), }); diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index ce126eb345..784a4fb9bf 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -27,11 +27,11 @@ describe('TarArchiveResponse', () => { const sourceDir = MockDirectory.create(); const targetDir = MockDirectory.create(); - beforeAll(async () => { - await sourceDir.setContent({ 'test-archive.tar.gz': archiveData }); + beforeAll(() => { + sourceDir.setContent({ 'test-archive.tar.gz': archiveData }); }); - beforeEach(async () => { - await targetDir.clear(); + beforeEach(() => { + targetDir.clear(); }); it('should read files', async () => { @@ -143,7 +143,7 @@ describe('TarArchiveResponse', () => { const res = new TarArchiveResponse(stream, 'docs', targetDir.path, 'etag'); const dir = await res.dir(); - await expect(targetDir.content({ path: dir })).resolves.toEqual({ + expect(targetDir.content({ path: dir })).toEqual({ 'index.md': '# Test\n', }); }); @@ -161,11 +161,11 @@ describe('TarArchiveResponse', () => { path => path.endsWith('.yml'), ); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const dir = await res.dir({ targetDir: targetDir.resolve('sub') }); expect(dir).toBe(targetDir.resolve('sub')); - await expect(targetDir.content()).resolves.toEqual({ + expect(targetDir.content()).toEqual({ sub: { 'mkdocs.yml': 'site_name: Test\n', }, @@ -187,7 +187,7 @@ describe('TarArchiveResponse', () => { }, ); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const sub = targetDir.resolve('sub'); const mkdtemp = jest @@ -216,7 +216,7 @@ describe('TarArchiveResponse', () => { }, ); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const sub = targetDir.resolve('sub'); await expect(fs.pathExists(sub)).resolves.toBe(true); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 20cf33241b..c92ee46aad 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -38,16 +38,16 @@ describe('ZipArchiveResponse', () => { const sourceDir = MockDirectory.create(); const targetDir = MockDirectory.create(); - beforeAll(async () => { - await sourceDir.setContent({ + beforeAll(() => { + sourceDir.setContent({ 'test-archive.zip': archiveData, 'test-archive-with-extra-root-dir.zip': archiveDataWithExtraDir, 'test-archive-corrupted.zip': archiveDataCorrupted, 'test-archive-malicious.zip': archiveWithMaliciousEntry, }); }); - beforeEach(async () => { - await targetDir.clear(); + beforeEach(() => { + targetDir.clear(); }); it('should read files', async () => { @@ -151,7 +151,7 @@ describe('ZipArchiveResponse', () => { const res = new ZipArchiveResponse(stream, 'docs/', targetDir.path, 'etag'); const dir = await res.dir(); - await expect(targetDir.content({ path: dir })).resolves.toEqual({ + expect(targetDir.content({ path: dir })).toEqual({ 'index.md': '# Test\n', }); }); @@ -167,13 +167,13 @@ describe('ZipArchiveResponse', () => { path => path.endsWith('.yml'), ); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const sub = targetDir.resolve('sub'); const dir = await res.dir({ targetDir: sub }); expect(dir).toBe(sub); - await expect(targetDir.content()).resolves.toEqual({ + expect(targetDir.content()).toEqual({ sub: { 'mkdocs.yml': 'site_name: Test\n', }, @@ -209,7 +209,7 @@ describe('ZipArchiveResponse', () => { const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); - await targetDir.addContent({ sub: {} }); + targetDir.addContent({ sub: {} }); const sub = targetDir.resolve('sub'); const dir = await res.dir({ targetDir: sub, diff --git a/packages/backend-common/src/util/DockerContainerRunner.test.ts b/packages/backend-common/src/util/DockerContainerRunner.test.ts index 6b4150540a..60f457a670 100644 --- a/packages/backend-common/src/util/DockerContainerRunner.test.ts +++ b/packages/backend-common/src/util/DockerContainerRunner.test.ts @@ -29,9 +29,9 @@ describe('DockerContainerRunner', () => { const inputDir = MockDirectory.create(); const outputDir = MockDirectory.create(); - beforeEach(async () => { - await inputDir.clear(); - await outputDir.clear(); + beforeEach(() => { + inputDir.clear(); + outputDir.clear(); jest.spyOn(mockDocker, 'pull').mockImplementation((async ( _image: string, From a71dbf4b34e12cd4e09ebdc00a918898d6b4dfbc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 Sep 2023 13:53:20 +0200 Subject: [PATCH 23/42] backend-test-utils: make sure MockDirectory contents are created with full permissions Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/src/filesystem/MockDirectory.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index b452fdcc08..809fd5ddf7 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -244,10 +244,10 @@ export class MockDirectory { } if (entry.type === 'dir') { - fs.ensureDirSync(fullPath); + fs.ensureDirSync(fullPath, { mode: 0o777 }); } else if (entry.type === 'file') { - fs.ensureDirSync(dirname(fullPath)); - fs.writeFileSync(fullPath, entry.content); + fs.ensureDirSync(dirname(fullPath), { mode: 0o777 }); + fs.writeFileSync(fullPath, entry.content, { mode: 0o666 }); } } } From 7c6141f54d82ff68bcf223936b1488d67eaa67d0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 22 Sep 2023 13:53:47 +0200 Subject: [PATCH 24/42] backend-test-utils: use built-in rmSync retries in MockDirectory Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 809fd5ddf7..499c7f29fb 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -28,7 +28,6 @@ import { win32, posix, } from 'path'; -import { isError } from '@backstage/errors'; /** * The content of a mock directory represented by a nested object structure. @@ -331,16 +330,7 @@ export class MockDirectory { * Removes the mock directory and all its contents. */ remove = (): void => { - try { - fs.rmSync(this.#root, { recursive: true, force: true }); - } catch (error: unknown) { - if (isError(error) && error.code === 'ENOTEMPTY') { - // Windows can be a bit flaky, give it another go - fs.rmSync(this.#root, { recursive: true, force: true }); - } else { - throw error; - } - } + fs.rmSync(this.#root, { recursive: true, force: true, maxRetries: 3 }); }; #transformInput(input: MockDirectoryContent[string]): MockEntry[] { From 82752ab5cbfde00e56da948af2f42ff666fce5ff Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 23 Sep 2023 16:41:04 +0200 Subject: [PATCH 25/42] backend-test-utils: throw resolved path when trying to write outside dir in MockDirectory Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/src/filesystem/MockDirectory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 499c7f29fb..2f949987f5 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -238,7 +238,7 @@ export class MockDirectory { const fullPath = resolvePath(this.#root, entry.path); if (!isChildPath(this.#root, fullPath)) { throw new Error( - `Provided path must resolve to a child path of the mock directory, got '${entry.path}'`, + `Provided path must resolve to a child path of the mock directory, got '${fullPath}'`, ); } From 0f58df31d6d5f9c226b10d225f5122437774fbad Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 23 Sep 2023 16:42:44 +0200 Subject: [PATCH 26/42] backend-test-utils: use remove instead of rm in MockDirectory Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/src/filesystem/MockDirectory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 2f949987f5..cb7ca5e726 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -330,7 +330,7 @@ export class MockDirectory { * Removes the mock directory and all its contents. */ remove = (): void => { - fs.rmSync(this.#root, { recursive: true, force: true, maxRetries: 3 }); + fs.removeSync(this.#root); }; #transformInput(input: MockDirectoryContent[string]): MockEntry[] { From 81726cd2d45a504e373baff347d8cfe3f7c7ef01 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Oct 2023 16:54:44 +0200 Subject: [PATCH 27/42] backend-test-utils: refactor to createMockDirectory + remove root option Signed-off-by: Patrik Oldsberg --- .changeset/flat-pandas-tan.md | 2 +- .../src/reading/AzureUrlReader.test.ts | 4 +- .../reading/BitbucketCloudUrlReader.test.ts | 4 +- .../reading/BitbucketServerUrlReader.test.ts | 4 +- .../src/reading/BitbucketUrlReader.test.ts | 4 +- .../src/reading/GerritUrlReader.test.ts | 4 +- .../src/reading/GithubUrlReader.test.ts | 4 +- .../src/reading/GitlabUrlReader.test.ts | 4 +- .../tree/ReadableArrayResponse.test.ts | 6 +- .../reading/tree/TarArchiveResponse.test.ts | 6 +- .../reading/tree/ZipArchiveResponse.test.ts | 6 +- .../src/util/DockerContainerRunner.test.ts | 6 +- packages/backend-test-utils/api-report.md | 19 +- .../src/filesystem/MockDirectory.test.ts | 28 +- .../src/filesystem/MockDirectory.ts | 273 +++++++++--------- .../src/filesystem/index.ts | 5 +- 16 files changed, 186 insertions(+), 193 deletions(-) diff --git a/.changeset/flat-pandas-tan.md b/.changeset/flat-pandas-tan.md index 8461c9beb8..dfe7965bec 100644 --- a/.changeset/flat-pandas-tan.md +++ b/.changeset/flat-pandas-tan.md @@ -2,4 +2,4 @@ '@backstage/backend-test-utils': patch --- -Added `MockDirectory` to help out with file system mocking in tests. +Added `createMockDirectory()` to help out with file system mocking in tests. diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index 45d64c5b96..2b4bc84be7 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -24,7 +24,7 @@ import { AzureIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -44,7 +44,7 @@ type AzureIntegrationConfigLike = Partial< const logger = getVoidLogger(); -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts index 7b1e0f9fb3..dc182af619 100644 --- a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts @@ -20,7 +20,7 @@ import { readBitbucketCloudIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -32,7 +32,7 @@ import { BitbucketCloudUrlReader } from './BitbucketCloudUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; import getRawBody from 'raw-body'; -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts index 82b467f70f..2870138535 100644 --- a/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketServerUrlReader.test.ts @@ -20,7 +20,7 @@ import { readBitbucketServerIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -31,7 +31,7 @@ import { NotModifiedError } from '@backstage/errors'; import { BitbucketServerUrlReader } from './BitbucketServerUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; -MockDirectory.mockOsTmpDir(); +createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 5e3884cf8e..4258ac9d5b 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -20,7 +20,7 @@ import { readBitbucketIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -69,7 +69,7 @@ describe('BitbucketUrlReader.factory', () => { }); describe('BitbucketUrlReader', () => { - const mockDir = MockDirectory.mockOsTmpDir(); + const mockDir = createMockDirectory({ mockOsTmpDir: true }); beforeEach(mockDir.clear); diff --git a/packages/backend-common/src/reading/GerritUrlReader.test.ts b/packages/backend-common/src/reading/GerritUrlReader.test.ts index d91e49e533..4a3b0a5de6 100644 --- a/packages/backend-common/src/reading/GerritUrlReader.test.ts +++ b/packages/backend-common/src/reading/GerritUrlReader.test.ts @@ -15,7 +15,7 @@ */ import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import { ConfigReader } from '@backstage/config'; @@ -35,7 +35,7 @@ import { DefaultReadTreeResponseFactory } from './tree'; import { GerritUrlReader } from './GerritUrlReader'; import getRawBody from 'raw-body'; -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 82ce07a3a9..5e6d30f519 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -21,7 +21,7 @@ import { readGithubIntegrationConfig, } from '@backstage/integration'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -38,7 +38,7 @@ import { } from './GithubUrlReader'; import { DefaultReadTreeResponseFactory } from './tree'; -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 098d48458d..3c065e8b81 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -16,7 +16,7 @@ import { ConfigReader } from '@backstage/config'; import { - MockDirectory, + createMockDirectory, setupRequestMockHandlers, } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; @@ -34,7 +34,7 @@ import { const logger = getVoidLogger(); -const mockDir = MockDirectory.mockOsTmpDir(); +const mockDir = createMockDirectory({ mockOsTmpDir: true }); const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index 1c95f653c1..4108697bce 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -18,7 +18,7 @@ import fs from 'fs-extra'; import path from 'path'; import { FromReadableArrayOptions } from '../types'; import { ReadableArrayResponse } from './ReadableArrayResponse'; -import { MockDirectory } from '@backstage/backend-test-utils'; +import { createMockDirectory } from '@backstage/backend-test-utils'; const name1 = 'file1.yaml'; const file1 = fs.readFileSync( @@ -31,8 +31,8 @@ const file2 = fs.readFileSync( ); describe('ReadableArrayResponse', () => { - const sourceDir = MockDirectory.create(); - const targetDir = MockDirectory.create(); + const sourceDir = createMockDirectory(); + const targetDir = createMockDirectory(); beforeEach(() => { sourceDir.setContent({ diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index 784a4fb9bf..f69fb2e0e1 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -17,15 +17,15 @@ import fs from 'fs-extra'; import { resolve as resolvePath } from 'path'; import { TarArchiveResponse } from './TarArchiveResponse'; -import { MockDirectory } from '@backstage/backend-test-utils'; +import { createMockDirectory } from '@backstage/backend-test-utils'; const archiveData = fs.readFileSync( resolvePath(__filename, '../../__fixtures__/mock-main.tar.gz'), ); describe('TarArchiveResponse', () => { - const sourceDir = MockDirectory.create(); - const targetDir = MockDirectory.create(); + const sourceDir = createMockDirectory(); + const targetDir = createMockDirectory(); beforeAll(() => { sourceDir.setContent({ 'test-archive.tar.gz': archiveData }); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index c92ee46aad..0eecad2098 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -19,7 +19,7 @@ import { Readable } from 'stream'; import { create as createArchive } from 'archiver'; import { resolve as resolvePath } from 'path'; import { ZipArchiveResponse } from './ZipArchiveResponse'; -import { MockDirectory } from '@backstage/backend-test-utils'; +import { createMockDirectory } from '@backstage/backend-test-utils'; const archiveData = fs.readFileSync( resolvePath(__filename, '../../__fixtures__/mock-main.zip'), @@ -35,8 +35,8 @@ const archiveWithMaliciousEntry = fs.readFileSync( ); describe('ZipArchiveResponse', () => { - const sourceDir = MockDirectory.create(); - const targetDir = MockDirectory.create(); + const sourceDir = createMockDirectory(); + const targetDir = createMockDirectory(); beforeAll(() => { sourceDir.setContent({ diff --git a/packages/backend-common/src/util/DockerContainerRunner.test.ts b/packages/backend-common/src/util/DockerContainerRunner.test.ts index 60f457a670..737bf3d178 100644 --- a/packages/backend-common/src/util/DockerContainerRunner.test.ts +++ b/packages/backend-common/src/util/DockerContainerRunner.test.ts @@ -19,15 +19,15 @@ import Docker from 'dockerode'; import Stream, { PassThrough } from 'stream'; import { ContainerRunner } from './ContainerRunner'; import { DockerContainerRunner, UserOptions } from './DockerContainerRunner'; -import { MockDirectory } from '@backstage/backend-test-utils'; +import { createMockDirectory } from '@backstage/backend-test-utils'; const mockDocker = new Docker() as jest.Mocked; describe('DockerContainerRunner', () => { let containerTaskApi: ContainerRunner; - const inputDir = MockDirectory.create(); - const outputDir = MockDirectory.create(); + const inputDir = createMockDirectory(); + const outputDir = createMockDirectory(); beforeEach(() => { inputDir.clear(); diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index dff218edb1..ac4c5ffe85 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -31,20 +31,23 @@ import { ServiceRef } from '@backstage/backend-plugin-api'; import { TokenManagerService } from '@backstage/backend-plugin-api'; import { UrlReaderService } from '@backstage/backend-plugin-api'; +// @public +export function createMockDirectory( + options?: MockDirectoryOptions, +): MockDirectory; + // @public (undocumented) export function isDockerDisabledForTests(): boolean; // @public -export class MockDirectory { +export interface MockDirectory { addContent(root: MockDirectoryContent): void; - clear: () => void; + clear(): void; content( options?: MockDirectoryContentOptions, ): MockDirectoryContent | undefined; - static create(options?: MockDirectoryCreateOptions): MockDirectory; - static mockOsTmpDir(): MockDirectory; - get path(): string; - remove: () => void; + readonly path: string; + remove(): void; resolve(...paths: string[]): string; setContent(root: MockDirectoryContent): void; } @@ -61,8 +64,8 @@ export interface MockDirectoryContentOptions { } // @public -export interface MockDirectoryCreateOptions { - root?: string; +export interface MockDirectoryOptions { + mockOsTmpDir?: boolean; } // @public (undocumented) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 19c56119f0..8c4ff49e64 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -21,10 +21,10 @@ import { resolve as resolvePath, relative as relativePath, } from 'path'; -import { MockDirectory } from './MockDirectory'; +import { createMockDirectory, MockDirectory } from './MockDirectory'; -describe('MockDirectory', () => { - const mockDir = MockDirectory.create(); +describe('createMockDirectory', () => { + const mockDir = createMockDirectory(); beforeEach(mockDir.clear); @@ -277,7 +277,7 @@ describe('MockDirectory', () => { let cleanupMockDir: MockDirectory; describe('inner', () => { - cleanupMockDir = MockDirectory.create(); + cleanupMockDir = createMockDirectory(); it('should populate a directory', () => { cleanupMockDir.setContent({ @@ -299,7 +299,7 @@ describe('MockDirectory', () => { let tmpDirMock: MockDirectory; describe('inner', () => { - tmpDirMock = MockDirectory.mockOsTmpDir(); + tmpDirMock = createMockDirectory({ mockOsTmpDir: true }); it('should mock os.tmpdir()', () => { expect(os.tmpdir()).toBe(tmpDirMock.path); @@ -310,22 +310,4 @@ describe('MockDirectory', () => { expect(os.tmpdir()).not.toBe(tmpDirMock.path); }); }); - - describe('existing directory', () => { - let existingMockDir: MockDirectory; - - describe('inner', () => { - existingMockDir = MockDirectory.create({ root: __dirname }); // hardcore mode - - it('should read existing directory', () => { - expect(existingMockDir.content()).toMatchObject({ - 'index.ts': expect.any(String), - }); - }); - }); - - it('should remove existing directory', () => { - expect(fs.pathExistsSync(__dirname)).toBe(true); - }); - }); }); diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index cb7ca5e726..43df843946 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -56,20 +56,6 @@ export type MockDirectoryContent = { [name in string]: MockDirectoryContent | string | Buffer; }; -/** - * Options for {@link MockDirectory.create}. - * - * @public - */ -export interface MockDirectoryCreateOptions { - /** - * The root path to create the directory in. Defaults to a temporary directory. - * - * If an existing directory is provided, it will not be cleaned up after the test. - */ - root?: string; -} - /** * Options for {@link MockDirectory.content}. * @@ -91,107 +77,21 @@ export interface MockDirectoryContentOptions { shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); } -/** @internal */ -type MockEntry = - | { - type: 'file'; - path: string; - content: Buffer; - } - | { - type: 'dir'; - path: string; - }; - /** * A utility for creating a mock directory that is automatically cleaned up. * * @public */ -export class MockDirectory { - /** - * Creates a new temporary mock directory that will be removed after the tests have completed. - * - * @remarks - * - * This method is intended to be called outside of any test, either at top-level or - * within a `describe` block. It will call `afterAll` to make sure that the mock directory - * is removed after the tests have run. - * - * @example - * ```ts - * describe('MySubject', () => { - * const mockDir = MockDirectory.create(); - * - * beforeEach(mockDir.clear); - * - * it('should work', () => { - * // ... use mockDir - * }) - * }) - * ``` - */ - static create(options?: MockDirectoryCreateOptions): MockDirectory { - const root = - options?.root ?? - fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); - - const mocker = new MockDirectory(root); - - const shouldCleanup = !options?.root || !fs.pathExistsSync(options.root); - if (shouldCleanup) { - process.on('beforeExit', mocker.remove); - - try { - afterAll(mocker.remove); - } catch { - /* ignore */ - } - } - - return mocker; - } - - /** - * Like {@link MockDirectory.create}, but also mocks `os.tmpdir()` to return the - * mock directory path until the end of the test suite. - * - * @returns - */ - static mockOsTmpDir(): MockDirectory { - const mocker = MockDirectory.create(); - const origTmpdir = os.tmpdir; - os.tmpdir = () => mocker.path; - - try { - afterAll(() => { - os.tmpdir = origTmpdir; - }); - } catch { - /* ignore */ - } - return mocker; - } - - readonly #root: string; - - private constructor(root: string) { - this.#root = root; - } - +export interface MockDirectory { /** * The path to the root of the mock directory */ - get path(): string { - return this.#root; - } + readonly path: string; /** * Resolves a path relative to the root of the mock directory. */ - resolve(...paths: string[]): string { - return resolvePath(this.#root, ...paths); - } + resolve(...paths: string[]): string; /** * Sets the content of the mock directory. This will remove any existing content. @@ -209,11 +109,7 @@ export class MockDirectory { * }); * ``` */ - setContent(root: MockDirectoryContent): void { - this.remove(); - - return this.addContent(root); - } + setContent(root: MockDirectoryContent): void; /** * Adds content of the mock directory. This will overwrite existing files. @@ -231,25 +127,7 @@ export class MockDirectory { * }); * ``` */ - addContent(root: MockDirectoryContent): void { - const entries = this.#transformInput(root); - - for (const entry of entries) { - const fullPath = resolvePath(this.#root, entry.path); - if (!isChildPath(this.#root, fullPath)) { - throw new Error( - `Provided path must resolve to a child path of the mock directory, got '${fullPath}'`, - ); - } - - if (entry.type === 'dir') { - fs.ensureDirSync(fullPath, { mode: 0o777 }); - } else if (entry.type === 'file') { - fs.ensureDirSync(dirname(fullPath), { mode: 0o777 }); - fs.writeFileSync(fullPath, entry.content, { mode: 0o666 }); - } - } - } + addContent(root: MockDirectoryContent): void; /** * Reads the content of the mock directory. @@ -274,6 +152,75 @@ export class MockDirectory { * }); * ``` */ + content( + options?: MockDirectoryContentOptions, + ): MockDirectoryContent | undefined; + + /** + * Clears the content of the mock directory, ensuring that the directory itself exists. + */ + clear(): void; + + /** + * Removes the mock directory and all its contents. + */ + remove(): void; +} + +/** @internal */ +type MockEntry = + | { + type: 'file'; + path: string; + content: Buffer; + } + | { + type: 'dir'; + path: string; + }; + +/** @internal */ +class MockDirectoryImpl { + readonly #root: string; + + constructor(root: string) { + this.#root = root; + } + + get path(): string { + return this.#root; + } + + resolve(...paths: string[]): string { + return resolvePath(this.#root, ...paths); + } + + setContent(root: MockDirectoryContent): void { + this.remove(); + + return this.addContent(root); + } + + addContent(root: MockDirectoryContent): void { + const entries = this.#transformInput(root); + + for (const entry of entries) { + const fullPath = resolvePath(this.#root, entry.path); + if (!isChildPath(this.#root, fullPath)) { + throw new Error( + `Provided path must resolve to a child path of the mock directory, got '${fullPath}'`, + ); + } + + if (entry.type === 'dir') { + fs.ensureDirSync(fullPath, { mode: 0o777 }); + } else if (entry.type === 'file') { + fs.ensureDirSync(dirname(fullPath), { mode: 0o777 }); + fs.writeFileSync(fullPath, entry.content, { mode: 0o666 }); + } + } + } + content( options?: MockDirectoryContentOptions, ): MockDirectoryContent | undefined { @@ -319,16 +266,10 @@ export class MockDirectory { return read(root); } - /** - * Clears the content of the mock directory, ensuring that the directory itself exists. - */ clear = (): void => { this.setContent({}); }; - /** - * Removes the mock directory and all its contents. - */ remove = (): void => { fs.removeSync(this.#root); }; @@ -359,3 +300,69 @@ export class MockDirectory { return entries; } } + +/** + * Options for {@link createMockDirectory}. + * + * @public + */ +export interface MockDirectoryOptions { + /** + * In addition to creating a temporary directory, also mock `os.tmpdir()` to return the + * mock directory path until the end of the test suite. + * + * @returns + */ + mockOsTmpDir?: boolean; +} + +/** + * Creates a new temporary mock directory that will be removed after the tests have completed. + * + * @public + * @remarks + * + * This method is intended to be called outside of any test, either at top-level or + * within a `describe` block. It will call `afterAll` to make sure that the mock directory + * is removed after the tests have run. + * + * @example + * ```ts + * describe('MySubject', () => { + * const mockDir = createMockDirectory(); + * + * beforeEach(mockDir.clear); + * + * it('should work', () => { + * // ... use mockDir + * }) + * }) + * ``` + */ +export function createMockDirectory( + options?: MockDirectoryOptions, +): MockDirectory { + const root = fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); + + const mocker = new MockDirectoryImpl(root); + + const origTmpdir = options?.mockOsTmpDir ? os.tmpdir : undefined; + if (origTmpdir) { + os.tmpdir = () => mocker.path; + } + + process.on('beforeExit', mocker.remove); + + try { + afterAll(() => { + if (origTmpdir) { + os.tmpdir = origTmpdir; + } + mocker.remove(); + }); + } catch { + /* ignore */ + } + + return mocker; +} diff --git a/packages/backend-test-utils/src/filesystem/index.ts b/packages/backend-test-utils/src/filesystem/index.ts index d1077db980..0a4d8c7d00 100644 --- a/packages/backend-test-utils/src/filesystem/index.ts +++ b/packages/backend-test-utils/src/filesystem/index.ts @@ -15,8 +15,9 @@ */ export { - MockDirectory, + createMockDirectory, + type MockDirectory, + type MockDirectoryOptions, type MockDirectoryContent, type MockDirectoryContentOptions, - type MockDirectoryCreateOptions, } from './MockDirectory'; From e4a83c1b85c8cd3eecd2f206c596a422684c480b Mon Sep 17 00:00:00 2001 From: Andres Mauricio Gomez P Date: Tue, 26 Sep 2023 15:07:50 -0500 Subject: [PATCH 28/42] kubernetes-backend proxy API invoke Authentication Strategies when Backstage-Kubernetes-Authorization-X-X are provided Signed-off-by: Andres Mauricio Gomez P --- .../src/service/KubernetesProxy.test.ts | 128 ++++++++++++++++++ .../src/service/KubernetesProxy.ts | 62 ++++++++- 2 files changed, 188 insertions(+), 2 deletions(-) diff --git a/plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts b/plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts index 327d580d10..27d57cd854 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts @@ -509,6 +509,134 @@ describe('KubernetesProxy', () => { }); }); + it('should invoke AuthStrategy if Backstage-Kubernetes-Authorization-X-X are provided', async () => { + const strategy: jest.Mocked = { + getCredential: jest + .fn() + .mockReturnValue({ type: 'bearer token', token: 'MY_TOKEN3' }), + validateCluster: jest.fn(), + }; + + proxy = new KubernetesProxy({ + logger: getVoidLogger(), + clusterSupplier: clusterSupplier, + authStrategy: strategy, + }); + + worker.use( + rest.get('https://localhost:9999/api/v1/namespaces', (req, res, ctx) => { + if (!req.headers.get('Authorization')) { + return res(ctx.status(401)); + } + + if (req.headers.get('Authorization') !== 'Bearer MY_TOKEN3') { + return res(ctx.status(403)); + } + + return res( + ctx.status(200), + ctx.json({ + kind: 'NamespaceList', + apiVersion: 'v1', + items: [], + }), + ); + }), + ); + + clusterSupplier.getClusters.mockResolvedValue([ + { + name: 'cluster1', + url: 'https://localhost:9999', + authMetadata: {}, + }, + ]); + + const requestPromise = setupProxyPromise({ + proxyPath: '/mountpath', + requestPath: '/api/v1/namespaces', + + headers: { + [HEADER_KUBERNETES_CLUSTER]: 'cluster1', + 'Backstage-Kubernetes-Authorization-google': 'MY_TOKEN1', + 'Backstage-Kubernetes-Authorization-aks': 'MY_TOKEN2', + 'Backstage-Kubernetes-Authorization-oidc-okta': 'MY_TOKEN3', + 'Backstage-Kubernetes-Authorization-oidc-gitlab': 'MY_TOKEN4', + 'Backstage-Kubernetes-Authorization-pinniped-audience1': 'MY_TOKEN5', + 'Backstage-Kubernetes-Authorization-pinniped-au-b-c-d-e': 'MY_TOKEN6', + }, + }); + + const response = await requestPromise; + + const authObj = { + google: 'MY_TOKEN1', + aks: 'MY_TOKEN2', + oidc: { okta: 'MY_TOKEN3', gitlab: 'MY_TOKEN4' }, + pinniped: { audience1: 'MY_TOKEN5', 'au-b-c-d-e': 'MY_TOKEN6' }, + }; + + expect(strategy.getCredential).toHaveBeenCalledTimes(1); + expect(strategy.getCredential).toHaveBeenCalledWith( + expect.anything(), + authObj, + ); + expect(response.status).toEqual(200); + expect(response.body).toStrictEqual({ + kind: 'NamespaceList', + apiVersion: 'v1', + items: [], + }); + }); + + it('should invoke the Auth strategy with an empty auth object when no Backstage-Kubernetes-Authorization-X-X are provided', async () => { + worker.use( + rest.get('https://localhost:9999/api/v1/namespaces', (_, res, ctx) => { + return res( + ctx.status(200), + ctx.json({ + kind: 'NamespaceList', + apiVersion: 'v1', + items: [], + }), + ); + }), + ); + + clusterSupplier.getClusters.mockResolvedValue([ + { + name: 'cluster1', + url: 'https://localhost:9999', + authMetadata: {}, + }, + ]); + + const requestPromise = setupProxyPromise({ + proxyPath: '/mountpath', + requestPath: '/api/v1/namespaces', + + headers: { + [HEADER_KUBERNETES_CLUSTER]: 'cluster1', + }, + }); + + const response = await requestPromise; + + const authObj = {}; + + expect(authStrategy.getCredential).toHaveBeenCalledTimes(1); + expect(authStrategy.getCredential).toHaveBeenCalledWith( + expect.anything(), + authObj, + ); + expect(response.status).toEqual(200); + expect(response.body).toStrictEqual({ + kind: 'NamespaceList', + apiVersion: 'v1', + items: [], + }); + }); + it('returns a response with a localKubectlProxy auth provider configuration', async () => { proxy = new KubernetesProxy({ logger: getVoidLogger(), diff --git a/plugins/kubernetes-backend/src/service/KubernetesProxy.ts b/plugins/kubernetes-backend/src/service/KubernetesProxy.ts index d2a3649c27..8a07267cde 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesProxy.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesProxy.ts @@ -21,7 +21,10 @@ import { serializeError, } from '@backstage/errors'; import { getBearerTokenFromAuthorizationHeader } from '@backstage/plugin-auth-node'; -import { kubernetesProxyPermission } from '@backstage/plugin-kubernetes-common'; +import { + KubernetesRequestAuth, + kubernetesProxyPermission, +} from '@backstage/plugin-kubernetes-common'; import { AuthorizeResult, PermissionEvaluator, @@ -34,6 +37,7 @@ import { AuthenticationStrategy } from '../auth'; import { ClusterDetails, KubernetesClustersSupplier } from '../types/types'; import type { Request } from 'express'; +import { IncomingHttpHeaders } from 'http'; export const APPLICATION_JSON: string = 'application/json'; @@ -113,9 +117,15 @@ export class KubernetesProxy { if (authHeader) { req.headers.authorization = authHeader; } else { + // Map Backstage-Kubernetes-Authorization-X-X headers to a KubernetesRequestAuth object + const authObj = KubernetesProxy.authHeadersToKubernetesRequestAuth( + req.headers, + ); + const credential = await this.getClusterForRequest(req).then(cd => { - return this.authStrategy.getCredential(cd, {}); + return this.authStrategy.getCredential(cd, authObj); }); + if (credential.type === 'bearer token') { req.headers.authorization = `Bearer ${credential.token}`; } @@ -221,4 +231,52 @@ export class KubernetesProxy { return cluster; } + + private static authHeadersToKubernetesRequestAuth( + originalHeaders: IncomingHttpHeaders, + ): KubernetesRequestAuth { + return Object.keys(originalHeaders) + .filter(header => header.startsWith('backstage-kubernetes-authorization')) + .map(header => + KubernetesProxy.headerToDictionary(header, originalHeaders), + ) + .filter(headerAsDic => Object.keys(headerAsDic).length !== 0) + .reduce(KubernetesProxy.combineHeaders, {}); + } + + private static headerToDictionary( + header: string, + originalHeaders: IncomingHttpHeaders, + ): KubernetesRequestAuth { + const obj: KubernetesRequestAuth = {}; + const headerSplitted = header.split('-'); + if (headerSplitted.length >= 4) { + const framework = headerSplitted[3].toLowerCase(); + if (headerSplitted.length >= 5) { + const provider = headerSplitted.slice(4).join('-').toLowerCase(); + obj[framework] = { [provider]: originalHeaders[header] }; + } else { + obj[framework] = originalHeaders[header]; + } + } + return obj; + } + + private static combineHeaders( + authObj: any, + header: any, + ): KubernetesRequestAuth { + const framework = Object.keys(header)[0]; + + if (authObj[framework]) { + authObj[framework] = { + ...authObj[framework], + ...header[framework], + }; + } else { + authObj[framework] = header[framework]; + } + + return authObj; + } } From 5dac12e4350aa690a546c95b0a3fb1fa24c30355 Mon Sep 17 00:00:00 2001 From: Andres Mauricio Gomez P Date: Thu, 28 Sep 2023 10:52:45 -0500 Subject: [PATCH 29/42] Backstage-Kubernetes-Authorization-X-X headers are provided when it gets invoke kubernetes endpoints on backstage Signed-off-by: Andres Mauricio Gomez P --- .changeset/two-dingos-dream.md | 6 ++ .../src/api/KubernetesBackendClient.test.ts | 77 +++++++++++++++++-- .../src/api/KubernetesBackendClient.ts | 57 ++++++++++++-- 3 files changed, 126 insertions(+), 14 deletions(-) create mode 100644 .changeset/two-dingos-dream.md diff --git a/.changeset/two-dingos-dream.md b/.changeset/two-dingos-dream.md new file mode 100644 index 0000000000..4fbe7651c9 --- /dev/null +++ b/.changeset/two-dingos-dream.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-kubernetes-backend': patch +'@backstage/plugin-kubernetes-react': patch +--- + +The kubernetes APIs invokes Authentication Strategies when Backstage-Kubernetes-Authorization-X-X headers are provided, this enable the possibility to invoke strategies that executes additional steps to get a kubernetes token like on pinniped or custom strategies diff --git a/plugins/kubernetes-react/src/api/KubernetesBackendClient.test.ts b/plugins/kubernetes-react/src/api/KubernetesBackendClient.test.ts index 7d2ef36602..fd999ab335 100644 --- a/plugins/kubernetes-react/src/api/KubernetesBackendClient.test.ts +++ b/plugins/kubernetes-react/src/api/KubernetesBackendClient.test.ts @@ -398,9 +398,26 @@ describe('KubernetesBackendClient', () => { identityApi.getCredentials.mockResolvedValue({ token: 'idToken' }); }); - it('hits the /proxy API', async () => { + it('hits the /proxy API with oidc as protocol and okta as auth provider', async () => { + worker.use( + rest.get( + 'http://localhost:1234/api/kubernetes/clusters', + (_, res, ctx) => + res( + ctx.json({ + items: [ + { + name: 'cluster-a', + authProvider: 'oidc', + oidcTokenProvider: 'okta', + }, + ], + }), + ), + ), + ); kubernetesAuthProvidersApi.getCredentials.mockResolvedValue({ - token: 'k8-token', + token: 'k8-token3', }); const nsResponse = { kind: 'Namespace', @@ -414,8 +431,56 @@ describe('KubernetesBackendClient', () => { 'http://localhost:1234/api/kubernetes/proxy/api/v1/namespaces', (req, res, ctx) => res( - req.headers.get('Backstage-Kubernetes-Authorization') === - 'Bearer k8-token' + req.headers.get( + 'Backstage-Kubernetes-Authorization-oidc-okta', + ) === 'k8-token3' + ? ctx.json(nsResponse) + : ctx.status(403), + ), + ), + ); + + const request = { + clusterName: 'cluster-a', + path: '/api/v1/namespaces', + }; + + const response = await backendClient.proxy(request); + + await expect(response.json()).resolves.toStrictEqual(nsResponse); + }); + + it('hits the /proxy API with serviceAccount as auth provider', async () => { + worker.use( + rest.get( + 'http://localhost:1234/api/kubernetes/clusters', + (_, res, ctx) => + res( + ctx.json({ + items: [ + { + name: 'cluster-a', + authProvider: 'serviceAccount', + }, + ], + }), + ), + ), + ); + + const nsResponse = { + kind: 'Namespace', + apiVersion: 'v1', + metadata: { + name: 'new-ns', + }, + }; + worker.use( + rest.get( + 'http://localhost:1234/api/kubernetes/proxy/api/v1/namespaces', + (req, res, ctx) => + res( + req.headers.get('Authorization') === 'Bearer idToken' ? ctx.json(nsResponse) : ctx.status(403), ), @@ -450,8 +515,8 @@ describe('KubernetesBackendClient', () => { 'http://localhost:1234/api/kubernetes/proxy/api/v1/namespaces', (req, res, ctx) => res( - req.headers.get('Backstage-Kubernetes-Authorization') === - 'Bearer k8-token' + req.headers.get('Backstage-Kubernetes-Authorization-aws') === + 'k8-token' ? ctx.json(nsResponse) : ctx.status(403), ), diff --git a/plugins/kubernetes-react/src/api/KubernetesBackendClient.ts b/plugins/kubernetes-react/src/api/KubernetesBackendClient.ts index b483c267d8..d4e11c43da 100644 --- a/plugins/kubernetes-react/src/api/KubernetesBackendClient.ts +++ b/plugins/kubernetes-react/src/api/KubernetesBackendClient.ts @@ -75,9 +75,11 @@ export class KubernetesBackendClient implements KubernetesApi { return this.handleResponse(response); } - private async getCluster( - clusterName: string, - ): Promise<{ name: string; authProvider: string }> { + private async getCluster(clusterName: string): Promise<{ + name: string; + authProvider: string; + oidcTokenProvider?: string; + }> { const cluster = await this.getClusters().then(clusters => clusters.find(c => c.name === clusterName), ); @@ -140,23 +142,62 @@ export class KubernetesBackendClient implements KubernetesApi { path: string; init?: RequestInit; }): Promise { - const { authProvider } = await this.getCluster(options.clusterName); - const { token: k8sToken } = await this.getCredentials(authProvider); + const { authProvider, oidcTokenProvider } = await this.getCluster( + options.clusterName, + ); + const kubernetesCredentials = await this.getCredentials(authProvider); const url = `${await this.discoveryApi.getBaseUrl('kubernetes')}/proxy${ options.path }`; const identityResponse = await this.identityApi.getCredentials(); - const headers = { + const headers = KubernetesBackendClient.getKubernetesHeaders( + options, + kubernetesCredentials?.token, + identityResponse, + authProvider, + oidcTokenProvider, + ); + return await fetch(url, { ...options.init, headers }); + } + + private static getKubernetesHeaders( + options: { + clusterName: string; + path: string; + init?: RequestInit; + }, + k8sToken: string | undefined, + identityResponse: { token?: string }, + authProvider: string, + oidcTokenProvider: string | undefined, + ) { + const kubernetesAuthHeader = + KubernetesBackendClient.getKubernetesAuthHeaderByAuthProvider( + authProvider, + oidcTokenProvider, + ); + return { ...options.init?.headers, [`Backstage-Kubernetes-Cluster`]: options.clusterName, ...(k8sToken && { - [`Backstage-Kubernetes-Authorization`]: `Bearer ${k8sToken}`, + [kubernetesAuthHeader]: k8sToken, }), ...(identityResponse.token && { Authorization: `Bearer ${identityResponse.token}`, }), }; + } - return await fetch(url, { ...options.init, headers }); + private static getKubernetesAuthHeaderByAuthProvider( + authProvider: string, + oidcTokenProvider: string | undefined, + ): string { + let header: string = 'Backstage-Kubernetes-Authorization'; + + header = header.concat('-', authProvider); + + if (oidcTokenProvider) header = header.concat('-', oidcTokenProvider); + + return header; } } From 201eb08c8413b584bf3b2da59081af8efc4ea691 Mon Sep 17 00:00:00 2001 From: Andres Mauricio Gomez P Date: Fri, 29 Sep 2023 17:23:22 -0500 Subject: [PATCH 30/42] Validating that custom auth strategies don't include dashes Signed-off-by: Andres Mauricio Gomez P --- .changeset/two-dingos-dream.md | 1 + .../src/service/KubernetesBuilder.test.ts | 20 +++++++++++++++++++ .../src/service/KubernetesBuilder.ts | 3 +++ plugins/kubernetes-common/api-report.md | 5 ++++- plugins/kubernetes-common/src/types.ts | 6 ++++-- 5 files changed, 32 insertions(+), 3 deletions(-) diff --git a/.changeset/two-dingos-dream.md b/.changeset/two-dingos-dream.md index 4fbe7651c9..9ebefacc69 100644 --- a/.changeset/two-dingos-dream.md +++ b/.changeset/two-dingos-dream.md @@ -1,6 +1,7 @@ --- '@backstage/plugin-kubernetes-backend': patch '@backstage/plugin-kubernetes-react': patch +'@backstage/plugin-kubernetes-common': patch --- The kubernetes APIs invokes Authentication Strategies when Backstage-Kubernetes-Authorization-X-X headers are provided, this enable the possibility to invoke strategies that executes additional steps to get a kubernetes token like on pinniped or custom strategies diff --git a/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts b/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts index 85a4326d60..c676cc6246 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts @@ -520,6 +520,26 @@ metadata: expect(response.body).toStrictEqual({ items: [] }); }); + + it('should not permit custom auth strategies with dashes', async () => { + const throwError = () => + KubernetesBuilder.createBuilder({ + logger: getVoidLogger(), + config, + catalogApi, + permissions, + }).addAuthStrategy('custom-strategy', { + getCredential: jest + .fn< + Promise, + [ClusterDetails, KubernetesRequestAuth] + >() + .mockResolvedValue({ type: 'anonymous' }), + validateCluster: jest.fn().mockReturnValue([]), + }); + + expect(throwError).toThrow('Strategy name can not include dashes'); + }); }); describe('get /.well-known/backstage/permissions/metadata', () => { it('lists permissions supported by the kubernetes plugin', async () => { diff --git a/plugins/kubernetes-backend/src/service/KubernetesBuilder.ts b/plugins/kubernetes-backend/src/service/KubernetesBuilder.ts index f3766d8513..9ac1bf6e5d 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesBuilder.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesBuilder.ts @@ -205,6 +205,9 @@ export class KubernetesBuilder { } public addAuthStrategy(key: string, strategy: AuthenticationStrategy) { + if (key.includes('-')) { + throw new Error('Strategy name can not include dashes'); + } this.getAuthStrategyMap()[key] = strategy; return this; } diff --git a/plugins/kubernetes-common/api-report.md b/plugins/kubernetes-common/api-report.md index 0840278394..8daeb9a875 100644 --- a/plugins/kubernetes-common/api-report.md +++ b/plugins/kubernetes-common/api-report.md @@ -7,6 +7,7 @@ import { BasicPermission } from '@backstage/plugin-permission-common'; import { Entity } from '@backstage/catalog-model'; import { FetchResponse as FetchResponse_2 } from '@backstage/plugin-kubernetes-common'; import type { JsonObject } from '@backstage/types'; +import type { JsonValue } from '@backstage/types'; import { ObjectsByEntityResponse as ObjectsByEntityResponse_2 } from '@backstage/plugin-kubernetes-common'; import { PodStatus } from '@kubernetes/client-node'; import { V1ConfigMap } from '@kubernetes/client-node'; @@ -319,7 +320,9 @@ export const kubernetesPermissions: BasicPermission[]; export const kubernetesProxyPermission: BasicPermission; // @public (undocumented) -export type KubernetesRequestAuth = JsonObject; +export type KubernetesRequestAuth = { + [providerKey: string]: JsonValue | undefined; +}; // @public (undocumented) export interface KubernetesRequestBody { diff --git a/plugins/kubernetes-common/src/types.ts b/plugins/kubernetes-common/src/types.ts index 9648293432..9f8d5eed82 100644 --- a/plugins/kubernetes-common/src/types.ts +++ b/plugins/kubernetes-common/src/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { JsonObject } from '@backstage/types'; +import type { JsonObject, JsonValue } from '@backstage/types'; import { PodStatus, V1ConfigMap, @@ -33,7 +33,9 @@ import { import { Entity } from '@backstage/catalog-model'; /** @public */ -export type KubernetesRequestAuth = JsonObject; +export type KubernetesRequestAuth = { + [providerKey: string]: JsonValue | undefined; +}; /** @public */ export interface CustomResourceMatcher { From a1d163305000ff3cb0749fe29b2f4f1584b5e268 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Tue, 3 Oct 2023 15:31:25 -0600 Subject: [PATCH 31/42] New permission-maintainers Signed-off-by: Tim Hansen --- OWNERS.md | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/OWNERS.md b/OWNERS.md index 000a3e2444..1cf8a8bd1a 100644 --- a/OWNERS.md +++ b/OWNERS.md @@ -73,16 +73,18 @@ Team: @backstage/permission-maintainers Scope: The Permission Framework and plugins integrating with the permission framework -| Name | Organization | Team | GitHub | Discord | -| -------------------- | ------------ | --------------- | ---------------------------------------------- | ---------------- | -| Ainhoa Larumbe | Spotify | Imaginary Goats | [ainhoaL](http://github.com/ainhoaL) | ainhoa#8085 | -| Claire Casey | Spotify | Imaginary Goats | [clairelcasey](http://github.com/clairelcasey) | clairecasey#2710 | -| Eric Peterson | Spotify | Imaginary Goats | [iamEAP](http://github.com/iamEAP) | iamEAP#3058 | -| Harry Hogg | Spotify | Imaginary Goats | [HHogg](http://github.com/HHogg) | simplex#3451 | -| Joon Park | Spotify | Imaginary Goats | [Joonpark13](http://github.com/Joonpark13) | Sixpool#5060 | -| Mike Lewis | Spotify | Imaginary Goats | [mtlewis](http://github.com/mtlewis) | mtlewis#3658 | -| Tim Hansen | Spotify | Imaginary Goats | [timbonicus](http://github.com/timbonicus) | timbonicus#6871 | -| Vincenzo Scamporlino | Spotify | Imaginary Goats | [vinzscam](http://github.com/vinzscam) | vinzscam#6944 | +| Name | Organization | Team | GitHub | Discord | +| -------------------- | ------------ | --------------- | ----------------------------------------------- | ---------------- | +| Ainhoa Larumbe | Spotify | Imaginary Goats | [ainhoaL](http://github.com/ainhoaL) | ainhoa#8085 | +| Aramis Sennyey | Spotify | Imaginary Goats | [sennyeya](https://github.com/sennyeya) | Aramis#7984 | +| Claire Casey | Spotify | Imaginary Goats | [clairelcasey](http://github.com/clairelcasey) | clairecasey#2710 | +| Eric Peterson | Spotify | Imaginary Goats | [iamEAP](http://github.com/iamEAP) | iamEAP#3058 | +| Harry Hogg | Spotify | Imaginary Goats | [HHogg](http://github.com/HHogg) | simplex#3451 | +| Joon Park | Spotify | Imaginary Goats | [Joonpark13](http://github.com/Joonpark13) | Sixpool#5060 | +| Lynette Lopez | Spotify | Imaginary Goats | [lynettelopez](https://github.com/lynettelopez) | - | +| Mike Lewis | Spotify | Imaginary Goats | [mtlewis](http://github.com/mtlewis) | mtlewis#3658 | +| Tim Hansen | Spotify | Imaginary Goats | [timbonicus](http://github.com/timbonicus) | timbonicus#6871 | +| Vincenzo Scamporlino | Spotify | Imaginary Goats | [vinzscam](http://github.com/vinzscam) | vinzscam#6944 | ### TechDocs From b7ae05d5b8b9d40d07016f7d9ceb2a80c5f9e4fe Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Oct 2023 18:59:00 +0200 Subject: [PATCH 32/42] backend-test-utils: win32 fix Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 8c4ff49e64..54d32f47d7 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -16,11 +16,7 @@ import fs from 'fs-extra'; import os from 'os'; -import { - join as joinPath, - resolve as resolvePath, - relative as relativePath, -} from 'path'; +import { join as joinPath, relative as relativePath } from 'path'; import { createMockDirectory, MockDirectory } from './MockDirectory'; describe('createMockDirectory', () => { @@ -261,7 +257,7 @@ describe('createMockDirectory', () => { }); it('should reject non-child paths', () => { - const path = resolvePath('/root/a.txt'); + const path = mockDir.resolve('/root/a.txt'); expect(() => mockDir.setContent({ '/root/a.txt': 'a' })).toThrow( `Provided path must resolve to a child path of the mock directory, got '${path}'`, ); From da0ef5a98f0cbf26e35e81025f52dde7b8754be0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Oct 2023 19:01:10 +0200 Subject: [PATCH 33/42] backend-test-utils: skip mock directory cleanup in CI Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 22 ------------------- .../src/filesystem/MockDirectory.ts | 10 +++++++-- 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 54d32f47d7..45a206d7df 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -269,28 +269,6 @@ describe('createMockDirectory', () => { ); }); - describe('cleanup', () => { - let cleanupMockDir: MockDirectory; - - describe('inner', () => { - cleanupMockDir = createMockDirectory(); - - it('should populate a directory', () => { - cleanupMockDir.setContent({ - 'a.txt': 'a', - }); - - expect(cleanupMockDir.content()).toEqual({ - 'a.txt': 'a', - }); - }); - }); - - it('should clean up after itself automatically', () => { - expect(cleanupMockDir.content()).toBeUndefined(); - }); - }); - describe('tmpdir mock', () => { let tmpDirMock: MockDirectory; diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 43df843946..1d48d157df 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -351,14 +351,20 @@ export function createMockDirectory( os.tmpdir = () => mocker.path; } - process.on('beforeExit', mocker.remove); + // In CI we expect there to be no need to clean up temporary directories + const needsCleanup = !process.env.CI; + if (needsCleanup) { + process.on('beforeExit', mocker.remove); + } try { afterAll(() => { if (origTmpdir) { os.tmpdir = origTmpdir; } - mocker.remove(); + if (needsCleanup) { + mocker.remove(); + } }); } catch { /* ignore */ From 8770ca25182daacb7140b652782e8a236afbf39e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Oct 2023 19:14:50 +0200 Subject: [PATCH 34/42] backend-test-utils: added content option for createMockDirectory Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/api-report.md | 1 + .../backend-test-utils/src/filesystem/MockDirectory.ts | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index ac4c5ffe85..a477f70298 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -65,6 +65,7 @@ export interface MockDirectoryContentOptions { // @public export interface MockDirectoryOptions { + content?: MockDirectoryContent; mockOsTmpDir?: boolean; } diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 1d48d157df..7619f191ec 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -314,6 +314,11 @@ export interface MockDirectoryOptions { * @returns */ mockOsTmpDir?: boolean; + + /** + * Initializes the directory with the given content, see {@link MockDirectory.setContent}. + */ + content?: MockDirectoryContent; } /** @@ -370,5 +375,9 @@ export function createMockDirectory( /* ignore */ } + if (options?.content) { + mocker.setContent(options.content); + } + return mocker; } From e2c854a398def32e01b77865d5f0b17c798eeacd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 10:39:40 +0200 Subject: [PATCH 35/42] backend-test-utils: make directory mocker use RUNNER_TEMP if available Signed-off-by: Patrik Oldsberg --- packages/backend-test-utils/src/filesystem/MockDirectory.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 7619f191ec..4ca66f2b68 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -347,7 +347,10 @@ export interface MockDirectoryOptions { export function createMockDirectory( options?: MockDirectoryOptions, ): MockDirectory { - const root = fs.mkdtempSync(joinPath(getTmpDir(), 'backstage-tmp-test-dir-')); + const tmpDir = process.env.RUNNER_TEMP; // GitHub Actions + const root = fs.mkdtempSync( + joinPath(tmpDir || getTmpDir(), 'backstage-tmp-test-dir-'), + ); const mocker = new MockDirectoryImpl(root); From 41866257190d58148d73d6ca1041035aa237d2a3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 10:33:39 +0200 Subject: [PATCH 36/42] .github/workflows: switch windows verification to windows-2022 Signed-off-by: Patrik Oldsberg --- .github/workflows/verify_windows.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/verify_windows.yml b/.github/workflows/verify_windows.yml index e5d3adf013..312c662b5a 100644 --- a/.github/workflows/verify_windows.yml +++ b/.github/workflows/verify_windows.yml @@ -9,9 +9,10 @@ on: jobs: build: - runs-on: windows-2019 + runs-on: windows-2022 strategy: + fail-fast: false matrix: node-version: [18.x, 20.x] From 3b7ec34b4a49db8ed64cdae474f7c416e7f3293a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 11:58:49 +0200 Subject: [PATCH 37/42] backend-test-utils: make MockDirectory refuse duplicate mock of os.tmpdir() Signed-off-by: Patrik Oldsberg --- .../src/filesystem/MockDirectory.test.ts | 6 ++++++ .../src/filesystem/MockDirectory.ts | 17 +++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts index 45a206d7df..9e3efdaec0 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.test.ts @@ -278,6 +278,12 @@ describe('createMockDirectory', () => { it('should mock os.tmpdir()', () => { expect(os.tmpdir()).toBe(tmpDirMock.path); }); + + it('should refuce to mock os.tmpdir() again', () => { + expect(() => createMockDirectory({ mockOsTmpDir: true })).toThrow( + 'Cannot mock os.tmpdir() when it has already been mocked', + ); + }); }); it('should restore os.tmpdir()', () => { diff --git a/packages/backend-test-utils/src/filesystem/MockDirectory.ts b/packages/backend-test-utils/src/filesystem/MockDirectory.ts index 4ca66f2b68..20345e4ff9 100644 --- a/packages/backend-test-utils/src/filesystem/MockDirectory.ts +++ b/packages/backend-test-utils/src/filesystem/MockDirectory.ts @@ -18,7 +18,6 @@ import os from 'os'; import { isChildPath } from '@backstage/backend-common'; import fs from 'fs-extra'; import textextensions from 'textextensions'; -import { tmpdir as getTmpDir } from 'os'; import { dirname, extname, @@ -29,6 +28,8 @@ import { posix, } from 'path'; +const tmpdirMarker = Symbol('os-tmpdir-mock'); + /** * The content of a mock directory represented by a nested object structure. * @@ -347,16 +348,20 @@ export interface MockDirectoryOptions { export function createMockDirectory( options?: MockDirectoryOptions, ): MockDirectory { - const tmpDir = process.env.RUNNER_TEMP; // GitHub Actions - const root = fs.mkdtempSync( - joinPath(tmpDir || getTmpDir(), 'backstage-tmp-test-dir-'), - ); + const tmpDir = process.env.RUNNER_TEMP || os.tmpdir(); // GitHub Actions + const root = fs.mkdtempSync(joinPath(tmpDir, 'backstage-tmp-test-dir-')); const mocker = new MockDirectoryImpl(root); const origTmpdir = options?.mockOsTmpDir ? os.tmpdir : undefined; if (origTmpdir) { - os.tmpdir = () => mocker.path; + if (Object.hasOwn(origTmpdir, tmpdirMarker)) { + throw new Error( + 'Cannot mock os.tmpdir() when it has already been mocked', + ); + } + const mock = Object.assign(() => mocker.path, { [tmpdirMarker]: true }); + os.tmpdir = mock; } // In CI we expect there to be no need to clean up temporary directories From 7790e080c204ccfc0a29fef9ff7bd305d4fece48 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Wed, 4 Oct 2023 07:56:19 -0600 Subject: [PATCH 38/42] Add Discord username for Lynette Signed-off-by: Tim Hansen --- OWNERS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OWNERS.md b/OWNERS.md index 1cf8a8bd1a..135d617203 100644 --- a/OWNERS.md +++ b/OWNERS.md @@ -81,7 +81,7 @@ Scope: The Permission Framework and plugins integrating with the permission fram | Eric Peterson | Spotify | Imaginary Goats | [iamEAP](http://github.com/iamEAP) | iamEAP#3058 | | Harry Hogg | Spotify | Imaginary Goats | [HHogg](http://github.com/HHogg) | simplex#3451 | | Joon Park | Spotify | Imaginary Goats | [Joonpark13](http://github.com/Joonpark13) | Sixpool#5060 | -| Lynette Lopez | Spotify | Imaginary Goats | [lynettelopez](https://github.com/lynettelopez) | - | +| Lynette Lopez | Spotify | Imaginary Goats | [lynettelopez](https://github.com/lynettelopez) | lynettelopez | | Mike Lewis | Spotify | Imaginary Goats | [mtlewis](http://github.com/mtlewis) | mtlewis#3658 | | Tim Hansen | Spotify | Imaginary Goats | [timbonicus](http://github.com/timbonicus) | timbonicus#6871 | | Vincenzo Scamporlino | Spotify | Imaginary Goats | [vinzscam](http://github.com/vinzscam) | vinzscam#6944 | From b95d66d4ea74d56edec96f8d893a925a6ed2e83f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Oct 2023 16:08:13 +0200 Subject: [PATCH 39/42] backend-common: end write stream after use in ZipArchiveResponse Signed-off-by: Patrik Oldsberg --- .changeset/great-walls-brake.md | 5 +++++ .../backend-common/src/reading/tree/ZipArchiveResponse.ts | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 .changeset/great-walls-brake.md diff --git a/.changeset/great-walls-brake.md b/.changeset/great-walls-brake.md new file mode 100644 index 0000000000..dd71c81a07 --- /dev/null +++ b/.changeset/great-walls-brake.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Properly close write stream when writing temporary archive for processing zip-based `.readTree()` responses. diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index cd57788743..a191163517 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -93,12 +93,13 @@ export class ZipArchiveResponse implements ReadTreeResponse { return new Promise((resolve, reject) => { writeStream.on('error', reject); - writeStream.on('finish', () => + writeStream.on('finish', () => { + writeStream.end(); resolve({ fileName: tmpFile, cleanup: () => fs.rm(tmpDir, { recursive: true }), - }), - ); + }); + }); stream.pipe(writeStream); }); } From 527142b06bc064f17920f706535eddbb0f8f0350 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Oct 2023 16:10:03 +0200 Subject: [PATCH 40/42] backend-common: close write streams after use in tests Signed-off-by: Patrik Oldsberg --- .../tree/ReadableArrayResponse.test.ts | 19 +++++++++--- .../reading/tree/ZipArchiveResponse.test.ts | 31 +++++++++++++------ 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index 4108697bce..0a27bdf09c 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -42,13 +42,24 @@ describe('ReadableArrayResponse', () => { targetDir.clear(); }); + const openStreams = new Array(); + function createReadStream(filePath: string) { + const stream = fs.createReadStream(filePath); + openStreams.push(stream); + return stream; + } + afterEach(() => { + openStreams.forEach(stream => stream.destroy()); + openStreams.length = 0; + }); + const path1 = sourceDir.resolve(name1); const path2 = sourceDir.resolve(name2); it('should read files', async () => { const arr: FromReadableArrayOptions = [ - { data: fs.createReadStream(path1), path: path1 }, - { data: fs.createReadStream(path2), path: path2 }, + { data: createReadStream(path1), path: path1 }, + { data: createReadStream(path2), path: path2 }, ]; const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); @@ -64,8 +75,8 @@ describe('ReadableArrayResponse', () => { it('should extract entire archive into directory', async () => { const arr: FromReadableArrayOptions = [ - { data: fs.createReadStream(path1), path: path1 }, - { data: fs.createReadStream(path2), path: path2 }, + { data: createReadStream(path1), path: path1 }, + { data: createReadStream(path2), path: path2 }, ]; const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 0eecad2098..ccf53e7cff 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -50,8 +50,19 @@ describe('ZipArchiveResponse', () => { targetDir.clear(); }); + const openStreams = new Array(); + function createReadStream(filePath: string) { + const stream = fs.createReadStream(filePath); + openStreams.push(stream); + return stream; + } + afterEach(() => { + openStreams.forEach(stream => stream.destroy()); + openStreams.length = 0; + }); + it('should read files', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const files = await res.files(); @@ -77,7 +88,7 @@ describe('ZipArchiveResponse', () => { }); it('should read files with filter', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse( stream, @@ -100,7 +111,7 @@ describe('ZipArchiveResponse', () => { }); it('should read as archive and files', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const buffer = await res.archive(); @@ -132,7 +143,7 @@ describe('ZipArchiveResponse', () => { }); it('should extract entire archive into directory', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); const dir = await res.dir(); @@ -146,7 +157,7 @@ describe('ZipArchiveResponse', () => { }); it('should extract archive into directory with a subpath', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse(stream, 'docs/', targetDir.path, 'etag'); @@ -157,7 +168,7 @@ describe('ZipArchiveResponse', () => { }); it('should extract archive into directory with a subpath and filter', async () => { - const stream = fs.createReadStream(sourceDir.resolve('test-archive.zip')); + const stream = createReadStream(sourceDir.resolve('test-archive.zip')); const res = new ZipArchiveResponse( stream, @@ -205,7 +216,7 @@ describe('ZipArchiveResponse', () => { archive.finalize(); }); - const stream = fs.createReadStream(filePath); + const stream = createReadStream(filePath); const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag'); @@ -225,7 +236,7 @@ describe('ZipArchiveResponse', () => { }); it('should throw on invalid archive', async () => { - const stream = fs.createReadStream( + const stream = createReadStream( sourceDir.resolve('test-archive-corrupted.zip'), ); @@ -238,7 +249,7 @@ describe('ZipArchiveResponse', () => { }); it('should throw on entries with a path outside the destination dir', async () => { - const stream = fs.createReadStream( + const stream = createReadStream( sourceDir.resolve('test-archive-malicious.zip'), ); @@ -249,7 +260,7 @@ describe('ZipArchiveResponse', () => { }); it('should throw on entries that attempt to write outside destination dir', async () => { - const stream = fs.createReadStream( + const stream = createReadStream( sourceDir.resolve('test-archive-malicious.zip'), ); From 8b8b1d23aef38e7908b7a2477a1f924e560e18c3 Mon Sep 17 00:00:00 2001 From: Adam Kunicki Date: Wed, 4 Oct 2023 10:14:02 -0700 Subject: [PATCH 41/42] auth-node: Refresh handler not returning persisted scope in response The refresh handler is returning an empty scope if scope was previously saved in a cookie. The session is successfully refreshed but the client receives a response without the scope it requested, prompting a new login. Resolves #20322 Signed-off-by: Adam Kunicki --- .changeset/stale-stingrays-explode.md | 5 +++++ plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 .changeset/stale-stingrays-explode.md diff --git a/.changeset/stale-stingrays-explode.md b/.changeset/stale-stingrays-explode.md new file mode 100644 index 0000000000..b8b66b8398 --- /dev/null +++ b/.changeset/stale-stingrays-explode.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-node': patch +--- + +Fixed cookie persisted scope not returned in OAuth refresh handler response. diff --git a/plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts b/plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts index 165d7ce6b3..a0735e3ba0 100644 --- a/plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts +++ b/plugins/auth-node/src/oauth/createOAuthRouteHandlers.ts @@ -320,7 +320,9 @@ export function createOAuthRouteHandlers( providerInfo: { idToken: result.session.idToken, accessToken: result.session.accessToken, - scope: result.session.scope, + scope: authenticator.shouldPersistScopes + ? scope + : result.session.scope, expiresInSeconds: result.session.expiresInSeconds, }, }; From 95518765ee6b5382357ca3cb1f177a1015fcc10d Mon Sep 17 00:00:00 2001 From: Matthew Clarke Date: Wed, 4 Oct 2023 15:31:30 -0400 Subject: [PATCH 42/42] feat: Kubernetes cluster plugin (#18760) * refactor: k8s plugins Signed-off-by: Matthew Clarke * feat: add kubernetes cluster plugin Signed-off-by: Matthew Clarke * fix: yarn fix Signed-off-by: Matthew Clarke * fix: tsc Signed-off-by: Matthew Clarke * fix: types Signed-off-by: Matthew Clarke * chore: missing changeset Signed-off-by: Matthew Clarke * chore: update api report Signed-off-by: Matthew Clarke --------- Signed-off-by: Matthew Clarke --- .changeset/slow-dodos-remember.md | 8 + packages/app/package.json | 1 + .../app/src/components/catalog/EntityPage.tsx | 11 ++ plugins/kubernetes-cluster/.eslintrc.js | 1 + plugins/kubernetes-cluster/api-report.md | 24 +++ plugins/kubernetes-cluster/catalog-info.yaml | 10 ++ plugins/kubernetes-cluster/package.json | 80 ++++++++++ plugins/kubernetes-cluster/src/Router.tsx | 56 +++++++ .../ApiResources/ApiResources.test.tsx | 64 ++++++++ .../components/ApiResources/ApiResources.tsx | 80 ++++++++++ .../ApiResources/useApiResources.ts | 48 ++++++ .../ClusterOverview/ClusterOverview.test.tsx | 60 ++++++++ .../ClusterOverview/ClusterOverview.tsx | 65 ++++++++ .../src/components/ClusterOverview/index.ts | 17 +++ .../components/ClusterOverview/useCluster.ts | 40 +++++ .../KubernetesClusterContent.tsx | 64 ++++++++ .../KubernetesClusterContent/index.ts | 17 +++ .../KubernetesClusterErrorContext.tsx | 50 ++++++ .../src/components/Nodes/Nodes.test.tsx | 107 +++++++++++++ .../src/components/Nodes/Nodes.tsx | 144 ++++++++++++++++++ .../src/components/Nodes/useNodes.ts | 48 ++++++ plugins/kubernetes-cluster/src/index.ts | 27 ++++ plugins/kubernetes-cluster/src/plugin.ts | 54 +++++++ plugins/kubernetes-react/api-report.md | 18 ++- .../src/api/KubernetesBackendClient.ts | 2 +- plugins/kubernetes-react/src/api/types.ts | 11 +- plugins/kubernetes/dev/index.tsx | 6 + yarn.lock | 52 ++++++- 28 files changed, 1158 insertions(+), 7 deletions(-) create mode 100644 .changeset/slow-dodos-remember.md create mode 100644 plugins/kubernetes-cluster/.eslintrc.js create mode 100644 plugins/kubernetes-cluster/api-report.md create mode 100644 plugins/kubernetes-cluster/catalog-info.yaml create mode 100644 plugins/kubernetes-cluster/package.json create mode 100644 plugins/kubernetes-cluster/src/Router.tsx create mode 100644 plugins/kubernetes-cluster/src/components/ApiResources/ApiResources.test.tsx create mode 100644 plugins/kubernetes-cluster/src/components/ApiResources/ApiResources.tsx create mode 100644 plugins/kubernetes-cluster/src/components/ApiResources/useApiResources.ts create mode 100644 plugins/kubernetes-cluster/src/components/ClusterOverview/ClusterOverview.test.tsx create mode 100644 plugins/kubernetes-cluster/src/components/ClusterOverview/ClusterOverview.tsx create mode 100644 plugins/kubernetes-cluster/src/components/ClusterOverview/index.ts create mode 100644 plugins/kubernetes-cluster/src/components/ClusterOverview/useCluster.ts create mode 100644 plugins/kubernetes-cluster/src/components/KubernetesClusterContent/KubernetesClusterContent.tsx create mode 100644 plugins/kubernetes-cluster/src/components/KubernetesClusterContent/index.ts create mode 100644 plugins/kubernetes-cluster/src/components/KubernetesClusterErrorContext/KubernetesClusterErrorContext.tsx create mode 100644 plugins/kubernetes-cluster/src/components/Nodes/Nodes.test.tsx create mode 100644 plugins/kubernetes-cluster/src/components/Nodes/Nodes.tsx create mode 100644 plugins/kubernetes-cluster/src/components/Nodes/useNodes.ts create mode 100644 plugins/kubernetes-cluster/src/index.ts create mode 100644 plugins/kubernetes-cluster/src/plugin.ts diff --git a/.changeset/slow-dodos-remember.md b/.changeset/slow-dodos-remember.md new file mode 100644 index 0000000000..fcc49e3f45 --- /dev/null +++ b/.changeset/slow-dodos-remember.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-kubernetes-backend': patch +'@backstage/plugin-kubernetes-cluster': patch +'@backstage/plugin-kubernetes-react': patch +'@backstage/plugin-kubernetes': patch +--- + +Add Kubernetes cluster plugin. Viewing Kubernetes clusters as an Admin from Backstage diff --git a/packages/app/package.json b/packages/app/package.json index e23da6c8dc..6e06d2ca03 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -52,6 +52,7 @@ "@backstage/plugin-jenkins": "workspace:^", "@backstage/plugin-kafka": "workspace:^", "@backstage/plugin-kubernetes": "workspace:^", + "@backstage/plugin-kubernetes-cluster": "workspace:^", "@backstage/plugin-lighthouse": "workspace:^", "@backstage/plugin-linguist": "workspace:^", "@backstage/plugin-linguist-common": "workspace:^", diff --git a/packages/app/src/components/catalog/EntityPage.tsx b/packages/app/src/components/catalog/EntityPage.tsx index ae78d0b771..81af3b91e7 100644 --- a/packages/app/src/components/catalog/EntityPage.tsx +++ b/packages/app/src/components/catalog/EntityPage.tsx @@ -103,6 +103,10 @@ import { } from '@backstage/plugin-jenkins'; import { EntityKafkaContent } from '@backstage/plugin-kafka'; import { EntityKubernetesContent } from '@backstage/plugin-kubernetes'; +import { + isKubernetesClusterAvailable, + EntityKubernetesClusterContent, +} from '@backstage/plugin-kubernetes-cluster'; import { EntityLastLighthouseAuditCard, EntityLighthouseContent, @@ -898,6 +902,13 @@ const resourcePage = ( + + + Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts +/// + +import { Entity } from '@backstage/catalog-model'; +import { default as React_2 } from 'react'; + +// @public +export const EntityKubernetesClusterContent: ( + props: EntityKubernetesClusterContentProps, +) => JSX.Element; + +// @public +export type EntityKubernetesClusterContentProps = {}; + +// @public (undocumented) +export const isKubernetesClusterAvailable: (entity: Entity) => boolean; + +// @public (undocumented) +export const Router: () => React_2.JSX.Element; +``` diff --git a/plugins/kubernetes-cluster/catalog-info.yaml b/plugins/kubernetes-cluster/catalog-info.yaml new file mode 100644 index 0000000000..3da5102e46 --- /dev/null +++ b/plugins/kubernetes-cluster/catalog-info.yaml @@ -0,0 +1,10 @@ +apiVersion: backstage.io/v1alpha1 +kind: Component +metadata: + name: backstage-plugin-kubernetes-cluster + title: '@backstage/plugin-kubernetes-cluster' + description: A Backstage plugin that integrates towards Kubernetes clusters +spec: + lifecycle: experimental + type: backstage-frontend-plugin + owner: kubernetes-maintainers diff --git a/plugins/kubernetes-cluster/package.json b/plugins/kubernetes-cluster/package.json new file mode 100644 index 0000000000..2e1c8d8309 --- /dev/null +++ b/plugins/kubernetes-cluster/package.json @@ -0,0 +1,80 @@ +{ + "name": "@backstage/plugin-kubernetes-cluster", + "description": "A Backstage plugin that shows details of Kubernetes clusters", + "version": "0.0.0", + "main": "src/index.ts", + "types": "src/index.ts", + "license": "Apache-2.0", + "publishConfig": { + "access": "public", + "main": "dist/index.esm.js", + "types": "dist/index.d.ts" + }, + "backstage": { + "role": "frontend-plugin" + }, + "homepage": "https://backstage.io", + "repository": { + "type": "git", + "url": "https://github.com/backstage/backstage", + "directory": "plugins/kubernetes-cluster" + }, + "keywords": [ + "backstage", + "kubernetes" + ], + "sideEffects": false, + "scripts": { + "build": "backstage-cli package build", + "start": "backstage-cli package start", + "lint": "backstage-cli package lint", + "test": "backstage-cli package test", + "prepack": "backstage-cli package prepack", + "postpack": "backstage-cli package postpack", + "clean": "backstage-cli package clean" + }, + "dependencies": { + "@backstage/catalog-model": "workspace:^", + "@backstage/config": "workspace:^", + "@backstage/core-components": "workspace:^", + "@backstage/core-plugin-api": "workspace:^", + "@backstage/errors": "workspace:^", + "@backstage/plugin-catalog-react": "workspace:^", + "@backstage/plugin-kubernetes-common": "workspace:^", + "@backstage/plugin-kubernetes-react": "workspace:^", + "@backstage/theme": "workspace:^", + "@kubernetes-models/apimachinery": "^1.1.0", + "@kubernetes-models/base": "^4.0.1", + "@material-ui/core": "^4.12.2", + "@material-ui/icons": "^4.9.1", + "@material-ui/lab": "4.0.0-alpha.61", + "@types/react": "^16.13.1 || ^17.0.0", + "cronstrue": "^2.2.0", + "js-yaml": "^4.0.0", + "kubernetes-models": "^4.1.0", + "lodash": "^4.17.21", + "luxon": "^3.0.0", + "react-use": "^17.2.4" + }, + "peerDependencies": { + "react": "^16.13.1 || ^17.0.0", + "react-dom": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" + }, + "devDependencies": { + "@backstage/cli": "workspace:^", + "@backstage/core-app-api": "workspace:^", + "@backstage/dev-utils": "workspace:^", + "@backstage/test-utils": "workspace:^", + "@testing-library/dom": "^8.0.0", + "@testing-library/jest-dom": "^5.10.1", + "@testing-library/react": "^12.1.3", + "@testing-library/react-hooks": "^8.0.0", + "@testing-library/user-event": "^14.0.0", + "@types/node": "^16.11.26", + "msw": "^1.0.0" + }, + "files": [ + "dist" + ] +} diff --git a/plugins/kubernetes-cluster/src/Router.tsx b/plugins/kubernetes-cluster/src/Router.tsx new file mode 100644 index 0000000000..86563439ce --- /dev/null +++ b/plugins/kubernetes-cluster/src/Router.tsx @@ -0,0 +1,56 @@ +/* + * 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 React from 'react'; +import { Entity } from '@backstage/catalog-model'; +import { useEntity } from '@backstage/plugin-catalog-react'; +import { Route, Routes } from 'react-router-dom'; +import { MissingAnnotationEmptyState } from '@backstage/core-components'; +import { ANNOTATION_KUBERNETES_API_SERVER } from '@backstage/plugin-kubernetes-common'; +import { KubernetesClusterContent } from './components/KubernetesClusterContent'; + +/** + * + * + * @public + */ +export const isKubernetesClusterAvailable = (entity: Entity) => + Boolean(entity.metadata.annotations?.[ANNOTATION_KUBERNETES_API_SERVER]); + +/** + * + * + * @public + */ +export const Router = () => { + const { entity } = useEntity(); + + if (isKubernetesClusterAvailable(entity)) { + return ( + + } /> + + ); + } + + return ( + <> + + + ); +}; diff --git a/plugins/kubernetes-cluster/src/components/ApiResources/ApiResources.test.tsx b/plugins/kubernetes-cluster/src/components/ApiResources/ApiResources.test.tsx new file mode 100644 index 0000000000..601ba54a6d --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/ApiResources/ApiResources.test.tsx @@ -0,0 +1,64 @@ +/* + * 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 React from 'react'; +import { renderInTestApp } from '@backstage/test-utils'; +import { ApiResources } from './ApiResources'; +import '@testing-library/jest-dom'; + +jest.mock('@backstage/plugin-catalog-react', () => ({ + useEntity: () => { + return { + entity: { + metadata: { + name: 'some-cluster', + }, + }, + }; + }, +})); + +jest.mock('./useApiResources', () => ({ + useApiResources: jest.fn().mockReturnValue({ + loading: false, + value: { + groups: [ + { + name: 'some-apiVersion', + preferredVersion: { + groupVersion: 'some-group-version', + }, + }, + ], + }, + }), +})); + +describe('ApiResources', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('displays ApiResources', async () => { + const { getByText } = await renderInTestApp(); + + // Title + expect(getByText('Name')).toBeInTheDocument(); + expect(getByText('Preferred Version')).toBeInTheDocument(); + + // Row 1 + expect(getByText('some-apiVersion')).toBeInTheDocument(); + expect(getByText('some-group-version')).toBeInTheDocument(); + }); +}); diff --git a/plugins/kubernetes-cluster/src/components/ApiResources/ApiResources.tsx b/plugins/kubernetes-cluster/src/components/ApiResources/ApiResources.tsx new file mode 100644 index 0000000000..f6e8bc65ce --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/ApiResources/ApiResources.tsx @@ -0,0 +1,80 @@ +/* + * Copyright 2023 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 { useApiResources } from './useApiResources'; +import React, { useCallback, useEffect } from 'react'; +import { useEntity } from '@backstage/plugin-catalog-react'; +import { Table, TableColumn } from '@backstage/core-components'; +import { IAPIGroup } from '@kubernetes-models/apimachinery/apis/meta/v1'; +import { useKubernetesClusterError } from '../KubernetesClusterErrorContext/KubernetesClusterErrorContext'; +import { makeStyles } from '@material-ui/core'; + +const useStyles = makeStyles(theme => ({ + empty: { + padding: theme.spacing(2), + display: 'flex', + justifyContent: 'center', + }, +})); + +const defaultColumns: TableColumn[] = [ + { + title: 'Name', + highlight: true, + render: (apiGroup: IAPIGroup) => { + return apiGroup.name; + }, + }, + { + title: 'Preferred Version', + highlight: true, + render: (apiGroup: IAPIGroup) => { + return apiGroup.preferredVersion?.groupVersion; + }, + }, +]; + +export const ApiResources = () => { + const classes = useStyles(); + const { entity } = useEntity(); + const { setError } = useKubernetesClusterError(); + const setErrorCallback = useCallback(setError, [setError]); + const { value, error, loading } = useApiResources({ + clusterName: entity.metadata.name, + }); + + useEffect(() => { + if (error) { + setErrorCallback(error.message); + } + }, [error, setErrorCallback]); + + return ( + + {error !== undefined + ? 'Error loading API Resources' + : 'No API Resources found'} + + } + columns={defaultColumns} + /> + ); +}; diff --git a/plugins/kubernetes-cluster/src/components/ApiResources/useApiResources.ts b/plugins/kubernetes-cluster/src/components/ApiResources/useApiResources.ts new file mode 100644 index 0000000000..b28b0b0b67 --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/ApiResources/useApiResources.ts @@ -0,0 +1,48 @@ +/* + * Copyright 2023 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 useAsync from 'react-use/lib/useAsync'; + +import { useApi } from '@backstage/core-plugin-api'; +import { kubernetesApiRef } from '@backstage/plugin-kubernetes-react'; +import { IAPIGroupList } from '@kubernetes-models/apimachinery/apis/meta/v1'; + +/** + * Arguments for useApiResources + * + * @public + */ +export interface ApiResourcesOptions { + clusterName: string; +} + +/** + * Retrieves the logs for the given pod + * + * @public + */ +export const useApiResources = ({ clusterName }: ApiResourcesOptions) => { + const kubernetesApi = useApi(kubernetesApiRef); + return useAsync(async () => { + return await kubernetesApi + .proxy({ + clusterName, + path: '/apis', + }) + .then(r => { + return r.json() as Promise; + }); + }, [clusterName]); +}; diff --git a/plugins/kubernetes-cluster/src/components/ClusterOverview/ClusterOverview.test.tsx b/plugins/kubernetes-cluster/src/components/ClusterOverview/ClusterOverview.test.tsx new file mode 100644 index 0000000000..6398093a74 --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/ClusterOverview/ClusterOverview.test.tsx @@ -0,0 +1,60 @@ +/* + * 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 React from 'react'; +import { renderInTestApp } from '@backstage/test-utils'; +import { ClusterOverview } from './ClusterOverview'; +import '@testing-library/jest-dom'; + +jest.mock('@backstage/plugin-catalog-react', () => ({ + useEntity: () => { + return { + entity: { + metadata: { + name: 'some-cluster', + }, + }, + }; + }, +})); + +jest.mock('./useCluster', () => ({ + useCluster: jest.fn().mockReturnValue({ + loading: false, + value: { + name: 'some-cluster', + authProvider: 'google', + }, + }), +})); + +describe('ClusterOverview', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('displays ClusterOverview', async () => { + const { getByText, queryAllByText } = await renderInTestApp( + , + ); + + expect(getByText('Name')).toBeInTheDocument(); + expect(getByText('some-cluster')).toBeInTheDocument(); + expect(getByText('Backstage Auth Provider')).toBeInTheDocument(); + expect(getByText('google')).toBeInTheDocument(); + expect(getByText('OIDC Token Provider')).toBeInTheDocument(); + expect(getByText('Dashboard Link')).toBeInTheDocument(); + expect(queryAllByText('N/A')).toHaveLength(2); + }); +}); diff --git a/plugins/kubernetes-cluster/src/components/ClusterOverview/ClusterOverview.tsx b/plugins/kubernetes-cluster/src/components/ClusterOverview/ClusterOverview.tsx new file mode 100644 index 0000000000..a4d4ffc40c --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/ClusterOverview/ClusterOverview.tsx @@ -0,0 +1,65 @@ +/* + * Copyright 2023 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 React, { useCallback, useEffect } from 'react'; +import { InfoCard, StructuredMetadataTable } from '@backstage/core-components'; +import { useEntity } from '@backstage/plugin-catalog-react'; +import { useCluster } from './useCluster'; +import { Skeleton } from '@material-ui/lab'; +import { Theme, createStyles, makeStyles } from '@material-ui/core'; +import { useKubernetesClusterError } from '../KubernetesClusterErrorContext/KubernetesClusterErrorContext'; + +const useStyles = makeStyles((_theme: Theme) => + createStyles({ + root: { + height: '100%', + }, + }), +); + +export const ClusterOverview = () => { + const classes = useStyles(); + const { entity } = useEntity(); + const { value, loading, error } = useCluster({ + clusterName: entity.metadata.name, + }); + const { setError } = useKubernetesClusterError(); + const setErrorCallback = useCallback(setError, [setError]); + useEffect(() => { + if (error) { + setErrorCallback(error.message); + } + }, [error, setErrorCallback]); + + return ( + + {!value && loading && ( + <> + + + )} + {value && ( + + )} + + ); +}; diff --git a/plugins/kubernetes-cluster/src/components/ClusterOverview/index.ts b/plugins/kubernetes-cluster/src/components/ClusterOverview/index.ts new file mode 100644 index 0000000000..5a46125f3e --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/ClusterOverview/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 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 { ClusterOverview } from './ClusterOverview'; diff --git a/plugins/kubernetes-cluster/src/components/ClusterOverview/useCluster.ts b/plugins/kubernetes-cluster/src/components/ClusterOverview/useCluster.ts new file mode 100644 index 0000000000..05106cdc5b --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/ClusterOverview/useCluster.ts @@ -0,0 +1,40 @@ +/* + * Copyright 2023 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 useAsync from 'react-use/lib/useAsync'; + +import { useApi } from '@backstage/core-plugin-api'; +import { kubernetesApiRef } from '@backstage/plugin-kubernetes-react'; + +/** + * Arguments for useApiResources + * + * @public + */ +export interface UseClusterOptions { + clusterName: string; +} + +/** + * Retrieves the logs for the given pod + * + * @public + */ +export const useCluster = ({ clusterName }: UseClusterOptions) => { + const kubernetesApi = useApi(kubernetesApiRef); + return useAsync(async () => { + return await kubernetesApi.getCluster(clusterName); + }, [clusterName]); +}; diff --git a/plugins/kubernetes-cluster/src/components/KubernetesClusterContent/KubernetesClusterContent.tsx b/plugins/kubernetes-cluster/src/components/KubernetesClusterContent/KubernetesClusterContent.tsx new file mode 100644 index 0000000000..665ca2acdd --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/KubernetesClusterContent/KubernetesClusterContent.tsx @@ -0,0 +1,64 @@ +/* + * Copyright 2023 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 React from 'react'; +import { ApiResources } from '../ApiResources/ApiResources'; +import { Grid, Typography } from '@material-ui/core'; +import { Nodes } from '../Nodes/Nodes'; +import { ClusterOverview } from '../ClusterOverview'; +import { + KubernetesClusterErrorProvider, + useKubernetesClusterError, +} from '../KubernetesClusterErrorContext/KubernetesClusterErrorContext'; +import { WarningPanel } from '@backstage/core-components'; + +const ContentGrid = () => { + const { error } = useKubernetesClusterError(); + return ( + <> + + {error && ( + + + {error} + + + )} + + + + + + + + + + + + ); +}; + +/** + * + * + * @public + */ +export const KubernetesClusterContent = () => { + return ( + + + + ); +}; diff --git a/plugins/kubernetes-cluster/src/components/KubernetesClusterContent/index.ts b/plugins/kubernetes-cluster/src/components/KubernetesClusterContent/index.ts new file mode 100644 index 0000000000..2f5850c10c --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/KubernetesClusterContent/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 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 { KubernetesClusterContent } from './KubernetesClusterContent'; diff --git a/plugins/kubernetes-cluster/src/components/KubernetesClusterErrorContext/KubernetesClusterErrorContext.tsx b/plugins/kubernetes-cluster/src/components/KubernetesClusterErrorContext/KubernetesClusterErrorContext.tsx new file mode 100644 index 0000000000..049a7b541d --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/KubernetesClusterErrorContext/KubernetesClusterErrorContext.tsx @@ -0,0 +1,50 @@ +/* + * Copyright 2023 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 React, { useCallback, useContext, useState } from 'react'; + +export interface ErrorContext { + error?: string; + setError: (message: string) => void; +} + +export const KubernetesClusterErrorContext = React.createContext({ + setError: (_: string) => {}, +}); + +export interface KubernetesClusterErrorProviderProps { + children: React.ReactNode; +} + +export const KubernetesClusterErrorProvider = ({ + children, +}: KubernetesClusterErrorProviderProps) => { + const [error, setError] = useState(undefined); + + const contextValue: ErrorContext = { + error, + setError: useCallback((message: string) => setError(message), []), + }; + + return ( + + {children} + + ); +}; + +export const useKubernetesClusterError = () => { + return useContext(KubernetesClusterErrorContext); +}; diff --git a/plugins/kubernetes-cluster/src/components/Nodes/Nodes.test.tsx b/plugins/kubernetes-cluster/src/components/Nodes/Nodes.test.tsx new file mode 100644 index 0000000000..f48fdfbcef --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/Nodes/Nodes.test.tsx @@ -0,0 +1,107 @@ +/* + * 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 React from 'react'; +import { renderInTestApp } from '@backstage/test-utils'; +import { Nodes } from './Nodes'; +import '@testing-library/jest-dom'; + +jest.mock('@backstage/plugin-catalog-react', () => ({ + useEntity: () => { + return { + entity: { + metadata: { + name: 'some-cluster', + }, + }, + }; + }, +})); + +jest.mock('./useNodes', () => ({ + useNodes: jest.fn().mockReturnValue({ + loading: false, + value: { + items: [ + { + metadata: { + name: 'some-node-name', + }, + status: { + conditions: [ + { + type: 'Ready', + status: 'True', + }, + ], + nodeInfo: { + operatingSystem: 'linux', + architecture: 'ARM', + }, + }, + }, + { + metadata: { + name: 'some-node-bad-name', + }, + spec: { + unschedulable: true, + }, + status: { + conditions: [ + { + type: 'Ready', + status: 'False', + }, + ], + nodeInfo: { + operatingSystem: 'windows', + architecture: 'x86', + }, + }, + }, + ], + }, + }), +})); + +describe('Nodes', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('displays nodes table - ready schedulable node', async () => { + const { getByText } = await renderInTestApp(); + + expect(getByText('Nodes')).toBeInTheDocument(); + + // Titles + expect(getByText('Name')).toBeInTheDocument(); + expect(getByText('Schedulable')).toBeInTheDocument(); + expect(getByText('Status')).toBeInTheDocument(); + expect(getByText('OS')).toBeInTheDocument(); + + // Row 1 + expect(getByText('some-node-name')).toBeInTheDocument(); + expect(getByText('✅')).toBeInTheDocument(); + expect(getByText('Ready')).toBeInTheDocument(); + expect(getByText('linux (ARM)')).toBeInTheDocument(); + + // Row 2 + expect(getByText('some-node-bad-name')).toBeInTheDocument(); + expect(getByText('❌')).toBeInTheDocument(); + expect(getByText('Not Ready')).toBeInTheDocument(); + expect(getByText('windows (x86)')).toBeInTheDocument(); + }); +}); diff --git a/plugins/kubernetes-cluster/src/components/Nodes/Nodes.tsx b/plugins/kubernetes-cluster/src/components/Nodes/Nodes.tsx new file mode 100644 index 0000000000..e7f6e957ae --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/Nodes/Nodes.tsx @@ -0,0 +1,144 @@ +/* + * Copyright 2023 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 { useNodes } from './useNodes'; +import React, { useCallback, useEffect } from 'react'; +import { useEntity } from '@backstage/plugin-catalog-react'; +import { + StructuredMetadataTable, + Table, + TableColumn, +} from '@backstage/core-components'; +import { INode } from 'kubernetes-models/v1'; +import { Grid, Typography, makeStyles } from '@material-ui/core'; +import { useKubernetesClusterError } from '../KubernetesClusterErrorContext/KubernetesClusterErrorContext'; +import { KubernetesDrawer } from '@backstage/plugin-kubernetes-react'; + +const useStyles = makeStyles(theme => ({ + empty: { + padding: theme.spacing(2), + display: 'flex', + justifyContent: 'center', + }, +})); + +const defaultColumns: TableColumn[] = [ + { + title: 'Name', + highlight: true, + width: 'auto', + render: (node: INode) => { + return ( + + + + Node Info + + + + Addresses + { + accum[next.type] = next.address; + return accum; + }, {} as any) ?? {} + } + /> + + + Taints + { + accum[`${next.effect}`] = `${next.key} (${next.value})`; + return accum; + }, {} as any) ?? {} + } + /> + + + + ); + }, + }, + { + title: 'Schedulable', + align: 'center', + width: 'auto', + render: (node: INode) => { + if (node.spec?.unschedulable) { + return '❌'; + } + return '✅'; + }, + }, + { + title: 'Status', + width: 'auto', + render: (node: INode) => { + // TODO add an icon + const readyCondition = node.status?.conditions?.find(c => { + return c.type === 'Ready'; + }); + if (!readyCondition) { + return 'Unknown'; + } + return readyCondition.status === 'True' ? 'Ready' : 'Not Ready'; + }, + }, + { + title: 'OS', + width: 'auto', + render: (node: INode) => { + return `${node.status?.nodeInfo?.operatingSystem ?? 'unknown'} (${ + node.status?.nodeInfo?.architecture ?? '?' + })`; + }, + }, +]; + +export const Nodes = () => { + const classes = useStyles(); + const { entity } = useEntity(); + const { value, error, loading } = useNodes({ + clusterName: entity.metadata.name, + }); + const { setError } = useKubernetesClusterError(); + const setErrorCallback = useCallback(setError, [setError]); + useEffect(() => { + if (error) { + setErrorCallback(error.message); + } + }, [error, setErrorCallback]); + + return ( +
+ {error !== undefined ? 'Error loading nodes' : 'No nodes found'} + + } + columns={defaultColumns} + /> + ); +}; diff --git a/plugins/kubernetes-cluster/src/components/Nodes/useNodes.ts b/plugins/kubernetes-cluster/src/components/Nodes/useNodes.ts new file mode 100644 index 0000000000..1feaff3bdb --- /dev/null +++ b/plugins/kubernetes-cluster/src/components/Nodes/useNodes.ts @@ -0,0 +1,48 @@ +/* + * Copyright 2023 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 useAsync from 'react-use/lib/useAsync'; + +import { useApi } from '@backstage/core-plugin-api'; +import { kubernetesApiRef } from '@backstage/plugin-kubernetes-react'; +import { NodeList } from 'kubernetes-models/v1'; + +/** + * Arguments for useApiResources + * + * @public + */ +export interface useNodesOptions { + clusterName: string; +} + +/** + * Retrieves nodes for a cluster + * + * @public + */ +export const useNodes = ({ clusterName }: useNodesOptions) => { + const kubernetesApi = useApi(kubernetesApiRef); + return useAsync(async () => { + return await kubernetesApi + .proxy({ + clusterName, + path: '/api/v1/nodes?limit=500', + }) + .then(r => { + return r.json() as Promise; + }); + }, [clusterName]); +}; diff --git a/plugins/kubernetes-cluster/src/index.ts b/plugins/kubernetes-cluster/src/index.ts new file mode 100644 index 0000000000..6171b151e4 --- /dev/null +++ b/plugins/kubernetes-cluster/src/index.ts @@ -0,0 +1,27 @@ +/* + * Copyright 2023 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. + */ + +/** + * A Backstage plugin that integrates towards Kubernetes + * + * @packageDocumentation + */ + +export { + EntityKubernetesClusterContent, + type EntityKubernetesClusterContentProps, +} from './plugin'; +export { Router, isKubernetesClusterAvailable } from './Router'; diff --git a/plugins/kubernetes-cluster/src/plugin.ts b/plugins/kubernetes-cluster/src/plugin.ts new file mode 100644 index 0000000000..5cd9b2287c --- /dev/null +++ b/plugins/kubernetes-cluster/src/plugin.ts @@ -0,0 +1,54 @@ +/* + * 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 { + createPlugin, + createRouteRef, + createRoutableExtension, +} from '@backstage/core-plugin-api'; + +export const rootCatalogKubernetesClusterRouteRef = createRouteRef({ + id: 'kubernetes-cluster', +}); + +export const kubernetesClusterPlugin = createPlugin({ + id: 'kubernetes-cluster', + apis: [], + routes: { + entityContent: rootCatalogKubernetesClusterRouteRef, + }, +}); + +/** + * Props of EntityKubernetesContent + * + * @public + */ +export type EntityKubernetesClusterContentProps = {}; + +/** + * Props of EntityKubernetesContent + * + * @public + */ +export const EntityKubernetesClusterContent: ( + props: EntityKubernetesClusterContentProps, +) => JSX.Element = kubernetesClusterPlugin.provide( + createRoutableExtension({ + name: 'EntityKubernetesClusterContent', + component: () => import('./Router').then(m => m.Router), + mountPoint: rootCatalogKubernetesClusterRouteRef, + }), +); diff --git a/plugins/kubernetes-react/api-report.md b/plugins/kubernetes-react/api-report.md index e182524426..bb8e4923f6 100644 --- a/plugins/kubernetes-react/api-report.md +++ b/plugins/kubernetes-react/api-report.md @@ -251,12 +251,22 @@ export type JobsAccordionsProps = { // @public (undocumented) export interface KubernetesApi { + // (undocumented) + getCluster(clusterName: string): Promise< + | { + name: string; + authProvider: string; + oidcTokenProvider?: string; + dashboardUrl?: string; + } + | undefined + >; // (undocumented) getClusters(): Promise< { name: string; authProvider: string; - oidcTokenProvider?: string | undefined; + oidcTokenProvider?: string; }[] >; // (undocumented) @@ -338,6 +348,12 @@ export class KubernetesBackendClient implements KubernetesApi { kubernetesAuthProvidersApi: KubernetesAuthProvidersApi; }); // (undocumented) + getCluster(clusterName: string): Promise<{ + name: string; + authProvider: string; + oidcTokenProvider?: string; + }>; + // (undocumented) getClusters(): Promise< { name: string; diff --git a/plugins/kubernetes-react/src/api/KubernetesBackendClient.ts b/plugins/kubernetes-react/src/api/KubernetesBackendClient.ts index d4e11c43da..a769eb088d 100644 --- a/plugins/kubernetes-react/src/api/KubernetesBackendClient.ts +++ b/plugins/kubernetes-react/src/api/KubernetesBackendClient.ts @@ -75,7 +75,7 @@ export class KubernetesBackendClient implements KubernetesApi { return this.handleResponse(response); } - private async getCluster(clusterName: string): Promise<{ + public async getCluster(clusterName: string): Promise<{ name: string; authProvider: string; oidcTokenProvider?: string; diff --git a/plugins/kubernetes-react/src/api/types.ts b/plugins/kubernetes-react/src/api/types.ts index c0f97ffd41..28cf2dde55 100644 --- a/plugins/kubernetes-react/src/api/types.ts +++ b/plugins/kubernetes-react/src/api/types.ts @@ -42,9 +42,18 @@ export interface KubernetesApi { { name: string; authProvider: string; - oidcTokenProvider?: string | undefined; + oidcTokenProvider?: string; }[] >; + getCluster(clusterName: string): Promise< + | { + name: string; + authProvider: string; + oidcTokenProvider?: string; + dashboardUrl?: string; + } + | undefined + >; getWorkloadsByEntity( request: WorkloadsByEntityRequest, ): Promise; diff --git a/plugins/kubernetes/dev/index.tsx b/plugins/kubernetes/dev/index.tsx index 520f8802a9..fbc666ae13 100644 --- a/plugins/kubernetes/dev/index.tsx +++ b/plugins/kubernetes/dev/index.tsx @@ -116,6 +116,12 @@ class MockKubernetesClient implements KubernetesApi { return [{ name: 'mock-cluster', authProvider: 'serviceAccount' }]; } + async getCluster( + _clusterName: string, + ): Promise<{ name: string; authProvider: string }> { + return { name: 'mock-cluster', authProvider: 'serviceAccount' }; + } + async proxy(_options: { clusterName: String; path: String }): Promise { return { kind: 'Namespace', diff --git a/yarn.lock b/yarn.lock index 2c48e783e0..38af62fcd1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7643,6 +7643,49 @@ __metadata: languageName: unknown linkType: soft +"@backstage/plugin-kubernetes-cluster@workspace:^, @backstage/plugin-kubernetes-cluster@workspace:plugins/kubernetes-cluster": + version: 0.0.0-use.local + resolution: "@backstage/plugin-kubernetes-cluster@workspace:plugins/kubernetes-cluster" + dependencies: + "@backstage/catalog-model": "workspace:^" + "@backstage/cli": "workspace:^" + "@backstage/config": "workspace:^" + "@backstage/core-app-api": "workspace:^" + "@backstage/core-components": "workspace:^" + "@backstage/core-plugin-api": "workspace:^" + "@backstage/dev-utils": "workspace:^" + "@backstage/errors": "workspace:^" + "@backstage/plugin-catalog-react": "workspace:^" + "@backstage/plugin-kubernetes-common": "workspace:^" + "@backstage/plugin-kubernetes-react": "workspace:^" + "@backstage/test-utils": "workspace:^" + "@backstage/theme": "workspace:^" + "@kubernetes-models/apimachinery": ^1.1.0 + "@kubernetes-models/base": ^4.0.1 + "@material-ui/core": ^4.12.2 + "@material-ui/icons": ^4.9.1 + "@material-ui/lab": 4.0.0-alpha.61 + "@testing-library/dom": ^8.0.0 + "@testing-library/jest-dom": ^5.10.1 + "@testing-library/react": ^12.1.3 + "@testing-library/react-hooks": ^8.0.0 + "@testing-library/user-event": ^14.0.0 + "@types/node": ^16.11.26 + "@types/react": ^16.13.1 || ^17.0.0 + cronstrue: ^2.2.0 + js-yaml: ^4.0.0 + kubernetes-models: ^4.1.0 + lodash: ^4.17.21 + luxon: ^3.0.0 + msw: ^1.0.0 + react-use: ^17.2.4 + peerDependencies: + react: ^16.13.1 || ^17.0.0 + react-dom: ^16.13.1 || ^17.0.0 + react-router-dom: 6.0.0-beta.0 || ^6.3.0 + languageName: unknown + linkType: soft + "@backstage/plugin-kubernetes-common@workspace:^, @backstage/plugin-kubernetes-common@workspace:plugins/kubernetes-common": version: 0.0.0-use.local resolution: "@backstage/plugin-kubernetes-common@workspace:plugins/kubernetes-common" @@ -17989,10 +18032,10 @@ __metadata: languageName: node linkType: hard -"@types/node@npm:^16.9.2": - version: 16.18.53 - resolution: "@types/node@npm:16.18.53" - checksum: 26c05cde59664360c22e0dda70776ca6f1b35f0b94e4f84d2c21e2afa2e69ac3a2c99bbb57b43405f81df1b2598f6d707ccc7c4c31865f90e45c4625d8400518 +"@types/node@npm:^16.11.26, @types/node@npm:^16.9.2": + version: 16.18.54 + resolution: "@types/node@npm:16.18.54" + checksum: 208e8fc64f605e9cd55ab5e620a0fd019d8fe5629e3e3c5de869a149b731ab0fac5720c516dccc0ecc834ac27df754723dfe6554551663f016ba5096ea8851df languageName: node linkType: hard @@ -25544,6 +25587,7 @@ __metadata: "@backstage/plugin-jenkins": "workspace:^" "@backstage/plugin-kafka": "workspace:^" "@backstage/plugin-kubernetes": "workspace:^" + "@backstage/plugin-kubernetes-cluster": "workspace:^" "@backstage/plugin-lighthouse": "workspace:^" "@backstage/plugin-linguist": "workspace:^" "@backstage/plugin-linguist-common": "workspace:^"