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

helm v3.6.1 breaks compatibility with azure container registry #9857

Closed
josefschabasser opened this issue Jun 24, 2021 · 20 comments
Closed

helm v3.6.1 breaks compatibility with azure container registry #9857

josefschabasser opened this issue Jun 24, 2021 · 20 comments

Comments

@josefschabasser
Copy link

josefschabasser commented Jun 24, 2021

Output of helm version:

version.BuildInfo{Version:"v3.6.1", GitCommit:"61d8e8c4a6f95540c15c6a65f36a6dd0a45e7a2f", GitTreeState:"clean", GoVersion:"go1.16.5"}

Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.1", GitCommit:"5e58841cce77d4bc13713ad2b91fa0d961e69192", GitTreeState:"archive", BuildDate:"2021-05-14T14:09:09Z", GoVersion:"go1.16.4", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.): AKS

Steps to reproduce:

  1. add acr using azure-cli
  2. update repo data
  3. install/upgrade/pull chart from said repo

Expected outcome: chart can be installed/upgraded/pulled without issues

Actual outcome: adding repo and updating repo data works, everything else fails

$ az acr helm repo add --name myrepo
This command is implicitly deprecated because command group 'acr helm' is deprecated and will be removed in a future release. Use 'helm v3' instead.
"myrepo" has been added to your repositories

$ helm repo up
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "myrepo" chart repository
Update Complete. ⎈Happy Helming!⎈

$ helm upgrade --reuse-values --debug mychart myrepo/mychart
Error: failed to fetch https://myrepo.azurecr.io/helm/v1/repo/_blobs/mychart-1.5.41.tgz : 401 Unauthorized
helm.go:81: [debug] failed to fetch https://myrepo.azurecr.io/helm/v1/repo/_blobs/mychart-1.5.41.tgz : 401 Unauthorized
helm.sh/helm/v3/pkg/getter.(*HTTPGetter).get
	helm.sh/helm/v3/pkg/downloader/chart_downloader.go:99
helm.sh/helm/v3/pkg/action.(*ChartPathOptions).LocateChart
	helm.sh/helm/v3/pkg/action/install.go:704
main.newUpgradeCmd.func2
	helm.sh/helm/v3/cmd/helm/upgrade.go:130
github.com/spf13/cobra.(*Command).execute
	github.com/spf13/cobra@v1.1.3/command.go:852
github.com/spf13/cobra.(*Command).ExecuteC
	github.com/spf13/cobra@v1.1.3/command.go:960
github.com/spf13/cobra.(*Command).Execute
	github.com/spf13/cobra@v1.1.3/command.go:897
main.main
	helm.sh/helm/v3/cmd/helm/helm.go:80
runtime.main
	runtime/proc.go:225
runtime.goexit
	runtime/asm_amd64.s:1371

helm.sh/helm/v3/pkg/downloader.(*ChartDownloader).DownloadTo
	helm.sh/helm/v3/pkg/downloader/chart_downloader.go:99
helm.sh/helm/v3/pkg/action.(*ChartPathOptions).LocateChart
	helm.sh/helm/v3/pkg/action/install.go:704
main.newUpgradeCmd.func2
	helm.sh/helm/v3/cmd/helm/upgrade.go:130
github.com/spf13/cobra.(*Command).execute
	github.com/spf13/cobra@v1.1.3/command.go:852
github.com/spf13/cobra.(*Command).ExecuteC
	github.com/spf13/cobra@v1.1.3/command.go:960
github.com/spf13/cobra.(*Command).Execute
	github.com/spf13/cobra@v1.1.3/command.go:897
main.main
	helm.sh/helm/v3/cmd/helm/helm.go:80
runtime.main
	runtime/proc.go:225
runtime.goexit
	runtime/asm_amd64.s:1371

Steps to mitigate: do not upgrade to v3.6.1, use HelmToolinstaller in Azure pipeline to install oder version

@bacongobbler
Copy link
Member

GHSA-56hp-xqp3-w2jf

@josefschabasser
Copy link
Author

josefschabasser commented Jun 24, 2021

I'm sorry, but posting only a single link isn't very helpful and I'm well aware of that change.
Here's some more information:

$ cat ~/.config/helm/repositories.yaml
apiVersion: ""
generated: "0001-01-01T00:00:00Z"
repositories:
- caFile: ""
  certFile: ""
  insecure_skip_tls_verify: false
  keyFile: ""
  name: myrepo
  pass_credentials_all: false
  password: REDACTED
  url: https://myrepo.azurecr.io/helm/v1/repo
  username: 00000000-0000-0000-0000-000000000000

$ helm repo pull myrepo/mychart --pass-credentials
Error: failed to fetch https://myrepo.azurecr.io/helm/v1/repo/_blobs/mychart-1.5.41.tgz : 401 Unauthorized

$ cat 
apiVersion: v1
entries:
  mychart:
  - annotations:
      azurecr.io/manifest-digest: sha256:REDACTED
    apiVersion: v2
    appVersion: 1.16.0
    created: "2021-01-14T14:00:16.4682893Z"
    description: A Helm chart for Kubernetes
    digest: REDACTED
    name: mychart
    urls:
    - _blobs/mychart-1.5.41.tgz
    version: 1.5.41

azure-cli does not support --pass-credentials to set pass_credentials_all: true in ~/.config/helm/repositories.yaml and it is unlikely to receive this flag as az acr helm repo add is deemed deprecated.
Does this mean manually setting this config entry or switching over to experimental OCI support are my only choices?

EDIT: it only works when I set pass_credentials_all: true in ~/.config/helm/repositories.yaml. What is --pass-credentials supposed to do during install/upgrade/pull? Because I observed no change in behaviour.

@josefschabasser
Copy link
Author

As a temporary workaround, one could do:

REPO="myrepo"
FILE="${HOME}/.config/helm/repositories.yaml"
FILE_TMP=$(mktemp)

# add repo
az acr helm repo add --name ${REPO}

# patch repo
yq -y '(.repositories[] | select(.name == $REPO) | .pass_credentials_all) |= true' --arg REPO ${REPO} < ${FILE} > ${FILE_TMP}

# overwrite config with patched one
mv ${FILE_TMP} ${FILE}

@bacongobbler
Copy link
Member

bacongobbler commented Jun 24, 2021

Apologies. Was in the middle of grabbing coffee and on my way to the helm dev call this morning. Was just posting a link to quickly give some background context.

The security release discusses a breaking change to the way Helm passes username/password credentials to the server hosting the chart package, listed in the urls field of the repository's index.yaml file. We no longer pass those credentials to upstream servers unless the URL

  1. Is relative (e.g. /foo-1.0.0.tgz)
  2. Is served from localhost (e.g. http://localhost/foo-1.0.0.tgz)
  3. Is served from the same host (e.g. you used helm repo add foo https://foo.com, and the index.yaml referenced https://foo.com/foo-1.0.0.tgz)

In Helm, we had to introduce a new --pass-credentials flag to helm repo add to accomodate use cases where the server hosting the chart package SHOULD receive the same username/password credentials as the chart repository, like artifactory, github container registry, or azure acr helm repository.

However, in the latter case, they use a custom az acr helm repo add command. As of Helm 3.6.1, they will need to implement the --pass-credentials flag in az acr helm repo add so the correct field is added to the repository config.

You can also work around this by adding the field to your local repositories.yaml file yourself, as you've done in your last comment.

Hope this is more helpful.

@bacongobbler
Copy link
Member

bacongobbler commented Jun 24, 2021

Does this mean manually setting this config entry or switching over to experimental OCI support are my only choices?

An alternative option may be to look at the implementation details of az acr helm repo add (it's on github), fork it, and implement the new functionality yourself. But if the Azure ACR team has deprecated this, then you may be out of luck with regards to official support, yes.

@bacongobbler
Copy link
Member

Marking as a support issue as this relates to a third-party implementation. We're happy to provide details, but it'll be up to the ACR team to implement.

@mattfarina
Copy link
Collaborator

Thanks for bringing this up.

--pass-credentials needs to be used with helm repo add or along with the --repo flag on a command like helm install.

If this is causing an issue with ACR it means that another domain is being used (possibly on a redirect). It might be a good put some more debugging logging to identify what's going on in helm.

@josefschabasser
Copy link
Author

josefschabasser commented Jun 25, 2021

Wait, it wasn't clear to me to use both --pass-credentials and --repo. Anyways, here's a snippet to get rid of az acr helm repo add and the patching from my previous post:

REPO="myrepo"
#CRED=($(az acr credential show -n ${REPO} --query '{username: username, password: passwords[0].value}' -o tsv))
CRED=($(az acr credential show -n ${REPO} | jq -r '.username, .passwords[0].value'))
HOST=$(az acr show -n ${REPO} --query 'loginServer' -o tsv)
REPO_URL="https://${HOST}/helm/v1/repo"

helm repo add ${REPO} ${REPO_URL} \
    --username ${CRED[0]} \
    --password ${CRED[1]} \
    --pass-credentials

EDIT: single query for credentials
EDIT 2: use jq for shorter code and direct extraction of values (azure-cli creates a new object)

@erpel
Copy link

erpel commented Jun 25, 2021

  • Is relative (e.g. /foo-1.0.0.tgz)
  • Is served from localhost (e.g. http://localhost/foo-1.0.0.tgz)
  • Is served from the same host (e.g. you used helm repo add foo https://foo.com, and the index.yaml referenced https://foo.com/foo-1.0.0.tgz)

Now as I understand, the last of these cases should mean that my setup should work, but it does not (I've only replaced a domain name with xxx):

✔ ~/work/temp 
14:38 $ helm repo list | grep internal
internal-regular    	https://packages.cloud.xxx.de/artifactory/internal-helm    
internal            	https://packages.cloud.xxx.de:443/artifactory/internal-helm
✔ ~/work/temp 
14:38 $ helm fetch internal/service-auth
Error: failed to fetch https://packages.cloud.xxx.de:443/artifactory/api/helm/internal-helm/service-auth-9.24.14000.tgz : 401 Unauthorized
✘-1 ~/work/temp 
14:38 $ helm fetch internal-regular/service-auth
Error: failed to fetch https://packages.cloud.xxx.de:443/artifactory/api/helm/internal-helm/service-auth-9.24.14000.tgz : 401 Unauthorized

I understand that helm might complain because the "-regular" repo is added without the port but the repo contains the port. But the repo added with the port should work right? It's identical between repo config and repo content.

Both repos were added using helm versions before 3.6.1.

@josefschabasser
Copy link
Author

josefschabasser commented Jun 25, 2021

In case of Azure Container Registry the host added to the repo list is the login server, the charts are served from another server.
The interesting thing here is, that helm always lists the login server in the error, but never shows the actual storage server.

Example:

$ helm repo add myrepo https://myrepo.azurecr.io/helm/v1/repo
"myrepo" has been added to your repositories

$ tail ~/.config/helm/repositories.yaml
- caFile: ""
  certFile: ""
  insecure_skip_tls_verify: false
  keyFile: ""
  name: myrepo
  pass_credentials_all: false
  password: ""
  url: https://myrepo.azurecr.io/helm/v1/repo
  username: ""

$ head ~/.cache/helm/repository/myrepo-index.yaml
apiVersion: v1
entries:
  mychart:
  - annotations:
      azurecr.io/manifest-digest: sha256:REDACTED
    apiVersion: v2
    appVersion: 1.16.0
    created: "2021-01-14T14:00:16.4682893Z"
    description: A Helm chart for Kubernetes
    digest: REDACTED

$ helm pull myrepo/mychart
Error: failed to fetch https://myrepo.azurecr.io/helm/v1/repo/_blobs/mychart-1.5.42.tgz : 401 Unauthorized

As you can see, the repository has hostname myrepo.azurecr.io but the charts are served from azurecr.io. This mismatch causes helm to fail, but always displays the repositories hostname, never the charts one.

EDIT: not sure if this information is correct...

@erpel
Copy link

erpel commented Jun 25, 2021

I'll add the same details as in @josefschabasser comment for my example:

✔ ~/Library/Preferences/helm 
15:24 $ grep internal repositories.yaml 
  name: internal-regular
  url: https://packages.cloud.xxx.de/artifactory/internal-helm
  name: internal
  url: https://packages.cloud.xxx.de:443/artifactory/internal-helm

✔ ~/Library/Caches/helm/repository 
15:26 $ grep  "9.24.14000" internal-index.yaml internal-regular-index.yaml 
internal-index.yaml:    appVersion: "9.24.14000"
internal-index.yaml:    - https://packages.cloud.xxx.de:443/artifactory/api/helm/internal-helm/service-auth-9.24.14000.tgz
internal-index.yaml:    version: "9.24.14000"
internal-regular-index.yaml:    appVersion: "9.24.14000"
internal-regular-index.yaml:    - https://packages.cloud.xxx.de:443/artifactory/api/helm/internal-helm/service-auth-9.24.14000.tgz
internal-regular-index.yaml:    version: "9.24.14000"

As far as I can tell, for "internal" both urls are the same https://packages.cloud.xxx.de:443/.

@bacongobbler
Copy link
Member

We’re aware of a few cases where artifactory could be using an internal redirect to another domain, which can cause the 401. Please do let us know if you can find out more about your case though.

@artem-nefedov
Copy link

I'm seeing the same issue with Artifactory, where I can't download the chart if the repo wasn't added with --pass-credentials, even though the domain in the index.yaml and in the repo url is exactly the same (including port):

helm repo add my-repo https://my-repo-name.com:443/helm-bld/
https://my-repo-name.com:443/api/helm/helm-bld/kiali-server/kiali-server-1.35.0.tgz

@erpel
Copy link

erpel commented Jun 25, 2021

It might be helpful to have such a redirect included in --debug output in the future.

@mattfarina
Copy link
Collaborator

Can you please test #9871 to see if it fixes your issue.

@erpel
Copy link

erpel commented Jun 30, 2021

I'm happy to report that Helm 3.6.2 fixes parts of the issue.
If both the repo URL and the chart URL both contain the port (https://...:443/...) then the download works as before Helm 3.6.1.
Since 443 is the default port, few people will add their repos using urls including :443. I'm not sure how many repository implementations add the default port explicitly, but at least Artifactory does.
As long as doing so is "legal", I think helm might need to normalize the URLs before comparing, so that explicitly adding default ports to a URL does not break what is expected to work.
Any thoughts?

@artem-nefedov
Copy link

Can confirm that specifying url with port now works correctly with Artifactory in helm 3.6.2 and --pass-credentials is not required.

@dol
Copy link
Contributor

dol commented Jun 30, 2021

@erpel We are also an user of JFrog Artifactory and face the same problem. Because of this the following PR ( #9886 ) was created.

@bacongobbler
Copy link
Member

Closing this issue as the OP's issue has been fixed in Helm 3.6.2. For follow-up discussions please open another ticket so that we can track that ticket separately. Thanks.

@josefschabasser
Copy link
Author

Sorry, I'm late to the party. helm v3.6.2 indeed restores compatibility with Azure Container Registries, no need to use --pass-credentials at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants