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

Support Helm charts from OCI registries #124

Closed
squaremo opened this issue Sep 1, 2020 · 40 comments
Closed

Support Helm charts from OCI registries #124

squaremo opened this issue Sep 1, 2020 · 40 comments
Labels
area/helm Helm related issues and pull requests area/oci OCI related issues and pull requests enhancement New feature or request
Milestone

Comments

@squaremo
Copy link
Member

squaremo commented Sep 1, 2020

The language in the Helm docs about registries suggests that the plan is support using OCI (https://helm.sh/docs/topics/registries/). This feature still experimental in Helm, but it looks like the plan is to graduate it in a release soon: helm/helm#7613, helm/community#136.

@squaremo squaremo added enhancement New feature or request area/helm Helm related issues and pull requests labels Sep 1, 2020
@fredgate
Copy link

fredgate commented Nov 3, 2020

Yes it would be a really great feature. OCI is the future for Helm chart, and Harbor already support replication to external registries like Azure Container Registry.

@hiddeco
Copy link
Member

hiddeco commented Jan 19, 2021

The OCIGetter was made available in Helm v3.5.0 but has not been extensively documented yet.

Update: no chances of integration yet, as the registry.Client required to pull it all off (🥁) is still in helm/internal.

@LittleChimera
Copy link

The OCIGetter was made available in Helm v3.5.0 but has not been extensively documented yet.

Update: no chances of integration yet, as the registry.Client required to pull it all off (🥁) is still in helm/internal.

Isn't it enough to just use OCIGetter which wraps the client you mention?

@hiddeco
Copy link
Member

hiddeco commented Feb 16, 2021

No, it is not as we do not have access to the RegistryClient that is injected using the options. We are therefore unable to construct the OCIGetter properly.

@hiddeco hiddeco added the blocked/upstream Blocked by an upstream dependency or issue label Feb 16, 2021
@Jimmy-Newtron
Copy link

I have browsed for few days searching a solution to have my Charts stored in a private GitLab container registry (OCI compliant registry) and be finally able to pull those charts from the usual Helm command.

I have found the code from two small projects that looks appealing:

The goal is to build a classic Chart registry from the content of the OCI registry using the following endpoints:

  • /index.yaml # Get the OCI data from --storage-registry-repo, then transform them into standard chart repository data structures.
  • /charts/:name # Download a chart artifact

Do you think that such solution can be the good direction to avoid rewriting a specific module to handle OCI registries?

@alxy
Copy link

alxy commented Apr 27, 2021

Is there any timeline available on when this might become available? We are also using Azure Container registry and MSFT is suggesting to use OCI with Helm3, which on the Helm side is considered experimental.
Now that we did the migration we also wanted to look into GitOps/Flux, however, its a major blocker now that I cannot pull the charts from the registry, as OCI3 is not supported.

@hiddeco
Copy link
Member

hiddeco commented Apr 27, 2021

See: helm/helm#9188 (comment)

@rchrd
Copy link

rchrd commented Apr 27, 2021

@alxy It is also possible to push tar.gz files of Helm chart to an ACR. We currently push both the OCI and the tar.gz, for now we only use the tar.gz with Flux but when OCI is available when can just switch without the need to push all charts again.

@alxy
Copy link

alxy commented Apr 27, 2021

@rchrd Thanks for the tip, we have also considered the hybrid approach, but it makes the build and deploy pipelines a lot more complex (as the helm commands to run are quite different). Will need to think about it... Especially since the deploy part is then potentially anyway handled by flux, it might be worth it.

@Jimmy-Newtron
Copy link

I have browsed for few days searching a solution to have my Charts stored in a private GitLab container registry (OCI compliant registry) and be finally able to pull those charts from the usual Helm command.

I have found the code from two small projects that looks appealing:

* https://github.com/alauda/oci-chartrepo

* https://github.com/hangyan/chart-registry

The goal is to build a classic Chart registry from the content of the OCI registry using the following endpoints:

* **/index.yaml**     # Get the OCI data from --storage-registry-repo, then transform them into standard chart repository data structures.

* **/charts/:name**   # Download a chart artifact

Do you think that such solution can be the good direction to avoid rewriting a specific module to handle OCI registries?

@alxy Have you though about a proxy/mirror/endpoint that can translate your OCI registry in a classic Chart registry ?
Have a look at the projects in the quote, they can help you to think about a different solution

@alxy
Copy link

alxy commented Apr 27, 2021

@Jimmy-Newtron I think in that case I'd rather use the approach that @rchrd suggested. At least the ACR (which is a managed cloud resource) supports both methods (in parallel), and I'd prefer that over setting up my custom repository and adding even more tooling to the already complex process and toolchain.

@monotek
Copy link

monotek commented Apr 28, 2021

I'm using chartmuseum for now and pushing helm charts parallel to chartmuseum and to gcr.io.
Hopefully we can get rid of chartmuseum soon :)

@bplasmeijer
Copy link

bplasmeijer commented May 4, 2021

It would be great if OCI registries are supported in FluxCD.

@alxy
Copy link

alxy commented May 4, 2021

@hiddeco May I ask if this is a technical thing that this cannot be added to flux, or something political, as in you are not using features considered as experimental by Helm?

The reason I am asking is that I have found this PR for argo-cd which seems to implement exactly the functionality we are looking for here...

@moolen
Copy link

moolen commented May 6, 2021

ArgoCD uses cmd.Exec under the hood, this controller integrates the go pkg helm.sh/helm/v3/pkg. As mentioned above, OCIGetter is partially internal and can not be constructed properly without internals.

We are also blocked by this (we in particular want to use AWS ECR). Would it be an option to copy the necessary parts from helms internal/ pkg? kustomize-controller does that too to support sops. I'm not saying that this is the best solution, but it has been quite some time and i would prefer the trade-off as it can be cleaned-up later.

*edit*
there's an update regarding helm/OCI support, see helm/helm#9188 (comment)

@bacongobbler
Copy link

There are a number of well-known issues with the current helm chart design. Much of which is being removed or refactored. HIP 6 describes most of that work, but in practical terms this means that many parts of internal/registry will be changed, refactored, or removed before we move to stable.

I would urge others not to copy code from internal into their own packages. You're just going to end up holding onto dead code with no migration plan.

@bacongobbler
Copy link

bacongobbler commented May 7, 2021

As a note, here's just the tip of the iceberg of current design issues we need to work through:

https://github.com/helm/helm/issues?q=is%3Aissue+is%3Aopen+label%3Abug+oci
https://github.com/helm/helm/issues?q=is%3Aissue+is%3Aopen+label%3Abug+helm+chart+save

At least 1/3 of the open issues labeled "bug" in Helm are related to the experimental OCI flag, so there's much work left to be done. Copying this code from Helm means you'll just be inheriting all those bugs into your code before we've had the chance to fix them.

Contributions/help is always welcome. If you'd like to help I'd be happy to point you in the right direction.

@hiddeco
Copy link
Member

hiddeco commented May 8, 2021

@bacongobbler thanks for jumping in on this issue as a Helm maintainer, much appreciated.

I do agree with all of your points and am well aware of the outstanding issues, which is the reason I did not copy things from internal to our own APIs, and am continuing to hold the boat till the API design has been finalized and made publicly available. At times this has been challenging to defend, as people see that cloud vendors have started with adoption. But your comments here should make this easier now, so again, many thanks 🌻

Although my available time to spend on other open-source projects while maintaining a healthy work/life balance is limited, I do find it quiet painful that we rely heavily on Helm – and I am personally well informed of its architecture – while my contributions have been limited.

Given this, I will take a look and see if I can find some time (or increased priority) to help with some of the issues you linked to.

I also noticed that plans are being made for the start of the Helm 4 development phase. If the Helm project is looking for feedback to improve SDK usage, I would be more than happy to have a chat.

@antoinedeschenes
Copy link

Major changes in the OCI implementation in Helm 3.7.0:
https://github.com/helm/helm/releases/tag/v3.7.0

@hiddeco
Copy link
Member

hiddeco commented Oct 1, 2021

With all OCI related APIs still being internal, nothing has changed for us. But the fact that things are moving is promising :-).

@bburky
Copy link

bburky commented Oct 5, 2021

Actually it appears since Helm v3.5.0 there was a public OCIGetter, but it wasn't connected to the normal helm code: https://pkg.go.dev/helm.sh/helm/v3@v3.7.0/pkg/getter#OCIGetter

With Helm v3.7.0, it looks like the oci scheme may just work with getter.Providers.ByScheme now?
https://github.com/helm/helm/blob/v3.7.0/pkg/getter/getter.go#L171-L180
And there's a WithRegistryClient(), but I suppose you can't construct a registry client still.

I think that's all public APIs to fetch the chart now? I don't know enough about helm to know if that's enough for you to use it in flux though.

@bburky
Copy link

bburky commented Oct 6, 2021

@hiddeco It almost seems usable, I tested enabling the oci protocol and you can implement most of source-controller with only public APIs. Helm's currently exposed API is limited in a few ways though, or I just don't know how to fully use it.

Currently flux supports semver on helm chart versions. To support this with OCI you would need to fetch the list of tags for an OCI image and do semver matching against them. I skipped this for now, I'm not sure if Helm provides any APIs to list these.

source-controller currently fetches the index yaml of HTTP HelmRepositories. It seems that Helm does store these in the OCI config as JSON, but I don't see a way to fetch this with OCIGetter. I think some flux features will need that (I think I saw something about remote chart dependencies?), and it has some useful metadata.

See main...bburky:oci-hack-test for the very hacky testing I did, but it was mostly enabling the getter for the oci scheme (I used getter.All(&cli.EnvSettings{})), and commenting out any index related code for now. It's possible to take advantage of the existing relative path handling in DownloadChart() and have repostitory.Get() return a ChartVersion with a relative path. I also upgraded to v3.7.0 to use the new Helm OCI format, but I don't think OCIGetter changed too much in this Helm update.

I used the following flux configuration for testing:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: test
  namespace: flux-system
spec:
  interval: 5m
  chart:
    spec:
      # This gets joined with the HelmRepository URL to become oci://172.17.0.1:5000/test/test:0.1.0
      # I couldn't find any other easy way to pass a name and version in ChartVersion to DownloadChart()
      # This does appear to match how `repository:` is handled in `dependencies:` which joins the name to the URL similarly
      chart: test
      version: "0.1.0"
      sourceRef:
        kind: HelmRepository
        name: test
        namespace: flux-system
      interval: 1m
---
apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: HelmRepository
metadata:
  name: test
  namespace: flux-system
spec:
  # This doesn't really work on 5000, it will require TLS without some further changes.
  url: oci://172.17.0.1:5000/test
  interval: 10m

@bacongobbler
Copy link

There are several technical reasons why we couldn't move that code under internal/experimental. But those exposed API endpoints are still being treated as though they were experimental, meaning you should not be relying on those APIs as they are subject to change.

I'm getting kinda tired of repeating this, but it bears repeating: this code is experimental. Please do not attempt to put experimental features in your code. Things will break. This is expected to happen as we iterate. We will not provide support when that happens.

@hiddeco
Copy link
Member

hiddeco commented Oct 6, 2021

While I do understand the desire of you all to move to OCI, we do not want to tire our beloved Helm maintainers with our issues.

When we can integrate this is clear to all controller maintainers, and I am locking the conversation to this select group of people until we have determined things to be GA on Helm's side.

@hiddeco
Copy link
Member

hiddeco commented Jan 31, 2022

With Helm 3.8.0 released and OCI being out of experimental, this is now ready to be worked on. It will require a bit of design tinkering to work well in combination with #450. @souleb is currently investigating our options, and is hoping to share more on this later in the week.

@hiddeco hiddeco removed the blocked/upstream Blocked by an upstream dependency or issue label Feb 1, 2022
@simingweng
Copy link

@hiddeco @souleb we're integrating with FluxCD Toolkit and OCI support is really important feature. I'm keen to help, please let me know if anything I can start with.

@souleb
Copy link
Member

souleb commented Feb 9, 2022

We are working on a design doc. Would be nice to have feedbacks.

@simingweng
Copy link

@souleb @hiddeco I've left some comments on the design doc. Basically, my angle is from modeling the OCI Source CRD on Registry rather than Repository, which, I feel, makes it easier to reason that a single generic OCI source can produce different types of Artifact (of different config.mediaType), helm chart, kustomize bundle, etc.

By the way, I found hackmd not really friendly in terms of commenting, there's a maximum number of character for each comment that forced me to break my comments into chunks, and also I couldn't edit any comment once submitted.

Is it ok to migrate to a Google Doc or something?

@simingweng
Copy link

simingweng commented Feb 11, 2022

In the case of using OCI for helm chart, we define an OCI Registry and a HelmRelease

apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: OCIRegistry
metadata:
  name: my-local-registry
  namespace: flux-system
spec:
  interval: 10m
  url: localhost:5000
  auth:  
    secretRef:
      name: regcred
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: my-chart
spec:
  interval: 5m
  chart:
    spec:
      chart: my-chart
      version: '4.0.x'
      sourceRef:
        kind: OCIRegistry
        name: my-local-registry
        namespace: flux-system
      interval: 1m

helm-controller still creates the HelmChart resource as it does today.

source-controller reconciles the HelmChart and knows it's targeting OCI registry, so it uses GET /v2/my-chart/tags/list Registry API to determine the desired tag, then delegates to helm OCI getter from Helm project to pull the chart.

In the case of using OCI for kustomize bundle, if the bundle is pushed to the same registry (with a unique mediaType application/vnd.fluxcd.kustomization.config.v1+json, we can even build this capability in flux CLI so we have a defined user experience in handling kustomization bundle with OCI), we don't need to define another instance of OCIRegistry, we can add a new field bundle to the kustomizationSpec:

apiVersion: kustomize.toolkit.fluxcd.io/v1beta2
kind: Kustomization
metadata:
  name: my-kustomization
spec:
  interval: 5m
  bundle: #ignored if source reference is not OCIRegistry
    name: my-kustomization
    #oneof
    tag: "1.0.x"
    digest: "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a"
  path: "./deploy/webapp/"
  prune: true
  sourceRef:
    kind: OCIRegistry
    name: my-local-registry
    namespace: flux-system
  wait: true
  timeout: 2m

the kustomize-controller then create a resource KustomizationBundle, equivalent to HelmChart:

apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: KustomizationBundle
metadata:
  name: my-kustomization
  namespace: flux-system
spec:
  baseName: my-kustomization #ignored if source reference is not OCIRegistry
  tag: '1.0.x' #ignored if source reference is not OCIRegistry
  path: "./deploy/webapp/"
  sourceRef:
    kind: OCIRegistry
    name: my-local-registry
  interval: 5m

@souleb
Copy link
Member

souleb commented Feb 14, 2022

@simingweng what about putting this in the alternative design section?

@stefanprodan
Copy link
Member

I'm not for introducing two types for Helm and Kustomize to source-controller API. I would very much prefer to reuse the OCIRepository type in both kustomize-controller and helm-controller APIs sourceRef. The OCIRepository implementation is here: #450

@hiddeco
Copy link
Member

hiddeco commented Feb 14, 2022

Isn't the OCIRegistry part a simple rename from OCIRepository -> OCIRegistry? Which might be a better name indeed.

@makkes
Copy link
Member

makkes commented Feb 14, 2022

Isn't the OCIRegistry part a simple rename from OCIRepository -> OCIRegistry? Which might be a better name indeed.

Not exactly. The difference is that an OCIRepository would point to a concrete repository like registry.gitlab.com/makkes/repo whereas the idea of a OCIRegistry is more generic.

With an OCIRepository you would have to create one such resource per Helm chart.

With an OCIRegistry you would create one resource for as many Helm charts or other actual resources you'd like to consume from the registry.

That's how I understand both suggestions. Please correct me if I'm wrong @souleb or @simingweng.

@simingweng
Copy link

@makkes yes, that's where I came from. The fundamental reason that makes me want to try model it at Registry level is that in current HelmRelease API, we offer HelmRelease.chart.version to filter desired versions of chart, and at the same time, in the suggested OCIRepository, it seems we also want to offer the same thing via ref or filterTag, etc.

Do we want to allocate this "version selection" semantics in a single resource, rather than overlap in both the "source" resource and the "consuming" resource? I know it sounds too much like a refactoring of existing API, which may not be the goal of this OCI support effort. Then, we just need to educate the user clearly which field takes priority when both the OCIRepositry and HelmRelease carries version selection intentions.

@simingweng
Copy link

@simingweng what about putting this in the alternative design section?

Yes, I can.

@souleb
Copy link
Member

souleb commented Mar 4, 2022

@simingweng We have updated the doc. Please review it.

@vkryzh

This comment was marked as off-topic.

@rashedkvm
Copy link
Member

Hi flux firends! I am actively working to complete PR #450. I am updating the OCIRepository reconciler implementation to match the new standard used in other reconcilers within this project.

Additionally, I propose to add "CertSecretRef" to OCIRepository spec as suggested in PR #450. The "CertSecretRef" will be optional and can have a name of a secret that contains certificate data. It will be used for connecting to the registry. I find this useful for registry that has certificates issued by a Private CA (non-public Cetificate Authorties)

@stefanprodan
Copy link
Member

@rashedkvm please hold off the implementation, we need to decide on how the API will look.

Everyone please add your feedback on the Flux OCI support for Helm, once the RFC is approved we can move forward with the implementation.

@stefanprodan
Copy link
Member

RFC-0002 is now marked as implementable, thanks everyone for your feedback. The implementation can be tracked in #669

@pjbgf pjbgf added this to the GA milestone Apr 13, 2022
@makkes makkes added the area/oci OCI related issues and pull requests label May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Helm related issues and pull requests area/oci OCI related issues and pull requests enhancement New feature or request
Projects
None yet
Development

No branches or pull requests