Test TLS termination, SSL-redirect, Canary, and CORS#330
Test TLS termination, SSL-redirect, Canary, and CORS#330k8s-ci-robot merged 46 commits intokubernetes-sigs:mainfrom
Conversation
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
We need a dummy workload so that ingresses and gateways have some service to send test traffic to. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Kong is both an ingress controller and a GWAPI implementation. Add helpers for both. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
We use port forwarding to reach ingress controllers and gateways in an infrastructure-agnostic way during testing. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
To be able to run multiple test cases concurrently on the same cluster, some resources must be shared. For example, CRDs are cluster-level rather than namespace-scoped. In addition, deploying multiple instances of an ingress controller or GWAPI implementation wastes time and resources even in cases where it's technically possible to do so. To address these concerns, we add a resource manager. This construct allows efficient and thread-safe sharing of resources. A shared resource is created on first usage, returned on subsequent attempts to acquire the resources and cleaned up when no users remain. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Verifiers are used to ensure ingress and GWAPI resources process traffic according to expectations. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
We can't use sudo on CI. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
This allows for more compact test case definitions and fewer indentation levels. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Rather than implementing bespoke retry logic across the code base, implement generic retry helpers and reuse them. This also extracts the retry logic out of verifiers for a better separation of concerns. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Call the binary instead of running Go code directly. This simulates actual customer usage better and works around concurrency issues caused by multiple test cases executing in parallel. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
|
@Stevenjin8: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
johananl
left a comment
There was a problem hiding this comment.
Thanks for expanding the e2e framework! I like many of the changes.
Added some comments. Some trivial, some critical.
| const ( | ||
| gatewayAPIVersion = "v1.4.1" | ||
| gatewayAPIInstallURL = "https://github.com/kubernetes-sigs/gateway-api/releases/download/" + gatewayAPIVersion + "/standard-install.yaml" | ||
| // gatewayAPIVersion = "v1.4.1" |
There was a problem hiding this comment.
not really. I think we want to target gwapi 1.5, which isn't released yet (rc coming this week) because we really want support for status code 308 in ReqestRedirects. Since 308 isn't available in 1.4, but is in 1.5, I figured I'd use a monthly release and just update it to 1.5/1.5-rc whenever that comes out.
| networkingv1 "k8s.io/api/networking/v1" | ||
| ) | ||
|
|
||
| func TestCanary(t *testing.T) { |
There was a problem hiding this comment.
I think we might want to include "nginx" in the function name. Right now we see the following when the tests are run:
TestCanary/to_Istio/base_canary
This doesn't tell us which provider gets tested.
My initial thought with the naming was to organize the hierarchy by "from", "to" and "test case". This way you get e.g. TestIngressNginx/to_Istio/with_host_field. It's clear what exactly is being tested.
We should also think about filtering tests when choosing the naming, e.g. maybe I want to do "run all tests to Istio" or "run basic conversions for all providers".
I'm happy to change the naming scheme but IMO we want to be deliberate about it.
WDYT?
There was a problem hiding this comment.
+1. I can either throw IngressNGINX into the test name, or I can make each of these functions called in scenarios_test.go. Up to you
There was a problem hiding this comment.
or I can make each of these functions called in scenarios_test.go
I don't understand this ☝️
I don't have a strong opinion on the naming, just wanted to make sure you're aware of my attempt to group things in a logical way.
|
|
||
| // A prefix for all namespaces used in the e2e tests. | ||
| const e2ePrefix = "i2gw" | ||
| const DummyAppName1 = "dummy-app1" |
There was a problem hiding this comment.
Are we assuming many test cases would need two dummy apps? I'm wondering if we need to define these consts and do so here rather than defining plain text directly in the test cases.
There was a problem hiding this comment.
(Most of what I'm doing is based on what istio does). I think its common enough to do it, but definitely worth thinking about if we are going to test timeouts or something
| networkingv1 "k8s.io/api/networking/v1" | ||
| ) | ||
|
|
||
| // Validates that a service is accessible and working correctly. The addr parameter is a |
There was a problem hiding this comment.
IMO we want some documentation here, especially if we have many types of verifiers (which we do).
| return fmt.Errorf("constructing HTTP request: %w", err) | ||
| } | ||
|
|
||
| // If the Host field is specified in the test case, use that. Otherwise, default to deriving |
| }) | ||
| suffix, err := randString(6) | ||
| require.NoError(t, err) | ||
| host := "foo.example.com" + suffix |
There was a problem hiding this comment.
I don't know if we should do it in this PR but IMO we want to remove scenarios_test.go and relocate any relevant tests to more specific _test.go files. This file contains some initial smoke tests I've included with the e2e framework. Since we moved to a multi-file setup this file is IMO obsolete.
| if m.negate { | ||
| return !match | ||
| } | ||
| return match |
There was a problem hiding this comment.
Nit: This is logical XOR. We can simplify. But maybe clear is better than clever.
| if m.negate { | |
| return !match | |
| } | |
| return match | |
| return m.pattern.MatchString(s) != m.negate |
| }) | ||
| suffix, err := randString(6) | ||
| require.NoError(t, err) | ||
| host := "foo.example.com" + suffix |
There was a problem hiding this comment.
OK. I don't understand why multiple implementations but let's discuss in the other PR.
|
Thanks! |
…#330) * Add k8s clients Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add logger interface Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add CRD helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add Helm helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add namespace helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add dummy app helpers We need a dummy workload so that ingresses and gateways have some service to send test traffic to. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add helper for installing Istio Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add helpers for Kong Kong is both an ingress controller and a GWAPI implementation. Add helpers for both. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add GWAPI helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add port forwarding helpers We use port forwarding to reach ingress controllers and gateways in an infrastructure-agnostic way during testing. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add ingress-nginx helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add helper for creating ingresses Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add resource manager To be able to run multiple test cases concurrently on the same cluster, some resources must be shared. For example, CRDs are cluster-level rather than namespace-scoped. In addition, deploying multiple instances of an ingress controller or GWAPI implementation wastes time and resources even in cases where it's technically possible to do so. To address these concerns, we add a resource manager. This construct allows efficient and thread-safe sharing of resources. A shared resource is created on first usage, returned on subsequent attempts to acquire the resources and cleaned up when no users remain. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add verifier interface and httpGetVerifier implementation Verifiers are used to ensure ingress and GWAPI resources process traffic according to expectations. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add main test logic Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add some initial test cases Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add Make targets for e2e Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add README for e2e tests Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Install kind to ./bin We can't use sudo on CI. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Use builder pattern for ingresses in test cases This allows for more compact test case definitions and fewer indentation levels. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Centralize retry logic Rather than implementing bespoke retry logic across the code base, implement generic retry helpers and reuse them. This also extracts the retry logic out of verifiers for a better separation of concerns. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Use ingress2gateway binary Call the binary instead of running Go code directly. This simulates actual customer usage better and works around concurrency issues caused by multiple test cases executing in parallel. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Canary tests * Path rewrite * Verify TLS Ingresses and TLS redirects * touchup * touchup * lint * lint * use regex body verifier * random canary host * CORS tests * move to module * Use monthly CRD and move packages * Review * lints * review * lint * remove unecessary host * comments --------- Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> Co-authored-by: Johanan Liebermann <jliebermann@microsoft.com>
…#330) * Add k8s clients Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add logger interface Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add CRD helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add Helm helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add namespace helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add dummy app helpers We need a dummy workload so that ingresses and gateways have some service to send test traffic to. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add helper for installing Istio Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add helpers for Kong Kong is both an ingress controller and a GWAPI implementation. Add helpers for both. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add GWAPI helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add port forwarding helpers We use port forwarding to reach ingress controllers and gateways in an infrastructure-agnostic way during testing. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add ingress-nginx helpers Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add helper for creating ingresses Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add resource manager To be able to run multiple test cases concurrently on the same cluster, some resources must be shared. For example, CRDs are cluster-level rather than namespace-scoped. In addition, deploying multiple instances of an ingress controller or GWAPI implementation wastes time and resources even in cases where it's technically possible to do so. To address these concerns, we add a resource manager. This construct allows efficient and thread-safe sharing of resources. A shared resource is created on first usage, returned on subsequent attempts to acquire the resources and cleaned up when no users remain. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add verifier interface and httpGetVerifier implementation Verifiers are used to ensure ingress and GWAPI resources process traffic according to expectations. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add main test logic Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add some initial test cases Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add Make targets for e2e Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Add README for e2e tests Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Install kind to ./bin We can't use sudo on CI. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Use builder pattern for ingresses in test cases This allows for more compact test case definitions and fewer indentation levels. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Centralize retry logic Rather than implementing bespoke retry logic across the code base, implement generic retry helpers and reuse them. This also extracts the retry logic out of verifiers for a better separation of concerns. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Use ingress2gateway binary Call the binary instead of running Go code directly. This simulates actual customer usage better and works around concurrency issues caused by multiple test cases executing in parallel. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Canary tests * Path rewrite * Verify TLS Ingresses and TLS redirects * touchup * touchup * lint * lint * use regex body verifier * random canary host * CORS tests * move to module * Use monthly CRD and move packages * Review * lints * review * lint * remove unecessary host * comments --------- Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> Co-authored-by: Johanan Liebermann <jliebermann@microsoft.com>
What type of PR is this?
/kind test
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: