fix: make RBAC and supporting templates nil-safe for optional provider and hub blocks

[victorlane]

What does this PR do?

Several templates dereference .Values.providers.* and .Values.hub.* without guarding the parent map, so rendering panics with nil pointer evaluating interface {}.X whenever a user-supplied values file omits or nulls an optional block. The pre-existing ((.x).y) "nil-safe" form in rolebinding.yaml was also broken in a different way: it returned nil (not list) and concat panicked on the nil argument.

Motivation

Running into this issue while trying to deploy the helm chart

More

• Yes, I updated the tests accordingly
• Yes, I updated the schema accordingly
• Yes, I ran make test and all the tests passed

[darkweaver87]

Hello @victorlane 👋,

Thanks for this contribution 🏅
I'll have a deep look into it 👀
In the meantime, would you mind sharing the values.yaml which led you to open this PR? I'd like to understand the exact case — these blocks have full defaults in the chart, so the panic only shows up when hub: or providers: (or a child like providers.file:) is explicitly set to null rather than merged into.

Thanks :-)

[victorlane]

Hello @victorlane 👋,

Thanks for this contribution 🏅 I'll have a deep look into it 👀 In the meantime, would you mind sharing the values.yaml which led you to open this PR? I'd like to understand the exact case — these blocks have full defaults in the chart, so the panic only shows up when hub: or providers: (or a child like providers.file:) is explicitly set to null rather than merged into.

Thanks :-)

Hi Remi,

Below is a minimal values yaml that reproduces the issues. It only happens when an optional block is explicitly nulled rather than left out fully.

rbac:
  namespaced: true

providers:
  kubernetesIngress:
    enabled: true
  kubernetesCRD:
    enabled: true
  kubernetesGateway:
    enabled: false
  knative: ~
  kubernetesIngressNGINX: ~

[victorlane]

@darkweaver87 had time to look after this yet? We are experiencing issues with this bug in (prod) installations.

[darkweaver87]

Hi @victorlane, and sorry for the late follow-up — I know you're hitting this in prod, thanks for your patience and for the minimal reproducer, it's very helpful. 🙏

Looking at it, everything in your values comes down to these two lines:

  knative: ~
  kubernetesIngressNGINX: ~

As you noted yourself, the panic only happens when an optional block is explicitly nulled rather than left out. Both of these providers are opt-in and already enabled: false by default, so if you simply remove those two lines, the chart's defaults apply, nothing is nil, and it renders fine — no chart change needed on your side. Out of curiosity: is there a reason you need to null them explicitly instead of just not specifying them? That would help me understand whether there's a real use case to support here.

On the PR itself, my hesitation is that guarding parents path-by-path makes the chart only look nil-safe while leaving gaps:

• rbac: ~ still panics — every template uses .Values.rbac.enabled / .Values.rbac.namespaced unguarded.
• Even within the PR's scope it's incomplete: in requirements.yaml, ((.Values.providers.kubernetesIngressNGINX).modsec).enabled is now guarded, but the sibling (not $.Values.hub.token) on the same line isn't — and since Go template and evaluates all its arguments, nulling hub still panics there.
• The guarded blocks here (knative, kubernetesIngressNGINX) are opt-in and disabled by default, so their map always exists from the defaults; nulling a provider you aren't using is what triggers this, which makes the guards protect a scenario that doesn't really need to occur.

So there's a design question underneath: do we want to officially support "nulling any optional block"? If yes, I'd rather we do it once and consistently (e.g. normalizing optional blocks in a helper, or leaning on schema defaults) than scatter parentheses that every future template author has to remember. If not, dropping the explicit ~ on your side resolves it cleanly.

The default list ... fixes in the RBAC templates and the added tests are genuinely nice, though — that part addresses a real concat-on-nil case.

Let me know your thoughts, and thanks again for pushing on this! 🙌