Merge pull request #2859 from dougm/issue-2835

fix: avoid possible panic in govc metric commands
vmware · Jun 15, 2022 · e42c94b · e42c94b
2 parents 62e5b57 + 37b3b24
commit e42c94b
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 21 deletions.
diff --git a/govc/metric/info.go b/govc/metric/info.go
@@ -185,12 +185,13 @@ func (cmd *info) Run(ctx context.Context, f *flag.FlagSet) error {
 			seen := make(map[int32]bool)
 			for i := range all {
 				id := &all[i]
-				if seen[id.CounterId] {
+				info, ok := nc[id.CounterId]
+				if !ok || seen[id.CounterId] {
 					continue
 				}
 				seen[id.CounterId] = true
 
-				names = append(names, nc[id.CounterId].Name())
+				names = append(names, info.Name())
 			}
 		}
 	}

diff --git a/govc/metric/ls.go b/govc/metric/ls.go
@@ -95,8 +95,8 @@ func (r *lsResult) Write(w io.Writer) error {
 
 	for _, id := range r.MetricList {
 		if r.cmd.group != "" {
-			info := r.counters[id.CounterId]
-			if info.GroupInfo.GetElementDescription().Label != r.cmd.group {
+			info, ok := r.counters[id.CounterId]
+			if !ok || info.GroupInfo.GetElementDescription().Label != r.cmd.group {
 				continue
 			}
 		}
@@ -116,7 +116,10 @@ func (r *lsResult) Write(w io.Writer) error {
 	}
 
 	for _, id := range res {
-		info := r.counters[id.CounterId]
+		info, ok := r.counters[id.CounterId]
+		if !ok {
+			continue
+		}
 
 		switch {
 		case r.cmd.long:
@@ -147,9 +150,9 @@ func (r *lsResult) MarshalJSON() ([]byte, error) {
 	m := make(map[string]*types.PerfCounterInfo)
 
 	for _, id := range r.MetricList {
-		info := r.counters[id.CounterId]
-
-		m[info.Name()] = info
+		if info, ok := r.counters[id.CounterId]; ok {
+			m[info.Name()] = info
+		}
 	}
 
 	return json.Marshal(m)

diff --git a/govc/test/metric.bats b/govc/test/metric.bats
@@ -13,6 +13,7 @@ load test_helper
 
   host=$(govc ls -t HostSystem ./... | head -n 1)
   pool=$(govc ls -t ResourcePool ./... | head -n 1)
+  vm=$(govc ls -t VirtualMachine ./... | head -n 1)
 
   run govc metric.ls "$host"
   assert_success
@@ -22,6 +23,9 @@ load test_helper
 
   run govc metric.ls "$pool"
   assert_success
+
+  run govc metric.ls "$vm"
+  assert_success
 }
 
 @test "metric.sample" {

diff --git a/performance/manager.go b/performance/manager.go
@@ -205,10 +205,18 @@ func (d groupPerfCounterInfo) Len() int {
 func (d groupPerfCounterInfo) Less(i, j int) bool {
 	ci := d.ids[i].CounterId
 	cj := d.ids[j].CounterId
-	gi := d.info[ci].GroupInfo.GetElementDescription()
-	gj := d.info[cj].GroupInfo.GetElementDescription()
 
-	return gi.Key < gj.Key
+	giKey := "-"
+	gjKey := "-"
+
+	if gi, ok := d.info[ci]; ok {
+		giKey = gi.GroupInfo.GetElementDescription().Key
+	}
+	if gj, ok := d.info[cj]; ok {
+		gjKey = gj.GroupInfo.GetElementDescription().Key
+	}
+
+	return giKey < gjKey
 }
 
 func (d groupPerfCounterInfo) Swap(i, j int) {
@@ -455,10 +463,14 @@ func (m *Manager) ToMetricSeries(ctx context.Context, series []types.BasePerfEnt
 
 		for j := range s.Value {
 			v := s.Value[j].(*types.PerfMetricIntSeries)
+			info, ok := counters[v.Id.CounterId]
+			if !ok {
+				continue
+			}
 
 			values = append(values, MetricSeries{
-				Name:     counters[v.Id.CounterId].Name(),
-				unit:     counters[v.Id.CounterId].UnitInfo.GetElementDescription().Key,
+				Name:     info.Name(),
+				unit:     info.UnitInfo.GetElementDescription().Key,
 				Instance: v.Id.Instance,
 				Value:    v.Value,
 			})

diff --git a/simulator/performance_manager.go b/simulator/performance_manager.go
@@ -81,14 +81,7 @@ func (p *PerformanceManager) QueryPerfCounter(ctx *Context, req *types.QueryPerf
 	body.Res = new(types.QueryPerfCounterResponse)
 	body.Res.Returnval = make([]types.PerfCounterInfo, len(req.CounterId))
 	for i, id := range req.CounterId {
-		if info, ok := p.perfCounterIndex[id]; !ok {
-			body.Fault_ = Fault("", &types.InvalidArgument{
-				InvalidProperty: "CounterId",
-			})
-			return body
-		} else {
-			body.Res.Returnval[i] = info
-		}
+		body.Res.Returnval[i] = p.perfCounterIndex[id]
 	}
 	return body
 }
@@ -135,6 +128,8 @@ func (p *PerformanceManager) buildAvailablePerfMetricsQueryResponse(ids []types.
 			r.Returnval = append(r.Returnval, types.PerfMetricId{CounterId: id.CounterId, Instance: id.Instance})
 		}
 	}
+	// Add a CounterId without a corresponding PerfCounterInfo entry. See issue #2835
+	r.Returnval = append(r.Returnval, types.PerfMetricId{CounterId: 10042})
 	return r
 }