credentials-binding implementation for Docker client certificates #54
Conversation
|
Is it still WiP? |
This is an implementation of Binding (the Extension for credentials-binding plugin) for DockerServerCredentials (client certificates). It gives a simple way to make these credentials available in any kind of build step or pipeline jobs, using the usual credentials-binding mecanism. The {ca,cert,key}.pem files will be written in a temporary directory, whose path can be bound to (by default) $DOCKER_CERT_PATH.
This PR depends on a little refactoring in credentials-binding-plugin, to reuse code from their FileBinding implementation - see jenkinsci/credentials-binding-plugin#25. Until it is accepted or rejected, this PR can't be merged, so I'm only submitting it for comments/reference.
thomasgl-orange
force-pushed
the
feature/credentials-binding
branch
from
3c9133a to
09d48bd
Compare
|
Well, it's still something I would like to see merged in the docker-commons plugin (in the mean time, I'm using a similar implementation in an in-house plugin, but I would happily get rid of it). But this PR depends on jenkinsci/credentials-binding-plugin#25 being merged, which is still pending... (sure, I could rewrite this without the refactoring to credentials-binding-plugin, but that would mean some code duplicated from FileCredentialsBinding). One other thing that was left to do was making the JUnit test work on Windows (it was relying on an 'sh' Pipeline step). I've just pushed the necessary changes: there is now an alternative 'bat' step for testing on Windows. And one last thing which would be nice to add is an @symbol annotation (to improve the syntax when using Pipeline or Job-DSL), but for that too I'm waiting for changes in credentials-binding-plugin to be merged (jenkinsci/credentials-binding-plugin#24). |
|
I summon @jglick to the thread |
| + " # check permissions and content of the certificate files\n" | ||
| + " [ $(stat -c %a \"$DOCKER_CERT_PATH/key.pem\") = 600 ]\n" | ||
| + " [ $(stat -c %a \"$DOCKER_CERT_PATH/cert.pem\") = 600 ]\n" | ||
| + " [ $(stat -c %a \"$DOCKER_CERT_PATH/ca.pem\") = 600 ]\n" |
There was a problem hiding this comment.
BTW this stuff could be put into src/test/resources/ and copied prior to running the build using TheClass.class.getResource[AsStream] and Node.getWorkspaceFor. Might be more readable that way.
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.
Cleanup the DockerServerCredentialsBinding test. Moved some multi-lines strings into separate files, because it was difficult to read.
thomasgl-orange
force-pushed
the
feature/credentials-binding
branch
from
c60a21f to
9381589
Compare
pom.xml
Outdated
| <artifactId>workflow-scm-step</artifactId> | ||
| <version>2.3</version> | ||
| <classifier>tests</classifier> | ||
| <scope>test</scope> |
There was a problem hiding this comment.
Deleted in a later version. You can probably delete this dep?
| <div> | ||
| Name of an environment variable to be set during the build.<br> | ||
| Its value will be the absolute path of the directory where the <code>{ca,cert,key}.pem</code> files will be created.<br> | ||
| You probably want to call this variable <code>DOCKER_CERT_PATH</code>, which will be undestood by the docker client binary.<br> |
jglick
changed the title
| @@ -1,2 +1,3 @@ | |||
| DockerServerDomainSpecification.DisplayName=Docker Server Credentials | |||
| DockerFingerprintAction.DisplayName=Docker Fingerprints | |||
| DockerServerCredentialsBinding.DisplayName=Docker client certificate | |||
|
|
||
| @DataBoundConstructor | ||
| public DockerServerCredentialsBinding(String variable, String credentialsId) { | ||
| super(variable, credentialsId); |
There was a problem hiding this comment.
Would be possible to extend MultiBinding directly (using UnbindableDir), which would enable you to supply a default value for variable by turning it into a @DataBoundSetter. (In the current inheritance chain this is impossible since the field is final.) Would enable Pipeline syntax to be simply
withCredentials([dockerCert(credentialsId: 'docker-client-cert')]) {
sh 'docker push'
}Not sure if it is worth the effort to make that refactoring, just FYI.
There was a problem hiding this comment.
I like the idea, but, well, I will pass. Lazy me...
I've pushed fixes for your other review comments.
3 participants
This is an implementation of Binding (the Extension for credentials-binding plugin) for DockerServerCredentials (client certificates). It gives a simple way to make these credentials available in any kind of build step or pipeline jobs, using the usual credentials-binding mecanism. The {ca,cert,key}.pem files will be written in a temporary directory, whose path can be bound to (by default) $DOCKER_CERT_PATH.
This PR depends on a little refactoring in credentials-binding-plugin, to reuse code from their FileBinding implementation - see jenkinsci/credentials-binding-plugin#25. Until it is accepted or rejected, this PR can't be merged, so I'm only submitting it for comments/reference.