-
Notifications
You must be signed in to change notification settings - Fork 355
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
Open
tsalzinger
wants to merge
4
commits into
jenkinsci:master
Choose a base branch
from
Tractive:track-credential-usage
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e91398a
Implement credentials tracking
55ea43a
Track credential usage within BitbucketCredentials#lookupCredentials
cc028d4
Rename BitbucketCredentials#lookupCredentials to lookupCredentialsAnd…
d1a1fe1
Replace deprecated usage of CredentialsProvider#lookupCredentials
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 27 additions & 0 deletions
27
src/main/java/com/cloudbees/jenkins/plugins/bitbucket/CredentialTrackingRunListener.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package com.cloudbees.jenkins.plugins.bitbucket; | ||
|
||
import com.cloudbees.plugins.credentials.CredentialsProvider; | ||
import hudson.Extension; | ||
import hudson.model.Run; | ||
import hudson.model.listeners.RunListener; | ||
|
||
/** | ||
* Tracks the usage of credentials | ||
*/ | ||
@Extension | ||
public class CredentialTrackingRunListener extends RunListener<Run<?, ?>> { | ||
@Override | ||
public void onInitialize(Run<?, ?> run) { | ||
final BitbucketSCMSource source = BitbucketSCMSource.findForRun(run); | ||
|
||
if (source == null) { | ||
return; | ||
} | ||
|
||
final boolean usesSshCheckout = source.getTraits().stream().anyMatch(scmSourceTrait -> scmSourceTrait instanceof SSHCheckoutTrait); | ||
|
||
if (!usesSshCheckout) { | ||
CredentialsProvider.track(run, source.credentials()); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 onlyCredentialsProvider.lookupCredentials
,CredentialsProvider.USE_ITEM
, andCredentialsProvider.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?