diff options
7 files changed, 27 insertions, 120 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutor.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutor.java index 529acc48cbe..110d994c179 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutor.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutor.java @@ -10,5 +10,7 @@ import com.yahoo.container.jdisc.HttpResponse; * @author Haakon Dybdahl */ public interface ConfigServerRestExecutor { - HttpResponse handle(ProxyRequest proxyRequest) throws ProxyException; + + HttpResponse handle(ProxyRequest request); + } 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 49a23a8337b..fcfc4eca32b 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 @@ -74,8 +74,8 @@ public class ConfigServerRestExecutorImpl extends AbstractComponent implements C } @Override - public ProxyResponse handle(ProxyRequest proxyRequest) throws ProxyException { - List<URI> targets = new ArrayList<>(proxyRequest.getTargets()); + public ProxyResponse handle(ProxyRequest request) { + List<URI> targets = new ArrayList<>(request.getTargets()); StringBuilder errorBuilder = new StringBuilder(); boolean singleTarget = targets.size() == 1; @@ -88,7 +88,7 @@ public class ConfigServerRestExecutorImpl extends AbstractComponent implements C } for (URI url : targets) { - Optional<ProxyResponse> proxyResponse = proxyCall(url, proxyRequest, errorBuilder); + Optional<ProxyResponse> proxyResponse = proxy(request, url, errorBuilder); if (proxyResponse.isPresent()) { return proxyResponse.get(); } @@ -97,26 +97,24 @@ public class ConfigServerRestExecutorImpl extends AbstractComponent implements C } } - throw new ProxyException(ErrorResponse.internalServerError("Failed talking to config servers: " - + errorBuilder.toString())); + throw new RuntimeException("Failed talking to config servers: " + errorBuilder.toString()); } - private Optional<ProxyResponse> proxyCall(URI uri, ProxyRequest proxyRequest, StringBuilder errorBuilder) - throws ProxyException { + private Optional<ProxyResponse> proxy(ProxyRequest request, URI url, StringBuilder errorBuilder) { HttpRequestBase requestBase = createHttpBaseRequest( - proxyRequest.getMethod(), proxyRequest.createConfigServerRequestUri(uri), proxyRequest.getData()); + request.getMethod(), request.createConfigServerRequestUri(url), request.getData()); // Empty list of headers to copy for now, add headers when needed, or rewrite logic. - copyHeaders(proxyRequest.getHeaders(), requestBase); + copyHeaders(request.getHeaders(), requestBase); try (CloseableHttpResponse response = client.execute(requestBase)) { String content = getContent(response); int status = response.getStatusLine().getStatusCode(); if (status / 100 == 5) { - errorBuilder.append("Talking to server ").append(uri.getHost()); + errorBuilder.append("Talking to server ").append(url.getHost()); errorBuilder.append(", got ").append(status).append(" ") .append(content).append("\n"); LOG.log(Level.FINE, () -> String.format("Got response from %s with status code %d and content:\n %s", - uri.getHost(), status, content)); + url.getHost(), status, content)); return Optional.empty(); } Header contentHeader = response.getLastHeader("Content-Type"); @@ -127,11 +125,11 @@ public class ConfigServerRestExecutorImpl extends AbstractComponent implements C contentType = "application/json"; } // Send response back - return Optional.of(new ProxyResponse(proxyRequest, content, status, uri, contentType)); + return Optional.of(new ProxyResponse(request, content, status, url, contentType)); } catch (Exception e) { - errorBuilder.append("Talking to server ").append(uri.getHost()); + errorBuilder.append("Talking to server ").append(url.getHost()); errorBuilder.append(" got exception ").append(e.getMessage()); - LOG.log(Level.FINE, e, () -> "Got exception while sending request to " + uri.getHost()); + LOG.log(Level.FINE, e, () -> "Got exception while sending request to " + url.getHost()); return Optional.empty(); } } @@ -142,33 +140,32 @@ public class ConfigServerRestExecutorImpl extends AbstractComponent implements C .orElse(""); } - private static HttpRequestBase createHttpBaseRequest(Method method, URI uri, InputStream data) throws ProxyException { + private static HttpRequestBase createHttpBaseRequest(Method method, URI url, InputStream data) { switch (method) { case GET: - return new HttpGet(uri); + return new HttpGet(url); case POST: - HttpPost post = new HttpPost(uri); + HttpPost post = new HttpPost(url); if (data != null) { post.setEntity(new InputStreamEntity(data)); } return post; case PUT: - HttpPut put = new HttpPut(uri); + HttpPut put = new HttpPut(url); if (data != null) { put.setEntity(new InputStreamEntity(data)); } return put; case DELETE: - return new HttpDelete(uri); + return new HttpDelete(url); case PATCH: - HttpPatch patch = new HttpPatch(uri); + HttpPatch patch = new HttpPatch(url); if (data != null) { patch.setEntity(new InputStreamEntity(data)); } return patch; - default: - throw new ProxyException(ErrorResponse.methodNotAllowed("Will not proxy such calls.")); } + throw new IllegalArgumentException("Refusing to proxy " + method + " " + url + ": Unsupported method"); } private static void copyHeaders(Map<String, List<String>> headers, HttpRequestBase toRequest) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ErrorResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ErrorResponse.java deleted file mode 100644 index 3673c0227a3..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ErrorResponse.java +++ /dev/null @@ -1,66 +0,0 @@ -// 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.container.jdisc.HttpResponse; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.JsonFormat; -import com.yahoo.slime.Slime; - -import java.io.IOException; -import java.io.OutputStream; - -import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; -import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; -import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED; -import static com.yahoo.jdisc.Response.Status.NOT_FOUND; - -/** - * Class for generating error responses. - * - * @author Haakon Dybdahl - */ -public class ErrorResponse extends HttpResponse { - - private final Slime slime = new Slime(); - public final String message; - - public ErrorResponse(int code, String errorType, String message) { - super(code); - this.message = message; - Cursor root = slime.setObject(); - root.setString("error-code", errorType); - root.setString("message", message); - } - - public enum errorCodes { - NOT_FOUND, - BAD_REQUEST, - METHOD_NOT_ALLOWED, - INTERNAL_SERVER_ERROR, - - } - - public static ErrorResponse notFoundError(String message) { - return new ErrorResponse(NOT_FOUND, errorCodes.NOT_FOUND.name(), message); - } - - public static ErrorResponse internalServerError(String message) { - return new ErrorResponse(INTERNAL_SERVER_ERROR, errorCodes.INTERNAL_SERVER_ERROR.name(), message); - } - - public static ErrorResponse badRequest(String message) { - return new ErrorResponse(BAD_REQUEST, errorCodes.BAD_REQUEST.name(), message); - } - - public static ErrorResponse methodNotAllowed(String message) { - return new ErrorResponse(METHOD_NOT_ALLOWED, errorCodes.METHOD_NOT_ALLOWED.name(), message); - } - - @Override - public void render(OutputStream stream) throws IOException { - new JsonFormat(true).encode(stream, slime); - } - - @Override - public String getContentType() { return "application/json"; } -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyException.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyException.java deleted file mode 100644 index aa828bc0c83..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyException.java +++ /dev/null @@ -1,16 +0,0 @@ -// 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; - -/** - * Exceptions related to proxying calls to config servers. - * - * @author Haakon Dybdahl - */ -public class ProxyException extends Exception { - public final ErrorResponse errorResponse; - - public ProxyException(ErrorResponse errorResponse) { - super(errorResponse.message); - this.errorResponse = errorResponse; - } -} 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 a1a7dab2955..f65f7534476 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 @@ -15,7 +15,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry; 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.yolean.Exceptions; @@ -95,11 +94,7 @@ public class ConfigServerApiHandler extends AuditLoggingRequestHandler { "' through /configserver/v1, following APIs are permitted: " + String.join(", ", WHITELISTED_APIS)); } - try { - return proxy.handle(ProxyRequest.tryOne(getEndpoint(zoneId), cfgPath, request)); - } catch (ProxyException e) { - throw new RuntimeException(e); - } + return proxy.handle(ProxyRequest.tryOne(getEndpoint(zoneId), cfgPath, 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 f1bb4ba9268..7c44ae6d1a5 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 @@ -16,7 +16,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry; 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.yolean.Exceptions; @@ -82,11 +81,7 @@ public class ZoneApiHandler extends AuditLoggingRequestHandler { if ( ! zoneRegistry.hasZone(zoneId)) { throw new IllegalArgumentException("No such zone: " + zoneId.value()); } - try { - return proxy.handle(proxyRequest(zoneId, path.getRest(), request)); - } catch (ProxyException e) { - throw new RuntimeException(e); - } + return proxy.handle(proxyRequest(zoneId, path.getRest(), request)); } private HttpResponse root(HttpRequest request) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerProxyMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerProxyMock.java index d6e1af07938..d481aaa2c77 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerProxyMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerProxyMock.java @@ -20,10 +20,10 @@ public class ConfigServerProxyMock extends AbstractComponent implements ConfigSe private volatile String requestBody = null; @Override - public HttpResponse handle(ProxyRequest proxyRequest) { - lastReceived = proxyRequest; + public HttpResponse handle(ProxyRequest request) { + lastReceived = request; // Copy request body as the input stream is drained once the request completes - requestBody = asString(proxyRequest.getData()); + requestBody = asString(request.getData()); return new StringResponse("ok"); } |