Consider dropping the use of `global` values to ease chart reuse

[svvac]

Currently, the chart uses the global key in the values to store common parameters for the various components deployed by the chart.

However, this complicates reusing the chart as a dependency due to the fact that values under global are shared across all (sub)charts in a deployment.

Because of this, and the fact that the values defined by the openbao chart use very generic names (e.g. enabled, imagePullSecrets, namespace), reusing the chart as-is risks creating conflicts with other charts that use the same global names for different purposes.

This chart does not have dependencies, thus has no need for global values. If they were moved to e.g. common, it would minimize cross-chart incompatibilities in the global namespace.

Currently, your options are:

• ensure that the openbao chart is not pulled along some other dependency that has conflicting uses in the global values
• install the openbao chart separately
• fork the chart to fix the names

This would probably be a breaking change in the chart, unless a fallback mechanism is put in place somehow

[Nerkho]

Hi @svvac, thanks for bringing this up.

So as I understand it, you are using the OpenBao chart as a subchart, and when defining global values in the main chart they supersede those in the OpenBao chart ? This indeed is something we should improve.

If you have a proposal for how this could be implemented while maintaining backward compatibility, we will happily review the PR. If not possible to maintain compatibility, we will need to consider the breaking change.

[svvac]

I think we could do backward compat using a useLegacyGlobalConfig flag in the values, defaulting to true, that controls where these values are looked up. In due time, the flag could be flipped to deprecate the global config table and ultimately remove it.

Obviously, this would result in some rather unwidely template expressions, but given the few instances I guess that can be acceptable by Helm standards: (assuming a removal of the global. prefix and the flag name above)

openbao-helm/charts/openbao/templates/_helpers.tpl

Lines 59 to 61 in df82b0f

	{{- $_ := set . "injectorEnabled" (or
	(eq (.Values.injector.enabled | toString) "true")
	(and (eq (.Values.injector.enabled | toString) "-") (eq (.Values.global.enabled | toString) "true"))) -}}

{{- $_ := set . "injectorEnabled" (or
  (eq (.Values.injector.enabled | toString) "true")
  (and
    (eq (.Values.injector.enabled | toString) "-")
    (eq ((.Values.useLegacyGlobalConfig | ternary .Values.global .Values | default dict).enabled) | toString) "true"))) -}}

This could be abstracted away in a template with some creative use of fromYaml, leaving you with a single place where to control these options, which could enable some more creative merging if needed:

{{ (include "openbao._common_config" . | fromYaml).enabled }}

[svvac]

Using the template helper, I'm pretty sure we could avoid having to maintain two copies of the default values currently under global by using mergeOverwrite of the (empty by default) global dict into the destination in legacy compat mode.

[Nerkho]

Thanks for the suggestions. :)

@pree @eyenx I'd be happy to have your input here.

[pree]

From what I've seen so far global is mostly used to disable all chart functionality and enable them individually.
I don't really see how we could implement this without a breaking change, other than modifying the helper functions with a default true value as you suggested.

So maybe we could disable everything by default and introduce the breaking change, so that every component is enabled implicitly?

Regarding the other values (imagePullSecrets, externalBaoAddr, etc) we would need to check for each one of them.

Regarding the global value sharing with subcharts, when the OpenBao chart is configured as a sub-chart, it's default values shouldn't override other charts values, even global ones. I think this only holds true for global values of the parent chart. As those values can be overwritten for each component individually I don't see this as a blocker of using the OpenBao chart as a sub-chart, or am I missing something?

[svvac]

Regarding the global value sharing with subcharts, when the OpenBao chart is configured as a sub-chart, it's default values shouldn't override other charts values, even global ones. I think this only holds true for global values of the parent chart. As those values can be overwritten for each component individually I don't see this as a blocker of using the OpenBao chart as a sub-chart, or am I missing something?

Sorry if I wasn't clear. I can confirm that the global values from the openbao chart don't «bubble up» to the parent chart. My issue is the other way around: by using e.g. global.imagePullSecrets in your chart, you pretty much forbid me to use the same key in any of my other charts for another purpose.

I see global as an escape hatch to allow otherwise impossible coordination between several charts in Helm. Since this chart is self-contained, I would prefer if it did not reach into that mechanism without an actual use for it, freeing up the global namespace.

[pree]

I agree that global isn't a nice solution, but introducing a breaking change is also not that nice.

you pretty much forbid me to use the same key in any of my other charts for another purpose.

Sorry, but I don't get why this prevents you from using the same values in another chart?

[svvac]

My main umbrella chart values would need look something like:

# I use global for some stuff somewhere
global:
  env:
    yolo: whatever
  
  # these values configure the openbao chart:
  enabled: false
  imagePullSecrets: {}

# Here I pull in a subchart and configure it
some-component:
  enabled: true
  imagePullSecrets: {} # ;—)
  # itself pulls a subchart:
  sub-sub:
    ad: infinitum

# I also need openbao!
openbao:
  # imagePullSecrets: see in global
  injector:
    enabled: true
  # ...

The openbao config is scattered across, takes up valuable names, and is harder to validate. Now if another chart wants to use global.enabled, I can't have it and this one in the same parent chart. (it gets worse as openbao is pulled in deeper in the dependency tree)

Regarding the breaking change, I believe the approach I suggested would allow me to set openbao.useLegacyGlobalConfig=false and free up my global namespace, while defaulting to the current behavior when used directly or otherwise until you want to pull the plug.

[svvac]

Regarding maintainability, I don't know why I didn't see it immediately:

{{ $common := include "openbao._common_config" . | fromYaml }}

 {{- $_ := set . "injectorEnabled" (or 
   (eq (.Values.injector.enabled | toString) "true") 
   (and (eq (.Values.injector.enabled | toString) "-") (eq ($common.enabled | toString) "true"))) -}} 

[pree]

Yes I get that the global config also applies to openbao, but you can still overwrite it inside the openbao chart without issues. So while not being nice right now, I don't see why you can't.

For example, instead of setting:

global:
  enabled: false

you can configure:

openbao:
  injector:
    enabled: false
  server:
    enabled: false
  ...

Have you tried if

openbao:
  global:
    enabled: false

would work? This would be interesting.

[svvac]

Don't have them on hand for reference, but I believe in my tests the openbao.global just gets dropped.

[svvac]

Also, if you think about it, every new openbao feature that touches global would technically be a breaking change for me :—°

[pree]

Btw, what's your use-case for pulling in openbao as a dependency? I see with the added features like declarative audit log configuration and auto initialization that it makes this feasible for a limited use case. But in general people are deploying OpenBao centrally (or once per cluster) and have the applications consume it, similar to databases.

[svvac]

It's mostly about having an all powerful master chart that can spin up the entire product stack as needed.

(btw, IIUC Gitlab is looking at integrating a solution like yours in their stack, and from my experience with their deployments they might have a similar use-case) well they obviously do their own.