Skip to content

Commit

Permalink
NET-5115 Add retry + timeout filters for api-gateway (#18324)
Browse files Browse the repository at this point in the history
* squash, implement retry/timeout in consul core

* update tests
  • Loading branch information
sarahalsmiller authored Aug 8, 2023
1 parent bfc519f commit e235c8b
Show file tree
Hide file tree
Showing 17 changed files with 1,363 additions and 613 deletions.
3 changes: 3 additions & 0 deletions .changelog/18324.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
api-gateway: add retry and timeout filters
```
23 changes: 23 additions & 0 deletions agent/consul/discoverychain/gateway_httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,28 @@ func httpRouteToDiscoveryChain(route structs.HTTPRouteConfigEntry) (*structs.Ser
}
}

if rule.Filters.RetryFilter != nil {
if rule.Filters.RetryFilter.NumRetries != nil {
destination.NumRetries = *rule.Filters.RetryFilter.NumRetries
}
if rule.Filters.RetryFilter.RetryOnConnectFailure != nil {
destination.RetryOnConnectFailure = *rule.Filters.RetryFilter.RetryOnConnectFailure
}

if len(rule.Filters.RetryFilter.RetryOn) > 0 {
destination.RetryOn = rule.Filters.RetryFilter.RetryOn
}

if len(rule.Filters.RetryFilter.RetryOnStatusCodes) > 0 {
destination.RetryOnStatusCodes = rule.Filters.RetryFilter.RetryOnStatusCodes
}
}

if rule.Filters.TimeoutFilter != nil {
destination.IdleTimeout = rule.Filters.TimeoutFilter.IdleTimeout
destination.RequestTimeout = rule.Filters.TimeoutFilter.RequestTimeout
}

// for each match rule a ServiceRoute is created for the service-router
// if there are no rules a single route with the destination is set
if len(rule.Matches) == 0 {
Expand All @@ -173,6 +195,7 @@ func httpRouteToDiscoveryChain(route structs.HTTPRouteConfigEntry) (*structs.Ser
Destination: &destination,
})
}

}

return router, splitters, defaults
Expand Down
19 changes: 17 additions & 2 deletions agent/structs/config_entry_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"strings"
"time"

"github.com/miekg/dns"

Expand Down Expand Up @@ -417,8 +418,10 @@ type HTTPQueryMatch struct {
// HTTPFilters specifies a list of filters used to modify a request
// before it is routed to an upstream.
type HTTPFilters struct {
Headers []HTTPHeaderFilter
URLRewrite *URLRewrite
Headers []HTTPHeaderFilter
URLRewrite *URLRewrite
RetryFilter *RetryFilter
TimeoutFilter *TimeoutFilter
}

// HTTPHeaderFilter specifies how HTTP headers should be modified.
Expand All @@ -432,6 +435,18 @@ type URLRewrite struct {
Path string
}

type RetryFilter struct {
NumRetries *uint32
RetryOn []string
RetryOnStatusCodes []uint32
RetryOnConnectFailure *bool
}

type TimeoutFilter struct {
RequestTimeout time.Duration
IdleTimeout time.Duration
}

// HTTPRouteRule specifies the routing rules used to determine what upstream
// service an HTTP request is routed to.
type HTTPRouteRule struct {
Expand Down
48 changes: 48 additions & 0 deletions agent/structs/structs.deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,30 @@ func (o *HTTPRouteConfigEntry) DeepCopy() *HTTPRouteConfigEntry {
cp.Rules[i2].Filters.URLRewrite = new(URLRewrite)
*cp.Rules[i2].Filters.URLRewrite = *o.Rules[i2].Filters.URLRewrite
}
if o.Rules[i2].Filters.RetryFilter != nil {
cp.Rules[i2].Filters.RetryFilter = new(RetryFilter)
*cp.Rules[i2].Filters.RetryFilter = *o.Rules[i2].Filters.RetryFilter
if o.Rules[i2].Filters.RetryFilter.NumRetries != nil {
cp.Rules[i2].Filters.RetryFilter.NumRetries = new(uint32)
*cp.Rules[i2].Filters.RetryFilter.NumRetries = *o.Rules[i2].Filters.RetryFilter.NumRetries
}
if o.Rules[i2].Filters.RetryFilter.RetryOn != nil {
cp.Rules[i2].Filters.RetryFilter.RetryOn = make([]string, len(o.Rules[i2].Filters.RetryFilter.RetryOn))
copy(cp.Rules[i2].Filters.RetryFilter.RetryOn, o.Rules[i2].Filters.RetryFilter.RetryOn)
}
if o.Rules[i2].Filters.RetryFilter.RetryOnStatusCodes != nil {
cp.Rules[i2].Filters.RetryFilter.RetryOnStatusCodes = make([]uint32, len(o.Rules[i2].Filters.RetryFilter.RetryOnStatusCodes))
copy(cp.Rules[i2].Filters.RetryFilter.RetryOnStatusCodes, o.Rules[i2].Filters.RetryFilter.RetryOnStatusCodes)
}
if o.Rules[i2].Filters.RetryFilter.RetryOnConnectFailure != nil {
cp.Rules[i2].Filters.RetryFilter.RetryOnConnectFailure = new(bool)
*cp.Rules[i2].Filters.RetryFilter.RetryOnConnectFailure = *o.Rules[i2].Filters.RetryFilter.RetryOnConnectFailure
}
}
if o.Rules[i2].Filters.TimeoutFilter != nil {
cp.Rules[i2].Filters.TimeoutFilter = new(TimeoutFilter)
*cp.Rules[i2].Filters.TimeoutFilter = *o.Rules[i2].Filters.TimeoutFilter
}
if o.Rules[i2].Matches != nil {
cp.Rules[i2].Matches = make([]HTTPMatch, len(o.Rules[i2].Matches))
copy(cp.Rules[i2].Matches, o.Rules[i2].Matches)
Expand Down Expand Up @@ -427,6 +451,30 @@ func (o *HTTPRouteConfigEntry) DeepCopy() *HTTPRouteConfigEntry {
cp.Rules[i2].Services[i4].Filters.URLRewrite = new(URLRewrite)
*cp.Rules[i2].Services[i4].Filters.URLRewrite = *o.Rules[i2].Services[i4].Filters.URLRewrite
}
if o.Rules[i2].Services[i4].Filters.RetryFilter != nil {
cp.Rules[i2].Services[i4].Filters.RetryFilter = new(RetryFilter)
*cp.Rules[i2].Services[i4].Filters.RetryFilter = *o.Rules[i2].Services[i4].Filters.RetryFilter
if o.Rules[i2].Services[i4].Filters.RetryFilter.NumRetries != nil {
cp.Rules[i2].Services[i4].Filters.RetryFilter.NumRetries = new(uint32)
*cp.Rules[i2].Services[i4].Filters.RetryFilter.NumRetries = *o.Rules[i2].Services[i4].Filters.RetryFilter.NumRetries
}
if o.Rules[i2].Services[i4].Filters.RetryFilter.RetryOn != nil {
cp.Rules[i2].Services[i4].Filters.RetryFilter.RetryOn = make([]string, len(o.Rules[i2].Services[i4].Filters.RetryFilter.RetryOn))
copy(cp.Rules[i2].Services[i4].Filters.RetryFilter.RetryOn, o.Rules[i2].Services[i4].Filters.RetryFilter.RetryOn)
}
if o.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnStatusCodes != nil {
cp.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnStatusCodes = make([]uint32, len(o.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnStatusCodes))
copy(cp.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnStatusCodes, o.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnStatusCodes)
}
if o.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnConnectFailure != nil {
cp.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnConnectFailure = new(bool)
*cp.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnConnectFailure = *o.Rules[i2].Services[i4].Filters.RetryFilter.RetryOnConnectFailure
}
}
if o.Rules[i2].Services[i4].Filters.TimeoutFilter != nil {
cp.Rules[i2].Services[i4].Filters.TimeoutFilter = new(TimeoutFilter)
*cp.Rules[i2].Services[i4].Filters.TimeoutFilter = *o.Rules[i2].Services[i4].Filters.TimeoutFilter
}
}
}
}
Expand Down
86 changes: 86 additions & 0 deletions agent/xds/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import (
"path/filepath"
"sort"
"testing"
"time"

envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
envoy_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
envoy_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
envoy_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
"k8s.io/utils/pointer"

"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/agent/consul/discoverychain"
Expand Down Expand Up @@ -567,6 +569,90 @@ func getAPIGatewayGoldenTestCases(t *testing.T) []goldenTestCase {
Remove: []string{"X-Header-Remove"},
},
},
RetryFilter: &structs.RetryFilter{
NumRetries: pointer.Uint32(3),
RetryOn: []string{"cancelled"},
RetryOnStatusCodes: []uint32{500},
RetryOnConnectFailure: pointer.Bool(true),
},
TimeoutFilter: &structs.TimeoutFilter{
IdleTimeout: time.Second * 30,
RequestTimeout: time.Second * 30,
},
},
Services: []structs.HTTPService{{
Name: "service",
}},
}},
Parents: []structs.ResourceReference{
{
Kind: structs.APIGateway,
Name: "api-gateway",
},
},
},
}, []structs.InlineCertificateConfigEntry{{
Kind: structs.InlineCertificate,
Name: "certificate",
PrivateKey: gatewayTestPrivateKey,
Certificate: gatewayTestCertificate,
}}, []proxycfg.UpdateEvent{{
CorrelationID: "discovery-chain:" + serviceUID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: serviceChain,
},
}, {
CorrelationID: "upstream-target:" + serviceChain.ID() + ":" + serviceUID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: proxycfg.TestUpstreamNodes(t, "service"),
},
}})
},
},
{
name: "api-gateway-with-http-route-timeoutfilter-one-set",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotAPIGateway(t, "default", nil, func(entry *structs.APIGatewayConfigEntry, bound *structs.BoundAPIGatewayConfigEntry) {
entry.Listeners = []structs.APIGatewayListener{
{
Name: "listener",
Protocol: structs.ListenerProtocolHTTP,
Port: 8080,
},
}
bound.Listeners = []structs.BoundAPIGatewayListener{
{
Name: "listener",
Certificates: []structs.ResourceReference{{
Kind: structs.InlineCertificate,
Name: "certificate",
}},
Routes: []structs.ResourceReference{{
Kind: structs.HTTPRoute,
Name: "route",
}},
},
}
}, []structs.BoundRoute{
&structs.HTTPRouteConfigEntry{
Kind: structs.HTTPRoute,
Name: "route",
Rules: []structs.HTTPRouteRule{{
Filters: structs.HTTPFilters{
Headers: []structs.HTTPHeaderFilter{
{
Add: map[string]string{
"X-Header-Add": "added",
},
Set: map[string]string{
"X-Header-Set": "set",
},
Remove: []string{"X-Header-Remove"},
},
},
TimeoutFilter: &structs.TimeoutFilter{
IdleTimeout: time.Second * 30,
},
},
Services: []structs.HTTPService{{
Name: "service",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
{
"versionInfo": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster",
"name": "service.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"altStatName": "service.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"type": "EDS",
"edsClusterConfig": {
"edsConfig": {
"ads": {},
"resourceApiVersion": "V3"
}
},
"connectTimeout": "5s",
"circuitBreakers": {},
"outlierDetection": {},
"commonLbConfig": {
"healthyPanicThreshold": {}
},
"transportSocket": {
"name": "tls",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext",
"commonTlsContext": {
"tlsParams": {},
"tlsCertificates": [
{
"certificateChain": {
"inlineString": "-----BEGIN CERTIFICATE-----\nMIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG\nA1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR\nAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7\nSkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD\nAgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6\nNDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6\nNWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf\nZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6\nZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw\nWQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1\nNTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG\nSM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA\npY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g=\n-----END CERTIFICATE-----\n"
},
"privateKey": {
"inlineString": "-----BEGIN EC PRIVATE KEY-----\nMHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49\nAwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav\nq5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ==\n-----END EC PRIVATE KEY-----\n"
}
}
],
"validationContext": {
"trustedCa": {
"inlineString": "-----BEGIN CERTIFICATE-----\nMIICXDCCAgKgAwIBAgIICpZq70Z9LyUwCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowFDESMBAG\nA1UEAxMJVGVzdCBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIhywH1gx\nAsMwuF3ukAI5YL2jFxH6Usnma1HFSfVyxbXX1/uoZEYrj8yCAtdU2yoHETyd+Zx2\nThhRLP79pYegCaOCATwwggE4MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTAD\nAQH/MGgGA1UdDgRhBF9kMToxMToxMTphYzoyYTpiYTo5NzpiMjozZjphYzo3Yjpi\nZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1ZTo0MTo2ZjpmMjo3\nMzo5NTo1ODowYzpkYjBqBgNVHSMEYzBhgF9kMToxMToxMTphYzoyYTpiYTo5Nzpi\nMjozZjphYzo3YjpiZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1\nZTo0MTo2ZjpmMjo3Mzo5NTo1ODowYzpkYjA/BgNVHREEODA2hjRzcGlmZmU6Ly8x\nMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqG\nSM49BAMCA0gAMEUCICOY0i246rQHJt8o8Oya0D5PLL1FnmsQmQqIGCi31RwnAiEA\noR5f6Ku+cig2Il8T8LJujOp2/2A72QcHZA57B13y+8o=\n-----END CERTIFICATE-----\n"
},
"matchSubjectAltNames": [
{
"exact": "spiffe://11111111-2222-3333-4444-555555555555.consul/ns/default/dc/dc1/svc/service"
}
]
}
},
"sni": "service.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
}
}
}
],
"typeUrl": "type.googleapis.com/envoy.config.cluster.v3.Cluster",
"nonce": "00000001"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"versionInfo": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"clusterName": "service.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "10.10.1.1",
"portValue": 8080
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
},
{
"endpoint": {
"address": {
"socketAddress": {
"address": "10.10.1.2",
"portValue": 8080
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
}
]
}
]
}
],
"typeUrl": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"nonce": "00000001"
}
Loading

0 comments on commit e235c8b

Please sign in to comment.