Skip to content

Commit

Permalink
Prevent Int64 Overflow (#3605)
Browse files Browse the repository at this point in the history
* Prevent Int64 Overflow

* Necessary changes

* Modify Counter count

* Review changes

* update unit test

* use big.Int

* Review changes

* changes in controller_test.go
  • Loading branch information
Kalaiselvi84 authored Jan 24, 2024
1 parent 38b1d33 commit 702141c
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 16 deletions.
10 changes: 10 additions & 0 deletions pkg/apis/agones/v1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package v1

import (
"math"

apivalidation "k8s.io/apimachinery/pkg/api/validation"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
Expand Down Expand Up @@ -183,3 +185,11 @@ type Priority struct {
// "Ascending" would be smaller Capacity is preferred.
Order string `json:"order"`
}

// SafeAdd prevents overflow by limiting the sum to math.MaxInt64.
func SafeAdd(x, y int64) int64 {
if x > math.MaxInt64-y {
return math.MaxInt64
}
return x + y
}
8 changes: 4 additions & 4 deletions pkg/fleets/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,10 +730,10 @@ func mergeCounters(c1, c2 map[string]agonesv1.AggregatedCounterStatus) map[strin
for key, val := range c2 {
// If the Counter exists in both maps, aggregate the values.
if counter, ok := c1[key]; ok {
counter.AllocatedCapacity += val.AllocatedCapacity
counter.AllocatedCount += val.AllocatedCount
counter.Capacity += val.Capacity
counter.Count += val.Count
counter.AllocatedCapacity = agonesv1.SafeAdd(counter.AllocatedCapacity, val.AllocatedCapacity)
counter.AllocatedCount = agonesv1.SafeAdd(counter.AllocatedCount, val.AllocatedCount)
counter.Capacity = agonesv1.SafeAdd(counter.Capacity, val.Capacity)
counter.Count = agonesv1.SafeAdd(counter.Count, val.Count)
c1[key] = counter
} else {
c1[key] = *val.DeepCopy()
Expand Down
16 changes: 8 additions & 8 deletions pkg/fleets/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,10 +704,10 @@ func TestControllerUpdateFleetCounterStatus(t *testing.T) {
gsSet1.ObjectMeta.Name = "gsSet1"
gsSet1.Status.Counters = map[string]agonesv1.AggregatedCounterStatus{
"fullCounter": {
AllocatedCount: 1000,
AllocatedCapacity: 1000,
Capacity: 1000,
Count: 1000,
AllocatedCount: 9223372036854775807,
AllocatedCapacity: 9223372036854775807,
Capacity: 9223372036854775807,
Count: 9223372036854775807,
},
"anotherCounter": {
AllocatedCount: 11,
Expand Down Expand Up @@ -752,10 +752,10 @@ func TestControllerUpdateFleetCounterStatus(t *testing.T) {
ua := action.(k8stesting.UpdateAction)
fleet := ua.GetObject().(*agonesv1.Fleet)

assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].AllocatedCount)
assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].AllocatedCapacity)
assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].Capacity)
assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].Count)
assert.Equal(t, int64(9223372036854775807), fleet.Status.Counters["fullCounter"].AllocatedCount)
assert.Equal(t, int64(9223372036854775807), fleet.Status.Counters["fullCounter"].AllocatedCapacity)
assert.Equal(t, int64(9223372036854775807), fleet.Status.Counters["fullCounter"].Capacity)
assert.Equal(t, int64(9223372036854775807), fleet.Status.Counters["fullCounter"].Count)

assert.Equal(t, int64(11), fleet.Status.Counters["anotherCounter"].AllocatedCount)
assert.Equal(t, int64(20), fleet.Status.Counters["anotherCounter"].AllocatedCapacity)
Expand Down
9 changes: 5 additions & 4 deletions pkg/gameserversets/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,12 +679,13 @@ func aggregateCounters(aggCounterStatus map[string]agonesv1.AggregatedCounterSta
// If the Counter exists in both maps, aggregate the values.
if counter, ok := aggCounterStatus[key]; ok {
// Aggregate for all game server statuses (expected IsBeingDeleted)
counter.Count += val.Count
counter.Capacity += val.Capacity
counter.Count = agonesv1.SafeAdd(counter.Count, val.Count)
counter.Capacity = agonesv1.SafeAdd(counter.Capacity, val.Capacity)

// Aggregate for Allocated game servers only
if gsState == agonesv1.GameServerStateAllocated {
counter.AllocatedCount += val.Count
counter.AllocatedCapacity += val.Capacity
counter.AllocatedCount = agonesv1.SafeAdd(counter.AllocatedCount, val.Count)
counter.AllocatedCapacity = agonesv1.SafeAdd(counter.AllocatedCapacity, val.Capacity)
}
aggCounterStatus[key] = counter
} else {
Expand Down
10 changes: 10 additions & 0 deletions pkg/gameserversets/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,20 +356,24 @@ func TestComputeStatus(t *testing.T) {
gs1.Status.Counters = map[string]agonesv1.CounterStatus{
"firstCounter": {Count: 5, Capacity: 10},
"secondCounter": {Count: 100, Capacity: 1000},
"fullCounter": {Count: 9223372036854775807, Capacity: 9223372036854775807},
}
gs2 := gsWithState(agonesv1.GameServerStateReserved)
gs2.Status.Counters = map[string]agonesv1.CounterStatus{
"firstCounter": {Count: 10, Capacity: 15},
"fullCounter": {Count: 9223372036854775807, Capacity: 9223372036854775807},
}
gs3 := gsWithState(agonesv1.GameServerStateCreating)
gs3.Status.Counters = map[string]agonesv1.CounterStatus{
"firstCounter": {Count: 20, Capacity: 30},
"secondCounter": {Count: 100, Capacity: 1000},
"fullCounter": {Count: 9223372036854775807, Capacity: 9223372036854775807},
}
gs4 := gsWithState(agonesv1.GameServerStateReady)
gs4.Status.Counters = map[string]agonesv1.CounterStatus{
"firstCounter": {Count: 15, Capacity: 30},
"secondCounter": {Count: 20, Capacity: 200},
"fullCounter": {Count: 9223372036854775807, Capacity: 9223372036854775807},
}
list = append(list, gs1, gs2, gs3, gs4)

Expand All @@ -391,6 +395,12 @@ func TestComputeStatus(t *testing.T) {
Count: 220,
Capacity: 2200,
},
"fullCounter": {
AllocatedCount: 9223372036854775807,
AllocatedCapacity: 9223372036854775807,
Count: 9223372036854775807,
Capacity: 9223372036854775807,
},
},
Lists: map[string]agonesv1.AggregatedListStatus{},
}
Expand Down

0 comments on commit 702141c

Please sign in to comment.