Validate zone in application API

2 files changed, 46 insertions, 37 deletions

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 22f92006c6b..824a374642a 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
@@ -666,7 +666,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse nodes(String tenantName, String applicationName, String instanceName, String environment, String region) {
         ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         List<Node> nodes = controller.serviceRegistry().configServer().nodeRepository().list(zone, id);
 
         Slime slime = new Slime();
@@ -689,7 +689,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse clusters(String tenantName, String applicationName, String instanceName, String environment, String region) {
         ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         Application application = controller.serviceRegistry().configServer().nodeRepository().getApplication(zone, id);
 
         Slime slime = new Slime();
@@ -762,7 +762,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse logs(String tenantName, String applicationName, String instanceName, String environment, String region, Map<String, String> queryParameters) {
         ApplicationId application = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         DeploymentId deployment = new DeploymentId(application, zone);
         InputStream logStream = controller.serviceRegistry().configServer().getLogs(deployment, queryParameters);
         return new HttpResponse(200) {
@@ -775,7 +775,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse metrics(String tenantName, String applicationName, String instanceName, String environment, String region) {
         ApplicationId application = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         DeploymentId deployment = new DeploymentId(application, zone);
         List<ProtonMetrics> protonMetrics = controller.serviceRegistry().configServer().getProtonMetrics(deployment);
         return buildResponseFromProtonMetrics(protonMetrics);
@@ -1090,7 +1090,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
                                       .orElseThrow(() -> new NotExistsException(id + " not found"));
 
         DeploymentId deploymentId = new DeploymentId(instance.id(),
-                                                     ZoneId.from(environment, region));
+                                                     requireZone(environment, region));
 
         Deployment deployment = instance.deployments().get(deploymentId.zoneId());
         if (deployment == null)
@@ -1257,7 +1257,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse setGlobalRotationOverride(String tenantName, String applicationName, String instanceName, String environment, String region, boolean inService, HttpRequest request) {
         Instance instance = controller.applications().requireInstance(ApplicationId.from(tenantName, applicationName, instanceName));
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         Deployment deployment = instance.deployments().get(zone);
         if (deployment == null) {
             throw new NotExistsException(instance + " has no deployment in " + zone);
@@ -1293,7 +1293,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse getGlobalRotationOverride(String tenantName, String applicationName, String instanceName, String environment, String region) {
         DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName),
-                                                     ZoneId.from(environment, region));
+                                                     requireZone(environment, region));
         Slime slime = new Slime();
         Cursor array = slime.setObject().setArray("globalrotationoverride");
         controller.routing().globalRotationStatus(deploymentId)
@@ -1311,7 +1311,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     private HttpResponse rotationStatus(String tenantName, String applicationName, String instanceName, String environment, String region, Optional<String> endpointId) {
         ApplicationId applicationId = ApplicationId.from(tenantName, applicationName, instanceName);
         Instance instance = controller.applications().requireInstance(applicationId);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         RotationId rotation = findRotationId(instance, endpointId);
         Deployment deployment = instance.deployments().get(zone);
         if (deployment == null) {
@@ -1400,7 +1400,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse suspended(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName),
-                                                     ZoneId.from(environment, region));
+                                                     requireZone(environment, region));
         boolean suspended = controller.applications().isSuspended(deploymentId);
         Slime slime = new Slime();
         Cursor response = slime.setObject();
@@ -1410,16 +1410,17 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse services(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         ApplicationView applicationView = controller.getApplicationView(tenantName, applicationName, instanceName, environment, region);
-        ServiceApiResponse response = new ServiceApiResponse(ZoneId.from(environment, region),
+        ZoneId zone = requireZone(environment, region);
+        ServiceApiResponse response = new ServiceApiResponse(zone,
                                                              new ApplicationId.Builder().tenant(tenantName).applicationName(applicationName).instanceName(instanceName).build(),
-                                                             controller.zoneRegistry().getConfigServerApiUris(ZoneId.from(environment, region)),
+                                                             controller.zoneRegistry().getConfigServerApiUris(zone),
                                                              request.getUri());
         response.setResponse(applicationView);
         return response;
     }
 
     private HttpResponse service(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, String restPath, HttpRequest request) {
-        DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), ZoneId.from(environment, region));
+        DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region));
 
         if ("container-clustercontroller".equals((serviceName)) && restPath.contains("/status/")) {
             String result = controller.serviceRegistry().configServer().getClusterControllerStatus(deploymentId, restPath);
@@ -1436,7 +1437,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     }
 
     private HttpResponse content(String tenantName, String applicationName, String instanceName, String environment, String region, String restPath, HttpRequest request) {
-        DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), ZoneId.from(environment, region));
+        DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region));
         return controller.serviceRegistry().configServer().getApplicationPackageContent(deploymentId, "/" + restPath, request.getUri());
     }
 
@@ -1544,7 +1545,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     /** Schedule reindexing of an application, or a subset of clusters, possibly on a subset of documents. */
     private HttpResponse reindex(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         List<String> clusterNames = Optional.ofNullable(request.getProperty("clusterId")).stream()
                                             .flatMap(clusters -> Stream.of(clusters.split(",")))
                                             .filter(cluster -> ! cluster.isBlank())
@@ -1563,7 +1564,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     /** Gets reindexing status of an application in a zone. */
     private HttpResponse getReindexing(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         ApplicationReindexing reindexing = controller.applications().applicationReindexing(id, zone);
 
         Slime slime = new Slime();
@@ -1620,7 +1621,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     /** Enables reindexing of an application in a zone. */
     private HttpResponse enableReindexing(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         controller.applications().enableReindexing(id, zone);
         return new MessageResponse("Enabled reindexing of " + id + " in " + zone);
     }
@@ -1628,7 +1629,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     /** Disables reindexing of an application in a zone. */
     private HttpResponse disableReindexing(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
         controller.applications().disableReindexing(id, zone);
         return new MessageResponse("Disabled reindexing of " + id + " in " + zone);
     }
@@ -1636,7 +1637,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     /** Schedule restart of deployment, or specific host in a deployment */
     private HttpResponse restart(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName),
-                                                     ZoneId.from(environment, region));
+                                                     requireZone(environment, region));
         RestartFilter restartFilter = new RestartFilter()
                 .withHostName(Optional.ofNullable(request.getProperty("hostname")).map(HostName::from))
                 .withClusterType(Optional.ofNullable(request.getProperty("clusterType")).map(ClusterSpec.Type::from))
@@ -1678,7 +1679,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse deploy(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         ApplicationId applicationId = ApplicationId.from(tenantName, applicationName, instanceName);
-        ZoneId zone = ZoneId.from(environment, region);
+        ZoneId zone = requireZone(environment, region);
 
         // Get deployOptions
         Map<String, byte[]> dataParts = parseDataParts(request);
@@ -1815,7 +1816,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse deactivate(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         DeploymentId id = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName),
-                                           ZoneId.from(environment, region));
+                                           requireZone(environment, region));
         // Attempt to deactivate application even if the deployment is not known by the controller
         controller.applications().deactivate(id.applicationId(), id.zoneId());
         return new MessageResponse("Deactivated " + id);
@@ -2203,6 +2204,14 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new MessageResponse("All deployments removed");
     }
 
+    private ZoneId requireZone(String environment, String region) {
+        ZoneId zone = ZoneId.from(environment, region);
+        if (!controller.zoneRegistry().hasZone(zone)) {
+            throw new IllegalArgumentException("Zone " + zone + " does not exist in this system");
+        }
+        return zone;
+    }
+
     private static Map<String, byte[]> parseDataParts(HttpRequest request) {
         String contentHash = request.getHeader("X-Content-Hash");
         if (contentHash == null)
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 e0a4141480d..27d7b6f3d7a 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
@@ -490,22 +490,22 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               new File("application-clusters.json"));
 
         // GET logs
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/environment/dev/region/us-central-1/instance/default/logs?from=1233&to=3214", GET)
+        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/environment/dev/region/us-east-1/instance/default/logs?from=1233&to=3214", GET)
                         .userIdentity(USER_ID),
                 "INFO - All good");
 
         // Get content - root
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/instance/default/environment/dev/region/us-central-1/content/", GET).userIdentity(USER_ID),
+        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/instance/default/environment/dev/region/us-east-1/content/", GET).userIdentity(USER_ID),
                 "{\"path\":\"/\"}");
         // Get content - ignore query params
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/instance/default/environment/dev/region/us-central-1/content/bar/file.json?query=param", GET).userIdentity(USER_ID),
+        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/instance/default/environment/dev/region/us-east-1/content/bar/file.json?query=param", GET).userIdentity(USER_ID),
                 "{\"path\":\"/bar/file.json\"}");
 
 
         updateMetrics();
 
         // GET metrics
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/environment/dev/region/us-central-1/instance/default/metrics", GET)
+        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/environment/dev/region/us-east-1/instance/default/metrics", GET)
                         .userIdentity(USER_ID),
                                 new File("proton-metrics.json"));
 
@@ -622,20 +622,20 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         addUserToHostedOperatorRole(HostedAthenzIdentities.from(SCREWDRIVER_ID));
 
-        // POST a 'restart application' in staging environment command
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-central-1/instance/instance1/restart", POST)
+        // POST a 'restart application' in staging environment
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/instance1/restart", POST)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
-                              "{\"message\":\"Requested restart of tenant1.application1.instance1 in staging.us-central-1\"}");
+                              "{\"message\":\"Requested restart of tenant1.application1.instance1 in staging.us-east-3\"}");
 
-        // POST a 'restart application' in staging test command
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-central-1/instance/instance1/restart", POST)
+        // POST a 'restart application' in test environment
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/instance1/restart", POST)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
-                              "{\"message\":\"Requested restart of tenant1.application1.instance1 in test.us-central-1\"}");
+                              "{\"message\":\"Requested restart of tenant1.application1.instance1 in test.us-east-1\"}");
 
-        // POST a 'restart application' in staging dev command
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-central-1/instance/instance1/restart", POST)
+        // POST a 'restart application' in dev environment
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-east-1/instance/instance1/restart", POST)
                                       .userIdentity(USER_ID),
-                              "{\"message\":\"Requested restart of tenant1.application1.instance1 in dev.us-central-1\"}");
+                              "{\"message\":\"Requested restart of tenant1.application1.instance1 in dev.us-east-1\"}");
 
         // POST a 'restart application' command with a host filter (other filters not supported yet)
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/restart", POST)
@@ -1158,28 +1158,28 @@ public class ApplicationApiTest extends ControllerContainerTest {
         
         // POST (deploy) an application with an invalid application package
         MultiPartStreamer entity = createApplicationDeployData(applicationPackageInstance1, true);
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/instance1/deploy", POST)
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-east-1/instance/instance1/deploy", POST)
                                       .data(entity)
                                       .userIdentity(USER_ID),
                               new File("deploy-failure.json"), 400);
 
         // POST (deploy) an application without available capacity
         configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Failed to prepare application", "Out of capacity", ConfigServerException.ErrorCode.OUT_OF_CAPACITY, null));
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/instance1/deploy", POST)
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-east-1/instance/instance1/deploy", POST)
                                       .data(entity)
                                       .userIdentity(USER_ID),
                               new File("deploy-out-of-capacity.json"), 400);
 
         // POST (deploy) an application where activation fails
         configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Failed to activate application", "Activation conflict", ConfigServerException.ErrorCode.ACTIVATION_CONFLICT, null));
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/instance1/deploy", POST)
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-east-1/instance/instance1/deploy", POST)
                                       .data(entity)
                                       .userIdentity(USER_ID),
                               new File("deploy-activation-conflict.json"), 409);
 
         // POST (deploy) an application where we get an internal server error
         configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Failed to deploy application", "Internal server error", ConfigServerException.ErrorCode.INTERNAL_SERVER_ERROR, null));
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/instance1/deploy", POST)
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-east-1/instance/instance1/deploy", POST)
                                       .data(entity)
                                       .userIdentity(USER_ID),
                               new File("deploy-internal-server-error.json"), 500);