Add missing global values to charts

[kfox1111]

If you try and install the chart its missing the global key at the root of the chart that many macro's are expecting. This pr adds the missing key so that it works.

[edwbuck]

If you try and install the chart its missing the global key at the root of the chart that many macro's are expecting. This pr adds the missing key so that it works.

Won't this just create the same problem when "Values.global.spire" is now missing?

[kfox1111]

If you try and install the chart its missing the global key at the root of the chart that many macro's are expecting. This pr adds the missing key so that it works.

Won't this just create the same problem when "Values.global.spire" is now missing?

The problem is a lot of the templates are staring from .Values.global and then looking for the rest optionally. For example:
https://github.com/spiffe/helm-charts/blob/main/charts/spire/templates/_spire-lib.tpl#L10

We can fix it in all the macros/templates, but there is a bunch of nontrivial logic that would have to be rewritten to make it work. Just adding the global section is enough to get it all working.

[marcofranssen]

I don't fully get in what occasion it fails? Could you describe the steps to reproduce the issue?

I'm successfully able to deploy the chart. Why do we need to add this?

[edwbuck]

I don't fully get in what occasion it fails? Could you describe the steps to reproduce the issue?

I'm successfully able to deploy the chart. Why do we need to add this?

If a template references a missing value, then it will either succeed or fail based on the parent tree of the value being present.

So, if you are attempting to read Values.globals.spire.namespace (as an example) there are lots of tools and tricks to handle if Values.globals.spire.namespace is missing, and even a few easy to implement approache for Values.globals.spire being missing, but once you go "two items up" you need to explicitly put in a lot of "default values" to make the logic for the the two preceeding examples to work, because they both would require Values.globals to be defined.

My main concern is that we are ignoring the core issue, should we have a "globals" value at all? It simplifies the value passing between charts, but only by making all nested charts reference a value in a "global" path. That means the sub-charts would then require a global object, even if they aren't sub-charts, and lack any kind of sharing.

Also the global approach is not cannon, it is a "pattern" to work around the issue of "where is the value" when a value in one sub-chart is needed in another sub-chart. The way it is "supposed" to work, from Helm's books and tutorials is that the child chart should "export" the value into a parent chart, which requires the parent chart to configure the "acceptance" of the value in the dependency. Then from the parent chart another nested chart could reference it without a "easy to break" relative path from one sub-chart to another.

And I'll come clean on this one. I've been searching for a week for any kind of documentation in a book or article that states that breaking an application comprised of micro-services into a bunch of sub-charts is desired, cannon, or even beneficial. I can't find it, and I used to do research professionally. Nested chart examples almost always present one of two examples:

• You're attaching a database to the application (this is a great example, but a database isn't generally considered a component of the application, and the data sharing is very small (just a user, possibly password, and a connection string, and most orgs can't keep their charts private, so they ban the use of a password, necessitating a more complex setup that the typical nested chart example)
• You're attaching relatively strong-ly separate things into a collection (like chapters into a book), where the sharing between components is relatively minor.

In my mind, that's not what we are doing, I really can't fathom the benefit of pulling out the OIDC connector and deploying it without a SPIRE server. I can't understand what use a registrar would be without a SPIRE server. And outside of a corner case in tired deployments, agents should deploy with servers. The corner case is easy to handle too, if you just make a "child" SPIRE deployment include the "parent's" agent with the child's server as a strong requirement.

That said, I've often permitted my ideas to take a back seat in the light of strong opposition. I have seen some strong opposition to non-nested SPIRE servers, agents, oidc connectors, and registrars / controller managers. I am putting trust into the team, but I wonder if the fascination with nested deployments are a bit like the fascination with operator overloading in C++. It's more complicated than most tasks, the last thing that one learns, and difficult to pull of bug-free, so naturally the problem solvers in us decide it's a requirement (I'm saying that in jest, but there's a kernel of truth in it).

I'm not against nesting, but I'd really like to limit it to databases or items that are clearly "other non-SPIRE applications".

[edwbuck]

@marcofranssen @faisal-memon @mrsabath I know that I've voiced some opinions on this matter where I restate my dislike for our use of nested charts for what I consider to be one application, but please don't let my statements block reviews. There is no need to agree with my comments.

If we have two passing reviews on this, even if I'm still holding reservations, let's merge it. But, please review it. It's been in this state for 2 weeks.

[faisal-memon]

We discussed this PR on the sync meeting today. This PR is intended to be a temporary fix to allow us to support individual charts. The reason is that the nested charts refer to global value, so they need a global section to be deployed independently.

Issue #343 is to track the proper fix of removing the reliance on global from the nested charts.

[marcofranssen]

LGTM 🚀