[JENKINS-36240] Better implementation of getTrustedRevision #96
Conversation
… precision whether a PR author would have had write access to the origin repo.
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
We used to retain information about what was trusted; now this is checked only if and when getTrustedRevision is called.
| trusted = false; // TODO JENKINS-37608: make this configurable | ||
| } catch (HttpException x) { | ||
| if (x.getResponseCode() == 403) { | ||
| listener.getLogger().format("Not authorized to do a direct permission check of %s on %s/%s; scan token has no access to this repository%n", sourceOwner, repoOwner, repository); |
There was a problem hiding this comment.
Cache this so you don't retry the request for all PRs, also perhaps we should drop back to the previous behaviour if the permission is missing
There was a problem hiding this comment.
Cache this so you don't retry the request for all PRs
Not sure why caching would be required; this code should only be run during an actual build of the branch project. We could try to cache 403s per repository × credentialsId, though this cache could become stale for several reasons—different token, perhaps different permissions for a given token, different repository or team settings…
Probably. Or at least figure out how to fall back to earlier behavior in the meantime.
AFAIK those would not have had permission to use the old API either. |
Or just implement JENKINS-37608 so that the user can configure a fixed behavior for all pull requests. The current implementation is so flawed I see little point in retaining it. |
|
I think what I want this to do is:
|
|
The functionality of this change will be reviewed after https://issues.jenkins-ci.org/browse/JENKINS-43426 has been delivered |
|
Indeed this will likely need to be rewritten to use the proposed new API. |
|
Needs to be reworked to offer an alternative to |
Downstream of jenkinsci/github-api-plugin#11.
@reviewbybees