kustomize icon indicating copy to clipboard operation
kustomize copied to clipboard

Support helm authentication for helm chart repositories.

Open ArthurCieslik opened this issue 3 years ago • 25 comments

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 avatar Dec 08 '21 14:12 ArthurCieslik

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

k8s-ci-robot avatar Dec 08 '21 14:12 k8s-ci-robot

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.

snuggie12 avatar Dec 30 '21 04:12 snuggie12

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

natasha41575 avatar Jan 25 '22 01:01 natasha41575

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.

QuinnBast avatar Mar 22 '22 23:03 QuinnBast

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.

tis-rpage avatar Mar 25 '22 21:03 tis-rpage

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?

minnie-jeong-otsk avatar Mar 28 '22 05:03 minnie-jeong-otsk

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

QuinnBast avatar Mar 28 '22 15:03 QuinnBast

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.

tis-rpage avatar Mar 28 '22 17:03 tis-rpage

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.

snuggie12 avatar Mar 28 '22 23:03 snuggie12

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.

tis-rpage avatar Mar 30 '22 18:03 tis-rpage

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!

cconnert avatar Apr 05 '22 12:04 cconnert

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

tis-rpage avatar Apr 05 '22 13:04 tis-rpage

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.

snuggie12 avatar Apr 06 '22 06:04 snuggie12

@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 avatar Apr 06 '22 15:04 tis-rpage

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}

tis-rpage avatar Apr 06 '22 15:04 tis-rpage

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.

snuggie12 avatar Apr 07 '22 16:04 snuggie12

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.

tis-rpage avatar Apr 07 '22 17:04 tis-rpage

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?

remidebette avatar Apr 29 '22 17:04 remidebette

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

QuinnBast avatar May 25 '22 16:05 QuinnBast

/assign

natasha41575 avatar Jul 06 '22 16:07 natasha41575

https://github.com/kubernetes-sigs/kustomize/issues/4335#issuecomment-1113555098

bump

jmmclean avatar Jul 07 '22 18:07 jmmclean

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

jmmclean avatar Jul 07 '22 18:07 jmmclean

@jmmclean This diff should work.

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

QuinnBast avatar Jul 13 '22 15:07 QuinnBast

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.

LucasMaciuga avatar Jul 27 '22 12:07 LucasMaciuga

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

gritzkoo avatar Aug 09 '22 22:08 gritzkoo

/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

KnVerey avatar Aug 31 '22 16:08 KnVerey

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

k8s-ci-robot avatar Aug 31 '22 16:08 k8s-ci-robot

reference KRM issue for others tracking https://github.com/kubernetes-sigs/kustomize/issues/4401

jmmclean avatar Sep 06 '22 12:09 jmmclean