Move template locator and reader code into new limatmpl package #3009

jandubois · 2024-12-11T06:17:57Z

It was part of cmd/limactl/start.go but should be generally reusable code. Moved related cmd/limactl/guessarg into limatmpl as well.

Also created a new limactl template command.

It takes a single template locator argument, loads the template, and writes it to STDOUT.

This is currently not very interesting; it is in preparation of implementing composable (multi-file) templates.

AkihiroSuda · 2024-12-11T14:57:52Z

pkg/limatmpl/locator.go

@@ -0,0 +1,166 @@
+package limatmpl


Maybe:

Suggested change

package limatmpl

package templatelocator

It may not make sense right now, but there will be a limatmpl/assemble.go in the future that contains all methods on a limatmpl.Template receiver, and then the package name templatelocator doesn't make sense anymore.

And while I haven't written it yet, the new FillDefaults() will also have to move to the limatmpl package (to avoid a circular dependency with usrlocalsharelima, which indirectly needs limayaml).

So for now at least I would like to keep the limatmpl package name.

cmd/limactl/template.go

nirs · 2024-12-11T20:08:10Z

It takes a single template locator argument, loads the template, and writes it to STDOUT.

This is currently not very interesting; it is in preparation of implementing composable (multi-file) templates.

This is interesting, a possible use case is creating new yaml from template, and then creating one or move vms from the yaml. Can be done today by copying a template from source, or from /opt/homebrew/share/doc/lima/templates/default.yaml which is much harder and error prone compared with template://default.

Not sure that template is a good name since we repeat "template":

limactl template template://default

Maybe it should work with edit?

limactl edit template://default

If this may be confusing since edit is used for instances, maybe:

limactl generate template://default

jandubois · 2024-12-11T20:27:01Z

Not sure that template is a good name since we repeat "template":

As @AkihiroSuda already pointed out, we will need additional subcommands under template, so it will no longer stutter:

limactl template print template://docker
limactl template assemble my.yaml
limactl template with-defaults my.yaml
limactl template validate your.yaml

I'm not sure about the with-defaults subcommand name, but it would be similar to limactl validate --fill, but in addition to the builtin defaults it would also combine it with default.yaml and override.yaml.

Maybe we need to have 2 different subcommands for that. I find it challenging to come up with meaningful names.

I will make the template command hidden, so we can continue to evolve it without worrying about backwards compatibility.

I still think templates is the best name for that group of commands.

I'm more concerned about the confusion with "text templates" inside lima.yaml, like we have "host templates", and "guest templates", and potentially "param templates" that all have different sets of variables for interpolation. But this is a concern for writing the docs; at the CLI level a template should always be a lima.yaml template.

Just for clarification, I consider my.yaml to be a template name too, it just isn't using the template:// URI scheme. That's why I started using the term "template locator" instead, similar to how URLs can use various schemes to specify the location.

jandubois · 2024-12-11T20:38:15Z

Just for illustration (and I guess as a teaser), here is an example of limactl template assemble my.yaml¹:

$ cat my.yaml
basedOn: template://fedora
mountTypesUnsupported: [virtiofs]
provision:
# Hello world!
- file: my.sh # Hi there

$ cat my.sh
#!/bin/sh
echo "Hello world!"

$ limactl template assemble my.yaml
# 9p is broken in Linux v6.9, v6.10, and v6.11 (used by Fedora 41).
# The issue was fixed in Linux v6.12-rc5 (https://github.com/torvalds/linux/commit/be2ca38).
mountTypesUnsupported: [virtiofs, "9p"]
provision:
# Hello world!
- script: |  # Hi there
    #!/bin/sh
    echo "Hello world!"
# This template requires Lima v0.7.0 or later.
images:
- location: "https://download.fedoraproject.org/pub/fedora/linux/releases/41/Cloud/x86_64/images/Fedora-Cloud-Base-Generic-41-1.4.x86_64.qcow2"
  arch: "x86_64"
  digest: "sha256:6205ae0c524b4d1816dbd3573ce29b5c44ed26c9fbc874fbe48c41c89dd0bac2"
- location: "https://download.fedoraproject.org/pub/fedora/linux/releases/41/Cloud/aarch64/images/Fedora-Cloud-Base-Generic-41-1.4.aarch64.qcow2"
  arch: "aarch64"
  digest: "sha256:085883b42c7e3b980e366a1fe006cd0ff15877f7e6e984426f3c6c67c7cc2faa"
- location: "https://download.fedoraproject.org/pub/fedora/linux/releases/41/Cloud/x86_64/images/Fedora-Cloud-Base-Generic-41-1.4.x86_64.qcow2"
  arch: "x86_64"
  digest: "sha256:6205ae0c524b4d1816dbd3573ce29b5c44ed26c9fbc874fbe48c41c89dd0bac2"
- location: "https://download.fedoraproject.org/pub/fedora/linux/releases/41/Cloud/aarch64/images/Fedora-Cloud-Base-Generic-41-1.4.aarch64.qcow2"
  arch: "aarch64"
  digest: "sha256:085883b42c7e3b980e366a1fe006cd0ff15877f7e6e984426f3c6c67c7cc2faa"
mounts:
- location: "~"
- location: "/tmp/lima"
  writable: true

You can see how both template://fedora and my.sh got merged into my.yaml, and how even the comments on the provision[0].file node moved to the provision[0].script entry.

And you can see how mountTypesUnsupported got merged, kept the comments from template://fedora because my.yaml didn't have a comment on that node, and the string presentation were maintained (virtiofs doesn't have quotes, "9p" does).

I cheated, currently the command is limactl template my.yaml, but I will change it to use the assemble subcommand. ↩

nirs · 2024-12-11T21:18:04Z

Tanks for the example, it is very helpful. limactl template assemble could be nicer as limactl template build.

jandubois · 2024-12-11T21:40:21Z

Tanks for the example, it is very helpful. limactl template assemble could be nicer as limactl template build.

We can bike-shed the names later. I went through several already, and used compose before I switched to assemble.

To me build is creating something from basic ingredients, whereas assemble means just combining existing more complex pieces into a whole. For that I like assemble better. I do see that build is shorter and easier to type... For now it is tmpl.Assemble() to perform the magic.

jandubois · 2024-12-12T19:03:15Z

I've realized that limactl template print should probably be limactl template copy because in the future it will create a working copy of the template, not print it verbatim.

The difference comes in with the tmpl.Assemble implementation that supports "relative template locators" (it is already implemented).

Assume you have templates/my.yaml stored in the lima.git repo, and also installed in /usr/local/share/lima/templates/my.yaml:

basedOn: fedora.yaml
provision:
- file: my.sh

You can reference it as ./template/my.yaml, template://my, or https://raw.githubusercontent.com/lima-vm/lima/refs/heads/master/templates/my.yaml. This will create different copies, to keep the copy working the same way as the original:

limactl template copy ./templates/my.yaml

basedOn: /Users/USERNAME/git/lima/templates/fedora.yaml
provision:
- file: /Users/USERNAME/git/lima/templates/my.sh

limactl template copy template://my¹

basedOn: template://fedora.yaml
provision:
- file: template://my.sh

limactl template copy https://raw.githubusercontent.com/lima-vm/lima/refs/heads/master/templates/my.yaml

basedOn: https://raw.githubusercontent.com/lima-vm/lima/refs/heads/master/templates/fedora.yaml
provision:
- file: https://raw.githubusercontent.com/lima-vm/lima/refs/heads/master/templates/my.sh

This means the copy will continue to reference the same base fedora.yaml template and the same my.sh provisioning file as the source template, even though the copy will be stored in a different location.

I'll rename the print subcommand to copy in the PR implementing relative locators, unless there are still other changes required for the current PR.

I'm glossing over the fact that the template:// scheme now supports file extensions, and can be used for script references too. ↩

AkihiroSuda · 2024-12-13T07:27:17Z

limactl template copy

Sounds confusing.
What about limactl template render ?

AkihiroSuda · 2024-12-13T07:27:46Z

Side topic:

basedOn: https://raw.githubusercontent.com/lima-vm/lima/refs/heads/master/templates/fedora.yaml

This has to support specifying sha256 digest

jandubois · 2024-12-13T07:38:10Z

This has to support specifying sha256 digest

Not sure what that means. Digest for what? For the template content? That seems impractical. If you need to fetch a digest, you can just fetch the template instead and load it locally because the templates are small.

How do you specify a digest for

limactl start https://raw.githubusercontent.com/lima-vm/lima/refs/heads/master/templates/fedora.yaml

If you want to have an immutable reference, maybe store your templates in an OCI registry and reference the data by hash?

jandubois · 2024-12-13T07:44:16Z

What about limactl template render ?

render sounds more like what I currently call assemble, that is embed all external references into a single lima.yaml so it can be stored in the instance directory.

The copy command just produces a copy of the template as-is, except all relative references are replaced by absolute references. Otherwise you end up with a broken copy.

I'm not sure how important the copy functionality is; I suspect you normally want assemble.

The part I dislike about render is that I expect it to also evaluate all the text templates inside the template file, like home: "/home/{{.User}}".

It may all make more sense once the actual implementation is merged (which isn't even part of this PR). I've made the limactl template command "hidden" so we can continue to bike-shed it.

AkihiroSuda · 2024-12-13T07:46:56Z

Digest

The digest of the source template.
e.g.,

base:
  location: "https://example.com/a.yaml"
  digest: "sha256:deadbeef"

As a side topic, limactl start https://... should also have a flag like --template-digest=sha256:deadbeef

AkihiroSuda · 2024-12-13T07:47:47Z

copy

I expect any command named "copy" to take two (or more) arguments: SRC and DST

jandubois · 2024-12-13T17:14:23Z

I expect any command named "copy" to take two (or more) arguments: SRC and DST

That totally makes sense. I was already thinking about adding an --output option so you don't have to use shell redirection to write to a file, but requiring a DST (and supporting - for STDOUT) makes much more sense.

Maybe most of the other planned subcommands, like assemble should just be options on copy?

copy --assemble  # embed all base templates and script files
copy --fill      # merge override.yaml, default.yaml, and builtin defaults
copy --render    # evaluate all Go text templates

copy             # make relative locators absolute
copy --verbatim  # copy as-is

Practically speaking --fill should imply --assemble, and --render should imply --fill.

limactl template copy --assemble is what limactl create would write to lima.yaml.
limactl template copy --fill is what we pass to validation.
limactl template copy --render is what is used to create cidata and start the instance.

Maybe we can allow the user to override, like copy --render --assenble=false. Not sure there is a use-case for this except for Lima developers.

jandubois · 2024-12-13T17:16:31Z

copy --assemble # embed all base templates and script files

Maybe --assemble (and tmpl.Assemble() etc.) should be --embed (and tmpl.Embed() etc.).

jandubois · 2024-12-13T17:22:24Z

copy --fill # merge override.yaml, default.yaml, and builtin defaults
copy --render # evaluate all Go text templates

Actually, I'm not sure about this; currently filling defaults and evaluating text templates is intermingled, and may need to continue to be so. So we may just have --fill and it would include the rendered text templates.

jandubois · 2024-12-13T17:41:42Z

As a side topic, limactl start https://... should also have a flag like --template-digest=sha256:deadbeef

A side topic to the side note... 😄

I've moved this to a discussion topic at #3019.

jandubois · 2024-12-14T05:06:56Z

I've renamed print to copy and require a target filename. I've also added a tmpl alias.

So limactl template print template://my is now limactl tmpl copy template://my -.

@AkihiroSuda I don't plan to make any other changes; do you have more feedback?

AkihiroSuda · 2024-12-14T15:44:45Z

cmd/limactl/validate.go

This should now invoke limactl template validate

Ok, limactl validate now invokes templateValidateAction.

AkihiroSuda · 2024-12-14T15:45:10Z

cmd/limactl/template.go

+		SilenceUsage:  true,
+		SilenceErrors: true,
+		GroupID:       advancedCommand,
+		Hidden:        true,


The reason should be explained in a comment or --help

cmd/limactl/template.go

It was part of cmd/limactl/start.go but should be generally reusable code. Moved related cmd/limactl/guessarg into limatmpl as well. Also created a new `limactl template` command. It takes a single template locator argument, loads the template, and writes it to STDOUT. This is currently not very interesting; it is in preparation of implementing composable (multi-file) templates. Signed-off-by: Jan Dubois <[email protected]>

AkihiroSuda

Thanks

AkihiroSuda added this to the v1.0.3 milestone Dec 11, 2024

AkihiroSuda added the kind/refactoring Refactoring label Dec 11, 2024

AkihiroSuda reviewed Dec 11, 2024

View reviewed changes

cmd/limactl/template.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed Dec 11, 2024

View reviewed changes

cmd/limactl/template.go Outdated Show resolved Hide resolved

jandubois force-pushed the limatmpl branch from 7900320 to 71f2773 Compare December 12, 2024 06:00

jandubois requested a review from AkihiroSuda December 12, 2024 06:06

jandubois force-pushed the limatmpl branch from 71f2773 to a0af8fb Compare December 12, 2024 06:11

jandubois force-pushed the limatmpl branch from a0af8fb to 138f55c Compare December 14, 2024 05:03

jandubois mentioned this pull request Dec 14, 2024

Add optional digest suffix to template locator #3021

Closed

AkihiroSuda reviewed Dec 14, 2024

View reviewed changes

cmd/limactl/template.go Outdated Show resolved Hide resolved

jandubois force-pushed the limatmpl branch 3 times, most recently from 3e18003 to ae1664a Compare December 22, 2024 01:45

AkihiroSuda reviewed Dec 22, 2024

View reviewed changes

cmd/limactl/template.go Outdated Show resolved Hide resolved

jandubois force-pushed the limatmpl branch from ae1664a to 0715393 Compare December 22, 2024 19:05

AkihiroSuda approved these changes Dec 22, 2024

View reviewed changes

AkihiroSuda merged commit 3b8e7b3 into lima-vm:master Dec 22, 2024
29 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move template locator and reader code into new limatmpl package #3009

Move template locator and reader code into new limatmpl package #3009

jandubois commented Dec 11, 2024

AkihiroSuda Dec 11, 2024

jandubois Dec 12, 2024

nirs commented Dec 11, 2024

jandubois commented Dec 11, 2024

jandubois commented Dec 11, 2024

nirs commented Dec 11, 2024

jandubois commented Dec 11, 2024

jandubois commented Dec 12, 2024

AkihiroSuda commented Dec 13, 2024 •

edited

Loading

AkihiroSuda commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 13, 2024

AkihiroSuda commented Dec 13, 2024

AkihiroSuda commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 14, 2024

AkihiroSuda Dec 14, 2024

jandubois Dec 22, 2024

AkihiroSuda Dec 14, 2024

AkihiroSuda left a comment

Move template locator and reader code into new limatmpl package #3009

Move template locator and reader code into new limatmpl package #3009

Conversation

jandubois commented Dec 11, 2024

AkihiroSuda Dec 11, 2024

Choose a reason for hiding this comment

jandubois Dec 12, 2024

Choose a reason for hiding this comment

nirs commented Dec 11, 2024

jandubois commented Dec 11, 2024

jandubois commented Dec 11, 2024

Footnotes

nirs commented Dec 11, 2024

jandubois commented Dec 11, 2024

jandubois commented Dec 12, 2024

Footnotes

AkihiroSuda commented Dec 13, 2024 • edited Loading

AkihiroSuda commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 13, 2024

AkihiroSuda commented Dec 13, 2024

AkihiroSuda commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 13, 2024

jandubois commented Dec 14, 2024

AkihiroSuda Dec 14, 2024

Choose a reason for hiding this comment

jandubois Dec 22, 2024

Choose a reason for hiding this comment

AkihiroSuda Dec 14, 2024

Choose a reason for hiding this comment

AkihiroSuda left a comment

Choose a reason for hiding this comment

AkihiroSuda commented Dec 13, 2024 •

edited

Loading