Un-hardcode Google Repository in AbstractDockerMojo and add AWS ECR support

[bjornbak]

Hi,

I like your plugin very much but I use AWS ECR and AWS ECS and not Google Repository.

I would like to contribute with AWS ECR and AWS ECS functionality but AbstractDockerMojo seems to be hardcoded to support Google Repository only adding an extra would mesh up the code.

Wouldn't it be nice to extract the Google Repository code and then be able to plug in Google or AWS ECR?

If you extract Google Repository I'll contribute with a AWS ECR plugin following your design principles.

[mattnworb]

What would ECR integration look like? Do they use short-lived access tokens like GCR does? It's hard to say what to do here without knowing anything about ECR.

[bjornbak]

Yes ECR uses a short-lived (12 hours) access token. When it is fetched a normal docker push is used.

I have implemented the ECR push using the maven-antrun-plugin and the AWS cli tool.

aws ecr get-login returns a docker login which is executed with the <exec command> afterwards.

<exec executable="docker"> <arg line="tag ${docker.image.name}:${git.tag} ${aws-id}.dkr.ecr.${aws-region}.amazonaws.com/${docker.image.name}:${git.tag}"/> </exec>
<exec executable="aws" outputproperty="get-login"> <arg line="ecr get-login --region ${aws-region} --no-include-email"/> </exec>
<exec command="${get-login}"/>
<exec executable="docker"> <arg line="push ${aws-id}.dkr.ecr.${aws-region}.amazonaws.com/${docker.image.name}:${git.tag}"/> </exec>

I have tried combining aws ecr get-login in the prepare-package phase with your push goal but it results in Could not push image: no basic auth credentials.

AWS offer a Java SDK so I would expect it to be possible to do a 1-to-1 replacement from the GCR implementation.

But as mentioned I think you need to extract the GCR implementation into a plugin so it is replacable with the ECR implementation...

[mattnworb]

Thanks for the information about ECR. I agree it would be best to support additional registries like ECR with some sort of plugin system, although I'm not sure at the moment exactly what that would look like. We might chose to not move the GCR auth logic currently there due to some internal spotify issues however (and concerns about backwards compatibility).

With that said if you already have ECR integration code ready, we'd be happy to review a PR and start discussing ideas for loading that type of support via plugins etc.

[ffazil]

+1

[mattnworb]

One idea I like for loading support for different authentication mechanisms would be to use a ServiceLoader to load any registered implementations of com.spotify.docker.client.auth.RegistryAuthSupplier from the classpath. This seems like the exact use case for a ServiceLoader.

This would allow someone using this plugin to add a dependency on another library/module that implemented ECR support without the plugin having to be aware of the ECR implementing class itself. In retrospect I wish I had done this with the GCR support from day one.

One potential issue would be that the ordering of the RegistryAuthSupplier instances loaded from the ServiceLoader would be undefined, but I think that would be ok as long as the plugin had a little logic about which suppliers to put in the tail of the list and then let the dynamic ones be loaded earlier in the list, as I imagine most RegistryAuthSupplier instances won't collide with one another.

[bjornbak]

I think a ServiceLoader the way to go. Do you still want to keep the GCR stuff in the core? How will you avoid it being loaded instead of an plugged in alternative implementation?

It is a bit difficult to test I got the ECR part right before I can plug it into your plugin. You know best how and where you want it...

I suggest that you create a branch where you create a Skeleton of a ServiceLoader mechanism and then I'll fillout the ECR specific parts and test that it works with ECR.

What do you think?

As mentioned before GCR and ECR seems very similar so I expect to be able to do 1-to-1 implementation of the GCR class.

[bino013]

Hi @mattnworb and @bjornbak,

Do you have any updates regarding dockerfile-maven integration to ECR? Have you come up with a configuration?

[bjornbak]

I have unfortunately no updates.

I am waiting for @mattnworb to create a plugin mechanism using the mentioned ServiceLoader.

[mattnworb]

There has been no progress on the ServiceLoader path, but I still think that is the way to go on this.

[mhermosi]

Come on... time passes... we need resolution on this issue...

[mattnworb]

@mhermosi we'd certainly welcome any pull requests related to this. As we don't use ECR ourselves we don't have any pressing need to add this support ourselves. See also spotify/docker-client#876

[sigpwned]

Is there still interest in adding ECR support? I've just made a pull request on docker-client. The implementation is very similar to the existing GCR implementation. If the team likes it and merges, then it should be fairly straightforward to add support for ECR here using the new ECR support in docker-client.

[mattmadhavan]

Hello,
Any update on the ECR integrations please?

Thanks
Matt

[bjornbak]

I moved on to a new job not using maven so I do not have the time and interest to do it anymore, sorry.