feat(acr): serve the Entra token exchange on {name}.azurecr.io - #299
Open
driverog wants to merge 5 commits into
Open
feat(acr): serve the Entra token exchange on {name}.azurecr.io#299driverog wants to merge 5 commits into
driverog wants to merge 5 commits into
Conversation
az acr login and the Azure SDK container-registry clients do not use a plain
Docker login: they trade an Entra access token for a registry refresh token,
and that for a scoped access token, over endpoints a stock registry:2 does
not have. Both steps build their URLs by concatenating onto loginServer, so
the login server has to be a host rather than a host plus a path.
AcrHandler now claims the .azurecr.io host suffix beside its provider
namespace, the way KeyVaultHandler claims .vault.azure.net, and serves the
bearer challenge on GET /v2/ plus /oauth2/exchange and /oauth2/token there.
Repository requests are proxied to the shared registry:2 container with the
registry name applied as an internal repository prefix, stripped again from
upload Location headers, tags/list and _catalog. A host naming a registry
nobody created answers NAME_UNKNOWN rather than minting tokens for it: a
wildcard resolver points the whole .azurecr.io space at the emulator, but
only created registries exist. The challenge realm carries the scheme the
request arrived with, so it does not point at a TLS port that is not
listening when TLS is off.
The proxy forwards the client's raw query rather than rebuilding it from the
decoded parameters, which does not round-trip the opaque _state registry:2
mints for an upload session; AzureRequest carries rawQuery for that, captured
in the routing filter next to rawPath.
The container's published port is unchanged and still serves the same
storage anonymously over plain HTTP. Since loginServer no longer carries
that port, the registry resource reports it as properties.localPort, the
field PostgreSQL and MySQL servers already use for the same purpose.
Tokens are issued, not verified, and the scopes they carry are not enforced;
both are documented as intentional deviations, alongside the name resolution
that {name}.azurecr.io now requires.
Note for anyone reading loginServer: it is now {name}.azurecr.io in both
mocked and real mode, instead of the path-style localhost:{port}/{name}.
Image references through the published port keep working unchanged, but a
client that reads loginServer now gets an Azure host name, which needs name
resolution and trust in the emulator's certificate. Deliberately recorded
here as prose rather than a BREAKING CHANGE footer: the emulator is pre-1.0
and this should not spend the 1.0.0 release.
|
🎉 Thanks for your first pull request to floci-az! Your CI checks need a maintainer to approve them before they run. That is GitHub's standard gate on first-time contributors, not a problem with your PR — so if the checks look like they are doing nothing, that is why. Once a maintainer approves, CI and the compatibility suite start automatically. Nothing is needed from you in the meantime. While you wait, a couple of things that make review faster:
Come join us in Slack — it is the fastest way to reach maintainers if you get stuck, or want feedback on an approach before investing more time in it. |
driverog
marked this pull request as ready for review
September 12, 2026 01:49
|
| Filename | Overview |
|---|---|
| src/main/java/io/floci/az/services/acr/AcrHandler.java | Routes Azure-hosted ACR requests, emits bearer challenges, and delegates token and repository operations. |
| src/main/java/io/floci/az/services/acr/AcrTokenService.java | Implements ACR refresh-token and scoped access-token protocol responses using the shared token issuer. |
| src/main/java/io/floci/az/services/acr/AcrRegistryProxy.java | Proxies registry traffic with tenant prefixes, streaming bodies, opaque query preservation, and translated catalog pagination. |
| src/main/java/io/floci/az/core/FormBody.java | Provides shared form decoding and now safely treats malformed percent-encoded pairs as absent. |
| src/main/java/io/floci/az/core/AzureRoutingFilter.java | Captures raw query strings so opaque registry upload state can be forwarded without re-encoding. |
| src/main/java/io/floci/az/core/tls/TlsConfigSource.java | Adds wildcard certificate coverage for Azure Container Registry hostnames. |
Sequence Diagram
sequenceDiagram
participant Client as Azure CLI / OCI Client
participant ACR as floci-az ({name}.azurecr.io)
participant Token as ACR Token Service
participant Registry as Shared registry:2
Client->>ACR: GET /v2/
ACR-->>Client: 401 Bearer challenge
Client->>ACR: POST /oauth2/exchange
ACR->>Token: Mint refresh token
Token-->>Client: RS256 refresh token
Client->>ACR: POST /oauth2/token (scope)
ACR->>Token: Mint scoped access token
Token-->>Client: RS256 access token
Client->>ACR: "/v2/{repo}/..."
ACR->>Registry: "/v2/{registry}/{repo}/..."
Registry-->>ACR: Registry response
ACR-->>Client: Prefix-translated response
Reviews (2): Last reviewed commit: "refactor(acr): fold the catalog into the..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
A percent escape that will not decode, such as `service=%zz`, made `URLDecoder.decode` throw out of `FormBody.parse`. Nothing on the way out catches it: `AzureRoutingFilter.dispatch` does not wrap the handler call and the application registers no `ExceptionMapper`. Client input therefore produced a generic 500 instead of the documented error shape. A pair that will not decode is now dropped, so the parameter reaches the endpoint as absent and the endpoint answers its own 400. This predates the ACR work: the same two lines have been in `EntraServiceHandler` all along and were moved into `FormBody` verbatim, so the Entra token endpoint is fixed with it.
…ng it A request with no `Content-Length` was read whole with `readAllBytes` before being forwarded, so a layer of unknown size was held in memory in its entirety. Docker and containerd both declare a length on a blob push, which is why the branch was written that way, but an OCI client streaming a layer it has not sized yet sends it chunked and would have been buffered. Such a body is now forwarded chunked, on the same one-shot stream the declared length path already uses. Only methods that can carry a body take that path: a bodyless GET or DELETE that simply declared no length must not acquire a chunked frame it never had.
`/v2/_catalog` filtered the shared container's catalog after the fact and
forwarded the container's `Link` untouched. That leaked internal names such as
`myreg/app` as pagination cursors, and a client that instead built `last` from
the rewritten body sent a cursor the container could not place. The page size
was applied by the container before the filter ran, so pages came back short,
and after the last of our repositories the walk kept serving empty pages.
The catalog now gets its own request shape, one backend request per page. The
container has no filter parameter, so the filter is expressed as a range: a
registry's repositories are a contiguous subtree of the container's walk, so
seeding `last` with `{registry}/` lands on the first of ours and `n` then counts
ours rather than everyone's. One repository is requested beyond the page,
because the container advertises a next page whenever the page it returned was
full rather than when results remain; that extra entry, outside the prefix,
is what says the block ended. Cursors are minted here and translated back on
the way in, so no internal name reaches a client.
The behaviour this rests on is not in the distribution spec, so
AcrCatalogPaginationDockerTest pins it against a real registry:2 with
neighbouring registries on both sides. Verified on Distribution 2.8.3, and
mutation-checked: removing the seed, the extra repository, or the stop at the
block edge each fails a test.
The catalog handler arrived with its own copy of the plumbing: the same request builder and timeout, the same header forwarding, the same send, and the same 502 in the catch, differing only in the wording of a log line. All of it now goes through one `send` helper and one `unavailable`, so the two callers read as what they are, the same transport shaped differently, and the timeout is named rather than repeated. `catalogBody` caught a `JsonProcessingException` that a map of strings cannot raise, so its fallback JSON was unreachable. It is inlined into `catalogPage`, which lets the exception reach the catch that was always going to handle it. `pageSize` folded `n=0` into the same answer as a missing `n` through a `Math.max`, so a client asking for an empty page received the entire catalog. Absent is now `UNPAGINATED` and distinct from zero, and a page of nothing is answered without a container request at all. No behaviour change beyond `n=0`. The mutation checks behind the pagination still hold: removing the seed, the extra repository, or the stop at the block edge each fails a test.
hectorvent
added
documentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
az acr loginand the Azure SDK container-registry clients cannot authenticate against the emulated registry. They do not use a plain Docker login: they trade an Entra access token for a registry refresh token at/oauth2/exchange, then that for a scoped access token at/oauth2/token. Those endpoints are ACR-specific, so the shared anonymousregistry:2container answers 404, and because it is anonymousGET /v2/returns 200 and never issues theWWW-Authenticatechallenge that starts the flow. There is no way in.The protocol here was taken from Azure CLI 2.89.1 (
command_modules/acr/_docker_utils.py) rather than from documentation, so the shapes match what the client actually sends.What changed
AcrHandlernow claims the.azurecr.iohost suffix beside its provider namespace, the wayKeyVaultHandlerclaims.vault.azure.net, and serves the bearer challenge plus both token endpoints there. Repository requests are proxied to the shared container with the registry name applied as an internal repository prefix, stripped again from uploadLocationheaders,tags/listand_catalog, so it never leaks into the client's view.*.azurecr.iois added to the generated certificate.Tokens are real RS256 JWTs minted by the existing
TokenIssuer, because the CLI decodes the access token to read itsaccessclaim.The login server moves, and why
loginServerbecomes{name}.azurecr.ioinstead of the path-stylelocalhost:{port}/{registryName}. This is the breaking part and it is deliberate.The clients build every URL by concatenating onto
loginServer, so it has to be a host, not a host plus a path. The old shape cannot carry the protocol: the CLI would look for the registry API under/{registry}/v2/while Docker looks under/v2/{registry}/, andserviceandscopewould both carry a registry name that Azure's never do.This follows what Key Vault already does.
ArmHandler.vaultUri()advertises the Azure-faithfulhttps://{name}.vault.azure.net/in the single ARM field and documents the convenient local address separately. Mocked mode already reported{name}.azurecr.io, so the two modes now agree on a value they previously disagreed about.The container's published port is unchanged.
localhost:{port}/{registry}/{repo}keeps working, anonymous, over plain HTTP, with no name resolution and no certificate trust. Both surfaces address the same storage.Intentional deviations
GET /v2/challenges; repository paths are served whether or not a token is presented.{name}.azurecr.iomust resolve to floci-az and clients must trust its certificate, the same requirement Key Vault already carries. Docker and podman need the CA atcerts.d/{host}/ca.crt.Testing
Unit tests cover scope parsing, token building, the challenge header and the repository prefixing. Protocol tests cover both endpoints, both verbs, the Docker
GETform and the error shapes, and run in mocked mode without Docker.AcrRegistryProxyHttpTestdrives the proxy against a stand-in registry over real HTTP. A newacr-login.batscase runsaz acr loginand pushes and pulls.Not covered locally: no test drives a real
docker loginand push against{name}.azurecr.io, because the machine this was developed on has no Docker socket. Certificate trust andLocationrewriting under a genuine layer upload are the first things worth exercising in review.Known nits
docs/services/acr.mdhardcodeslocalhost:5000in its examples while the container takes the first free port in the 5000-5099 range.