feat(core): Private registry

testcontainers / testcontainers-python

Testcontainers is a Python library that providing a friendly API to run Docker container. It is designed to create runtime environment to use during your automatic tests.

https://testcontainers-python.readthedocs.io/en/latest/

Apache License 2.0

1.51k stars 281 forks source link

feat(core): Private registry #566

Closed Tranquility2 closed 3 months ago

Tranquility2 commented 4 months ago

Ref #562

This enhancement adds capability to utilize the env var DOCKER_AUTH_CONFIG in-order to login to a private docker registry.

alexanderankin commented 4 months ago

i wonder if this is what the java client does underneath, have almost no opportunity to find out independently. i explored the testcontainers side of java yesterday and realized that i need to look at docker-java to find any real answers anyways.

Tranquility2 commented 4 months ago

Added tests

Tranquility2 commented 3 months ago

Some documentation on how I tested it: https://gist.github.com/Tranquility2/20652b77a012ee54ae1c8ca021aea1ff

alexanderankin commented 3 months ago

wdyt about testing using this module - https://github.com/testcontainers/testcontainers-python/tree/main/modules/registry - i know it inverts the dependencies on their head a bit. maybe we can just copy and paste the code as it is just a part of the test suite and therefore not actually being published to pypi. doesn't have to be a part of this PR, just think that would be the most comprehensive/on-brand test.

maybe its not actually a blocker, just pausing to think cause when this is merged, there is no more fixes for 4.4 line (RP will set the version in #551 to 4.5.x)

Tranquility2 commented 3 months ago

was thinking the same thing :) I had the exact same doubt as its not really a best practice but fits like a glove for a more comprehensive test. Do you think it should be in this PR or another? In any case I'll be happy to add it.

alexanderankin commented 3 months ago

Ok, so I'll just compile some research here for the future:

the docker api expects a json object with username and password properties - base 64 encoded in the X-Registry-Auth header
the login method will call add_auth - which saves things into a dict - which get used to set the auth header during inspect, pull and push (and also in plugin and service) but not create container, like java does: createCmdExec, AbstractDockerCommandExec.

If this actually works and doesn't break anything, I think this is as close as were going to get.

alexanderankin commented 3 months ago

Do you think it should be in this PR or another? In any case I'll be happy to add it.

probably another at this point. its more than welcome, but have to keep merging things or else they get stale and conflicted and unmergable.

Tranquility2 commented 3 months ago

@alexanderankin sorry to say but the latest changes broke the new planned test. Now the config is loaded even before the DockerRegistryContainer invocation/build so the test cannot build DOCKER_AUTH_CONFIG using its registry_url (which is dynamic..)

Even if I do manage to set it before starting DockerRegistryContainer it will fail as it doesn't have the registry at this point. I can do something like:

def get_docker_auth_config() -> Optional[str]:
    return c.docker_auth_config or os.getenv("DOCKER_AUTH_CONFIG")

That solves this race but makes all the config part that you added a bit redundant.

In any case we need a way to update this after the the first container init testcontainers_config right? got any ideas?

alexanderankin commented 3 months ago

got any ideas?

Yes i can try to take a look but I think seeing your code (even with that change) would help me understand, I don't think I'm following based on your description. It worked last time for the contribution, then some minor changes from me during review, so hopefully it will work again.

Tranquility2 commented 3 months ago

https://github.com/testcontainers/testcontainers-python/pull/582