Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
QuinnBast opened this issue Jan 11, 2022 · 29 comments · Fixed by #5167
Closed

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

QuinnBast opened this issue Jan 11, 2022 · 29 comments · Fixed by #5167
Labels
area/helm kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@QuinnBast
Copy link

QuinnBast commented Jan 11, 2022

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

@QuinnBast QuinnBast added the kind/bug Categorizes issue or PR as related to a bug. label Jan 11, 2022
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jan 11, 2022
@QuinnBast QuinnBast changed the title helmCharts repo field doesn't work with OCI protocol helmCharts repo field doesn't work with OCI registry Jan 11, 2022
@KlavsKlavsen
Copy link

KlavsKlavsen commented Jan 27, 2022

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
Copy link
Author

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
Copy link
Author

QuinnBast commented Mar 22, 2022

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
Copy link

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

QuinnBast added a commit to QuinnBast/kustomize that referenced this issue Apr 4, 2022
These fixes update the HelmChartInflation tool so that it works with Helm 3
These changes are related to issues kubernetes-sigs#4381 and kubernetes-sigs#4335 on the kustomize github
@minnie-jeong-otsk
Copy link

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

@tis-rpage
Copy link

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
Copy link
Author

QuinnBast commented Apr 5, 2022

@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
Copy link
Contributor

natasha41575 commented Apr 8, 2022

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-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 8, 2022
@mikebz mikebz mentioned this issue May 2, 2022
3 tasks
@k8s-triage-robot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 7, 2022
@QuinnBast
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 13, 2022
@QuinnBast
Copy link
Author

QuinnBast commented Aug 25, 2022

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
Copy link

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
Copy link

will this problem be solved soon?

@LelvProgrammer
Copy link

Bump

@85matthew
Copy link

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
Copy link

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
Copy link

antonydevanchi commented Jan 2, 2023

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

@kosta
Copy link

kosta commented Jan 11, 2023

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
Copy link

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

@mikebz
Copy link
Contributor

mikebz commented Jan 11, 2023

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
Copy link

andrzejgorski commented Feb 7, 2023

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

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

@diegoluisi
Copy link

Up!

@QuinnBast
Copy link
Author

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
Copy link
Contributor

annasong20 commented Sep 10, 2023

@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
Copy link
Contributor

@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,

@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
Copy link
Contributor

@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
Copy link

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

@mrclrchtr
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet