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

Allow providing scripts as separate files #2442

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

afbjorklund
Copy link
Member

@afbjorklund afbjorklund commented Jun 24, 2024

The "path" provided will be read, and used instead of the "script".

provision:
- mode: user
  path: foo.sh

probes:
- mode: readiness
  path: bar.sh

There are some unsolved issues with this, regarding relative paths.

Currently it requires that limactl is always run from the top directory.

The paths PWD should probably be recorded in the instance directory?

@afbjorklund
Copy link
Member Author

afbjorklund commented Jun 24, 2024

There are some issues with relative paths for the scripts, as has been discussed before...

Note: all the files/paths used here are on the host, and nothing is copied to the instance

@afbjorklund

This comment was marked as outdated.

@afbjorklund afbjorklund marked this pull request as draft June 24, 2024 10:59
@afbjorklund
Copy link
Member Author

afbjorklund commented Jun 24, 2024

As noted in the older issue, this becomes extra interesting if lima.yaml is loaded from an URL.

limactl start https://raw.githubusercontent.com/lima-vm/lima/master/examples/default.yaml

@afbjorklund afbjorklund marked this pull request as ready for review June 24, 2024 15:38
@@ -206,6 +207,7 @@ type Probe struct {
Mode ProbeMode // default: "readiness"
Description string
Script string
Path string `yaml:",omitempty" json:",omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we consistently use the File object?

Copy link
Member Author

Choose a reason for hiding this comment

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

The file objects also adds arch and digest, neither of which is applicable here...

We could rename Path to Location, to allow for running scripts from URLs?

Copy link
Member

Choose a reason for hiding this comment

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

at least digest seems to be applicable here

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, probably needs more design then

Copy link
Member Author

Choose a reason for hiding this comment

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

i.e. the wish was to "include" common scripts

If the digest changes every time you edit the script, you still need to copy/paste the sha256?

cmd/limactl/start.go Outdated Show resolved Hide resolved
@jandubois
Copy link
Member

jandubois commented Oct 3, 2024

I think relative filenames should always be relative to the location of lima.yaml itself, and not depend on the cwd of the limactl process.

The proper way to handle local script files then is to copy them during limactl create into a subdirectory of the instance directory, e.g. ~/.lima/debian/scripts/foo.sh (with a mechanism to prevent collisions1).

Then lima.yaml is modified with updated relative pathnames:

provision:
- mode: user
  path: scripts/foo.sh

probes:
- mode: readiness
  path: scripts/bar.sh

This makes sure that when the user in the future runs limactl edit and the cidata.iso is re-created, that the instance will continue to use the original scripts used to create the instance, and not a modified version that exists elsewhere on the filesystem. Or fail because the original script files have since been deleted.

For these reasons I think even external scripts referenced by absolute filename (or URL) should be copied into the instance directory and decoupled from the original source.

I don't know how this would work for ansible playbooks, as they can in turn reference additional files, and parsing/rewriting the whole chain seems a bit too complex, but idk what else you can do.

I would still say that we copy the playbook itself, and if that in turn references additional files, then it is up to the user to keep track of them. We can try to display at least a warning when this happens.

I still don't know how external script files would be handled for templates loaded via template:// URL, but whatever the mechanism, they also need to be copied into the scripts/ subdirectory.

Footnotes

  1. E.g. a second foo.sh would become foo-2.sh etc.

@jandubois
Copy link
Member

For these reasons I think even external scripts referenced by absolute filename (or URL) should be copied into the instance directory and decoupled from the original source.

This is also a security concern because a hijacked domain could inject a malicious provisioning script into an existing instance.

@afbjorklund
Copy link
Member Author

afbjorklund commented Oct 3, 2024

I think relative filenames should always be relative to the location of lima.yaml itself, and not depend on the cwd of the limactl process.

We can start with this change, and look at copying them from the template folder to the instance folder later.

EDIT: Then again, the LimaYAML does not know which file it originated from - so that path isn't known (either)

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.

3 participants