diff options
Diffstat (limited to 'controller-server')
3 files changed, 108 insertions, 49 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 3e303dfd0cd..6fc65253da3 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 @@ -20,7 +20,6 @@ import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.api.AthenzIdentity; -import com.yahoo.vespa.athenz.api.AthenzPrincipal; import com.yahoo.vespa.athenz.api.AthenzUser; import com.yahoo.vespa.athenz.api.NToken; import com.yahoo.vespa.config.SlimeUtils; @@ -106,6 +105,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private final Controller controller; private final Authorizer authorizer; private final AthenzClientFactory athenzClientFactory; + private final ApplicationInstanceAuthorizer applicationInstanceAuthorizer; @Inject public ApplicationApiHandler(LoggingRequestHandler.Context parentCtx, @@ -115,6 +115,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { this.controller = controller; this.authorizer = authorizer; this.athenzClientFactory = athenzClientFactory; + this.applicationInstanceAuthorizer = new ApplicationInstanceAuthorizer(controller.zoneRegistry(), athenzClientFactory); } @Override @@ -191,13 +192,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler { Path path = new Path(request.getUri().getPath()); if (path.matches("/application/v4/tenant/{tenant}")) return createTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return createApplication(path.get("tenant"), path.get("application"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/promote")) return promoteApplication(path.get("tenant"), path.get("application")); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/promote")) return promoteApplication(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return deploy(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deploy(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}/deploy")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/restart")) return restart(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}/log")) return log(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}/promote")) return promoteApplicationDeployment(path.get("tenant"), path.get("application"), path.get("environment"), path.get("region")); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/log")) return log(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}/promote")) return promoteApplicationDeployment(path.get("tenant"), path.get("application"), path.get("environment"), path.get("region"), path.get("instance"), request); return ErrorResponse.notFoundError("Nothing at " + path); } @@ -206,7 +207,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}")) return deleteTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return deleteApplication(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application")); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deactivate(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}")) return deactivate(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}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), true, request); return ErrorResponse.notFoundError("Nothing at " + path); @@ -699,6 +700,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } /** Trigger deployment of the last built application package, on a given version */ + // TODO Add authorization + // TODO Consider move to API for maintenance related operations private HttpResponse deploy(String tenantName, String applicationName, HttpRequest request) { Version version = decideDeployVersion(request); if ( ! systemHasVersion(version)) @@ -718,6 +721,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } /** Cancel any ongoing change for given application */ + // TODO Add authorization + // TODO Consider move to API for maintenance related operations private HttpResponse cancelDeploy(String tenantName, String applicationName) { ApplicationId id = ApplicationId.from(tenantName, applicationName, "default"); Application application = controller.applications().require(id); @@ -735,8 +740,14 @@ public class ApplicationApiHandler extends LoggingRequestHandler { 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)); + // TODO: Propagate all filters Optional<Hostname> hostname = Optional.ofNullable(request.getProperty("hostname")).map(Hostname::new); + + applicationInstanceAuthorizer.throwIfUnauthorized(authorizer.getPrincipal(request), + Environment.from(environment), + getTenantOrThrow(tenantName), + deploymentId.applicationId().application()); controller.applications().restart(deploymentId, hostname); // TODO: Change to return JSON @@ -752,10 +763,15 @@ public class ApplicationApiHandler extends LoggingRequestHandler { * the application is working. It is called for all production zones, also those in which the application is not present, * and possibly before it is present, so failures are normal and expected. */ - private HttpResponse log(String tenantName, String applicationName, String instanceName, String environment, String region) { + private HttpResponse log(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) { try { DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), ZoneId.from(environment, region)); + + applicationInstanceAuthorizer.throwIfUnauthorized(authorizer.getPrincipal(request), + Environment.from(environment), + getTenantOrThrow(tenantName), + deploymentId.applicationId().application()); return new JacksonJsonResponse(controller.grabLog(deploymentId)); } catch (RuntimeException e) { @@ -777,10 +793,11 @@ public class ApplicationApiHandler extends LoggingRequestHandler { Optional<ApplicationPackage> applicationPackage = Optional.ofNullable(dataParts.get("applicationZip")) .map(ApplicationPackage::new); - ApplicationInstanceAuthorizer applicationInstanceAuthorizer = new ApplicationInstanceAuthorizer(controller.zoneRegistry(), athenzClientFactory); - Tenant tenant = controller.tenants().tenant(new TenantId(tenantName)).orElseThrow(() -> new NotExistsException(new TenantId(tenantName))); - AthenzPrincipal principal = authorizer.getPrincipal(request); - applicationInstanceAuthorizer.throwIfUnauthorizedForDeploy(principal, Environment.from(environment), tenant, ApplicationName.from(applicationName), applicationPackage); + applicationInstanceAuthorizer.throwIfUnauthorizedForDeploy(authorizer.getPrincipal(request), + Environment.from(environment), + getTenantOrThrow(tenantName), + ApplicationName.from(applicationName), + applicationPackage); // TODO: get rid of the json object DeployOptions deployOptionsJsonClass = new DeployOptions(screwdriverBuildJobFromSlime(deployOptions.field("screwdriverBuildJob")), @@ -813,11 +830,17 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new EmptyJsonResponse(); // TODO: Replicates current behavior but should return a message response instead } - private HttpResponse deactivate(String tenantName, String applicationName, String instanceName, String environment, String region) { + private HttpResponse deactivate(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) { Application application = controller.applications().require(ApplicationId.from(tenantName, applicationName, instanceName)); ZoneId zone = ZoneId.from(environment, region); Deployment deployment = application.deployments().get(zone); + + applicationInstanceAuthorizer.throwIfUnauthorized(authorizer.getPrincipal(request), + Environment.from(environment), + getTenantOrThrow(tenantName), + ApplicationName.from(applicationName)); + if (deployment == null) { // Attempt to deactivate application even if the deployment is not known by the controller controller.applications().deactivate(application, zone); @@ -836,8 +859,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler { /** * Promote application Chef environments. To be used by component jobs only */ - private HttpResponse promoteApplication(String tenantName, String applicationName) { + private HttpResponse promoteApplication(String tenantName, String applicationName, HttpRequest request) { try{ + applicationInstanceAuthorizer.throwIfUnauthorized(authorizer.getPrincipal(request), + getTenantOrThrow(tenantName), + ApplicationName.from(applicationName)); + ApplicationChefEnvironment chefEnvironment = new ApplicationChefEnvironment(controller.system()); String sourceEnvironment = chefEnvironment.systemChefEnvironment(); String targetEnvironment = chefEnvironment.applicationSourceEnvironment(TenantName.from(tenantName), ApplicationName.from(applicationName)); @@ -852,8 +879,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler { /** * Promote application Chef environments for jobs that deploy applications */ - private HttpResponse promoteApplicationDeployment(String tenantName, String applicationName, String environmentName, String regionName) { + private HttpResponse promoteApplicationDeployment(String tenantName, String applicationName, String environmentName, String regionName, String instanceName, HttpRequest request) { try { + applicationInstanceAuthorizer.throwIfUnauthorized(authorizer.getPrincipal(request), + Environment.from(environmentName), + getTenantOrThrow(tenantName), + ApplicationName.from(applicationName)); + ApplicationChefEnvironment chefEnvironment = new ApplicationChefEnvironment(controller.system()); String sourceEnvironment = chefEnvironment.applicationSourceEnvironment(TenantName.from(tenantName), ApplicationName.from(applicationName)); String targetEnvironment = chefEnvironment.applicationTargetEnvironment(TenantName.from(tenantName), ApplicationName.from(applicationName), Environment.from(environmentName), RegionName.from(regionName)); @@ -865,6 +897,11 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } } + private Tenant getTenantOrThrow(String tenantName) { + return controller.tenants().tenant(new TenantId(tenantName)) + .orElseThrow(() -> new NotExistsException(new TenantId(tenantName))); + } + private void toSlime(Cursor object, Tenant tenant, HttpRequest request, boolean listApplications) { object.setString("tenant", tenant.getId().id()); object.setString("type", tenant.tenantType().name()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationInstanceAuthorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationInstanceAuthorizer.java index 1b40dacd858..283d700c2bd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationInstanceAuthorizer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationInstanceAuthorizer.java @@ -41,29 +41,9 @@ public class ApplicationInstanceAuthorizer { this.athenzClientFactory = athenzClientFactory; } - public void throwIfUnauthorizedForDeploy(AthenzPrincipal principal, - Environment environment, - Tenant tenant, - ApplicationName application, - Optional<ApplicationPackage> applicationPackage) { - // Validate that domain in identity configuration (deployment.xml) is same as tenant domain - applicationPackage.map(ApplicationPackage::deploymentSpec).flatMap(DeploymentSpec::athenzDomain) - .ifPresent(identityDomain -> { - AthenzDomain tenantDomain = tenant.getAthensDomain().orElseThrow(() -> new IllegalArgumentException("Identity provider only available to Athenz onboarded tenants")); - if (! Objects.equals(tenantDomain.getName(), identityDomain.value())) { - throw new ForbiddenException( - String.format( - "Athenz domain in deployment.xml: [%s] must match tenant domain: [%s]", - identityDomain.value(), - tenantDomain.getName() - )); - } - }); - - if (!environmentRequiresAuthorization(environment)) { - return; - } - + public void throwIfUnauthorized(AthenzPrincipal principal, + Tenant tenant, + ApplicationName application) { AthenzDomain principalDomain = principal.getDomain(); if (!principalDomain.equals(SCREWDRIVER_DOMAIN)) { @@ -82,13 +62,44 @@ public class ApplicationInstanceAuthorizer { if (!hasDeployAccessToAthenzApplication(principal, tenantDomain, application)) { throw loggedForbiddenException( "Screwdriver principal '%1$s' does not have deploy access to '%2$s'. " + - "Either the application has not been created at " + zoneRegistry.getDashboardUri() + " or " + - "'%1$s' is not added to the application's deployer role in Athenz domain '%3$s'.", + "Either the application has not been created at " + zoneRegistry.getDashboardUri() + " or " + + "'%1$s' is not added to the application's deployer role in Athenz domain '%3$s'.", principal.getIdentity().getFullName(), application.value(), tenantDomain.getName()); } } } + public void throwIfUnauthorized(AthenzPrincipal principal, + Environment environment, + Tenant tenant, + ApplicationName application) { + if (!environmentRequiresAuthorization(environment)) { + return; + } + throwIfUnauthorized(principal, tenant, application); + } + + public void throwIfUnauthorizedForDeploy(AthenzPrincipal principal, + Environment environment, + Tenant tenant, + ApplicationName application, + Optional<ApplicationPackage> applicationPackage) { + // Validate that domain in identity configuration (deployment.xml) is same as tenant domain + applicationPackage.map(ApplicationPackage::deploymentSpec).flatMap(DeploymentSpec::athenzDomain) + .ifPresent(identityDomain -> { + AthenzDomain tenantDomain = tenant.getAthensDomain().orElseThrow(() -> new IllegalArgumentException("Identity provider only available to Athenz onboarded tenants")); + if (! Objects.equals(tenantDomain.getName(), identityDomain.value())) { + throw new ForbiddenException( + String.format( + "Athenz domain in deployment.xml: [%s] must match tenant domain: [%s]", + identityDomain.value(), + tenantDomain.getName() + )); + } + }); + throwIfUnauthorized(principal, environment, tenant, application); + } + private static ForbiddenException loggedForbiddenException(String message, Object... args) { String formattedMessage = String.format(message, args); log.info(formattedMessage); 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 61a4a883904..e6fe7531fdc 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 @@ -197,7 +197,8 @@ public class ApplicationApiTest extends ControllerContainerTest { .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId))) .screwdriverIdentity(SCREWDRIVER_ID), new File("deploy-result.json")); - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default", DELETE), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default", DELETE) + .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default"); controllerTester.notifyJobCompletion(id, screwdriverProjectId, true, DeploymentJobs.JobType.systemTest); // Called through the separate screwdriver/v1 API @@ -206,7 +207,8 @@ public class ApplicationApiTest extends ControllerContainerTest { .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId))) .screwdriverIdentity(SCREWDRIVER_ID), new File("deploy-result.json")); - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default", DELETE), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default", DELETE) + .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default"); controllerTester.notifyJobCompletion(id, screwdriverProjectId, true, DeploymentJobs.JobType.stagingTest); @@ -252,10 +254,12 @@ public class ApplicationApiTest extends ControllerContainerTest { // POST a 'restart application' command - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/restart", POST), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/restart", POST) + .screwdriverIdentity(SCREWDRIVER_ID), "Requested restart of tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default"); // 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/corp-us-east-1/instance/default/restart?hostname=host1", POST), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/restart?hostname=host1", POST) + .screwdriverIdentity(SCREWDRIVER_ID), "Requested restart of tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default"); // POST a 'log' command tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/log", POST), @@ -275,14 +279,17 @@ public class ApplicationApiTest extends ControllerContainerTest { new File("delete-with-active-deployments.json"), 400); // DELETE (deactivate) a deployment - dev - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default", DELETE), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default", DELETE) + .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default"); // DELETE (deactivate) a deployment - prod - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default", DELETE), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default", DELETE) + .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default"); // DELETE (deactivate) a deployment is idempotent - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default", DELETE), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default", DELETE) + .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default"); // PUT (create) the authenticated user @@ -315,9 +322,11 @@ public class ApplicationApiTest extends ControllerContainerTest { .data("{\"reason\":\"because i can\"}"), new File("global-rotation-delete.json")); - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/promote", POST), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/promote", POST) + .screwdriverIdentity(SCREWDRIVER_ID), "{\"message\":\"Successfully copied environment hosted-verified-prod to hosted-instance_tenant1_application1_placeholder_component_default\"}"); - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default/promote", POST), + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default/promote", POST) + .screwdriverIdentity(SCREWDRIVER_ID), "{\"message\":\"Successfully copied environment hosted-instance_tenant1_application1_placeholder_component_default to hosted-instance_tenant1_application1_us-west-1_prod_default\"}"); // DELETE an application @@ -816,7 +825,8 @@ public class ApplicationApiTest extends ControllerContainerTest { .data(deployData) .screwdriverIdentity(SCREWDRIVER_ID), new File("deploy-result.json")); - tester.assertResponse(request(testPath, DELETE), + tester.assertResponse(request(testPath, DELETE) + .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated " + testPath.replaceFirst("/application/v4/", "")); controllerTester.notifyJobCompletion(application, projectId, true, DeploymentJobs.JobType.systemTest); @@ -827,7 +837,8 @@ public class ApplicationApiTest extends ControllerContainerTest { .data(deployData) .screwdriverIdentity(SCREWDRIVER_ID), new File("deploy-result.json")); - tester.assertResponse(request(stagingPath, DELETE), + tester.assertResponse(request(stagingPath, DELETE) + .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated " + stagingPath.replaceFirst("/application/v4/", "")); controllerTester.notifyJobCompletion(application, projectId, true, DeploymentJobs.JobType.stagingTest); } |