diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2020-12-15 17:55:16 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-12-15 17:55:16 +0100 |
commit | ae830ce87e6ebdd5565d56ab80fec45745f9d838 (patch) | |
tree | b9923b6557d3d33a7f10b63d118e7f5bb0b1af75 | |
parent | 0dd69b57d88cf06ffb832ea735ae5773df437792 (diff) | |
parent | b421c9609528ab6c8ab1eb94aa718ee4abc6ee98 (diff) |
Merge pull request #15828 from vespa-engine/mpolden/validate-zone
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); |