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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions examples/default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ containerd:
# Provisioning scripts need to be idempotent because they might be called
# multiple times, e.g. when the host VM is being restarted.
# The scripts can use the following template variables: {{.Home}}, {{.UID}}, {{.User}}, and {{.Param.Key}}
# They can be provided inline (as field `script`), or in external files (as field `path`).
# 🟢 Builtin default: null
# provision:
# # `system` is executed with root privileges
Expand Down Expand Up @@ -251,6 +252,7 @@ containerd:
# Probe scripts to check readiness.
# The scripts run in user mode. They must start with a '#!' line.
# The scripts can use the following template variables: {{.Home}}, {{.UID}}, {{.User}}, and {{.Param.Key}}
# They can be provided inline (as field `script`), or in external files (as field `path`).
# 🟢 Builtin default: null
# probes:
# # Only `readiness` probes are supported right now.
Expand Down
44 changes: 32 additions & 12 deletions pkg/cidata/cidata.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,10 @@ func GenerateISO9660(instDir, name string, instConfig *limayaml.LimaYAML, udpDNS
args.CACerts.Trusted = append(args.CACerts.Trusted, cert)
}

args.BootCmds = getBootCmds(instConfig.Provision)
args.BootCmds, err = getBootCmds(instConfig.Provision)
if err != nil {
return err
}

for _, f := range instConfig.Provision {
if f.Mode == limayaml.ProvisionModeDependency && *f.SkipDefaultDependencyResolution {
Expand All @@ -329,11 +332,19 @@ func GenerateISO9660(instDir, name string, instConfig *limayaml.LimaYAML, udpDNS
}

for i, f := range instConfig.Provision {
script := f.Script
if f.Path != "" {
b, err := os.ReadFile(f.Path)
if err != nil {
return err
}
script = string(b)
}
switch f.Mode {
case limayaml.ProvisionModeSystem, limayaml.ProvisionModeUser, limayaml.ProvisionModeDependency:
layout = append(layout, iso9660util.Entry{
Path: fmt.Sprintf("provision.%s/%08d", f.Mode, i),
Reader: strings.NewReader(f.Script),
Reader: strings.NewReader(script),
})
case limayaml.ProvisionModeBoot:
continue
Expand Down Expand Up @@ -406,21 +417,30 @@ func getCert(content string) Cert {
return Cert{Lines: lines}
}

func getBootCmds(p []limayaml.Provision) []BootCmds {
func getBootCmds(p []limayaml.Provision) ([]BootCmds, error) {
var bootCmds []BootCmds
for _, f := range p {
if f.Mode == limayaml.ProvisionModeBoot {
lines := []string{}
for _, line := range strings.Split(f.Script, "\n") {
if line == "" {
continue
}
lines = append(lines, strings.TrimSpace(line))
if f.Mode != limayaml.ProvisionModeBoot {
continue
}
script := f.Script
if f.Path != "" {
b, err := os.ReadFile(f.Path)
if err != nil {
return nil, err
}
script = string(b)
}
lines := []string{}
for _, line := range strings.Split(script, "\n") {
if line == "" {
continue
}
bootCmds = append(bootCmds, BootCmds{Lines: lines})
lines = append(lines, strings.TrimSpace(line))
}
bootCmds = append(bootCmds, BootCmds{Lines: lines})
}
return bootCmds
return bootCmds, nil
}

func diskDeviceNameFromOrder(order int) string {
Expand Down
12 changes: 11 additions & 1 deletion pkg/hostagent/requirements.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package hostagent
import (
"errors"
"fmt"
"os"
"time"

"github.com/lima-vm/lima/pkg/limayaml"
Expand Down Expand Up @@ -201,10 +202,19 @@ Also see "/var/log/cloud-init-output.log" in the guest.
})
}
for _, probe := range a.instConfig.Probes {
script := probe.Script
if probe.Path != "" {
b, err := os.ReadFile(probe.Path)
if err != nil {
logrus.WithError(err).Errorf("failed to read script %q", probe.Path)
continue
}
script = string(b)
}
if probe.Mode == limayaml.ProbeModeReadiness {
req = append(req, requirement{
description: probe.Description,
script: probe.Script,
script: script,
debugHint: probe.Hint,
})
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/limayaml/limayaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ type Provision struct {
Mode ProvisionMode `yaml:"mode" json:"mode"` // default: "system"
SkipDefaultDependencyResolution *bool `yaml:"skipDefaultDependencyResolution,omitempty" json:"skipDefaultDependencyResolution,omitempty"`
Script string `yaml:"script" json:"script"`
Path string `yaml:"path,omitempty" json:"path,omitempty"`
Playbook string `yaml:"playbook,omitempty" json:"playbook,omitempty"`
}

Expand All @@ -217,6 +218,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?

Hint string
}

Expand Down
16 changes: 16 additions & 0 deletions pkg/limayaml/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,14 @@ func Validate(y *LimaYAML, warn bool) error {
return fmt.Errorf("field `provision[%d].mode` must one of %q, %q, %q, %q, or %q",
i, ProvisionModeSystem, ProvisionModeUser, ProvisionModeBoot, ProvisionModeDependency, ProvisionModeAnsible)
}
if p.Path != "" {
if p.Script != "" {
return fmt.Errorf("field `provision[%d].script must be empty if path is set", i)
}
if _, err := os.Stat(p.Path); err != nil {
return fmt.Errorf("field `provision[%d].path` refers to an inaccessible path: %q: %w", i, p.Path, err)
}
}
if strings.Contains(p.Script, "LIMA_CIDATA") {
logrus.Warn("provisioning scripts should not reference the LIMA_CIDATA variables")
}
Expand All @@ -232,6 +240,14 @@ func Validate(y *LimaYAML, warn bool) error {
default:
return fmt.Errorf("field `probe[%d].mode` can only be %q", i, ProbeModeReadiness)
}
if p.Path != "" {
if p.Script != "" {
return fmt.Errorf("field `probe[%d].script must be empty if path is set", i)
}
if _, err := os.Stat(p.Path); err != nil {
return fmt.Errorf("field `probe[%d].path` refers to an inaccessible path: %q: %w", i, p.Path, err)
}
}
}
for i, rule := range y.PortForwards {
field := fmt.Sprintf("portForwards[%d]", i)
Expand Down
Loading