Clean up values.yaml files

[faisal-memon]

The files are getting a little messy and can be organized better.

• Locate all SPIRE related config together and at the top of the file
• Locate all K8s config together
• Add comment header to each section
• Add comments for all SPIRE related config that matches the official docs
• (optional) Nest SPIRE Server config under a server: section

[mrsabath]

I volunteer to add the probes details parameters as I did with Tornjak frontend: https://github.com/spiffe/helm-charts/blob/main/charts/spire/charts/tornjak-frontend/values.yaml#L78-L90

[edwbuck]

Can we describe the goals here? This seems to be an unbounded task without them, and unless we can set a few goals, it might even be an entirely opinion driven task.

[edwbuck]

@mrsabath Was the work you did on the probes sufficient enough to close this issue.

@marcofranssen @faisal-memon @kfox1111 It seems that my call for well defined goals was never answered. If we don't see a more specific goal by the end of the week, I propose we close this issue as too vague.

[krishnakv]

Hi @edwbuck , happy to pick this up. Will generate a PR this week with the approach/ understanding that helm show values from the command line should generate all the relevant values with comments. This will help an operator understand the overall chart and generate a modified YAML for their purpose, all from the command line.

[faisal-memon]

Thanks @krishnakv for volunteering.

[kfox1111]

Just to double check, I think the plan was to find the most common config options most users will need and copy those in with comments. Lots of the options will be rarely used and we don't plan on putting those in the parent chart?

[marcofranssen]

I suggested to put a single comment to the readme.md section with values which holds all options already so we don't have to duplicate the info. Just a hyperlink so people searching there are directed to location where they can find it.

Duplicating then is a bad idea IMHO.

[kfox1111]

It wasn't to copy all the options, it was to copy the few options that users would likely need in the normal useage of the chart, then remove the flag that sets all the subchart values into the parent's readme?

[marcofranssen]

It wasn't to copy all the options, it was to copy the few options that users would likely need in the normal useage of the chart, then remove the flag that sets all the subchart values into the parent's readme?

That would be a bad choice IMHO. then we would end up with a half documented chart + the fact for advanced users that have to dive deeper things will get more complicated. What we should do is add those --@ignore annotations for things we really want to keep out of the docs.

Probably we should also add more clear documentation.

[kfox1111]

https://spiffe.slack.com/archives/C04BURZP1KK/p1689098579028339 has record of the consensus.

[krishnakv]

Hi Folks, just to add the reasoning behind why this values format i.e. all allowed values with commentary and examples is typically maintained in the default values file.

1. Helm charts are self describing and most operators expect this format
2. A workflow involving cutting and pasting from a README file is messy as YAML is whitespace sensitive - with this default, operators just to have uncomment a few lines and fill in their values
3. Again, most charts have all values in the public interface because users will just skip past or delete the infrequently used options (e.g. tolerations??)
4. The fact that we are using sub-charts to organize - a very good choice given the complexity of what we are trying to accomplish - is an implementation detail and should not dictate the public interface

When initially using the chart, I had to spend quite a bit of time getting into the subp-charts and understanding what has been set and not set as defaults (e.g. the default SPIFFEE ID format), this will greatly reduce adoption esp with people who just want to test out Spire and dig into what all the chart can do. It is a bit more work as we need to make changes in the values.yaml also when changing functionality but JMHO its something we would need to setup before we hit v1.0.

Increasing adoption will also help us with understanding what demand exists for features and what to focus on. Happy to discuss - if there is any other way to achieve this workflow for our users - i.e. examining allowed values and crafting custom values from the command line, we can go with that.

Just to verify the existing best practice/ convention spent sometime going a mix of charts in artifact hub: oci://registry-1.docker.io/bitnamicharts/postgresql, prometheus-community/kube-prometheus-stack, jetstack/cert-manager, argo/argo-cd. They all follow some variation of exposing all allowed values with description/ examples.

[edwbuck]

@krishnakv Please read Issue 406. It is currently blocked on Issue 405.

I believe that 406 better captures the overall spirit of the call to "clean this up" than this issue does. If you see an additional effort not covered in Issue 406, please stand up a new issue. At least with 406 we can know if the effort is complete for a topic.

By reducing the scope to smaller items, we can make progress because we will know when the smaller item is completed. A general "clean up" task will never be completed, as there will always be some way to perform "more additional cleaning"

When you believe all the known concerns are documented with acceptance criteria, please post your opinion that this item should be closed as a comment.

[krishnakv]

Hi @edwbuck , will look through the issues and move this under #406 . With that, the entire scope will be covered under #405 and #406.

[marcofranssen]

Implementation is being done in #431