Repository: lima-vm/lima
Pull request #3596 (2025-05-31 08:28:48)

Reject shrinking disk during YAML validation
> I wish we would detect the attempt to downsize the disk size, and then reject the change as invalid, but that can be a separate PR.

As @jandubois  suggested in #3533. 

Currently, rejecting shrinking disk size happens during instance startup.
This PR adds validation earlier during YAML validation.
The function compares values between the new config and the latest config YAML files to reject disk size shrinking during validation.

songponssw: The disk value can be `nil` when editing some YAML file.
For example, the editing kernel and initrd location in `hack/inject-cmdline-to-template.sh.`


AkihiroSuda: Please squash and sign off the commits 
songponssw: Got it! However, I noticed some unexpected behavior when no disk size is specified.

The generated YAML doesn't include the disk field, causing this function to return nil.
However, the disk shrinking is still rejected later when preparing the disk image.

I'm not sure whether we should inject the default disk size into the YAML when creating the new instance, or handle it another way.


Patch:
diff --git a/cmd/limactl/edit.go b/cmd/limactl/edit.go
index 33ee9581..9c57e18a 100644
--- a/cmd/limactl/edit.go
+++ b/cmd/limactl/edit.go
@@ -118,13 +118,13 @@ func editAction(cmd *cobra.Command, args []string) error {
 		return err
 	}
 	if err := limayaml.Validate(y, true); err != nil {
-		rejectedYAML := "lima.REJECTED.yaml"
-		if writeErr := os.WriteFile(rejectedYAML, yBytes, 0o644); writeErr != nil {
-			return fmt.Errorf("the YAML is invalid, attempted to save the buffer as %q but failed: %w: %w", rejectedYAML, writeErr, err)
-		}
-		// TODO: may need to support editing the rejected YAML
-		return fmt.Errorf("the YAML is invalid, saved the buffer as %q: %w", rejectedYAML, err)
+		return saveRejectedYAML(yBytes, err)
+	}
+
+	if err := limayaml.ValidateYAMLAgainstLatestConfig(yBytes, yContent); err != nil {
+		return saveRejectedYAML(yBytes, err)
 	}
+
 	if err := os.WriteFile(filePath, yBytes, 0o644); err != nil {
 		return err
 	}
@@ -171,3 +171,13 @@ func askWhetherToStart() (bool, error) {
 func editBashComplete(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
 	return bashCompleteInstanceNames(cmd)
 }
+
+// saveRejectedYAML writes the rejected config and returns an error.
+func saveRejectedYAML(y []byte, origErr error) error {
+	rejectedYAML := "lima.REJECTED.yaml"
+	if writeErr := os.WriteFile(rejectedYAML, y, 0o644); writeErr != nil {
+		return fmt.Errorf("the YAML is invalid, attempted to save the buffer as %q but failed: %w", rejectedYAML, errors.Join(writeErr, origErr))
+	}
+	// TODO: may need to support editing the rejected YAML
+	return fmt.Errorf("the YAML is invalid, saved the buffer as %q: %w", rejectedYAML, origErr)
+}
diff --git a/pkg/limayaml/validate.go b/pkg/limayaml/validate.go
index 05aac468..443eaff0 100644
--- a/pkg/limayaml/validate.go
+++ b/pkg/limayaml/validate.go
@@ -610,3 +610,40 @@ func warnExperimental(y *LimaYAML) {
 		logrus.Warn("`mountInotify` is experimental")
 	}
 }
+
+// ValidateYAMLAgainstLatestConfig validates the values between the latest YAML and the updated(New) YAML.
+// This validates configuration rules that disallow certain changes, such as shrinking the disk.
+func ValidateYAMLAgainstLatestConfig(yNew, yLatest []byte) error {
+	var n LimaYAML
+
+	// Load the latest YAML and fill in defaults
+	l, err := LoadWithWarnings(yLatest, "")
+	if err != nil {
+		return err
+	}
+	if err := Unmarshal(yNew, &n, "Unmarshal new YAML bytes"); err != nil {
+		return err
+	}
+
+	// Handle editing the template without a disk value
+	if n.Disk == nil || l.Disk == nil {
+		return nil
+	}
+
+	// Disk value must be provided, as it is required when creating an instance.
+	nDisk, err := units.RAMInBytes(*n.Disk)
+	if err != nil {
+		return err
+	}
+	lDisk, err := units.RAMInBytes(*l.Disk)
+	if err != nil {
+		return err
+	}
+
+	// Reject shrinking disk
+	if nDisk < lDisk {
+		return fmt.Errorf("field `disk`: shrinking the disk (%v --> %v) is not supported", *l.Disk, *n.Disk)
+	}
+
+	return nil
+}


Test patch:
diff --git a/pkg/limayaml/validate_test.go b/pkg/limayaml/validate_test.go
index 859d8ea9..e136c6cb 100644
--- a/pkg/limayaml/validate_test.go
+++ b/pkg/limayaml/validate_test.go
@@ -4,6 +4,7 @@
 package limayaml
 
 import (
+	"errors"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -264,3 +265,55 @@ provision:
 		"field `provision[0].mode` must one of \"system\", \"user\", \"boot\", \"data\", \"dependency\", or \"ansible\"\n"+
 		"field `provision[1].path` must not be empty when mode is \"data\"")
 }
+
+func TestValidateYAMLAgainstLatestConfig(t *testing.T) {
+	tests := []struct {
+		name    string
+		yNew    string
+		yLatest string
+		wantErr error
+	}{
+		{
+			name:    "Valid disk size unchanged",
+			yNew:    `disk: 100GiB`,
+			yLatest: `disk: 100GiB`,
+		},
+		{
+			name:    "Valid disk size increased",
+			yNew:    `disk: 200GiB`,
+			yLatest: `disk: 100GiB`,
+		},
+		{
+			name:    "No disk field in both YAMLs",
+			yNew:    ``,
+			yLatest: ``,
+		},
+		{
+			name:    "No disk field in new YAMLs",
+			yNew:    ``,
+			yLatest: `disk: 100GiB`,
+		},
+		{
+			name:    "No disk field in latest YAMLs",
+			yNew:    `disk: 100GiB`,
+			yLatest: ``,
+		},
+		{
+			name:    "Disk size shrunk",
+			yNew:    `disk: 50GiB`,
+			yLatest: `disk: 100GiB`,
+			wantErr: errors.New("field `disk`: shrinking the disk (100GiB --> 50GiB) is not supported"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateYAMLAgainstLatestConfig([]byte(tt.yNew), []byte(tt.yLatest))
+			if tt.wantErr == nil {
+				assert.NilError(t, err)
+			} else {
+				assert.Error(t, err, tt.wantErr.Error())
+			}
+		})
+	}
+}