Skip to content

Commit 25cc3bd

Browse files
parkerbxyzgr2m
parkerbxyz
and
gr2m
authored
refactor: remove redundant API call (#175)
Combines the two installation requests (org and user) into one because `/org/{org}` can also be accessed at `/users/{org}`. --------- Co-authored-by: Gregor Martynus <[email protected]>
1 parent a2c2dfa commit 25cc3bd

8 files changed

Lines changed: 270 additions & 189 deletions

‎dist/main.cjs‎

Lines changed: 180 additions & 67 deletions
Large diffs are not rendered by default.

‎dist/post.cjs‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -560,10 +560,10 @@ var require_proxy = __commonJS({
560560
})();
561561
if (proxyVar) {
562562
try {
563-
return new URL(proxyVar);
563+
return new DecodedURL(proxyVar);
564564
} catch (_a) {
565565
if (!proxyVar.startsWith("http://") && !proxyVar.startsWith("https://"))
566-
return new URL(`http://${proxyVar}`);
566+
return new DecodedURL(`http://${proxyVar}`);
567567
}
568568
} else {
569569
return void 0;
@@ -606,6 +606,19 @@ var require_proxy = __commonJS({
606606
const hostLower = host.toLowerCase();
607607
return hostLower === "localhost" || hostLower.startsWith("127.") || hostLower.startsWith("[::1]") || hostLower.startsWith("[0:0:0:0:0:0:0:1]");
608608
}
609+
var DecodedURL = class extends URL {
610+
constructor(url, base) {
611+
super(url, base);
612+
this._decodedUsername = decodeURIComponent(super.username);
613+
this._decodedPassword = decodeURIComponent(super.password);
614+
}
615+
get username() {
616+
return this._decodedUsername;
617+
}
618+
get password() {
619+
return this._decodedPassword;
620+
}
621+
};
609622
}
610623
});
611624

@@ -18140,7 +18153,7 @@ var require_lib = __commonJS({
1814018153
}
1814118154
const usingSsl = parsedUrl.protocol === "https:";
1814218155
proxyAgent = new undici_1.ProxyAgent(Object.assign({ uri: proxyUrl2.href, pipelining: !this._keepAlive ? 0 : 1 }, (proxyUrl2.username || proxyUrl2.password) && {
18143-
token: `${proxyUrl2.username}:${proxyUrl2.password}`
18156+
token: `Basic ${Buffer.from(`${proxyUrl2.username}:${proxyUrl2.password}`).toString("base64")}`
1814418157
}));
1814518158
this._proxyAgentDispatcher = proxyAgent;
1814618159
if (usingSsl && this._ignoreSslError) {
@@ -36791,11 +36804,11 @@ var RequestError = class extends Error {
3679136804
response;
3679236805
constructor(message, statusCode, options) {
3679336806
super(message);
36794-
if (Error.captureStackTrace) {
36795-
Error.captureStackTrace(this, this.constructor);
36796-
}
3679736807
this.name = "HttpError";
36798-
this.status = statusCode;
36808+
this.status = Number.parseInt(statusCode);
36809+
if (Number.isNaN(this.status)) {
36810+
this.status = 0;
36811+
}
3679936812
if ("response" in options) {
3680036813
this.response = options.response;
3680136814
}

‎lib/main.js‎

Lines changed: 49 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@ export async function main(
2626

2727
// If neither owner nor repositories are set, default to current repository
2828
if (!owner && repositories.length === 0) {
29-
const [owner, repo] = String(
30-
process.env.GITHUB_REPOSITORY
31-
).split("/");
29+
const [owner, repo] = String(process.env.GITHUB_REPOSITORY).split("/");
3230
parsedOwner = owner;
3331
parsedRepositoryNames = [repo];
3432

@@ -52,7 +50,9 @@ export async function main(
5250
parsedRepositoryNames = repositories;
5351

5452
core.info(
55-
`owner not set, creating owner for given repositories "${repositories.join(',')}" in current owner ("${parsedOwner}")`
53+
`owner not set, creating owner for given repositories "${repositories.join(
54+
","
55+
)}" in current owner ("${parsedOwner}")`
5656
);
5757
}
5858

@@ -62,7 +62,9 @@ export async function main(
6262
parsedRepositoryNames = repositories;
6363

6464
core.info(
65-
`owner and repositories set, creating token for repositories "${repositories.join(',')}" owned by "${owner}"`
65+
`owner and repositories set, creating token for repositories "${repositories.join(
66+
","
67+
)}" owned by "${owner}"`
6668
);
6769
}
6870

@@ -76,24 +78,38 @@ export async function main(
7678
// If at least one repository is set, get installation ID from that repository
7779

7880
if (parsedRepositoryNames.length > 0) {
79-
({ authentication, installationId, appSlug } = await pRetry(() => getTokenFromRepository(request, auth, parsedOwner, parsedRepositoryNames), {
80-
onFailedAttempt: (error) => {
81-
core.info(
82-
`Failed to create token for "${parsedRepositoryNames.join(',')}" (attempt ${error.attemptNumber}): ${error.message}`
83-
);
84-
},
85-
retries: 3,
86-
}));
81+
({ authentication, installationId, appSlug } = await pRetry(
82+
() =>
83+
getTokenFromRepository(
84+
request,
85+
auth,
86+
parsedOwner,
87+
parsedRepositoryNames
88+
),
89+
{
90+
onFailedAttempt: (error) => {
91+
core.info(
92+
`Failed to create token for "${parsedRepositoryNames.join(
93+
","
94+
)}" (attempt ${error.attemptNumber}): ${error.message}`
95+
);
96+
},
97+
retries: 3,
98+
}
99+
));
87100
} else {
88101
// Otherwise get the installation for the owner, which can either be an organization or a user account
89-
({ authentication, installationId, appSlug } = await pRetry(() => getTokenFromOwner(request, auth, parsedOwner), {
90-
onFailedAttempt: (error) => {
91-
core.info(
92-
`Failed to create token for "${parsedOwner}" (attempt ${error.attemptNumber}): ${error.message}`
93-
);
94-
},
95-
retries: 3,
96-
}));
102+
({ authentication, installationId, appSlug } = await pRetry(
103+
() => getTokenFromOwner(request, auth, parsedOwner),
104+
{
105+
onFailedAttempt: (error) => {
106+
core.info(
107+
`Failed to create token for "${parsedOwner}" (attempt ${error.attemptNumber}): ${error.message}`
108+
);
109+
},
110+
retries: 3,
111+
}
112+
));
97113
}
98114

99115
// Register the token with the runner as a secret to ensure it is masked in logs
@@ -111,23 +127,13 @@ export async function main(
111127
}
112128

113129
async function getTokenFromOwner(request, auth, parsedOwner) {
114-
// https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#get-an-organization-installation-for-the-authenticated-app
115-
const response = await request("GET /orgs/{org}/installation", {
116-
org: parsedOwner,
130+
// https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-user-installation-for-the-authenticated-app
131+
// This endpoint works for both users and organizations
132+
const response = await request("GET /users/{username}/installation", {
133+
username: parsedOwner,
117134
request: {
118135
hook: auth.hook,
119136
},
120-
}).catch((error) => {
121-
/* c8 ignore next */
122-
if (error.status !== 404) throw error;
123-
124-
// https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-user-installation-for-the-authenticated-app
125-
return request("GET /users/{username}/installation", {
126-
username: parsedOwner,
127-
request: {
128-
hook: auth.hook,
129-
},
130-
});
131137
});
132138

133139
// Get token for for all repositories of the given installation
@@ -137,12 +143,17 @@ async function getTokenFromOwner(request, auth, parsedOwner) {
137143
});
138144

139145
const installationId = response.data.id;
140-
const appSlug = response.data['app_slug'];
146+
const appSlug = response.data["app_slug"];
141147

142148
return { authentication, installationId, appSlug };
143149
}
144150

145-
async function getTokenFromRepository(request, auth, parsedOwner, parsedRepositoryNames) {
151+
async function getTokenFromRepository(
152+
request,
153+
auth,
154+
parsedOwner,
155+
parsedRepositoryNames
156+
) {
146157
// https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-repository-installation-for-the-authenticated-app
147158
const response = await request("GET /repos/{owner}/{repo}/installation", {
148159
owner: parsedOwner,
@@ -160,7 +171,7 @@ async function getTokenFromRepository(request, auth, parsedOwner, parsedReposito
160171
});
161172

162173
const installationId = response.data.id;
163-
const appSlug = response.data['app_slug'];
174+
const appSlug = response.data["app_slug"];
164175

165176
return { authentication, installationId, appSlug };
166177
}

tests/main-token-get-owner-set-to-user-repo-unset.test.js renamed to tests/main-token-get-owner-set-fail-response.test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { test } from "./main.js";
22

3-
// Verify `main` successfully obtains a token when the `owner` input is set (to a user), but the `repositories` input isn’t set.
3+
// Verify retries work when getting a token for a user or organization fails on the first attempt.
44
await test((mockPool) => {
55
process.env.INPUT_OWNER = "smockle";
66
delete process.env.INPUT_REPOSITORIES;
@@ -10,15 +10,15 @@ await test((mockPool) => {
1010
const mockAppSlug = "github-actions";
1111
mockPool
1212
.intercept({
13-
path: `/orgs/${process.env.INPUT_OWNER}/installation`,
13+
path: `/users/${process.env.INPUT_OWNER}/installation`,
1414
method: "GET",
1515
headers: {
1616
accept: "application/vnd.github.v3+json",
1717
"user-agent": "actions/create-github-app-token",
1818
// Intentionally omitting the `authorization` header, since JWT creation is not idempotent.
1919
},
2020
})
21-
.reply(404);
21+
.reply(500, "GitHub API not available");
2222
mockPool
2323
.intercept({
2424
path: `/users/${process.env.INPUT_OWNER}/installation`,
@@ -31,7 +31,7 @@ await test((mockPool) => {
3131
})
3232
.reply(
3333
200,
34-
{ id: mockInstallationId, "app_slug": mockAppSlug },
34+
{ id: mockInstallationId, app_slug: mockAppSlug },
3535
{ headers: { "content-type": "application/json" } }
3636
);
3737
});

tests/main-token-get-owner-set-to-org-repo-unset.test.js renamed to tests/main-token-get-owner-set-repo-unset.test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
import { test } from "./main.js";
22

3-
// Verify `main` successfully obtains a token when the `owner` input is set (to an org), but the `repositories` input isn’t set.
3+
// Verify `main` successfully obtains a token when the `owner` input is set, and the `repositories` input isn’t set.
44
await test((mockPool) => {
55
process.env.INPUT_OWNER = process.env.GITHUB_REPOSITORY_OWNER;
66
delete process.env.INPUT_REPOSITORIES;
77

8-
// Mock installation id and app slug request
8+
// Mock installation ID and app slug request
99
const mockInstallationId = "123456";
1010
const mockAppSlug = "github-actions";
1111
mockPool
1212
.intercept({
13-
path: `/orgs/${process.env.INPUT_OWNER}/installation`,
13+
path: `/users/${process.env.INPUT_OWNER}/installation`,
1414
method: "GET",
1515
headers: {
1616
accept: "application/vnd.github.v3+json",
@@ -20,7 +20,7 @@ await test((mockPool) => {
2020
})
2121
.reply(
2222
200,
23-
{ id: mockInstallationId, "app_slug": mockAppSlug },
23+
{ id: mockInstallationId, app_slug: mockAppSlug },
2424
{ headers: { "content-type": "application/json" } }
2525
);
2626
});

‎tests/main-token-get-owner-set-to-user-fail-response.test.js‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

0 commit comments

Comments
 (0)