budimanjojo / talhelper

A tool to help creating Talos kubernetes cluster
https://budimanjojo.github.io/talhelper
BSD 3-Clause "New" or "Revised" License
295 stars 17 forks source link

Plan for the upcoming upstream gen secret subcommand #13

Closed budimanjojo closed 2 years ago

budimanjojo commented 2 years ago

Following the merged PR in the upstream talos: https://github.com/siderolabs/talos/pull/5870. It's really a great idea that I didn't think of at the time I do this. Now I have some ideas about how this should be implemented. Which approach should I go?

  1. Have talhelper gensecret to output a yaml encoded data like talosctl gen secret. talhelper genconfig will read a file called talsecret.yaml if it exists to generate the manifests. This will preserve the current talenv.yaml and no breaking change. The talhelper gensecret --path-configfile flag will be deleted though.
  2. Have talhelper gensecret to output a yaml encoded data like talosctl gen secret. talhelper gensecret --patch-envfile will patch the talenv.yaml file a new field secretBundle that contains the mapping of the secret. talhelper genconfig will generate the manifests with those secrets if it's specified and this shouldn't be a breaking change too. The talhelper gensecret --patch-configfile flag will be deleted too.
  3. Ignore them and just continue doing it the current way.

I will begin working when the talosctl version with this PR is released and when I have decided which route to go.

szinn commented 2 years ago

I think the end-goal, which talhelper gensecret / sops -e achieves is to be able to add the generated secrets to the repo in the encrypted form.

So, if talosctl gensecret can achieve this then that's great and talhelper gensecret should just delegate to talosctl. If not, then I wouldn't change a thing and leave talhelper as is since it already achieves the end goal.

szinn commented 2 years ago

Another thing to remember is the current functionality might just need to stick around too unless you are going to do a migration to the talosctl gensecret mechanism

budimanjojo commented 2 years ago

Another thing to remember is the current functionality might just need to stick around too unless you are going to do a migration to the talosctl gensecret mechanism

Yes, the current functionality of talenv.yaml will always work no matter what, whatever you put inside will still be decrypted and being envsubst. It's more about separating cert secrets from env file. I always feel that the current implementation of --patch-configfile a bit of ugly because it put so many stuffs into talconfig.yaml.

szinn commented 2 years ago

So another idea to replace --patch-configfile on the gensecrets command is talhelper genconfig --with-secrets. It silently adds the configfile patches to the user's talconfig.yaml (if they aren't there) and goes from there. Source file is cleaner and the functionality is maintained.

budimanjojo commented 2 years ago

I think this should be good? https://github.com/budimanjojo/talhelper/pull/29

budimanjojo commented 2 years ago

https://github.com/budimanjojo/talhelper/releases/tag/v1.4.0

szinn commented 2 years ago

Works as expected - great job!