Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the Docker registry handler’s OIDC credential storage to use the shared oidc.OIDCRegistry type (introduced in #78) instead of a per-handler map+mutex, as part of the phased migration to prevent OIDC credential collisions (#87).
Changes:
- Replace
oidcCredentials map + RWMutexwithoidcRegistry *oidc.OIDCRegistryinDockerRegistryHandler. - Register OIDC credentials via
oidcRegistry.Register(...)during handler construction. - Authenticate requests via
oidcRegistry.TryAuth(...)instead ofTryAuthOIDCRequestWithPrefix(...).
internal/handlers/docker_registry.go
Outdated
| if _, _, ok := handler.oidcRegistry.Register(cred, []string{"registry"}, "docker registry"); ok { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Register(...) success now continues the credentials loop, so OIDC-configured docker_registry entries will no longer be added to h.credentials for static auth fallback. Previously (with the inline CreateOIDCCredential block) the handler registered OIDC and still appended username/password creds, allowing fallback when OIDC token acquisition fails. If the intent is a pure refactor/no behavior change, remove this continue (or only skip static registration when no static fields are present).
kbukum1
force-pushed
the
kamil/oidc-migrate-docker
branch
from
72928b2 to
9b6cfa7
Compare
Replace manual OIDC credential map and mutex with the shared OIDCRegistry type. Docker already used the raw registry value as the key, so this is a pure structural refactor with no behavior change.
kbukum1
force-pushed
the
kamil/oidc-migrate-docker
branch
from
9b6cfa7 to
e19c62a
Compare
What
Migrate the Docker registry handler from manual OIDC credential map + mutex to the shared
OIDCRegistrytype introduced in #78.Why
Part of the phased migration to fix OIDC credential collisions when multiple registries share a host (#87).
Docker already stored the raw
registryvalue as the OIDC key, so this is a pure structural refactor with no behavior change. Single-file diff, no test changes.Behavior changes
Credential selection is now deterministic. The old code iterated over a Go map (
map[string]*OIDCCredential), so with multiple OIDC credentials on the same host, which one matched was nondeterministic.OIDCRegistry.TryAuthuses longest path-prefix matching, ensuring the most specific credential always wins. This is the core fix for OIDC credential collision when multiple registries share a host #87.Host matching uses
strings.ToLowerinstead of IDNA normalization. The oldTryAuthOIDCRequestWithPrefixusedhelpers.AreHostnamesEqual(IDNAToASCII), whileOIDCRegistry.TryAuthuses lowercase comparison. This is acceptable because all real OIDC registries (Azure DevOps, JFrog, AWS CodeArtifact, Cloudsmith) use ASCII hostnames — no package registry uses internationalized domain names.