thaloco/banger
1
Fork 0
Code Pull requests Releases Packages Activity Actions

Reject invalid VM CPU and memory values

Browse source 
VM create and vm set accepted zero or negative CPU and memory values, which either got stored directly or silently fell back to defaults and only surfaced as failures later. This tightens validation so bad settings are rejected at the user boundary and again in the daemon before any VM record is persisted.

Change vm.create CPU and memory request fields to optional pointers so omitted values still mean defaults, while explicit non-positive values can be distinguished and rejected. Update Cobra create/set parsing, keep the TUI aligned with the new API shape, and add regression tests for CLI parsing, daemon-side validation, and the create-defaults path.

Validation: go test ./... and make build. Left my-rootfs.ext4 untracked.
This commit is contained in:
Thales Maciel 2026-03-16 16:28:17 -03:00
parent 67cfdd659f
commit 6e00fa690d
7 changed files with 202 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

68
internal/cli/banger.go
View file

@ -163,12 +163,25 @@ func newVMKillCommand() *cobra.Command {
}
func newVMCreateCommand() *cobra.Command {
var params api.VMCreateParams
var (
name string
imageName string
vcpu int
memory int
systemOverlaySize string
workDiskSize string
natEnabled bool
noStart bool
)
cmd := &cobra.Command{
Use: "create",
Short: "Create a VM",
Args: noArgsUsage("usage: banger vm create"),
RunE: func(cmd *cobra.Command, args []string) error {
params, err := vmCreateParamsFromFlags(cmd, name, imageName, vcpu, memory, systemOverlaySize, workDiskSize, natEnabled, noStart)
if err != nil {
return err
}
if err := system.EnsureSudo(cmd.Context()); err != nil {
return err
}
@ -183,14 +196,14 @@ func newVMCreateCommand() *cobra.Command {
return printVMSummary(cmd.OutOrStdout(), result.VM)
},
}
cmd.Flags().StringVar(&params.Name, "name", "", "vm name")
cmd.Flags().StringVar(&params.ImageName, "image", "", "image name or id")
cmd.Flags().IntVar(&params.VCPUCount, "vcpu", 0, "vcpu count")
cmd.Flags().IntVar(&params.MemoryMiB, "memory", 0, "memory in MiB")
cmd.Flags().StringVar(&params.SystemOverlaySize, "system-overlay-size", "", "system overlay size")
cmd.Flags().StringVar(&params.WorkDiskSize, "disk-size", "", "work disk size")
cmd.Flags().BoolVar(&params.NATEnabled, "nat", false, "enable NAT")
cmd.Flags().BoolVar(&params.NoStart, "no-start", false, "create without starting")
cmd.Flags().StringVar(&name, "name", "", "vm name")
cmd.Flags().StringVar(&imageName, "image", "", "image name or id")
cmd.Flags().IntVar(&vcpu, "vcpu", 0, "vcpu count")
cmd.Flags().IntVar(&memory, "memory", 0, "memory in MiB")
cmd.Flags().StringVar(&systemOverlaySize, "system-overlay-size", "", "system overlay size")
cmd.Flags().StringVar(&workDiskSize, "disk-size", "", "work disk size")
cmd.Flags().BoolVar(&natEnabled, "nat", false, "enable NAT")
cmd.Flags().BoolVar(&noStart, "no-start", false, "create without starting")
return cmd
}
@ -648,9 +661,15 @@ func vmSetParamsFromFlags(idOrName string, vcpu, memory int, diskSize string, na
}
params := api.VMSetParams{IDOrName: idOrName, WorkDiskSize: diskSize}
if vcpu >= 0 {
if err := validatePositiveSetting("vcpu", vcpu); err != nil {
return api.VMSetParams{}, err
}
params.VCPUCount = &vcpu
}
if memory >= 0 {
if err := validatePositiveSetting("memory", memory); err != nil {
return api.VMSetParams{}, err
}
params.MemoryMiB = &memory
}
if nat || noNat {
@ -663,6 +682,37 @@ func vmSetParamsFromFlags(idOrName string, vcpu, memory int, diskSize string, na
return params, nil
}
func vmCreateParamsFromFlags(cmd *cobra.Command, name, imageName string, vcpu, memory int, systemOverlaySize, workDiskSize string, natEnabled, noStart bool) (api.VMCreateParams, error) {
params := api.VMCreateParams{
Name: name,
ImageName: imageName,
SystemOverlaySize: systemOverlaySize,
WorkDiskSize: workDiskSize,
NATEnabled: natEnabled,
NoStart: noStart,
}
if cmd.Flags().Changed("vcpu") {
if err := validatePositiveSetting("vcpu", vcpu); err != nil {
return api.VMCreateParams{}, err
}
params.VCPUCount = &vcpu
}
if cmd.Flags().Changed("memory") {
if err := validatePositiveSetting("memory", memory); err != nil {
return api.VMCreateParams{}, err
}
params.MemoryMiB = &memory
}
return params, nil
}
func validatePositiveSetting(label string, value int) error {
if value <= 0 {
return fmt.Errorf("%s must be a positive integer", label)
}
return nil
}
func sshCommandArgs(cfg model.DaemonConfig, guestIP string, extra []string) ([]string, error) {
if guestIP == "" {
return nil, errors.New("vm has no guest IP")

53
internal/cli/cli_test.go
View file

@ -75,12 +75,65 @@ func TestVMSetParamsFromFlags(t *testing.T) {
}
}
func TestVMCreateParamsFromFlagsUsesNilForOmittedCPUAndMemory(t *testing.T) {
cmd := NewBangerCommand()
vm, _, err := cmd.Find([]string{"vm"})
if err != nil {
t.Fatalf("find vm: %v", err)
}
create, _, err := vm.Find([]string{"create"})
if err != nil {
t.Fatalf("find create: %v", err)
}
params, err := vmCreateParamsFromFlags(create, "devbox", "default", 0, 0, "8G", "16G", false, false)
if err != nil {
t.Fatalf("vmCreateParamsFromFlags: %v", err)
}
if params.VCPUCount != nil || params.MemoryMiB != nil {
t.Fatalf("expected omitted cpu/memory to stay nil: %+v", params)
}
}
func TestVMCreateParamsFromFlagsRejectsNonPositiveCPUAndMemory(t *testing.T) {
cmd := NewBangerCommand()
vm, _, err := cmd.Find([]string{"vm"})
if err != nil {
t.Fatalf("find vm: %v", err)
}
create, _, err := vm.Find([]string{"create"})
if err != nil {
t.Fatalf("find create: %v", err)
}
if err := create.Flags().Set("vcpu", "0"); err != nil {
t.Fatalf("set vcpu flag: %v", err)
}
if _, err := vmCreateParamsFromFlags(create, "devbox", "default", 0, 0, "", "", false, false); err == nil || !strings.Contains(err.Error(), "vcpu must be a positive integer") {
t.Fatalf("vmCreateParamsFromFlags(vcpu=0) error = %v", err)
}
if err := create.Flags().Set("memory", "-1"); err != nil {
t.Fatalf("set memory flag: %v", err)
}
if _, err := vmCreateParamsFromFlags(create, "devbox", "default", 1, -1, "", "", false, false); err == nil || !strings.Contains(err.Error(), "memory must be a positive integer") {
t.Fatalf("vmCreateParamsFromFlags(memory=-1) error = %v", err)
}
}
func TestVMSetParamsFromFlagsConflict(t *testing.T) {
if _, err := vmSetParamsFromFlags("devbox", -1, -1, "", true, true); err == nil {
t.Fatal("expected nat conflict error")
}
}
func TestVMSetParamsFromFlagsRejectsNonPositiveCPUAndMemory(t *testing.T) {
if _, err := vmSetParamsFromFlags("devbox", 0, -1, "", false, false); err == nil || !strings.Contains(err.Error(), "vcpu must be a positive integer") {
t.Fatalf("vmSetParamsFromFlags(vcpu=0) error = %v", err)
}
if _, err := vmSetParamsFromFlags("devbox", -1, 0, "", false, false); err == nil || !strings.Contains(err.Error(), "memory must be a positive integer") {
t.Fatalf("vmSetParamsFromFlags(memory=0) error = %v", err)
}
}
func TestSSHCommandArgs(t *testing.T) {
args, err := sshCommandArgs(model.DaemonConfig{SSHKeyPath: "/bundle/id_ed25519"}, "172.16.0.2", []string{"--", "uname", "-a"})
if err != nil {

4
internal/cli/tui.go
View file

@ -399,8 +399,8 @@ func (f *vmForm) createRequest() (actionRequest, error) {
params := api.VMCreateParams{
Name: strings.TrimSpace(f.fields[0].value()),
ImageName: strings.TrimSpace(f.fields[1].value()),
VCPUCount: vcpu,
MemoryMiB: memory,
VCPUCount: &vcpu,
MemoryMiB: &memory,
SystemOverlaySize: strings.TrimSpace(f.fields[4].value()),
WorkDiskSize: strings.TrimSpace(f.fields[5].value()),
NATEnabled: isYes(f.fields[6].value()),

2
internal/cli/tui_test.go
View file

@ -25,7 +25,7 @@ func TestCreateVMFormSubmit(t *testing.T) {
if action.create.Name != "devbox" || action.create.ImageName != "default" {
t.Fatalf("unexpected create params: %+v", action.create)
}
if action.create.VCPUCount != 4 || action.create.MemoryMiB != 2048 {
if action.create.VCPUCount == nil || *action.create.VCPUCount != 4 || action.create.MemoryMiB == nil || *action.create.MemoryMiB != 2048 {
t.Fatalf("unexpected cpu/memory: %+v", action.create)
}
if action.create.SystemOverlaySize != "12G" || action.create.WorkDiskSize != "24G" {