testcontainers / testcontainers-dotnet

A library to support tests with throwaway instances of Docker containers for all compatible .NET Standard versions.
https://dotnet.testcontainers.org
MIT License
3.74k stars 266 forks source link

[Bug]: cannot replace command (or any other enumerable) #1227

Open johnnyggalt opened 1 month ago

johnnyggalt commented 1 month ago

Testcontainers version

3.9.0

Using the latest Testcontainers version?

Yes

Host OS

Windows

Host arch

x86

.NET version

8.0.303

Docker version

Client:
 Version:           27.0.3
 API version:       1.46
 Go version:        go1.21.11
 Git commit:        7d4bcd8
 Built:             Sat Jun 29 00:03:32 2024
 OS/Arch:           windows/amd64
 Context:           desktop-linux

Server: Docker Desktop 4.32.0 (157355)
 Engine:
  Version:          27.0.3
  API version:      1.46 (minimum version 1.24)
  Go version:       go1.21.11
  Git commit:       662f78c
  Built:            Sat Jun 29 00:02:50 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.7.18
  GitCommit:        ae71819c4f5e67bb4d5ae76a6b735f29cc25774e
 runc:
  Version:          1.7.18
  GitCommit:        v1.1.13-0-g58aa920
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Docker info

Client:
 Version:    27.0.3
 Context:    desktop-linux
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.15.1-desktop.1
    Path:     C:\Program Files\Docker\cli-plugins\docker-buildx.exe
  compose: Docker Compose (Docker Inc.)
    Version:  v2.28.1-desktop.1
    Path:     C:\Program Files\Docker\cli-plugins\docker-compose.exe
  debug: Get a shell into any image or container (Docker Inc.)
    Version:  0.0.32
    Path:     C:\Program Files\Docker\cli-plugins\docker-debug.exe
  desktop: Docker Desktop commands (Alpha) (Docker Inc.)
    Version:  v0.0.14
    Path:     C:\Program Files\Docker\cli-plugins\docker-desktop.exe
  dev: Docker Dev Environments (Docker Inc.)
    Version:  v0.1.2
    Path:     C:\Program Files\Docker\cli-plugins\docker-dev.exe
  extension: Manages Docker extensions (Docker Inc.)
    Version:  v0.2.25
    Path:     C:\Program Files\Docker\cli-plugins\docker-extension.exe
  feedback: Provide feedback, right in your terminal! (Docker Inc.)
    Version:  v1.0.5
    Path:     C:\Program Files\Docker\cli-plugins\docker-feedback.exe
  init: Creates Docker-related starter files for your project (Docker Inc.)
    Version:  v1.3.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-init.exe
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc.)
    Version:  0.6.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-sbom.exe
  scout: Docker Scout (Docker Inc.)
    Version:  v1.10.0
    Path:     C:\Program Files\Docker\cli-plugins\docker-scout.exe

Server:
 Containers: 8
  Running: 5
  Paused: 0
  Stopped: 3
 Images: 24
 Server Version: 27.0.3
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: ae71819c4f5e67bb4d5ae76a6b735f29cc25774e
 runc version: v1.1.13-0-g58aa920
 init version: de40ad0
 Security Options:
  seccomp
   Profile: unconfined
 Kernel Version: 5.15.146.1-microsoft-standard-WSL2
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 8
 Total Memory: 31.31GiB
 Name: docker-desktop
 ID: f2376507-5297-4eca-ae87-0b375ce30fd4
 Docker Root Dir: /var/lib/docker
 Debug Mode: true
  File Descriptors: 110
  Goroutines: 161
  System Time: 2024-08-06T04:28:03.00615065Z
  EventsListeners: 13
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Labels:
  com.docker.desktop.address=npipe://\\.\pipe\docker_cli
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5555
  splinter:5000
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: No blkio throttle.read_bps_device support
WARNING: No blkio throttle.write_bps_device support
WARNING: No blkio throttle.read_iops_device support
WARNING: No blkio throttle.write_iops_device support
WARNING: daemon is not using the default seccomp profile

What happened?

When building a container - any container, but in my case Keycloak - specifying a different command results in merging. If the ContainerBuilder already defines a "default" command, there's seemingly no way to replace that command completely. For example:

let keycloakContainer =
    KeycloakBuilder()
        .WithImage("...")
        .WithCommand("start")
        ...
        .Build()

This will result in an attempt to run the command start-dev start rather than the desired start because KeycloakBuilder.Init already invokes .WithCommand("start-dev"). Moreover, I cannot find a way to supplant the default like you can with non-enumerable values. The easiest thing to do for a quick workaround would be to inherit KeycloakBuilder and override Init so that it doesn't invoke WithCommand, but KeycloakBuilder is sealed.

Relevant log output

No response

Additional information

No response

HofmeisterAn commented 1 month ago

Hi, at this point, this is intentional. Modules are pre-configured, opinionated, and follow best practices (configurations) driven by the community. The builder API is "immutable" to support A/B testing, it is currently not possible to override (remove) configurations for most elements that rely on lists or dictionaries (due to the way the BuildConfiguration works).

Supporting a feature that allows developers to entirely override the configuration would likely be a good enhancement, but it should be implemented explicitly. There are many variations that could potentially break the pre-configured module configuration, and developers should be aware that this could easily cause issues.

However, if a module's configuration does not suit your use case, you can always fall back to the generic builder and configure it yourself.

johnnyggalt commented 1 month ago

Thanks @HofmeisterAn ...

you can always fall back to the generic builder and configure it yourself

Can you please clarify this? Do you mean to extend ContainerBuilder<TBuilderEntity, TContainerEntity, TConfigurationEntity> myself? If so, that's what I've done as a workaround, but it turned into quite a thing as compared to simply overriding a deafult value or, absent that ability, inheriting from KeycloakBuilder and overriding its behavior.

HofmeisterAn commented 1 month ago

Can you please clarify this? Do you mean to extend ContainerBuilder<TBuilderEntity, TContainerEntity, TConfigurationEntity> myself?

Yes, this is what I meant.

I've done as a workaround, but it turned into quite a thing as compared to simply overriding a deafult value or, absent that ability, inheriting from KeycloakBuilder and overriding its behavior.

I understand, but it is difficult to provide the desired flexibility, supporting all kinds of configurations and maintaining reliability at the same time.

johnnyggalt commented 1 month ago

Got it. Just for the record, I decided to use reflection to reduce the amount of code. It's in my test fixture anyway, so not a huge deal.

type KeycloakBuilder with
    member this.WithOptimizedStartup() =
        let keycloakConfiguration =
            this
                .GetType()
                .GetProperty("DockerResourceConfiguration", BindingFlags.Instance ||| BindingFlags.NonPublic ||| BindingFlags.DeclaredOnly)
                .GetValue(this)
                :?> KeycloakConfiguration

        if keycloakConfiguration = null then
            failwith "Failed to resolve DockerResourceConfiguration property on KeycloakConfiguration"

        let commandField =
            keycloakConfiguration
                .GetType()
                .BaseType
                .GetFields(BindingFlags.Instance ||| BindingFlags.NonPublic)
            |> Array.tryFind _.Name.StartsWith("<Command>")
            |> Option.toObj

        if commandField = null then
            failwith "Failed to resolve backing field for ContainerConfiguration.Command property"

        commandField.SetValue(keycloakConfiguration, [ "start"; "--optimized" ] |> box)
        this
HofmeisterAn commented 1 month ago

If you like, we can keep the issue open. IMO, it is a valid feature request to be able to override module configurations. With the current design, overriding some parts is very tricky, especially when trying to prevent developers from making misconfigurations. However, we might find a better design that allows developers to properly override module configurations while also creating awareness that with the overridden values, wait strategies, connection strings, etc., may not work as expected anymore.

johnnyggalt commented 1 month ago

Sure thing, thanks