fix(helm): helm pull --repo to use repo creds from helm repos file #9760

andreaskaris · 2021-06-01T14:07:32Z

Up to this point, helm pull --repo required --username and --password
even if the repository had already been added via helm repo add.

From now on, check repositories in the repositories file and if the URL
matches, pull the chart with the username and password from the entry.

Fixes #9599

Signed-off-by: Andreas Karis ak.karis@gmail.com

What this PR does / why we need it:

Set up the following reproducer - set up the following directory structure for httpd:

yum install httpd -y
mkdir cd /var/www/html/charts
cd  cd /var/www/html/charts
helm repo add brigade https://brigadecore.github.io/charts
helm pull brigade/brigade
helm pull brigade/kashti
htpasswd -c .htpasswd testuser   # --> "password"

vi .htaccess
# AuthType Basic
# AuthName "Restricted Access"
# AuthUserFile /var/www/html/charts/.htpasswd
# Require valid-user

vi /etc/httpd/conf/httpd.conf
# <Directory "/var/www/html/charts"> 
#    AllowOverride AuthConfig
# </Directory>

Now, the repo should be password protected:

curl -u testuser:password  http://127.0.0.1/charts/index.yaml

# helm repo add localhost http://127.0.0.1/charts/ --username testuser --password password
# helm search repo localhost
NAME             	CHART VERSION	APP VERSION	DESCRIPTION                                       
localhost/brigade	1.10.0       	v1.5.0     	Brigade provides event-driven scripting of Kube...
localhost/kashti 	0.7.0        	v0.4.0     	A Helm chart for Kubernetes
# helm pull localhost/brigade
# ll | grep brigade
-rw-r--r--. 1 root root 14278 Feb 10 15:01 brigade-1.10.0.tgz

However, with --repo, this won't work:

# helm pull --repo http://127.0.0.1/charts/ brigade
Error: failed to fetch http://127.0.0.1/charts/brigade-1.10.0.tgz : 401 Unauthorized

The username and password must be provided:

# helm pull --repo http://127.0.0.1/charts/ brigade
Error: looks like "http://127.0.0.1/charts/" is not a valid chart repository or cannot be reached: failed to fetch http://127.0.0.1/charts/index.yaml : 401 Unauthorized

After applying the fix:

[root@fedora1 helm]# /home/akaris/development/helm/bin/helm pull --repo http://127.0.0.1/charts/ brigade; if [ -f brigade-1.10.0.tgz ]; then echo ok; rm -f brigade-1.10.0.tgz;fi
ok
[root@fedora1 helm]# /home/akaris/development/helm/bin/helm pull --repo http://127.0.0.1/charts brigade; if [ -f brigade-1.10.0.tgz ]; then echo ok; rm -f brigade-1.10.0.tgz;fi
ok
[root@fedora1 helm]# /home/akaris/development/helm/bin/helm pull --repo http://127.0.0.1/charts//// brigade; if [ -f brigade-1.10.0.tgz ]; then echo ok; rm -f brigade-1.10.0.tgz;fi
ok
[root@fedora1 helm]# /home/akaris/development/helm/bin/helm pull --repo http://127.0.0.1///charts//// brigade; if [ -f brigade-1.10.0.tgz ]; then echo ok; rm -f brigade-1.10.0.tgz;fi
ok
[root@fedora1 helm]# /home/akaris/development/helm/bin/helm pull --repo http://127.0.0.1/charts/a//// brigade; if [ -f brigade-1.10.0.tgz ]; then echo ok; rm -f brigade-1.10.0.tgz;fi
Error: looks like "http://127.0.0.1/charts/a////" is not a valid chart repository or cannot be reached: failed to fetch http://127.0.0.1/charts/a////index.yaml : 401 Unauthorized

Special notes for your reviewer:

If applicable:

this PR contains unit tests
this PR has been tested for backwards compatibility

felipecrs · 2021-06-01T15:51:43Z

I can't assess much from the technical perspective here, my only concern is:

When we run helm dep update, the exact same thing occurs (Helm tries to find repos with credentials already set in which matches the repo url set in the dependencies). So maybe, this does not need to be reimplemented, but reused from the helm dep update.

andreaskaris · 2021-06-01T17:56:00Z

The code for helm dep update that's in charge of this is here:

helm/pkg/downloader/manager.go

Line 243 in e87f815

func (m *Manager) downloadAll(deps []*chart.Dependency) error {

and that calls findChartURL:

helm/pkg/downloader/manager.go

Line 689 in e87f815

    
           func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository) (url, username, password string, insecureskiptlsverify bool, err error) {

It then uses DownloadTo to download the chart:

helm/pkg/downloader/manager.go

Line 355 in e87f815

_, _, err = dl.DownloadTo(churl, version, destPath)

These methods are all private to the downloader package so they can't be reused.

The suggested code merely browses the repository .yaml file, looks for the first match for the repository's URL (exact match) and then returns the repository name. It then prepends the repository name to the chart name and then calls c.DownloadTo as if the user had called this with repository_name/chart_name. So it mostly recycles existing logic.

The alternative would be to fetch the username and password from the file in p.Settings.RepositoryConfig and to set them in repo.FindChartInAuthAndTLSRepoURL instead of p.Username and p.Password.

felipecrs · 2021-06-01T18:01:45Z

Methods being private does not mean they their access can not be changed, right? But anyway, your approach does the trick for me, as I said, I can't assess much from the implementation perspective.

andreaskaris · 2021-06-01T18:24:08Z

I'm not a maintainer here, either. I'm just looking into helm and I'm chasing for some low-hanging fruit bug reports.

I changed it a bit and I think its cleaner now.

This now checks if username and password are both not provided and if they are not provided, then it will attempt to find a repository entry in the RepositoryConfig file and it will use the username and password which are provided there.

Before:

[akaris@linux helm]$ helm pull  --repo http://localhost:8080 test
Error: looks like "http://localhost:8080" is not a valid chart repository or cannot be reached: failed to fetch http://localhost:8080/index.yaml : 401 Unauthorized
[akaris@linux helm]$ 
[akaris@linux helm]$ helm pull  --repo http://localhost:8080 test --username test
Error: looks like "http://localhost:8080" is not a valid chart repository or cannot be reached: failed to fetch http://localhost:8080/index.yaml : 401 Unauthorized
[akaris@linux helm]$ 
[akaris@linux helm]$ helm pull  --repo http://localhost:8080 test --username test --password test2
Error: looks like "http://localhost:8080" is not a valid chart repository or cannot be reached: failed to fetch http://localhost:8080/index.yaml : 401 Unauthorized
[akaris@linux helm]$ 
[akaris@linux helm]$ helm pull  --repo http://localhost:8080 test --username test --password test
[akaris@linux helm]$ rm -f test-0.1.0.tgz

After:

[akaris@linux helm]$ bin/helm pull  --repo http://localhost:8080 test --username test --password test
[akaris@linux helm]$ rm -f test-0.1.0.tgz 
[akaris@linux helm]$ bin/helm pull  --repo http://localhost:8080 test --username test --password test2
Error: looks like "http://localhost:8080" is not a valid chart repository or cannot be reached: failed to fetch http://localhost:8080/index.yaml : 401 Unauthorized
[akaris@linux helm]$ bin/helm pull  --repo http://localhost:8080 test --username test
Error: looks like "http://localhost:8080" is not a valid chart repository or cannot be reached: failed to fetch http://localhost:8080/index.yaml : 401 Unauthorized
[akaris@linux helm]$ bin/helm pull  --rep
8000
o http://localhost:8080 test
[akaris@linux helm]$ rm -f test-0.1.0.tgz

felipecrs · 2021-06-01T18:32:23Z

This looks cool!

cndoit18 · 2021-06-02T11:32:29Z

No offense, as I understand it, you shouldn't read data in the local configuration when using --repo (it may cause additional understanding issues).
As in this comment of mine
#9762 (comment)

felipecrs · 2021-06-02T13:28:46Z

@cndoit18 what would be your solution for the problem stated on #9599 so?

I still believe this is the best way to do it, the same way docker and podman reuses your local defined credentials when you try to pull an image from a repository specifying its url.

(it may cause additional understanding issues)

That can be easily solved by writing a proper help description to the parameter.

cndoit18 · 2021-06-02T13:50:19Z

@cndoit18 what would be your solution for the problem stated on #9599 so?

I still believe this is the best way to do it, the same way docker and podman reuses your local defined credentials when you try to pull an image from a repository specifying its url.

(it may cause additional understanding issues)

That can be easily solved by writing a proper help description to the parameter.

I've replied in your issue

mattfarina · 2021-07-01T16:23:58Z

@andreaskaris could you please fix the conflict

andreaskaris · 2021-07-02T17:34:20Z

@mattfarina Done

With current master:

[akaris@linux helm-upstream]$ git log --oneline | head -1
eb994345 Merge pull request #9871 from mattfarina/logging-for-creds
[akaris@linux helm-upstream]$ make
make: Nothing to be done for 'all'.
[akaris@linux helm-upstream]$ bin/helm pull  --repo http://localhost:8080 test
Error: looks like "http://localhost:8080" is not a valid chart repository or cannot be reached: failed to fetch http://localhost:8080/index.yaml : 401 Unauthorized

With the fix rebased:

[akaris@linux helm]$  git log --oneline | head -2
8af97d56 fix(helm): helm pull --repo to use repo creds from helm repos file
eb994345 Merge pull request #9871 from mattfarina/logging-for-creds
[akaris@linux helm]$ make
make: Nothing to be done for 'all'.
[akaris@linux helm]$ bin/helm pull  --repo http://localhost:8080 test
[akaris@linux helm]$

mattfarina · 2021-07-06T14:50:30Z

I added this PR and the corresponding issue to the next Helm developer call. I think it's worth discussing to come to a quick resolution so we can see when a change would go in.

mattfarina · 2021-07-09T21:55:43Z

Moving this to 3.7.0 as it is a feature request.

pkg/repo/repo.go

pkg/repo/repo_test.go

pkg/repo/repo.go

mattfarina · 2021-11-24T13:46:25Z

@andreaskaris We are looking at this for the 3.8.0 release in January. There's a small amount of feedback on the PR. Would you be able to update it?

felipecrs · 2021-11-29T04:18:30Z

@mattfarina I created a follow-up PR addressing the comments: #10406

andreaskaris · 2025-02-21T12:10:21Z

Hello @mattfarina, as discussed during the helm meeting last Thursday, can I get a review for this, please? I'm o.k. making modifications or if this is a new feature or a PR that won't fly, I'm fine with that, too. But I'd like to explore the options here.

Thanks a bunch!

isarns · 2025-03-03T23:32:29Z

Any updates?

andreaskaris · 2025-03-04T10:02:57Z

@gjenkins8 I rebased this to match your changes, can you review, pls?

isarns · 2025-03-19T16:05:56Z

Hi! Just checking if there are any updates on this PR.

andreaskaris · 2025-03-20T10:06:46Z

I had joined the helm meeting in Feb and someone promised to look at it, but since then, unfortunately, nothing has happened review wise :(

isarns · 2025-04-01T13:31:35Z

Hi @gjenkins8 , I hope you're doing well. Could you please review this PR when you have a moment? Thank you so much!

panteparak · 2025-05-20T01:18:31Z

@andreaskaris
Hi, could you have a look at the conflicts, and hopefully it can be merge by the next milestones.

Up to this point, helm pull --repo required --username and --password even if the repository had already been added via helm repo add. From now on, check repositories in the repositories file and if the URL matches, pull the chart with the username and password from the entry. Fixes helm#9599 Co-authored-by: Andreas Karis <ak.karis@gmail.com> Co-authored-by: Felipe Santos <felipecassiors@gmail.com> Signed-off-by: Andreas Karis <ak.karis@gmail.com>

andreaskaris · 2025-06-04T16:21:25Z

O.k.,I rebased it yet again ;-)

panteparak · 2025-06-05T09:21:25Z

hello, @mattfarina, @yxxhero, @gjenkins8,
I hope you're doing well, It would be good if you could review and merge this PR if you have some available slot.

helm-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 1, 2021

andreaskaris force-pushed the issue9599 branch 2 times, most recently from 9b2dd6a to 18e2746 Compare June 1, 2021 14:09

andreaskaris force-pushed the issue9599 branch from 18e2746 to ef49a54 Compare June 1, 2021 18:17

mattfarina added this to the 3.6.1 milestone Jun 4, 2021

andreaskaris force-pushed the issue9599 branch from ef49a54 to 8af97d5 Compare July 2, 2021 17:12

mattfarina modified the milestones: 3.6.3, 3.7.0 Jul 9, 2021

bacongobbler mentioned this pull request Aug 4, 2021

Helm reponds with 401 error on private repo #9997

Closed

hickeyma added the feature label Aug 26, 2021

yxxhero suggested changes Aug 29, 2021

View reviewed changes

pkg/repo/repo.go Outdated Show resolved Hide resolved

pkg/repo/repo_test.go Outdated Show resolved Hide resolved

pkg/repo/repo.go Outdated Show resolved Hide resolved

mattfarina modified the milestones: 3.7.0, 3.8.0 Aug 30, 2021

This was referenced Nov 29, 2021

helm pull --repo [url] does not use the credentials set with helm repo add #9599

Open

fix(helm): helm pull --repo to use repo creds from helm repos file #10406

Closed

nickperkins mentioned this pull request Nov 29, 2021

Private Helm Repository cannot be reached tarioch/flux-check-hook#1

Open

mattfarina modified the milestones: 3.17.1, 3.17.2 Feb 12, 2025

andreaskaris requested a review from yxxhero February 13, 2025 17:31

gjenkins8 self-assigned this Feb 23, 2025

andreaskaris force-pushed the issue9599 branch 2 times, most recently from 14c6c34 to ee3f7a8 Compare March 4, 2025 09:47

scottrigby modified the milestones: 3.17.2, 3.17.3 Mar 13, 2025

scottrigby added the bug Categorizes issue or PR as related to a bug. label Mar 13, 2025

mattfarina modified the milestones: 3.17.3, 3.18.1 May 19, 2025

mattfarina modified the milestones: 3.18.1, 3.18.3 May 28, 2025

andreaskaris force-pushed the issue9599 branch from ee3f7a8 to 377451c Compare June 4, 2025 16:11

andreaskaris force-pushed the issue9599 branch from 377451c to b1a5d7c Compare June 4, 2025 16:19

mattfarina modified the milestones: 3.18.3, 3.18.4 Jun 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(helm): helm pull --repo to use repo creds from helm repos file #9760

fix(helm): helm pull --repo to use repo creds from helm repos file #9760

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix(helm): helm pull --repo to use repo creds from helm repos file #9760

Are you sure you want to change the base?

fix(helm): helm pull --repo to use repo creds from helm repos file #9760

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!