`helmCharts` repo field doesn't work with OCI registry

QuinnBast commented 2 years ago

Describe the bug

Kustomize fails to pull charts through helmCharts yaml that are hosted within an OCI registry.

Example

Create a kustomize.yaml file:

helmCharts:
  - name: someName
    includeCRDs: true
    valuesFile: values.yaml
    releaseName: myRelease
    version: 0.0.26
    repo: oci://myRegistry

Run the command kustomize build --enable-helm --helm-command $helmCommand ./folder

Kustomize will fail to pull the helm chart and produce the following error:

Error: Error: looks like "oci://myRegistry" is not a valid chart repository or cannot be reached: object required
: unable to run: '/usr/sbin/helm pull --untar --untardir <someDir> --repo oci://myRegistry myRelease --version 0.0.26' with env=[HELM_CONFIG_HOME=/tmp/kustomize-helm-991395685/helm HELM_CACHE_HOME=/tmp/kustomize-helm-991395685/helm/.cache HELM_DATA_HOME=/tmp/kustomize-helm-991395685/helm/.data] (is '/usr/sbin/helm' installed?)

Running a regular helm pull works fine on the registry, but pulling with --repo causes issues:

# Working
$ helm pull oci://myRegistry/myRelease --version 0.0.26
Pulled: myRegistry/myRelease:0.0.26
Digest: sha256:14e1bc1ec1d0147eb8b41f081faae2ecea63e862ad4259d3a9cd1c7d1584be63

# Not working with `--repo`
$ helm pull myRelease --version 0.0.26 --repo myRegistry
Error: could not find protocol handler for: 

$ helm pull myRelease --version 0.0.26 --repo oci://myRegistry
Error: looks like "oci://myRegistry" is not a valid chart repository or cannot be reached: object required

I also cannot work around this by just adding it as a repo:

$ helm repo add reg myRegistry
Error: could not find protocol handler for: 

$ helm repo add reg oci://myRegistry
Error: looks like "oci://myRegistry" is not a valid chart repository or cannot be reached: object required

Expected output

Expected Kustomize to be able to pull the chart and apply the values.yaml to the chart. The solution would be for oci:// urls, to not use the --repo flag when performing a helm pull.

Kustomize version

$ kustomize version {Version:kustomize/v4.3.0 GitCommit:cd17338759ef64c14307991fd25d52259697f1fb BuildDate:2021-08-24T19:24:28Z GoOs:linux GoArch:amd64}

$ helm version version.BuildInfo{Version:"v3.7.2", GitCommit:"663a896f4a815053445eec4153677ddc24a0a361", GitTreeState:"clean", GoVersion:"go1.16.10"}

Platform

Ubuntu

KlavsKlavsen commented 2 years ago

Helm 3.8.0 was just released with the new OCI support moved to 'production' status - this implements the changes that was done in 3.7.1 as stable (no longer experiemental env needed to use it in Helm) :)

QuinnBast commented 2 years ago

I upgraded to helm 3.8.0 and am getting the same error. The use of --repo is problematic for oci registries.

$ helm version
version.BuildInfo{Version:"v3.8.0", GitCommit:"d14138609b01886f544b2025f5000351c9eb092e", GitTreeState:"clean", GoVersion:"go1.17.5"}

$ kustomize build --enable-helm ./test
Error: Error: looks like "oci://myRegistry" is not a valid chart repository or cannot be reached: object required
: unable to run: 'helm pull --untar --untardir <someDir> --repo oci://myRegistry myRelease --version 0.0.26' with env=[HELM_CONFIG_HOME=/tmp/kustomize-helm-991395685/helm HELM_CACHE_HOME=/tmp/kustomize-helm-991395685/helm/.cache HELM_DATA_HOME=/tmp/kustomize-helm-991395685/helm/.data] (is 'helm' installed?)

Can pull without specifying --repo:

$ helm pull oci://myRegistry --version 0.0.26
Pulled: myRegistry/myRelease:0.0.26
Digest: sha256:14e1bc1ec1d0147eb8b41f081faae2ecea63e862ad4259d3a9cd1c7d1584be63

Cannot pull with --repo

$ helm pull myRelease --version 0.0.26 --repo myRegistry
Error: could not find protocol handler for: 

$ helm pull myRelease --version 0.0.26 --repo oci://myRegistry
Error: looks like "oci://myRegistry" is not a valid chart repository or cannot be reached: object required

This seems to be the issue. Maybe if the repo is oci:// you can make this not use the --repo flag?

QuinnBast commented 2 years ago

I am unable to make a MR because I can't sign the CLA but can someone make this MR for me?

This line should be:

func (p *HelmChartInflationGeneratorPlugin) pullCommand() []string {
    args := []string{
        "pull",
        "--untar",
        "--untardir", p.absChartHome(),
-               "--repo", p.Repo
-       p.Name}
+             }
    if p.Version != "" {
        args = append(args, "--version", p.Version)
    }
+   if strings.HasPrefix(p.Repo, "oci://") {
+           args = append(args, p.Repo)
+   } else {
+              args = append(args, "--repo", p.Repo)
+              args = append(args, p.Name)
+       }
    return args
}

This is untested, but --repo should not be in the pull command if oci:// is specified. If oci:// is specified, just do a straight helm pull

tis-rpage commented 2 years ago

If anyone takes the above patch, I think they should note the trailing comma needs fixing:

-       "--untardir", p.absChartHome(),
+       "--untardir", p.absChartHome()

I am unable to make a MR because I can't sign the CLA but can someone make this MR for me?

This line should be:
func (p *HelmChartInflationGeneratorPlugin) pullCommand() []string {
  args := []string{
      "pull",
      "--untar",
      "--untardir", p.absChartHome(),
-               "--repo", p.Repo
-     p.Name}
+             }
  if p.Version != "" {
      args = append(args, "--version", p.Version)
  }
+ if strings.HasPrefix(p.Repo, "oci://") {
+         args = append(args, p.Repo)
+ } else {
+              args = append(args, "--repo", p.Repo)
+              args = append(args, p.Name)
+       }
  return args
}
This is untested, but --repo should not be in the pull command if oci:// is specified. If oci:// is specified, just do a straight helm pull

minnie-jeong-otsk commented 2 years ago

It'd be greatly appreciated if this fix is deployed soon!

tis-rpage commented 2 years ago

To be fair, this work is required because the Helm team doesn't follow SemVer with their tooling. They made a minor release with major CLI interface changes, which kustomize should not be responsible for fixing. If you read the comments, there appears to be a larger SIG working group discussion to evaluate how to properly handle Helm longer term, which I suspect this behavior will be addressed from that working group.

QuinnBast commented 2 years ago

@minnie-jeong-otsk I created a patch here and also published a release for this patch on my fork of kustomize (download the patched binary here). This patch works for us to pull oci:// registries. I will be submitting a MR to kustomize once my company allows us to sign the CLA.

Note that this release is NOT officially supported by kustomize.

natasha41575 commented 2 years ago

The Helm plugin builtin into kustomize is being migrated to a third party KRM extension, in which we will support OCI [issue]. I will try to prioritize the migration and make some progress for OCI support in Q2 given that there are a large number of requests for this.

/triage accepted

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

QuinnBast commented 2 years ago

/remove-lifecycle stale

QuinnBast commented 2 years ago

Based off of this comment, it appears that the helm chart inflation tool is supporting bug fixes and adding fields that are normally supported through helm template. That would make this issue and #4335 valid and would not be waiting for the new KRM functionality. Is there any priority on these two issues which already have a fix?

Edit: Never-mind... I didn't read the "we will not support OCI or private registries"... That is sad. I would consider being incompatible with Helm 3.8.0 features (which has been released for almost a year now) a bug...

elamcheliyana commented 2 years ago

I have tried the issue is still present. Does any one when they would consider to fix this problem or it going to be never?

igstbagusdharmaputra commented 2 years ago

will this problem be solved soon?

LelvProgrammer commented 2 years ago

Bump

85matthew commented 1 year ago

Any reason we can't reconsider one of the other pull requests that address this as a short term solution while the long term vision is still being planned?

https://github.com/kubernetes-sigs/kustomize/pull/4614/files

sthomson-wyn commented 1 year ago

There are PRs to fix this problem in kustomize. KRM is touted as the eventual solution, but has not had a commit in over 9 months...

antonydevanchi commented 1 year ago

@mikebz i'll dare you free blowjob if you close this shit, dude >_<

kosta commented 1 year ago

While it is stated under Long term support that

We will not add support for:

OCI registries

I would kindly ask you to reconsider this opinion, given that the fix actually is just +7-2 LOC and the krm function does not support this yet, either and is not production-ready it seems.

antonydevanchi commented 1 year ago

@kosta nevertheless it uses widely by Yandex.Cloud: site:cloud.yandex.ru "oci:" for example.

mikebz commented 1 year ago

I think there is a concern about backwards compatibility here, but the backwards compatibility issue was a side effect of the previous implementation and not an advertised feature. My vote is to move forward with the proposed PR.

andrzejgorski commented 1 year ago

As another ugly workaround, I found the solution: Running kustomize with --helm-command ~/tmp/sanitized_helm.sh

where ~/tmp/sanitized_helm.sh is:

#!/bin/bash
if [[ $@ = pull* ]]; then
    # If the command is `helm pull (..)` skips --repo flag and chartName
    # from command line args to make helm pull run

    # For explanation:
    # https://github.com/kubernetes-sigs/kustomize/issues/4381
    arr=(${@//--repo/});  # Skipping --repo
    args="${arr[@]:0:5} ${arr[@]:6}";  # Skipping chartName
else
    args="$@"
fi
cmd="/usr/bin/helm --registry-config ~/.config/helm/registry/config.json $args"
echo "Running '$cmd' " >> /tmp/helm-debug
eval $cmd

SachithKasthuriarachchi commented 1 year ago

any update on this issue? this will be a great feature for kustomize and it will expand kustomize's adaptations

diegoluisi commented 1 year ago

Up!

QuinnBast commented 1 year ago

No, it's not in yet.

This MR is not against the actual kustomize repository; it will only get in after #5167 is approved by @natasha41575 and a release is created that contains the MR

annasong20 commented 1 year ago

@natasha41575 I'd like to check my understanding of this issue.

Initially, we planned to support this patch in the KRM Helm function, which would've had increased feature-freedom because of its independence from Kustomize. When progress on KRM functions slowed, we limited HelmChartInflationGenerator changes to lower maintenance costs.

Just to confirm, we are still open to this patch on HelmChartInflationGenerator due to its popularity?

annasong20 commented 1 year ago

@natasha41575 @QuinnBast or anyone else on this thread: Stemming from my lack of understanding of Helm, is Helm's inability to pull charts with the --repo flag a bug on their end? By adding the changes in #5167,

are we patching a Helm bug or
is the --repo behavior intentional and the #5167 changes help Kustomize HelmChartInflationGenerator reflect new Helm behavior in relation to OCI

@natasha41575 If this is a Helm bug, I assume we're patching it in Kustomize instead of Helm as a short-term workaround for users?

natasha41575 commented 1 year ago

@annasong20 discussed offline - we will add support in kustomize for oci registries by having the different commands running that helm provides for oci-based charts.

pratik705 commented 5 months ago

is this fixed? I still cant use OCI registry with kustomize

mrclrchtr commented 5 months ago

Yes it is working. I use it like this::

helmCharts:
  - name: spegel
    repo: oci://ghcr.io/spegel-org/helm-charts
    releaseName: spegel
    namespace: spegel
    version: v0.0.23
    valuesFile: values.yaml

A few days ago I thought it wasn't working either, until I found out that I had entered the wrong repo URL 🤦 When you do that, you get an auth error that sends you on the wrong track.

kubernetes-sigs / kustomize

`helmCharts` repo field doesn't work with OCI registry #4381