From 6db445ba29173176491349e477b52fe1470fae01 Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Tue, 28 Feb 2023 14:15:40 -0600 Subject: [PATCH] Gateway Test HTTPPathRewrite (#16418) * add http url path rewrite * add Mike's test back in * update kind to use api.APIGateway --- .../test/gateways/gateway_endpoint_test.go | 97 +++++-- .../test/gateways/http_route_test.go | 250 ++++++++++++++---- 2 files changed, 271 insertions(+), 76 deletions(-) diff --git a/test/integration/consul-container/test/gateways/gateway_endpoint_test.go b/test/integration/consul-container/test/gateways/gateway_endpoint_test.go index 05fb3d8961..2aa81954f8 100644 --- a/test/integration/consul-container/test/gateways/gateway_endpoint_test.go +++ b/test/integration/consul-container/test/gateways/gateway_endpoint_test.go @@ -51,7 +51,7 @@ func TestAPIGatewayCreate(t *testing.T) { }, } _, _, err := client.ConfigEntries().Set(apiGateway, nil) - assert.NoError(t, err) + require.NoError(t, err) tcpRoute := &api.TCPRouteConfigEntry{ Kind: "tcp-route", @@ -70,32 +70,18 @@ func TestAPIGatewayCreate(t *testing.T) { } _, _, err = client.ConfigEntries().Set(tcpRoute, nil) - assert.NoError(t, err) + require.NoError(t, err) // Create a client proxy instance with the server as an upstream _, gatewayService := createServices(t, cluster, listenerPortOne) - //check statuses - gatewayReady := false - routeReady := false - //make sure the gateway/route come online - require.Eventually(t, func() bool { - entry, _, err := client.ConfigEntries().Get("api-gateway", "api-gateway", nil) - assert.NoError(t, err) - apiEntry := entry.(*api.APIGatewayConfigEntry) - gatewayReady = isAccepted(apiEntry.Status.Conditions) - - e, _, err := client.ConfigEntries().Get("tcp-route", "api-gateway-route", nil) - assert.NoError(t, err) - routeEntry := e.(*api.TCPRouteConfigEntry) - routeReady = isBound(routeEntry.Status.Conditions) - - return gatewayReady && routeReady - }, time.Second*10, time.Second*1) + //make sure config entries have been properly created + checkGatewayConfigEntry(t, client, "api-gateway", "") + checkTCPRouteConfigEntry(t, client, "api-gateway-route", "") port, err := gatewayService.GetPort(listenerPortOne) - assert.NoError(t, err) + require.NoError(t, err) libassert.HTTPServiceEchoes(t, "localhost", port, "") } @@ -150,12 +136,64 @@ func createCluster(t *testing.T, ports ...int) *libcluster.Cluster { return cluster } +func createGateway(gatewayName string, protocol string, listenerPort int) *api.APIGatewayConfigEntry { + return &api.APIGatewayConfigEntry{ + Kind: api.APIGateway, + Name: gatewayName, + Listeners: []api.APIGatewayListener{ + { + Name: "listener", + Port: listenerPort, + Protocol: protocol, + }, + }, + } +} + +func checkGatewayConfigEntry(t *testing.T, client *api.Client, gatewayName string, namespace string) { + require.Eventually(t, func() bool { + entry, _, err := client.ConfigEntries().Get(api.APIGateway, gatewayName, &api.QueryOptions{Namespace: namespace}) + require.NoError(t, err) + if entry == nil { + return false + } + apiEntry := entry.(*api.APIGatewayConfigEntry) + return isAccepted(apiEntry.Status.Conditions) + }, time.Second*10, time.Second*1) +} + +func checkHTTPRouteConfigEntry(t *testing.T, client *api.Client, routeName string, namespace string) { + require.Eventually(t, func() bool { + entry, _, err := client.ConfigEntries().Get(api.HTTPRoute, routeName, &api.QueryOptions{Namespace: namespace}) + require.NoError(t, err) + if entry == nil { + return false + } + + apiEntry := entry.(*api.HTTPRouteConfigEntry) + return isBound(apiEntry.Status.Conditions) + }, time.Second*10, time.Second*1) +} + +func checkTCPRouteConfigEntry(t *testing.T, client *api.Client, routeName string, namespace string) { + require.Eventually(t, func() bool { + entry, _, err := client.ConfigEntries().Get(api.TCPRoute, routeName, &api.QueryOptions{Namespace: namespace}) + require.NoError(t, err) + if entry == nil { + return false + } + + apiEntry := entry.(*api.TCPRouteConfigEntry) + return isBound(apiEntry.Status.Conditions) + }, time.Second*10, time.Second*1) +} + func createService(t *testing.T, cluster *libcluster.Cluster, serviceOpts *libservice.ServiceOpts, containerArgs []string) libservice.Service { node := cluster.Agents[0] client := node.GetClient() // Create a service and proxy instance service, _, err := libservice.CreateAndRegisterStaticServerAndSidecar(node, serviceOpts, containerArgs...) - assert.NoError(t, err) + require.NoError(t, err) libassert.CatalogServiceExists(t, client, serviceOpts.Name+"-sidecar-proxy") libassert.CatalogServiceExists(t, client, serviceOpts.Name) @@ -183,15 +221,18 @@ func createServices(t *testing.T, cluster *libcluster.Cluster, ports ...int) (li return clientConnectProxy, gatewayService } -// checkRoute, customized version of libassert.RouteEchos to allow for headers/distinguishing between the server instances - type checkOptions struct { debug bool statusCode int testName string } -func checkRoute(t *testing.T, ip string, port int, path string, headers map[string]string, expected checkOptions) { +// checkRoute, customized version of libassert.RouteEchos to allow for headers/distinguishing between the server instances +func checkRoute(t *testing.T, port int, path string, headers map[string]string, expected checkOptions) { + ip := "localhost" + if expected.testName != "" { + t.Log("running " + expected.testName) + } const phrase = "hello" failer := func() *retry.Timer { @@ -199,17 +240,15 @@ func checkRoute(t *testing.T, ip string, port int, path string, headers map[stri } client := cleanhttp.DefaultClient() - url := fmt.Sprintf("http://%s:%d", ip, port) - if path != "" { - url += "/" + path - } + path = strings.TrimPrefix(path, "/") + url := fmt.Sprintf("http://%s:%d/%s", ip, port, path) retry.RunWith(failer(), t, func(r *retry.R) { t.Logf("making call to %s", url) reader := strings.NewReader(phrase) req, err := http.NewRequest("POST", url, reader) - assert.NoError(t, err) + require.NoError(t, err) headers["content-type"] = "text/plain" for k, v := range headers { diff --git a/test/integration/consul-container/test/gateways/http_route_test.go b/test/integration/consul-container/test/gateways/http_route_test.go index 943d93d860..61343f3495 100644 --- a/test/integration/consul-container/test/gateways/http_route_test.go +++ b/test/integration/consul-container/test/gateways/http_route_test.go @@ -83,7 +83,7 @@ func TestHTTPRouteFlattening(t *testing.T) { } _, _, err := client.ConfigEntries().Set(proxyDefaults, nil) - assert.NoError(t, err) + require.NoError(t, err) apiGateway := &api.APIGatewayConfigEntry{ Kind: "api-gateway", @@ -174,11 +174,11 @@ func TestHTTPRouteFlattening(t *testing.T) { } _, _, err = client.ConfigEntries().Set(apiGateway, nil) - assert.NoError(t, err) + require.NoError(t, err) _, _, err = client.ConfigEntries().Set(routeOne, nil) - assert.NoError(t, err) + require.NoError(t, err) _, _, err = client.ConfigEntries().Set(routeTwo, nil) - assert.NoError(t, err) + require.NoError(t, err) //create gateway service gatewayService, err := libservice.NewGatewayService(context.Background(), gatewayName, "api", cluster.Agents[0], listenerPort) @@ -186,77 +186,233 @@ func TestHTTPRouteFlattening(t *testing.T) { libassert.CatalogServiceExists(t, client, gatewayName) //make sure config entries have been properly created - require.Eventually(t, func() bool { - entry, _, err := client.ConfigEntries().Get(api.APIGateway, gatewayName, &api.QueryOptions{Namespace: namespace}) - assert.NoError(t, err) - if entry == nil { - return false - } - apiEntry := entry.(*api.APIGatewayConfigEntry) - t.Log(entry) - return isAccepted(apiEntry.Status.Conditions) - }, time.Second*10, time.Second*1) - - require.Eventually(t, func() bool { - entry, _, err := client.ConfigEntries().Get(api.HTTPRoute, routeOneName, &api.QueryOptions{Namespace: namespace}) - assert.NoError(t, err) - if entry == nil { - return false - } - - apiEntry := entry.(*api.HTTPRouteConfigEntry) - t.Log(entry) - return isBound(apiEntry.Status.Conditions) - }, time.Second*10, time.Second*1) - - require.Eventually(t, func() bool { - entry, _, err := client.ConfigEntries().Get(api.HTTPRoute, routeTwoName, nil) - assert.NoError(t, err) - if entry == nil { - return false - } - - apiEntry := entry.(*api.HTTPRouteConfigEntry) - return isBound(apiEntry.Status.Conditions) - }, time.Second*10, time.Second*1) + checkGatewayConfigEntry(t, client, gatewayName, namespace) + checkHTTPRouteConfigEntry(t, client, routeOneName, namespace) + checkHTTPRouteConfigEntry(t, client, routeTwoName, namespace) //gateway resolves routes - ip := "localhost" gatewayPort, err := gatewayService.GetPort(listenerPort) - assert.NoError(t, err) + require.NoError(t, err) + + //route 2 with headers //Same v2 path with and without header - checkRoute(t, ip, gatewayPort, "v2", map[string]string{ + checkRoute(t, gatewayPort, "/v2", map[string]string{ "Host": "test.foo", "x-v2": "v2", }, checkOptions{statusCode: service2ResponseCode, testName: "service2 header and path"}) - checkRoute(t, ip, gatewayPort, "v2", map[string]string{ + checkRoute(t, gatewayPort, "/v2", map[string]string{ "Host": "test.foo", }, checkOptions{statusCode: service2ResponseCode, testName: "service2 just path match"}) ////v1 path with the header - checkRoute(t, ip, gatewayPort, "check", map[string]string{ + checkRoute(t, gatewayPort, "/check", map[string]string{ "Host": "test.foo", "x-v2": "v2", }, checkOptions{statusCode: service2ResponseCode, testName: "service2 just header match"}) - checkRoute(t, ip, gatewayPort, "v2/path/value", map[string]string{ + checkRoute(t, gatewayPort, "/v2/path/value", map[string]string{ "Host": "test.foo", "x-v2": "v2", }, checkOptions{statusCode: service2ResponseCode, testName: "service2 v2 with path"}) //hit service 1 by hitting root path - checkRoute(t, ip, gatewayPort, "", map[string]string{ + checkRoute(t, gatewayPort, "", map[string]string{ "Host": "test.foo", }, checkOptions{debug: false, statusCode: service1ResponseCode, testName: "service1 root prefix"}) //hit service 1 by hitting v2 path with v1 hostname - checkRoute(t, ip, gatewayPort, "v2", map[string]string{ + checkRoute(t, gatewayPort, "/v2", map[string]string{ "Host": "test.example", }, checkOptions{debug: false, statusCode: service1ResponseCode, testName: "service1, v2 path with v2 hostname"}) } +func TestHTTPRoutePathRewrite(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + + //infrastructure set up + listenerPort := 6001 + //create cluster + cluster := createCluster(t, listenerPort) + client := cluster.Agents[0].GetClient() + fooStatusCode := 400 + barStatusCode := 201 + fooPath := "/v1/foo" + barPath := "/v1/bar" + + fooService := createService(t, cluster, &libservice.ServiceOpts{ + Name: "foo", + ID: "foo", + HTTPPort: 8080, + GRPCPort: 8081, + }, []string{ + //customizes response code so we can distinguish between which service is responding + "-echo-debug-path", fooPath, + "-echo-server-default-params", fmt.Sprintf("status=%d", fooStatusCode), + }) + barService := createService(t, cluster, &libservice.ServiceOpts{ + Name: "bar", + ID: "bar", + //TODO we can potentially get conflicts if these ports are the same + HTTPPort: 8079, + GRPCPort: 8078, + }, []string{ + "-echo-debug-path", barPath, + "-echo-server-default-params", fmt.Sprintf("status=%d", barStatusCode), + }, + ) + + namespace := getNamespace() + gatewayName := randomName("gw", 16) + invalidRouteName := randomName("route", 16) + validRouteName := randomName("route", 16) + fooUnrewritten := "/foo" + barUnrewritten := "/bar" + + //write config entries + proxyDefaults := &api.ProxyConfigEntry{ + Kind: api.ProxyDefaults, + Name: api.ProxyConfigGlobal, + Namespace: namespace, + Config: map[string]interface{}{ + "protocol": "http", + }, + } + + _, _, err := client.ConfigEntries().Set(proxyDefaults, nil) + require.NoError(t, err) + + apiGateway := createGateway(gatewayName, "http", listenerPort) + + fooRoute := &api.HTTPRouteConfigEntry{ + Kind: api.HTTPRoute, + Name: invalidRouteName, + Parents: []api.ResourceReference{ + { + Kind: api.APIGateway, + Name: gatewayName, + Namespace: namespace, + }, + }, + Hostnames: []string{ + "test.foo", + }, + Namespace: namespace, + Rules: []api.HTTPRouteRule{ + { + Filters: api.HTTPFilters{ + URLRewrite: &api.URLRewrite{ + Path: fooPath, + }, + }, + Services: []api.HTTPService{ + { + Name: fooService.GetServiceName(), + Namespace: namespace, + }, + }, + Matches: []api.HTTPMatch{ + { + Path: api.HTTPPathMatch{ + Match: api.HTTPPathMatchPrefix, + Value: fooUnrewritten, + }, + }, + }, + }, + }, + } + + barRoute := &api.HTTPRouteConfigEntry{ + Kind: api.HTTPRoute, + Name: validRouteName, + Parents: []api.ResourceReference{ + { + Kind: api.APIGateway, + Name: gatewayName, + Namespace: namespace, + }, + }, + Hostnames: []string{ + "test.foo", + }, + Namespace: namespace, + Rules: []api.HTTPRouteRule{ + { + Filters: api.HTTPFilters{ + URLRewrite: &api.URLRewrite{ + Path: barPath, + }, + }, + Services: []api.HTTPService{ + { + Name: barService.GetServiceName(), + Namespace: namespace, + }, + }, + Matches: []api.HTTPMatch{ + { + Path: api.HTTPPathMatch{ + Match: api.HTTPPathMatchPrefix, + Value: barUnrewritten, + }, + }, + }, + }, + }, + } + + _, _, err = client.ConfigEntries().Set(apiGateway, nil) + require.NoError(t, err) + _, _, err = client.ConfigEntries().Set(fooRoute, nil) + require.NoError(t, err) + _, _, err = client.ConfigEntries().Set(barRoute, nil) + require.NoError(t, err) + + //create gateway service + gatewayService, err := libservice.NewGatewayService(context.Background(), gatewayName, "api", cluster.Agents[0], listenerPort) + require.NoError(t, err) + libassert.CatalogServiceExists(t, client, gatewayName) + + //make sure config entries have been properly created + checkGatewayConfigEntry(t, client, gatewayName, namespace) + checkHTTPRouteConfigEntry(t, client, invalidRouteName, namespace) + checkHTTPRouteConfigEntry(t, client, validRouteName, namespace) + + gatewayPort, err := gatewayService.GetPort(listenerPort) + require.NoError(t, err) + + //TODO these were the assertions we had in the original test. potentially would want more test cases + + //NOTE: Hitting the debug path code overrides default expected value + debugExpectedStatusCode := 200 + + //hit foo, making sure path is being rewritten by hitting the debug page + checkRoute(t, gatewayPort, fooUnrewritten, map[string]string{ + "Host": "test.foo", + }, checkOptions{debug: true, statusCode: debugExpectedStatusCode, testName: "foo service"}) + //make sure foo is being sent to proper service + checkRoute(t, gatewayPort, fooUnrewritten+"/foo", map[string]string{ + "Host": "test.foo", + }, checkOptions{debug: false, statusCode: fooStatusCode, testName: "foo service"}) + + //hit bar, making sure its been rewritten + checkRoute(t, gatewayPort, barUnrewritten, map[string]string{ + "Host": "test.foo", + }, checkOptions{debug: true, statusCode: debugExpectedStatusCode, testName: "bar service"}) + + //hit bar, making sure its being sent to the proper service + checkRoute(t, gatewayPort, barUnrewritten+"/bar", map[string]string{ + "Host": "test.foo", + }, checkOptions{debug: false, statusCode: barStatusCode, testName: "bar service"}) + +} + func TestHTTPRouteParentRefChange(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") @@ -420,7 +576,7 @@ func TestHTTPRouteParentRefChange(t *testing.T) { // hit service by requesting root path // TODO: testName field in checkOptions struct looked to be unused, is it needed? - checkRoute(t, address, gatewayOnePort, "", map[string]string{ + checkRoute(t, gatewayOnePort, "", map[string]string{ "Host": "test.foo", }, checkOptions{debug: false, statusCode: 200}) @@ -457,7 +613,7 @@ func TestHTTPRouteParentRefChange(t *testing.T) { }, time.Second*10, time.Second*1) // hit service by requesting root path on other gateway with different hostname - checkRoute(t, address, gatewayTwoPort, "", map[string]string{ + checkRoute(t, gatewayTwoPort, "", map[string]string{ "Host": "test.example", }, checkOptions{debug: false, statusCode: 200})