Skip to content
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

Pass the token input through on GHES #427

Merged
merged 7 commits into from
Jun 13, 2022
Merged

Pass the token input through on GHES #427

merged 7 commits into from
Jun 13, 2022

Conversation

brcrista
Copy link
Contributor

@brcrista brcrista commented Jun 10, 2022

Closes #229

Description

  • Customers hit this issue because they have a fleet of self-hosted runners behind a handful of NAT gateways. So, all of those runners look like the same small set of IPs to GitHub.

  • We currently null out the PAT when we're on GHES. This was done just to avoid confusion and not for any security reason.

  • Passing a github.com PAT to the token input is an inexpensive fix that gets the job done without a lot of complexity. While GitHub Connect is the ideal pattern here, using token doesn't seem that bad in this case. The description for the token input says:

    Used to pull python distributions from actions/python-versions

    Users already seem to expect it to be a github.com PAT, and being dropped silently is already unintuitive.

Given that we're not going to fund the GitHub Connect work, I think just removing the code to null out the PAT on GHES and updating the description for token to make the behavior clear is the best fix.

Testing

  1. Pushed my version of the action to a GHES instance and configured a self-hosted runner
  2. Created a PAT for github.com (no scopes) and added it as a secret
  3. Ran the YAML snippet from the issue
  4. Confirmed that there was no change to the unauthenticated rate limit

Check list

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@brcrista brcrista requested a review from a team June 10, 2022 23:17
@brcrista brcrista changed the title Pass thetoken input through on GHES Pass the token input through on GHES Jun 11, 2022
@brcrista
Copy link
Contributor Author

The failing checks are for pipenv caching on Windows. Doesn't appear to be related, and appears in runs for other branches.

@brcrista brcrista merged commit 7e4abae into main Jun 13, 2022
@brcrista brcrista deleted the brcrista/ghes-token branch June 13, 2022 18:55
brcrista added a commit that referenced this pull request Jun 15, 2022
brcrista added a commit that referenced this pull request Jun 16, 2022
brcrista added a commit to ChristopherHX/setup-python that referenced this pull request Aug 5, 2022
brcrista added a commit that referenced this pull request Aug 30, 2022
* Only use github.token on github.com

This expression evaluates to `''` if called from GHES hosted elsewhere
You can still provide your token on both github.com and GHES

* Enshure blank result of expression and not false

* Revert "Revert "Pass the `token` input through on GHES (#427)" (#437)"

This reverts commit cf86e08.

* fix typo

* Add back the doc on the tool cache for self-hosted

Co-authored-by: Brian Cristante <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not using "token" on github server
4 participants