fix(helm): skip login if no credentials are provided

[thde]

Description of your changes

Skips login in helm if no credentials are provided:

failed to login to registry: error storing credentials - err: no credentials username

Fixes #132

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable to ensure this PR is ready for review. - No rule to make target 'reviewable'

How has this code been tested

Use an OCI registry (like oci://registry-1.docker.io/bitnamicharts) and try to pull a chart (like redis). It will fail without there changes as described above.

[jbw976]

Thank you very much for taking a stab at this @thde! Looks like there's more demand on #132, so it would be great to get this to the finish line and enable this scenario for folks. Thanks for doing your part! 💪

Do you have more explicit testing details you would be able to provide? e.g. the exact manifest you tested with and the exact behavior you saw? Not strictly required I'd say, but always really helpful for reviewers to a) get more confidence and b) test the PR changes themselves more quickly.

Thanks again!! 🙇‍♂️

[thde]

Do you have more explicit testing details you would be able to provide? e.g. the exact manifest you tested with and the exact behavior you saw?

I use the following configuration that worked with the "old" https repo, but not with the oci one:

Works ATM:

				Chart: meta.ChartMeta{
					Name:    "redis",
					Version: "18.13.0",
					Wait:    true,
				},
				Repository: meta.ChartRepository{
					Address: "https://charts.bitnami.com/bitnami",
				},

Does not work ATM:

				Chart: meta.ChartMeta{
					Name:    "redis",
					Version: "18.13.0",
					Wait:    true,
				},
				Repository: meta.ChartRepository{
					Address: "oci://registry-1.docker.io/bitnamicharts",
				},

[jbw976]

@thde thank you for that testing info and for integrating the feedback so far 🙇‍♂️ - i'm taking it for a test spin myself right now with simple yaml manifests to exercise the behavior.

I'm a bit confused by your examples though, e.g. I can't find the meta.ChartMeta type in https://github.com/crossplane-contrib/provider-helm/blob/master/apis/release/v1beta1/types.go. Where are those types coming from? Is it possible to provider more context around those snippets, so I can better understand what exactly your testing? Thank you!

[jbw976]

Actually, I tried to get a repro scenario that demonstrates the failing behavior for an OCI chart based on your snippets above @thde, but my scenario surprisingly worked OK. I wonder if it's because of the in-cluster config I'm using? 🤔

Here's a full gist of the test scenario and manifests I tried to install the redis chart from oci://registry-1.docker.io/bitnamicharts:
https://gist.github.com/jbw976/d5cc480367ee4c950203bc98712f28fc

More details about your full repro scenario will be appreciated, so we can understand why mine works with no changes and yours requires the code changes in this PR to work correctly 🤓

[thde]

Where are those types coming from? Is it possible to provider more context around those snippets, so I can better understand what exactly your testing?

Sorry that was my fault. I didn't realize that those were internal types.

More details about your full repro scenario will be appreciated, so we can understand why mine works with no changes and yours requires the code changes in this PR to work correctly 🤓

@jbw976 thank you or the test scenario! That helped me to get closer to the underlying issue. I was able to successfully deploy your example. But mine was still failing...

It seems that the error only occurs, if the docker command is available on the system where provider-helm is executed.

The dependency tree:

- github.com/crossplane-contrib/provider-helm/pkg/clients/helm
-- helm.sh/helm/v3/pkg/registry
--- oras.land/oras-go/pkg/auth/docker
---- github.com/docker/cli/cli/config/credentials

The error seems to originate from docker-credential-helpers/client/client.go#40.

Helm uses the NewClientWithDockerFallback function from oras-go for new clients in github.com/helm/helm/pkg/registry/client.go#L84.

So if I run my code without having Docker available, it works successfully even without my proposed changes in this PR.

How should we go from here? For me it would still make sense to not execute Loginif no credentials were given. Even though the error could probably also be fixed in one of the underlying packages.

[jbw976]

How should we go from here? For me it would still make sense to not execute Loginif no credentials were given

good question! with the repro suggestion in #132 (comment), i was able to reproduce the original behavior by trying to install oci://ghcr.io/dragonflydb/dragonfly/helm/dragonfly. My gist is updated with that test case too: https://gist.github.com/jbw976/d5cc480367ee4c950203bc98712f28fc

Then I built your changes in this PR that skips login and tried that scenario again - unfortunately, it seems to just delay the error to a later step during chart pull/download (failed to fetch anonymous token):

 k get events --field-selector involvedObject.name=dragonfly-oci-example
LAST SEEN   TYPE      REASON                         OBJECT                          MESSAGE
11s         Warning   CannotCreateExternalResource   release/dragonfly-oci-example   failed to install release: failed to pull chart: failed to authorize: failed to fetch anonymous token: unexpected status from GET request to https://ghcr.io/token?scope=repository%!A(MISSING)dragonflydb%!F(MISSING)dragonfly%!F(MISSING)helm%!F(MISSING)dragonfly%!F(MISSING)dragonfly%!A(MISSING)pull&service=ghcr.io: 403 Forbidden

This error message looks like what other folks were hitting in helm/helm#12424 - so I wonder if this could be solved by updating the dependencies in this repo?

[thde]

This error message looks like what other folks were hitting in helm/helm#12424 - so I wonder if this could be solved by updating the dependencies in this repo?

👍🏼 That's probably the best thing to try. Especially if an error occurs later on anyways.

[jbw976]

I tried updating the dependencies in go.mod to the latest helm.sh/helm/v3 v3.14.3, but unfortunately I'm still seeing the error of failed to fetch anonymous token: unexpected status from GET request...403 Forbidden.

It makes sense now that just updating the dependencies wouldn't work, because I was able to get the helm binary to successfully pull an OCI image with both v3.13.2 (the current version this provider is using) and v3.14.3 (the latest):

❯ ./helm-v3.13.2 pull oci://ghcr.io/dragonflydb/dragonfly/helm/dragonfly --version v1.15.1
Pulled: ghcr.io/dragonflydb/dragonfly/helm/dragonfly:v1.15.1
Digest: sha256:d31982a06c07756e05023101690549910ec16a532f9bfcaae467b6821eb691ef

❯ ./helm-v3.14.3 pull oci://ghcr.io/dragonflydb/dragonfly/helm/dragonfly --version v1.15.1
Pulled: ghcr.io/dragonflydb/dragonfly/helm/dragonfly:v1.15.1
Digest: sha256:d31982a06c07756e05023101690549910ec16a532f9bfcaae467b6821eb691ef

So this issue doesn't look related to just the helm version, it must be in the way we're configuring or invoking the helm pull functionality from inside the provider. From the helm pull CLI implementation, they are passing some options/settings while creating the registry client that we probably aren't:

• https://github.com/helm/helm/blob/release-3.14/cmd/helm/pull.go#L46
• https://github.com/helm/helm/blob/release-3.14/cmd/helm/pull.go#L61
• https://github.com/helm/helm/blob/release-3.14/cmd/helm/root.go#L261-L293

And here's where we're initializinfg some of those same objects:

• https://github.com/crossplane-contrib/provider-helm/blob/release-0.17/pkg/clients/helm/client.go#L100-L117

Notably, we're using an empty &cli.EnvSettings{}, but my attempts to locally test using cli.New() like helm does were also unsuccessful 😞

Have you gotten a chance to look at this more deeply? Any leads on what configuration we should be passing to the helm pull/registry clients, or how we should be invoking them, so that this scenario works like it does in the helm pull CLI? 🤔

[aerfio]

This issue also prevents me from installing https://github.com/grafana/grafana-operator/pkgs/container/helm-charts%2Fgrafana-operator/199945144?tag=v5.8.1, same error: failed to install release: failed to login to registry: Get "https://ghcr.io/v2/": denied

I've managed to install grafana-operator from different source, but in company where I work at we're moving to OCI artifacts internally and this issue stops internal adoption 😢

[turkenh]

I have been debugging this for the last couple of hours, did run helm cli and provider-helm side by side and debugged them line by line deep in the go http library calls. And you won't believe in the conclusion 🤯 We have an extra dragonfly at the end of the non working manifest.

---

This PR indeed fixes the issue 🎉 Thanks @thde, and thanks for all the breadcrumbs @jbw976 🙌

The manifest for the dragonfly release (the one used to test ghcr.io) was wrong because it incorrectly appended the chart name to the repository URI, resulting in one extra dragonfly at the end.

With this PR both of the below manifests works fine:

apiVersion: helm.crossplane.io/v1beta1
kind: Release
metadata:
  name: dragonfly-oci-example
spec:
  forProvider:
    namespace: default
    wait: true
    chart:
      name: dragonfly
      repository: oci://ghcr.io/dragonflydb/dragonfly/helm # Previously there was a "/dragonfly" at the end here
      version: v1.15.1
  providerConfigRef:
    name: helm-provider
---
apiVersion: helm.crossplane.io/v1beta1
kind: Release
metadata:
  name: redis-oci-example
spec:
  forProvider:
    namespace: default
    wait: true
    chart:
      name: redis
      repository: oci://registry-1.docker.io/bitnamicharts
      version: 18.13.0
  providerConfigRef:
    name: helm-provider

[turkenh]

I agree that this is a terrible UX and I am open for ideas to improve this. But this is not different than somebody making the cli call as below:

❯ helm pull oci://ghcr.io/dragonflydb/dragonfly/helm/dragonfly/dragonfly --version v1.15.1
Error: failed to authorize: failed to fetch anonymous token: unexpected status from GET request to https://ghcr.io/token?scope=repository%3Adragonflydb%2Fdragonfly%2Fhelm%2Fdragonfly%2Fdragonfly%3Apull&service=ghcr.io: 403 Forbidden

Instead of

❯ helm pull oci://ghcr.io/dragonflydb/dragonfly/helm/dragonfly --version v1.15.1
Pulled: ghcr.io/dragonflydb/dragonfly/helm/dragonfly:v1.15.1
Digest: sha256:d31982a06c07756e05023101690549910ec16a532f9bfcaae467b6821eb691ef

[github-actions]

Successfully created backport PR #221 for release-0.18.

[turkenh]

This fix released with v0.18.1.

[jbw976]

Amazing @turkenh, thank you so much for taking the investigation the final mile and figuring this out! you're always able to get to the bottom of things man, really grateful you took some time to look at this for us! 🙇‍♂️ 💪 🤩

Thanks again @thde for opening this PR and making the code change here, sorry for the distractions along the way hehe 😇

[ApolloDS]

Thank you very much @turkenh , great job!
I can confirm that downloading oci:// images works again after using v0.18.1.