Merge pull request #13169 from marcus-crane/scaffolder-patch
Add support for "broken" symlinks in the scaffolder backend
This commit is contained in:
@@ -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.
|
||||
@@ -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,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
@@ -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'),
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -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<T>(
|
||||
array: T[],
|
||||
callback: (value: T, index: number, array: T[]) => Promise<boolean>,
|
||||
): Promise<T[]> {
|
||||
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(),
|
||||
})),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -18,4 +18,5 @@ export interface SerializedFile {
|
||||
path: string;
|
||||
content: Buffer;
|
||||
executable?: boolean;
|
||||
symlink?: boolean;
|
||||
}
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+54
@@ -359,6 +359,60 @@ describe('createPublishGithubPullRequestAction', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('with broken symlink', () => {
|
||||
let input: GithubPullRequestActionInput;
|
||||
let ctx: ActionContext<GithubPullRequestActionInput>;
|
||||
|
||||
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<GithubPullRequestActionInput>;
|
||||
|
||||
+21
-6
@@ -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)),
|
||||
},
|
||||
]),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user