add error checking for sqs metrics to avoid runtime panics

kedacore · Oct 13, 2024 · 3875d4b · 3875d4b
1 parent 0b7ac6d
commit 3875d4b
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 1 deletion.
diff --git a/pkg/scalers/aws_sqs_queue_scaler.go b/pkg/scalers/aws_sqs_queue_scaler.go
@@ -243,12 +243,23 @@ func (s *awsSqsQueueScaler) getAwsSqsQueueLength(ctx context.Context) (int64, er
 		return -1, err
 	}
 
+	return s.processQueueLengthFromSqsQueueAttributesOutput(output)
+}
+
+func (s *awsSqsQueueScaler) processQueueLengthFromSqsQueueAttributesOutput(output *sqs.GetQueueAttributesOutput) (int64, error) {
 	var approximateNumberOfMessages int64
+
 	for _, awsSqsQueueMetric := range s.metadata.awsSqsQueueMetricNames {
-		metricValue, err := strconv.ParseInt(output.Attributes[string(awsSqsQueueMetric)], 10, 32)
+		metricValueString, exists := output.Attributes[string(awsSqsQueueMetric)]
+		if !exists {
+			return -1, fmt.Errorf("metric %s not found in SQS queue attributes", awsSqsQueueMetric)
+		}
+
+		metricValue, err := strconv.ParseInt(metricValueString, 10, 64)
 		if err != nil {
 			return -1, err
 		}
+
 		approximateNumberOfMessages += metricValue
 	}
 

diff --git a/pkg/scalers/aws_sqs_queue_scaler_test.go b/pkg/scalers/aws_sqs_queue_scaler_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/aws/aws-sdk-go-v2/service/sqs"
+	"github.com/aws/aws-sdk-go-v2/service/sqs/types"
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
 
@@ -444,3 +445,91 @@ func TestAWSSQSScalerGetMetrics(t *testing.T) {
 		}
 	}
 }
+
+func TestProcessQueueLengthFromSqsQueueAttributesOutput(t *testing.T) {
+	scalerCreationFunc := func() *awsSqsQueueScaler {
+		return &awsSqsQueueScaler{
+			metadata: &awsSqsQueueMetadata{
+				awsSqsQueueMetricNames: []types.QueueAttributeName{types.QueueAttributeNameApproximateNumberOfMessages, types.QueueAttributeNameApproximateNumberOfMessagesNotVisible, types.QueueAttributeNameApproximateNumberOfMessagesDelayed},
+			},
+		}
+	}
+
+	tests := map[string]struct {
+		s           *awsSqsQueueScaler
+		attributes  *sqs.GetQueueAttributesOutput
+		expected    int64
+		errExpected bool
+	}{
+		"properly formed queue attributes": {
+			s: scalerCreationFunc(),
+			attributes: &sqs.GetQueueAttributesOutput{
+				Attributes: map[string]string{
+					"ApproximateNumberOfMessages":           "1",
+					"ApproximateNumberOfMessagesNotVisible": "0",
+					"ApproximateNumberOfMessagesDelayed":    "0",
+				},
+			},
+			expected:    1,
+			errExpected: false,
+		},
+		"missing ApproximateNumberOfMessages": {
+			s: scalerCreationFunc(),
+			attributes: &sqs.GetQueueAttributesOutput{
+				Attributes: map[string]string{},
+			},
+			expected:    -1,
+			errExpected: true,
+		},
+		"invalid ApproximateNumberOfMessages": {
+			s: scalerCreationFunc(),
+			attributes: &sqs.GetQueueAttributesOutput{
+				Attributes: map[string]string{
+					"ApproximateNumberOfMessages":           "NotInt",
+					"ApproximateNumberOfMessagesNotVisible": "0",
+					"ApproximateNumberOfMessagesDelayed":    "0",
+				},
+			},
+			expected:    -1,
+			errExpected: true,
+		},
+		"32 bit int upper bound": {
+			s: scalerCreationFunc(),
+			attributes: &sqs.GetQueueAttributesOutput{
+				Attributes: map[string]string{
+					"ApproximateNumberOfMessages":           "2147483647",
+					"ApproximateNumberOfMessagesNotVisible": "0",
+					"ApproximateNumberOfMessagesDelayed":    "0",
+				},
+			},
+			expected:    2147483647,
+			errExpected: false,
+		},
+		"32 bit int upper bound + 1": {
+			s: scalerCreationFunc(),
+			attributes: &sqs.GetQueueAttributesOutput{
+				Attributes: map[string]string{
+					"ApproximateNumberOfMessages":           "2147483648",
+					"ApproximateNumberOfMessagesNotVisible": "0",
+					"ApproximateNumberOfMessagesDelayed":    "0",
+				},
+			},
+			expected:    2147483648,
+			errExpected: false,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			result, err := test.s.processQueueLengthFromSqsQueueAttributesOutput(test.attributes)
+
+			if test.errExpected {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+
+			assert.Equal(t, test.expected, result)
+		})
+	}
+}