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

Support helm authentication for helm chart repositories. #4335

Closed
ArthurCieslik opened this issue Dec 8, 2021 · 29 comments
Closed

Support helm authentication for helm chart repositories. #4335

ArthurCieslik opened this issue Dec 8, 2021 · 29 comments
Assignees
Labels
area/helm kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. triage/out-of-scope Indicates an issue or PR is not a fit for Kustomize's scope and/or principles

Comments

@ArthurCieslik
Copy link

When using helmCharts in kustomize, helm chart repositories authentication is not supported.
Further, kustomize prevents the helm credentials storage and usage due to the way kustomize is using helm.

Example:

In a case like this:

helmCharts:
- name: minecraft
  repo: https://itzg.github.io/minecraft-server-charts

Kustomize uses to download the chart a helm command like this

helm pull minecraft --repo https://itzg.github.io/minecraft-server-charts

Helm supports credentials to chart repositories via parameters or it's own repository management. You can template, install or pull a helm chart via a secured repository either by

helm pull helmChart --repo https://secured.com/chartrepo --username <username> --password <password>

or adding a repository to the helm configuration including the repository credentials

helm repo add securedRepo https://secured.com/chartrepo --username <username> --password <password>
helm pull securedRepo/helmChart

Issue:

Kustomize is not supporting the handover of repository credentials, but it is also prevents the usage of helm repository management by making the field 'repo' mandatory.
It is an optional field in the kustomize yaml but in such case the repo parameter is set during the helm command anyway, which prevents helm to look into it's own repository configuration:

Solution:

Make the 'repo' field optional, since the repository can be predefined in a helm configuration and the repository name becomes part of the chartName:

helmCharts:
- name: chartRepoName/minecraft

Alternatively introduce an repoName field in the kustomize helmCharts section where repoName becomes part of the helm command:

helm pull repoName/name
@ArthurCieslik ArthurCieslik added the kind/bug Categorizes issue or PR as related to a bug. label Dec 8, 2021
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Dec 8, 2021
@k8s-ci-robot
Copy link
Contributor

@ArthurCieslik: This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@snuggie12
Copy link

This is pretty easy to fix, but I have quite a few (newb) questions before I could throw out a PR:

  • How do I get a fork into my gopath? Do I just create the fork, create the dirs in src and then git clone??
  • Do the maintainers even want this change? The functionality used to be there but was removed in 4.x
  • Does helm support other protocols besides http(s)?? If so, should I change the regex to a made up one like repo://??
  • I suppose it could wait until the PR but I really have no clue whether creating that variable or appending fmt.Sprintf is better. Same goes for my syntax on the if condition (not a developer)

tested diff:

diff --git a/api/internal/builtins/HelmChartInflationGenerator.go b/api/internal/builtins/HelmChartInflationGenerator.go
index 2a654ad1f..11308139e 100644
--- a/api/internal/builtins/HelmChartInflationGenerator.go
+++ b/api/internal/builtins/HelmChartInflationGenerator.go
@@ -290,9 +290,13 @@ func (p *HelmChartInflationGeneratorPlugin) pullCommand() []string {
        args := []string{
                "pull",
                "--untar",
-               "--untardir", p.absChartHome(),
-               "--repo", p.Repo,
-               p.Name}
+               "--untardir", p.absChartHome()}
+       if match, _ := regexp.MatchString("^https?://", p.Repo); match == true {
+               args = append(args, "--repo", p.Repo, p.Name)
+       } else {
+               repoName := p.Repo + "/" + p.Name
+               args = append(args, repoName)
+       }
        if p.Version != "" {
                args = append(args, "--version", p.Version)
        }

PS It appears as if helm pull is broken in another way in 4.X+. It no longer downloads the chart to a temp dir if you specify helmGlobals.configHome. If that issue doesn't exist I'll file that one separately.

@natasha41575
Copy link
Contributor

We will be supporting this in our next iteration of the helm generator.

@QuinnBast
Copy link

I am now facing this issue as well. It seems like the helm chart inflator is non-functional after the helm v3 release.

Since oci registry support is not functional, I attempted to use a chartmuseum repository instead to hopefully make this tool work using a non-oci protocol. However, I ended up running into this issue. I was unable to add a repo with the saved credentials through helm repo add, nor was I able to specify helmCommand to be a command that contained the desired flags for the helm pull command (something like --helm-command helm --ca-file <pathToFile>).

In the issue I linked above I proposed a similar change to the one outlined in @snuggie12 's comment. I would love to see these changes get applied as this would eliminate a TON of bash scripting that we currently have around deployment.

@tis-rpage
Copy link

I think this is a bit of a mis-feature, as authentication can be established with repositories prior to kustomize being involved, ala helm login.

@minnie-jeong-otsk
Copy link

It might be a silly question, but in which pod should I do :
helm repo add securedRepo https://secured.com/chartrepo --username <username> --password <password>

I see lots of pods like: argocd-repo-server, argocd-server, argocd-redis-ha-server, ...

Or can I do helm repo add setting somewhere in yaml file? If so, where it would be?

@QuinnBast
Copy link

It might be a silly question, but in which pod should I do : helm repo add securedRepo https://secured.com/chartrepo --username <username> --password <password>

You shouldn't have to run this in a pod. You should just need to run it on your local machine where you are running the helm commands. However, even if you do that the helmCharts inflator within Kustomize does not work (which is why this issue is open).

@tis-rpage
Copy link

tis-rpage commented Mar 28, 2022

The environment variable for HELM_REGISTRY_CONFIG can be specified so that the registry authentication is available to kustomize so it's inherited by the helm subshell, and the pod can mount the authentication secrets into the namespace in an appropriate location thereby negating the need for this ticket.

@snuggie12
Copy link

The environment variable for HELM_REGISTRY_CONFIG can be specified so that the registry authentication is available to kustomize so it's inherited by the helm subshell, and the pod can mount the authentication secrets into the namespace in an appropriate location thereby negating the need for this ticket.

Could you supply a working example including versions of kustomize and helm plus all relevant yaml and config files?

The problem as I know it is that beginning in 4.x of kustomize the api for a helm chart was reduced and when a "helm pull" is initiated it doesn't have the correct format to pull from a repo that is locally stored and had no capability of providing a username and password to a straight URL.

@tis-rpage
Copy link

tis-rpage commented Mar 30, 2022

HELM_REGISTRY_CONFIG uses the same json format pioneered by DOCKER_CONFIG. If you run a helm login it'll generate ${XDG_CONFIG_HOME}/helm/registry.json which you can then mount into your pod and set the HELM_REGISTRY_CONFIG variable to point to the mounted file. Then when kustomize executes, it'll inherit the HELM_REGISTRY_CONFIG and the helm pull subshell will have the same environment and be able to get the authentication details.

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

cconnert commented Apr 5, 2022

HELM_REGISTRY_CONFIG uses the same json format pioneered by DOCKER_CONFIG. If you run a helm login it'll generate ${XDG_CONFIG_HOME}/helm/registry.json which you can then mount into your pod and set the HELM_REGISTRY_CONFIG variable to point to the mounted file. Then when kustomize executes, it'll inherit the HELM_REGISTRY_CONFIG and the helm pull subshell will have the same environment and be able to get the authentication details.

Thanks @tis-rpage. However I would also like to request a working example. First of all at my system I don not find the ${XDG_CONFIG_HOME}/helm/registry.json. Instead I do have a ${XDG_CONFIG_HOME}/helm/repositories.yaml. Setting HELM_REGISTRY_CONFIG to point to that leads to:

error: WARNING: invalid character 'a' looking for beginning of value

Thus I converted the yaml into a json but still the helm pull ... fails. Further I would like to know how I can set that environment variable with the -e or --env argument, as this has no effect in my case. I am on helm 3.7.0 and kubectl 1.23.5. Thanks!

@tis-rpage
Copy link

tis-rpage commented Apr 5, 2022

Don't try to hand create the file contents, use helm login to generate the contents. It should look something like this:

{
    "auths": {
        "<AWSACCOUNTIDGOESHERE>.dkr.ecr.us-east-1.amazonaws.com": {
            "auth": "<BASE64encodedAUTHSTRING>"
        },
        "<OCIREGISTRY>.example.com": {
            "auth": "<BASE64encodedAUTHSTRING>"
        }
    }
}

The BASE64encodedAUTHSTRING is roughly this format:

<USERNAME>:<BASE64encodedPAYLOAD>

With BASE64encodedPAYLOAD for my AWS creds being:

{
    "payload": "<BASE64encodedBINARY>",
    "datakey": "<BASE64encodedBINARY>",
    "version": "2",
    "type": "DATA_KEY",
    "expiration": "<INTEGER_OFFSET>"
}

@snuggie12
Copy link

Sorry, I have been away from work and hadn't gotten a chance to clear this up.

@tis-rpage As I said above, it is not about the authentication. It's the format of the helm pull command. If you look at my working diff and take the example of using the old stable repo and we'll say prometheus chart you'll see in the current version it performs:

helm pull --untar --untardir /Users/snuggie-12/...omitted... --repo stable prometheus

when it should be:

helm pull --untar --untardir /Users/snuggie-12/...omitted... stable/prometheus

I think what's important is that this is going to get fixed in #4401 and the kep for the registry.

@natasha41575 are the features for the new helm plugin specifically tracked? I haven't seen one yet.

@QuinnBast thanks for also including my code in your patch and release! I know you were more focused on oci so I appreciate it. I'll give it a shot in the next day or two.

@tis-rpage
Copy link

@tis-rpage As I said above, it is not about the authentication. It's the format of the helm pull command. If you look at my working diff and take the example of using the old stable repo and we'll say prometheus chart you'll see in the current version it performs:
...
I think what's important is that this is going to get fixed in #4401 and the kep for the registry.

QuinnBast already opened up #4381 specifically to address OCI connections. My comments on this ticket are regarding authentication, which kustomize/helm already support via external authentication passed in via secured files and environment variable references.

In general, putting credentials on the command line is poor practice, as it exposes credentials via /proc/$$/cmdline. Therefore I recommend removing any capability from patch submission to continue to discourage poor practices.

@tis-rpage
Copy link

tis-rpage commented Apr 6, 2022

The environment variable for HELM_REGISTRY_CONFIG can be specified so that the registry authentication is available to kustomize so it's inherited by the helm subshell, and the pod can mount the authentication secrets into the namespace in an appropriate location thereby negating the need for this ticket.

Could you supply a working example including versions of kustomize and helm plus all relevant yaml and config files?

The problem as I know it is that beginning in 4.x of kustomize the api for a helm chart was reduced and when a "helm pull" is initiated it doesn't have the correct format to pull from a repo that is locally stored and had no capability of providing a username and password to a straight URL.

If you pre-seed charts/mychart with your expanded repo, you can reference it via kustomize.

mkdir -p bases/aws-ack-rds/charts
VER=v0.0.19
helm pull oci://public.ecr.aws/aws-controllers-k8s/rds-chart --version "${VER}"
tar xf "rds-chart-${VER}.tgz" -C bases/aws-ack-rds/charts
rm "rds-chart-${VER}"

<<-EOF cat > bases/aws-ack-rds/kustomization.yaml
helmCharts:
  - namespace: ack
    name: rds-chart
    repo: oci://public.ecr.aws/aws-controllers-k8s/rds-chart
    version: v0.0.19
    releaseName: ee
    includeCRDs: true
EOF
kustomize build --enable-helm bases/aws-ack-rds

helm version ; kustomize version

version.BuildInfo{Version:"v3.8+unreleased", GitCommit:"fd6c4d191ad97de73d628b39fe81d7e5781c2d4b", GitTreeState:"clean", GoVersion:"go1.18"}
{Version:kustomize/v4.5.3 GitCommit:de6b9784912a5c1df309e6ae9152b962be4eba47 BuildDate:2022-03-24T20:51:20Z GoOs:linux GoArch:amd64}

@snuggie12
Copy link

If you pre-seed charts/mychart with your expanded repo, you can reference it via kustomize.

I think "pre-seed" is where the disconnect may be. A lot of issues created around the future of helm chart inflation with kustomize are about running it in a gitops environment like argocd. There's no chance to run helm login or the helm pull you listed above.

@tis-rpage
Copy link

tis-rpage commented Apr 7, 2022

If you pre-seed charts/mychart with your expanded repo, you can reference it via kustomize.

I think "pre-seed" is where the disconnect may be. A lot of issues created around the future of helm chart inflation with kustomize are about running it in a gitops environment like argocd. There's no chance to run helm login or the helm pull you listed above.

Most workflows allow you to have a pre: stage for this type of use-case, I'd be surprised if ArgoCD is glaringly deficient in that regard. I believe ArgoWorkflows supports this kind of concept, though it's likely not as opinionated as ArgoCD.

@remidebette
Copy link

remidebette commented Apr 29, 2022

Hi all,
I am not able to make the HELM_REGISTRY_CONFIG workaround work for a Harbor authenticated Helm repository. (I get a 401)

The kustomization.yaml:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

helmCharts:
  - name: <CHARTNAME>
    version: <some version>
    repo: https://<host>/chartrepo/<repo>
    valuesFile: values-develop.yaml
    namespace: argocd
    releaseName: ...

I see that the helm command ran behind the scenes is :

HELM_REGISTRY_CONFIG=/.../registry.json helm pull --untar --untardir <some path>/<CHARTNAME>/charts --repo https://<host>/chartrepo/<repo> <CHARTNAME> --version <some version>

Instead, in my case I can generate a HELM_REPOSITORY_CONFIG that works with an Harbor username and API key.
When you do that, one can then use a <repository name> instead of the url https://<host>/chartrepo/<repo>

But then the helm command that would works should not have --repo

HELM_POSITORY_CONFIG=/.../repository.yaml helm pull --untar --untardir <some path>/<CHARTNAME>/charts  <repository name>/<CHARTNAME> --version <some version>

Hence I tried to remove the repository url in the kustomization.yaml like so:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

helmCharts:
  - name: <repository name>/<CHARTNAME>
    version: <some version>
    repo: null
    valuesFile: values-develop.yaml
    namespace: argocd
    releaseName: ...

Alas, I get the following error:

$ kustomize build --enable-helm
Error: no repo specified for pull, no chart found at ''

Am I missing anything?

@QuinnBast
Copy link

@remidebette Yeah, that is the entire purpose of this issue and this issue. In the linked issue I've created a binary which contains the fix although it is not officially supported.

@natasha41575
Copy link
Contributor

/assign

@jmmclean
Copy link

jmmclean commented Jul 7, 2022

#4335 (comment)

bump

@jmmclean
Copy link

jmmclean commented Jul 7, 2022

@natasha41575 it looks like the code specifically for this is here - https://github.com/kubernetes-sigs/kustomize/blob/master/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go#L235-L238

I wish i had more Golang expertise to properly contribute, but is there a way we can make the --repo flag optional if repo/chartname is supplied for the name? I believe this would satisfy the request by @remidebette .

@QuinnBast
Copy link

@jmmclean
This diff should work.

I also released an unofficial patch for this a few months ago

@LucasMaciuga
Copy link

Would be great to hear some news about this topic and what speaks against the fix @QuinnBast provided?

Also what I don't understand how to use/implement the render-helm-chart from krm-functions-registry. I have understood that it is here mentiond as the new home of the "helm-chart generator plugin", but I have no clue how to use it.

@gritzkoo
Copy link

gritzkoo commented Aug 9, 2022

I'm getting the same issue trying to use a private helm repository using Github.
Locally, to download the helm repo, I need to use the command:

helm repo add my-private-repo https://my-corp.github.io/my-private-helm \
  --username some-one \
  --password $GITHUB_TOKEN \
  --pass-credentials
helm pull my-private-repo/my-chart

It would be great if kustomization installation/config accepted global credentials to be used in all helm contexts using the flags like --username --password --pass-credentials. The flag --pass-credentials is essential because using GitHub pages to host helm index.yaml with a https://raw.githubusercontent.com DNS to download artifacts needs the authentication context to pass through the credentials used in helm to Github

@KnVerey
Copy link
Contributor

KnVerey commented Aug 31, 2022

/triage out-of-scope
/kind feature
/close

This will be supported in the new KRM Function implementation, but is out of scope for the current Kustomize built-in:
https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/helmcharts/#long-term-support

@k8s-ci-robot k8s-ci-robot added triage/out-of-scope Indicates an issue or PR is not a fit for Kustomize's scope and/or principles kind/feature Categorizes issue or PR as related to a new feature. labels Aug 31, 2022
@KnVerey KnVerey removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Aug 31, 2022
@k8s-ci-robot
Copy link
Contributor

@KnVerey: Closing this issue.

In response to this:

/triage out-of-scope
/kind feature
/close

This will be supported in the new KRM Function implementation, but is out of scope for the current Kustomize built-in:
https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/helmcharts/#long-term-support

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jmmclean
Copy link

jmmclean commented Sep 6, 2022

reference KRM issue for others tracking #4401

@QuinnBast
Copy link

QuinnBast commented Sep 4, 2024

Wait, this got closed?

That is very disappointing... This makes Kustomize completely unusable to me as we need to use the helmCharts block on a locally hosted private registry but have no way of doing so.

The Helm long-term plan has had no action in over 2 years so it's sad that such a simple feature as logging in to a helm registry is "out of scope"...

The helm chart inflator doesn't even work if run kustomize locally against an already authenticated repository...

Works:

helm repo add my-private-repo https://someURL --username username --password password
helm pull my-private-repo/someChart --version 0.0.1

But Kustomize Does not work:

helmCharts:
  - name: someChart
    releaseName: someChart
    namespace: someChart
    # Also Tried with
    # repo: https://someURL
    repo: my-private-repo
    version: 0.0.1
    valuesFile: ../base/base_values.yaml

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. kind/feature Categorizes issue or PR as related to a new feature. triage/out-of-scope Indicates an issue or PR is not a fit for Kustomize's scope and/or principles
Projects
None yet
Development

No branches or pull requests