weaveworks / gitopssets-controller

Manages the GitOpsSet CRDs.
Apache License 2.0
9 stars 5 forks source link

Config generator #85

Closed bigkevmcd closed 1 year ago

bigkevmcd commented 1 year ago

This implements a new Config generator for generating from ConfigMaps and Secrets.

squaremo commented 1 year ago

This seems like a special case of lifting resources into generator values (e.g., you can imagine asking for all the Deployment objects); is it a principled limitation that you can only do this with ConfigMap and Secret, and only one of them?

bigkevmcd commented 1 year ago

This seems like a special case of lifting resources into generator values (e.g., you can imagine asking for all the Deployment objects); is it a principled limitation that you can only do this with ConfigMap and Secret, and only one of them?

I had thought that an extension of this might be a LabelSelector, and generate from all of those (for ConfigMaps and Secrets).

And I had also thought about genericising this (there's a similar mechanism elsewhere in this space), and yeah, we could definitely query generic resources, and JSONify them and expose them.

Config feels explicit, where as ResourceQuery doesn't?

squaremo commented 1 year ago

Config feels explicit, where as ResourceQuery doesn't?

Sure, yeah; this controller lean towards explicit use cases, rather than being the most general it can be, and that's a perfectly reasonable answer.

I had thought that an extension of this might be a LabelSelector, and generate from all of those (for ConfigMaps and Secrets).

If this would be a backward compatible extension, I can review this in good conscience.

bigkevmcd commented 1 year ago

One thing I'm acutely aware of, pulling random resources, and injecting them into templates is a bit scary, so loosening this up is always a consideration, folks putting your templates to uses you didn't expect (and they maybe didn't either).

bigkevmcd commented 1 year ago

If this would be a backward compatible extension, I can review this in good conscience.

Yeah, it'd need to be, the CR would be trivial to change, the watch/indexing mechanism would need a little bit of work too.