diff options
author | Valerij Fredriksen <valerijf@verizonmedia.com> | 2019-10-31 13:45:22 +0100 |
---|---|---|
committer | Valerij Fredriksen <valerijf@verizonmedia.com> | 2019-10-31 13:45:22 +0100 |
commit | d1ec7b663b51ad2013f38d7ee03fbf2e3aa677c3 (patch) | |
tree | fd87c51409b88afda636253d09ff793c5641979c /controller-server | |
parent | a4a78f54c995b7fbcc0a3d7a115af09da4ce8256 (diff) |
Refactor ConfigServerRestExecutorImpl
Diffstat (limited to 'controller-server')
7 files changed, 139 insertions, 193 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java index 6212569d788..e3c048e865a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java @@ -17,7 +17,6 @@ import org.apache.http.client.methods.HttpPatch; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpRequestBase; -import org.apache.http.conn.ssl.X509HostnameVerifier; import org.apache.http.entity.InputStreamEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; @@ -26,7 +25,6 @@ import org.apache.http.util.EntityUtils; import javax.net.ssl.HostnameVerifier; import java.io.IOException; import java.io.InputStream; -import java.io.UncheckedIOException; import java.net.URI; import java.time.Duration; import java.util.ArrayList; @@ -37,7 +35,8 @@ import java.util.Optional; import java.util.Set; import java.util.logging.Logger; -import static java.util.Collections.singleton; +import static com.yahoo.yolean.Exceptions.uncheck; + /** * @author Haakon Dybdahl @@ -55,17 +54,15 @@ public class ConfigServerRestExecutorImpl implements ConfigServerRestExecutor { private final ServiceIdentityProvider sslContextProvider; @Inject - public ConfigServerRestExecutorImpl(ZoneRegistry zoneRegistry, - ServiceIdentityProvider sslContextProvider) { + public ConfigServerRestExecutorImpl(ZoneRegistry zoneRegistry, ServiceIdentityProvider sslContextProvider) { this.zoneRegistry = zoneRegistry; this.sslContextProvider = sslContextProvider; } @Override public ProxyResponse handle(ProxyRequest proxyRequest) throws ProxyException { - ZoneId zoneId = ZoneId.from(proxyRequest.getEnvironment(), proxyRequest.getRegion()); - HostnameVerifier hostnameVerifier = createHostnameVerifier(zoneId); - List<URI> allServers = getConfigserverEndpoints(zoneId); + HostnameVerifier hostnameVerifier = createHostnameVerifier(proxyRequest.getZoneId()); + List<URI> allServers = getConfigserverEndpoints(proxyRequest.getZoneId()); StringBuilder errorBuilder = new StringBuilder(); if (queueFirstServerIfDown(allServers, hostnameVerifier)) { @@ -86,25 +83,17 @@ public class ConfigServerRestExecutorImpl implements ConfigServerRestExecutor { // TODO: Use config server VIP for all zones that have one // Make a local copy of the list as we want to manipulate it in case of ping problems. if (zoneId.region().value().startsWith("aws-") || zoneId.region().value().contains("-aws-")) { - return Collections.singletonList(zoneRegistry.getConfigServerVipUri(zoneId)); + return List.of(zoneRegistry.getConfigServerVipUri(zoneId)); } else { return new ArrayList<>(zoneRegistry.getConfigServerUris(zoneId)); } } - private static String removeFirstSlashIfAny(String url) { - if (url.startsWith("/")) { - return url.substring(1); - } - return url; - } - private Optional<ProxyResponse> proxyCall( URI uri, ProxyRequest proxyRequest, HostnameVerifier hostnameVerifier, StringBuilder errorBuilder) throws ProxyException { - String fullUri = uri.toString() + removeFirstSlashIfAny(proxyRequest.getConfigServerRequest()); final HttpRequestBase requestBase = createHttpBaseRequest( - proxyRequest.getMethod(), fullUri, proxyRequest.getData()); + proxyRequest.getMethod(), proxyRequest.createConfigServerRequestUri(uri), proxyRequest.getData()); // Empty list of headers to copy for now, add headers when needed, or rewrite logic. copyHeaders(proxyRequest.getHeaders(), requestBase); @@ -145,20 +134,12 @@ public class ConfigServerRestExecutorImpl implements ConfigServerRestExecutor { private static String getContent(CloseableHttpResponse response) { return Optional.ofNullable(response.getEntity()) - .map(entity -> - { - try { - return EntityUtils.toString(entity); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - ).orElse(""); + .map(entity -> uncheck(() -> EntityUtils.toString(entity))) + .orElse(""); } - private static HttpRequestBase createHttpBaseRequest(String method, String uri, InputStream data) throws ProxyException { - Method enumMethod = Method.valueOf(method); - switch (enumMethod) { + private static HttpRequestBase createHttpBaseRequest(Method method, URI uri, InputStream data) throws ProxyException { + switch (method) { case GET: return new HttpGet(uri); case POST: @@ -216,7 +197,6 @@ public class ConfigServerRestExecutorImpl implements ConfigServerRestExecutor { try ( CloseableHttpClient client = createHttpClient(config, sslContextProvider, hostnameVerifier); CloseableHttpResponse response = client.execute(httpget) - ) { if (response.getStatusLine().getStatusCode() == 200) { return false; @@ -226,8 +206,7 @@ public class ConfigServerRestExecutorImpl implements ConfigServerRestExecutor { // We ignore this, if server is restarting this might happen. } // Some error happened, move this server to the back. The other servers should be running. - allServers.remove(0); - allServers.add(uri); + Collections.rotate(allServers, -1); return true; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java index aa6205f63a3..100292a0bdc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java @@ -1,15 +1,17 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.proxy; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.net.HostName; -import java.io.IOException; import java.io.InputStream; import java.net.URI; -import java.net.URLDecoder; +import java.net.URISyntaxException; import java.util.List; import java.util.Map; +import java.util.Objects; + +import static com.yahoo.jdisc.http.HttpRequest.Method; /** * Keeping information about the calls that are being proxied. @@ -19,95 +21,84 @@ import java.util.Map; */ public class ProxyRequest { - private final String environment; - private final String region; - private final String configServerRequest; - private final InputStream requestData; + private final Method method; + private final URI requestUri; private final Map<String, List<String>> headers; - private final String method; - private final String controllerPrefix; - private final String scheme; + private final InputStream requestData; + + private final ZoneId zoneId; + private final String proxyPath; /** * The constructor calls exception if the request is invalid. * * @param request the request from the jdisc framework. - * @param pathPrefix the path prefix of the proxy. + * @param zoneId the zone to proxy to. + * @param proxyPath the path to proxy to. * @throws ProxyException on errors */ - public ProxyRequest(HttpRequest request, String pathPrefix) throws ProxyException, IOException { - this(request.getUri(), request.getJDiscRequest().headers(), request.getData(), request.getMethod().name(), - pathPrefix); + public ProxyRequest(HttpRequest request, ZoneId zoneId, String proxyPath) throws ProxyException { + this(request.getMethod(), request.getUri(), request.getJDiscRequest().headers(), request.getData(), + zoneId, proxyPath); } - ProxyRequest(URI requestUri, Map<String, List<String>> headers, InputStream body, String method, - String pathPrefix) throws ProxyException, IOException { - if (requestUri == null) { - throw new ProxyException(ErrorResponse.badRequest("Request not set.")); - } - final String path = URLDecoder.decode(requestUri.getPath(),"UTF-8"); - if (! path.startsWith(pathPrefix)) { - // This has to be caused by wrong mapping of path in services.xml. - throw new ProxyException(ErrorResponse.notFoundError("Request not starting with " + pathPrefix)); - } - final String uriNoPrefix = path.replaceFirst(pathPrefix, "") - + (requestUri.getRawQuery() == null ? "" : "?" + requestUri.getRawQuery()); - - final String[] parts = uriNoPrefix.split("/"); + ProxyRequest(Method method, URI requestUri, Map<String, List<String>> headers, InputStream body, + ZoneId zoneId, String proxyPath) throws ProxyException { + Objects.requireNonNull(requestUri, "Request must be non-null"); + if (!requestUri.getPath().endsWith(proxyPath)) + throw new ProxyException(ErrorResponse.badRequest(String.format( + "Request path '%s' does not end with proxy path '%s'", requestUri.getPath(), proxyPath))); - this.environment = parts.length > 0 ? parts[0] : ""; - this.region = parts.length > 1 ? parts[1] : ""; - this.configServerRequest = parts.length > 2 ? uriNoPrefix.replace(environment + "/" + region, "") : ""; + this.method = Objects.requireNonNull(method); + this.requestUri = Objects.requireNonNull(requestUri); + this.headers = Objects.requireNonNull(headers); this.requestData = body; - this.headers = headers; - this.method = method; - - String hostPort = headers.containsKey("host") - ? headers.get("host").get(0) - : HostName.getLocalhost() + ":" + requestUri.getPort(); - StringBuilder prefix = new StringBuilder(hostPort + pathPrefix); - if (! environment.isEmpty()) { - prefix.append(environment).append("/").append(region); - } - this.controllerPrefix = prefix.toString(); - this.scheme = requestUri.getScheme(); + this.zoneId = Objects.requireNonNull(zoneId); + this.proxyPath = proxyPath.startsWith("/") ? proxyPath : "/" + proxyPath; } - public String getRegion() { - return region; - } - - public String getEnvironment() { - return environment; + public Method getMethod() { + return method; } - public String getConfigServerRequest() { - return configServerRequest; + public Map<String, List<String>> getHeaders() { + return headers; } public InputStream getData() { return requestData; } - @Override - public String toString() { - return "[ region: " + region + " env: " + environment + " request: " + configServerRequest + "]"; + public ZoneId getZoneId() { + return zoneId; } - public Map<String, List<String>> getHeaders() { - return headers; + public URI createConfigServerRequestUri(URI baseURI) { + try { + return new URI(baseURI.getScheme(), baseURI.getUserInfo(), baseURI.getHost(), + baseURI.getPort(), proxyPath, requestUri.getQuery(), requestUri.getFragment()); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } } - public String getMethod() { - return method; + public URI getControllerPrefixUri() { + String prefixPath = proxyPath.equals("/") && !requestUri.getPath().endsWith("/") ? + requestUri.getPath() + proxyPath : + requestUri.getPath().substring(0, requestUri.getPath().length() - proxyPath.length() + 1); + try { + return new URI(requestUri.getScheme(), requestUri.getUserInfo(), requestUri.getHost(), + requestUri.getPort(), prefixPath, null, null); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } } - public String getControllerPrefix() { - return controllerPrefix; + @Override + public String toString() { + return "[zone: " + zoneId + " request: " + proxyPath + "]"; } - public String getScheme() { return scheme; } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java index 64a82c2f5b5..b0b4f1a556a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java @@ -15,7 +15,7 @@ import java.nio.charset.StandardCharsets; * * @author Haakon Dybdahl */ -public class ProxyResponse extends HttpResponse { +public class ProxyResponse extends HttpResponse { private final String bodyResponseRewritten; private final String contentType; @@ -29,11 +29,6 @@ public class ProxyResponse extends HttpResponse { super(statusResponse); this.contentType = contentType; - if (controllerRequest.getControllerPrefix().isEmpty()) { - bodyResponseRewritten = bodyResponse; - return; - } - final String configServerPrefix; final String controllerRequestPrefix; try { @@ -41,12 +36,9 @@ public class ProxyResponse extends HttpResponse { .setScheme(configServer.getScheme()) .setHost(configServer.getHost()) .setPort(configServer.getPort()) + .setPath("/") .build().toString(); - controllerRequestPrefix = new URIBuilder() - .setScheme(controllerRequest.getScheme()) - // controller prefix is more than host, so it is a bit hackish, but verified by tests. - .setHost(controllerRequest.getControllerPrefix()) - .build().toString(); + controllerRequestPrefix = controllerRequest.getControllerPrefixUri().toString(); } catch (URISyntaxException e) { throw new RuntimeException(e); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java index 5ce679276f7..be601511763 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java @@ -1,25 +1,24 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.restapi.zone.v2; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.config.provision.zone.ZoneList; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; +import com.yahoo.restapi.ErrorResponse; import com.yahoo.restapi.Path; +import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.config.provision.zone.ZoneList; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; import com.yahoo.vespa.hosted.controller.auditlog.AuditLoggingRequestHandler; import com.yahoo.vespa.hosted.controller.proxy.ConfigServerRestExecutor; import com.yahoo.vespa.hosted.controller.proxy.ProxyException; import com.yahoo.vespa.hosted.controller.proxy.ProxyRequest; -import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.yolean.Exceptions; -import java.io.IOException; import java.util.logging.Level; /** @@ -83,8 +82,8 @@ public class ZoneApiHandler extends AuditLoggingRequestHandler { throw new IllegalArgumentException("No such zone: " + zoneId.value()); } try { - return proxy.handle(new ProxyRequest(request, "/zone/v2/")); - } catch (ProxyException | IOException e) { + return proxy.handle(new ProxyRequest(request, zoneId, path.getRest())); + } catch (ProxyException e) { throw new RuntimeException(e); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java index 3d27058f089..b9d68c2a3da 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java @@ -1,15 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.proxy; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.jdisc.http.HttpRequest; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import java.io.IOException; import java.net.URI; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; import java.util.Map; import static org.junit.Assert.assertEquals; @@ -24,40 +22,53 @@ public class ProxyRequestTest { @Test public void testEmpty() throws Exception { - exception.expectMessage("Request not set."); - testRequest(null, "/zone/v2/"); + exception.expectMessage("Request must be non-null"); + new ProxyRequest(HttpRequest.Method.GET, null, Map.of(), null, ZoneId.from("dev", "us-north-1"), "/zone/v2"); } @Test public void testBadUri() throws Exception { - exception.expectMessage("Request not starting with /zone/v2/"); - testRequest(URI.create("http://foo"), "/zone/v2/"); + exception.expectMessage("Request path '/path' does not end with proxy path '/zone/v2/'"); + testRequest("http://domain.tld/path", "/zone/v2/"); } @Test - public void testProxyRequest() throws Exception { - ProxyRequest proxyRequest = testRequest(URI.create("http://foo/zone/v2/foo/bar/bla/bla/v1/something"), - "/zone/v2/"); - assertEquals("foo", proxyRequest.getEnvironment()); - assertEquals("/bla/bla/v1/something", proxyRequest.getConfigServerRequest()); - } + public void testUris() throws Exception { + { + // Root request + ProxyRequest request = testRequest("http://controller.domain.tld/my/path", ""); + assertEquals(URI.create("http://controller.domain.tld/my/path/"), request.getControllerPrefixUri()); + assertEquals(URI.create("https://cfg.prod.us-north-1.domain.tld:1234/"), + request.createConfigServerRequestUri(URI.create("https://cfg.prod.us-north-1.domain.tld:1234/"))); + } - @Test - public void testProxyRequestWithParameters() throws Exception { - ProxyRequest proxyRequest = testRequest(URI.create("http://foo/zone/v2/foo/bar/something?p=v&q=y"), - "/zone/v2/"); - assertEquals("foo", proxyRequest.getEnvironment()); - assertEquals("/something?p=v&q=y", proxyRequest.getConfigServerRequest()); - } + { + // Root request with trailing / + ProxyRequest request = testRequest("http://controller.domain.tld/my/path/", "/"); + assertEquals(URI.create("http://controller.domain.tld/my/path/"), request.getControllerPrefixUri()); + assertEquals(URI.create("https://cfg.prod.us-north-1.domain.tld:1234/"), + request.createConfigServerRequestUri(URI.create("https://cfg.prod.us-north-1.domain.tld:1234/"))); + } - private static ProxyRequest testRequest(URI url, String pathPrefix) throws IOException, ProxyException { - return new ProxyRequest(url, headers("controller:49152"), null, "GET", pathPrefix); - } + { + // API path test + ProxyRequest request = testRequest("http://controller.domain.tld:1234/my/path/nodes/v2", "/nodes/v2"); + assertEquals(URI.create("http://controller.domain.tld:1234/my/path/"), request.getControllerPrefixUri()); + assertEquals(URI.create("https://cfg.prod.us-north-1.domain.tld/nodes/v2"), + request.createConfigServerRequestUri(URI.create("https://cfg.prod.us-north-1.domain.tld"))); + } - private static Map<String, List<String>> headers(String hostPort) { - Map<String, List<String>> headers = new HashMap<>(); - headers.put("host", Collections.singletonList(hostPort)); - return Collections.unmodifiableMap(headers); + { + // API path test with query + ProxyRequest request = testRequest("http://controller.domain.tld:1234/my/path/nodes/v2/?some=thing", "/nodes/v2/"); + assertEquals(URI.create("http://controller.domain.tld:1234/my/path/"), request.getControllerPrefixUri()); + assertEquals(URI.create("https://cfg.prod.us-north-1.domain.tld/nodes/v2/?some=thing"), + request.createConfigServerRequestUri(URI.create("https://cfg.prod.us-north-1.domain.tld"))); + } } + private static ProxyRequest testRequest(String url, String pathPrefix) throws ProxyException { + return new ProxyRequest( + HttpRequest.Method.GET, URI.create(url), Map.of(), null, ZoneId.from("dev", "us-north-1"), pathPrefix); + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java index 8dbd1c4ef61..efe7e17c58e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java @@ -1,15 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.proxy; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.jdisc.http.HttpRequest; import org.junit.Test; import java.io.ByteArrayOutputStream; import java.net.URI; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; +import java.nio.charset.StandardCharsets; import java.util.Map; -import java.util.Optional; import static org.junit.Assert.assertEquals; @@ -20,50 +19,37 @@ public class ProxyResponseTest { @Test public void testRewriteUrl() throws Exception { - String controllerPrefix = "/zone/v2/"; - URI configServer = URI.create("http://configserver:1234"); - ProxyRequest request = new ProxyRequest(URI.create("http://foo/zone/v2/env/region/configserver"), - headers("controller:49152"), null, "GET", - controllerPrefix); + ProxyRequest request = new ProxyRequest(HttpRequest.Method.GET, URI.create("http://domain.tld/zone/v2/dev/us-north-1/configserver"), + Map.of(), null, ZoneId.from("dev", "us-north-1"), "configserver"); ProxyResponse proxyResponse = new ProxyResponse( request, "response link is http://configserver:1234/bla/bla/", 200, - Optional.of(configServer), + URI.create("http://configserver:1234"), "application/json"); ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); proxyResponse.render(outputStream); - String document = new String(outputStream.toByteArray(),"UTF-8"); - assertEquals("response link is http://controller:49152/zone/v2/env/region/bla/bla/", document); + String document = new String(outputStream.toByteArray(), StandardCharsets.UTF_8); + assertEquals("response link is http://domain.tld/zone/v2/dev/us-north-1/bla/bla/", document); } @Test public void testRewriteSecureUrl() throws Exception { - String controllerPrefix = "/zone/v2/"; - URI configServer = URI.create("http://configserver:1234"); - ProxyRequest request = new ProxyRequest(URI.create("https://foo/zone/v2/env/region/configserver"), - headers("controller:49152"), null, "GET", - controllerPrefix); + ProxyRequest request = new ProxyRequest(HttpRequest.Method.GET, URI.create("https://domain.tld/zone/v2/prod/eu-south-3/configserver"), + Map.of(), null, ZoneId.from("prod", "eu-south-3"), "configserver"); ProxyResponse proxyResponse = new ProxyResponse( request, "response link is http://configserver:1234/bla/bla/", 200, - Optional.of(configServer), + URI.create("http://configserver:1234"), "application/json"); ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); proxyResponse.render(outputStream); - String document = new String(outputStream.toByteArray(),"UTF-8"); - assertEquals("response link is https://controller:49152/zone/v2/env/region/bla/bla/", document); + String document = new String(outputStream.toByteArray(), StandardCharsets.UTF_8); + assertEquals("response link is https://domain.tld/zone/v2/prod/eu-south-3/bla/bla/", document); } - - private static Map<String, List<String>> headers(String hostPort) { - Map<String, List<String>> headers = new HashMap<>(); - headers.put("host", Collections.singletonList(hostPort)); - return Collections.unmodifiableMap(headers); - } - } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiTest.java index 19061b61431..33ea538e9b6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiTest.java @@ -6,12 +6,14 @@ import com.yahoo.application.container.handler.Request.Method; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.zone.ZoneApi; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.text.Utf8; import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzUser; import com.yahoo.vespa.hosted.controller.integration.ConfigServerProxyMock; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; +import com.yahoo.vespa.hosted.controller.proxy.ProxyRequest; import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import org.junit.Before; @@ -59,53 +61,33 @@ public class ZoneApiTest extends ControllerContainerTest { // GET /zone/v2/prod/us-north-1 tester.containerTester().assertResponse(authenticatedRequest("http://localhost:8080/zone/v2/prod/us-north-1"), "ok"); - assertEquals("prod", proxy.lastReceived().get().getEnvironment()); - assertEquals("us-north-1", proxy.lastReceived().get().getRegion()); - assertEquals("", proxy.lastReceived().get().getConfigServerRequest()); - assertEquals("GET", proxy.lastReceived().get().getMethod()); + assertLastRequest(ZoneId.from("prod", "us-north-1"), "GET"); // GET /zone/v2/nodes/v2/node/?recursive=true tester.containerTester().assertResponse(authenticatedRequest("http://localhost:8080/zone/v2/prod/us-north-1/nodes/v2/node/?recursive=true"), "ok"); - - assertEquals("prod", proxy.lastReceived().get().getEnvironment()); - assertEquals("us-north-1", proxy.lastReceived().get().getRegion()); - assertEquals("/nodes/v2/node/?recursive=true", proxy.lastReceived().get().getConfigServerRequest()); - assertEquals("GET", proxy.lastReceived().get().getMethod()); + assertLastRequest(ZoneId.from("prod", "us-north-1"), "GET"); // POST /zone/v2/dev/us-north-2/nodes/v2/command/restart?hostname=node1 tester.containerTester().assertResponse(hostedOperatorRequest("http://localhost:8080/zone/v2/dev/us-north-2/nodes/v2/command/restart?hostname=node1", new byte[0], Method.POST), "ok"); - assertEquals("dev", proxy.lastReceived().get().getEnvironment()); - assertEquals("us-north-2", proxy.lastReceived().get().getRegion()); - assertEquals("/nodes/v2/command/restart?hostname=node1", proxy.lastReceived().get().getConfigServerRequest()); - assertEquals("POST", proxy.lastReceived().get().getMethod()); // PUT /zone/v2/prod/us-north-1/nodes/v2/state/dirty/node1 tester.containerTester().assertResponse(hostedOperatorRequest("http://localhost:8080/zone/v2/prod/us-north-1/nodes/v2/state/dirty/node1", new byte[0], Method.PUT), "ok"); - assertEquals("prod", proxy.lastReceived().get().getEnvironment()); - assertEquals("us-north-1", proxy.lastReceived().get().getRegion()); - assertEquals("/nodes/v2/state/dirty/node1", proxy.lastReceived().get().getConfigServerRequest()); - assertEquals("PUT", proxy.lastReceived().get().getMethod()); + assertLastRequest(ZoneId.from("prod", "us-north-1"), "PUT"); // DELETE /zone/v2/prod/us-north-1/nodes/v2/node/node1 tester.containerTester().assertResponse(hostedOperatorRequest("http://localhost:8080/zone/v2/prod/us-north-1/nodes/v2/node/node1", new byte[0], Method.DELETE), "ok"); - assertEquals("prod", proxy.lastReceived().get().getEnvironment()); - assertEquals("us-north-1", proxy.lastReceived().get().getRegion()); - assertEquals("/nodes/v2/node/node1", proxy.lastReceived().get().getConfigServerRequest()); - assertEquals("DELETE", proxy.lastReceived().get().getMethod()); + assertLastRequest(ZoneId.from("prod", "us-north-1"), "DELETE"); // PATCH /zone/v2/prod/us-north-1/nodes/v2/node/node1 tester.containerTester().assertResponse(hostedOperatorRequest("http://localhost:8080/zone/v2/prod/us-north-1/nodes/v2/node/node1", Utf8.toBytes("{\"currentRestartGeneration\": 1}"), Method.PATCH), "ok"); - assertEquals("prod", proxy.lastReceived().get().getEnvironment()); - assertEquals("us-north-1", proxy.lastReceived().get().getRegion()); - assertEquals("/nodes/v2/node/node1", proxy.lastReceived().get().getConfigServerRequest()); - assertEquals("PATCH", proxy.lastReceived().get().getMethod()); + assertLastRequest(ZoneId.from("prod", "us-north-1"), "PATCH"); assertEquals("{\"currentRestartGeneration\": 1}", proxy.lastRequestBody().get()); assertFalse("Actions are logged to audit log", tester.controller().auditLogger().readLog().entries().isEmpty()); @@ -120,6 +102,12 @@ public class ZoneApiTest extends ControllerContainerTest { assertFalse(proxy.lastReceived().isPresent()); } + private void assertLastRequest(ZoneId zoneId, String method) { + ProxyRequest last = proxy.lastReceived().orElseThrow(); + assertEquals(zoneId, last.getZoneId()); + assertEquals(com.yahoo.jdisc.http.HttpRequest.Method.valueOf(method), last.getMethod()); + } + private static Request hostedOperatorRequest(String uri, byte[] body, Request.Method method) { return addIdentityToRequest(new Request(uri, body, method), HOSTED_VESPA_OPERATOR); } |