Ideas for new functionality

[benperez]

First of all, just wanted to say thanks for putting this library together. I've been using it for a few weeks and it works great. I'm primarily using it to programmatically build, tag and push docker images on an Amazon EC2 instance to a private ECR registry.

I've added a few pieces of functionality in my fork that I'd be happy to merge back in but I wanted to run the implementations by you first.

Easy stuff:

1. Tagging images.

tagImage :: forall m. MonadUnliftIO m => T.Text -> T.Text -> Maybe Tag -> DockerT m (Either DockerError ())
tagImage name repo maybeTag = requestUnit POST (TagImageEndpoint name repo maybeTag)

2. Pushing images.

pushImage :: forall m. MonadUnliftIO m => T.Text ->  Maybe Tag -> DockerT m (Either DockerError ())
pushImage name maybeTag = requestUnit POST (PushImageEndpoint name maybeTag)

Medium Stuff

1. MonadUnliftIO. How would you feel about switching the constraints on the Api.hs functions from (MonadIO m, MonadMask m) to just MonadUnliftIO m? I noticed you have a #if MIN_VERSION_http_conduit(2,3,0) check in Http.hs for backwards compatibility with older snapshots.

Bigger Stuff

1. Authentication. I added X-REGISTRY-CONFIG for building images and X-REGISTRY-AUTH for tagging and pushing images.
2. Credentials helpers. AWS and other platforms have different methods for doing docker authentication that are a little more complex than just the docker username and password. It seems like the preferred method for accomplishing this is described here. The gist of it is that you have a section in your ~/.docker/config.json which specifies a binary to run to get registry creds. The API that I'm thinking of for accessing these would let the user specify how to do auth in DockerClientOpts:

data DockerAuthStrategy = NoAuthStrategy
                        | ExplicitStrategy RegistryConfig
                        | DiscoverStrategy

NoAuthStrategy obviously uses no authentication. ExplicitStrategy let's you explicitly pass in a RegistryConfig mapping. DiscoverStrategy will look for a credsStore field in the ~/.docker/config.json file and use the binary it points to to automatically retrieve the auth parameters for each request. The DiscoverStrategy is inspired by amazonka

Let me know what you think!

[jprider63]

Hi @benperez. Support for tagging and pushing images would definitely be appreciated. I think we'd need to be careful in parsing and validating inputs (we probably shouldn't just accept Texts).

I think @denibertovic and I would probably need to discuss these bigger API changes. What's your motivation for switching to MonadUnliftIO?

For authentication, I wonder if it makes sense to let the user make these header changes in their HttpHandler. Maybe we can provide a helper function to modify a HttpHandler to support the AWS method of authentication.

[denibertovic]

@benperez This is great. We'd definitely appreciate the above changes. A couple of points:

• I agree with @jprider63 that it would make sense to use newtypes in the push/tag endpoints
• I'm definitely in favor of MonadUnliftIO but as @jprider63 mentioned we'd have to think about what this does in terms of breaking changes and how easy it is to migrate. I think it shouldn't be a problem.
• I'm super excited about the Auth parts! There's actually a TODO in the code (next to the pullImage function) where I talk about needing to implement this. I like the approach you're taking and the amazonka inspiration. This seems good to me. I do feel like we would have to think a bit about the exact implementation as @jprider63 mentions. I think a function like withDockerRegistryAuth or something could be useful but I'd have to play with the code a bit (I've only taken a very brief look at your commit that adds auth)

[benperez]

Thanks for the feedback. I'll clean up code I have now and open a couple of PR's in the next few days (time permitting) for the following:

1. tag and push with newtype wrappers and some integration tests to show their use.
2. A cleaned up version of auth so I can show a clearer picture of what I was envisioning.

I think I can hold off on the MonadUnliftIO stuff for now. I threw that in my fork when I was reworking the monad transformer stack I was using. I don't think I actually need those changes any more.