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

Research learnings and ideas from similar GitHub Actions #42

Open
gr2m opened this issue Sep 4, 2023 · 7 comments
Open

Research learnings and ideas from similar GitHub Actions #42

gr2m opened this issue Sep 4, 2023 · 7 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@gr2m
Copy link
Contributor

gr2m commented Sep 4, 2023

Follow up to #39 (comment).

In preparation for #3 and #4 it might make sense to learn from similar actions.

@gr2m gr2m added good first issue Good for newcomers help wanted Extra attention is needed labels Sep 4, 2023
@gr2m
Copy link
Contributor Author

gr2m commented Sep 4, 2023

I looked through them again and two things that stood out for me

There is also a lot to learn in terms of documentation, in particular how to register a GitHub App.

@parkerbxyz parkerbxyz changed the title Research learnings and ideas from similar ideas Research learnings and ideas from similar GitHub Actions Sep 11, 2023
@Moser-ss
Copy link
Contributor

I was trying to migrate from the action peter-murray/workflow-application-token-action to this one, and I noticed this action does not support the private-key input as a Base64 encoded string.

Does it make sense to create an issue for that? I don't have any problem opening a PR to support that use case.

Also, an issue we have with the action peter-murray/workflow-application-token-action that didn't have any internal retry mechanism; maybe we can improve that in this action.

I will be happy to start working in some PRs to improve this action

@gr2m
Copy link
Contributor Author

gr2m commented Oct 26, 2023

private-key input as a Base64 encoded string

what is the use case? Maybe the decoding a base64 string could be done in a separate step before using this action?

didn't have any internal retry mechanism

For retries, @octokit has the retry plugin, it's rather heavy though, due to its dependency on bottleneck. We try to keep the code of this action, including its dependencies, to a minimum. It might make sense to implement a custom retry in this case particular case

@Moser-ss
Copy link
Contributor

So joining these two use cases, I have experienced failures getting the token from a GitHub app, so we start to use an extra action to retry Wandalen/wretry.action, then we have the issue with the input that is broken with multiline strings like the format of the PEM files, so we had to do encode the private key with base64.

Also, it is annoying to keep copying and pasting the ugly syntax of an action that retries another action.
So, if this action itself has the internal retry, I don't even need the base64 encoding.

So, does it make sense to implement a retry logic inside the action?

@gr2m
Copy link
Contributor Author

gr2m commented Oct 29, 2023

the input that is broken with multiline strings like the format of the PEM files, so we had to do encode the private key with base64

would replacing line breaks with "\n" work? That's what we do in environments where multi-line environment variables are not supported

if this action itself has the internal retry, I don't even need the base64 encoding

Perfect 👍🏼

I agree we should implement a retry, could you please file a separate issue for it?

@newbloke82
Copy link

I would like to try and help implement the proxy support. Getting this working from self-hosted runners behind a corporate proxy is important for my project.
I'm interested in changing the behaviour of the app to use GITHUB_API_URL as a default but allow for an override to deal with the use case of calling GHES from GHEC. This would also support GHES to GHES requests. This is useful for services like renovatebot/dependabot that need to lookup dependencies in private repos:
https://docs.github.com/en/code-security/dependabot/working-with-dependabot/configuring-access-to-private-registries-for-dependabot
https://docs.renovatebot.com/getting-started/private-packages/

@gr2m
Copy link
Contributor Author

gr2m commented Nov 2, 2023

@newbloke82 makes sense, thank you for laying it out! Can you please first file an issue? I think tibdex/github-app-token is implementing proxy support the way you suggest, right? See https://github.com/tibdex/github-app-token/blob/3eb77c7243b85c65e84acfa93fdbac02fb6bd532/action.yml#L8-L10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants