Merge pull request #1203 from courageJ/fix/kubelet-poll-marshaling

Fix potential nil pointer dereferences in kubelet-to-gcm

Author: courageJ

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

@@ -98,6 +98,9 @@ func (c *Client) doRequestAndParse(req *http.Request) (*Metrics, error) {
 	if err != nil {
 		return nil, err
 	}
+	if response == nil {
+		return nil, fmt.Errorf("empty response from controller")
+	}
 	defer response.Body.Close()
 	body, err := ioutil.ReadAll(response.Body)
 	if err != nil {

kubelet-to-gcm/monitor/convert.go

@@ -25,3 +25,8 @@ func Float64Ptr(f float64) *float64 {
 func Int64Ptr(i int64) *int64 {
 	return &i
 }
+
+// Uint64Ptr gives a pointer to a variable with the same value as u.
+func Uint64Ptr(u uint64) *uint64 {
+	return &u
+}

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

@@ -57,6 +57,9 @@ func (k *Client) doRequestAndUnmarshal(client *http.Client, req *http.Request, v
 	if err != nil {
 		return err
 	}
+	if response == nil {
+		return fmt.Errorf("empty response from kubelet")
+	}
 	defer response.Body.Close()
 	body, err := ioutil.ReadAll(response.Body)
 	if err != nil {

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

@@ -456,10 +456,10 @@ func translateCPU(cpu *stats.CPUStats, tsFactory *timeSeriesFactory, startTime t
 
 func containerTranslateFS(volume string, rootfs *stats.FsStats, logs *stats.FsStats, tsFactory *timeSeriesFactory, startTime time.Time) ([]*v3.TimeSeries, error) {
 	var combinedUsage uint64
-	if rootfs != nil {
+	if rootfs != nil && rootfs.UsedBytes != nil {
 		combinedUsage = *rootfs.UsedBytes
 	}
-	if logs != nil {
+	if logs != nil && logs.UsedBytes != nil {
 		combinedUsage = combinedUsage + *logs.UsedBytes
 	}
 
@@ -533,7 +533,7 @@ func translateMemory(memory *stats.MemoryStats, tsFactory *timeSeriesFactory, st
 			}, startTime, memory.Time.Time, pageFaultsMD.MetricKind)
 			timeSeries = append(timeSeries, tsFactory.newTimeSeries(majorPageFaultLabels, pageFaultsMD, majorPFPoint))
 		}
-		if memory.PageFaults != nil {
+		if memory.PageFaults != nil && memory.MajorPageFaults != nil {
 			// Minor page faults.
 			minorPFPoint := tsFactory.newPoint(&v3.TypedValue{
 				Int64Value:      monitor.Int64Ptr(int64(*memory.PageFaults - *memory.MajorPageFaults)),

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

@@ -25,6 +25,8 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	stats "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
+
+	"github.com/GoogleCloudPlatform/k8s-stackdriver/kubelet-to-gcm/monitor"
 )
 
 const (
@@ -300,6 +302,59 @@ func TestTranslator(t *testing.T) {
 	}
 }
 
+func TestTranslator_NilFields(t *testing.T) {
+	summary := &stats.Summary{
+		Node: stats.NodeStats{
+			StartTime: metav1.NewTime(time.Now()),
+			Memory: &stats.MemoryStats{
+				Time:            metav1.NewTime(time.Now().Add(time.Second)),
+				WorkingSetBytes: monitor.Uint64Ptr(100),
+				PageFaults:      monitor.Uint64Ptr(10),
+				MajorPageFaults: nil, // This would cause panic in translateMemory
+			},
+			Fs: &stats.FsStats{
+				CapacityBytes: monitor.Uint64Ptr(1000),
+				UsedBytes:     monitor.Uint64Ptr(500),
+			},
+			CPU: &stats.CPUStats{
+				Time:                 metav1.NewTime(time.Now().Add(time.Second)),
+				UsageCoreNanoSeconds: monitor.Uint64Ptr(100),
+			},
+		},
+		Pods: []stats.PodStats{
+			{
+				PodRef: stats.PodReference{Name: "test-pod", Namespace: "test-ns"},
+				Containers: []stats.ContainerStats{
+					{
+						Name:      "test-container",
+						StartTime: metav1.NewTime(time.Now()),
+						CPU: &stats.CPUStats{
+							Time:                 metav1.NewTime(time.Now().Add(time.Second)),
+							UsageCoreNanoSeconds: monitor.Uint64Ptr(100),
+						},
+						Memory: &stats.MemoryStats{
+							Time:            metav1.NewTime(time.Now().Add(time.Second)),
+							WorkingSetBytes: monitor.Uint64Ptr(100),
+						},
+						Rootfs: &stats.FsStats{
+							UsedBytes: nil, // This would cause panic in containerTranslateFS
+						},
+						Logs: &stats.FsStats{
+							UsedBytes: nil, // This would cause panic in containerTranslateFS
+						},
+					},
+				},
+			},
+		},
+	}
+
+	translator := NewTranslator("zone", "project", "cluster", "location", "instance", "id", "k8s_", map[string]string{}, time.Minute)
+	_, err := translator.Translate(summary)
+	if err != nil {
+		t.Errorf("Translate failed: %v", err)
+	}
+}
+
 func TestTranslateContainers(t *testing.T) {
 	aliceContainer := *getContainerStats(false)
 	bobContainer := *getContainerStats(false)

kubelet-to-gcm/monitor/poll.go

@@ -44,6 +44,10 @@ type MetricsSource interface {
 
 // Once polls the backend and puts the data to the given service one time.
 func Once(src MetricsSource, gcm *v3.Service) {
+	if gcm == nil {
+		log.Warningf("GCM service is nil, cannot push metrics")
+		return
+	}
 	scrapeTimestamp := time.Now()
 	req, err := src.GetTimeSeriesReq()
 	if err != nil {