Section for raw config for custom plugins

[faisal-memon]

Standard plugins should follow standard helm yaml syntax. To support custom plugins users have created we need a section where they can enter raw config for these plugins.

[kfox1111]

looked into this a bit... unfortunately yaml -> toml mapping can be a little trickly. what do folks think about: sclevine/yj:latest

to convert yaml to toml as an init container for this?

[kfox1111]

Seems like helm has a toToml function now, but isn't well documented? Strange... I'll try to play with it some.

[kfox1111]

I was mistaken. Its HCL, not TOML. :/ No support for that...

Interestingly, they look to have added json support at some point? https://spiffe.io/docs/latest/deploying/spire_server/#server-configuration-file

Though they don't have an example.... That would be MUCH easier to map.

[kfox1111]

https://github.com/spiffe/spire/issues/2808

Its a little ugly, but does seem to work.

[faisal-memon]

As an alternative, can we just use a multi line string and embed that into the template? For example, in the values.yaml we would have this:

RawPluginConfig: |-
   NodeAttestor "my-custom-attestor" {
     plugin_data {
       my-data: "test"
     }
   }

You can define as much as you want in that section. Then in the in configmap template we just drop that variable in the plugins{...} section:

    plugins {
    ....
{{ .Values.RawPluginConfig | indent 6 }}
    }

I have a small POC in this branch: https://github.com/spiffe/helm-charts/tree/custom-plugin

[kfox1111]

Yeah, here doc style blobs work. They just aren't a very natural fit as your having to mix hcl and yaml in the same place. I prefer yaml all the way through the config.

[faisal-memon]

Yea I get that. I like consistency. I think the extra work and maintenance overhead isn't worth it though. Most people won't be using custom plugins. I'd like to go with this simpler approach first, and if it doesn't fit then we can always do the more complex solution later.

[kfox1111]

Is the complexity really just the fact that it started as hcl and needs to be converted to json? Would we be having the same conversation if it started off as json?

I'm ok doing the work. Already did about half of it.

Why do you think it would be more work to maintain? I think that's probably the most compelling argument to keep it as is.

The RawPluginConfig route could still be done with json as well if you like that option better then the individual plugin config. I can quickly prototype that as well.

[kfox1111]

Basically, pulled apart from the json conversion, it looks basically like this: https://github.com/spiffe/helm-charts/pull/35/files#diff-95a921e948f7af4dcca64089a1d722399e1ab49bf5b42f420c6ee4f42c3cdd25R42-R46

Which is pretty similar to your prototype.

[faisal-memon]

I appreciate the desire to maintain consistency. I don't believe this change is the right direction for this project at this time for the following reasons:

1. Id like to get a release out soon. Reviewing and testing a refactor will only delay that.
2. Having a few lines of hcl mixed in with json is not that big of a deal. ingress-nginx does something similar to allow custom nginx snippets, which I think most people are ok with.
3. Philosophically I don't like the idea of designing the whole solution around a valid but not common use case.
4. It seems to be a runtime solution with additional init containers. If I understand the change correctly, that will mean helm template will not give an accurate rendering.

Im happy to revisit this at a later time, I'm not ruling it out. I'd like to think it over for a bit. For now we have a simple solution that solves this problem so lets move forward with that.

[kfox1111]

Some thoughts.

1. I think that's premature. There's still a bunch of stuff I think that needs to be done before its release worthy. If we release to early then we will want to maintain backwards compatibility with parts of the charts and make it significantly harder to clean up the chart.
2. I agree, but there's other considerations not considered yet that weigh higher I think. For example, a lot of the config is currently hard coded. If we continue on the current path, we have to template out every one of them. If we switch to json, we can push them into values and import them into the json doc directly. Greatly simplifying the end template. I plan on exploring that option.
3. +1. But as with 2, I don't think that's the only design goal though.
4. It doesn't need init containers at all. I think you misunderstand the pr. helm template renders out directly. thats one of the benefits of using the json format.

I have a pr that just does the json conversion. I'll build a second pr on top of that to do config passing via values and lets take a look at that. it may be significantly easier to see what I'm trying to do?

[kfox1111]

Here's a pr that shows off that route. Everything in the server section is now configurable by the end user. https://github.com/spiffe/helm-charts/pull/48 (superseded by the next patch. may not want to review this)

[kfox1111]

And here's a final prototype of layering onto the json config prototype configuring spire. https://github.com/spiffe/helm-charts/pull/49 (consider reviewing the raw changed configmap. The patch is large because of the reformatting.)

Basically everything in the server section is now configurable, and like your example, any plugin can be merged into the config via values. but all the config is proper yaml.

The templates are now yaml rather then json too. Its converted to json at the very end with a function so we don't have to deal with the json at all.

[kfox1111]

Partially implemented in https://github.com/spiffe/helm-charts/pull/198

Still need the agent equiv.