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

fix: fallback matching of registry authentication config #1927

Merged

Conversation

sermio-te
Copy link
Contributor

What does this PR do?

This PR will fix an issue when config.json contains auths with keys set to the a registry URL such as https://artifactory.com. This way the authentication configuration for the registry will be matched with the configuration file and used.

Why is it important?

In case where a registry is configured with an https:// prefix, the authentication to pull images from that registry fails.

How to test this PR

Manually add a registry and its authentication config in the ~/.docker/config.json and run a test that requires authentication to pull images and start containers.

@sermio-te sermio-te requested a review from a team as a code owner November 28, 2023 14:46
Copy link

netlify bot commented Nov 28, 2023

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 4951aca
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/65671f82627a6b0008947e3c
😎 Deploy Preview https://deploy-preview-1927--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.

Copy link
Collaborator

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for submitting this PR, could you please add tests demonstrating the bug? 🙏

From your PR description I totally understand, but I'd like to have automated tests for the fix, thanks!

@sermio-te sermio-te force-pushed the registry-to-authentication-matching branch from 4fcdacc to 4951aca Compare November 29, 2023 11:24
@sermio-te
Copy link
Contributor Author

Thank you for submitting this PR, could you please add tests demonstrating the bug? 🙏

From your PR description I totally understand, but I'd like to have automated tests for the fix, thanks!

Thanks for you prompt response. I added unit tests to cover/verify the new code.

@mdelapenya mdelapenya self-assigned this Nov 29, 2023
@mdelapenya mdelapenya added the bug An issue with the library label Nov 29, 2023
@sermio-te sermio-te changed the title Fallback matching of registry authentication config per image fix: fallback matching of registry authentication config Nov 29, 2023
Copy link
Collaborator

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for this contribution, much appreciated!!

@mdelapenya mdelapenya merged commit 5046458 into testcontainers:main Nov 29, 2023
112 checks passed
@sermio-te sermio-te deleted the registry-to-authentication-matching branch November 29, 2023 20:17
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
bug An issue with the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants