Skip to content
This repository has been archived by the owner on Jul 20, 2019. It is now read-only.

Sometimes GPG check fails, even though all commits are verified #59

Open
rallytime opened this issue Oct 24, 2017 · 11 comments
Open

Sometimes GPG check fails, even though all commits are verified #59

rallytime opened this issue Oct 24, 2017 · 11 comments
Assignees
Milestone

Comments

@rallytime
Copy link

I am not sure why, but occasionally I see the GPG check fail, even though all of the individual commits are verified.

Here are some examples:

At first I thought it might have something to do with the shear number of commits in a PR because I thought i was seeing them fail only on PRs with many commits. However, that doesn't appear to be the case since I found a PR today that only had 2 commits in it.

@jarrodldavis jarrodldavis self-assigned this Oct 25, 2017
@jarrodldavis jarrodldavis added this to the v0.6.0 milestone Oct 25, 2017
@jarrodldavis
Copy link
Owner

Hmm... interesting. It doesn't look like the cause is a rebase since the 2-commit PR doesn't look like it was force pushed so I'm not sure what's going on. I'll need to add logging (#45) to get more information about what the app sees so I can track it down. Thanks for reporting the issue.

@rallytime
Copy link
Author

@jarrodldavis Sounds good. Thank you for looking into it and let me know if you need anything else from me.

@jarrodldavis
Copy link
Owner

Hey @rallytime, I just merged #61 into develop and I have a development instance of the app running. If you want to help me track down the issue, you can install https://github.com/apps/gpg-development on one of your repositories so I can see if the issue happens again. Be sure to have the production instance of the app disabled on any repo you install this dev instance on to prevent any conflicts.

@jarrodldavis
Copy link
Owner

jarrodldavis commented Oct 28, 2017

This gets even weirder… saltstack/salt#44253 and saltstack/salt#44259 have been updates with success statuses where it previous marked them as failure. Looking through the GitHub API, something triggered the app to update the statuses again on the 25th, a day after the commits in question were made…

saltstack/salt#43379 remains failed, though.

@jarrodldavis
Copy link
Owner

Hmm, so I saw a couple more pull requests with the issue today. It appears the initial open was marked as success but subsequent pushes were marked with error. After manually going through the verification steps locally and also redelivering web hooks, I can't reproduce the issue. There has to be some timing issue with the API or something.

@rallytime do you think you could switch the saltstack/salt repo over to the development deployment so I can catch the logs when this happens?

@rallytime
Copy link
Author

@jarrodldavis - yes, I will look into getting the development deployment installed today.

@rallytime
Copy link
Author

Ok, I have switched those over now on the https://github.com/saltstack/salt repo. Please let me know if there is anything else I can help with. :)

@jarrodldavis
Copy link
Owner

After more investigation, there appears to be an issue with the GitHub API after a pull request is updated with additional commits. My assumption is that when a pull request (or probably a branch in general) is pushed to, GitHub's signature verification service re-verifies all of the commits, and there's a delay between when the webhook is sent in response to the push and when the verification service completes.

@rallytime
Copy link
Author

Is there a way to handle the delayed response/verification service when a pull request receives a push?

@jarrodldavis
Copy link
Owner

Other than hard coding a delay when receiving a pull_request.synchronized event or implementing a retry mechanism when encountering any unsigned commit, not really. I get the same response from the API as if the commit was actually unsigned, even though the reason field is documented with some values for when the verification service has issues.

I've reached out to GitHub Support about the issue and they've followed up with some requests for more info, so hopefully we can track down the cause of the problem. I'll keep you updated.

@rallytime
Copy link
Author

@jarrodldavis Thanks for looking into it! I look forward to any updates.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants