From 6dd21ea1335d4da72ddac1dcfe802bb7282c85eb Mon Sep 17 00:00:00 2001 From: candita Date: Mon, 19 Dec 2022 18:39:20 -0500 Subject: [PATCH] Issue #1579 TLSRoute Passthrough - PR review update - rebase --- conformance/conformance_test.go | 1 + .../tests/tlsroute-simple-same-namespace.go | 17 ++++--- .../tests/tlsroute-simple-same-namespace.yaml | 2 +- conformance/utils/config/timeout.go | 10 ++--- conformance/utils/flags/flags.go | 1 - conformance/utils/http/http.go | 16 +++++-- conformance/utils/kubernetes/helpers.go | 32 ++++++------- .../utils/roundtripper/roundtripper.go | 22 ++++++--- conformance/utils/tls/tls.go | 45 +++---------------- site-src/concepts/conformance.md | 5 +++ 10 files changed, 66 insertions(+), 85 deletions(-) diff --git a/conformance/conformance_test.go b/conformance/conformance_test.go index a81adc221c..91a788c667 100644 --- a/conformance/conformance_test.go +++ b/conformance/conformance_test.go @@ -49,6 +49,7 @@ func TestConformance(t *testing.T) { for feature := range exemptFeatures { supportedFeatures[feature] = false } + t.Logf("Running conformance tests with %s GatewayClass\n cleanup: %t\n debug: %t\n supported features: [%v]\n exempt features: [%v]", *flags.GatewayClassName, *flags.CleanupBaseResources, *flags.ShowDebug, *flags.SupportedFeatures, *flags.ExemptFeatures) diff --git a/conformance/tests/tlsroute-simple-same-namespace.go b/conformance/tests/tlsroute-simple-same-namespace.go index 8859a3c45b..d42ed9e1b0 100644 --- a/conformance/tests/tlsroute-simple-same-namespace.go +++ b/conformance/tests/tlsroute-simple-same-namespace.go @@ -48,19 +48,18 @@ var TLSRouteSimpleSameNamespace = suite.ConformanceTest{ gwNN := types.NamespacedName{Name: "gateway-tlsroute", Namespace: string(ns)} certNN := types.NamespacedName{Name: "tls-passthrough-checks-certificate", Namespace: string(ns)} - gwAddr, server := kubernetes.GatewayAndTLSRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) - if len(server) != 1 { - fmt.Errorf("one and only one server required for TLS") + gwAddr, hostnames := kubernetes.GatewayAndTLSRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + if len(hostnames) != 1 { + t.Fatalf("unexpected error in test configuration, found %d hostnames", len(hostnames)) } - serverStr := string(server[0]) + serverStr := string(hostnames[0]) - cPem, kPem, err := GetTLSSecret(suite.Client, certNN) + cPem, keyPem, err := GetTLSSecret(suite.Client, certNN) if err != nil { - fmt.Errorf("unexpected error finding TLS secret: %w", err) + t.Fatalf("unexpected error finding TLS secret: %v", err) } - - t.Run("Simple HTTP request for TLSRoute should reach infra-backend", func(t *testing.T) { - tls.MakeTLSRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, cPem, kPem, serverStr, + t.Run("Simple TLS request matching TLSRoute should reach infra-backend", func(t *testing.T) { + tls.MakeTLSRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, cPem, keyPem, serverStr, http.ExpectedResponse{ Request: http.Request{Host: serverStr, Path: "/"}, Backend: "infra-backend-v4", diff --git a/conformance/tests/tlsroute-simple-same-namespace.yaml b/conformance/tests/tlsroute-simple-same-namespace.yaml index 07c878a2d3..dea677bb9a 100644 --- a/conformance/tests/tlsroute-simple-same-namespace.yaml +++ b/conformance/tests/tlsroute-simple-same-namespace.yaml @@ -32,4 +32,4 @@ spec: kinds: - kind: TLSRoute tls: - mode: Passthrough \ No newline at end of file + mode: Passthrough diff --git a/conformance/utils/config/timeout.go b/conformance/utils/config/timeout.go index 039e4e4e6a..91a1fd23fd 100644 --- a/conformance/utils/config/timeout.go +++ b/conformance/utils/config/timeout.go @@ -51,9 +51,9 @@ type TimeoutConfig struct { // Max value for conformant implementation: None HTTPRouteMustHaveCondition time.Duration - // HTTPRouteMustHaveParents represents the maximum time for an HTTPRoute to have parents in status that match the expected parents. + // RouteMustHaveParents represents the maximum time for an xRoute to have parents in status that match the expected parents. // Max value for conformant implementation: None - HTTPRouteMustHaveParents time.Duration + RouteMustHaveParents time.Duration // ManifestFetchTimeout represents the maximum time for getting content from a https:// URL. // Max value for conformant implementation: None @@ -83,7 +83,7 @@ func DefaultTimeoutConfig() TimeoutConfig { GWCMustBeAccepted: 180 * time.Second, HTTPRouteMustNotHaveParents: 60 * time.Second, HTTPRouteMustHaveCondition: 60 * time.Second, - HTTPRouteMustHaveParents: 60 * time.Second, + RouteMustHaveParents: 60 * time.Second, ManifestFetchTimeout: 10 * time.Second, MaxTimeToConsistency: 30 * time.Second, NamespacesMustBeReady: 300 * time.Second, @@ -117,8 +117,8 @@ func SetupTimeoutConfig(timeoutConfig *TimeoutConfig) { if timeoutConfig.HTTPRouteMustHaveCondition == 0 { timeoutConfig.HTTPRouteMustHaveCondition = defaultTimeoutConfig.HTTPRouteMustHaveCondition } - if timeoutConfig.HTTPRouteMustHaveParents == 0 { - timeoutConfig.HTTPRouteMustHaveParents = defaultTimeoutConfig.HTTPRouteMustHaveParents + if timeoutConfig.RouteMustHaveParents == 0 { + timeoutConfig.RouteMustHaveParents = defaultTimeoutConfig.RouteMustHaveParents } if timeoutConfig.ManifestFetchTimeout == 0 { timeoutConfig.ManifestFetchTimeout = defaultTimeoutConfig.ManifestFetchTimeout diff --git a/conformance/utils/flags/flags.go b/conformance/utils/flags/flags.go index 15b8334f6d..5159773987 100644 --- a/conformance/utils/flags/flags.go +++ b/conformance/utils/flags/flags.go @@ -29,5 +29,4 @@ var ( CleanupBaseResources = flag.Bool("cleanup-base-resources", true, "Whether to cleanup base test resources after the run") SupportedFeatures = flag.String("supported-features", "", "Supported features included in conformance tests suites") ExemptFeatures = flag.String("exempt-features", "", "Exempt Features excluded from conformance tests suites") - RunOneTest = flag.String("test", "", "A single test name when you want to run only one") ) diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index 333bdfb014..2c1dd6fbfa 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -96,6 +96,14 @@ const requiredConsecutiveSuccesses = 3 func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripper.RoundTripper, timeoutConfig config.TimeoutConfig, gwAddr string, expected ExpectedResponse) { t.Helper() + req := MakeRequest(t, &expected, gwAddr, "HTTP", "http") + + WaitForConsistentResponse(t, r, req, expected, requiredConsecutiveSuccesses, timeoutConfig.MaxTimeToConsistency) +} + +func MakeRequest(t *testing.T, expected *ExpectedResponse, gwAddr, protocol, scheme string) roundtripper.Request { + t.Helper() + if expected.Request.Method == "" { expected.Request.Method = "GET" } @@ -104,15 +112,15 @@ func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripp expected.Response.StatusCode = 200 } - t.Logf("Making %s request to http://%s%s", expected.Request.Method, gwAddr, expected.Request.Path) + t.Logf("Making %s request to %s://%s%s", expected.Request.Method, scheme, gwAddr, expected.Request.Path) path, query, _ := strings.Cut(expected.Request.Path, "?") req := roundtripper.Request{ Method: expected.Request.Method, Host: expected.Request.Host, - URL: url.URL{Scheme: "http", Host: gwAddr, Path: path, RawQuery: query}, - Protocol: "HTTP", + URL: url.URL{Scheme: scheme, Host: gwAddr, Path: path, RawQuery: query}, + Protocol: protocol, Headers: map[string][]string{}, UnfollowRedirect: expected.Request.UnfollowRedirect, } @@ -129,7 +137,7 @@ func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripp } req.Headers["X-Echo-Set-Header"] = []string{strings.Join(backendSetHeaders, ",")} - WaitForConsistentResponse(t, r, req, expected, requiredConsecutiveSuccesses, timeoutConfig.MaxTimeToConsistency) + return req } // AwaitConvergence runs the given function until it returns 'true' `threshold` times in a row. diff --git a/conformance/utils/kubernetes/helpers.go b/conformance/utils/kubernetes/helpers.go index 77dd07bfbf..ca60ebeea2 100644 --- a/conformance/utils/kubernetes/helpers.go +++ b/conformance/utils/kubernetes/helpers.go @@ -152,8 +152,8 @@ func ConditionsHaveLatestObservedGeneration(obj metav1.Object, conditions []meta return errors.New(b.String()) } -// FilterStaleConditions returns the list of status condition whos observedGeneration does not -// match the objects metadata.Generation +// FilterStaleConditions returns the list of status condition whose observedGeneration does not +// match the object's metadata.Generation func FilterStaleConditions(obj metav1.Object, conditions []metav1.Condition) []metav1.Condition { stale := make([]metav1.Condition, 0, len(conditions)) for _, condition := range conditions { @@ -387,7 +387,7 @@ func HTTPRouteMustHaveParents(t *testing.T, client client.Client, timeoutConfig t.Helper() var actual []v1beta1.RouteParentStatus - waitErr := wait.PollImmediate(1*time.Second, timeoutConfig.HTTPRouteMustHaveParents, func() (bool, error) { + waitErr := wait.PollImmediate(1*time.Second, timeoutConfig.RouteMustHaveParents, func() (bool, error) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -410,36 +410,31 @@ func HTTPRouteMustHaveParents(t *testing.T, client client.Client, timeoutConfig require.NoErrorf(t, waitErr, "error waiting for HTTPRoute to have parents matching expectations") } -// TLSRouteInfo waits for the specified TLSRoute to have parents -// in status that match the expected parents, and also returns the assigned -// hostnames of the TLSRoute. This will cause the test to halt if the -// specified timeout is exceeded. -func TLSRouteInfo(t *testing.T, client client.Client, timeoutConfig config.TimeoutConfig, routeName types.NamespacedName, parents []v1beta1.RouteParentStatus, namespaceRequired bool) []v1beta1.Hostname { +// TLSRouteMustHaveParents waits for the specified TLSRoute to have parents +// in status that match the expected parents, and also returns the TLSRoute. +// This will cause the test to halt if the specified timeout is exceeded. +func TLSRouteMustHaveParents(t *testing.T, client client.Client, timeoutConfig config.TimeoutConfig, routeName types.NamespacedName, parents []v1alpha2.RouteParentStatus, namespaceRequired bool) v1alpha2.TLSRoute { t.Helper() var actual []v1beta1.RouteParentStatus - var hostnames []v1beta1.Hostname + var route v1alpha2.TLSRoute - waitErr := wait.PollImmediate(1*time.Second, timeoutConfig.HTTPRouteMustHaveParents, func() (bool, error) { + waitErr := wait.PollImmediate(1*time.Second, timeoutConfig.RouteMustHaveParents, func() (bool, error) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - route := &v1alpha2.TLSRoute{} - err := client.Get(ctx, routeName, route) + err := client.Get(ctx, routeName, &route) if err != nil { return false, fmt.Errorf("error fetching TLSRoute: %w", err) } actual = route.Status.Parents - hostnames = route.Spec.Hostnames match := parentsForRouteMatch(t, routeName, parents, actual, namespaceRequired) return match, nil }) - if waitErr != nil { - fmt.Errorf("error waiting for TLSRoute to have parents matching expectations") - } + require.NoErrorf(t, waitErr, "error waiting for TLSRoute to have parents matching expectations") - return hostnames + return route } func parentsForRouteMatch(t *testing.T, routeName types.NamespacedName, expected, actual []v1beta1.RouteParentStatus, namespaceRequired bool) bool { @@ -598,7 +593,8 @@ func GatewayAndTLSRoutesMustBeAccepted(t *testing.T, c client.Client, timeoutCon }, }) } - hostnames = TLSRouteInfo(t, c, timeoutConfig, routeNN, parents, namespaceRequired) + route := TLSRouteMustHaveParents(t, c, timeoutConfig, routeNN, parents, namespaceRequired) + hostnames = route.Spec.Hostnames } return gwAddr, hostnames diff --git a/conformance/utils/roundtripper/roundtripper.go b/conformance/utils/roundtripper/roundtripper.go index 295ae8b3f1..608a105cc6 100644 --- a/conformance/utils/roundtripper/roundtripper.go +++ b/conformance/utils/roundtripper/roundtripper.go @@ -22,11 +22,13 @@ import ( "crypto/x509" "encoding/json" "fmt" - "io/ioutil" + "io" + iou "io/ioutil" "net/http" "net/http/httputil" "net/url" "regexp" + "sigs.k8s.io/gateway-api/conformance/utils/config" ) @@ -133,7 +135,9 @@ func (d *DefaultRoundTripper) CaptureRoundTrip(request Request) (*CapturedReques if err != nil { return nil, nil, err } - defer resp.Body.Close() + defer func(Body io.ReadCloser) { + _ = Body.Close() + }(resp.Body) if d.Debug { var dump []byte @@ -145,7 +149,7 @@ func (d *DefaultRoundTripper) CaptureRoundTrip(request Request) (*CapturedReques fmt.Printf("Received Response:\n%s\n\n", formatDump(dump, "< ")) } - body, _ := ioutil.ReadAll(resp.Body) + body, _ := iou.ReadAll(resp.Body) // we cannot assume the response is JSON if resp.Header.Get("Content-type") == "application/json" { @@ -197,12 +201,12 @@ func IsRedirect(statusCode int) bool { // captured request and response from echoserver. An error will be returned if // there is an error running the function but not if an HTTP error status code // is received. -func (d *DefaultRoundTripper) CaptureTLSRoundTrip(request Request, cPem, kPem []byte, server string) (*CapturedRequest, *CapturedResponse, error) { +func (d *DefaultRoundTripper) CaptureTLSRoundTrip(request Request, cPem, keyPem []byte, server string) (*CapturedRequest, *CapturedResponse, error) { cReq := &CapturedRequest{} client := http.DefaultClient // Create a certificate from the provided cert and key - cert, err := tls.X509KeyPair(cPem, kPem) + cert, err := tls.X509KeyPair(cPem, keyPem) if err != nil { return nil, nil, fmt.Errorf("unexpected error creating cert: %w", err) } @@ -223,6 +227,8 @@ func (d *DefaultRoundTripper) CaptureTLSRoundTrip(request Request, cPem, kPem [] Certificates: []tls.Certificate{cert}, ServerName: server, RootCAs: certPool, + MinVersion: tls.VersionTLS10, + MaxVersion: tls.VersionTLS13, }, } @@ -261,7 +267,9 @@ func (d *DefaultRoundTripper) CaptureTLSRoundTrip(request Request, cPem, kPem [] if err != nil { return nil, nil, err } - defer resp.Body.Close() + defer func(Body io.ReadCloser) { + _ = Body.Close() + }(resp.Body) if d.Debug { var dump []byte @@ -273,7 +281,7 @@ func (d *DefaultRoundTripper) CaptureTLSRoundTrip(request Request, cPem, kPem [] fmt.Printf("Received Response:\n%s\n\n", formatDump(dump, "< ")) } - body, _ := ioutil.ReadAll(resp.Body) + body, _ := iou.ReadAll(resp.Body) // we cannot assume the response is JSON if resp.Header.Get("Content-type") == "application/json" { diff --git a/conformance/utils/tls/tls.go b/conformance/utils/tls/tls.go index 917a110e7e..e696be8ae9 100644 --- a/conformance/utils/tls/tls.go +++ b/conformance/utils/tls/tls.go @@ -17,8 +17,6 @@ limitations under the License. package tls import ( - "net/url" - "strings" "testing" "time" @@ -38,53 +36,20 @@ const requiredConsecutiveSuccesses = 3 // // Once the request succeeds consistently with the response having the expected status code, make // additional assertions on the response body using the provided ExpectedResponse. -func MakeTLSRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripper.RoundTripper, timeoutConfig config.TimeoutConfig, gwAddr string, cPem, kPem []byte, server string, expected http.ExpectedResponse) { +func MakeTLSRequestAndExpectEventuallyConsistentResponse(t *testing.T, r roundtripper.RoundTripper, timeoutConfig config.TimeoutConfig, gwAddr string, cPem, keyPem []byte, server string, expected http.ExpectedResponse) { t.Helper() - protocol := "HTTPS" - scheme := "https" + req := http.MakeRequest(t, &expected, gwAddr, "HTTPS", "https") - if expected.Request.Method == "" { - expected.Request.Method = "GET" - } - - if expected.Response.StatusCode == 0 { - expected.Response.StatusCode = 200 - } - - t.Logf("Making %s request to %s://%s%s", expected.Request.Method, scheme, gwAddr, expected.Request.Path) - - path, query, _ := strings.Cut(expected.Request.Path, "?") - - req := roundtripper.Request{ - Method: expected.Request.Method, - Host: expected.Request.Host, - URL: url.URL{Scheme: scheme, Host: gwAddr, Path: path, RawQuery: query}, - Protocol: protocol, - Headers: map[string][]string{}, - } - - if expected.Request.Headers != nil { - for name, value := range expected.Request.Headers { - req.Headers[name] = []string{value} - } - } - - backendSetHeaders := []string{} - for name, val := range expected.BackendSetResponseHeaders { - backendSetHeaders = append(backendSetHeaders, name+":"+val) - } - req.Headers["X-Echo-Set-Header"] = []string{strings.Join(backendSetHeaders, ",")} - - WaitForConsistentTLSResponse(t, r, req, expected, requiredConsecutiveSuccesses, timeoutConfig.MaxTimeToConsistency, cPem, kPem, server) + WaitForConsistentTLSResponse(t, r, req, expected, requiredConsecutiveSuccesses, timeoutConfig.MaxTimeToConsistency, cPem, keyPem, server) } // WaitForConsistentTLSResponse - repeats the provided request until it completes with a response having // the expected response consistently. The provided threshold determines how many times in // a row this must occur to be considered "consistent". -func WaitForConsistentTLSResponse(t *testing.T, r roundtripper.RoundTripper, req roundtripper.Request, expected http.ExpectedResponse, threshold int, maxTimeToConsistency time.Duration, cPem, kPem []byte, server string) { +func WaitForConsistentTLSResponse(t *testing.T, r roundtripper.RoundTripper, req roundtripper.Request, expected http.ExpectedResponse, threshold int, maxTimeToConsistency time.Duration, cPem, keyPem []byte, server string) { http.AwaitConvergence(t, threshold, maxTimeToConsistency, func(elapsed time.Duration) bool { - cReq, cRes, err := r.CaptureTLSRoundTrip(req, cPem, kPem, server) + cReq, cRes, err := r.CaptureTLSRoundTrip(req, cPem, keyPem, server) if err != nil { t.Logf("Request failed, not ready yet: %v (after %v)", err.Error(), elapsed) return false diff --git a/site-src/concepts/conformance.md b/site-src/concepts/conformance.md index e53fc21437..cfecc0240f 100644 --- a/site-src/concepts/conformance.md +++ b/site-src/concepts/conformance.md @@ -91,6 +91,11 @@ suppress cleanup: ```shell go test ./conformance/... -args -gateway-class=istio -cleanup-base-resources=false ``` +If you'd like to run a single test instead of the entire conformance suite, find your test name +`(suite.ConformanceTest.ShortName)` and use it like this: +```shell +go test ./conformance/... --run TestConformance/YOURTESTNAME --gateway-class=istio +``` ## Contributing to Conformance Many implementations run conformance tests as part of their full e2e test suite.