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

feat(action): add base-url #22

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat(action): add base-url #22

wants to merge 6 commits into from

Conversation

DariuszPorowski
Copy link
Collaborator

@DariuszPorowski DariuszPorowski commented Mar 9, 2024

💌 Description

  • Add option to set own GitHub REST API, default to GITHUB_API_URL
  • Enforce actionlint version / matcher check from public github
  • Inputs naming adjustments
  • Update docs

🔗 Related issue

Fixes: #14

🏗️ Type of change

  • 📚 Examples/docs/tutorials
  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 🥂 Improvement (non-breaking change which improves an existing feature)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🚨 Security fix
  • ⬆️ Dependencies update

✅ Checklist

@DariuszPorowski DariuszPorowski added the enhancement New feature or request label Mar 9, 2024
@DariuszPorowski DariuszPorowski self-assigned this Mar 9, 2024
@DariuszPorowski DariuszPorowski marked this pull request as ready for review March 9, 2024 18:19
@DariuszPorowski DariuszPorowski requested a review from a team as a code owner March 9, 2024 18:19
@DariuszPorowski DariuszPorowski added the documentation Improvements or additions to documentation label Mar 9, 2024
@bhundven-axon
Copy link

@DariuszPorowski I'll give this a shot in a little while and will update with results.

README.md Outdated
| `pyflakes` | false | `bool` | `true` | Use `pyflakes` with `actionlint` (and install if it does not exist) |
| `cache` | false | `bool` | `true` | Use GitHub cache for caching binaries for the next runs. |
| `github-token` | false | `string` | `github.token` | GitHub Token for API authentication. |
| `github-api-url` | false | `string` | `github.api_url` | GitHub REST API URL to connect to a different GitHub instance. For example, `https://my.github-enterprise-server.com/api/v3` |

Choose a reason for hiding this comment

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

This isn't actually needed in my option. github.api_url will point to the correct API URL of the GHES instance.

action.yml Outdated
@@ -86,7 +96,8 @@ runs:
uses: actions/github-script@v7
id: environment
with:
github-token: ${{ inputs.token || env.GITHUB_TOKEN }}
github-token: ${{ inputs.github-token || inputs.token || env.GITHUB_TOKEN }}

Choose a reason for hiding this comment

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

This will not work (and breaks as expected in my tests due to a 401 error), because the token here will be my GHES token by default, which is unknown to GitHub. If I override the github-token option with a real github.com token, then consequently the later steps which target my GHES API will fail due to always using the same configured token.

I think you would need to introduce an additional github-token-downloadurl (name just for illustration) which is used for the environment step. This can also default to ${{ github.api_url }}, thus remaining backwards compatible.

Additionally we should allow not using a token at all when accessing the API (unauthenticated request). These calls are heavily rate-limited, but our GHES workflow may not have access to any github.com token at all.

The list releases API is available without authentication:

This endpoint can be used without authentication or the aforementioned permissions if only public resources are requested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@frederikb great input! thanks for checking. Going to address your suggestions today, and appreciate GHES test after will update this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@frederikb the problem with anonymous GH public api calls is, it's a very limited rate limits: The primary rate limit for unauthenticated requests is 60 requests per hour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 [bug]: Unable to set URL of actionlint 🐛 [bug]: using v1 gives node16 deprecation warnings
3 participants