diff --git a/.gitignore b/.gitignore index e7be6a4f74..5a0ce8cb14 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,5 @@ charts/gateway-addons-helm/dashboards/vendor/ *.tar.gz *.yaml-e + +*.out diff --git a/test/e2e/tests/local_ratelimit.go b/test/e2e/tests/local_ratelimit.go index 18fa6e4ab5..a077e8aa9e 100644 --- a/test/e2e/tests/local_ratelimit.go +++ b/test/e2e/tests/local_ratelimit.go @@ -30,8 +30,15 @@ func init() { const ( RatelimitLimitHeaderName = "x-ratelimit-limit" RatelimitRemainingHeaderName = "x-ratelimit-remaining" + RatelimitResetHeaderName = "x-ratelimit-reset" ) +var allRateLimitHeaders = []string{ + RatelimitLimitHeaderName, + RatelimitRemainingHeaderName, + RatelimitResetHeaderName, +} + var LocalRateLimitTest = suite.ConformanceTest{ ShortName: "LocalRateLimit", Description: "Make sure local rate limit work", @@ -39,16 +46,19 @@ var LocalRateLimitTest = suite.ConformanceTest{ Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { for _, disableHeader := range []bool{true, false} { runNoRateLimitTest(t, suite, disableHeader) - - t.Run(fmt.Sprintf("SpecificUser-%t", disableHeader), func(t *testing.T) { + caseSuffix := "disableHeader" + if !disableHeader { + caseSuffix = "withHeader" + } + t.Run(fmt.Sprintf("SpecificUser-%s", caseSuffix), func(t *testing.T) { runSpecificUserRateLimitTest(t, suite, disableHeader) }) - t.Run(fmt.Sprintf("AllTraffic-%t", disableHeader), func(t *testing.T) { + t.Run(fmt.Sprintf("AllTraffic-%s", caseSuffix), func(t *testing.T) { runAllTrafficRateLimitTest(t, suite, disableHeader) }) - t.Run(fmt.Sprintf("HeaderInvertMatch-%t", disableHeader), func(t *testing.T) { + t.Run(fmt.Sprintf("HeaderInvertMatch-%s", caseSuffix), func(t *testing.T) { runHeaderInvertMatchRateLimitTest(t, suite, disableHeader) }) } @@ -87,19 +97,19 @@ func runNoRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, disable }, Response: http.Response{ StatusCode: 200, - AbsentHeaders: []string{RatelimitLimitHeaderName, RatelimitRemainingHeaderName}, + AbsentHeaders: allRateLimitHeaders, }, Namespace: ns, } // keep sending requests till get 200 first, that will cost one 200 - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, expectOkResp) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) // send 10+ more total := 10 for total > 0 { // keep sending requests till get 200 first, that will cost one 200 - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, expectOkResp) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) total-- } } @@ -134,10 +144,13 @@ func runSpecificUserRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuit if !disableHeader { okResponse.Response.Headers = map[string]string{ RatelimitLimitHeaderName: "3", - RatelimitRemainingHeaderName: "", // empty string means we don't care about the value + RatelimitRemainingHeaderName: "1", + RatelimitResetHeaderName: "0", } + } else { + okResponse.Response.AbsentHeaders = allRateLimitHeaders } - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, okResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, okResponse) // this request should be limited because the user is john limitResponse := http.ExpectedResponse{ @@ -154,11 +167,13 @@ func runSpecificUserRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuit } if !disableHeader { limitResponse.Response.Headers = map[string]string{ - RatelimitLimitHeaderName: "4", - RatelimitRemainingHeaderName: "0", // at the end the remaining should be 0 + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "0", } + } else { + limitResponse.Response.AbsentHeaders = allRateLimitHeaders } - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, limitResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, limitResponse) // this request should not be limited because the user is not john hit default bucket notJohnResponse := http.ExpectedResponse{ @@ -177,9 +192,12 @@ func runSpecificUserRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuit notJohnResponse.Response.Headers = map[string]string{ RatelimitLimitHeaderName: "10", RatelimitRemainingHeaderName: "2", // there almost 8 requests before reach this + RatelimitResetHeaderName: "0", } + } else { + notJohnResponse.Response.AbsentHeaders = allRateLimitHeaders } - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, notJohnResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, notJohnResponse) // In the end it will hit the limit notJohnLimitResponse := http.ExpectedResponse{ @@ -190,10 +208,6 @@ func runSpecificUserRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuit }, }, Response: http.Response{ - Headers: map[string]string{ - RatelimitLimitHeaderName: "10", - RatelimitRemainingHeaderName: "0", // it will be limited at the end - }, StatusCode: 429, }, Namespace: ns, @@ -203,8 +217,10 @@ func runSpecificUserRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuit RatelimitLimitHeaderName: "10", RatelimitRemainingHeaderName: "0", // it will be limited at the end } + } else { + notJohnLimitResponse.Response.AbsentHeaders = allRateLimitHeaders } - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, notJohnLimitResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, notJohnLimitResponse) } func runAllTrafficRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, disableHeader bool) { @@ -232,11 +248,14 @@ func runAllTrafficRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, if !disableHeader { okResponse.Response.Headers = map[string]string{ RatelimitLimitHeaderName: "3", - RatelimitRemainingHeaderName: "", // empty string means we don't care about the value + RatelimitRemainingHeaderName: "1", + RatelimitResetHeaderName: "0", } + } else { + okResponse.Response.AbsentHeaders = allRateLimitHeaders } // keep sending requests till get 200 first, that will cost one 200 - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, okResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, okResponse) limitResponse := http.ExpectedResponse{ Request: http.Request{ @@ -252,9 +271,11 @@ func runAllTrafficRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, RatelimitLimitHeaderName: "3", RatelimitRemainingHeaderName: "0", // at the end the remaining should be 0 } + } else { + limitResponse.Response.AbsentHeaders = allRateLimitHeaders } // this request should be limited at the end - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, limitResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, limitResponse) } func runHeaderInvertMatchRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, disableHeader bool) { @@ -287,10 +308,11 @@ func runHeaderInvertMatchRateLimitTest(t *testing.T, suite *suite.ConformanceTes if !disableHeader { okResponse.Response.Headers = map[string]string{ RatelimitLimitHeaderName: "3", - RatelimitRemainingHeaderName: "", // empty string means we don't care about the value + RatelimitRemainingHeaderName: "1", + RatelimitResetHeaderName: "0", } } - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, okResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, okResponse) // this request should be limited because the user is one and org is not test and the limit is 3 limitResponse := http.ExpectedResponse{ @@ -302,10 +324,6 @@ func runHeaderInvertMatchRateLimitTest(t *testing.T, suite *suite.ConformanceTes }, }, Response: http.Response{ - Headers: map[string]string{ - RatelimitLimitHeaderName: "3", - RatelimitRemainingHeaderName: "0", // at the end the remaining should be 0 - }, StatusCode: 429, }, Namespace: ns, @@ -313,10 +331,12 @@ func runHeaderInvertMatchRateLimitTest(t *testing.T, suite *suite.ConformanceTes if !disableHeader { limitResponse.Response.Headers = map[string]string{ RatelimitLimitHeaderName: "3", - RatelimitRemainingHeaderName: "0", // empty string means we don't care about the value + RatelimitRemainingHeaderName: "0", } + } else { + limitResponse.Response.AbsentHeaders = allRateLimitHeaders } - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, limitResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, limitResponse) // with test org testOrgResponse := http.ExpectedResponse{ @@ -334,9 +354,11 @@ func runHeaderInvertMatchRateLimitTest(t *testing.T, suite *suite.ConformanceTes } if !disableHeader { testOrgResponse.Response.Headers = map[string]string{ - RatelimitLimitHeaderName: "", // we don't care the real number - RatelimitRemainingHeaderName: "", + RatelimitLimitHeaderName: "4294967295", + RatelimitResetHeaderName: "0", } + } else { + testOrgResponse.Response.AbsentHeaders = allRateLimitHeaders } - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, testOrgResponse) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, testOrgResponse) } diff --git a/test/e2e/tests/local_ratelimit_distinct_cidr.go b/test/e2e/tests/local_ratelimit_distinct_cidr.go index 2b943e5e1d..8f7ab61b06 100644 --- a/test/e2e/tests/local_ratelimit_distinct_cidr.go +++ b/test/e2e/tests/local_ratelimit_distinct_cidr.go @@ -19,7 +19,6 @@ import ( "github.com/envoyproxy/gateway/internal/gatewayapi" "github.com/envoyproxy/gateway/internal/gatewayapi/resource" - "github.com/envoyproxy/gateway/test/e2e/utils" ) func init() { @@ -100,7 +99,7 @@ var LocalRateLimitDistinctCIDRTest = suite.ConformanceTest{ } func testRatelimit(t *testing.T, suite *suite.ConformanceTestSuite, headers map[string]string, ns, gwAddr, path string) { - utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, http.ExpectedResponse{ + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, http.ExpectedResponse{ Request: http.Request{ Path: path, Headers: headers, @@ -115,7 +114,7 @@ func testRatelimit(t *testing.T, suite *suite.ConformanceTestSuite, headers map[ StatusCode: 200, Headers: map[string]string{ RatelimitLimitHeaderName: "3", - RatelimitRemainingHeaderName: "", // empty string means we don't care about the value + RatelimitRemainingHeaderName: "1", }, }, Namespace: ns, diff --git a/test/e2e/utils/http.go b/test/e2e/utils/http.go index 41338f58cd..3158761f10 100644 --- a/test/e2e/utils/http.go +++ b/test/e2e/utils/http.go @@ -8,46 +8,16 @@ package utils import ( - "fmt" - "strings" "testing" - "time" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" - "sigs.k8s.io/gateway-api/conformance/utils/http" "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" - "sigs.k8s.io/gateway-api/conformance/utils/roundtripper" "sigs.k8s.io/gateway-api/conformance/utils/suite" - "sigs.k8s.io/gateway-api/conformance/utils/tlog" ) -// MakeRequestAndExpectEventuallyConsistentResponse sends a request to the gateway and waits for an eventually consistent response. -// This's a fork of upstream unless https://github.com/kubernetes-sigs/gateway-api/issues/3794 fixed. -func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, suite *suite.ConformanceTestSuite, gwAddr string, expected http.ExpectedResponse) { - t.Helper() - - req := http.MakeRequest(t, &expected, gwAddr, "HTTP", "http") - - http.AwaitConvergence(t, suite.TimeoutConfig.RequiredConsecutiveSuccesses, suite.TimeoutConfig.MaxTimeToConsistency, func(elapsed time.Duration) bool { - cReq, cRes, err := suite.RoundTripper.CaptureRoundTrip(req) - if err != nil { - tlog.Logf(t, "Request failed, not ready yet: %v (after %v)", err.Error(), elapsed) - return false - } - - if err := compareRequest(t, &req, cReq, cRes, expected); err != nil { - tlog.Logf(t, "Response expectation failed for request: %+v not ready yet: %v (after %v)", req, err, elapsed) - return false - } - - return true - }) - tlog.Logf(t, "Request passed") -} - // GatewaysMustBeAccepted waits for the Gateways to be accepted and returns the address of the Gateways. // This is used when a HTTPRoute referenced by multiple Gateways. // Warning: we didn't check the status of HTTPRoute. @@ -83,170 +53,3 @@ func GatewaysMustBeAccepted(t *testing.T, suite *suite.ConformanceTestSuite, gwR return gwAddress } - -func compareRequest(t *testing.T, req *roundtripper.Request, cReq *roundtripper.CapturedRequest, cRes *roundtripper.CapturedResponse, expected http.ExpectedResponse) error { - if roundtripper.IsTimeoutError(cRes.StatusCode) { - if roundtripper.IsTimeoutError(expected.Response.StatusCode) { - return nil - } - } - if expected.Response.StatusCode != cRes.StatusCode { - return fmt.Errorf("expected status code to be %d, got %d", expected.Response.StatusCode, cRes.StatusCode) - } - if cRes.StatusCode == 200 { - // The request expected to arrive at the backend is - // the same as the request made, unless otherwise - // specified. - if expected.ExpectedRequest == nil { - expected.ExpectedRequest = &http.ExpectedRequest{Request: expected.Request} - } - - if expected.ExpectedRequest.Method == "" { - expected.ExpectedRequest.Method = "GET" - } - - if expected.ExpectedRequest.Host != "" && expected.ExpectedRequest.Host != cReq.Host { - return fmt.Errorf("expected host to be %s, got %s", expected.ExpectedRequest.Host, cReq.Host) - } - - if expected.ExpectedRequest.Path != cReq.Path { - return fmt.Errorf("expected path to be %s, got %s", expected.ExpectedRequest.Path, cReq.Path) - } - if expected.ExpectedRequest.Method != cReq.Method { - return fmt.Errorf("expected method to be %s, got %s", expected.ExpectedRequest.Method, cReq.Method) - } - if expected.Namespace != cReq.Namespace { - return fmt.Errorf("expected namespace to be %s, got %s", expected.Namespace, cReq.Namespace) - } - if expected.ExpectedRequest.Headers != nil { - if cReq.Headers == nil { - return fmt.Errorf("no headers captured, expected %v", len(expected.ExpectedRequest.Headers)) - } - for name, val := range cReq.Headers { - cReq.Headers[strings.ToLower(name)] = val - } - for name, expectedVal := range expected.ExpectedRequest.Headers { - actualVal, ok := cReq.Headers[strings.ToLower(name)] - if !ok { - return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cReq.Headers) - } else if strings.Join(actualVal, ",") != expectedVal { - return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, strings.Join(actualVal, ",")) - } - } - } - - if expected.Response.Headers != nil { - if cRes.Headers == nil { - return fmt.Errorf("no headers captured, expected %v", len(expected.ExpectedRequest.Headers)) - } - for name, val := range cRes.Headers { - cRes.Headers[strings.ToLower(name)] = val - } - - for name, expectedVal := range expected.Response.Headers { - actualVal, ok := cRes.Headers[strings.ToLower(name)] - if !ok { - return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cRes.Headers) - } - - if expectedVal == "" { - // If the expected value is empty, we don't care about the actual value. - // This is useful for headers that are set by the backend, and we don't - // care about their values. - continue - } - - if strings.Join(actualVal, ",") != expectedVal { - return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, strings.Join(actualVal, ",")) - } - } - } - - if len(expected.Response.AbsentHeaders) > 0 { - for name, val := range cRes.Headers { - cRes.Headers[strings.ToLower(name)] = val - } - - for _, name := range expected.Response.AbsentHeaders { - val, ok := cRes.Headers[strings.ToLower(name)] - if ok { - return fmt.Errorf("expected %s header to not be set, got %s", name, val) - } - } - } - - // Verify that headers expected *not* to be present on the - // request are actually not present. - if len(expected.ExpectedRequest.AbsentHeaders) > 0 { - for name, val := range cReq.Headers { - cReq.Headers[strings.ToLower(name)] = val - } - - for _, name := range expected.ExpectedRequest.AbsentHeaders { - val, ok := cReq.Headers[strings.ToLower(name)] - if ok { - return fmt.Errorf("expected %s header to not be set, got %s", name, val) - } - } - } - - if !strings.HasPrefix(cReq.Pod, expected.Backend) { - return fmt.Errorf("expected pod name to start with %s, got %s", expected.Backend, cReq.Pod) - } - } else if roundtripper.IsRedirect(cRes.StatusCode) { - if expected.RedirectRequest == nil { - return nil - } - - setRedirectRequestDefaults(req, cRes, &expected) - - if expected.RedirectRequest.Host != cRes.RedirectRequest.Host { - return fmt.Errorf("expected redirected hostname to be %q, got %q", expected.RedirectRequest.Host, cRes.RedirectRequest.Host) - } - - gotPort := cRes.RedirectRequest.Port - if expected.RedirectRequest.Port == "" { - // If the test didn't specify any expected redirect port, we'll try to use - // the scheme to determine sensible defaults for the port. Well known - // schemes like "http" and "https" MAY skip setting any port. - if strings.ToLower(cRes.RedirectRequest.Scheme) == "http" && gotPort != "80" && gotPort != "" { - return fmt.Errorf("for http scheme, expected redirected port to be 80 or not set, got %q", gotPort) - } - if strings.ToLower(cRes.RedirectRequest.Scheme) == "https" && gotPort != "443" && gotPort != "" { - return fmt.Errorf("for https scheme, expected redirected port to be 443 or not set, got %q", gotPort) - } - if strings.ToLower(cRes.RedirectRequest.Scheme) != "http" || strings.ToLower(cRes.RedirectRequest.Scheme) != "https" { - tlog.Logf(t, "Can't validate redirectPort for unrecognized scheme %v", cRes.RedirectRequest.Scheme) - } - } else if expected.RedirectRequest.Port != gotPort { - // An expected port was specified in the tests but it didn't match with - // gotPort. - return fmt.Errorf("expected redirected port to be %q, got %q", expected.RedirectRequest.Port, gotPort) - } - - if expected.RedirectRequest.Scheme != cRes.RedirectRequest.Scheme { - return fmt.Errorf("expected redirected scheme to be %q, got %q", expected.RedirectRequest.Scheme, cRes.RedirectRequest.Scheme) - } - - if expected.RedirectRequest.Path != cRes.RedirectRequest.Path { - return fmt.Errorf("expected redirected path to be %q, got %q", expected.RedirectRequest.Path, cRes.RedirectRequest.Path) - } - } - return nil -} - -func setRedirectRequestDefaults(req *roundtripper.Request, cRes *roundtripper.CapturedResponse, expected *http.ExpectedResponse) { - // If the expected host is nil it means we do not test host redirect. - // In that case we are setting it to the one we got from the response because we do not know the ip/host of the gateway. - if expected.RedirectRequest.Host == "" { - expected.RedirectRequest.Host = cRes.RedirectRequest.Host - } - - if expected.RedirectRequest.Scheme == "" { - expected.RedirectRequest.Scheme = req.URL.Scheme - } - - if expected.RedirectRequest.Path == "" { - expected.RedirectRequest.Path = req.URL.Path - } -} diff --git a/tools/make/kube.mk b/tools/make/kube.mk index 2ecf50f6f5..d9ba808457 100644 --- a/tools/make/kube.mk +++ b/tools/make/kube.mk @@ -28,6 +28,8 @@ BENCHMARK_REPORT_DIR ?= benchmark_report E2E_RUN_TEST ?= E2E_CLEANUP ?= true E2E_TIMEOUT ?= 20m +# E2E_REDIRECT allow you specified a redirect when run e2e test locally, e.g. `>> test_output.out 2>&1` +E2E_REDIRECT ?= E2E_TEST_ARGS ?= -v -tags e2e -timeout $(E2E_TIMEOUT) KUBE_DEPLOY_PROFILE ?= default @@ -215,13 +217,13 @@ e2e-prepare: prepare-ip-family ## Prepare the environment for running e2e tests run-e2e: ## Run e2e tests @$(LOG_TARGET) ifeq ($(E2E_RUN_TEST),) - go test $(E2E_TEST_ARGS) ./test/e2e --gateway-class=envoy-gateway --debug=true --cleanup-base-resources=false + go test $(E2E_TEST_ARGS) ./test/e2e --gateway-class=envoy-gateway --debug=true --cleanup-base-resources=false $(E2E_REDIRECT) go test $(E2E_TEST_ARGS) ./test/e2e/merge_gateways --gateway-class=merge-gateways --debug=true --cleanup-base-resources=false go test $(E2E_TEST_ARGS) ./test/e2e/multiple_gc --debug=true --cleanup-base-resources=true LAST_VERSION_TAG=$(shell cat VERSION) go test $(E2E_TEST_ARGS) ./test/e2e/upgrade --gateway-class=upgrade --debug=true --cleanup-base-resources=$(E2E_CLEANUP) else go test $(E2E_TEST_ARGS) ./test/e2e --gateway-class=envoy-gateway --debug=true --cleanup-base-resources=$(E2E_CLEANUP) \ - --run-test $(E2E_RUN_TEST) + --run-test $(E2E_RUN_TEST) $(E2E_REDIRECT) endif run-e2e-upgrade: