Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Private registry #566

Merged
merged 9 commits into from
May 25, 2024

Conversation

Tranquility2
Copy link
Contributor

Ref #562

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

@alexanderankin
Copy link
Member

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 alexanderankin changed the title Private registry feat: Private registry May 8, 2024
@Tranquility2
Copy link
Contributor Author

Added tests

@alexanderankin alexanderankin changed the title feat: Private registry feat(core): Private registry May 11, 2024
@Tranquility2
Copy link
Contributor Author

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

@alexanderankin
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

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 alexanderankin merged commit 59fbcfa into testcontainers:main May 25, 2024
7 checks passed
alexanderankin pushed a commit that referenced this pull request May 25, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.5.0](testcontainers-v4.4.1...testcontainers-v4.5.0)
(2024-05-25)


### Features

* **core:** Private registry
([#566](#566))
([59fbcfa](59fbcfa))


### Bug Fixes

* added types to exec & tc_properties_get_tc_host
([#561](#561))
([9eabb79](9eabb79))
* on windows, DockerCompose.get_service_host returns an unusable
"0.0.0.0" - adjust to 127.0.0.1
([#457](#457))
([2aa3d37](2aa3d37))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
mbenabda pushed a commit to mbenabda/testcontainers-python that referenced this pull request May 25, 2024
Ref testcontainers#562

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

---------

Co-authored-by: David Ankin <daveankin@gmail.com>
mbenabda pushed a commit to mbenabda/testcontainers-python that referenced this pull request May 25, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.5.0](testcontainers/testcontainers-python@testcontainers-v4.4.1...testcontainers-v4.5.0)
(2024-05-25)


### Features

* **core:** Private registry
([testcontainers#566](testcontainers#566))
([59fbcfa](testcontainers@59fbcfa))


### Bug Fixes

* added types to exec & tc_properties_get_tc_host
([testcontainers#561](testcontainers#561))
([9eabb79](testcontainers@9eabb79))
* on windows, DockerCompose.get_service_host returns an unusable
"0.0.0.0" - adjust to 127.0.0.1
([testcontainers#457](testcontainers#457))
([2aa3d37](testcontainers@2aa3d37))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@Tranquility2
Copy link
Contributor Author

@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
Copy link
Member

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
Copy link
Contributor Author

#582

alexanderankin pushed a commit that referenced this pull request May 27, 2024
…ature (DOCKER_AUTH_CONFIG) (#582)

Follow up on #566 - Testing using the registry module
alexanderankin pushed a commit that referenced this pull request Jun 5, 2024
Following #566 - Private registry, adding the relevant doc so the usage
will be clear and (hopefully) reachable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants