-
Notifications
You must be signed in to change notification settings - Fork 612
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
GitHub authentication failures updating statuses after upgrading to 1.24 #117
Comments
There are two tie in points for the credentials, they need to be selected on the main page, and then chosen for each job. Is the job set using the credentials? |
And the setup worked in 1.23? |
I upgraded to 1.23.3 (I think) this morning before upgrading to 1.24, and everything was working fine. I tried blowing away all of the GitHub tokens/auth config and setting it up fresh, but now the plugin isn't even scheduling any builds. The system log shows messages like
but nothing is getting scheduled, even though I'm positive there are PRs to test. |
Sorry, that reference was for a different issue. |
I think I may have a theory on why the plugin stopped scheduling builds, maybe. In the
section, there was a very suspicious tag: I removed that, and it started processing PRs again. It's still sometimes failing to update statuses and sometimes working. I haven't been able to track that down yet. |
The secret tag is for signature verification on webhooks. |
are you using webhooks or the cron? |
We are using cron. |
Set up a logger for org.jenkinsci.plugins.ghprb and see if there is any suspicious logs around the failures. |
The logger shows nothing abnormal. Without really changing anything (besides restarting Jenkins), I've seen the plugin on v1.24.
|
Update: it seems to be working again, without me changing anything. I wonder if GitHub was having issues or was rate-limiting our bot or something. @DavidTanner does the Cron trigger print out an error message if it fails to poll GitHub for any reason? I haven't seen any errors in the log, but I'm wondering if that's expected. |
I upgraded to 1.24.1 and restarted Jenkins a few minutes ago, and the plugin is again failing to schedule any builds, even though the logs indicate the trigger is running (though it's showing no updates on any PRs). We have 173 open PRs right now, so if the plugin is querying GitHub for all of these PRs at startup, I would not be surprised if we're being rate-limited. |
About 20m after startup it finally started running PRs. |
I also have same problem. After upgrading to 1.24.1 github has stopped triggering builds for PRs. |
Hey, I got the some problems when upgrading to 1.24.2. For me it even prints out this message when saving my configuration. I configured the plugin to automatically create the hooks for me, so i think the error starts here for me. |
Config file: http://pastebin.com/He4y7GTs |
@dblenkus you don't have any credentials set up, so it is using an anonymous connection. Anything requiring authorization will be rejected by GitHub. |
Thanks for pointing this out! It looks that credentials were reset during update.
|
So without credentials, the plugin is trying to get more information about the pull request, but is denied by GitHub. Once you have credentials set, and added to the main config then this should no longer happen. |
I've added new credentials ("Connect to API" button is passing), but am still getting the same error. Do you have any idea why? |
same for me, after i configuring credentials, it's still throwing errors. Do you need the config also after re-adding credentials? |
Yes, go to each job and set the credentials. |
I think I was also hosed in the upgrade :( I tried a few things and now it looks like the plugin is connecting via the Jenkins config screen but it looks like there is no user. Is this a valid expected response when I hit the "Connect to API" button? Connected to https://api.github.com as null The Credentials section under "GitHub Auth" in the "GitHub Pull Request Builder" will not let me select any global credentials. Is this expected behavior as well? |
There are rules applied to credentials that are beyond the control of the plugin. If something is set to a scope that the plugin isn't privilege to then it won't display them. As for connected as null, do you have a name set up in GitHub? |
I have a private org in GitHub and I have created a "bot" account for Jenkins CI to use associated with the PR builder. There was another security domain that matched api.github.com, but it is now gone after trying to eliminate the various credentials that weren't working. I'm now back at a point where the github hooks work for push into a target branch or merge of PR into a target branch, but my PR triggered jobs aren't running. I think I'm seeing a couple of different problems. If I use the "magic phrase" to try and trigger a job I get the error associated with the path being invalid and if a "new" PR is created, the job just doesn't trigger. |
Jun 24, 2015 3:11:00 PM FINE org.jenkinsci.plugins.ghprb.GhprbTrigger |
If you log in as that bot user, can you see the pull request you want to access? |
Yes, and I posted a comment on the PR just to confirm. |
I may have figured out what is happening after looking at the code. If my suspicion is correct, probably this should be
|
OK, that wasn't it - it's still not working. |
Dang. I was hoping that you had found it. So it is probably using the wrong credentials then, and ending up as anonymous is what you think? |
Maybe something like that, though I'm still a bit confused - it seems to be using our unauth API quota, but it's definitely posting comments as the bot account. When I converted from GLOBAL to SYSTEM, I guess I replaced the oauth token in the credentials store with something bad (it was autofilled with something - maybe that was junk), which caused some 401s to appear in the logs:
Saving the token again, it stopped printing that error, at least, though it still seems stuck. |
One inconsistency I've noticed is that Following the definition of that method, there are three differences:
|
OK, the build log from a recent run is puzzling, but maybe it's showing what might be part of the issue. Full log is available here, corresponding PR is kubernetes/kubernetes/pull/10246. When the build started, it failed to get PR status:
But immediately afterwards it (apparently) successfully updated the pending status:
When the run finished, it commented on the PR and updated the commit status (you can see this in the linked PR):
So why did it fail to query PR status, but succeed at updating the commit status and commenting on the commit? When it reads in the list of PRs from |
Some further updates: It appears to be using more authenticated requests, but there were 2 unauthenticated requests, still. Also, two builds failed to get PR status with the same 401 error status. |
I also swapped in to call After the PRs finish running I'll swap that back out to see if it makes a difference. |
Regarding the "leaked" unauthenticated requests:
The unauthenticated quota dropped by exactly one request - so it's possible this was retried unauthenticated? |
OK, I'm fairly certain I've tracked down the bug(s) here. When a new token is created in the global settings page and saved to the credentials store, the GitHub API URL is used to set specifications of the new credential. In particular, for the URL
When reading the credentials, there are two paths used, as I noted in an earlier comment. In the "test" case (used in the global settings page),
and so it works. In the "normal" case (used for all other operations),
Since the path is not the empty string, it fails. (Side note: it looks like the fact that scheme isn't present here is a bug that has existed for a while.) Anyway, there are a few ways to address these issues, though I'm not sure of a good way to fix all of the now-existing bad configurations with an empty path spec set. The path should either not be set, or it should be set to '/' - it shouldn't be empty. |
Would it be better to not add the path at all, or just default to "/"? I removed the GitURI code. |
Does #122 address everything? |
So the fix for users with the past setup is to add the / path to the credentials, and I am going to release #122 now. |
#122 didn't actually fix the issue, but I'll be sending a PR soon which should. |
- Actually use the new path generated as a fix in jenkinsci#122 - Use URIRequirementBuilder instead of manually building a requirements list - Add logging around accessing credentials to debug failures, and give better error messages when something is wrong - General refactoring of a few methods to reduce code duplication
- Actually use the new path generated as a fix in jenkinsci#122 - Use URIRequirementBuilder instead of manually building a requirements list - Add logging around accessing credentials to debug failures, and give better error messages when something is wrong - General refactoring of a few methods to reduce code duplication
Fix credentials bugs as noted in #117
Is anyone else still getting this issue? |
@juharris please try 1.29, there were a lot of changes acquiring and maintaining auth context throughout the workflow |
Thanks! Seems to work now in 1.29 :) |
👍 |
Remove a null pointer error, fixes #jenkinsci#117 Cleanup some of the logging in the main trigger, fixes jenkinsci#118
- Actually use the new path generated as a fix in jenkinsci#122 - Use URIRequirementBuilder instead of manually building a requirements list - Add logging around accessing credentials to debug failures, and give better error messages when something is wrong - General refactoring of a few methods to reduce code duplication
Fix credentials bugs as noted in jenkinsci#117
After upgrading to 1.24, the plugin is no longer able to update commit statuses, though the "Connect to API" button in the global settings successfully connects to GitHub.
From the console output:
The text was updated successfully, but these errors were encountered: