Skip to content

Commit 34e1e9e

Browse files
Backport of Zero value check for api gateway limits into release/2.0.x (#23556)
backport of commit 744998d Co-authored-by: LakshmiNarayananDesikan <lakshminarayanan.desikan@hashicorp.com>
1 parent 0b5009d commit 34e1e9e

2 files changed

Lines changed: 69 additions & 12 deletions

File tree

agent/xds/clusters.go

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,13 +1097,10 @@ func mergedAPIGatewayUpstreamConfig(configMap map[string]interface{}, limits *st
10971097

10981098
func mergeAPIGatewayUpstreamLimits(existing, incoming *structs.UpstreamLimits) *structs.UpstreamLimits {
10991099
if existing == nil {
1100-
if incoming == nil {
1101-
return nil
1102-
}
1103-
return incoming.Clone()
1100+
return normalizeAPIGatewayUpstreamLimits(incoming)
11041101
}
11051102
if incoming == nil {
1106-
return existing.Clone()
1103+
return normalizeAPIGatewayUpstreamLimits(existing)
11071104
}
11081105

11091106
return &structs.UpstreamLimits{
@@ -1115,13 +1112,10 @@ func mergeAPIGatewayUpstreamLimits(existing, incoming *structs.UpstreamLimits) *
11151112

11161113
func mergeAPIGatewayLimitValue(existing, incoming *int) *int {
11171114
if existing == nil {
1118-
if incoming == nil {
1119-
return nil
1120-
}
1121-
return intPointerCopy(incoming)
1115+
return positiveIntPointerCopy(incoming)
11221116
}
11231117
if incoming == nil {
1124-
return intPointerCopy(existing)
1118+
return positiveIntPointerCopy(existing)
11251119
}
11261120

11271121
e := *existing
@@ -1141,8 +1135,7 @@ func mergeAPIGatewayLimitValue(existing, incoming *int) *int {
11411135
return intPointerCopy(incoming)
11421136
}
11431137

1144-
zero := 0
1145-
return &zero
1138+
return nil
11461139
}
11471140

11481141
func intPointerCopy(v *int) *int {
@@ -1153,6 +1146,25 @@ func intPointerCopy(v *int) *int {
11531146
return &v2
11541147
}
11551148

1149+
func positiveIntPointerCopy(v *int) *int {
1150+
if v == nil || *v <= 0 {
1151+
return nil
1152+
}
1153+
return intPointerCopy(v)
1154+
}
1155+
1156+
func normalizeAPIGatewayUpstreamLimits(limits *structs.UpstreamLimits) *structs.UpstreamLimits {
1157+
if limits == nil {
1158+
return nil
1159+
}
1160+
1161+
return &structs.UpstreamLimits{
1162+
MaxConnections: positiveIntPointerCopy(limits.MaxConnections),
1163+
MaxPendingRequests: positiveIntPointerCopy(limits.MaxPendingRequests),
1164+
MaxConcurrentRequests: positiveIntPointerCopy(limits.MaxConcurrentRequests),
1165+
}
1166+
}
1167+
11561168
func (s *ResourceGenerator) configIngressUpstreamCluster(c *envoy_cluster_v3.Cluster, cfgSnap *proxycfg.ConfigSnapshot, listenerKey proxycfg.IngressListenerKey, u *structs.Upstream) {
11571169
var threshold *envoy_cluster_v3.CircuitBreakers_Thresholds
11581170
setThresholdLimit := func(limitType string, limit int) {
@@ -2351,17 +2363,25 @@ func makeThresholdsIfNeeded(limits *structs.UpstreamLimits) []*envoy_cluster_v3.
23512363
}
23522364

23532365
threshold := &envoy_cluster_v3.CircuitBreakers_Thresholds{}
2366+
hasLimit := false
23542367

23552368
// Likewise, make sure to not set any threshold values on the zero-value in
23562369
// order to rely on Envoy defaults
23572370
if limits.MaxConnections != nil {
23582371
threshold.MaxConnections = response.MakeUint32Value(*limits.MaxConnections)
2372+
hasLimit = true
23592373
}
23602374
if limits.MaxPendingRequests != nil {
23612375
threshold.MaxPendingRequests = response.MakeUint32Value(*limits.MaxPendingRequests)
2376+
hasLimit = true
23622377
}
23632378
if limits.MaxConcurrentRequests != nil {
23642379
threshold.MaxRequests = response.MakeUint32Value(*limits.MaxConcurrentRequests)
2380+
hasLimit = true
2381+
}
2382+
2383+
if !hasLimit {
2384+
return nil
23652385
}
23662386

23672387
return []*envoy_cluster_v3.CircuitBreakers_Thresholds{threshold}

agent/xds/clusters_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,43 @@ func TestMergeAPIGatewayUpstreamLimits(t *testing.T) {
963963
require.Equal(t, 50, *merged.MaxConcurrentRequests)
964964
}
965965

966+
func TestMergeAPIGatewayUpstreamLimits_ZeroValuesOmitted(t *testing.T) {
967+
t.Parallel()
968+
969+
merged := mergeAPIGatewayUpstreamLimits(
970+
&structs.UpstreamLimits{
971+
MaxConnections: intPointer(0),
972+
MaxPendingRequests: intPointer(4),
973+
MaxConcurrentRequests: intPointer(0),
974+
},
975+
&structs.UpstreamLimits{
976+
MaxPendingRequests: intPointer(3),
977+
MaxConcurrentRequests: intPointer(2),
978+
},
979+
)
980+
981+
require.NotNil(t, merged)
982+
require.Nil(t, merged.MaxConnections)
983+
require.NotNil(t, merged.MaxPendingRequests)
984+
require.Equal(t, 3, *merged.MaxPendingRequests)
985+
require.NotNil(t, merged.MaxConcurrentRequests)
986+
require.Equal(t, 2, *merged.MaxConcurrentRequests)
987+
988+
merged = mergeAPIGatewayUpstreamLimits(
989+
&structs.UpstreamLimits{MaxConnections: intPointer(0)},
990+
nil,
991+
)
992+
require.NotNil(t, merged)
993+
require.Nil(t, merged.MaxConnections)
994+
995+
merged = mergeAPIGatewayUpstreamLimits(
996+
nil,
997+
&structs.UpstreamLimits{MaxConnections: intPointer(0)},
998+
)
999+
require.NotNil(t, merged)
1000+
require.Nil(t, merged.MaxConnections)
1001+
}
1002+
9661003
func TestMergedAPIGatewayUpstreamConfig(t *testing.T) {
9671004
t.Parallel()
9681005

0 commit comments

Comments
 (0)