-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for GHE #618
Add support for GHE #618
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -68,6 +68,7 @@ Full list of the options | |||||
|
||||||
| NAME | DESCRIPTION | TYPE | DEFAULT | OPTIONS | | ||||||
| ----------------------------------- | -------------------------------------------------------------- | -------- | --------------------- | ---------------------------------------- | | ||||||
| `github-api-url` | The Github API endpoint. Override for Github Enterprise usage. | `string` | `true` | `https://api.github.com` | | ||||||
| `github-token` | The GITHUB_TOKEN secret. You can use PAT if you want. | `string` | `${{ github.token }}` | | | ||||||
| `wait-seconds-before-first-polling` | Wait this interval before first polling | `number` | `10` | | | ||||||
| `min-interval-seconds` | Wait this interval or the multiplied value (and jitter) | `number` | `15` | | | ||||||
|
@@ -90,6 +91,15 @@ permissions: | |||||
actions: read | ||||||
``` | ||||||
|
||||||
## Support for Github Enterprise | ||||||
|
||||||
To run this action in your Github Enterprise (GHE) instance you need to override `github-api-url`: | ||||||
|
||||||
```yaml | ||||||
with: | ||||||
github-api-url: 'https://ghe-host.acme.net/api/v3' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I don't know the detail for acme and the domain owner, don't we need to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I guess resolving conflict drops old review points... #618 (comment) Sorry for my lately response 🙇♂️ |
||||||
``` | ||||||
|
||||||
## outputs.<output_id> | ||||||
|
||||||
- `dump`\ | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -9,6 +9,10 @@ inputs: | |||||
description: 'The GITHUB_TOKEN secret' | ||||||
required: false | ||||||
default: ${{ github.token }} | ||||||
github-api-url: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think this will be better to clarify the difference of REST endpoints There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case - octokit graphql actually does "magic" to figure out graphql endpoint from rest endpoint here: If we pass graphql endpoint as So in this case we need to leave it as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/octokit/graphql.js/blob/9c0643d34f36ed558e55193438d7aa8b031ca43d/src/graphql.ts#L23 const GHES_V3_SUFFIX_REGEX = /\/api\/v3\/?$/;
// workaround for GitHub Enterprise baseUrl set with /api/v3 suffix
// https://github.com/octokit/auth-app.js/issues/111#issuecomment-657610451
const baseUrl = parsedOptions.baseUrl || request.endpoint.DEFAULTS.baseUrl;
if (GHES_V3_SUFFIX_REGEX.test(baseUrl)) {
requestOptions.url = baseUrl.replace(GHES_V3_SUFFIX_REGEX, "/api/graphql");
}
The workaround seems to focus only on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we have
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In our GHE instance There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for sharing! I'm reading how some other actions are handling this .... https://github.com/google-github-actions/release-please-action/pull/532 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NOTE to me: The patch introduced in octokit/graphql.js#186 is still active in latest https://github.com/octokit/graphql.js/blob/a801c2f72fc5e56d531b16d0f4c4946e687e32e6/src/graphql.ts#L67-L72 |
||||||
description: 'Github API URL' | ||||||
required: true | ||||||
default: 'https://api.github.com' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Can we use placeholder for getting default value? |
||||||
wait-seconds-before-first-polling: | ||||||
description: 'Wait this seconds before first polling' | ||||||
required: false | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -109,6 +109,7 @@ export type RetryMethod = z.infer<typeof retryMethods>; | |
// - Do not specify default values with zod. That is an action.yml role | ||
// - Do not include secrets here, for example githubToken. See https://github.com/colinhacks/zod/issues/1783 | ||
export const Options = z.object({ | ||
apiUrl: z.string().url(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't know zod provides url validations 👍 |
||
waitList: WaitList, | ||
skipList: SkipList, | ||
initialDuration: Duration, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least for REST endpoint, this option name is good 👍 ref: actions/create-github-app-token#88
? (sorry for unformatting)