GitHubAppCredentials.writeReplace to avoid recalculating tokens #302
Conversation
|
There are code paths in this plugin that depend on the type of credentials used being |
Checked, and they are. On the agent side, you are just interested in a An alternative fix would be to create a |
jglick
changed the title
See the before state in jenkinsci/ssh-credentials-plugin@18b3121. |
I suppose you are talking about some |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@jglick Even comments in the code about why it is needed would be start. |
|
Added comments. As previously mentioned, while I believe it would be straightforward to add test coverage for node('remote') {
withCredentials([usernameColonPassword(credentialsId: 'ghapp', variable: 'AUTH')]) {
sh 'curl -f -u $AUTH $ENDPOINT/api/v3/app'
}
}will not cut it, because |
Despite #291, using app authentication made checkouts on agents really slow. On a server I tested with using the
kubernetesplugin, this reduces the time taken bycheckout scmon a tiny github.com repo from 46s to 26s. Otherwise agent stack traces during checkout show lots of delays likebecause not only is the token generated on the master not being reused, the agent is loading a ton of additional classes.
Note that the poor design of
git-client-apistill results in plenty of overhead; you would expect the master to just issue some/usr/bin/gitcommands to the agent, but instead a lot of classes are transferred to the agent and computation done there. See JENKINS-30600.Note that https://github.com/jenkinsci/credentials-plugin/blob/6f710a7e30156e94aff8a3c6c6799cef04ad35df/docs/implementation.adoc#additional-concerns suggests using
CredentialsSnapshotTaker. After jenkinsci/ssh-credentials-plugin@18b3121 there seems to be no implementation in a widely used credentials type. I did not see any particular benefit to using this extension point since you need to implement your ownwriteReplaceanyway—may as well inline the snapshotting (no reason for it to be extensible).