Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize the NewHelmValuesGenerator implementation #10

Open
markgoddard opened this issue Nov 13, 2024 · 1 comment
Open

Optimize the NewHelmValuesGenerator implementation #10

markgoddard opened this issue Nov 13, 2024 · 1 comment

Comments

@markgoddard
Copy link
Contributor

In addition to adding test coverage for the NewHelmValuesGenerator implementation (and it's associated methods), there may be several ways to improve its implementation, including:

  1. add a function to build quoted, dotted keys:
func keyName(sections ...string) string {}
  1. Build up sections incrementally:
func (g *HelmValuesGenerator) section(names ...string) helmValuesSection {}

type helmValuesSection struct {
    names []string
    generator *HelmValuesGenerator
}


func (s Section) section(names ...string) helmValuesSection {}

func (s Section) set(name string, val any) {}

// Then build up sections:

agent = g.section("spire-agent")
agent.set("logLevel", "DEBUG")
agent.section("server").set("address", "spire-server.spire")
@markgoddard
Copy link
Contributor Author

While working on #5 , I bumped up against the use of CUE to build the Helm values.

Comment from the code:

We need to merge the values generated by this function with the user-specified extra helm values.
CUE does not easily handle merging values with multiple levels of precedence, since it sees different concrete values for the same path as a conflict. It supports disjunctions with defaults, but that would only work for a 2-level merge.

For now we can start with the higher precedence user values, then merge in the defaults to paths that do not exist.
TODO: Revisit the use of CUE here, consider reworking to use native Go types with a merge function.

https://github.com/cofide/cofidectl/pull/51/files#diff-0d44b61e7d05cefa049422df7917d212564419fbf2a5a9964b79f51daaf5138fR144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant