-
Notifications
You must be signed in to change notification settings - Fork 354
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
Implement credentials tracking #834
base: master
Are you sure you want to change the base?
Conversation
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java
Outdated
Show resolved
Hide resolved
@Extension | ||
public class CredentialTrackingRunListener extends RunListener<Run<?, ?>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised that this is needed. Is this how other plugins track the credentials of runs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest I have no idea.
This is my first time working on the jenkins / a jenksin plugin codebase and all of it is rather overwhelming. I just noticed that we didn't see any usages of our bitbucket credentials so I started to look into it. But even to find CredentialsProvider.track
I had to look through the code of the credentials plugin as I couldn't find any documentation about it - the only hint was the UI showing Note: usage tracking requires the cooperation of plugins and consequently may not track every use. which set me down that path.
With my limited knowledge though it kind of makes sense as at the time of the actual job invocation any generic code wouldn't know about the configured credentials anymore as they would have been transfored already into whatever format is actually required to perform the task (eg an environment variable, or a request header, etc.), and even if you would hook exactly into that transformation you most likely wouldn't have access to the actual Run
anymore. Of course I might be completely wrong here - so I'm happy about any insight. I am also fine with dropping this specific thing from the PR and opening up a separat ticket / PR if for the actual credentials tracking of job runs.
Please just let me know on how you want to proceed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other plugins may be using CredentialsProvider.findCredentialById, which usually calls CredentialsProvider.track. However, a search for CredentialsProvider
in github-branch-source-plugin shows only CredentialsProvider.lookupCredentials
, CredentialsProvider.USE_ITEM
, and CredentialsProvider.lookupStores
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So how should we procceed here?
Should I drop the listener for now? I don't think I can dig into this in more detail before end of next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KalleOlaviNiemitalo any feedback on this?
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketCredentials.java
Outdated
Show resolved
Hide resolved
a504676
to
0c3b6ee
Compare
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketCredentials.java
Fixed
Show fixed
Hide fixed
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketCredentials.java
Fixed
Show fixed
Hide fixed
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketCredentials.java
Fixed
Show fixed
Hide fixed
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketCredentials.java
Fixed
Show fixed
Hide fixed
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketCredentials.java
Fixed
Show fixed
Hide fixed
3cece97
to
095e979
Compare
This allows to see jobs which execute repository / branch scanning as users of credentials, allowing to figure out which jobs actually reference these credentials more easily.
The previous implementation actually missed out on a couple of usages, for example within `BitbucketSCMNavigator`.
…TrackUsage The new name clearly indicates the additional behavior which was introduced previously and does not hide away a potentially unintended side effect.
`CredentialsProvider#lookupCredentials` was deprecated in favor of `CredentialsProvider.lookupCredentialsInItem` which uses `org.springframework.security.core.Authentication` instead of `org.acegisecurity.Authentication`.
095e979
to
d1a1fe1
Compare
This allows to see jobs which execute repository / branch scanning as users of credentials, allowing to figure out which jobs actually reference these credentials more easily.
closes #833
Your checklist for this pull request
No - I honestly don't know on how to provide automated tests for this - I verified the behavior manually but I'd gladly implement some unit / integration tests if I can get some pointers