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

[Enhancement]: Support image override #1260

Closed
rudiv opened this issue Sep 10, 2024 · 2 comments
Closed

[Enhancement]: Support image override #1260

rudiv opened this issue Sep 10, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@rudiv
Copy link

rudiv commented Sep 10, 2024

Problem

Image Names are currently constants within the Builders, example https://github.com/testcontainers/testcontainers-dotnet/blob/1d32bc86e902f4684c8b348dbb82a0f44a579c0e/src/Testcontainers.Azurite/AzuriteBuilder.cs#L7C5-L7C89

Given that, especially in the case of Azurite, to update the Storage packages from NuGet in consuming projects means that you have to match the corresponding API version on the container, this is very prohibitive.

The classes are also sealed for some reason. There's no way to override this without duplicating the class in your own project.

Solution

Provide an option to configure the Image that's used for these containers, whilst retaining the benefits of the rest of the module.

This could be easily achieved by allowing either a configuration object to be passed in or, at the very least, allow the image name to be overridden within the constructor.

public sealed class AzuriteBuilder : ContainerBuilder<AzuriteBuilder, AzuriteContainer, AzuriteConfiguration>
{
    public readonly string AzuriteImage = "mcr.microsoft.com/azure-storage/azurite:3.28.0";

    // ...

    /// <summary>
    /// Initializes a new instance of the <see cref="AzuriteBuilder" /> class.
    /// </summary>
    public AzuriteBuilder(string? imageName = default)
        : this(new AzuriteConfiguration())
    {
        AzureiteImage = imageName ?? AzuriteImage;
        DockerResourceConfiguration = Init().DockerResourceConfiguration;
    }

I'm happy to go through and implement this where necessary, but this may be an intentional restriction for reasons I can't understand?

Benefit

Allows more flexibility for tests without restricting projects to specific versions of images.

I understand that this might be done for major changes, but obviously it doesn't have to be a fully supported option when using these builders.

Alternatives

Unseal the classes to allow overrides to be manually performed.

Would you like to help contributing this enhancement?

Yes

@rudiv rudiv added the enhancement New feature or request label Sep 10, 2024
@HofmeisterAn
Copy link
Collaborator

There's no way to override this without duplicating the class in your own project.

That's not right. Actually, we encourage and recommend overriding and pinning the image version in our best practices. Every container builder supports the WithImage(string) API, which lets you set the exact image and version used:

_ = new AzuriteBuilder().WithImage("mcr.microsoft.com/azure-storage/azurite:3.32.0").Build();

There might be cases where a new image version breaks an existing module configuration, like we recently had with MSSQL. When that happens, we update the module configuration accordingly to support multiple versions. Another good example is our MongoDB module that supports multiple major versions too.

@HofmeisterAn HofmeisterAn closed this as not planned Won't fix, can't repro, duplicate, stale Sep 11, 2024
@rudiv
Copy link
Author

rudiv commented Sep 11, 2024

This is why you don't start doing thing things (especially publicly) late at night, kids!

Thank you for the information and being kind about my stupidity - I'll be sure to delete my classes 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants