megalinter icon indicating copy to clipboard operation
megalinter copied to clipboard

Clarify How to Use Status Check Integration w/ GitHub

Open andrewvaughan opened this issue 5 months ago • 7 comments

Is your feature request related to a problem? Please describe. Apparently to use Branch rulesets that enforce Megalinter as a status check to merge a PR, it fails if both the following conditions exist (which I believe is common):

  1. Megalinter is configured not to use a secrets.PAT, but secrets.GITHUB_TOKEN
  2. Megalinter has any type of auto-fix enabled
  3. The GitHub repository has Megalinter enabled as a required status check in a branch ruleset

If this is the case, the status check will stick in limbo forever:

Screenshot 2024-01-21 at 2 12 10 PM

It took me a while to determine the root cause, but this SO post was the trick.

tl;dr - GitHub action restrictions prevents "user initiated" actions from calling another action.

Describe the solution you'd like There are tons of workarounds online for how to fix this - and they all suck - so I recommend making it more clear in the Megalinter docs and the generated workflow .yml file how to properly fix this problem.

Namely, reminding users they must use a PAT and not the GITHUB_TOKEN secret to ensure all actions from the GitHub workflow are coming from the GitHub user if they intend to use Branch Rulesets enforcing Megalinter.

Describe alternatives you've considered N/A

Additional context N/A

andrewvaughan avatar Jan 21 '24 19:01 andrewvaughan

To be honest, status check is an inheritance from Super-Linter, but since we implemented GitHub PR Comments and SARIF, I think it's kind of deprecated... :)

nvuillam avatar Jan 21 '24 23:01 nvuillam

To be honest, status check is an inheritance from Super-Linter, but since we implemented GitHub PR Comments and SARIF, I think it's kind of deprecated... :)

We may be talking past each other here, as I don't understand your reference. I'm talking about the new Rulesets that GitHub has implemented, where you can enforce status-checks to pass:

Screenshot 2024-01-21 at 8 12 31 PM

In depth documentation: https://docs.github.com/en/enterprise-cloud@latest/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets#require-status-checks-to-pass-before-merging

Per my referenced link above - the issue appears to be that this won't work if you use your GITHUB_TOKEN. My recommendation is simply to make that clear in the documentation and the workflow .yml that, if users want to use Megalinter with GitHub Branch Rulesets, they must (not can) create a PAT instead.

andrewvaughan avatar Jan 22 '24 01:01 andrewvaughan

The guidance you provide about using a PAT isn't related to the branch protections or rulesets. It is a design choice that GitHub actions made a long time ago that protects from creating an infinite loop. So the guidance can be limited to applying fixes/committing fixes, saying that Megalinter and all other workflows can only run on the new commit if a PAT is used.

That limitation can also be bumped with say, a periodic workflow that creates a PR each month updating some files. Using only a GITHUB_TOKEN, all workflows won't run (note here that I didn't talk about Megalinter or super-linter here).

At first, reading your issue, without having the link you sent, I thought you were talking about a problem with something like

permissions:
  checks: write
  ...
  (or)
  ...
  statuses: write
  ...
  (and other normal permissions like)
  ...
  pull-requests: read
  issues: read
  pages: none

https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs

For simply marking a job in a workflow as failed, we simply need to have a step with an exit code different of zero, like

exit 1

That wasn't what was the problem here, but that's how to set a failing status (See the difference between statuses and checks here https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks#types-of-status-checks-on-github, I had to reread that and around 5 times to finally understand the difference.)

You might want to take note of the last paragraph in the warning at the top of the article. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks Since you are playing with introducing required status, you'll probably get stung by this soon. A skipped job is considered successful for this. Read about the difference between outcome, result, and conclusion at least from the contexts article, but there should be more https://docs.github.com/en/actions/learn-github-actions/contexts

During the holidays I looked this up a lot, and also saw that relating to the handling of skipped/cancelled jobs, there was cases where some more business-oriented repos had to implement an extra job that needs the jobs, that was set to run always to make sure that a required check would always be valid. I saw one (I don't remember which one, but I saw two different approaches to solve that) that looked like it was a DevOps-specific employee, that introduced something with required checks, and in the next two weeks it was already tuned a couple times because of these nasty details, since it was daily used for other working employees.

echoix avatar Jan 22 '24 02:01 echoix

Wow! Thanks for the resources. I will check up on these tomorrow.

I will admit - finding information on how to resolve the hanging checks was challenging, at best. Nearly all recommendations were to:

  1. Just disable check enforcement; or,
  2. Push an empty commit as that somehow "fixes it"

And something about half-measures boils my blood 🔥

Let me get back once I absorb what you sent. I'll probably take a few reads, too, as this stuff isn't documented well, at all.

andrewvaughan avatar Jan 22 '24 03:01 andrewvaughan

Once you get through, you might want to have a look at my thread for my PR of that solution. It is in a GPL repo though, so I really wrote and developed it from scratch https://github.com/OSGeo/grass/pull/3320

echoix avatar Jan 22 '24 06:01 echoix

So @echoix I appreciate your patience here as I try to decipher all this. GitHub having, checks, statuses, and status checks all as (potentially) different things makes this so much more fun.

So, if I run MegaLinter without any Branch Rulesets requiring MegaLinter to pass, I get the green ✅ or ❌ . To me, it seems that the workflow, itself, doesn't have any issue reporting its status checks.

That is, until I require it using a Branch Ruleset. For some reason, it's only when I add this configuration that it hangs:

Screenshot 2024-01-23 at 2 16 36 PM

Resulting in:

image

Even though the action is complete... and would have given a happy ✅ in this status check were I not to have that Ruleset in place.

This is where I'm confused (although very appreciative!) from your feedback (and I'm sure it's my own user error), since it seems to focus on whether status checks are properly reporting at all - in the situation of skipping, matrices, etc - which seems to work fine. At least that's the impression I got focusing my attention on the warning:

Note: A job that is skipped will report its status as "Success". It will not prevent a pull request from merging, even if it is a required check.

So is this a naming issue for the Status Check? Is what's being reported from MegaLinter not matching what I have configured in the Ruleset so it's hanging? Is it a permissions issue from my first deep dive in the initial post? It's hard to tell, since GitHub is just saying "I'm waiting for MegaLinter to tell me what status it is," but only when the Ruleset is in place.

I'll keep hammering, but if you have any thoughts or further direction, it would be extremely appreciated - I imagine I'm not the only one that will raise eyebrows trying to get Rulesets to work here.

andrewvaughan avatar Jan 23 '24 19:01 andrewvaughan

Sorry, I totally forgot going back to your messages! I was just thinking about how it always is a pleasure to interact with you and your PRs and issues!

Did you ever try allowing the status check to be set by any source instead of only GitHub actions (pressing the blue "GitHub Actions" text).

Is it a case where an auto fix has created a commit using the GITHUB_TOKEN, and thus the new commit isn't ran again? (Solution: using a PAT)

Is it the first time of integrating Megalinter to the repo, where the workflow doesn't exist in the default branch (main)?

Even though from your previous answers, I don't think it's the case, but are the workflows really running for the branches where the required check is set?

You could also check for your hypothesis of a name mismatch by defining a required status check by clicking "Add 'the-autocomplete-not-found-name'" check even when the name isn't found. So you could try with capitals or not inside the name.

The name that is there is not the full name that you see in the PR, it matches the name of the job like the text that labels a job on the right tab when looking at the logs of a workflow with multiple jobs (underneath the tab where the Summary can be found).

Oh, I think I might have found it. I just remembered a note in the docs somewhere or the UI, that says that if a branch's commit (or HEAD, I don't remember the exact term) is found on multiple PR branches, then if the other ones are have something that blocks the PR, that PR can't be merged also.

I'll check for the reference.

Ok:

Even after all required reviewers have approved a pull request, collaborators cannot merge the pull request if there are other open pull requests that have a head branch pointing to the same commit with pending or rejected reviews. Someone with write permissions must approve or dismiss the blocking review on the other pull requests first.

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets#require-a-pull-request-before-merging

So it seems to be about reviews though.

Can you check that the PR has a unique HEAD (a different commit from other branches)?

Otherwise, if you can't find it, is there a public repo for this that I could explore this weekend if I manage to have time?

echoix avatar Jan 31 '24 00:01 echoix

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

github-actions[bot] avatar Mar 01 '24 00:03 github-actions[bot]