Add DelegatingDockerClient

[pjdarton]

Add a new class called "DelegatingDockerClient" which allows other plugins to easily "wrap" a DockerClient without also needing to stay tightly-coupled with a specific version of this plugin.

The problem:

This plugin controls which version of docker-java we have; the version of docker-java controls what methods etc the DockerClient interface has.
However, any other plugins (e.g. the docker-plugin) that need to extend/wrap/override DockerClient do not control what version of DockerClient they get, so while they have a compilation dependency on the exact version of docker-java that this plugin provides, they'll break if it changes.
This means that any change to this plugin has to be tightly coupled to matching changes in other plugins that use it, e.g. docker-plugin.

The solution:

Adding this class (which was originally coded in the docker-plugin) into this plugin means that the docker-plugin doesn't need to change its code every single time this plugin is updated; the bit of code that has to stay "in lock step" with docker-java will be in the plugin that controls it, so any time this plugin changes the version of docker-java, it can also change the code to match the new DockerClient interface.
This should then mean that upgrading this plugin won't "break the world" anymore.

TL;DR: This puts some docker-java specific code (that needs to be kept "in step" with this plugin's docker-java) into the place that controls docker-java, protecting everyone else from changes.

[pjdarton]

I believe this PR is safe and ready to merge.
It doesn't affect the docker-java version, it's just a quality-of-life improvement for dependent plugins, so just a minor enhancement.

[bguerin]

While I perfectly understand the interest of this class, I do not get if for this method ...

I might be wrong, but since it is generic, one will not be able to overload it with different signatures :

protected AttachContainerCmd interceptAnswer(AttachContainerCmd original) { ... }
protected AuthCmd interceptAnswer(AuthCmd original) { ... }

The only option is to use an ugly switch / case based on parameter class.

While you could just overload the public method you need to :

@Override
public AuthCmd authCmd() {
  AuthCmd result = super.authCmd();
  // do whatever you will have done in interceptAnswer()
  return result;
}

[pjdarton]

I agree that it will be difficult to do anything "method specific" at that point, and so if you're doing anything "method specific" you're better off overriding the method ... but my rationale here was that the interceptAnswer method is here to allow a plugin to intercept methods that weren't part of the API when they were coded.

e.g. a plugin could implement interceptAnswer to throw some sort of NotPermittedException on all methods but override the few methods they wanted to permit to let those operate normally.
e.g. a plugin that needed to only tinker with container creation could use if (originalAnswer instanceof CreateContainerCmd) clauses and have their code continue to work even if new createContainerCmd methods were added to the API at a later date.
i.e. it provides a way of coping with the API changing without forcing everyone to re-code (the plugins outside of this one) every time this plugin changes.

TL;DR: The "interceptAnswer" method is like using a "default" clause on a switch(someEnum)

[bguerin]

ok, understood.

However, I still do not see clearly how all of this will work, having the API updates but not the plugin, or the opposite, ...
I fear binary compatibility problems will still occur.

Let's see !

[pjdarton]

This plugin is 100% in control over what version of the docker-java library is on the classpath in Jenkins.
The problem I (personally) have is that the docker-plugin needs to be able to use a class like DelegatingDockerClient (and currently has its own implementation) but it (the docker-plugin) does not control the version of the docker-java library, so whenever this plugin changes to update the docker-java library, the docker-plugin stops working (and gets compilation errors) until it's changed to match.
i.e. we have a tight dependency between two supposed-to-be-independent plugins - they either both change, or neither.

My theory is that by having DelegatingDockerClient code in this plugin (that's tightly bound to the DockerClient API from the docker-java library, whose version is also decided by this plugin), another plugin (like the docker-plugin) can "simply" use this plugin's DelegatingDockerClient safe in the knowledge that, even if the docker-java library is later changed to a different version, this plugin will have made matching changes to DelegatingDockerClient, so everything (including the docker-plugin) should keep working (and thus reducing stress & workload on plugin maintainers).

i.e. we're co-locating "control of the DockerClient API" with "code that needs to change every time the DockerClient API changes" so that, hopefully, nobody else needs to worry about it again 🤞

PS. This should keep working until the DockerClient changes its API sufficiently to be entirely incompatible, but that seems to happen less often than it merely being extended. If we do encounter that issue often enough that it becomes a big problem then we could put in additional code here to bridge the gap between the docker-java versions (by making this plugin responsible for presenting a consistent API to Jenkins), but I figure we can cross that bridge when we come to it...

[bguerin]

agreed and convinced ;)

[pjdarton]

@ericcitaire If you can do a release (that contains the same version of docker-java as the current latest) then I can put together a matching PR on the docker-plugin.

Once that's done, we won't have to worry about releasing this plugin and the docker-plugin at the same time 😉

[ericcitaire]

@pjdarton 3.1.5-31.v70b0ca3e8310 was released a few hours ago.

I'm working on an upgrade of docker-java to 3.2.13 (see #12). I had to add a couple of methods to DelegatingDockerClient. Would you mind take a look and tell me what you think ?

Thanks.