From 4b1831ac551f4bf83645364281b84cdb23a3d8b9 Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Tue, 16 Aug 2022 20:59:06 +1200 Subject: [PATCH 1/9] Patch scaffolder backend to support broken symlinks Signed-off-by: Marcus Crane --- .../deserializeDirectoryContents.test.ts | 4 ++ .../files/serializeDirectoryContents.test.ts | 38 +++++++++++++ .../lib/files/serializeDirectoryContents.ts | 25 +++++++-- .../scaffolder-backend/src/lib/files/types.ts | 1 + .../builtin/publish/githubPullRequest.test.ts | 54 +++++++++++++++++++ .../builtin/publish/githubPullRequest.ts | 8 +-- 6 files changed, 123 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/files/deserializeDirectoryContents.test.ts b/plugins/scaffolder-backend/src/lib/files/deserializeDirectoryContents.test.ts index 0e7de7f88d..c272e04205 100644 --- a/plugins/scaffolder-backend/src/lib/files/deserializeDirectoryContents.test.ts +++ b/plugins/scaffolder-backend/src/lib/files/deserializeDirectoryContents.test.ts @@ -41,6 +41,7 @@ describe('deserializeDirectoryContents', () => { path: 'a.txt', content: Buffer.from('a', 'utf8'), executable: false, + symlink: false, }, ]); }); @@ -65,16 +66,19 @@ describe('deserializeDirectoryContents', () => { path: 'a.txt', content: Buffer.from('a', 'utf8'), executable: false, + symlink: false, }, { path: 'a/b.txt', content: Buffer.from('b', 'utf8'), executable: false, + symlink: false, }, { path: 'a/b/c.txt', content: Buffer.from('c', 'utf8'), executable: false, + symlink: false, }, ]); }); diff --git a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts index b24a867504..efda22da02 100644 --- a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts +++ b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts @@ -28,21 +28,25 @@ describe('serializeDirectoryContents', () => { { path: 'index.ts', executable: false, + symlink: false, content: expect.any(Buffer), }, { path: 'types.ts', executable: false, + symlink: false, content: expect.any(Buffer), }, { path: 'serializeDirectoryContents.ts', executable: false, + symlink: false, content: expect.any(Buffer), }, { path: 'serializeDirectoryContents.test.ts', executable: false, + symlink: false, content: expect.any(Buffer), }, ]), @@ -72,26 +76,31 @@ describe('serializeDirectoryContents', () => { { path: 'a.txt', executable: false, + symlink: false, content: Buffer.from('a', 'utf8'), }, { path: 'b/b1.txt', executable: false, + symlink: false, content: Buffer.from('b1', 'utf8'), }, { path: 'b/b2.txt', executable: false, + symlink: false, content: Buffer.from('b2', 'utf8'), }, { path: 'c/c1/c11.txt', executable: false, + symlink: false, content: Buffer.from('c11', 'utf8'), }, { path: 'c/c1/c11/c111.txt', executable: false, + symlink: false, content: Buffer.from('c111', 'utf8'), }, ]); @@ -111,11 +120,31 @@ describe('serializeDirectoryContents', () => { { path: 'a.txt', executable: false, + symlink: false, content: Buffer.from('some text', 'utf8'), }, ]); }); + it('should pick up broken symlinks', async () => { + mockFs({ + root: { + 'b.txt': mockFs.symlink({ + path: './a.txt' + }) + } + }) + + await expect(serializeDirectoryContents('root')).resolves.toEqual([ + { + path: 'b.txt', + executable: false, + symlink: true, + content: './a.txt', + } + ]) + }) + it('should ignore symlinked folder files', async () => { mockFs({ root: { @@ -133,11 +162,13 @@ describe('serializeDirectoryContents', () => { { path: 'a.txt', executable: false, + symlink: false, content: Buffer.from('some text', 'utf8'), }, { path: 'linkme/b.txt', executable: false, + symlink: false, content: Buffer.from('lols', 'utf8'), }, ]); @@ -160,11 +191,13 @@ describe('serializeDirectoryContents', () => { { path: '.gitignore', executable: false, + symlink: false, content: Buffer.from('*.txt', 'utf8'), }, { path: 'a.log', executable: false, + symlink: false, content: Buffer.from('a', 'utf8'), }, ]); @@ -198,26 +231,31 @@ describe('serializeDirectoryContents', () => { { path: 'a.txt', executable: false, + symlink: false, content: Buffer.from('a', 'utf8'), }, { path: 'b/.b', executable: false, + symlink: false, content: Buffer.from('b', 'utf8'), }, { path: 'b/b.txt', executable: false, + symlink: false, content: Buffer.from('b', 'utf8'), }, { path: 'c/c.log', executable: false, + symlink: false, content: Buffer.from('c', 'utf8'), }, { path: 'c/c.txt', executable: false, + symlink: false, content: Buffer.from('c', 'utf8'), }, ]); diff --git a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts index 608b9e71a5..b3d565973e 100644 --- a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts +++ b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts @@ -44,6 +44,9 @@ export async function serializeDirectoryContents( dot: true, gitignore: options?.gitignore, followSymbolicLinks: false, + // In order to pick up 'broken' symlinks, we oxymoronically request files AND folders yet we filter out folders + // This is because broken symlinks aren't classed as files so we need to glob everything + onlyFiles: false, objectMode: true, stats: true, }); @@ -51,12 +54,26 @@ export async function serializeDirectoryContents( const limiter = limiterFactory(10); return Promise.all( - paths.map(async ({ path, stats }) => ({ + paths + .filter(({ dirent }) => !dirent.isDirectory()) + .filter(({ dirent, path }) => { + if (!dirent.isSymbolicLink()) return true + if (!fs.existsSync(joinPath(sourcePath, path))) return true // We only want symlinks that DO NOT exist + return false + }) + .map(async ({ dirent, path, stats }) => ({ path, - content: await limiter(async () => - fs.readFile(joinPath(sourcePath, path)), - ), + content: await limiter(async () => { + const absFilePath = joinPath(sourcePath, path) + // Treat readlink as an explicit Buffer instead of implict utf-8 for consistency between types + const readLinkConf = { options: { encoding: null }} + if (dirent.isSymbolicLink()) { + return fs.readlink(absFilePath, readLinkConf) + } + return fs.readFile(absFilePath) + }), executable: isExecutable(stats?.mode), + symlink: dirent.isSymbolicLink(), })), ); } diff --git a/plugins/scaffolder-backend/src/lib/files/types.ts b/plugins/scaffolder-backend/src/lib/files/types.ts index 390d6791ee..d3a1eaa008 100644 --- a/plugins/scaffolder-backend/src/lib/files/types.ts +++ b/plugins/scaffolder-backend/src/lib/files/types.ts @@ -18,4 +18,5 @@ export interface SerializedFile { path: string; content: Buffer; executable?: boolean; + symlink?: boolean; } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts index 1bc768e4bd..b1f6ebfda2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts @@ -359,6 +359,60 @@ describe('createPublishGithubPullRequestAction', () => { }); }); + describe('with broken symlink', () => { + let input: GithubPullRequestActionInput; + let ctx: ActionContext; + + beforeEach(() => { + input = { + repoUrl: 'github.com?owner=myorg&repo=myrepo', + title: 'Create my new app', + branchName: 'new-app', + description: 'This PR is really good', + }; + + mockFs({ + [workspacePath]: { + 'Makefile': mockFs.symlink({ + path: '../../nothing/yet' + }) + }, + }); + + ctx = { + createTemporaryDirectory: jest.fn(), + output: jest.fn(), + logger: getRootLogger(), + logStream: new Writable(), + input, + workspacePath, + }; + }); + it('creates a pull request', async () => { + await instance.handler(ctx); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'myorg', + repo: 'myrepo', + title: 'Create my new app', + head: 'new-app', + body: 'This PR is really good', + changes: [ + { + commit: 'Create my new app', + files: { + 'Makefile': { + content: Buffer.from('../../nothing/yet').toString('utf-8'), + encoding: 'utf-8', + mode: '120000', + }, + }, + }, + ], + }); + }); + }) + describe('with executable file mode 755', () => { let input: GithubPullRequestActionInput; let ctx: ActionContext; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index 51049e4357..8dc4fd1c8c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -255,15 +255,17 @@ export const createPublishGithubPullRequestAction = ({ { // See the properties of tree items // in https://docs.github.com/en/rest/reference/git#trees - mode: file.executable ? '100755' : '100644', + mode: file.symlink ? '120000' : (file.executable ? '100755' : '100644'), // Always use base64 encoding to avoid doubling a binary file in size // due to interpreting a binary file as utf-8 and sending github // the utf-8 encoded content. // // For example, the original gradle-wrapper.jar is 57.8k in https://github.com/kennethzfeng/pull-request-test/pull/5/files. // Its size could be doubled to 98.3K (See https://github.com/kennethzfeng/pull-request-test/pull/4/files) - encoding: 'base64' as const, - content: file.content.toString('base64'), + encoding: file.symlink ? 'utf-8' : 'base64', + content: file.content.toString( + (file.symlink ? 'utf-8' : 'base64') + ), }, ]), ); From 096631e57129cad00eaf6e28bee92ba096fb14cf Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Tue, 16 Aug 2022 21:03:48 +1200 Subject: [PATCH 2/9] Add changeset Signed-off-by: Marcus Crane --- .changeset/light-donuts-obey.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/light-donuts-obey.md diff --git a/.changeset/light-donuts-obey.md b/.changeset/light-donuts-obey.md new file mode 100644 index 0000000000..abe6a1697c --- /dev/null +++ b/.changeset/light-donuts-obey.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Added support for handling broken symlinks within the scaffolder backend. This is intended for templates that may hold a symlink that is invalid at build time but valid within the destination repo. From 97ac8232019008e493c3481b73fca753c76aecbd Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Tue, 16 Aug 2022 21:14:30 +1200 Subject: [PATCH 3/9] Add extra template fetch test Signed-off-by: Marcus Crane --- .../actions/builtin/fetch/template.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index af353bef70..73dcba5b42 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -297,6 +297,9 @@ describe('fetch:template', () => { symlink: mockFs.symlink({ path: 'a-binary-file.png', }), + brokenSymlink: mockFs.symlink({ + path: './not-a-real-file.txt' + }) }, }); @@ -371,6 +374,17 @@ describe('fetch:template', () => { fs.realpath(`${workspacePath}/target/symlink`), ).resolves.toBe(joinPath(workspacePath, 'target', 'a-binary-file.png')); }); + it('copies broken symlinks as-is without processing them', async () => { + await expect( + fs + .lstat(`${workspacePath}/target/brokenSymlink`) + .then(i => i.isSymbolicLink()), + ).resolves.toBe(true); + + await expect( + fs.readlink(`${workspacePath}/target/brokenSymlink`) + ).resolves.toEqual('./not-a-real-file.txt'); + }) }); }); From f6a7e66dd04d941fbb0bb5c1704414ddf211953f Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Sat, 20 Aug 2022 23:24:32 +1200 Subject: [PATCH 4/9] Incorporate requested changes Signed-off-by: Marcus Crane --- .../lib/files/serializeDirectoryContents.ts | 32 +++++++++---------- .../builtin/publish/githubPullRequest.ts | 28 +++++++++++----- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts index b3d565973e..46c1002259 100644 --- a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts +++ b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts @@ -17,7 +17,7 @@ import fs from 'fs-extra'; import globby from 'globby'; import limiterFactory from 'p-limit'; -import { join as joinPath } from 'path'; +import { resolveSafeChildPath } from '@backstage/backend-common'; import { SerializedFile } from './types'; const DEFAULT_GLOB_PATTERNS = ['./**', '!.git']; @@ -57,23 +57,21 @@ export async function serializeDirectoryContents( paths .filter(({ dirent }) => !dirent.isDirectory()) .filter(({ dirent, path }) => { - if (!dirent.isSymbolicLink()) return true - if (!fs.existsSync(joinPath(sourcePath, path))) return true // We only want symlinks that DO NOT exist - return false + if (!dirent.isSymbolicLink()) return true; + if (!fs.existsSync(resolveSafeChildPath(sourcePath, path))) return true; // We only want symlinks that DO NOT exist (yet) + return false; }) .map(async ({ dirent, path, stats }) => ({ - path, - content: await limiter(async () => { - const absFilePath = joinPath(sourcePath, path) - // Treat readlink as an explicit Buffer instead of implict utf-8 for consistency between types - const readLinkConf = { options: { encoding: null }} - if (dirent.isSymbolicLink()) { - return fs.readlink(absFilePath, readLinkConf) - } - return fs.readFile(absFilePath) - }), - executable: isExecutable(stats?.mode), - symlink: dirent.isSymbolicLink(), - })), + path, + content: await limiter(async () => { + const absFilePath = resolveSafeChildPath(sourcePath, path); + if (dirent.isSymbolicLink()) { + return fs.readlinkSync(absFilePath, 'buffer'); + } + return fs.readFile(absFilePath); + }), + executable: isExecutable(stats?.mode), + symlink: dirent.isSymbolicLink(), + })), ); } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index 8dc4fd1c8c..b2c7a60120 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -26,7 +26,10 @@ import { InputError, CustomErrorBase } from '@backstage/errors'; import { createPullRequest } from 'octokit-plugin-create-pull-request'; import { resolveSafeChildPath } from '@backstage/backend-common'; import { getOctokitOptions } from '../github/helpers'; -import { serializeDirectoryContents } from '../../../../lib/files'; +import { + SerializedFile, + serializeDirectoryContents, +} from '../../../../lib/files'; import { Logger } from 'winston'; export type Encoding = 'utf-8' | 'base64'; @@ -249,23 +252,32 @@ export const createPublishGithubPullRequestAction = ({ const directoryContents = await serializeDirectoryContents(fileRoot, { gitignore: true, }); + + const determineFileMode = (file: SerializedFile): string => { + if (file.symlink) return '120000'; + if (file.executable) return '100755'; + return '100644'; + }; + + const determineFileEncoding = (file: SerializedFile): string => + file.symlink ? 'utf-8' : 'base64'; + const files = Object.fromEntries( directoryContents.map(file => [ targetPath ? path.posix.join(targetPath, file.path) : file.path, { // See the properties of tree items // in https://docs.github.com/en/rest/reference/git#trees - mode: file.symlink ? '120000' : (file.executable ? '100755' : '100644'), - // Always use base64 encoding to avoid doubling a binary file in size + mode: determineFileMode(file), + // Always use base64 encoding where possible to avoid doubling a binary file in size // due to interpreting a binary file as utf-8 and sending github - // the utf-8 encoded content. + // the utf-8 encoded content. Symlinks are kept as utf-8 to avoid them + // being formatted as a series of scrambled characters // // For example, the original gradle-wrapper.jar is 57.8k in https://github.com/kennethzfeng/pull-request-test/pull/5/files. // Its size could be doubled to 98.3K (See https://github.com/kennethzfeng/pull-request-test/pull/4/files) - encoding: file.symlink ? 'utf-8' : 'base64', - content: file.content.toString( - (file.symlink ? 'utf-8' : 'base64') - ), + encoding: determineFileEncoding(file), + content: file.content.toString(determineFileEncoding(file)), }, ]), ); From 9a895c052f2047f295cdbf0b94dd903bb5d7aa55 Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Sat, 20 Aug 2022 23:34:36 +1200 Subject: [PATCH 5/9] Fix up some type clashes Signed-off-by: Marcus Crane --- .../scaffolder/actions/builtin/publish/githubPullRequest.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index b2c7a60120..20b0ba83aa 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -259,8 +259,9 @@ export const createPublishGithubPullRequestAction = ({ return '100644'; }; - const determineFileEncoding = (file: SerializedFile): string => - file.symlink ? 'utf-8' : 'base64'; + const determineFileEncoding = ( + file: SerializedFile, + ): 'utf-8' | 'base64' => (file.symlink ? 'utf-8' : 'base64'); const files = Object.fromEntries( directoryContents.map(file => [ From c628c51b271f1761d26b1dfd3de4cec16d7b3a14 Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Sat, 20 Aug 2022 23:44:42 +1200 Subject: [PATCH 6/9] Fix up test that didn't wrap assertion in a buffer Signed-off-by: Marcus Crane --- .../lib/files/serializeDirectoryContents.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts index efda22da02..a22ea85301 100644 --- a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts +++ b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.test.ts @@ -130,20 +130,20 @@ describe('serializeDirectoryContents', () => { mockFs({ root: { 'b.txt': mockFs.symlink({ - path: './a.txt' - }) - } - }) + path: './a.txt', + }), + }, + }); await expect(serializeDirectoryContents('root')).resolves.toEqual([ { path: 'b.txt', executable: false, symlink: true, - content: './a.txt', - } - ]) - }) + content: Buffer.from('./a.txt', 'utf8'), + }, + ]); + }); it('should ignore symlinked folder files', async () => { mockFs({ From 83b1217faf32261f2164475db52c0ad4ef9724f3 Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Sun, 21 Aug 2022 00:10:11 +1200 Subject: [PATCH 7/9] =?UTF-8?q?Ran=20prettier=20=E2=9C=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marcus Crane --- .../scaffolder/actions/builtin/fetch/template.test.ts | 8 ++++---- .../actions/builtin/publish/githubPullRequest.test.ts | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 73dcba5b42..65a48672fb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -298,8 +298,8 @@ describe('fetch:template', () => { path: 'a-binary-file.png', }), brokenSymlink: mockFs.symlink({ - path: './not-a-real-file.txt' - }) + path: './not-a-real-file.txt', + }), }, }); @@ -382,9 +382,9 @@ describe('fetch:template', () => { ).resolves.toBe(true); await expect( - fs.readlink(`${workspacePath}/target/brokenSymlink`) + fs.readlink(`${workspacePath}/target/brokenSymlink`), ).resolves.toEqual('./not-a-real-file.txt'); - }) + }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts index b1f6ebfda2..2860d2a370 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts @@ -373,9 +373,9 @@ describe('createPublishGithubPullRequestAction', () => { mockFs({ [workspacePath]: { - 'Makefile': mockFs.symlink({ - path: '../../nothing/yet' - }) + Makefile: mockFs.symlink({ + path: '../../nothing/yet', + }), }, }); @@ -401,7 +401,7 @@ describe('createPublishGithubPullRequestAction', () => { { commit: 'Create my new app', files: { - 'Makefile': { + Makefile: { content: Buffer.from('../../nothing/yet').toString('utf-8'), encoding: 'utf-8', mode: '120000', @@ -411,7 +411,7 @@ describe('createPublishGithubPullRequestAction', () => { ], }); }); - }) + }); describe('with executable file mode 755', () => { let input: GithubPullRequestActionInput; From aa447735b6193db207d3dedb555530cb61f4ffb4 Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Sat, 27 Aug 2022 13:35:09 +1200 Subject: [PATCH 8/9] Use fs promises where possible Signed-off-by: Marcus Crane --- .../src/lib/files/serializeDirectoryContents.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts index 46c1002259..0c389969b9 100644 --- a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts +++ b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import fs from 'fs-extra'; +import { promises as fs, existsSync } from 'fs'; import globby from 'globby'; import limiterFactory from 'p-limit'; import { resolveSafeChildPath } from '@backstage/backend-common'; @@ -58,7 +58,8 @@ export async function serializeDirectoryContents( .filter(({ dirent }) => !dirent.isDirectory()) .filter(({ dirent, path }) => { if (!dirent.isSymbolicLink()) return true; - if (!fs.existsSync(resolveSafeChildPath(sourcePath, path))) return true; // We only want symlinks that DO NOT exist (yet) + const safePath = resolveSafeChildPath(sourcePath, path); + if (!existsSync(safePath)) return true; // We only want symlinks that DO NOT exist (yet) return false; }) .map(async ({ dirent, path, stats }) => ({ @@ -66,7 +67,7 @@ export async function serializeDirectoryContents( content: await limiter(async () => { const absFilePath = resolveSafeChildPath(sourcePath, path); if (dirent.isSymbolicLink()) { - return fs.readlinkSync(absFilePath, 'buffer'); + return fs.readlink(absFilePath, 'buffer'); } return fs.readFile(absFilePath); }), From 0674095ea3d717ec685fde1103da14c7da7690e6 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 31 Aug 2022 11:22:31 +0200 Subject: [PATCH 9/9] chore: remove the `existsSync` with an async filter Signed-off-by: blam --- .../lib/files/serializeDirectoryContents.ts | 58 ++++++++++++------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts index 0c389969b9..1197db000a 100644 --- a/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts +++ b/plugins/scaffolder-backend/src/lib/files/serializeDirectoryContents.ts @@ -14,11 +14,12 @@ * limitations under the License. */ -import { promises as fs, existsSync } from 'fs'; +import { promises as fs } from 'fs'; import globby from 'globby'; import limiterFactory from 'p-limit'; import { resolveSafeChildPath } from '@backstage/backend-common'; import { SerializedFile } from './types'; +import { isError } from '@backstage/errors'; const DEFAULT_GLOB_PATTERNS = ['./**', '!.git']; @@ -32,6 +33,14 @@ export const isExecutable = (fileMode: number | undefined) => { return res > 0; }; +async function asyncFilter( + array: T[], + callback: (value: T, index: number, array: T[]) => Promise, +): Promise { + const filterMap = await Promise.all(array.map(callback)); + return array.filter((_value, index) => filterMap[index]); +} + export async function serializeDirectoryContents( sourcePath: string, options?: { @@ -53,26 +62,33 @@ export async function serializeDirectoryContents( const limiter = limiterFactory(10); + const valid = await asyncFilter(paths, async ({ dirent, path }) => { + if (dirent.isDirectory()) return false; + if (!dirent.isSymbolicLink()) return true; + + const safePath = resolveSafeChildPath(sourcePath, path); + + // we only want files that don't exist + try { + await fs.stat(safePath); + return false; + } catch (e) { + return isError(e) && e.code === 'ENOENT'; + } + }); + return Promise.all( - paths - .filter(({ dirent }) => !dirent.isDirectory()) - .filter(({ dirent, path }) => { - if (!dirent.isSymbolicLink()) return true; - const safePath = resolveSafeChildPath(sourcePath, path); - if (!existsSync(safePath)) return true; // We only want symlinks that DO NOT exist (yet) - return false; - }) - .map(async ({ dirent, path, stats }) => ({ - path, - content: await limiter(async () => { - const absFilePath = resolveSafeChildPath(sourcePath, path); - if (dirent.isSymbolicLink()) { - return fs.readlink(absFilePath, 'buffer'); - } - return fs.readFile(absFilePath); - }), - executable: isExecutable(stats?.mode), - symlink: dirent.isSymbolicLink(), - })), + valid.map(async ({ dirent, path, stats }) => ({ + path, + content: await limiter(async () => { + const absFilePath = resolveSafeChildPath(sourcePath, path); + if (dirent.isSymbolicLink()) { + return fs.readlink(absFilePath, 'buffer'); + } + return fs.readFile(absFilePath); + }), + executable: isExecutable(stats?.mode), + symlink: dirent.isSymbolicLink(), + })), ); }