Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NET-5115 Add retry + timeout filters for api-gateway #18324

Merged
merged 2 commits into from
Aug 8, 2023
Merged

NET-5115 Add retry + timeout filters for api-gateway #18324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fdafd35
squash, implement retry/timeout in consul core
sarahalsmiller Aug 2, 2023
17bd17b
update tests
sarahalsmiller Aug 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/18324.txt
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Copy link
Member

@nathancoleman nathancoleman Aug 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we can actually remove RetryOnConnectFailure based on this code + comment pointing out that there are two ways of getting "connect-failure" added to the RetryOn list. The standalone field is referred to as "legacy".

I'm fine leaving for now since our code is already built to handle both, but I'd like to discuss with the team prior to release whether we want to drop the "legacy" option from our API surface.

}

type TimeoutFilter struct {
RequestTimeout time.Duration
IdleTimeout time.Duration
nathancoleman marked this conversation as resolved.
Show resolved Hide resolved
}

// 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
View file
Open in desktop
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
View file
Open in desktop
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
55 changes: 55 additions & 0 deletions agent/xds/testdata/clusters/api-gateway-with-http-route-timeoutfilter-one-set.latest.golden
View file
Open in desktop
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"
}
41 changes: 41 additions & 0 deletions agent/xds/testdata/endpoints/api-gateway-with-http-route-timeoutfilter-one-set.latest.golden
View file
Open in desktop
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