Merge pull request #7738 from kennethzfeng/kfeng.fix-file-mode

plugin/scaffolder: set the file mode in publish:github:pull-request.

.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

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"

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<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]: {
+          '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',
+              },
             },
           },
         ],

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,
         },
       ];

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"