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

Allow custom docker registry #335

Closed
mervyn-mccreight opened this issue Feb 23, 2022 · 13 comments · Fixed by #549
Closed

Allow custom docker registry #335

mervyn-mccreight opened this issue Feb 23, 2022 · 13 comments · Fixed by #549

Comments

@mervyn-mccreight
Copy link
Contributor

I think it would be nice to allow some configuration for a custom docker registry to pull the images from.
I'm thinking about a general point of configuration.

This is useful if e.g. you are working in a company that relies on an internal docker repository (e.g. as a caching solution).

@thomaseizinger
Copy link
Collaborator

Agreed.

I think the best way to extend the current codebase here is to have an optional field in the docker client and produce FQ container names in case they aren't already fully-qualified.

There may also be a config in the CLI that we can use.

@mervyn-mccreight
Copy link
Contributor Author

Cool, I think I'll give it a try 😃

@DDtKey
Copy link
Collaborator

DDtKey commented Feb 18, 2024

Actually I might want to use the same client against different registries (e.g containers from different registires, but within the same bridge network).

Moreover the name of images may vary between registries. For example, AWS ECR for docker library would be: gallery.ecr.aws/docker/library/{name}, but not all images from docker-hub are available there (depending on owner) and not all of them have the same prefix (e.g bitnami images https://gallery.ecr.aws/bitnami/ ) It could be even different owner names.

Currently it's enough to set name for container to necessary one (e.gpublic.ecr.aws/docker/library/redis or public.ecr.aws/bitnami/kafka)

But we don't expose the way to override the name through RunnableImage currently. So I used to re-define images locally with custom name for such purposes.

I think it is correct to allow overriding the name as well as tag (already possible) as part of RunnableImage.

As an alternative we could extend Image::name to three configs: registry (optional), owner (optional) and name (required). It could be a bit more clear for users, but not in all cases, taking ECR example above, we can see gallery.ecr.aws is registry, docker/library is owner. So probably single name still makes sense

WDYT?

UPD: I've created a PR #549 & #550

DDtKey added a commit to DDtKey/testcontainers-rs that referenced this issue Feb 18, 2024
This change allows to override name of the image. Thus, user will be able to change registry, owner and the name itself.

Closes testcontainers#335
DDtKey added a commit to DDtKey/testcontainers-rs that referenced this issue Feb 18, 2024
This change allows to override name of the image. Thus, user will be able to change registry, owner and the name itself.

Closes testcontainers#335
DDtKey added a commit to DDtKey/testcontainers-rs that referenced this issue Feb 18, 2024
This change allows to override name of the image. Thus, user will be able to change registry, owner and the name itself.

Closes testcontainers#335
DDtKey added a commit to DDtKey/testcontainers-rs that referenced this issue Feb 18, 2024
This change allows to override name of the image. Thus, user will be able to change registry, owner and the name itself.

Closes testcontainers#335
@thomaseizinger
Copy link
Collaborator

But we don't expose the way to override the name through RunnableImage currently.

Yeah and there is a good reason for that! The Image trait is what ties together the image with its version and configuration. That creates a type-safe contract for a working container. Adding the ability to replace the image with a completely different one breaks that contract.

@mervyn-mccreight
Copy link
Contributor Author

The contract is already kind of broken with allowing to replace the tag, because nothing can guarantee that everything is working with a new version, especially if the readiness check is log message based.

Please don't get me wrong here, being able to replace the tag is crucial at least to me, because I want to mirror my dependency versions in production in tests.

@DDtKey
Copy link
Collaborator

DDtKey commented Feb 19, 2024

Yeah, we do allow tag to be overridden and there is a comment that it may broke the image.

What do you think about the second solution? We could allow to change only registry and/or owner in RunnableImage. However it's basically the same: changing the registry also may lead to the failure, for example - the image with another hash is kept for the tag

Anyway by redefining the tag/name/registry, users clearly take responsibility on themselves.

In addition, looks like it's allowed in other languages: https://github.com/testcontainers/testcontainers-go/blob/357cff81c2e44ce1c27112244dce4d40229be347/options.go#L64 (part of ContainerCustomizer) or https://java.testcontainers.org/features/image_name_substitution/

@DDtKey
Copy link
Collaborator

DDtKey commented Feb 19, 2024

Moreover, nobody prevents us to expose API to change the name on Image level (like MyImageX:new().with_name("another") - testcontainers can't prevent this, people decide how to implement the trait)

But I'd better expose it as part of RunnableImage, because in fact, it does broke Image itself. We may consider this as a separate customized image (i.e by wrapping Image with RunnableImage and overriding any configs - user creates his own invariant of the image. Original Image is just predefined "base"/"skeleton" for your customization here)

Even changing of ENV var can break the image and we do allow that, otherwise it would be a mess.

@DDtKey
Copy link
Collaborator

DDtKey commented Feb 19, 2024

That creates a type-safe contract for a working container.

Btw, I think it doesn't work anyway. In practice, I've encountered many cases where a docker tag gets overwritten and breaks existing code based on that particular tag.
We cannot provide any guarantees regarding the actual image located by "name" + "tag".

Just for example: bitnami/containers#55274 - image just didn't work for several days (it affected not only latest tag, but quite a lot of specific ones) - only manual changes could help in that scenario

@thomaseizinger
Copy link
Collaborator

thomaseizinger commented Mar 7, 2024

Perhaps it is a lost battle until Nix takes over the world.

DDtKey added a commit to DDtKey/testcontainers-rs that referenced this issue Apr 19, 2024
This change allows to override name of the image. Thus, user will be able to change registry, owner and the name itself.

Closes testcontainers#335
github-merge-queue bot pushed a commit that referenced this issue Apr 19, 2024
This change allows to override name of the image. Thus, user will be
able to change registry, owner and the name itself.
See
#335 (comment)
for more details

Closes #335
@noelmcloughlin
Copy link

So TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX environment variable is not supported. I did not expect this as works with other languages, so I have been troubleshooting for few hours

@noelmcloughlin
Copy link

Its increasingly regulatory compliance issue, companies have to proxy external software supply chains, can't use Internet directly

@DDtKey
Copy link
Collaborator

DDtKey commented Nov 15, 2024

@noelmcloughlin currently it's possible to override the registry utilizing ImageExt::with_name

@noelmcloughlin
Copy link

noelmcloughlin commented Nov 15, 2024

Thank you, I was looking at ContainerRequest API, but this ImageExt API looks like it might work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants