diff options
author | Jon Marius Venstad <jonmv@gmail.com> | 2022-04-06 19:35:30 +0200 |
---|---|---|
committer | Jon Marius Venstad <jonmv@gmail.com> | 2022-04-06 19:35:30 +0200 |
commit | 039589faf5f989d80b9fec2b28ed955ac6fd86f6 (patch) | |
tree | 45c314cc9ede2d5c26a5d6b4f030ad3db2246a91 /controller-server/src | |
parent | ec92b5f8882e400f94b851dffcf0b3511373e890 (diff) |
Use HttpURL.Path for Path.getRest()
Diffstat (limited to 'controller-server/src')
12 files changed, 102 insertions, 108 deletions
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 2a1b8a19475..7e1c9c8884f 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 @@ -3,6 +3,9 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.HttpURL.Path; +import com.yahoo.restapi.HttpURL.Query; import com.yahoo.text.Text; import java.io.InputStream; import java.net.URI; @@ -22,28 +25,27 @@ import static com.yahoo.jdisc.http.HttpRequest.Method; public class ProxyRequest { private final Method method; - private final URI requestUri; + private final HttpURL requestUri; private final Map<String, List<String>> headers; private final InputStream requestData; private final List<URI> targets; - private final String targetPath; + private final Path targetPath; - ProxyRequest(Method method, URI url, Map<String, List<String>> headers, InputStream body, List<URI> targets, - String path) { - Objects.requireNonNull(url); - if (!url.getPath().endsWith(path)) { - throw new IllegalArgumentException(Text.format("Request path '%s' does not end with proxy path '%s'", url.getPath(), path)); + ProxyRequest(Method method, URI uri, Map<String, List<String>> headers, InputStream body, List<URI> targets, Path path) { + this.requestUri = HttpURL.from(uri); + if ( requestUri.path().segments().size() < path.segments().size() + || ! requestUri.path().tail(path.segments().size()).equals(path)) { + throw new IllegalArgumentException(Text.format("Request %s does not end with proxy %s", requestUri.path(), path)); } if (targets.isEmpty()) { throw new IllegalArgumentException("targets must be non-empty"); } this.method = Objects.requireNonNull(method); - this.requestUri = Objects.requireNonNull(url); this.headers = Objects.requireNonNull(headers); this.requestData = body; this.targets = List.copyOf(targets); - this.targetPath = path.startsWith("/") ? path : "/" + path; + this.targetPath = path; } @@ -64,24 +66,12 @@ public class ProxyRequest { } public URI createConfigServerRequestUri(URI baseURI) { - try { - return new URI(baseURI.getScheme(), baseURI.getUserInfo(), baseURI.getHost(), - baseURI.getPort(), targetPath, requestUri.getQuery(), requestUri.getFragment()); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } + return HttpURL.from(baseURI).withPath(targetPath).withQuery(requestUri.query()).asURI(); } public URI getControllerPrefixUri() { - String prefixPath = targetPath.equals("/") && !requestUri.getPath().endsWith("/") ? - requestUri.getPath() + targetPath : - requestUri.getPath().substring(0, requestUri.getPath().length() - targetPath.length() + 1); - try { - return new URI(requestUri.getScheme(), requestUri.getUserInfo(), requestUri.getHost(), - requestUri.getPort(), prefixPath, null, null); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } + Path prefixPath = requestUri.path().cut(targetPath.segments().size()).withTrailingSlash(); + return requestUri.withPath(prefixPath).withQuery(Query.empty()).asURI(); } @Override @@ -90,7 +80,7 @@ public class ProxyRequest { } /** Create a proxy request that repeatedly tries a single target */ - public static ProxyRequest tryOne(URI target, String path, HttpRequest request) { + public static ProxyRequest tryOne(URI target, Path path, HttpRequest request) { return new ProxyRequest(request.getMethod(), request.getUri(), request.getJDiscRequest().headers(), request.getData(), List.of(target), path); } 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 0b629a577d0..9ac30898f8b 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 @@ -2,6 +2,8 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.HttpURL.Path; import org.apache.http.client.utils.URIBuilder; import java.io.IOException; @@ -29,19 +31,8 @@ public class ProxyResponse extends HttpResponse { super(statusResponse); this.contentType = contentType; - final String configServerPrefix; - final String controllerRequestPrefix; - try { - configServerPrefix = new URIBuilder() - .setScheme(configServer.getScheme()) - .setHost(configServer.getHost()) - .setPort(configServer.getPort()) - .setPath("/") - .build().toString(); - controllerRequestPrefix = controllerRequest.getControllerPrefixUri().toString(); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } + String configServerPrefix = HttpURL.from(configServer).withPath(Path.empty()).asURI().toString(); + String controllerRequestPrefix = controllerRequest.getControllerPrefixUri().toString(); bodyResponseRewritten = bodyResponse.replace(configServerPrefix, controllerRequestPrefix); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index d9a38a5b578..c2c4049bcc3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.restapi.application; import ai.vespa.hosted.api.Signatures; +import ai.vespa.validation.Validation; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Joiner; @@ -25,8 +26,10 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; import com.yahoo.io.IOUtils; +import com.yahoo.net.DomainName; import com.yahoo.restapi.ByteArrayResponse; import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.HttpURL; import com.yahoo.restapi.MessageResponse; import com.yahoo.restapi.Path; import com.yahoo.restapi.ResourceResponse; @@ -155,6 +158,7 @@ import java.util.logging.Level; import java.util.stream.Collectors; import java.util.stream.Stream; +import static ai.vespa.validation.Validation.requireAtLeast; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.CONFLICT; import static com.yahoo.yolean.Exceptions.uncheck; @@ -270,7 +274,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindexing")) return getReindexing(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/suspended")) return suspended(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service")) return services(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/state/v1/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/{host}/status/{*}")) return status(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.get("host"), path.getRest(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/nodes")) return nodes(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/clusters")) return clusters(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/content/{*}")) return content(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.getRest(), request); @@ -284,7 +289,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deployment(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/suspended")) return suspended(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service")) return services(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/state/v1/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/{host}/status/{*}")) return status(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.get("host"), path.getRest(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/nodes")) return nodes(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/clusters")) return clusters(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/logs")) return logs(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request.propertyMap()); @@ -1857,40 +1863,29 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { return response; } - private HttpResponse service(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, String restPath, HttpRequest request) { + private HttpResponse status(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, String host, HttpURL.Path restPath, HttpRequest request) { DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); + String result = controller.serviceRegistry().configServer().getServiceStatusPage(deploymentId, + serviceName, + DomainName.of(host), + restPath); // TODO: add query + return new HtmlResponse(result); + } - if (restPath.contains("/status/")) { - String[] parts = restPath.split("/status/", 2); - String result = controller.serviceRegistry().configServer().getServiceStatusPage(deploymentId, serviceName, parts[0], parts[1]); - return new HtmlResponse(result); - } - - String normalizedRestPath = URI.create(restPath).normalize().toString(); - // Only state/v1 is allowed - if (! normalizedRestPath.startsWith("state/v1/")) { - return ErrorResponse.forbidden("Access denied"); - } - + private HttpResponse service(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, HttpURL.Path restPath, HttpRequest request) { + DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); Map<?,?> result = controller.serviceRegistry().configServer().getServiceApiResponse(deploymentId, serviceName, restPath); ServiceApiResponse response = new ServiceApiResponse(deploymentId.zoneId(), deploymentId.applicationId(), List.of(controller.zoneRegistry().getConfigServerVipUri(deploymentId.zoneId())), request.getUri()); - response.setResponse(result, serviceName, restPath); + response.setResponse(result, serviceName, HttpURL.Path.parse("/state/v1").append(restPath)); return response; } - private HttpResponse content(String tenantName, String applicationName, String instanceName, String environment, String region, String restPath, HttpRequest request) { + private HttpResponse content(String tenantName, String applicationName, String instanceName, String environment, String region, HttpURL.Path restPath, HttpRequest request) { DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); - - String normalizedRestPath = URI.create("content/" + restPath).normalize().toString(); - // Only content/ is allowed - if ( ! normalizedRestPath.startsWith("content/")) { - return ErrorResponse.forbidden("Access denied"); - } - - return controller.serviceRegistry().configServer().getApplicationPackageContent(deploymentId, "/" + restPath, request.getUri()); + return controller.serviceRegistry().configServer().getApplicationPackageContent(deploymentId, restPath, request.getUri()); } private HttpResponse updateTenant(String tenantName, HttpRequest request) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java index 888c402b6ec..a21f93bdaea 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java @@ -4,6 +4,9 @@ package com.yahoo.vespa.hosted.controller.restapi.application; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.HttpURL.Path; +import com.yahoo.restapi.HttpURL.Query; import com.yahoo.slime.Cursor; import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; @@ -34,11 +37,11 @@ class ServiceApiResponse extends HttpResponse { private final ApplicationId application; private final List<URI> configServerURIs; private final Slime slime; - private final UriBuilder requestUri; + private final HttpURL requestUri; // Only set for one of the setResponse calls private String serviceName = null; - private String restPath = null; + private Path restPath = null; public ServiceApiResponse(ZoneId zone, ApplicationId application, List<URI> configServerURIs, URI requestUri) { super(200); @@ -46,7 +49,7 @@ class ServiceApiResponse extends HttpResponse { this.application = application; this.configServerURIs = configServerURIs; this.slime = new Slime(); - this.requestUri = new UriBuilder(requestUri).withoutParameters(); + this.requestUri = HttpURL.from(requestUri).withQuery(Query.empty()); } public void setResponse(ApplicationView applicationView) { @@ -68,7 +71,7 @@ class ServiceApiResponse extends HttpResponse { } } - public void setResponse(Map<?,?> responseData, String serviceName, String restPath) { + public void setResponse(Map<?,?> responseData, String serviceName, Path restPath) { this.serviceName = serviceName; this.restPath = restPath; mapToSlime(responseData, slime.setObject()); @@ -138,7 +141,7 @@ class ServiceApiResponse extends HttpResponse { mapToSlime((Map)entry, array.addObject()); } - private String rewriteIfUrl(String urlOrAnyString, UriBuilder requestUri) { + private String rewriteIfUrl(String urlOrAnyString, HttpURL requestUri) { if (urlOrAnyString == null) return null; String hostPattern = "(" + @@ -163,19 +166,18 @@ class ServiceApiResponse extends HttpResponse { if (matcher.find()) { String proxiedPath = urlOrAnyString.substring(matcher.group().length()); - return requestUri.append(proxiedPath).toString(); + return requestUri.withPath(requestUri.path().append(Path.parse(proxiedPath))).asURI().toString(); } else { return urlOrAnyString; // not a service url } } - private UriBuilder generateLocalLinkPrefix(String identifier, String restPath) { - String proxiedPath = identifier + "/" + restPath; - - if (this.requestUri.toString().endsWith(proxiedPath)) { - return new UriBuilder(this.requestUri.toString().substring(0, this.requestUri.toString().length() - proxiedPath.length())); + private HttpURL generateLocalLinkPrefix(String identifier, Path restPath) { + Path proxiedPath = Path.parse(identifier).append(restPath); + if (requestUri.path().tail(proxiedPath.segments().size()).equals(proxiedPath)) { + return requestUri.withPath(requestUri.path().cut(proxiedPath.segments().size())); } else { - throw new IllegalStateException("Expected the resource path '" + this.requestUri + "' to end with '" + proxiedPath + "'"); + throw new IllegalStateException("Expected the resource " + requestUri.path() + " to end with " + proxiedPath); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java index 0e09825ec41..27a8cbeaf3e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.zone.ZoneList; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.HttpURL; import com.yahoo.restapi.Path; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; @@ -21,8 +22,11 @@ import com.yahoo.yolean.Exceptions; import java.net.URI; import java.util.List; import java.util.logging.Level; +import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.yahoo.restapi.HttpURL.Path.parse; + /** * REST API for proxying operator APIs to config servers in a given zone. * @@ -32,7 +36,9 @@ import java.util.stream.Stream; public class ConfigServerApiHandler extends AuditLoggingRequestHandler { private static final URI CONTROLLER_URI = URI.create("https://localhost:4443/"); - private static final List<String> WHITELISTED_APIS = List.of("/flags/v1/", "/nodes/v2/", "/orchestrator/v1/"); + private static final List<HttpURL.Path> WHITELISTED_APIS = List.of(parse("/flags/v1/"), + parse("/nodes/v2/"), + parse("/orchestrator/v1/")); private final ZoneRegistry zoneRegistry; private final ConfigServerRestExecutor proxy; @@ -84,17 +90,18 @@ public class ConfigServerApiHandler extends AuditLoggingRequestHandler { } ZoneId zoneId = ZoneId.from(path.get("environment"), path.get("region")); - if (! zoneRegistry.hasZone(zoneId) && ! controllerZone.equals(zoneId)) { + if ( ! zoneRegistry.hasZone(zoneId) && ! controllerZone.equals(zoneId)) { throw new IllegalArgumentException("No such zone: " + zoneId.value()); } - String cfgPath = "/" + path.getRest(); - if (WHITELISTED_APIS.stream().noneMatch(cfgPath::startsWith)) { - return ErrorResponse.forbidden("Cannot access '" + cfgPath + - "' through /configserver/v1, following APIs are permitted: " + String.join(", ", WHITELISTED_APIS)); + if (path.getRest().segments().size() < 2 || ! WHITELISTED_APIS.contains(path.getRest().head(2).withTrailingSlash())) { + return ErrorResponse.forbidden("Cannot access " + path.getRest() + + " through /configserver/v1, following APIs are permitted: " + WHITELISTED_APIS.stream() + .map(p -> "/" + String.join("/", p.segments()) + "/") + .collect(Collectors.joining(", "))); } - return proxy.handle(ProxyRequest.tryOne(getEndpoint(zoneId), cfgPath, request)); + return proxy.handle(ProxyRequest.tryOne(getEndpoint(zoneId), path.getRest(), request)); } private HttpResponse root(HttpRequest request) { 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 a7416cfa0ed..40a402f209b 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 @@ -7,6 +7,7 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.HttpURL; import com.yahoo.restapi.Path; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; @@ -107,7 +108,7 @@ public class ZoneApiHandler extends AuditLoggingRequestHandler { return ErrorResponse.notFoundError("Nothing at " + path); } - private ProxyRequest proxyRequest(ZoneId zoneId, String path, HttpRequest request) { + private ProxyRequest proxyRequest(ZoneId zoneId, HttpURL.Path path, HttpRequest request) { return ProxyRequest.tryOne(zoneRegistry.getConfigServerVipUri(zoneId), path, request); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index e0ce1c060bc..1643f1f818b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -13,6 +13,8 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.net.DomainName; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.text.Text; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; @@ -521,7 +523,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer // Returns a canned example response @Override - public Map<?,?> getServiceApiResponse(DeploymentId deployment, String serviceName, String restPath) { + public Map<?,?> getServiceApiResponse(DeploymentId deployment, String serviceName, Path restPath) { Map<String,List<?>> root = new HashMap<>(); List<Map<?,?>> resources = new ArrayList<>(); Map<String,String> resource = new HashMap<>(); @@ -532,7 +534,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } @Override - public String getServiceStatusPage(DeploymentId deployment, String serviceName, String node, String subPath) { + public String getServiceStatusPage(DeploymentId deployment, String serviceName, DomainName node, Path subPath) { return "<h1>OK</h1>"; } @@ -567,8 +569,8 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } @Override - public ProxyResponse getApplicationPackageContent(DeploymentId deployment, String path, URI requestUri) { - return new ProxyResponse(("{\"path\":\"" + path + "\"}").getBytes(StandardCharsets.UTF_8), "application/json", 200); + public ProxyResponse getApplicationPackageContent(DeploymentId deployment, Path path, URI requestUri) { + return new ProxyResponse(("{\"path\":\"/" + String.join("/", path.segments()) + "\"}").getBytes(StandardCharsets.UTF_8), "application/json", 200); } public void setLogStream(Supplier<String> log) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java index 0b28d94386d..e10a41cbae3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java @@ -6,6 +6,7 @@ import com.github.tomakehurst.wiremock.stubbing.Scenario; import com.yahoo.config.provision.SystemName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.yolean.concurrent.Sleeper; import org.apache.http.protocol.HttpContext; @@ -46,7 +47,7 @@ public class ConfigServerRestExecutorImplTest { stubRequests(path); HttpRequest request = HttpRequest.createTestRequest(url.toString(), com.yahoo.jdisc.http.HttpRequest.Method.GET); - ProxyRequest proxyRequest = ProxyRequest.tryOne(url, path, request); + ProxyRequest proxyRequest = ProxyRequest.tryOne(url, Path.parse(path), request); // Request is retried HttpResponse response = proxy.handle(proxyRequest); @@ -71,7 +72,7 @@ public class ConfigServerRestExecutorImplTest { stubRequests(path); HttpRequest request = HttpRequest.createTestRequest(url.toString(), com.yahoo.jdisc.http.HttpRequest.Method.GET); - ProxyRequest proxyRequest = ProxyRequest.tryOne(url, path, request); + ProxyRequest proxyRequest = ProxyRequest.tryOne(url, Path.parse(path), request); // Connections are reused assertEquals(200, proxy.handle(proxyRequest).getStatus()); 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 8542530628c..e8b5df7efa1 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.jdisc.http.HttpRequest; +import com.yahoo.restapi.HttpURL.Path; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -11,20 +12,18 @@ import java.util.List; import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; /** * @author Haakon Dybdahl */ public class ProxyRequestTest { - @SuppressWarnings("deprecation") - @Rule - public final ExpectedException exception = ExpectedException.none(); - @Test public void testBadUri() { - exception.expectMessage("Request path '/path' does not end with proxy path '/zone/v2/'"); - testRequest("http://domain.tld/path", "/zone/v2/"); + assertThrows("Request path '/path' does not end with proxy path '/zone/v2/'", + IllegalArgumentException.class, + () -> testRequest("http://domain.tld/path", "/zone/v2/")); } @Test @@ -33,7 +32,7 @@ public class ProxyRequestTest { // 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/"), + 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/"))); } @@ -64,7 +63,7 @@ public class ProxyRequestTest { private static ProxyRequest testRequest(String url, String pathPrefix) { return new ProxyRequest(HttpRequest.Method.GET, URI.create(url), Map.of(), null, - List.of(URI.create("http://example.com")), pathPrefix); + List.of(URI.create("http://example.com")), Path.parse(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 4cc2c1cc79e..1ba0200eec3 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.jdisc.http.HttpRequest; +import com.yahoo.restapi.HttpURL.Path; import org.junit.Test; import java.io.ByteArrayOutputStream; @@ -20,7 +21,7 @@ public class ProxyResponseTest { @Test public void testRewriteUrl() throws Exception { ProxyRequest request = new ProxyRequest(HttpRequest.Method.GET, URI.create("http://domain.tld/zone/v2/dev/us-north-1/configserver"), - Map.of(), null, List.of(URI.create("http://example.com")), "configserver"); + Map.of(), null, List.of(URI.create("http://example.com")), Path.parse("configserver")); ProxyResponse proxyResponse = new ProxyResponse( request, "response link is http://configserver:1234/bla/bla/", @@ -31,14 +32,14 @@ public class ProxyResponseTest { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); proxyResponse.render(outputStream); - String document = new String(outputStream.toByteArray(), StandardCharsets.UTF_8); + String document = outputStream.toString(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 { ProxyRequest request = new ProxyRequest(HttpRequest.Method.GET, URI.create("https://domain.tld/zone/v2/prod/eu-south-3/configserver"), - Map.of(), null, List.of(URI.create("http://example.com")), "configserver"); + Map.of(), null, List.of(URI.create("http://example.com")), Path.parse("configserver")); ProxyResponse proxyResponse = new ProxyResponse( request, "response link is http://configserver:1234/bla/bla/", @@ -49,7 +50,7 @@ public class ProxyResponseTest { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); proxyResponse.render(outputStream); - String document = new String(outputStream.toByteArray(), StandardCharsets.UTF_8); + String document = outputStream.toString(StandardCharsets.UTF_8); assertEquals("response link is https://domain.tld/zone/v2/prod/eu-south-3/bla/bla/", document); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 581c0160640..9736745d257 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -1657,15 +1657,15 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request(serviceApi + "/storagenode-awe3slno6mmq2fye191y324jl/document/v1/", GET) .userIdentity(USER_ID) .oAuthCredentials(OKTA_CREDENTIALS), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Access denied\"}", - 403); + "{\"error-code\":\"NOT_FOUND\",\"message\":\"Nothing at path '/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/service/storagenode-awe3slno6mmq2fye191y324jl/document/v1/'\"}", + 404); // Test path traversal tester.assertResponse(request(serviceApi + "/storagenode-awe3slno6mmq2fye191y324jl/state/v1/../../document/v1/", GET) .userIdentity(USER_ID) .oAuthCredentials(OKTA_CREDENTIALS), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Access denied\"}", - 403); + "{\"error-code\":\"NOT_FOUND\",\"message\":\"Nothing at path '/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/service/storagenode-awe3slno6mmq2fye191y324jl/document/v1/'\"}", + 404); // Test urlencoded path traversal tester.assertResponse(request(serviceApi + "/storagenode-awe3slno6mmq2fye191y324jl/state%2Fv1%2F..%2F..%2Fdocument%2Fv1%2F", GET) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java index 9d0054f327a..ce6f713a13d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java @@ -50,6 +50,11 @@ public class ConfigServerApiHandlerTest extends ControllerContainerTest { tester.assertResponse(operatorRequest("http://localhost:8080/configserver/v1"), new File("root.json")); + // GET /configserver/v1/nodes/v2 + tester.assertResponse(operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2"), + "ok"); + assertLastRequest("https://cfg.prod.us-north-1.test.vip:4443/", "GET"); + // GET /configserver/v1/nodes/v2/node/?recursive=true tester.assertResponse(operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2/node/?recursive=true"), "ok"); @@ -83,12 +88,12 @@ public class ConfigServerApiHandlerTest extends ControllerContainerTest { @Test public void test_allowed_apis() { // GET /configserver/v1/prod/us-north-1 - tester.assertResponse(() -> operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1"), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access '/' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}", + tester.assertResponse(() -> operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/"), + "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access path '/' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}", 403); tester.assertResponse(() -> operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/application/v2/tenant/vespa"), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access '/application/v2/tenant/vespa' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}", + "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access path '/application/v2/tenant/vespa' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}", 403); } |