helm / helm

The Kubernetes Package Manager
https://helm.sh
Apache License 2.0
27.1k stars 7.13k forks source link

Error: invalid_reference: invalid tag when chart exposed as OCI url in index.yaml #13466

Open michelesr opened 3 days ago

michelesr commented 3 days ago

Running an helm command with --repo fails with invalid tag error when the selected chart is exposed as OCI url in the repository index.yaml .

$ helm template --repo https://charts.bitnami.com/bitnami external-dns --version 8.5.1 | head
---
# Source: external-dns/templates/networkpolicy.yaml
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
metadata:
  name: release-name-external-dns
  namespace: "default"
  labels:
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/managed-by: Helm

$ helm template --repo https://charts.bitnami.com/bitnami external-dns --version 8.6.0
Error: invalid_reference: invalid tag

These are the relevant index.yaml bits:

8.6.0

  external-dns:
  - name: external-dns
    urls:
    - oci://registry-1.docker.io/bitnamicharts/external-dns:8.6.0
    version: 8.6.0
    ...

8.5.1

  - name: external-dns
    urls:
    - https://charts.bitnami.com/bitnami/external-dns-8.5.1.tgz
    version: 8.5.1
    ...

Output of helm version:

version.BuildInfo{Version:"v3.16.3", GitCommit:"cfd07493f46efc9debd9cc1b02a0961186df7fdf", GitTreeState:"dirty", GoVersion:"go1.23.3"}
termdew commented 2 days ago

The problem is that the OCI path for 8.6.0 has the tag already in it. In pkg/downloader/chart_downloader.go, the function getOciURI wants to assemble an OCI URI based on the field in urls and version.

In your example, the resulting OCI URI is /bitnamicharts/external-dns:8.6.0:8.6.0, which fails obviously.

I'll try to create a small fix.

michelesr commented 2 days ago

The problem is that the OCI path for 8.6.0 has the tag already in it. In pkg/downloader/chart_downloader.go, the function getOciURI wants to assemble an OCI URI based on the field in urls and version.

In your example, the resulting OCI URI is /bitnamicharts/external-dns:8.6.0:8.6.0, which fails obviously.

I'll try to create a small fix.

Yes, I was looking at this right now, I tried to do this here:

func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *provenance.Verification, error) {
    u, err := c.ResolveChartVersion(ref, version)
    if err != nil {
        return "", nil, err
    }

    s := u.Path
    fmt.Println(s)

    parts := strings.Split(s, ":")
    parts = parts[:len(parts)-1] // remove the last part
    s = strings.Join(parts, ":")
    fmt.Println(s)

    u.Path = s

    g, err := c.Getters.ByScheme(u.Scheme)
    if err != nil {
        return "", nil, err
    }

And that fixed the problem (but obviously it's a hack, not the proper fix)

termdew commented 2 days ago

I created a pull request, but I don't know if its a pretty way to do it.. 🤔

michelesr commented 2 days ago

I created a pull request, but I don't know if its a pretty way to do it.. 🤔

Honestly, I have no idea, but I suppose it's fine as long as the right version (from --version) is used, but that should always be the case if the repository is pointing you to the right OCI url. What happens if you specify the oci uri directly with that patch, does is still work fine? I suppose if that broke the test should catch it?