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

[feature] Allow reading Helm values from custom resources #5167

Open
simonostendorf opened this issue Jan 31, 2025 · 8 comments
Open

[feature] Allow reading Helm values from custom resources #5167

simonostendorf opened this issue Jan 31, 2025 · 8 comments

Comments

@simonostendorf
Copy link

I would like to get helm values from other sources than ConfigMap or Secrets.

I think it would be really useful to allow every kind of kubernetes resource and specify a path where the value should be read from.

Example:

I have a CRD called MyProviderNetwork. The controller for this crd creates a new cloud provider network in the background and writes the id to .status.networkId. It would be really cool to read this value as a flux value.

Example Configuration for Flux:

apiVersion: helm.toolkit.fluxcd.io/v2
kind: HelmRelease
...
spec:
  valuesFrom:
    - kind: MyProviderNetwork
      name: resource-name
      valuesKey: helmValueWhereNetworkIdShouldBeWritten
      path: .status.networkId
      optional: false
@stefanprodan
Copy link
Member

We'll need a diffrent field if we want to do this, as we can't break the current API. You can't refer to a custom resource by kind, you also need the exact apiVersion. In your example, you mixed up valuesKey, this field points to .data inside the ConfigMap/Secret, also there is no path, it's targetPath and this points to the Helm values. Docs here: https://fluxcd.io/flux/components/helm/helmreleases/#values

My proposal would be to add a dedicated section valuesFromResource e.g.

apiVersion: helm.toolkit.fluxcd.io/v2
kind: HelmRelease
...
spec:
  valuesFromResource:
    - apiVersion: network.foo.bar/v1
      kind: NetworkProvider
      name: my-network
      sourcePath: "/status/networkID" # JSONPath for querying the custom resource 
      targetPath: "network.id" # YAML dot notation to the Helm values field
      optional: false

I'm not sure if JSONPath is the best query language, maybe CEL would be better.

@stefanprodan stefanprodan changed the title [feature] allow more helm value sources [feature] Allow reading Helm value from custom resources Feb 1, 2025
@stefanprodan stefanprodan changed the title [feature] Allow reading Helm value from custom resources [feature] Allow reading Helm values from custom resources Feb 1, 2025
@simonostendorf
Copy link
Author

That's a way better proposal, thanks!

Would be really cool if this is possible.

@matheuscscp
Copy link
Member

matheuscscp commented Feb 1, 2025

The counter-proposal API is very close to the one we have today. If reusing .spec.ValuesFrom (which is a better UX), would it be too bad to choose and document the following?

  • We introduce apiVersion as optional. When apiVersion is not set, kind can only be ConfigMap or Secret.
  • We introduce the valuesExpr CEL expression (same as sourcePath in the counter-proposal). This field is compatible with any resources, including ConfigMap and Secret.
  • We deprecate valuesKey but still support it only for ConfigMap and Secret (at least for several releases), and print a deprecation warning in the controller when used.
  • The fields valuesExpr and valuesKey cannot be both specified. We can use the API Server CEL validation to prevent this (and also error out in the reconciliation logic in case an object like this lands in etcd anyway).

This would not break the current API and is much closer. I feel like introducing a new field only for CRs would make the overall API more confusing :/

@stefanprodan
Copy link
Member

We deprecate valuesKey but still support it

We can't do that in v2, this would require a v3.

We introduce the valuesExpr CEL expression

Can CEL be used to extract random structs from YAML? as in base64.decode(data."values.yaml") should return apiextensionsv1.JSON. We may need to fork CEL and do something like https://github.com/google/cel-policy-templates-go/blob/master/policy/parser/yml/parser.go

@matheuscscp
Copy link
Member

matheuscscp commented Feb 1, 2025

We can't do that in v2, this would require a v3.

Can we support valuesKey only for ConfigMap and Secret in v2, and if v3 ever lands we remove it?

Can CEL be used to extract random structs from YAML? as in base64.decode(data."values.yaml") should return apiextensionsv1.JSON. We may need to fork CEL and do something like https://github.com/google/cel-policy-templates-go/blob/master/policy/parser/yml/parser.go

Looks like there's nothing built-in for YAML/base64, but in the worst case we can load custom libraries like this:

https://github.com/fluxcd/notification-controller/pull/948/files#diff-04b2e531980aaa6c95108fd3ea7f9f4e88889309d86878a7fcaf25c5e11a9d44R129-R180

This wouldn't require a fork, just loading the library into the CEL environment.

Edit: For base64 there is, ext.Encoders(), which we already have in fluxcd/pkg/runtime/cel.

@matheuscscp
Copy link
Member

Hmm I see what you mean about the YAML return type now, yeah this needs more investigation.

@matheuscscp
Copy link
Member

Can pkg/chartutil depend on pkg/runtime/cel?

@matheuscscp
Copy link
Member

matheuscscp commented Feb 1, 2025

This would integrate pretty seamlessly with pkg/chartutil.ChartValuesFromReferences():

https://github.com/fluxcd/pkg/compare/cel-bytes?expand=1

And CEL provides base64.decode() and base64.encode(), but that would'nt even be needed, see the test cases :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants