Skip to content

Commit

Permalink
fix: avoid possible panic in HostSystem.ManagementIPs
Browse files Browse the repository at this point in the history
If the HostSystem.Config or HostSystem.Config.VirtualNicManagerInfo fields are nil, the ManagementIPs method would panic.
I was not able to reproduce this situation in a real VC, even with disconnecting a host from VC.
Assuming this state is possible while an ESX host is being upgraded. Updated the test to nil both fields to ensure we don't panic.
  • Loading branch information
dougm committed Nov 23, 2021
1 parent 6209be5 commit f04d77d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
12 changes: 11 additions & 1 deletion object/host_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,17 @@ func (h HostSystem) ManagementIPs(ctx context.Context) ([]net.IP, error) {
return nil, err
}

return internal.HostSystemManagementIPs(mh.Config.VirtualNicManagerInfo.NetConfig), nil
config := mh.Config
if config == nil {
return nil, nil
}

info := config.VirtualNicManagerInfo
if info == nil {
return nil, nil
}

return internal.HostSystemManagementIPs(info.NetConfig), nil
}

func (h HostSystem) Disconnect(ctx context.Context) (*Task, error) {
Expand Down
34 changes: 21 additions & 13 deletions object/host_system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package object_test

import (
"context"
"log"
"testing"

"github.com/vmware/govmomi/find"
Expand All @@ -24,16 +23,8 @@ import (
)

func TestHostSystemManagementIPs(t *testing.T) {
m := simulator.ESX()
m.Run(func(ctx context.Context, c *vim25.Client) error {
finder := find.NewFinder(c, false)
dc, err := finder.DefaultDatacenter(ctx)
if err != nil {
log.Fatalf("Failed to get default DC")
}
finder.SetDatacenter(dc)

host, err := finder.DefaultHostSystem(ctx)
simulator.Test(func(ctx context.Context, c *vim25.Client) {
host, err := find.NewFinder(c).HostSystem(ctx, "DC0_C0_H0")
if err != nil {
t.Fatal(err)
}
Expand All @@ -43,11 +34,28 @@ func TestHostSystemManagementIPs(t *testing.T) {
t.Fatal(err)
}
if len(ips) != 1 {
t.Fatalf("no mgmt ip found")
t.Fatal("no mgmt ip found")
}
if ips[0].String() != "127.0.0.1" {
t.Fatalf("Expected management ip %s, got %s", "127.0.0.1", ips[0].String())
}
return nil

// These fields can be nil while ESX is being upgraded
hs := simulator.Map.Get(host.Reference()).(*simulator.HostSystem)
tests := []func(){
func() { hs.Config.VirtualNicManagerInfo = nil },
func() { hs.Config = nil },
}

for _, f := range tests {
f()
ips, err = host.ManagementIPs(ctx)
if err != nil {
t.Fatal(err)
}
if len(ips) != 0 {
t.Fatal("expected zero ips")
}
}
})
}

0 comments on commit f04d77d

Please sign in to comment.