Refactor ConfigServerRestExecutorImpl

3 files changed, 65 insertions, 80 deletions

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);
     }