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

docs: Advise setting GITHUB_TOKEN to avoid rate limits #818

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbrunton
Copy link
Contributor

@jbrunton jbrunton commented Jan 2, 2023

I realised this is likely the easiest fix for #815. (I imagine also related to #810 and #812.)

I say likely because it's a little bit difficult proving a fix for something which only occasionally failed, but in my testing this action has been successful in about a dozen recent runs after setting the GITHUB_TOKEN variable, whereas before I was seeing failures every 3-4 runs.

EDIT: In fact, I've tested this more extensively since opening this PR and it's worked every time across dozens of runs.

I'm fairly confident this is the correct fix, however:

  1. The failures were related to downloading plugins, which are hosted on GitHub.
    • Although the error message would reference get.pulumi.com (e.g. https://get.pulumi.com/releases/plugins/pulumi-resource-synced-folder-v0.0.9-linux-amd64.tar.gz), it seems that the Pulumi retry mechanism for downloading plugins isn't robust and sometimes falls back to the wrong server (see discussion in Pulumi plugin 403 error pulumi#11743).
  2. GitHub returns a 403 when rate limiting.
  3. We see 403s frequently in CI while using this action. This makes sense: GitHub's rate limits would be shared across all users.
  4. Pulumi does use the GITHUB_TOKEN when it's set (link). But by default, this action doesn't encourage this (or set it automatically).

EDIT: It might also be possible to find a way for the action to automatically set the GITHUB_TOKEN variable before executing Pulumi, since the github.token context should always be available to the action. I've not looked into how straightforward this is, however. And it's probably good to be transparent about how access tokens get used.

@jbrunton jbrunton changed the title docs: set GITHUB_TOKEN to avoid rate limits docs: Advise setting GITHUB_TOKEN to avoid rate limits Jan 2, 2023
README.md Outdated
@@ -115,6 +116,10 @@ By default, this action will try to authenticate Pulumi with the
`PULUMI_ACCESS_TOKEN` then you will need to specify an alternative backend via
the `cloud-url` argument.

If you are using any Pulumi plugins then you should also set the `GITHUB_TOKEN`
environment variable in order to avoid hitting GitHub rate limits. You can set
it to the default `${{ secrets.GITHUB_TOKEN }}`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess users need to set a personal GitHub-token?

Copy link
Contributor Author

@jbrunton jbrunton Jan 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If by "set", you mean set a repo secret? If so, then no: the GITHUB_TOKEN secret is always added by GitHub automatically: https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret

However, they would have the option of using another token if they wish.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can clarify that in the text, however. And a link to the above wouldn't hurt.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cobraz: I also just realised it's possible the action could automatically set the GITHUB_TOKEN variable in the process before invoking pulumi, if users don't set it themselves. Actions should always be able to access the github.token context. I've not looked into this, however.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbrunton
Will this approach work for GitHub Enterprise?

Copy link
Contributor Author

@jbrunton jbrunton Jan 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Moon1706 : I don't use GitHub Enterprise so I can't verify it myself, but I don't see any reason why not: secrets.GITHUB_TOKEN should always be set, even for Enterprise repos. (This is explicitly mentioned in the docs.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add this link about GITHUB_TOKEN and rate limits as it mentions GH Enterprise https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#requests-from-github-actions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@1oglop1: Thanks for the suggestion. I updated the PR with that link.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already set the Github-token, it's used for pull request comments. We can easily reuse that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cobraz: Something like this instead?

I'd be happy to contribute a PR, but would appreciate a design review first. I couldn't see a better way to pass the token to Pulumi but the above change works.

@jbrunton
Copy link
Contributor Author

jbrunton commented Jan 7, 2023

Btw, I realise the Pulumi docs for this action would also need updating: https://github.com/pulumi/pulumi-hugo/blob/master/themes/default/content/docs/guides/continuous-delivery/github-actions.md

I would be happy to contribute a PR for that too, but I will wait for feedback on this PR first.

@Moon1706
Copy link
Contributor

@jbrunton
@cobraz
Unfortunately, adding GITHUB_TOKEN env didn't help for GitHub Enterprise server.

@Moon1706
Copy link
Contributor

@cobraz
Could you add more stable functionality for Enterprise solutions? I mean this or this PR?

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.

4 participants