Skip to content

Conversation

@LaurentGoderre
Copy link
Contributor

What does this PR do?

When doing a Docker pull on an image with the default registry docker.io, the auth helper doesn't match the image to the authentication data stored by the engine under https://index.docker.io/v1 causing the error: credentials not found in native keychain

Why is it important?

This is important to support the default registry prefix.

How to test this PR

Unit test added

@LaurentGoderre LaurentGoderre requested a review from a team as a code owner November 3, 2025 15:41
@netlify
Copy link

netlify bot commented Nov 3, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 00cbc7d
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/69120405b938400008e1a895
😎 Deploy Preview https://deploy-preview-3482--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 project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Nov 3, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Docker image authentication to properly normalize registry aliases (docker.io and registry.hub.docker.com) to the configured default registry, ensuring credentials are correctly matched for image operations.
  • Tests

    • Added test coverage verifying default registry authentication is correctly matched by host.

Walkthrough

The Docker image auth resolver now normalizes docker.io and registry.hub.docker.com to the configured default registry before credential lookup. A new unit sub-test verifies that DockerImageAuth returns the default registry's credentials for an image hosted at docker.io.

Changes

Cohort / File(s) Summary
Docker auth implementation
docker_auth.go
Normalize image host aliases (docker.io, registry.hub.docker.com, case-insensitive) to the configured default registry prior to credential lookup.
Docker authentication tests
docker_auth_test.go
Added sub-test "match the default registry authentication by host" in TestDockerImageAuth that sets a default registry, injects credentials, calls DockerImageAuth for docker.io/my/image:latest, and asserts registry, username, password, and encoded Auth.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant AuthLayer as Auth Resolver
    participant CredStore as Credential Store

    rect rgb(245,250,255)
    Client->>AuthLayer: Request auth for "docker.io/my/image:latest"
    note right of AuthLayer: Normalize host aliases to default registry
    AuthLayer->>AuthLayer: if host in {"docker.io","registry.hub.docker.com"} then host = defaultRegistry()
    end
    AuthLayer->>CredStore: Lookup credentials for remapped host
    CredStore-->>AuthLayer: Credentials (username, password, auth)
    AuthLayer-->>Client: Return registry, username, password, encoded Auth
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify host alias matching is case-insensitive and covers both docker.io and registry.hub.docker.com.
  • Check handling of an empty or unset default registry and any trailing slash/port edge cases.
  • Ensure the test sets/restores global/default registry state and that encoded Auth comparisons match production encoding.

Poem

🐰 I hopped from host to host with curious cheer,
docker.io became my default registry here,
I fetched the creds, checked username and key,
Encoded and returned — a rabbit's decree. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main fix: resolving Docker authentication for docker.io images by normalizing registry aliases.
Description check ✅ Passed The description clearly explains the problem (auth helper mismatch with docker.io), its importance, and how it was tested, all directly related to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df49f93 and 00cbc7d.

📒 Files selected for processing (1)
  • docker_auth.go (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • docker_auth.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: lint (modules/milvus) / lint: modules/milvus
  • GitHub Check: lint (modules/mockserver) / lint: modules/mockserver
  • GitHub Check: lint (modules/dockermodelrunner) / lint: modules/dockermodelrunner
  • GitHub Check: Analyze (go)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@LaurentGoderre LaurentGoderre changed the title fix docker auth for docker.io images fix: docker auth for docker.io images Nov 3, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf4458c and df49f93.

📒 Files selected for processing (2)
  • docker_auth.go (1 hunks)
  • docker_auth_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • docker_auth_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: lint (modules/dynamodb) / lint: modules/dynamodb
  • GitHub Check: lint (modulegen) / lint: modulegen
  • GitHub Check: lint (modules/dind) / lint: modules/dind
  • GitHub Check: lint (modules/registry) / lint: modules/registry
  • GitHub Check: lint (modules/dockermodelrunner) / lint: modules/dockermodelrunner
  • GitHub Check: lint (modules/postgres) / lint: modules/postgres
  • GitHub Check: lint (modules/qdrant) / lint: modules/qdrant
  • GitHub Check: lint (modules/pinecone) / lint: modules/pinecone
  • GitHub Check: lint (modules/redpanda) / lint: modules/redpanda
  • GitHub Check: lint (modules/solace) / lint: modules/solace
  • GitHub Check: lint (modules/memcached) / lint: modules/memcached
  • GitHub Check: lint (modules/openfga) / lint: modules/openfga
  • GitHub Check: lint (modules/pulsar) / lint: modules/pulsar
  • GitHub Check: lint (modules/artemis) / lint: modules/artemis
  • GitHub Check: lint (modules/influxdb) / lint: modules/influxdb
  • GitHub Check: lint (modules/rabbitmq) / lint: modules/rabbitmq
  • GitHub Check: lint (modules/dolt) / lint: modules/dolt
  • GitHub Check: lint (modules/mariadb) / lint: modules/mariadb
  • GitHub Check: lint (modules/consul) / lint: modules/consul
  • GitHub Check: Analyze (go)

Comment on lines +46 to +50
// Make comparison case-insensitive and handle both aliases
if strings.EqualFold(reg, "docker.io") || strings.EqualFold(reg, "registry.hub.docker.com") {
reg = defaultRegistry
}

Copy link
Contributor

Choose a reason for hiding this comment

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

question: should this be handled by core.ExtractRegistry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could. Up to y'all

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should live there, @mdelapenya thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, let's move it there, and once we use the go-sdk, it will be resolved and easier to replace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants