terraform-linters / tflint

A Pluggable Terraform Linter
Mozilla Public License 2.0
4.96k stars 357 forks source link

Github API rate limit exceeded when installing rulesets #1142

Closed Skaronator closed 3 years ago

Skaronator commented 3 years ago

Hi,

we've recently added tflint to our CI Pipeline and we love it. However we noticed that we run in to the API rate limit from Github. The default limit for unauthenticated request is 60 request per hour which is quite low and tflint does multiple request when you have multiple ruleset.

$ tflint --init
Installing `aws` plugin...
Failed to install a plugin. An error occurred:
Error: Failed to fetch GitHub releases: GET https://api.github.com/repos/terraform-linters/tflint-ruleset-aws/releases/tags/v0.4.3: 403 API rate limit exceeded for 54.x.x.x. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.) [rate reset in 27m36s]

When using an OAUTH token you can do 5000 requests per hour but there doesn't seem to be a setting in tflint to configure a Github API Token?

BUT regardless of that it doesn't seems to be a good architecture design when you have to create a Github Account and a API token just to run tflint in a CI pipeline successfully. So instead of adding a option for a API token I'd suggest looking into better ways to handle this.

One way of this could be to host the JSON file with the same content as Github Page.

https://docs.github.com/en/rest/overview/resources-in-the-rest-api

wata727 commented 3 years ago

There are several ways. One is, as you say, accepting OAuth2 access tokens. This is the solution I was thinking of when faced with the rate limit issue.

The other is to cache the installed rulesets. Major CI services such as GitHub Actions and CircleCI support caching a specific directory, which can be used to prevent from installing each time. For now, I think the hash value of the content in .tflint.hcl is a good cache key.

I'm wondering if hosting release metadata on GitHub Pages can be difficult to force on all third-party ruleset vendors...

If you have any problems that these do not solve well, please let us know. Thank you.

Skaronator commented 3 years ago

Caching wouldn't work since you still want to run tflint --init to update the cache and the command would probably still do Github API request even if the files exist.

tflint could provide a Github Actions hook for the Github Pages workaround. Would make setting up the CI at least pretty simple.

Using access tokens is probably the most straight forward solution but I would like to avoid having to set access tokens just to have tflint work properly. This would mean that I have to setup a new Github Account for my Company and configure credentials in CI etc.


Maybe another solution would be to just do a Git HTTP Clone using the Version field as tag? I think HTTP clones doesn't have the same resource limits as the REST API.

wata727 commented 3 years ago

TFLint will not call the GitHub API if the plugin is already installed. So, caching works fine. https://github.com/terraform-linters/tflint/blob/v0.29.1/cmd/init.go#L28-L29

I agree that we don't want to generate a PAT to only get around the rate limit, but I would like to adopt this workaround as it may be possible to use implicitly generated tokens such as GitHub Actions. Also, this feature is useful if you have already created a robot account for such a purpose. In fact, such functionality is common in projects that access GitHub resources via API (eg Composer in PHP). I think the way to avoid rate limits, such as with GitHub Pages, is a bit too complicated. Using the API is the most appropriate way to install the binaries on GitHub releases.

Unfortunately, Git HTTP Clone won't be the solution. The --init feature's constraint includes the release binary name, checksum file, and signature file, so you'll need to access GitHub release in some way.

Skaronator commented 3 years ago

Forgot to respond but thanks for implementing the token and thanks for documenting the caching part!