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

Add limayaml param settings to provisioning script environment #2570

Merged
merged 1 commit into from
Sep 7, 2024

Conversation

jandubois
Copy link
Member

@jandubois jandubois commented Aug 30, 2024

They will be prefixed with PARAM_, so param.FOO becomes PARAM_FOO.

This is useful because parameter substitution happens when a template is instantiated, so [ "{{.Param.ROOTFUL}}" = true ] becomes [ "true" = true ] in the cloud-init-output.log.

This mechanism also works better when the parameter contains quotes, which would break a simplistic FOO="{{.Param.FOO}}".

Test sample:

param:
  FOO: 'foo"bar'

provision:
- mode: system
  script: |
    echo "$PARAM_FOO"

Output:

LIMA 2024-08-30T09:13:11-07:00| Executing /mnt/lima-cidata/provision.system/00000000
foo"bar
LIMA 2024-08-30T09:13:11-07:00| Exiting with code 0

Example of real-life usage (from #2515) will be to replace

-     # Setting shell variable makes it easier to read cloud-init-output.log
-     readonly ROOTFUL="{{.Param.ROOTFUL}}"
-     if [ "$ROOTFUL" = true ]; then
+     if [ "$PARAM_ROOTFUL" = true ]; then

Copy link
Member Author

@jandubois jandubois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs docs!

Also wondering if probes should have access too (they currently don't have access to LIMA_CIDATA_* variables, I think).

@jandubois jandubois marked this pull request as draft August 30, 2024 17:20
@jandubois jandubois force-pushed the param-env branch 2 times, most recently from b13d8b1 to 22676c1 Compare September 6, 2024 06:32
@jandubois jandubois marked this pull request as ready for review September 6, 2024 06:35
@jandubois
Copy link
Member Author

I'm not proud of the hack to make the $PARAM_* variables available to the probes, but I couldn't figure out a cleaner way as long as we support arbitrary interpreters via the hashbang line.

Anyways, I think the PR is ready for review now.

Copy link
Contributor

@norio-nomura norio-nomura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks like something I considered before implementing #2498 but ultimately abandoned because it seemed too complex for me. I find it impressive and at the same time feel that it was indeed too challenging for me.

I haven't finished reading the probes hack yet, but I'll share what I understand so far.

pkg/limayaml/validate_test.go Show resolved Hide resolved
pkg/cidata/cidata.TEMPLATE.d/boot.sh Show resolved Hide resolved
@jandubois jandubois force-pushed the param-env branch 2 times, most recently from 81f7fda to b8fcab6 Compare September 6, 2024 18:19
@jandubois
Copy link
Member Author

This change looks like something I considered before implementing #2498

This change just build on top of #2498, which is still needed so we can use {{.Param.Key}} in all the places that are not provisioning scripts or probes.

I haven't finished reading the probes hack yet, but I'll share what I understand so far.

I've moved the hack to a separate function and added rather verbose documentation about it. It feels kind of too long, but maybe it is useful.

Copy link
Contributor

@norio-nomura norio-nomura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation and comments for prefixExportParam() made it very clear. Thanks to that, I can suggest a simpler command.
Adding tests to test-misc.yaml is also very clear and good.

examples/default.yaml Show resolved Hide resolved
hack/test-templates.sh Show resolved Hide resolved
pkg/cidata/cidata.TEMPLATE.d/user-data Show resolved Hide resolved
pkg/hostagent/requirements.go Show resolved Hide resolved
pkg/limayaml/validate.go Show resolved Hide resolved
They will be prefixed with `PARAM_`, so `param.FOO` becomes `PARAM_FOO`.

This is useful because parameter substitution happens when a template is
instantiated, so `[ "{{.Param.ROOTFUL}}" = true ]` becomes `[ "true" = true ]`
in the cloud-init-output.log.

This mechanism also works better when the parameter contains quotes, which
would break a simplistic `FOO="{{.Param.FOO}}"`.

Signed-off-by: Jan Dubois <[email protected]>
Copy link
Contributor

@norio-nomura norio-nomura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

pkg/limayaml/validate_test.go Show resolved Hide resolved
pkg/limayaml/validate_test.go Show resolved Hide resolved
@jandubois
Copy link
Member Author

LGTM!

🙏 Thanks! All checks have passed, so I will merge now.

@jandubois jandubois merged commit 54ba0b3 into lima-vm:master Sep 7, 2024
27 checks passed
@jandubois jandubois deleted the param-env branch September 7, 2024 07:46
@AkihiroSuda AkihiroSuda added this to the v1.0 milestone Sep 7, 2024
@nirs
Copy link
Member

nirs commented Sep 8, 2024

This is awesome improvement, I wanted to ask how to inject values from the instance yaml to the cloud-init scripts.

However the names makes this harder to understand. Maybe rename to:

env:
  MY_KEY: VALUE
provision:
  - mode: system
    script: |
      #!/bin/bash
      echo "MY_KEY"

No magic prefix, what you put in env added to the provision scripts.

This is also similar to docker/podman --env flag.

@norio-nomura
Copy link
Contributor

Maybe rename to:

The .env field already exists. However, it had too broad of an impact to use solely for modifying the behavior of scripts within limayaml. That's why I wanted variables that could only be used in Lima, and after some consideration, I implemented .param.

@nirs
Copy link
Member

nirs commented Sep 9, 2024

Yes being able to limit the scope is nice. Did you consider something like this?

env:
  KEY: global
bootEnv:
  BOOT_KEY: boot
provision:
  - mode: system
    script: |
      echo $KEY       # -> gloabl
      echo $BOOT_KEY  # -> boot
probes:
  - mode: system
    script: |
      echo $KEY       # -> gloabl
      echo $BOOT_KEY  # -> boot

Since all provision steps are run by the same global boot.sh script, we can pass the bootEnv to the scripts by exporting the boot env vars from the script.

I did not check how probes are run, but since they are scripts there must be some shell running them and we can modify its environment.

@etho201
Copy link

etho201 commented Nov 21, 2024

Is it possible to define the parameters from an external file (for example, variables.yaml) in order to keep custom/sensitive parameters out of the template files?

@jandubois
Copy link
Member Author

Is it possible to define the parameters from an external file (for example, variables.yaml) in order to keep custom/sensitive parameters out of the template files?

Not right now, but it will be once the implementation of the basedOn mechanism I described in #2520 (reply in thread) is complete. I'm working on it right now; the tricky part is merging YAML files while retaining comments and not just values.

You will be able to write your template like this:

basedOn:
- ~/.variables.yaml
- template://fedora

provision:

Or you could have your private template that contains just the variables, and then reference a public base template:

basedOn: [https://example.com/my-shared-template.yaml]

param:
  secret: password123

The settings from the base templates will be merged with your instance template when the instance is created, similar to $LIMA_HOME/_config/default.yaml is providing defaults when the instance starts.

That means values from the base templates act as defaults, but the settings in the main template take precedence.

It also means that the combined template will include your secrets, but it is stored in the Lima instance directory, which is only readable by your own user (permission 600).

@nirs
Copy link
Member

nirs commented Nov 21, 2024

Or you could have your private template that contains just the variables, and then reference a public base template:

basedOn: [https://example.com/my-shared-template.yaml]

param:
  secret: password123

This is not a good way to store secrets. We can add keychain integration so the owner of the secret can unlock the keychain or lima for accessing the secret when stating a cluster.

I looked at this for this issue:
nirs/oc-clusterset#1

@jandubois
Copy link
Member Author

This is not a good way to store secrets.

I agree, but @etho201 asked about secrets that are already in a text file. It is similar to ~/.aws containing credentials.

We can add keychain integration so the owner of the secret can unlock the keychain or lima for accessing the secret when stating a cluster.

As long as the secret eventually ends up in ~/.lima/INSTANCE/lima.yaml it does not make a big difference; you still end up with the clear text on the disk. Even if not there, it will still be in the cloud-init data somehow, with the instance being able to access it.

So ideally you want something like ssh agent forwarding, but it all depends on how those secrets are going to be used during provisioning. I can't think of a generic solution.

@jandubois
Copy link
Member Author

BTW, if you want to continue this discussion, please move it to the Discussions. Having it at the bottom of the thread of an already closed pull request means it will not be very visible.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Dec 13, 2024
⚠️ **CAUTION: this is a major update, indicating a breaking change!** ⚠️

This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [lima-vm/lima](https://github.com/lima-vm/lima) | major | `v0.23.2` -> `v1.0.2` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>lima-vm/lima (lima-vm/lima)</summary>

### [`v1.0.2`](https://github.com/lima-vm/lima/releases/tag/v1.0.2)

[Compare Source](lima-vm/lima@v1.0.1...v1.0.2)

#### Changes

-   DNS:
    -   Fixed the host resolver regression in v1.0.0 [#&#8203;2939](lima-vm/lima#2939) ([#&#8203;2964](lima-vm/lima#2964))

-   `limactl create`:
    -   Fixed races during parallel downloads ([#&#8203;2903](lima-vm/lima#2903), thanks to [@&#8203;nirs](https://github.com/nirs))
    -   Optimized qcow2-to-raw conversion for vz mode ([#&#8203;2933](lima-vm/lima#2933), thanks to [@&#8203;nirs](https://github.com/nirs))

-   `limactl start-at-login`:
    -   Fixed the support for Linux hosts (systemd) ([#&#8203;2943](lima-vm/lima#2943), thanks to [@&#8203;kachick](https://github.com/kachick))

-   nerdctl:
    -   Updated to [v2.0.1](https://github.com/containerd/nerdctl/releases/tag/v2.0.1) ([#&#8203;2966](lima-vm/lima#2966))

-   Templates:
    -   Updated to the latest revisions ([#&#8203;2936](lima-vm/lima#2936) [#&#8203;2953](lima-vm/lima#2953), thanks to [@&#8203;tcooper](https://github.com/tcooper))

-   Web site:
    -   Added an example of running Lima on GitHub Actions to run commands on non-Ubuntu ([#&#8203;2954](lima-vm/lima#2954)): https://lima-vm.io/docs/examples/gha/

-   Project:
    -   Invite Nir Soffer ([@&#8203;nirs](https://github.com/nirs)) as a Reviewer ([#&#8203;2916](lima-vm/lima#2916), thanks to [@&#8203;jandubois](https://github.com/jandubois))

Full changes: https://github.com/lima-vm/lima/milestone/51?closed=1
Thanks to [@&#8203;SpiffyEight77](https://github.com/SpiffyEight77) [@&#8203;alexandear](https://github.com/alexandear) [@&#8203;jandubois](https://github.com/jandubois) [@&#8203;kachick](https://github.com/kachick) [@&#8203;nirs](https://github.com/nirs) [@&#8203;norio-nomura](https://github.com/norio-nomura) [@&#8203;tamird](https://github.com/tamird) [@&#8203;tcooper](https://github.com/tcooper)

#### Usage

```console
[macOS]$ limactl create
[macOS]$ limactl start
...
INFO[0029] READY. Run `lima` to open the shell.

[macOS]$ lima uname
Linux
```

***

The binaries were built automatically on GitHub Actions.
The build log is available for 90 days: https://github.com/lima-vm/lima/actions/runs/12134682585

The sha256sum of the SHA256SUMS file itself is `02ef78494c498ca4180915ba78d5e2fc471ed401f63dfb2b5864c3711f3c0fb2` .

***

Release manager: [@&#8203;AkihiroSuda](https://github.com/AkihiroSuda)

### [`v1.0.1`](https://github.com/lima-vm/lima/releases/tag/v1.0.1)

[Compare Source](lima-vm/lima@v1.0.0...v1.0.1)

Reverted the default port forwarder from gRPC to SSH for the stability reason ([#&#8203;2864](lima-vm/lima#2864)).
This reversion fixes several regressions related to `docker run -p` in Lima v1.0.0 ([#&#8203;2859](lima-vm/lima#2859)).

Although the gRPC forwarder is faster and has an advanced feature (UDP support), it turned out to be still immature.
Set `LIMA_SSH_PORT_FORWARDER=false` to opt-in to the gRPC forwarder.
See <https://lima-vm.io/docs/config/port/>.

Full changes: https://github.com/lima-vm/lima/milestone/50?closed=1
Thanks to [@&#8203;alexandear](https://github.com/alexandear) [@&#8203;jandubois](https://github.com/jandubois) [@&#8203;norio-nomura](https://github.com/norio-nomura)

#### Usage

```console
[macOS]$ limactl create
[macOS]$ limactl start
...
INFO[0029] READY. Run `lima` to open the shell.

[macOS]$ lima uname
Linux
```

***

The binaries were built automatically on GitHub Actions.
The build log is available for 90 days: https://github.com/lima-vm/lima/actions/runs/11735352652

The sha256sum of the SHA256SUMS file itself is `f5c12d003e25dc46291803a8acae9e9d325a45eca0c1f9f40bd6852ec8ed9be1` .

***

Release manager: [@&#8203;AkihiroSuda](https://github.com/AkihiroSuda)

### [`v1.0.0`](https://github.com/lima-vm/lima/releases/tag/v1.0.0)

[Compare Source](lima-vm/lima@v0.23.2...v1.0.0)

With the support from 110+ contributors in 3+ years, the Lima project has finally reached v1.0. 🎉

This release introduces several breaking changes, such as switching the default machine driver from QEMU to VZ for better filesystem performance.

The `limactl` CLI is designed to print hints when the user hits those breaking changes.
e.g., `limactl create template://experimental/vz` now fails with a hint that suggests using `limactl create --vm-type=vz template://default` instead.

🔴 = Major breaking changes
🟡 = Minor breaking changes

-   VZ:
    -   Graduate VZ machine driver from experimental ([#&#8203;2758](lima-vm/lima#2758))
    -   🔴 Use VZ by default for new instances on macOS >= 13.5 ([#&#8203;1951](lima-vm/lima#1951))
    -   Support nested virtualization on M3 ([#&#8203;2530](lima-vm/lima#2530), thanks to [@&#8203;abiosoft](https://github.com/abiosoft))
    -   Optimize qcow2-to-raw image conversion (lima-vm/go-qcow2reader@v0.1.2...v0.4.0 , thanks to [@&#8203;nirs](https://github.com/nirs))
    -   Support specifying a custom kernel ([#&#8203;2562](lima-vm/lima#2562), thanks to [@&#8203;norio-nomura](https://github.com/norio-nomura))

-   QEMU:
    -   Graduate 9p mount driver from experimental ([#&#8203;2758](lima-vm/lima#2758))
    -   🔴 Use 9p by default for most templates ([#&#8203;1953](lima-vm/lima#1953), [#&#8203;2822](lima-vm/lima#2822))
    -   riscv64: switch from u-boot to EDK2 ([#&#8203;2592](lima-vm/lima#2592))

-   Network:
    -   Graduate user-v2 network driver from experimental ([#&#8203;2758](lima-vm/lima#2758))
    -   Support UDP port forwarding ([#&#8203;2411](lima-vm/lima#2411), thanks to [@&#8203;balajiv113](https://github.com/balajiv113))
    -   🔴 Strictly require `socket_vmnet` binary to be owned by root ([#&#8203;2734](lima-vm/lima#2734))

-   SSH:
    -   🟡 Disable `ssh.loadDotSSHPubKeys` by default ([#&#8203;2706](lima-vm/lima#2706))

-   YAML:
    -   Support generating jsonschema ([#&#8203;2306](lima-vm/lima#2306), thanks to [@&#8203;afbjorklund](https://github.com/afbjorklund))
    -   Support specifying `param` for provisioning scripts ([#&#8203;2570](lima-vm/lima#2570), thanks to [@&#8203;jandubois](https://github.com/jandubois))
    -   Support specifying `minimumLimaVersion` and `vmOpts.qemu.minimumVersion` ([#&#8203;2659](lima-vm/lima#2659), thanks to [@&#8203;jandubois](https://github.com/jandubois))
    -   Support template expansion in mounts ([#&#8203;2588](lima-vm/lima#2588), thanks to [@&#8203;norio-nomura](https://github.com/norio-nomura))

-   `limactl` CLI:
    -   Add `limactl tunnel` command so as to allow the host to join the guest network ([#&#8203;2710](lima-vm/lima#2710))
    -   Add `--log-format=json` ([#&#8203;2584](lima-vm/lima#2584), thanks to [@&#8203;nirs](https://github.com/nirs))
    -   `limactl prune`: Add `--keep-referred` ([#&#8203;2569](lima-vm/lima#2569), thanks to [@&#8203;norio-nomura](https://github.com/norio-nomura))

-   nerdctl:
    -   Updated to [v2.0.0](https://github.com/containerd/nerdctl/releases/tag/v2.0.0) ([#&#8203;2178](lima-vm/lima#2178))
    -   rootless: allocate 1G subuids from 524288 (0x80000) for new users ([#&#8203;2725](lima-vm/lima#2725))

-   Templates:
    -   🔴 `experimental/vz`: Merged into the `default` template ([#&#8203;2730](lima-vm/lima#2730), [#&#8203;2736](lima-vm/lima#2736))
    -   🟡 `experimental/{riscv64, armv7l}`: Merged into the `default` template ([#&#8203;2730](lima-vm/lima#2730), [#&#8203;2736](lima-vm/lima#2736))
    -   🔴 `vmnet`: Removed in favor of `limactl create --network=lima:shared template://default` ([#&#8203;2736](lima-vm/lima#2736))
    -   🟡 `experimental/net-user-v2`: Removed in favor of `limactl create --network=lima:user-v2 template://default` ([#&#8203;2736](lima-vm/lima#2736))
    -   🔴 `experimental/9p`: Removed in favor of `limactl create --mount-type=9p template://default` ([#&#8203;2736](lima-vm/lima#2736))
    -   🟡 `experimental/virtiofs-linux`: Removed in favor of `limactl create --mount-type=virtiofs template://default` ([#&#8203;2736](lima-vm/lima#2736))
    -   🔴 `alpine`: Renamed to `alpine-iso` ([#&#8203;2704](lima-vm/lima#2704))
    -   🔴 `alpine-image`: Renamed to `alpine` ([#&#8203;2704](lima-vm/lima#2704))
    -   `archlinux`: Demoted from Tier 1 to Tier 2 ([#&#8203;2717](lima-vm/lima#2717), [#&#8203;2823](lima-vm/lima#2823))
    -   `default`, `ubuntu`, ...: Updated to Ubuntu 24.10. The older versions are available as `ubuntu-20.04`, `ubuntu-22.04`, and `ubuntu-24.04` ([#&#8203;2755](lima-vm/lima#2755), [#&#8203;2795](lima-vm/lima#2795))
    -   `fedora`: Updated to Fedora 41 ([#&#8203;2821](lima-vm/lima#2821), [#&#8203;2822](lima-vm/lima#2822), thanks to [@&#8203;subpop](https://github.com/subpop))
    -   `opensuse`: Renamed to `opensuse-leap`. Still aliased as `opensuse` ([#&#8203;2612](lima-vm/lima#2612), thanks to [@&#8203;afbjorklund](https://github.com/afbjorklund))
    -   `experimental/opensuse-tumbleweed`: Support aarch64 ([#&#8203;2613](lima-vm/lima#2613), thanks to [@&#8203;afbjorklund](https://github.com/afbjorklund))
    -   `hack/update-template.sh` is added for automating updates ([#&#8203;1347](lima-vm/lima#1347), thanks to [@&#8203;norio-nomura](https://github.com/norio-nomura))

-   Project:
    -   Invite Norio Nomura ([@&#8203;norio-nomura](https://github.com/norio-nomura)) as a Reviewer ([#&#8203;2567](lima-vm/lima#2567))

Full changes: https://github.com/lima-vm/lima/milestone/47?closed=1
Thanks to [@&#8203;AdamKorcz](https://github.com/AdamKorcz) [@&#8203;Mr-Sunglasses](https://github.com/Mr-Sunglasses) [@&#8203;SmartManoj](https://github.com/SmartManoj) [@&#8203;YorikSar](https://github.com/YorikSar) [@&#8203;abiosoft](https://github.com/abiosoft) [@&#8203;afbjorklund](https://github.com/afbjorklund) [@&#8203;alexandear](https://github.com/alexandear) [@&#8203;balajiv113](https://github.com/balajiv113) [@&#8203;hasan4791](https://github.com/hasan4791) [@&#8203;jandubois](https://github.com/jandubois) [@&#8203;nirs](https://github.com/nirs) [@&#8203;norio-nomura](https://github.com/norio-nomura) [@&#8203;pvdvreede](https://github.com/pvdvreede) [@&#8203;subpop](https://github.com/subpop) [@&#8203;tsukasaI](https://github.com/tsukasaI)

#### Usage

```console
[macOS]$ limactl create
[macOS]$ limactl start
...
INFO[0029] READY. Run `lima` to open the shell.

[macOS]$ lima uname
Linux
```

***

The binaries were built automatically on GitHub Actions.
The build log is available for 90 days: https://github.com/lima-vm/lima/actions/runs/11695321667

The sha256sum of the SHA256SUMS file itself is `4bd200a163111fe78c6f3e6de405113d416053802fe1507597f9a42f89a98c90` .

***

Release manager: [@&#8203;AkihiroSuda](https://github.com/AkihiroSuda)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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

Successfully merging this pull request may close these issues.

5 participants