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

Support GitHub workflow badge for private repos #9189

Closed
agboom opened this issue May 21, 2023 · 5 comments
Closed

Support GitHub workflow badge for private repos #9189

agboom opened this issue May 21, 2023 · 5 comments

Comments

@agboom
Copy link
Contributor

agboom commented May 21, 2023

📋 Description

The GitHub workflow badge does not support private repos, even with a PAT with access to them.

If I understand the code correctly, the current GithubActionsWorkflowStatus badge service uses the public url for Workflow Status badges to fetch the workflow status and converts it to a Shields badge. That would explain why the status of private repos results in a repo or workflow not found.
As said in the GitHub docs:

Workflow badges in a private repository are not accessible externally, so you won't be able to embed them or link to them from an external site.

To support this I think the GitHub API needs to be used instead.

@chris48s
Copy link
Member

The history behind why this badge doesn't use the API is long and spread across many issues, but we did at one point migrate this to use the API instead of scraping GitHub's native badge.

Unfortunately, this introduced several bugs that we weren't able to solve using the data/endpoints exposed by the API so we switched back to scraping the native badge. The key issue is #8733 (this was one of the bugs introduced by switching to the API as the data source) and to solve it we picked option 3 from #8733 (comment) . We accepted that the tradeoff of that solution is that users who self host would not be able to use it for private repos, but it is a known limitation rather than an unexpected bug.

If you want to read the whole saga that lead to this, the relevant issues and PRs are:

I haven't really looked at #9190 but I suspect you're probably introducing the same bugs I introduced in #8475 when I tried to migrate this before and then reverted in #8738.

@agboom
Copy link
Contributor Author

agboom commented May 22, 2023

Thanks for your write-up! I should have searched more thoroughly before I created this issue, sorry about that.

Based on the information I see a few roads forward:

  • We keep the implementation as-is and close the PR. It might be useful to document the private repo limitation somewhere on the Shields site if it's possible?
  • We implement the API based version as a separate badge service with a different url. Maybe mark it as experimental to indicate there may be bugs (show this experimental status on the Shields site if possible?).
  • We try to reproduce the bugs using tests and fix them in the new implementation (although if the bugs are upstream on the API side, this is not possible of course).

Let me know what you think. I'm entirely fine with closing the PR and this issue, but I'm also willing to try some more to make it work.

@agboom
Copy link
Contributor Author

agboom commented May 23, 2023

Looking at the issues more closely I understand that using the API has some problems outside of our control that affect the performance required for this badge. In that case I think we indeed have no choice but to use the SVG scraper.

@chris48s
Copy link
Member

I should have searched more thoroughly before I created this issue

No worries. As I said, the history of this is spread across multiple discussions so there isn't really a single search result that would have given all the background on this.

Looking at the issues more closely I understand that using the API has some problems outside of our control that affect the performance required for this badge

Yeah pretty much. It is not that it is impossible to implement the correct behaviour using the data exposed by the API, but we found it was not really possible to do it inside the timeframe we need to render a badge.

@agboom
Copy link
Contributor Author

agboom commented May 24, 2023

I'll close this issue and the PR, thanks for following up on it 🙏.

@agboom agboom closed this as completed May 24, 2023
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

No branches or pull requests

2 participants