add error checking for sqs metrics (#6235)

* add error checking for sqs metrics to avoid runtime panics Signed-off-by: Rob Pickerill <[email protected]> * update changelog Signed-off-by: Rob Pickerill <[email protected]> --------- Signed-off-by: Rob Pickerill <[email protected]> Co-authored-by: Jorge Turrado Ferrero <[email protected]>
kedacore · Nov 3, 2024 · 9980181 · 9980181
1 parent ea6adc4
commit 9980181
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -91,6 +91,7 @@ Here is an overview of all new **experimental** features:
 ### Fixes
 
 - **AWS Secret Manager**: Pod identity overrides are honored ([#6195](https://github.com/kedacore/keda/issues/6195))
+- **AWS SQS Scaler**: Improve error handling for SQS queue metrics ([#6178](https://github.com/kedacore/keda/issues/6178))
 - **Azure Event Hub Scaler**: Checkpointer errors are correctly handled ([#6084](https://github.com/kedacore/keda/issues/6084))
 - **Metrics API Scaler**: Prometheus metrics can have multiple labels ([#6077](https://github.com/kedacore/keda/issues/6077))
 

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)
+		})
+	}
+}