From e7cb6aa910981e6e43418366a5137fd7dc9a6989 Mon Sep 17 00:00:00 2001 From: I Hsin Cheng Date: Thu, 16 Oct 2025 17:27:55 +0800 Subject: [PATCH] Make Provision.Script a pointer As the TODO in limayaml/defaults.go claims, change the type of "Script" under "type Provision struct" from "string" to "*string". Update all callers to pass a pointer and handle "nil" in comparisons, validation, and marshalling. Signed-off-by: I Hsin Cheng --- pkg/cidata/cidata.go | 4 ++-- pkg/limatmpl/embed.go | 2 +- pkg/limatype/lima_yaml.go | 2 +- pkg/limayaml/defaults.go | 9 ++++----- pkg/limayaml/defaults_test.go | 8 ++++---- pkg/limayaml/validate.go | 12 +++++++----- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/pkg/cidata/cidata.go b/pkg/cidata/cidata.go index 888e17c5198..6eac7496f24 100644 --- a/pkg/cidata/cidata.go +++ b/pkg/cidata/cidata.go @@ -400,7 +400,7 @@ func GenerateISO9660(ctx context.Context, drv driver.Driver, instDir, name strin case limatype.ProvisionModeSystem, limatype.ProvisionModeUser, limatype.ProvisionModeDependency: layout = append(layout, iso9660util.Entry{ Path: fmt.Sprintf("provision.%s/%08d", f.Mode, i), - Reader: strings.NewReader(f.Script), + Reader: strings.NewReader(*f.Script), }) case limatype.ProvisionModeData: layout = append(layout, iso9660util.Entry{ @@ -490,7 +490,7 @@ func getBootCmds(p []limatype.Provision) []BootCmds { for _, f := range p { if f.Mode == limatype.ProvisionModeBoot { lines := []string{} - for _, line := range strings.Split(f.Script, "\n") { + for _, line := range strings.Split(*f.Script, "\n") { if line == "" { continue } diff --git a/pkg/limatmpl/embed.go b/pkg/limatmpl/embed.go index c42f81deda0..823e64afca3 100644 --- a/pkg/limatmpl/embed.go +++ b/pkg/limatmpl/embed.go @@ -663,7 +663,7 @@ func (tmpl *Template) embedAllScripts(ctx context.Context, embedAll bool) error continue } default: - if p.Script != "" { + if p.Script != nil && *p.Script != "" { continue } } diff --git a/pkg/limatype/lima_yaml.go b/pkg/limatype/lima_yaml.go index 57470c10350..0933e45e68e 100644 --- a/pkg/limatype/lima_yaml.go +++ b/pkg/limatype/lima_yaml.go @@ -238,7 +238,7 @@ const ( type Provision struct { Mode ProvisionMode `yaml:"mode,omitempty" json:"mode,omitempty" jsonschema:"default=system"` SkipDefaultDependencyResolution *bool `yaml:"skipDefaultDependencyResolution,omitempty" json:"skipDefaultDependencyResolution,omitempty"` - Script string `yaml:"script,omitempty" json:"script,omitempty"` + Script *string `yaml:"script,omitempty" json:"script,omitempty"` File *LocatorWithDigest `yaml:"file,omitempty" json:"file,omitempty" jsonschema:"nullable"` Playbook string `yaml:"playbook,omitempty" json:"playbook,omitempty"` // DEPRECATED // All ProvisionData fields must be nil unless Mode is ProvisionModeData diff --git a/pkg/limayaml/defaults.go b/pkg/limayaml/defaults.go index 49be6d0fa8f..1f617dbd490 100644 --- a/pkg/limayaml/defaults.go +++ b/pkg/limayaml/defaults.go @@ -439,12 +439,11 @@ func FillDefault(ctx context.Context, y, d, o *limatype.LimaYAML, filePath strin provision.Permissions = ptr.Of("644") } } - // TODO Turn Script into a pointer; it is a plain string for historical reasons only - if provision.Script != "" { - if out, err := executeGuestTemplate(provision.Script, instDir, y.User, y.Param); err == nil { - provision.Script = out.String() + if provision.Script != nil && *provision.Script != "" { + if out, err := executeGuestTemplate(*provision.Script, instDir, y.User, y.Param); err == nil { + *provision.Script = out.String() } else { - logrus.WithError(err).Warnf("Couldn't process provisioning script %q as a template", provision.Script) + logrus.WithError(err).Warnf("Couldn't process provisioning script %q as a template", *provision.Script) } } } diff --git a/pkg/limayaml/defaults_test.go b/pkg/limayaml/defaults_test.go index 768650a5466..55227992d71 100644 --- a/pkg/limayaml/defaults_test.go +++ b/pkg/limayaml/defaults_test.go @@ -150,7 +150,7 @@ func TestFillDefault(t *testing.T) { }, MountType: ptr.Of(limatype.NINEP), Provision: []limatype.Provision{ - {Script: "#!/bin/true # {{.Param.ONE}}"}, + {Script: ptr.Of("#!/bin/true # {{.Param.ONE}}")}, }, Probes: []limatype.Probe{ {Script: "#!/bin/false # {{.Param.ONE}}"}, @@ -234,7 +234,7 @@ func TestFillDefault(t *testing.T) { expect.Provision = slices.Clone(y.Provision) expect.Provision[0].Mode = limatype.ProvisionModeSystem - expect.Provision[0].Script = "#!/bin/true # Eins" + expect.Provision[0].Script = ptr.Of("#!/bin/true # Eins") expect.Probes = slices.Clone(y.Probes) expect.Probes[0].Mode = limatype.ProbeModeReadiness @@ -364,7 +364,7 @@ func TestFillDefault(t *testing.T) { }, Provision: []limatype.Provision{ { - Script: "#!/bin/true", + Script: ptr.Of("#!/bin/true"), Mode: limatype.ProvisionModeUser, }, }, @@ -572,7 +572,7 @@ func TestFillDefault(t *testing.T) { MountInotify: ptr.Of(true), Provision: []limatype.Provision{ { - Script: "#!/bin/true", + Script: ptr.Of("#!/bin/true"), Mode: limatype.ProvisionModeSystem, }, }, diff --git a/pkg/limayaml/validate.go b/pkg/limayaml/validate.go index 87ffb7e65be..e13844e7fff 100644 --- a/pkg/limayaml/validate.go +++ b/pkg/limayaml/validate.go @@ -212,7 +212,7 @@ func Validate(y *limatype.LimaYAML, warn bool) error { errs = errors.Join(errs, fmt.Errorf("field `provision[%d].permissions` must be an octal number: %w", i, err)) } default: - if p.Script == "" && p.Mode != limatype.ProvisionModeAnsible { + if (p.Script == nil || *p.Script == "") && p.Mode != limatype.ProvisionModeAnsible { errs = errors.Join(errs, fmt.Errorf("field `provision[%d].script` must not be empty", i)) } if p.Content != nil { @@ -238,7 +238,7 @@ func Validate(y *limatype.LimaYAML, warn bool) error { if p.Mode != limatype.ProvisionModeAnsible { errs = errors.Join(errs, fmt.Errorf("field `provision[%d].playbook can only be set when mode is %q", i, limatype.ProvisionModeAnsible)) } - if p.Script != "" { + if p.Script != nil && *p.Script != "" { errs = errors.Join(errs, fmt.Errorf("field `provision[%d].script must be empty if playbook is set", i)) } playbook := p.Playbook @@ -247,8 +247,10 @@ func Validate(y *limatype.LimaYAML, warn bool) error { } logrus.Warnf("provision mode %q is deprecated, use `ansible-playbook %q` instead", limatype.ProvisionModeAnsible, playbook) } - if strings.Contains(p.Script, "LIMA_CIDATA") { - logrus.Warn("provisioning scripts should not reference the LIMA_CIDATA variables") + if p.Script != nil { + if strings.Contains(*p.Script, "LIMA_CIDATA") { + logrus.Warn("provisioning scripts should not reference the LIMA_CIDATA variables") + } } } needsContainerdArchives := (y.Containerd.User != nil && *y.Containerd.User) || (y.Containerd.System != nil && *y.Containerd.System) @@ -521,7 +523,7 @@ func validateParamIsUsed(y *limatype.LimaYAML) error { } keyIsUsed := false for _, p := range y.Provision { - for _, ptr := range []*string{&p.Script, p.Content, p.Expression, p.Owner, p.Path, p.Permissions} { + for _, ptr := range []*string{p.Script, p.Content, p.Expression, p.Owner, p.Path, p.Permissions} { if ptr != nil && re.MatchString(*ptr) { keyIsUsed = true break