Citi / gradle-helm-plugin

A suite of Gradle Plugins for building, publishing and managing Helm Charts
https://citi.github.io/gradle-helm-plugin
MIT License
9 stars 6 forks source link

[Feature Request] implement dealing with OCI registry #43

Open bademux opened 4 months ago

bademux commented 4 months ago

Implement dealing with OCI registry Migrating Feature Request https://github.com/unbroken-dome/gradle-helm-plugin/issues/155

bsgrd commented 2 months ago

Are there any plans to implement this feature? I would also consider giving it a go myself, if I can get a pointer of where to start? 😃

JamieSlome commented 2 months ago

@imanushin @magner669 - what do we think about @bsgrd's proposal?

bademux commented 2 months ago

Are there any plans to implement this feature? I would also consider giving it a go myself, if I can get a pointer of where to start? 😃

How about adapting the snippet (https://github.com/unbroken-dome/gradle-helm-plugin/issues/155#issuecomment-1267202124) to actual helm-plugin architecture\language

imanushin commented 2 months ago

@bademux, thank you. It seems like it would be very easy to add this support. However the question is how it would be better to test this feature.

I will check some options and return later. If there are any approaches which could be applied via GitHub Actions then I will be happy to add them.

bademux commented 2 months ago

You need to decide what are you testing (the scope). If you doing some integration testing, just mock helm invocation and check arguments passed to the mock. If you more to blackbox/acceptance testing, setup mock rest server (wire mock). Helm is just rest client to k8s API with templating features.

magner669 commented 2 months ago

This is an important feature. I would be happy to help out.

I think we would need unit testing with mocks , but we also need a test with a real OCI repo.

Do you have one handy @bademux ?

bademux commented 2 months ago

This is an important feature. I would be happy to help out.

I think we would need unit testing with mocks , but we also need a test with a real OCI repo.

Do you have one handy @bademux ?

AWS ECR supports OCI, Artifactory as well.

There is official Docker of OCI registry https://hub.docker.com/_/registry. It will be handy for blackbox testing with testcontainers-java; unfortunately I can't say anything about last one, as I'm not using it.

Technically helm OCI image is 2 layers - first with manifest and second is compressed tar full of yamls. So assertion in test is really simple with this knowledge.

magner669 commented 2 months ago

Good.

I'd just like to clarify what I think shuld be the test requirement here.

In the build, I think we shouldn't have a dependency on some helm chart with some version at a third party/external OCI repo. By murphys law, it will break and we will have to fix it in a hurry . That's why I suggested to use mocks.

It would be suffecient to manually test with a real repo and provide a screenshot in the PR as test evidence.

iamaverrick commented 1 month ago

Hello,

I’m currently using the Gradle Helm Plugin to manage Helm charts within my project. We have a requirement to use OCI-based Helm charts hosted in a Docker registry (e.g., Docker Hub or Harbor).

As Helm now natively supports OCI registries (via the HELM_EXPERIMENTAL_OCI=1 environment variable), we attempted to configure the plugin to work with our OCI registry. However, the plugin does not support this out of the box, leading to issues when updating chart dependencies in an OCI registry. Critical feature.

JamieSlome commented 1 month ago

@imanushin @magner669 - following on from @iamaverrick comments above, can we think about what we need to push this over the line?

Is anyone willing to support in the development of this?

bademux commented 1 month ago

Hello,

I’m currently using the Gradle Helm Plugin to manage Helm charts within my project. We have a requirement to use OCI-based Helm charts hosted in a Docker registry (e.g., Docker Hub or Harbor).

As Helm now natively supports OCI registries (via the HELM_EXPERIMENTAL_OCI=1 environment variable), we attempted to configure the plugin to work with our OCI registry. However, the plugin does not support this out of the box, leading to issues when updating chart dependencies in an OCI registry. Critical feature.

no need to set HELM_EXPERIMENTAL_OCI.

iamaverrick commented 1 month ago

Hello, I’m currently using the Gradle Helm Plugin to manage Helm charts within my project. We have a requirement to use OCI-based Helm charts hosted in a Docker registry (e.g., Docker Hub or Harbor). As Helm now natively supports OCI registries (via the HELM_EXPERIMENTAL_OCI=1 environment variable), we attempted to configure the plugin to work with our OCI registry. However, the plugin does not support this out of the box, leading to issues when updating chart dependencies in an OCI registry. Critical feature.

no need to set HELM_EXPERIMENTAL_OCI.

How would I use this plugin with registries the already adapted OCI, such as Harbor, etc? This plugin should implement the ability to use either or.

bademux commented 1 month ago

@iamaverrick I'm not sure I understand the question, but OCI registry references are always prefixed with oci:// oci as per helm v3.8.0 is no longer experimental, ref https://helm.sh/docs/topics/registries/

iamaverrick commented 1 month ago

@iamaverrick I'm not sure I understand the question, but OCI registry references are always prefixed with oci://

oci as per helm v3.8.0 is no longer experimental, ref https://helm.sh/docs/topics/registries/

Ok, basically when using the plugin I'm unable to use the plugin due to the fact that the plugin uses URL, instead of OCI, so the builds fails. I understand that OCI is no longer an experimental feature it's stable in helm, and harbor. This plugin doesn't support OCI, if it does please advise how. Thank you in advance

bademux commented 1 month ago

@iamaverrick as mentioned several comments before, impl it by yourself as a custom task. https://github.com/unbroken-dome/gradle-helm-plugin/issues/155#issuecomment-1267202124