-
Notifications
You must be signed in to change notification settings - Fork 672
Reject shrinking disk during YAML validation #3596
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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: %w", rejectedYAML, writeErr, origErr) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error message wraps two errors using '%w: %w', which might be confusing. Consider simplifying the error wrapping to clearly associate the cause of failure with a single wrapped error.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As Copilot suggested above, you cannot use 2 So either explicitly choose which one should be wrapped, and use
Suggested change
|
||||||||||
} | ||||||||||
// TODO: may need to support editing the rejected YAML | ||||||||||
return fmt.Errorf("the YAML is invalid, saved the buffer as %q: %w", rejectedYAML, origErr) | ||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -595,3 +595,37 @@ func warnExperimental(y *LimaYAML) { | |||||||||||||||||||||||
logrus.Warn("`mountInotify` is experimental") | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// ValidateYAMLAgainstLatestConfig validates the values between the latest YAML and the updated(New) YAML. | ||||||||||||||||||||||||
func ValidateYAMLAgainstLatestConfig(yNew, yLatest []byte) error { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's cover this function with unit tests. |
||||||||||||||||||||||||
var l, n LimaYAML | ||||||||||||||||||||||||
var err error | ||||||||||||||||||||||||
if err = Unmarshal(yLatest, &l, "Unmarshal latest YAML bytes"); err != nil { | ||||||||||||||||||||||||
return err | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
if err = Unmarshal(yNew, &n, "Unmarshal new YAML byte"); err != nil { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Comment on lines
+602
to
+606
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can avoid
Suggested change
|
||||||||||||||||||||||||
return err | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Handle editing the template without a disk value | ||||||||||||||||||||||||
if n.Disk == nil && l.Disk == nil { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are dereferencing both, so you need to bail if either one is
Suggested change
|
||||||||||||||||||||||||
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 | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
songponssw marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
// 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 | ||||||||||||||||||||||||
} |
Uh oh!
There was an error while loading. Please reload this page.