diff --git a/.changeset/two-cougars-breathe.md b/.changeset/two-cougars-breathe.md new file mode 100644 index 0000000000..0b61078e85 --- /dev/null +++ b/.changeset/two-cougars-breathe.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Fixed bug where the mode of an executable file was ignored diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index e6812c6501..c6e7eaf409 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -65,7 +65,7 @@ "luxon": "^2.0.2", "morgan": "^1.10.0", "nunjucks": "^3.2.3", - "octokit-plugin-create-pull-request": "^3.9.3", + "octokit-plugin-create-pull-request": "^3.10.0", "uuid": "^8.2.0", "winston": "^3.2.1", "yaml": "^1.10.0" 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 ed72b14a65..27c6f1e59d 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 @@ -98,7 +98,11 @@ describe('createPublishGithubPullRequestAction', () => { { commit: 'Create my new app', files: { - 'file.txt': 'Hello there!', + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, }, }, ], @@ -167,7 +171,11 @@ describe('createPublishGithubPullRequestAction', () => { { commit: 'Create my new app', files: { - 'foo.txt': 'Hello there!', + 'foo.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, }, }, ], @@ -221,7 +229,79 @@ describe('createPublishGithubPullRequestAction', () => { { commit: 'Create my new app', files: { - 'file.txt': 'Hello there!', + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + }, + ], + }); + }); + + it('creates outputs for the url', async () => { + await instance.handler(ctx); + + expect(ctx.output).toHaveBeenCalledWith( + 'remoteUrl', + 'https://github.com/myorg/myrepo/pull/123', + ); + }); + afterEach(() => { + mockFs.restore(); + jest.resetAllMocks(); + }); + }); + + describe('with executable file', () => { + 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]: { + 'file.txt': mockFs.file({ + content: 'Hello there!', + mode: 33277, // File mode: 100755 + }), + }, + }); + + 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: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100755', + }, }, }, ], 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 253fd72b38..e147f49b23 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { readFile } from 'fs-extra'; +import fs from 'fs-extra'; import path from 'path'; import { parseRepoUrl } from './util'; @@ -30,6 +30,8 @@ import { createPullRequest } from 'octokit-plugin-create-pull-request'; import globby from 'globby'; import { resolveSafeChildPath } from '@backstage/backend-common'; +export type Encoding = 'utf-8' | 'base64'; + class GithubResponseError extends CustomErrorBase {} type CreatePullRequestResponse = { @@ -194,7 +196,29 @@ export const createPublishGithubPullRequestAction = ({ }); const fileContents = await Promise.all( - localFilePaths.map(p => readFile(path.resolve(fileRoot, p))), + localFilePaths.map(filePath => { + const absPath = path.resolve(fileRoot, filePath); + const base64EncodedContent = fs + .readFileSync(absPath) + .toString('base64'); + const fileStat = fs.statSync(absPath); + const isExecutable = fileStat.mode === 33277; // aka. 100755 + // See the properties of tree items + // in https://docs.github.com/en/rest/reference/git#trees + const githubTreeItemMode = isExecutable ? '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) + const encoding: Encoding = 'base64'; + return { + encoding: encoding, + content: base64EncodedContent, + mode: githubTreeItemMode, + }; + }), ); const repoFilePaths = localFilePaths.map(repoFilePath => { @@ -203,10 +227,7 @@ export const createPublishGithubPullRequestAction = ({ const changes = [ { - files: zipObject( - repoFilePaths, - fileContents.map(buf => buf.toString()), - ), + files: zipObject(repoFilePaths, fileContents), commit: title, }, ]; diff --git a/yarn.lock b/yarn.lock index 304807b13e..f7cdd98a03 100644 --- a/yarn.lock +++ b/yarn.lock @@ -21141,10 +21141,10 @@ obuf@^1.0.0, obuf@^1.1.2: resolved "https://registry.npmjs.org/obuf/-/obuf-1.1.2.tgz#09bea3343d41859ebd446292d11c9d4db619084e" integrity sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg== -octokit-plugin-create-pull-request@^3.9.3: - version "3.9.3" - resolved "https://registry.npmjs.org/octokit-plugin-create-pull-request/-/octokit-plugin-create-pull-request-3.9.3.tgz#f99f53907ac322a3494cc970514a023d7b659e2b" - integrity sha512-lTyNnCRoT4IvCQx2Cb4eFMqg8aIpsaDd59MNwf4OPnWAJM7hT6g7RW/icImvAzZLR4t5ENSLNzWarv2XqLL+Lg== +octokit-plugin-create-pull-request@^3.10.0: + version "3.10.0" + resolved "https://registry.npmjs.org/octokit-plugin-create-pull-request/-/octokit-plugin-create-pull-request-3.10.0.tgz#c9a589e0014e949eadd24a03ace10565007784d5" + integrity sha512-QU3nk62+OimV7ki+pV90cXoqqbUAQLdbqccS7/cNajdjQ2KYmaakz21FL1y78a5N0mA2P4WOs0o2+aunvbWI0w== dependencies: "@octokit/types" "^6.8.2"