Skip to content

Commit 3c0b7d5

Browse files
committed
Fix potential nil pointer dereferences in kubelet-to-gcm
1 parent fe8f334 commit 3c0b7d5

6 files changed

Lines changed: 73 additions & 3 deletions

File tree

kubelet-to-gcm/monitor/controller/client.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ func (c *Client) doRequestAndParse(req *http.Request) (*Metrics, error) {
9898
if err != nil {
9999
return nil, err
100100
}
101+
if response == nil {
102+
return nil, fmt.Errorf("empty response from controller")
103+
}
101104
defer response.Body.Close()
102105
body, err := ioutil.ReadAll(response.Body)
103106
if err != nil {

kubelet-to-gcm/monitor/convert.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,8 @@ func Float64Ptr(f float64) *float64 {
2525
func Int64Ptr(i int64) *int64 {
2626
return &i
2727
}
28+
29+
// Uint64Ptr gives a pointer to a variable with the same value as u.
30+
func Uint64Ptr(u uint64) *uint64 {
31+
return &u
32+
}

kubelet-to-gcm/monitor/kubelet/client.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ func (k *Client) doRequestAndUnmarshal(client *http.Client, req *http.Request, v
5757
if err != nil {
5858
return err
5959
}
60+
if response == nil {
61+
return fmt.Errorf("empty response from kubelet")
62+
}
6063
defer response.Body.Close()
6164
body, err := ioutil.ReadAll(response.Body)
6265
if err != nil {

kubelet-to-gcm/monitor/kubelet/translate.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -456,10 +456,10 @@ func translateCPU(cpu *stats.CPUStats, tsFactory *timeSeriesFactory, startTime t
456456

457457
func containerTranslateFS(volume string, rootfs *stats.FsStats, logs *stats.FsStats, tsFactory *timeSeriesFactory, startTime time.Time) ([]*v3.TimeSeries, error) {
458458
var combinedUsage uint64
459-
if rootfs != nil {
459+
if rootfs != nil && rootfs.UsedBytes != nil {
460460
combinedUsage = *rootfs.UsedBytes
461461
}
462-
if logs != nil {
462+
if logs != nil && logs.UsedBytes != nil {
463463
combinedUsage = combinedUsage + *logs.UsedBytes
464464
}
465465

@@ -533,7 +533,7 @@ func translateMemory(memory *stats.MemoryStats, tsFactory *timeSeriesFactory, st
533533
}, startTime, memory.Time.Time, pageFaultsMD.MetricKind)
534534
timeSeries = append(timeSeries, tsFactory.newTimeSeries(majorPageFaultLabels, pageFaultsMD, majorPFPoint))
535535
}
536-
if memory.PageFaults != nil {
536+
if memory.PageFaults != nil && memory.MajorPageFaults != nil {
537537
// Minor page faults.
538538
minorPFPoint := tsFactory.newPoint(&v3.TypedValue{
539539
Int64Value: monitor.Int64Ptr(int64(*memory.PageFaults - *memory.MajorPageFaults)),

kubelet-to-gcm/monitor/kubelet/translate_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import (
2525

2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2727
stats "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
28+
29+
"github.com/GoogleCloudPlatform/k8s-stackdriver/kubelet-to-gcm/monitor"
2830
)
2931

3032
const (
@@ -300,6 +302,59 @@ func TestTranslator(t *testing.T) {
300302
}
301303
}
302304

305+
func TestTranslator_NilFields(t *testing.T) {
306+
summary := &stats.Summary{
307+
Node: stats.NodeStats{
308+
StartTime: metav1.NewTime(time.Now()),
309+
Memory: &stats.MemoryStats{
310+
Time: metav1.NewTime(time.Now().Add(time.Second)),
311+
WorkingSetBytes: monitor.Uint64Ptr(100),
312+
PageFaults: monitor.Uint64Ptr(10),
313+
MajorPageFaults: nil, // This would cause panic in translateMemory
314+
},
315+
Fs: &stats.FsStats{
316+
CapacityBytes: monitor.Uint64Ptr(1000),
317+
UsedBytes: monitor.Uint64Ptr(500),
318+
},
319+
CPU: &stats.CPUStats{
320+
Time: metav1.NewTime(time.Now().Add(time.Second)),
321+
UsageCoreNanoSeconds: monitor.Uint64Ptr(100),
322+
},
323+
},
324+
Pods: []stats.PodStats{
325+
{
326+
PodRef: stats.PodReference{Name: "test-pod", Namespace: "test-ns"},
327+
Containers: []stats.ContainerStats{
328+
{
329+
Name: "test-container",
330+
StartTime: metav1.NewTime(time.Now()),
331+
CPU: &stats.CPUStats{
332+
Time: metav1.NewTime(time.Now().Add(time.Second)),
333+
UsageCoreNanoSeconds: monitor.Uint64Ptr(100),
334+
},
335+
Memory: &stats.MemoryStats{
336+
Time: metav1.NewTime(time.Now().Add(time.Second)),
337+
WorkingSetBytes: monitor.Uint64Ptr(100),
338+
},
339+
Rootfs: &stats.FsStats{
340+
UsedBytes: nil, // This would cause panic in containerTranslateFS
341+
},
342+
Logs: &stats.FsStats{
343+
UsedBytes: nil, // This would cause panic in containerTranslateFS
344+
},
345+
},
346+
},
347+
},
348+
},
349+
}
350+
351+
translator := NewTranslator("zone", "project", "cluster", "location", "instance", "id", "k8s_", map[string]string{}, time.Minute)
352+
_, err := translator.Translate(summary)
353+
if err != nil {
354+
t.Errorf("Translate failed: %v", err)
355+
}
356+
}
357+
303358
func TestTranslateContainers(t *testing.T) {
304359
aliceContainer := *getContainerStats(false)
305360
bobContainer := *getContainerStats(false)

kubelet-to-gcm/monitor/poll.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ type MetricsSource interface {
4444

4545
// Once polls the backend and puts the data to the given service one time.
4646
func Once(src MetricsSource, gcm *v3.Service) {
47+
if gcm == nil {
48+
log.Warningf("GCM service is nil, cannot push metrics")
49+
return
50+
}
4751
scrapeTimestamp := time.Now()
4852
req, err := src.GetTimeSeriesReq()
4953
if err != nil {

0 commit comments

Comments
 (0)