From 796b88dc585016182c9da7fa3e5dfe98e0a9b19f Mon Sep 17 00:00:00 2001 From: Peter Evans <18365890+peter-evans@users.noreply.github.com> Date: Wed, 11 Sep 2024 21:54:50 +0100 Subject: [PATCH] feat: allow repositories input to be comma or newline-separated (#169) Resolves https://github.com/actions/create-github-app-token/issues/106 - Fixes the parsing to cope with whitespace in the input string. - Allows the input to be comma or newline-separated. (I've done this for all array-type inputs in my own actions, but I'm happy to remove this if you only want to support comma-separated.) - Added tests for parsing comma and newline-separated inputs. --- README.md | 6 ++-- action.yml | 2 +- dist/main.cjs | 30 ++++++++-------- lib/main.js | 32 ++++++++++-------- main.js | 5 ++- package-lock.json | 4 +-- ...owner-set-repo-set-to-many-newline.test.js | 9 +++++ ...ken-get-owner-set-repo-set-to-many.test.js | 3 +- tests/snapshots/index.js.md | 21 +++++++++++- tests/snapshots/index.js.snap | Bin 1326 -> 1329 bytes 10 files changed, 75 insertions(+), 37 deletions(-) create mode 100644 tests/main-token-get-owner-set-repo-set-to-many-newline.test.js diff --git a/README.md b/README.md index 9637ad3..f129b79 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,9 @@ jobs: app-id: ${{ vars.APP_ID }} private-key: ${{ secrets.PRIVATE_KEY }} owner: ${{ github.repository_owner }} - repositories: "repo1,repo2" + repositories: | + repo1 + repo2 - uses: peter-evans/create-or-update-comment@v3 with: token: ${{ steps.app-token.outputs.token }} @@ -302,7 +304,7 @@ steps: ### `repositories` -**Optional:** Comma-separated list of repositories to grant access to. +**Optional:** Comma or newline-separated list of repositories to grant access to. > [!NOTE] > If `owner` is set and `repositories` is empty, access will be scoped to all repositories in the provided repository owner's installation. If `owner` and `repositories` are empty, access will be scoped to only the current repository. diff --git a/action.yml b/action.yml index 09cc8fa..184c59f 100644 --- a/action.yml +++ b/action.yml @@ -23,7 +23,7 @@ inputs: description: "The owner of the GitHub App installation (defaults to current repository owner)" required: false repositories: - description: "Repositories to install the GitHub App on (defaults to current repository if owner is unset)" + description: "Comma or newline-separated list of repositories to install the GitHub App on (defaults to current repository if owner is unset)" required: false skip-token-revoke: description: "If truthy, the token will not be revoked when the current job is complete" diff --git a/dist/main.cjs b/dist/main.cjs index 515e8be..eb2283b 100644 --- a/dist/main.cjs +++ b/dist/main.cjs @@ -39700,33 +39700,35 @@ async function pRetry(input, options) { // lib/main.js async function main(appId2, privateKey2, owner2, repositories2, core3, createAppAuth2, request2, skipTokenRevoke2) { let parsedOwner = ""; - let parsedRepositoryNames = ""; - if (!owner2 && !repositories2) { - [parsedOwner, parsedRepositoryNames] = String( + let parsedRepositoryNames = []; + if (!owner2 && repositories2.length === 0) { + const [owner3, repo] = String( process.env.GITHUB_REPOSITORY ).split("/"); + parsedOwner = owner3; + parsedRepositoryNames = [repo]; core3.info( - `owner and repositories not set, creating token for the current repository ("${parsedRepositoryNames}")` + `owner and repositories not set, creating token for the current repository ("${repo}")` ); } - if (owner2 && !repositories2) { + if (owner2 && repositories2.length === 0) { parsedOwner = owner2; core3.info( `repositories not set, creating token for all repositories for given owner "${owner2}"` ); } - if (!owner2 && repositories2) { + if (!owner2 && repositories2.length > 0) { parsedOwner = String(process.env.GITHUB_REPOSITORY_OWNER); parsedRepositoryNames = repositories2; core3.info( - `owner not set, creating owner for given repositories "${repositories2}" in current owner ("${parsedOwner}")` + `owner not set, creating owner for given repositories "${repositories2.join(",")}" in current owner ("${parsedOwner}")` ); } - if (owner2 && repositories2) { + if (owner2 && repositories2.length > 0) { parsedOwner = owner2; parsedRepositoryNames = repositories2; core3.info( - `owner and repositories set, creating token for repositories "${repositories2}" owned by "${owner2}"` + `owner and repositories set, creating token for repositories "${repositories2.join(",")}" owned by "${owner2}"` ); } const auth5 = createAppAuth2({ @@ -39735,11 +39737,11 @@ async function main(appId2, privateKey2, owner2, repositories2, core3, createApp request: request2 }); let authentication, installationId, appSlug; - if (parsedRepositoryNames) { + if (parsedRepositoryNames.length > 0) { ({ authentication, installationId, appSlug } = await pRetry(() => getTokenFromRepository(request2, auth5, parsedOwner, parsedRepositoryNames), { onFailedAttempt: (error) => { core3.info( - `Failed to create token for "${parsedRepositoryNames}" (attempt ${error.attemptNumber}): ${error.message}` + `Failed to create token for "${parsedRepositoryNames.join(",")}" (attempt ${error.attemptNumber}): ${error.message}` ); }, retries: 3 @@ -39789,7 +39791,7 @@ async function getTokenFromOwner(request2, auth5, parsedOwner) { async function getTokenFromRepository(request2, auth5, parsedOwner, parsedRepositoryNames) { const response = await request2("GET /repos/{owner}/{repo}/installation", { owner: parsedOwner, - repo: parsedRepositoryNames.split(",")[0], + repo: parsedRepositoryNames[0], request: { hook: auth5.hook } @@ -39797,7 +39799,7 @@ async function getTokenFromRepository(request2, auth5, parsedOwner, parsedReposi const authentication = await auth5({ type: "installation", installationId: response.data.id, - repositoryNames: parsedRepositoryNames.split(",") + repositoryNames: parsedRepositoryNames }); const installationId = response.data.id; const appSlug = response.data["app_slug"]; @@ -39847,7 +39849,7 @@ if (!privateKey) { throw new Error("Input required and not supplied: private-key"); } var owner = import_core2.default.getInput("owner"); -var repositories = import_core2.default.getInput("repositories"); +var repositories = import_core2.default.getInput("repositories").split(/[\n,]+/).map((s) => s.trim()).filter((x) => x !== ""); var skipTokenRevoke = Boolean( import_core2.default.getInput("skip-token-revoke") || import_core2.default.getInput("skip_token_revoke") ); diff --git a/lib/main.js b/lib/main.js index 97443c0..bc15f95 100644 --- a/lib/main.js +++ b/lib/main.js @@ -5,7 +5,7 @@ import pRetry from "p-retry"; * @param {string} appId * @param {string} privateKey * @param {string} owner - * @param {string} repositories + * @param {string[]} repositories * @param {import("@actions/core")} core * @param {import("@octokit/auth-app").createAppAuth} createAppAuth * @param {import("@octokit/request").request} request @@ -22,21 +22,23 @@ export async function main( skipTokenRevoke ) { let parsedOwner = ""; - let parsedRepositoryNames = ""; + let parsedRepositoryNames = []; // If neither owner nor repositories are set, default to current repository - if (!owner && !repositories) { - [parsedOwner, parsedRepositoryNames] = String( + if (!owner && repositories.length === 0) { + const [owner, repo] = String( process.env.GITHUB_REPOSITORY ).split("/"); + parsedOwner = owner; + parsedRepositoryNames = [repo]; core.info( - `owner and repositories not set, creating token for the current repository ("${parsedRepositoryNames}")` + `owner and repositories not set, creating token for the current repository ("${repo}")` ); } // If only an owner is set, default to all repositories from that owner - if (owner && !repositories) { + if (owner && repositories.length === 0) { parsedOwner = owner; core.info( @@ -45,22 +47,22 @@ export async function main( } // If repositories are set, but no owner, default to `GITHUB_REPOSITORY_OWNER` - if (!owner && repositories) { + if (!owner && repositories.length > 0) { parsedOwner = String(process.env.GITHUB_REPOSITORY_OWNER); parsedRepositoryNames = repositories; core.info( - `owner not set, creating owner for given repositories "${repositories}" in current owner ("${parsedOwner}")` + `owner not set, creating owner for given repositories "${repositories.join(',')}" in current owner ("${parsedOwner}")` ); } // If both owner and repositories are set, use those values - if (owner && repositories) { + if (owner && repositories.length > 0) { parsedOwner = owner; parsedRepositoryNames = repositories; core.info( - `owner and repositories set, creating token for repositories "${repositories}" owned by "${owner}"` + `owner and repositories set, creating token for repositories "${repositories.join(',')}" owned by "${owner}"` ); } @@ -73,11 +75,11 @@ export async function main( let authentication, installationId, appSlug; // If at least one repository is set, get installation ID from that repository - if (parsedRepositoryNames) { + if (parsedRepositoryNames.length > 0) { ({ authentication, installationId, appSlug } = await pRetry(() => getTokenFromRepository(request, auth, parsedOwner, parsedRepositoryNames), { onFailedAttempt: (error) => { core.info( - `Failed to create token for "${parsedRepositoryNames}" (attempt ${error.attemptNumber}): ${error.message}` + `Failed to create token for "${parsedRepositoryNames.join(',')}" (attempt ${error.attemptNumber}): ${error.message}` ); }, retries: 3, @@ -144,7 +146,7 @@ async function getTokenFromRepository(request, auth, parsedOwner, parsedReposito // https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-repository-installation-for-the-authenticated-app const response = await request("GET /repos/{owner}/{repo}/installation", { owner: parsedOwner, - repo: parsedRepositoryNames.split(",")[0], + repo: parsedRepositoryNames[0], request: { hook: auth.hook, }, @@ -154,11 +156,11 @@ async function getTokenFromRepository(request, auth, parsedOwner, parsedReposito const authentication = await auth({ type: "installation", installationId: response.data.id, - repositoryNames: parsedRepositoryNames.split(","), + repositoryNames: parsedRepositoryNames, }); const installationId = response.data.id; const appSlug = response.data['app_slug']; return { authentication, installationId, appSlug }; - } \ No newline at end of file +} diff --git a/main.js b/main.js index b4d919d..8011b51 100644 --- a/main.js +++ b/main.js @@ -25,7 +25,10 @@ if (!privateKey) { throw new Error("Input required and not supplied: private-key"); } const owner = core.getInput("owner"); -const repositories = core.getInput("repositories"); +const repositories = core.getInput("repositories") + .split(/[\n,]+/) + .map(s => s.trim()) + .filter(x => x !== ''); const skipTokenRevoke = Boolean( core.getInput("skip-token-revoke") || core.getInput("skip_token_revoke") diff --git a/package-lock.json b/package-lock.json index fef3933..332bbb1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "create-github-app-token", - "version": "1.10.3", + "version": "1.10.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "create-github-app-token", - "version": "1.10.3", + "version": "1.10.4", "license": "MIT", "dependencies": { "@actions/core": "^1.10.1", diff --git a/tests/main-token-get-owner-set-repo-set-to-many-newline.test.js b/tests/main-token-get-owner-set-repo-set-to-many-newline.test.js new file mode 100644 index 0000000..ad03c67 --- /dev/null +++ b/tests/main-token-get-owner-set-repo-set-to-many-newline.test.js @@ -0,0 +1,9 @@ +import { test } from "./main.js"; + +// Verify `main` successfully obtains a token when the `owner` and `repositories` inputs are set (and the latter is a list of repos). +await test(() => { + process.env.INPUT_OWNER = process.env.GITHUB_REPOSITORY_OWNER; + const currentRepoName = process.env.GITHUB_REPOSITORY.split("/")[1]; + // Intentional unnecessary whitespace to test parsing to array + process.env.INPUT_REPOSITORIES = `\n ${currentRepoName}\ntoolkit \n\n checkout \n`; +}); diff --git a/tests/main-token-get-owner-set-repo-set-to-many.test.js b/tests/main-token-get-owner-set-repo-set-to-many.test.js index 173fcf4..074e53b 100644 --- a/tests/main-token-get-owner-set-repo-set-to-many.test.js +++ b/tests/main-token-get-owner-set-repo-set-to-many.test.js @@ -4,5 +4,6 @@ import { test } from "./main.js"; await test(() => { process.env.INPUT_OWNER = process.env.GITHUB_REPOSITORY_OWNER; const currentRepoName = process.env.GITHUB_REPOSITORY.split("/")[1]; - process.env.INPUT_REPOSITORIES = `${currentRepoName},toolkit`; + // Intentional unnecessary whitespace to test parsing to array + process.env.INPUT_REPOSITORIES = ` ${currentRepoName}, toolkit ,checkout`; }); diff --git a/tests/snapshots/index.js.md b/tests/snapshots/index.js.md index 023d104..76d4b78 100644 --- a/tests/snapshots/index.js.md +++ b/tests/snapshots/index.js.md @@ -134,6 +134,25 @@ Generated by [AVA](https://avajs.dev). ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ::save-state name=expiresAt::2016-07-11T22:14:10Z` +## main-token-get-owner-set-repo-set-to-many-newline.test.js + +> stderr + + '' + +> stdout + + `owner and repositories set, creating token for repositories "create-github-app-token,toolkit,checkout" owned by "actions"␊ + ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=installation-id::123456␊ + ␊ + ::set-output name=app-slug::github-actions␊ + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ::save-state name=expiresAt::2016-07-11T22:14:10Z` + ## main-token-get-owner-set-repo-set-to-many.test.js > stderr @@ -142,7 +161,7 @@ Generated by [AVA](https://avajs.dev). > stdout - `owner and repositories set, creating token for repositories "create-github-app-token,toolkit" owned by "actions"␊ + `owner and repositories set, creating token for repositories "create-github-app-token,toolkit,checkout" owned by "actions"␊ ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ diff --git a/tests/snapshots/index.js.snap b/tests/snapshots/index.js.snap index 89369aac6f4c1bc3492e578620e484db70cc9830..9956084d13d02d50f1d1fa56d82b95b338b825d9 100644 GIT binary patch literal 1329 zcmV-11hO`7L+Y z(nX!gZy$>Y00000000B+na_(OMHI&;iU8@(4UMHKI_&3O2#RWkS_9}?}1^yL!@F-q93R>0q)l4!oOiz|&Nlx9J z>i53&-ltwwSAExPgv#%jhmVj1h4=utPeMr+V64&SgaH#WPRNu94VAty!bl#Qwj%TO zqafhKKYV`a#U=YIys-4j5{A5iVbf56TBi-}7x-k8=Z*?Ij_V?TKfd|~is^L`gn_Y7 z;4*-o2t#NfXpB@B0}v88G!sfRLKn3WN|xZBL1%+G8`2Q+ptKxo(dC5o4)#+o6A$va z8W>`!p`kz15pkVV2W4GjS9LJ;dg^gL-_{&+=Ge&GK*2cdDxqO6S0XMX^AkfVVl6@K zq``PBSfax`+59<^%^wS@CsDhuXpEH+q_jW-25B99jhuvRfs+kQrL&DzTmu}d8+335 ztknP-eOO6}G%L%V$Af@H+;lvz)i(9Y)(6|TS-oA|t@>NLmFoWXcBSm^Z0u}oZtr1b zd*{|BUtab+gTz!qaY7(@gzq{J`Apcj^DyLYtcbw#D%Fk68(Y(w+CG^uX?fnTlWBi! zQ{F{ph!b8;*l7#+AQl?U9(i81T-jpfZC0rqSF2uS)2oy}or=jdhe;%i5wgY7u{4v9 zlN*!SFBEIGIDp146QKt{OGRLkI1U8{9;EiBfHaG=31zQklsV333p>aN+dZfqAKtrF zKia>0=i}P(oum8pJD+^Ce*^<->kuV|?6@;XKnjXCo&EQUmy-$^sS{OVwqP$Kc>UZ+ zP8hwM#y2w>N8|7eiH8}9-l>?L8_nxCt((gRwd4d(^qchrwHX?p#~3h)y--NZH8uT| zDP}*+DrPPdGYT5D5&T4Jlr(;+534h4+uEX%&83oc9WFKn%yh6f8|!~EN&j=HSTiQE zV~&d8hg?LkS`?g$Ya^_=gqkF5p_{)n8k8voZ3Ge#0<}50J8leW8y4ts1Hu_DEr0gD z@=0|BUFQk4EeoH+fuO^r0eg3AsR;|6=^MgmaIX;p(QKk4tI>yzG8pug3`_}*kaXWF zumuARfgKr6!0;6xf&0Enq+Mtf0C8*VJvtT2rxR?=XeTKR8=mA~f8%5C?*olDX(I$u`knu~x~ODN4yVe6 zaav%sz-T5$Ew%s7Q1bg#Qeq-Nyh(wZUoOo;35D3ws3e! literal 1326 zcmV+}1=0FJRzVQng$u(pyYZ}#h2Z|m_Fk8 zBIfw&v>%HI00000000B+nL%&kL=?vx5JIYiwA>JjU{tM?7V2!Ac9Ygx5M9)5bAYZ& zS9DpSXkx#lL+r8fjFasR@fm88P1{T_7<8;SkR;PAKUmGKqlgJM(zvHHY{y}3klS#Ek-dLqMgnF zj#DqW6XjfGhOE@ki9a!5QAdUY;GGzT_&@b}>U25Z%ABRmSz~jVf=SrJM4sewDWHs7 zZX#qDSbZjslCDJ~W+ky-CY!$++5D*zjyS}6DltT%U^0vu2qHkP5;q1a8SfJyBOjbSfs-H6{TQ z0uyDWwN%K*;jMi3E7i7Y4w%y~67-t_%|l5<5=A}(&mpP3DIgV*mQ(h+p-gjDENsUR zwz<8y_{5HNX>PL zV!@6fc=g;!=8RrW<6DNtaUGr^ao3ROoQmnW(Y$$Exw&jmYg%w_-qr|9$O7Tg2p1*} zBcE|7wKV-=G_xOznwic-K|_;AK-?sbfy>E7$7H#%x2?=O*-SdwP~)OfV2J^Yin0FJ zNcvw3)wU3csySIDu1{G&ma7cEtc{NPb3knA}aeWb0l#HKCvr$GDyc*>~Ikc&yh0O%|?% zYjtGM4Y?<3WFKUFtQ4q*f%7D)GPHn`OI)6*|Kt1|E|`Ow2kHrnb$5; zi6s+BVy`&)no=#B4sj&y`Y=atHt5-Bqo)pJh+*f&n~V#9_qze!&ojB>Se0SuV}qVI zuBS&&{X+DFyj0hk->yDi&@>96QLM!2jg6JyS|`h3^MirSx0MUu4q%*3Xw=C*8{*F? zdUHZ%Oi1%ALK57bU)`4IAA^!Vu96ZFgzmtHvv)!#gXX)SXLAF70^Bp-lg6~o5t#`h z?@UER6EWWGltJS+1C1xyF6T58>G?_eFSxy7eEf(jwXi&1#pX}5W`vA&9x|o-$^7Bx zbA(G3gDgphp^wEq%roYe85>svI6pLRp zov*5Cs9H{?WM)^1dRZDxSe;0m`s6q#tW-@MUN=O&GKtRsymMosHJ6IX+lKo$PNhdZ klg_+MRr6X-P099h_@<%dTB?PO+lk@-f9lZ=`G_C@0MRCd*Z=?k