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. 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..a22ea85301 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: Buffer.from('./a.txt', 'utf8'), + }, + ]); + }); + 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..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 fs from 'fs-extra'; +import { promises as fs } from 'fs'; 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'; +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?: { @@ -44,19 +53,42 @@ 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, }); 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.map(async ({ path, stats }) => ({ + valid.map(async ({ dirent, path, stats }) => ({ path, - content: await limiter(async () => - fs.readFile(joinPath(sourcePath, 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(), })), ); } 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/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 165289de23..0bff5db596 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'); + }); }); }); 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 43b4eeb58d..e6649d1ffe 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..20b0ba83aa 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,21 +252,33 @@ 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, + ): 'utf-8' | 'base64' => (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.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: 'base64' as const, - content: file.content.toString('base64'), + encoding: determineFileEncoding(file), + content: file.content.toString(determineFileEncoding(file)), }, ]), );