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.

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

fix: rate limit doesn't work with two(and more) listeners #3085

Merged
merged 5 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 13 additions & 2 deletions internal/xds/translator/ratelimit.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,16 @@ func GetRateLimitServiceConfigStr(pbCfg *rlsconfv3.RateLimitConfig) (string, err
enc.SetIndent(2)
// Translate pb config to yaml
yamlRoot := config.ConfigXdsProtoToYaml(pbCfg)
err := enc.Encode(*yamlRoot)
rateLimitConfig := &struct {
Name string
Domain string
Descriptors []config.YamlDescriptor
}{
Name: pbCfg.Name,
Domain: yamlRoot.Domain,
Descriptors: yamlRoot.Descriptors,
}
err := enc.Encode(rateLimitConfig)
return buf.String(), err
}

Expand Down Expand Up @@ -306,8 +315,10 @@ func BuildRateLimitServiceConfig(irListener *ir.HTTPListener) *rlsconfv3.RateLim
return nil
}

domain := getRateLimitDomain(irListener)
return &rlsconfv3.RateLimitConfig{
Domain: getRateLimitDomain(irListener),
Name: domain,
Domain: domain,
Descriptors: pbDescriptors,
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: first-listener
domain: first-listener
descriptors:
- key: first-route
Expand Down
54 changes: 54 additions & 0 deletions test/e2e/testdata/ratelimit-multiple-listeners.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
name: eg-rate-limit
namespace: gateway-conformance-infra
spec:
gatewayClassName: "{GATEWAY_CLASS_NAME}"
listeners:
- name: http-80
protocol: HTTP
port: 80
- name: http-8080
protocol: HTTP
port: 8080
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
name: cidr-ratelimit
namespace: gateway-conformance-infra
spec:
parentRefs:
- name: eg-rate-limit
rules:
- matches:
- path:
type: PathPrefix
value: /
backendRefs:
- name: infra-backend-v1
port: 8080
---
apiVersion: gateway.envoyproxy.io/v1alpha1
kind: BackendTrafficPolicy
metadata:
name: ratelimit-all-ips
namespace: gateway-conformance-infra
spec:
targetRef:
group: gateway.networking.k8s.io
kind: HTTPRoute
name: cidr-ratelimit
namespace: gateway-conformance-infra
rateLimit:
type: Global
global:
rules:
- clientSelectors:
- sourceCIDR:
value: 0.0.0.0/0
type: distinct
limit:
requests: 3
unit: Hour
63 changes: 63 additions & 0 deletions test/e2e/tests/ratelimit.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
package tests

import (
"fmt"
"net"
"testing"

"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/gateway-api/conformance/utils/http"
"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
Expand All @@ -22,6 +25,7 @@ func init() {
ConformanceTests = append(ConformanceTests, RateLimitCIDRMatchTest)
ConformanceTests = append(ConformanceTests, RateLimitHeaderMatchTest)
ConformanceTests = append(ConformanceTests, RateLimitBasedJwtClaimsTest)
ConformanceTests = append(ConformanceTests, RateLimitMultipleListenersTest)
}

var RateLimitCIDRMatchTest = suite.ConformanceTest{
Expand Down Expand Up @@ -285,6 +289,65 @@ var RateLimitBasedJwtClaimsTest = suite.ConformanceTest{
},
}

var RateLimitMultipleListenersTest = suite.ConformanceTest{
ShortName: "RateLimitMultipleListeners",
Description: "Limit requests on multiple listeners",
Manifests: []string{"testdata/ratelimit-multiple-listeners.yaml"},
Test: func(t *testing.T, suite *suite.ConformanceTestSuite) {
t.Run("block all ips on listener 80 and 8080", func(t *testing.T) {
ns := "gateway-conformance-infra"
routeNN := types.NamespacedName{Name: "cidr-ratelimit", Namespace: ns}
gwNN := types.NamespacedName{Name: "eg-rate-limit", Namespace: ns}
gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN)
gwIP, _, err := net.SplitHostPort(gwAddr)
require.NoError(t, err)

gwPorts := []string{"80", "8080"}
for _, port := range gwPorts {
gwAddr = fmt.Sprintf("%s:%s", gwIP, port)

ratelimitHeader := make(map[string]string)
expectOkResp := http.ExpectedResponse{
Request: http.Request{
Path: "/",
},
Response: http.Response{
StatusCode: 200,
Headers: ratelimitHeader,
},
Namespace: ns,
}
expectOkResp.Response.Headers["X-Ratelimit-Limit"] = "3, 3;w=3600"
expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http")

expectLimitResp := http.ExpectedResponse{
Request: http.Request{
Path: "/",
},
Response: http.Response{
StatusCode: 429,
},
Namespace: ns,
}
expectLimitReq := http.MakeRequest(t, &expectLimitResp, gwAddr, "HTTP", "http")

// should just send exactly 4 requests, and expect 429

// keep sending requests till get 200 first, that will cost one 200
http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp)

// fire the rest of requests
if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil {
t.Errorf("failed to get expected response for the first three requests: %v", err)
}
if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil {
t.Errorf("failed to get expected response for the last (fourth) request: %v", err)
}
}
})
},
}

func GotExactExpectedResponse(t *testing.T, n int, r roundtripper.RoundTripper, req roundtripper.Request, resp http.ExpectedResponse) error {
for i := 0; i < n; i++ {
cReq, cRes, err := r.CaptureRoundTrip(req)
Expand Down
Loading