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: support customizing the Docker build command #1931

Merged
merged 7 commits into from Nov 29, 2023

Conversation

mdelapenya
Copy link
Collaborator

  • break: add BuildOptions method in order to support advanced customisations at build time
  • chore: add a test for not finding the given target in the Dockerfile
  • chore: make sure the auth configs are always added
  • docs: document the advanced usage

What does this PR do?

This PR adds a new field to the FromDockerfile struct, in order to hold a customizer function that will be executed when building a Docker image. This customizer will have access to the ImageBuildOptions Docker type, allowing users to build the image with as many configuration as they need. Of course, under their own risk.

We were forced to add a new method to the ImageBuildInfo interface, BuildOptions, in order to call the customizer defined by the user.

This customizer causes the deprecation of the GetAuthConfigs method from the same interface, although it probably needed to be deprecated before, when the code added support to discover the registry auths automatically. In any case, we are doing it so, creating a private method that automatically extracts the auths from the Dockerfile. These auths will be appended after the build options modifier is applied, to avoid undesired mistakes from the user.

The same applies for the Build args, the Image tag, the Dockerfile path and the build context, which are applied after the modifier for the same reasons.

Why is it important?

Instead of adding fields for the build image options type, we prefer having a small reduced of fields/methods to it, and one single modifier function that allows the user to modify what they need. Of course, with power comes responsibility, so it's on their own risk to modify the build options with all the possible modifications.

Testcontainers for Go will keep the build args, the tag for the image, the Dockerfile and build context paths, automatically discovering the registry auths for the Dockerfile.

Related issues

@mdelapenya mdelapenya requested a review from a team as a code owner November 29, 2023 12:43
@mdelapenya mdelapenya added the breaking change Causing compatibility issues. label Nov 29, 2023
@mdelapenya mdelapenya self-assigned this Nov 29, 2023
Copy link

netlify bot commented Nov 29, 2023

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit e5e5297
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/656732bcc118240008c1fa51
😎 Deploy Preview https://deploy-preview-1931--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@danvergara
Copy link
Contributor

Just for clarification. I can do this:

c, err := GenericContainer(ctx, GenericContainerRequest{
			ContainerRequest: ContainerRequest{
				FromDockerfile: FromDockerfile{
					Context:       ".",
					PrintBuildLog: true,
					KeepImage:     false,
					BuildOptionsModifier: func(buildOptions *types.ImageBuildOptions) {
						buildOptions.Target = "development"
					},
				},
			},
			Started: true,
		})

and it'll immediately pick up the target stage, right?

If so, that's awesome! That's a way more elegant solution than my proposal. I bit riskier as well.

@mdelapenya
Copy link
Collaborator Author

mdelapenya commented Nov 29, 2023

Exactly, PTAL at the tests we added: https://github.com/testcontainers/testcontainers-go/pull/1931/files#diff-fb8725b2e45d4dd45709211d85444408a4af558390996f8a54c5806f61f35a5aR169-R171

From my observations, Docker will build all the stages and will stop at the defined target: https://docs.docker.com/build/building/multi-stage/#stop-at-a-specific-build-stage

When you build your image, you don't necessarily need to build the entire Dockerfile including every stage. You can specify a target build stage. The following command assumes you are using the previous Dockerfile but stops at the stage named build:

And indeed riskier: we transfer the full control of the Docker type to the user, and if the Docker folks change that type, breaking changes are coming! But this will give more flexibility to our users, as I explained in the description. Again, I like to think that we treat our users as adults 😅 so they know what they're doing.

Thanks for your feedback!

@mdelapenya mdelapenya merged commit 3a513f3 into testcontainers:main Nov 29, 2023
112 checks passed
@mdelapenya mdelapenya deleted the build-image-modifier branch November 29, 2023 16:50
@danvergara
Copy link
Contributor

Awesome!!

Thanks @mdelapenya for the quick response and action!!!

mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Nov 30, 2023
* main: (100 commits)
  fix: fallback matching of registry authentication config (testcontainers#1927)
  feat: support customizing the Docker build command (testcontainers#1931)
  docs: include MongoDB's username and password options into the docs (testcontainers#1930)
  feat: support for custom registry prefixes at the configuration level (testcontainers#1928)
  Add username and password functions to mongodb (testcontainers#1910)
  chore: skip TestContainerLogWithErrClosed as flaky on rootless docker (testcontainers#1925)
  docs: add some Vault module examples (testcontainers#1825)
  feat: support for executing commands in a container with user, workDir and env (testcontainers#1914)
  fix(modules.kafka): Switch to MaxInt for 32-bit support (testcontainers#1923)
  docs: fix code snippet for image substitution (testcontainers#1918)
  Add database driver note to SQL Wait strategy docs (testcontainers#1916)
  Reduce flakiness in ClickHouse tests (testcontainers#1902)
  lint: enable nonamedreturns (testcontainers#1909)
  chore: deprecate BindMount APIs (testcontainers#1907)
  fix(reaper): fix race condition when reusing reapers (testcontainers#1904)
  feat: Allow the container working directory to be specified (testcontainers#1899)
  chore: make rabbitmq examples more readable (testcontainers#1905)
  chore(deps): bump github.com/twmb/franz-go and github.com/twmb/franz-go/pkg/kadm in /modules/redpanda (testcontainers#1896)
  Fix - respect ContainerCustomizer in neo4j module (testcontainers#1903)
  chore(deps): bump github.com/nats-io/nkeys and github.com/nats-io/nats.go in /modules/nats (testcontainers#1897)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Causing compatibility issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: Add Target field to FromDockerfile struct
3 participants