From 4b1831ac551f4bf83645364281b84cdb23a3d8b9 Mon Sep 17 00:00:00 2001 From: Marcus Crane Date: Tue, 16 Aug 2022 20:59:06 +1200 Subject: [PATCH] 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') + ), }, ]), );