From ea04cea48bf2b312e506ae03c412c6fd95929c5c Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Tue, 20 May 2025 19:05:20 -0400 Subject: [PATCH] Fix 3.18.0 regression: registry login with scheme Helm 3.18.0 released an upgrade of ORAS from v1 to v2. ORAS v2 correctly does not accept http/https scheme for registry login, while ORAS v1 previously did. Even if v1 should not have, we want to preserve backwards compatibility for Helm 3 users who pass the scheme. This will be removed in Helm 4, where registry login will not accept http/https scheme. Co-authored-by: Andrew Block Co-authored-by: Terry Howe Signed-off-by: Scott Rigby (cherry picked from commit c0f3ace52d974b7465f33079bbf54ed961f875f1) --- pkg/registry/client.go | 31 +++++++++++++++++++++++++++++++ pkg/registry/client_test.go | 21 +++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/pkg/registry/client.go b/pkg/registry/client.go index de7e636e1..2c85cd3b6 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -229,8 +229,39 @@ type ( } ) +// Deprecated: will be removed in Helm 4 +// Added for backwards compatibility for Helm < 3.18.0 after moving to ORAS v2 +// ref: https://github.com/helm/helm/issues/30873 +// TODO: document that Helm 4 `registry login` does accept full URLs +func (c *Client) stripURL(host string) string { + // strip scheme from host in URL + for _, s := range []string{"oci://", "http://", "https://"} { + if strings.HasPrefix(host, s) { + plain := strings.TrimPrefix(host, s) + if c.debug { + fmt.Fprintf(c.out, "[WARNING] Invalid registry passed: registries should NOT be prefixed with a URL scheme. Use %q instead\n", plain) + } + host = plain + break + } + } + // strip repo from registry in URL + if idx := strings.Index(host, "/"); idx != -1 { + host = host[:idx] + if c.debug { + fmt.Fprintf(c.out, "[WARNING] Invalid registry passed: registries should NOT include a repository. Use %q instead\n", host) + } + return host + } + + return host +} + // Login logs into a registry func (c *Client) Login(host string, options ...LoginOption) error { + // This is the lowest available point to strip incorrect URL parts + host = c.stripURL(host) + for _, option := range options { option(&loginOperation{host, c}) } diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 4c5a78849..dcc54c0c4 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -17,6 +17,7 @@ limitations under the License. package registry import ( + "io" "testing" "github.com/containerd/containerd/remotes" @@ -31,3 +32,23 @@ func TestNewClientResolverNotSupported(t *testing.T) { require.Equal(t, err, errDeprecatedRemote) assert.Nil(t, client) } + +func TestStripURL(t *testing.T) { + client := &Client{ + out: io.Discard, + } + // no change with supported host formats + assert.Equal(t, "username@localhost:8000", client.stripURL("username@localhost:8000")) + assert.Equal(t, "localhost:8000", client.stripURL("localhost:8000")) + assert.Equal(t, "docker.pkg.dev", client.stripURL("docker.pkg.dev")) + + // test strip scheme from host in URL + assert.Equal(t, "docker.pkg.dev", client.stripURL("oci://docker.pkg.dev")) + assert.Equal(t, "docker.pkg.dev", client.stripURL("http://docker.pkg.dev")) + assert.Equal(t, "docker.pkg.dev", client.stripURL("https://docker.pkg.dev")) + + // test strip repo from Registry in URL + assert.Equal(t, "127.0.0.1:15000", client.stripURL("127.0.0.1:15000/asdf")) + assert.Equal(t, "127.0.0.1:15000", client.stripURL("127.0.0.1:15000/asdf/asdf")) + assert.Equal(t, "127.0.0.1:15000", client.stripURL("127.0.0.1:15000")) +}