Handle nil pointers and empty arrays properly in Azure Monitor S… (#680)

kedacore · Mar 16, 2020 · 2c1373e · 2c1373e
1 parent 4bf72ed
commit 2c1373e
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 5 deletions.
diff --git a/pkg/scalers/azure_monitor.go b/pkg/scalers/azure_monitor.go
@@ -118,19 +118,19 @@ func extractValue(azMetricRequest azureExternalMetricRequest, metricResult insig
 		return -1, err
 	}
 
-	timeseries := *metricVals[0].Timeseries
-	if timeseries == nil {
+	timeseriesPtr := metricVals[0].Timeseries
+	if timeseriesPtr == nil || len(*timeseriesPtr) == 0 {
 		err := fmt.Errorf("Got metric result for %s/%s and aggregate type %s without timeseries", azMetricRequest.ResourceProviderNamespace, azMetricRequest.MetricName, insights.AggregationType(strings.ToTitle(azMetricRequest.Aggregation)))
 		return -1, err
 	}
 
-	data := *timeseries[0].Data
-	if data == nil {
+	dataPtr := (*timeseriesPtr)[0].Data
+	if dataPtr == nil || len(*dataPtr) == 0 {
 		err := fmt.Errorf("Got metric result for %s/%s and aggregate type %s without any metric values", azMetricRequest.ResourceProviderNamespace, azMetricRequest.MetricName, insights.AggregationType(strings.ToTitle(azMetricRequest.Aggregation)))
 		return -1, err
 	}
 
-	valuePtr, err := verifyAggregationTypeIsSupported(azMetricRequest.Aggregation, data)
+	valuePtr, err := verifyAggregationTypeIsSupported(azMetricRequest.Aggregation, *dataPtr)
 	if err != nil {
 		return -1, fmt.Errorf("Unable to get value for metric %s/%s with aggregation %s. No value returned by Azure Monitor", azMetricRequest.ResourceProviderNamespace, azMetricRequest.MetricName, azMetricRequest.Aggregation)
 	}

diff --git a/pkg/scalers/azure_monitor_test.go b/pkg/scalers/azure_monitor_test.go
@@ -2,6 +2,8 @@ package scalers
 
 import (
 	"testing"
+
+	"github.com/Azure/azure-sdk-for-go/services/preview/monitor/mgmt/2018-03-01/insights"
 )
 
 type parseAzMonitorMetadataTestData struct {
@@ -62,3 +64,47 @@ func TestAzMonitorParseMetadata(t *testing.T) {
 		}
 	}
 }
+
+type testExtractAzMonitorTestData struct {
+	testName      string
+	isError       bool
+	expectedValue float64
+	metricRequest azureExternalMetricRequest
+	metricResult  insights.Response
+}
+
+var testExtractAzMonitordata = []testExtractAzMonitorTestData{
+	{"nothing returned", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{}}},
+	{"timeseries null", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: nil}}}},
+	{"timeseries empty", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{}}}}},
+	{"data nil", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: nil}}}}}},
+	{"data empty", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{}}}}}}},
+	{"Total Aggregation requested", false, 40, azureExternalMetricRequest{Aggregation: "Total"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Total: returnFloat64Ptr(40)}}}}}}}},
+	{"Average Aggregation requested", false, 41, azureExternalMetricRequest{Aggregation: "Average"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Average: returnFloat64Ptr(41)}}}}}}}},
+	{"Maximum Aggregation requested", false, 42, azureExternalMetricRequest{Aggregation: "Maximum"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Maximum: returnFloat64Ptr(42)}}}}}}}},
+	{"Minimum Aggregation requested", false, 43, azureExternalMetricRequest{Aggregation: "Minimum"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Minimum: returnFloat64Ptr(43)}}}}}}}},
+	{"Count Aggregation requested", false, 44, azureExternalMetricRequest{Aggregation: "Count"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Count: returnint64Ptr(44)}}}}}}}},
+}
+
+func returnFloat64Ptr(x float64) *float64 {
+	return &x
+}
+
+func returnint64Ptr(x int64) *int64 {
+	return &x
+}
+
+func TestAzMonitorextractValue(t *testing.T) {
+	for _, testData := range testExtractAzMonitordata {
+		value, err := extractValue(testData.metricRequest, testData.metricResult)
+		if err != nil && !testData.isError {
+			t.Errorf("Test: %v; Expected success but got error: %v", testData.testName, err)
+		}
+		if testData.isError && err == nil {
+			t.Errorf("Test: %v; Expected error but got success. testData: %v", testData.testName, testData)
+		}
+		if err != nil && value != testData.expectedValue {
+			t.Errorf("Test: %v; Expected value %v but got %v testData: %v", testData.testName, testData.expectedValue, value, testData)
+		}
+	}
+}