From 0acc7bd4dcc8a24cd3cc3999dfea094e74f6b6af Mon Sep 17 00:00:00 2001
From: Omer Aplatony <omerap12@gmail.com>
Date: Sun, 1 Sep 2024 11:12:05 +0300
Subject: [PATCH] HPA: Fix int overflow in GetExternalPerPodMetricReplicas

Signed-off-by: Omer Aplatony <omerap12@gmail.com>
---
 .../podautoscaler/replica_calculator.go       | 20 ++++++++++--
 .../podautoscaler/replica_calculator_test.go  | 32 +++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/pkg/controller/podautoscaler/replica_calculator.go b/pkg/controller/podautoscaler/replica_calculator.go
index 97bf1ead639..bbaf7acc800 100644
--- a/pkg/controller/podautoscaler/replica_calculator.go
+++ b/pkg/controller/podautoscaler/replica_calculator.go
@@ -375,15 +375,31 @@ func (c *ReplicaCalculator) GetExternalPerPodMetricReplicas(statusReplicas int32
 	usage = 0
 	for _, val := range metrics {
 		usage = usage + val
+		if usage < 0 {
+			// the only way we would ever get here is because of an Int64 overflow
+			usage = math.MaxInt64
+			break
+		}
 	}
 
 	replicaCount = statusReplicas
 	usageRatio := float64(usage) / (float64(targetUsagePerPod) * float64(replicaCount))
 	if !tolerances.isWithin(usageRatio) {
 		// update number of replicas if the change is large enough
-		replicaCount = int32(math.Ceil(float64(usage) / float64(targetUsagePerPod)))
+		replicaCountResult := math.Ceil(float64(usage) / float64(targetUsagePerPod))
+		// Ensure that the result exceeds the bounds of an int32
+		if replicaCountResult > float64(math.MaxInt32) {
+			replicaCount = math.MaxInt32
+		} else {
+			replicaCount = int32(replicaCountResult)
+		}
+	}
+	// Handle usage overflow cases
+	if float64(usage) >= float64(math.MaxInt64) {
+		usage = math.MaxInt64
+	} else {
+		usage = int64(math.Ceil(float64(usage) / float64(statusReplicas)))
 	}
-	usage = int64(math.Ceil(float64(usage) / float64(statusReplicas)))
 	return replicaCount, usage, timestamp, nil
 }
 
diff --git a/pkg/controller/podautoscaler/replica_calculator_test.go b/pkg/controller/podautoscaler/replica_calculator_test.go
index a91ffa4a09f..3522c0f1d9a 100644
--- a/pkg/controller/podautoscaler/replica_calculator_test.go
+++ b/pkg/controller/podautoscaler/replica_calculator_test.go
@@ -561,6 +561,38 @@ func TestReplicaCalcScaleUpUnreadyNoScale(t *testing.T) {
 	tc.runTest(t)
 }
 
+func TestExternalPerPodMetricReplicaOverflow(t *testing.T) {
+	tc := replicaCalcTestCase{
+		currentReplicas:  1,
+		expectedReplicas: math.MaxInt32,
+		metric: &metricInfo{
+			name:              "qps",
+			levels:            []int64{math.MaxInt64},
+			perPodTargetUsage: 1, // Set to 1 to test replica calculation when targeting a 1:1 ratio between metric value and number of pods
+			metricType:        externalPerPodMetric,
+			expectedUsage:     math.MaxInt64,
+			selector:          &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+		},
+	}
+	tc.runTest(t)
+}
+
+func TestExternalPerPodMetricUsageOverflow(t *testing.T) {
+	tc := replicaCalcTestCase{
+		currentReplicas:  1,
+		expectedReplicas: 1,
+		metric: &metricInfo{
+			name:              "qps",
+			levels:            []int64{math.MaxInt64},
+			perPodTargetUsage: math.MaxInt64, // Set to a high value to test replica calculation when targeting a high value:1 ratio between metric value and number of pods
+			metricType:        externalPerPodMetric,
+			expectedUsage:     math.MaxInt64,
+			selector:          &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}},
+		},
+	}
+	tc.runTest(t)
+}
+
 func TestReplicaCalcScaleHotCpuNoScale(t *testing.T) {
 	tc := replicaCalcTestCase{
 		currentReplicas:  3,