Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Commit

Permalink
envoy/cds: add nil check for ConnectionSettings (#4821)
Browse files Browse the repository at this point in the history
With the addition of rate limiting to the
UpstreamTrafficSetting spec, it is possible
for the UpstreamTrafficSetting object to be non
nil while having its ConnectionSettings as nil.
Add nil check so the code doesn't panic when
ConnectionSettings is nil and RateLimit is configured.

Signed-off-by: Shashank Ram <shashr2204@gmail.com>
  • Loading branch information
shashankram committed Jun 15, 2022
1 parent 5ee33f3 commit a5b3716
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/envoy/cds/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ func applyUpstreamTrafficSetting(upstreamTrafficSetting *policyv1alpha1.Upstream
Thresholds: []*xds_cluster.CircuitBreakers_Thresholds{threshold},
}

if upstreamTrafficSetting == nil {
if upstreamTrafficSetting == nil || upstreamTrafficSetting.Spec.ConnectionSettings == nil {
return
}

Expand Down
12 changes: 11 additions & 1 deletion pkg/envoy/cds/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ func TestGetUpstreamServiceCluster(t *testing.T) {
},
},
},
{
name: "Cluster without circuit breaker but with valid UpstreamTrafficSetting should not error/panic",
clusterConfig: trafficpolicy.MeshClusterConfig{
Name: "default/bookstore-v1_14001",
Service: upstreamSvc,
UpstreamTrafficSetting: &policyv1alpha1.UpstreamTrafficSetting{
Spec: policyv1alpha1.UpstreamTrafficSettingSpec{},
},
},
},
}

for _, tc := range testCases {
Expand All @@ -106,7 +116,7 @@ func TestGetUpstreamServiceCluster(t *testing.T) {
assert.Nil(remoteCluster.HealthChecks)
}

if tc.clusterConfig.UpstreamTrafficSetting != nil {
if tc.expectedCircuitBreakerThreshold != nil {
assert.Equal(tc.expectedCircuitBreakerThreshold, remoteCluster.CircuitBreakers)
}
})
Expand Down

0 comments on commit a5b3716

Please sign in to comment.