testcontainers-python icon indicating copy to clipboard operation
testcontainers-python copied to clipboard

feat(core): Private registry

Open Tranquility2 opened this issue 1 year ago • 5 comments

Ref #562

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

Tranquility2 avatar May 08 '24 13:05 Tranquility2

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.

alexanderankin avatar May 08 '24 19:05 alexanderankin

Added tests

Tranquility2 avatar May 09 '24 09:05 Tranquility2

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

Tranquility2 avatar May 12 '24 17:05 Tranquility2

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)

alexanderankin avatar May 14 '24 07:05 alexanderankin

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.

Tranquility2 avatar May 14 '24 17:05 Tranquility2

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

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

alexanderankin avatar May 25 '24 18:05 alexanderankin

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.

alexanderankin avatar May 25 '24 19:05 alexanderankin

@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?

Tranquility2 avatar May 26 '24 18:05 Tranquility2

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.

alexanderankin avatar May 26 '24 19:05 alexanderankin

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

Tranquility2 avatar May 26 '24 21:05 Tranquility2