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

Add chartutil package and ValuesReference type to APIs #836

Merged
merged 1 commit into from
Dec 10, 2024
Merged

Conversation

stefanprodan
Copy link
Member

@stefanprodan stefanprodan commented Dec 9, 2024

This PR extracts the logic used by helm-controller to query and merge Helm values to a shared package called chartutil. In addition, the HelmRelease v2 ValuesReference was copied to pkg/apis. This will allow the reuse of values composition in Flux CLI.

Part of: fluxcd/flux2#5101

Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
@stefanprodan stefanprodan added the area/api API related issues and pull requests label Dec 9, 2024
@stefanprodan stefanprodan requested review from darkowlzz, hiddeco and a team as code owners December 9, 2024 10:56
@stefanprodan stefanprodan merged commit f58229d into main Dec 10, 2024
12 checks passed
@stefanprodan stefanprodan deleted the chartutil branch December 10, 2024 09:23

// ValuesReference contains a reference to a resource containing Helm values,
// and optionally the key they can be found at.
type ValuesReference struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the context of this package, the type seems to be generically named. LocalObjectReference, NamespacedObjectReference, SecretKeyReference, KubeConfigReference, etc have the context in the name prefix. Something like HelmValuesReference would have been better.
I'm assuming that because it originated in helm-controller API, it was named this way. But in this package, the name seems confusing compared to others in the package.

Since helm-controller now has an alias to this, if we want to, we can rename this to anything without breaking the helm-controller API.

It feels like it's too late for the change now. I wouldn't insist on the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants