Skip to content

Conversation

@thomasgl-orange
Copy link
Contributor

This PR is the successor of #25 (which was merged and then rolled-back, and thus can't be updated anymore). The goal is still to ease writing new credentials binding implementations which write to temp files without copy/pasting what exists already in FileBinder.

The code from #25 was reviewed already by @jglick, and it was fine, expect it was specific to writing Binding implementation, and was not supporting MultiBinding at all. Meaning it wasn't helping in cases like the SSH key binding proposed in #18 for instance.

This new PR includes two commits:

Here is an example of using this new API to simplify a MultiBinding implementation (the SSH keys one from #18): thomasgl-orange/credentials-binding-plugin@a4423f3

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right from a brief review. Are there downstream PRs showing this being used for SSH and/or certificate and/or Docker credentials?

@jglick
Copy link
Member

jglick commented Jun 13, 2017

Deployed these changes as version 1.12-20170613.180829-1 which would allow them to be consumed from a PR in another plugin, compatible with CI. For a potential PR in this plugin, you would simply merge this branch.

thomasgl-orange added a commit to thomasgl-orange/docker-commons-plugin that referenced this pull request Jun 14, 2017
Updated credentials-binding dependency to use the build from jenkinsci/credentials-binding-plugin#36.
Also had to switch to newer Jenkins core, and to Java 7, to get everything (including new test case) working.
@thomasgl-orange
Copy link
Contributor Author

Are there downstream PRs showing this being used for SSH and/or certificate and/or Docker credentials?

I have submitted #38 to trigger a CI build of a modified version of the SSH Credentials binding implementation proposed by @matthauck in #18.

I have also updated jenkinsci/docker-commons-plugin#54.

@thomasgl-orange
Copy link
Contributor Author

I've now also submitted #39 to trigger a CI build of a modified version of the Certificate binding implementation proposed by @idstein in #11.

@jglick jglick merged commit 7f75534 into jenkinsci:master Jun 15, 2017
jglick added a commit that referenced this pull request Jun 15, 2017
Certificate binding to variables (with PR #36)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants