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 <andy.block@gmail.com>
Co-authored-by: Terry Howe <terrylhowe@gmail.com>

Signed-off-by: Scott Rigby <scott@r6by.com>

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})
 	}

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"))
+}