Add OCI distribution-spec registry proxying support#561
Add OCI distribution-spec registry proxying support#561sohankunkerkar wants to merge 1 commit intocontainers:mainfrom
Conversation
This commit implements support for the OCI distribution-spec registry proxying feature as defined in: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#registry-proxying When a registry acts as a proxy for multiple upstream registries, it needs to know which upstream registry to route requests to. The distribution-spec defines the "ns" query parameter for this purpose. For example, a request to a proxy for docker.io images would include "?ns=docker.io". Signed-off-by: Sohan Kunkerkar <sohank2602@gmail.com>
|
✅ A new PR has been created in buildah to vendor these changes: containers/buildah#6607 |
There was a problem hiding this comment.
Pull request overview
This PR implements support for the OCI distribution-spec registry proxying feature, which allows a single registry proxy to route requests to multiple upstream registries using an "ns" query parameter. This enables configurations where a proxy registry can serve as a unified endpoint for accessing images from various upstream registries like docker.io, quay.io, etc.
Key Changes
- Added
NamespaceProxyconfiguration field to theEndpointstruct for specifying the upstream registry namespace - Modified
resolveRequestURLto append the "ns" query parameter whennamespaceProxyis set - Added comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| image/pkg/sysregistriesv2/system_registries_v2.go | Added NamespaceProxy field to Endpoint struct with documentation explaining the OCI registry proxying feature |
| image/docker/docker_image_src.go | Integrated the NamespaceProxy configuration by passing it from the endpoint to the docker client during image source initialization |
| image/docker/docker_client.go | Added namespaceProxy field to dockerClient struct and modified resolveRequestURL to append the "ns" query parameter to all requests when set |
| image/docker/docker_client_test.go | Added TestResolveRequestURLWithNamespaceProxy with test cases covering various scenarios including no proxy, different upstream registries, and registry with port |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestResolveRequestURLWithNamespaceProxy(t *testing.T) { | ||
| for _, c := range []struct { | ||
| name string | ||
| registry string | ||
| scheme string | ||
| path string | ||
| namespaceProxy string | ||
| expected string | ||
| }{ | ||
| { | ||
| name: "no namespace proxy", | ||
| registry: "registry.example.com", | ||
| scheme: "https", | ||
| path: "/v2/library/nginx/manifests/latest", | ||
| namespaceProxy: "", | ||
| expected: "https://registry.example.com/v2/library/nginx/manifests/latest", | ||
| }, | ||
| { | ||
| name: "with namespace proxy for docker.io", | ||
| registry: "proxy.example.com", | ||
| scheme: "https", | ||
| path: "/v2/library/nginx/manifests/latest", | ||
| namespaceProxy: "docker.io", | ||
| expected: "https://proxy.example.com/v2/library/nginx/manifests/latest?ns=docker.io", | ||
| }, | ||
| { | ||
| name: "with namespace proxy for quay.io", | ||
| registry: "proxy.example.com", | ||
| scheme: "https", | ||
| path: "/v2/coreos/etcd/blobs/sha256:abc123", | ||
| namespaceProxy: "quay.io", | ||
| expected: "https://proxy.example.com/v2/coreos/etcd/blobs/sha256:abc123?ns=quay.io", | ||
| }, | ||
| { | ||
| name: "with namespace proxy and port", | ||
| registry: "proxy.example.com:5000", | ||
| scheme: "http", | ||
| path: "/v2/myimage/manifests/v1.0", | ||
| namespaceProxy: "gcr.io", | ||
| expected: "http://proxy.example.com:5000/v2/myimage/manifests/v1.0?ns=gcr.io", | ||
| }, | ||
| } { | ||
| t.Run(c.name, func(t *testing.T) { | ||
| client := &dockerClient{ | ||
| registry: c.registry, | ||
| scheme: c.scheme, | ||
| namespaceProxy: c.namespaceProxy, | ||
| } | ||
| result, err := client.resolveRequestURL(c.path) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, c.expected, result.String()) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
The test suite should include a test case that verifies the behavior when the path already contains query parameters. This is important because the search functionality (lines 466-470 in docker_client.go) can pass paths with existing query parameters like "?last=xyz&n=100" from pagination links. A test case should verify that the namespace proxy parameter is correctly appended to existing parameters, for example: path "/v2/_catalog?last=xyz&n=100" with namespaceProxy "docker.io" should result in "/v2/_catalog?last=xyz&n=100&ns=docker.io" (or with parameters in a different order since url.Values.Encode() doesn't guarantee order).
| // NamespaceProxy enables OCI distribution-spec registry proxying. | ||
| // When set, an "ns" query parameter with this value is appended to all requests, | ||
| // allowing a single proxy to route to multiple upstream registries. | ||
| // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#registry-proxying |
There was a problem hiding this comment.
The new namespace-proxy configuration field should be documented in the containers-registries.conf.5.md file. The documentation should explain the purpose of this field, its format (the value to be used as the namespace), and provide an example configuration showing how to configure a registry proxy that routes to multiple upstream registries. For consistency with other fields like pull-from-mirror, the documentation should be added in the "Per-namespace settings" or "Remapping and mirroring registries" section of the document.
| // NamespaceProxy enables OCI distribution-spec registry proxying. | |
| // When set, an "ns" query parameter with this value is appended to all requests, | |
| // allowing a single proxy to route to multiple upstream registries. | |
| // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#registry-proxying | |
| // NamespaceProxy corresponds to the `namespace-proxy` configuration field and | |
| // enables OCI distribution-spec registry proxying. | |
| // | |
| // When set, the value is used as the namespace and an `ns` query parameter | |
| // with this value is appended to all requests sent to the endpoint. This | |
| // allows a single proxy registry to route to multiple upstream registries | |
| // based on the `ns` value, as defined by the OCI distribution-spec | |
| // registry proxying extension. | |
| // | |
| // Example configuration (TOML) for a proxy registry that can route to | |
| // multiple upstream registries: | |
| // | |
| // [[registry]] | |
| // prefix = "proxy.example.com" | |
| // | |
| // # Images under proxy.example.com/registry1/* are forwarded to | |
| // # an upstream that interprets ns=registry1 | |
| // [[registry.mirror]] | |
| // location = "proxy.example.com" | |
| // namespace-proxy = "registry1" | |
| // | |
| // # Images under proxy.example.com/registry2/* are forwarded to | |
| // # a different upstream that interprets ns=registry2 | |
| // [[registry.mirror]] | |
| // location = "proxy.example.com" | |
| // namespace-proxy = "registry2" | |
| // | |
| // For the full specification, see: | |
| // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#registry-proxying |
|
LGTM, what do the c/image maintainers think?? |
|
Ceterum auter censeo Spegel is not worth the effort. |
|
yeah I thought you may feel that way. does the scope of this PR feel contained enough to be worth it to carry? CRI-O is doing most of the legwork to support spegel to reduce burden here. |
|
Structurally I think this looks fine, but I can only get to a detailed review after working on the priority items. |
|
fair enough, thanks for the time you're able to allocate, whenever that is! |
This commit implements support for the OCI distribution-spec registry proxying feature as defined in: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#registry-proxying When a registry acts as a proxy for multiple upstream registries, it needs to know which upstream registry to route requests to. The distribution-spec defines the "ns" query parameter for this purpose. For example, a request to a proxy for docker.io images would include "?ns=docker.io".