Add ability to configure registry-specific proxy in `registries.conf` #650

cyqsimon · 2026-02-10T06:03:22Z

Closes #624.

Checklist

impl
tests
docs

mtrmac

https://github.com/containers/container-libs/blob/main/CONTRIBUTING.md#sign-your-prs please, we can’t really even look at code with unclear copyright status.

cyqsimon · 2026-02-10T17:29:03Z

https://github.com/containers/container-libs/blob/main/CONTRIBUTING.md#sign-your-prs please, we can’t really even look at code with unclear copyright status.

Sorry about that. Was planning to do an interactive rebase when things are ready, so I didn't bother.

Will fix tomorrow.

packit-as-a-service · 2026-02-11T12:54:26Z

Packit jobs failed. @containers/packit-build please check.

packit-as-a-service · 2026-02-11T12:54:26Z

Packit jobs failed. @containers/packit-build please check.

Signed-off-by: cyqsimon <[email protected]>

cyqsimon · 2026-02-11T13:29:48Z

Still missing a test in docker_image_src_test.go to test mirrors' proxy config. I'll work on that tomorrow.

mtrmac

Thanks!

I agree this makes sense to do for Podman accessed through the remote API, and the config file addition looks good.

mtrmac · 2026-02-11T18:16:08Z

image/pkg/sysregistriesv2/system_registries_v2.go


+// ParseProxy parses the input string for a proxy configuration.
+// Errors if a scheme is unsupported or unspecified, or if the input is not a valid URL.
+func ParseProxy(input string) (*url.URL, error) {


After other changes, this can be package-private — we probably don’t want to commit to this as a public API.

mtrmac · 2026-02-11T18:17:10Z

image/pkg/sysregistriesv2/system_registries_v2.go

+
+	var hasSupportedScheme bool
+	for _, scheme := range []string{"http://", "https://", "socks5://", "socks5h://"} {
+		if strings.HasPrefix(input, scheme) {


It seems more accurate to first do the url.Parse and then consult the schema in the parsed output.

mtrmac · 2026-02-11T18:21:30Z

image/pkg/sysregistriesv2/system_registries_v2.go

 			return err
 		}

+		if _, err = ParseProxy(reg.Proxy); err != nil {


It is a bit wasteful to parse the value into a full URL, and discard it only to re-parse later.

Adding a ProxyURL *net.URL field with a toml:"-" annotation to Endpoint could avoid that.

mtrmac · 2026-02-11T18:23:59Z

image/pkg/sysregistriesv2/system_registries_v2.go

 			return err
 		}

+		if _, err = ParseProxy(reg.Proxy); err != nil {


Please keep the validation somewhat in field order; in particular, do not add this into the middle of the part that deals with Prefix/Location.

Also elsewhere.

mtrmac · 2026-02-11T18:27:01Z

image/docker/docker_client.go

-	// Check if TLS verification shall be skipped (default=false) which can
-	// be specified in the sysregistriesv2 configuration.
-	skipVerify := false
+	// Fetch and load sysregistriesv2 configurations.


Please avoid comments that add nothing of value (compare WIP #636 )

mtrmac · 2026-02-11T18:33:04Z

image/docker/docker_client_test.go

+	testProxyForRegistry(t, ctx, sys, "registry-1.com", "")
+	testProxyForRegistry(t, ctx, sys, "registry-2.com", "https://proxy-2.example.com")


Unit tests contacting random public URLs is rather undesirable.

One option would be to start a few httptest servers and differentiate them via URLs.

At the very least, there is RFC 2606.

mtrmac · 2026-02-11T18:34:49Z

image/docker/docker_client.go

 	tlsClientConfig *tls.Config
+	// registryProxy is the proxy URL from the registry configuration, if any.
+	// It has the lowest priority and can be overridden by either DockerProxyURL or environment variables.
+	// When pulling, this value could be overwritten by a mirror-specific proxy. See docker_client_src.go.


When pulling, this value could be overwritten by a mirror-specific proxy. See docker_client_src.go.

That’s true but a knowledge this lower layer doesn’t really need to have, and the comment doesn’t really say how this is intended to work.

I rather prefer the form above, which explicitly lists newDockerClient/detectProperties and when the callers are allowed to edit the value.

mtrmac · 2026-02-11T18:39:22Z

image/pkg/sysregistriesv2/system_registries_v2_test.go

+
+	registries, err := GetRegistries(sys)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(registries))


(Non-blocking: I suspect right now all of this could be a single assert.Equal against a literal, instead of 10 individual checks. But add the *net.URL field first before trying that transformation.)

mtrmac · 2026-02-11T18:44:25Z

image/docker/docker_client_test.go

 }
+
+// Helper function to test that the selected proxy for a registry matches expected.
+func testProxyForRegistry(t *testing.T, ctx context.Context, sys *types.SystemContext, registry string, expectedProxy string) {


(Non-blocking: This can be inlined into the caller, using a table-driven test.)

mtrmac · 2026-02-11T18:45:33Z

image/pkg/sysregistriesv2/system_registries_v2.go

 	// connections will be allowed.
 	Insecure bool `toml:"insecure,omitempty"`
+	// The forwarding proxy to be used for accessing this endpoint.
+	Proxy string `toml:"proxy,omitempty"`


This will need documentation in image/docs/containers-registries.conf.5.md.

github-actions bot added the image Related to "image" package label Feb 10, 2026

mtrmac reviewed Feb 10, 2026

View reviewed changes

cyqsimon force-pushed the registries-proxy-config branch from 27a5643 to f82f918 Compare February 11, 2026 12:51

cyqsimon added 2 commits February 11, 2026 21:01

Registries proxy config: initial impl

4278ca8

Signed-off-by: cyqsimon <[email protected]>

Registries proxy config: add tests

bc60b07

Signed-off-by: cyqsimon <[email protected]>

cyqsimon force-pushed the registries-proxy-config branch from f82f918 to bc60b07 Compare February 11, 2026 13:01

mtrmac reviewed Feb 11, 2026

View reviewed changes

		testProxyForRegistry(t, ctx, sys, "registry-1.com", "")
		testProxyForRegistry(t, ctx, sys, "registry-2.com", "https://proxy-2.example.com")

Add ability to configure registry-specific proxy in registries.conf #650

Are you sure you want to change the base?

Add ability to configure registry-specific proxy in registries.conf #650

Conversation

cyqsimon commented Feb 10, 2026

Checklist

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

cyqsimon commented Feb 10, 2026

Uh oh!

packit-as-a-service bot commented Feb 11, 2026

Uh oh!

packit-as-a-service bot commented Feb 11, 2026

Uh oh!

cyqsimon commented Feb 11, 2026

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add ability to configure registry-specific proxy in `registries.conf` #650

Add ability to configure registry-specific proxy in `registries.conf` #650