budimanjojo/talhelper

Read machineFiles from a local file

mrueg opened this issue · 10 comments

It would be great if there were a similar mechanism for machineFiles similar to

if strings.HasPrefix(patchString, "@") {

so one could refer to an externally generates machineFile inside the talhepler config.

Thanks for the suggestion!

I have created a PR that will do just that, I'll do more testing tomorrow and then the next release will have this feature.

Try out the new release!

Thanks a lot for the quick solution @budimanjojo !

@budimanjojo do you think it would be possible to add another layer on top of this to also get the UX from patches? (I also understand if you don't wanna do it as it complicates things, because it's another indirection)

so my config could look like:

machineFiles:
  - "@./machine_files/node_1.yaml"

node_1.yaml

 - content: "@./tsauth.env"
   permissions: 0o644
   path: /var/etc/tailscale2/auth.env
   op: create

Sorry, but I don't quite understand the example you show above.

Sorry if I wasn't precise enough here, hope this is a better explanation:

Right now, you can point to a local file for the content, instead of inlining it.
My request was if we also additionally can point to an array of machineFiles similar to the patches.

patches:
  - "@./my-user-patchset.yaml"

that contains then the patch config.

Right now for this isn't possible for machineFiles:

machineFiles:
  - "@./machine_files/list_of_machinefiles.yaml"

as right now you'd need to inline the permissions, op, path etc. for a machineFile like this:

machineFiles:
 - content: "@./tsauth.env"
   permissions: 0o644
   path: /var/etc/tailscale2/auth.env
   op: create

If it were additionally possible to point to a list of machineFile configs, one could do the following:

machineFiles:
  - "@./machine_files/list_of_machinefiles.yaml"

list_of_machinefiles.yaml

 - content: "@./tsauth.env"
   permissions: 0o644
   path: /var/etc/tailscale2/auth.env
   op: create
 - content: "Hello World"
   permissions: 0o644
   path: /my/other/file
   op: create

I see, that might be impossible right now because the type of machineFiles is using the upstream type like so:

MachineFiles []*v1alpha1.MachineFile `yaml:"machineFiles,omitempty" jsonschema:"description=List of files to create inside the node"`

And my knowledge in go is not good enough to make it support multiple type (what you want is machineFiles can be type of []string or []*v1alpha1.MachineFile).

I see how that's an improvement but unfortunately I don't have enough skill for that, a PR is always welcome though :)

Thanks, I'll look into it.
I found one issue with the current implementation:
If the actual file content starts with an @ (which probably is unlikely but there's a small chance), talhelper will think this is a file and try to read from it.

But that's the point of this feature request? Which is treating a string with @ prefix as a file and try to read the file?

Maybe what you mean is it shouldn't error out if the file is not found and treat it like an actual content instead when it can't read from the file?

Yes, that's the point of the feature request to read from a file. It'll cause a regression for others though that have their file content start with @.

Maybe we need to add a different variable that takes precedence, like "contentFromPath", I'll try to give it a shot in the next couple of days. Here's my current branch https://github.com/mrueg/talhelper/tree/read-from-file