Add snapshot taker for special StandardUsernamePasswordCredentials impls

[jamesrobson-secondmind]

This adds a snapshot taker for StandardUsernamePasswordCredentials. This is needed to fix an issue in hashicorp-vault plugin which currently prevents ssh keys being sent to agents which is being developed in jenkinsci/hashicorp-vault-plugin#218

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[jetersen]

cc @jglick

[jglick]

Looks right.

[jglick]

Ah no, it fails to copy the usernameSecret field.

I would suggest just returning the argument unchanged if it happens to be a UsernamePasswordCredentialsImpl.

[jglick]

Suggested change

	return new UsernamePasswordCredentialsImpl(credentials.getScope(), credentials.getId(), credentials.getDescription(), credentials.getUsername(), Secret.toString(credentials.getPassword()));
	UsernamePasswordCredentialsImpl snapshot = new UsernamePasswordCredentialsImpl(credentials.getScope(), credentials.getId(), credentials.getDescription(), credentials.getUsername(), Secret.toString(credentials.getPassword()));
	snapshot.setUsernameSecret(credentials.isUsernameSecret());
	return snapshot;

just in case other impls now implement this parameter as well.

Main problem addressed. A minor suggestion remains.

[jamesrobson-secondmind]

We have a confirmation of this change working with vault (jenkinsci/hashicorp-vault-plugin#201 (comment)), from commit e331be4. I've tested the following changes my self so we should now be good to go.

[jtnord]

noting this seemingly breaks GitHubAppCredentials as it claims support for them and snapshots them 😱

org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials.generateAppInstallationToken(GitHubAppCredentials.java:216)
  at org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials.getToken(GitHubAppCredentials.java:298)
  at org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials.getPassword(GitHubAppCredentials.java:327)
  at com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsSnapshotTaker.snapshot(UsernamePasswordCredentialsSnapshotTaker.java:28)
  at com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsSnapshotTaker.snapshot(UsernamePasswordCredentialsSnapshotTaker.java:9)
  at com.cloudbees.plugins.credentials.CredentialsProvider.snapshot(CredentialsProvider.java:821)

[jglick]

@jtnord by “breaks” do you mean specifically the token refresh https://github.com/jenkinsci/github-branch-source-plugin/blob/7618247e672d2008fa8dc006ee8e87f4e64ab2c4/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java#L497?

If so, I guess the correct fix is to replace writeReplace with a new CredentialsSnapshotTaker taking precedence over UsernamePasswordCredentialsSnapshotTaker.

credentials-plugin/src/main/java/com/cloudbees/plugins/credentials/CredentialsProvider.java

Lines 808 to 814 in be8bbce

	Class bestType = null;
	CredentialsSnapshotTaker bestTaker = null;
	for (CredentialsSnapshotTaker taker : ExtensionList.lookup(CredentialsSnapshotTaker.class)) {
	if (clazz.isAssignableFrom(taker.type()) && taker.type().isInstance(credential)) {
	if (bestTaker == null || bestType.isAssignableFrom(taker.type())) {
	bestTaker = taker;
	bestType = taker.type();

should make this straightforward: no need for https://javadoc.jenkins.io/hudson/Extension.html#ordinal()

[jtnord]

@jtnord by “breaks” do you mean specifically the token refresh https://github.com/jenkinsci/github-branch-source-plugin/blob/7618247e672d2008fa8dc006ee8e87f4e64ab2c4/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java#L497?

exactly that.

If so, I guess the correct fix is to replace writeReplace with a new CredentialsSnapshotTaker taking precedence over UsernamePasswordCredentialsSnapshotTaker.

credentials-plugin/src/main/java/com/cloudbees/plugins/credentials/CredentialsProvider.java

Lines 808 to 814 in be8bbce

	Class bestType = null;
	CredentialsSnapshotTaker bestTaker = null;
	for (CredentialsSnapshotTaker taker : ExtensionList.lookup(CredentialsSnapshotTaker.class)) {
	if (clazz.isAssignableFrom(taker.type()) && taker.type().isInstance(credential)) {
	if (bestTaker == null || bestType.isAssignableFrom(taker.type())) {
	bestTaker = taker;
	bestType = taker.type();

should make this straightforward: no need for https://javadoc.jenkins.io/hudson/Extension.html#ordinal()

I'm working on it, there be some dragons I am trying to understand first.

[jglick]

See discussion in jenkinsci/github-branch-source-plugin#334.