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

kustomize: Add support for OCI based helm repos #5167

Merged
merged 13 commits into from
Oct 17, 2023

Conversation

jkroepke
Copy link
Contributor

@jkroepke jkroepke commented May 10, 2023

Bitnami recently change from classical helm repositories to OCI based repositories, which may have an impact of the support for helm charts from OCI repositories.

this PR add support for fetch charts from OCI repositories.

Example:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
helmCharts:
- name: external-dns
#  repo: https://charts.bitnami.com/bitnami
  repo: oci://registry-1.docker.io/bitnamicharts
  version: 6.19.2
  releaseName: external-dns
  namespace: external-dns
  valuesFile: values.yaml

This PR does not implement a breaking change.

Fixes #4381
ref #4614

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 10, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @jkroepke. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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 k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 10, 2023
@jkroepke jkroepke force-pushed the helm-oci branch 3 times, most recently from 7d2eb30 to b90d861 Compare May 10, 2023 07:24
@k8s-ci-robot
Copy link
Contributor

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

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.

@jkroepke jkroepke requested a review from soudaburger May 17, 2023 00:16
@jkroepke
Copy link
Contributor Author

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label May 17, 2023
@natasha41575
Copy link
Contributor

/assign @koba1t

/triage accepted

@k8s-ci-robot k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label May 24, 2023
@natasha41575
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 24, 2023
@koba1t
Copy link
Member

koba1t commented May 24, 2023

Hi @jkroepke
Thanks for your contribution!

This PR almost looks good to me.
So, it looks like the lint failed on CI. Could you check it and fix it?

@jkroepke
Copy link
Contributor Author

I run go fmt, that should be sufficient. I tried to run make lint, but it got killed because of OOM.

@DanielYWoo
Copy link

Do we have an ETA for this to be released?

@koba1t
Copy link
Member

koba1t commented May 26, 2023

@jkroepke

I found the below error when running on my laptop. Could you fix it?

api/krusty/helmchartinflationgenerator_test.go:49:7: var-naming: const expectedHelmExternalDns should be expectedHelmExternalDNS (revive)
const expectedHelmExternalDns = `
      ^
make[1]: *** [lint] Error 1
make: *** [lint] Error 2

@jkroepke
Copy link
Contributor Author

Done

@koba1t
Copy link
Member

koba1t commented May 26, 2023

@jkroepke

Thanks!
Almost looks good to me!
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2023
@koba1t
Copy link
Member

koba1t commented May 26, 2023

/cc @natasha41575
Could you check this PR and rerun GitHub Actions?

@aristosvo
Copy link

Thanks @koba1t and @jkroepke!

Can we expect a release with this functionality in the near future?

@natasha41575
Copy link
Contributor

This is available in the latest release.

@MPV
Copy link

MPV commented Oct 21, 2023

Beautiful!

FYI: I’m adding it into ArgoCD over at:

@zimmertr
Copy link
Member

Is this not working with GHCR? The repo is public?

helmCharts:
  - name: csi-proxmox
    repo: oci://ghcr.io/sergelogvinov/charts/proxmox-csi-plugin
    version: 0.1.15 # https://ghcr.io/sergelogvinov/charts/proxmox-csi-plugin
    releaseName: csi-proxmox
    namespace: csi-proxmox

Error: failed to authorize: failed to fetch anonymous token: unexpected status from GET request to https://ghcr.io/token?scope=repository%!A(MISSING)sergelogvinov%!F(MISSING)cha
rts%!F(MISSING)proxmox-csi-plugin%!F(MISSING)csi-proxmox%!A(MISSING)pull&service=ghcr.io: 403 Forbidden

@koba1t
Copy link
Member

koba1t commented Jan 15, 2024

@zimmertr

I think you made a mistake in your kustomization.yaml file; the below file works on my computer.

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

helmCharts:
  - name: proxmox-csi-plugin
    repo: oci://ghcr.io/sergelogvinov/charts
    version: 0.1.15 # https://ghcr.io/sergelogvinov/charts/proxmox-csi-plugin
    releaseName: csi-proxmox
    namespace: csi-proxmox

https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_helmchartinflationgenerator_

@zimmertr
Copy link
Member

Doi, Thank you :)

@marcusnh
Copy link

Hello, I have the same problem with my setup to Azure Container Registery:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
# Referencing a public repo outside the main repository

helmCharts:
  - name: <ARO-REPO-CHART-NAME>
    repo: oci://ACR-NAME.azurecr.io/
    version: 0.1.6-5
    releaseName:ARO-REPO-NAME
    namespace: poseidon2-dev
    valuesFile: values.yaml

The error message is also similar:

level=error msg="finished unary call with code Unknown" error="Manifest generation error (cached): `kustomize build <path to cached source>/applicationsets/dev/demo-helm-2 --enable-helm` failed exit status 1: Error: Error: failed to authorize: failed to fetch anonymous token: unexpected status from GET request to https://<ACR-NAME>.azurecr.io/oauth2/token?scope=repository%!A(MISSING)<ARO-REPO-NAME>%!F(MISSING)<ARO-REPO-NAME>%!A(MISSING)pull&service=<ACR-NAME>.azurecr.io: 401 Unauthorized\n: unable to run: 'helm pull --untar --untardir <path to cached source>/applicationsets/dev/demo-helm-2/charts oci://<ACR-NAME>.azurecr.io/<ARO-REPO-NAME>/<ARO-REPO-NAME> --version 0.1.6-5' with env=[HELM_CONFIG_HOME=/tmp/kustomize-helm-3509023410/helm HELM_CACHE_HOME=/tmp/kustomize-helm-3509023410/helm/.cache HELM_DATA_HOME=/tmp/kustomize-helm-3509023410/helm/.data] (is 'helm' installed?): exit status 1" grpc.code=Unknown grpc.method=GenerateManifest grpc.service=repository.RepoServerService grpc.start_time="2024-01-17T09:44:38Z" grpc.time_ms=2.234 span.kind=server system=grpc

@koba1t
Copy link
Member

koba1t commented Jan 18, 2024

According to your error message, you failed to exec helm pull command.
Please try to exec helm pull directly to check if the helm chart is valid.

@marcusnh
Copy link

This works fine on my local computer. I'm able to run helm pull:

helm pull oci://ACR-NAME.azurecr.io/ARO-REPO-CHART-NAME  --version 0.1.6-5
Pulled: ACR-NAME.azurecr.io/ARO-REPO-CHART-NAME:0.1.6-5
Digest: sha256:9be2d3dd16cee312918f2b321453683e8b-NOT-THE-SAME-57b

I have also tested the chart with a ArgoCD application and that works fine with the kubernetes secret passed to my ArgocD instance:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: test-<ARO-REPO-NAME>
  namespace: gitops-developers # argocd instance namespace
spec:
  source:
    chart: <ARO-REPO-CHART-NAME>
    repoURL: <ARO-NAME>.azurecr.io
    targetRevision: 0.1.6-5
    helm:
      values: |
        application_name: "<ARO-REPO--CHART-NAME>-test"
        namespace: poseidon2-test

  destination:
    namespace: poseidon2-test
    server: https://kubernetes.default.svc
  project: poseidon2
  syncPolicy:
    automated:
      prune: true
      selfHeal: true
      allowEmpty: true
    # syncOptions:
    #   - Replace=true

@koba1t
Copy link
Member

koba1t commented Jan 18, 2024

I have also tested the chart with a ArgoCD application and that works fine with the kubernetes secret passed to my ArgocD instance:

I apologize for not knowing the details of ArgoCD. Do you use any credentials to pull helm charts?
kustomize doesn't support private repository and registry authentication for helm pull.
https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/helmcharts/

@TeamPoseidonOCP
Copy link

To get access to the private ACR, I need some form of authentication, and creating helm credentials with a Kubernetes secret has been sufficient to get access with ARgoCD.

In the link you sent, it says: "We will not add support for:

  • private repository or registry authentication
  • OCI registries
  • other large features that increase the complexity of the feature and/or have significant security implications."

OCI registries are now supported. Are there no planned features for enabling private repositories or registry authentication? This would help us a lot and simplify the setup for our developers.

@koba1t
Copy link
Member

koba1t commented Jan 18, 2024

Sorry, we don't have a plan to add more functions to helmCharts now.

We discuss in this issue about the features of Helm support. Please check, If you have your opinions.
#4401

@MrFreezeex
Copy link
Member

MrFreezeex commented Jan 18, 2024

To get access to the private ACR, I need some form of authentication, and creating helm credentials with a Kubernetes secret has been sufficient to get access with ARgoCD.

FYI I put some observations/possible followups on this (private oci support): #5407 (comment) and there is two possibles (ugly) workaround that you could do with existing versions

@felixlut
Copy link

I think the above issues are related to this ticket: #5407

My workaround for the time being is to run kustomize build in CI (with a short bash script as a workaround to auth to my private helm registry), generate a manifest.lock.yaml containing the entire k8s manifest, and have Argo cd look at that directly. Essentially move the kustomize build step from ArgoCD to CI instead, since I have more control there.

This is also consistent with the rendered manifests pattern:
https://akuity.io/blog/the-rendered-manifests-pattern

With all that said, I really think that kustomize should respect the local Helm credentials, as mentioned in the issue I linked above

@marcusnh
Copy link

Thank you for the feedback. I saw the workarounds, and there is the possibility to add them with a config map to the ArgoCD instance, but I think we will have to use another approach until this is supported. Using the workaround provided in the ticket #5407 will fix the problem, but it is not the cloud-native way that we prefer to do things. I think we will have to stick with ArgoCD applications and helm integration until this is supported.
Is there a feature request that I can follow or contribute to?

@jkroepke
Copy link
Contributor Author

jkroepke commented Jan 18, 2024

About the authentication issues, you can use helm login, crane login or docker login before running kustomize. The helm OCI integration will re-use the docker credentials for authentication. If you are running private ACR with Azure RBAC, consider to use az acr login instead docker login.

From my point of view, it's not necessary to integrate the login functionality in kustomize.

@MrFreezeex
Copy link
Member

MrFreezeex commented Jan 18, 2024

About the authentication issues, you can use helm login, crane login or docker login before running kustomize. The helm OCI integration will re-use the docker credentials for authentication. If you are running private ACR with Azure RBAC, consider to use az acr login instead docker login.

From my point of view, it's not necessary to integrate the login functionality in kustomize.

Did you tested that? My experience about this is that helm launched through kustomize is not able to use the docker config/credential...

@jkroepke
Copy link
Contributor Author

jkroepke commented Jan 18, 2024

Tested with ghcr.io. Works fine.

Additionally, I run docker logout ghcr.io after testing and I got auth errors back.

It also works with helm registry login ghcr.io

@MrFreezeex
Copy link
Member

Actually it does seems to work with helm 3.13.3 indeed thanks! Pretty sure there is something that affected the usage of helm through kustomize that got fixed at the helm level in the last couple of months as I was not alone affected by this so it would be weird that we all messed up at the same time 🤔. Anyway looks fixed to me!

@marcusnh
Copy link

Is there an update from v3.13.2 to v3.13.3 that has fixed the authentication issues?
Our infrastructure is using ArgoCD to provision applications for our developers, so we need a way of doing the helm registry login command through that API. Similar to ArgoCD already does today through ArgoCD applications.

If you are running private ACR with Azure RBAC, consider to use az acr login instead docker login.
Need to be able to use any registry, not just ARC.

@jkroepke
Copy link
Contributor Author

Is there an update from v3.13.2 to v3.13.3 that has fixed the authentication issues? Our infrastructure is using ArgoCD to provision applications for our developers, so we need a way of doing the helm registry login command through that API. Similar to ArgoCD already does today through ArgoCD applications.

If you are running private ACR with Azure RBAC, consider to use az acr login instead docker login.
Need to be able to use any registry, not just ARC.

Thats a topic for ArgoCD, isn't it?

@marcusnh
Copy link

It is kustomize that doesn't support private OCI registries, so the issue needs to be fixed there and not in ArgoCD. This feature request is only to make kustomize more compatible with ArgoCD and Helm. It is possible to add a workaround, but it is not very cloud-native. Take a look at this issue; might clear up why this is a kustomize problem and not an ArgoCD error:

@jkroepke
Copy link
Contributor Author

jkroepke commented Jan 21, 2024

It does support it. See above.

ArgoCD is should provide the authenticated context. Argo does this for helm, too.

You can also run helm registry login before running kustomize.

There is already a linked PR at the ArgoCD issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

Successfully merging this pull request may close these issues.

helmCharts repo field doesn't work with OCI registry