diff options
4 files changed, 30 insertions, 52 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java index 40903b02465..aeb5419b682 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java @@ -56,7 +56,8 @@ public enum RoleDefinition { Policy.submission, Policy.paymentInstrumentRead, Policy.paymentInstrumentDelete, - Policy.billingInformationRead), + Policy.billingInformationRead, + Policy.secretStoreOperations), /** Admin — the administrative function for user management etc. */ administrator(Policy.tenantUpdate, @@ -69,8 +70,7 @@ public enum RoleDefinition { Policy.paymentInstrumentDelete, Policy.paymentInstrumentCreate, Policy.planUpdate, - Policy.billingInformationRead, - Policy.secretStoreOperations), + Policy.billingInformationRead), /** Headless — the application specific role identified by deployment keys for production */ headless(Policy.submission), 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 19db1bb0eb6..f252e532ea8 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 @@ -224,7 +224,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant")) return tenants(request); if (path.matches("/application/v4/tenant/{tenant}")) return tenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/info")) return tenantInfo(path.get("tenant"), request); - if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}/region/{region}/parameter-name/{parameter-name}/validate")) return validateSecretStore(path.get("tenant"), path.get("name"), path.get("region"), path.get("parameter-name")); + if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}/validate")) return validateSecretStore(path.get("tenant"), path.get("name"), request); if (path.matches("/application/v4/tenant/{tenant}/application")) return applications(path.get("tenant"), Optional.empty(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return application(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/compile-version")) return compileVersion(path.get("tenant"), path.get("application")); @@ -584,29 +584,32 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new SlimeJsonResponse(root); } + private HttpResponse validateSecretStore(String tenantName, String secretStoreName, HttpRequest request) { - private HttpResponse validateSecretStore(String tenantName, String name, String region, String parameterName) { - var tenant = TenantName.from(tenantName); - if (controller.tenants().require(tenant).type() != Tenant.Type.cloud) - return ErrorResponse.badRequest("Tenant '" + tenant + "' is not a cloud tenant"); + var awsRegion = request.getProperty("aws-region"); + var parameterName = request.getProperty("parameter-name"); + var applicationId = ApplicationId.fromFullString(request.getProperty("application-id")); + var zoneId = ZoneId.from(request.getProperty("zone")); + var deploymentId = new DeploymentId(applicationId, zoneId); - var cloudTenant = (CloudTenant)controller.tenants().require(tenant); - var tenantSecretStore = cloudTenant.tenantSecretStores() + var tenant = (CloudTenant)controller.tenants().require(applicationId.tenant()); + if (tenant.type() != Tenant.Type.cloud) { + return ErrorResponse.badRequest("Tenant '" + applicationId.tenant() + "' is not a cloud tenant"); + } + + var tenantSecretStore = tenant.tenantSecretStores() .stream() - .filter(secretStore -> secretStore.getName().equals(name)) + .filter(secretStore -> secretStore.getName().equals(secretStoreName)) .findFirst(); - var deployment = getActiveDeployment(tenant); - if (deployment.isEmpty()) - return ErrorResponse.badRequest("Tenant '" + tenantName + "' has no active deployments"); if (tenantSecretStore.isEmpty()) - return ErrorResponse.notFoundError("No secret store '" + name + "' configured for tenant '" + tenantName + "'"); + return ErrorResponse.notFoundError("No secret store '" + secretStoreName + "' configured for tenant '" + tenantName + "'"); - var response = controller.serviceRegistry().configServer().validateSecretStore(deployment.get(), tenantSecretStore.get(), region, parameterName); + var response = controller.serviceRegistry().configServer().validateSecretStore(deploymentId, tenantSecretStore.get(), awsRegion, parameterName); try { var responseRoot = new Slime(); var responseCursor = responseRoot.setObject(); - responseCursor.setString("target", deployment.get().toString()); + responseCursor.setString("target", deploymentId.toString()); var responseResultCursor = responseCursor.setObject("result"); var responseSlime = SlimeUtils.jsonToSlime(response); SlimeUtils.copyObject(responseSlime.get(), responseResultCursor); @@ -616,23 +619,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } } - private Optional<DeploymentId> getActiveDeployment(TenantName tenant) { - for (var application : controller.applications().asList(tenant)) { - var optionalInstance = application.instances().values() - .stream() - .filter(instance -> instance.deployments().keySet().size() > 0) - .findFirst(); - - if (optionalInstance.isPresent()) { - var instance = optionalInstance.get(); - var applicationId = instance.id(); - var zoneId = instance.deployments().keySet().stream().findFirst().orElseThrow(); - return Optional.of(new DeploymentId(applicationId, zoneId)); - } - } - return Optional.empty(); - } - private HttpResponse removeDeveloperKey(String tenantName, HttpRequest request) { if (controller.tenants().require(TenantName.from(tenantName)).type() != Tenant.Type.cloud) throw new IllegalArgumentException("Tenant '" + tenantName + "' is not a cloud tenant"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java index 605abf63a66..62865ea9be2 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java @@ -128,7 +128,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "\"role\": \"role-id\"," + "\"externalId\": \"321\"" + "}") - .roles(Set.of(Role.administrator(tenantName))); + .roles(Set.of(Role.developer(tenantName))); tester.assertResponse(secretStoreRequest, "{\"secretStores\":[{\"name\":\"some-name\",\"awsId\":\"123\",\"role\":\"role-id\"}]}", 200); tester.assertResponse(secretStoreRequest, "{" + "\"error-code\":\"BAD_REQUEST\"," + @@ -142,7 +142,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "\"role\": \"role-id\"," + "\"externalId\": \"321\"" + "}") - .roles(Set.of(Role.administrator(tenantName))); + .roles(Set.of(Role.developer(tenantName))); tester.assertResponse(secretStoreRequest, "{" + "\"error-code\":\"BAD_REQUEST\"," + "\"message\":\"Secret store TenantSecretStore{name='should-fail', awsId=' ', role='role-id'} is invalid\"" + @@ -151,18 +151,10 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { @Test public void validate_secret_store() { - var secretStoreRequest = - request("/application/v4/tenant/scoober/secret-store/secret-foo/region/us-west-1/parameter-name/foo/validate", GET) - .roles(Set.of(Role.administrator(tenantName))); - tester.assertResponse(secretStoreRequest, "{" + - "\"error-code\":\"BAD_REQUEST\"," + - "\"message\":\"Tenant 'scoober' has no active deployments\"" + - "}", 400); - deployApplication(); - secretStoreRequest = - request("/application/v4/tenant/scoober/secret-store/secret-foo/region/us-west-1/parameter-name/foo/validate", GET) - .roles(Set.of(Role.administrator(tenantName))); + var secretStoreRequest = + request("/application/v4/tenant/scoober/secret-store/secret-foo/validate?aws-region=us-west-1¶meter-name=foo&application-id=scoober.albums.default&zone=prod.us-central-1", GET) + .roles(Set.of(Role.developer(tenantName))); tester.assertResponse(secretStoreRequest, "{" + "\"error-code\":\"NOT_FOUND\"," + "\"message\":\"No secret store 'secret-foo' configured for tenant 'scoober'\"" + @@ -175,8 +167,8 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { // ConfigServerMock returns message on format deployment.toString() + " - " + tenantSecretStore.toString() secretStoreRequest = - request("/application/v4/tenant/scoober/secret-store/secret-foo/region/us-west-1/parameter-name/foo/validate", GET) - .roles(Set.of(Role.administrator(tenantName))); + request("/application/v4/tenant/scoober/secret-store/secret-foo/validate?aws-region=us-west-1¶meter-name=foo&application-id=scoober.albums.default&zone=prod.us-central-1", GET) + .roles(Set.of(Role.developer(tenantName))); tester.assertResponse(secretStoreRequest, "{\"target\":\"scoober.albums in prod.us-central-1\",\"result\":{\"settings\":{\"name\":\"foo\",\"role\":\"vespa-secretstore-access\",\"awsId\":\"892075328880\",\"externalId\":\"*****\",\"region\":\"us-east-1\"},\"status\":\"ok\"}}", 200); } @@ -184,7 +176,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { public void delete_secret_store() { var deleteRequest = request("/application/v4/tenant/scoober/secret-store/secret-foo", DELETE) - .roles(Set.of(Role.administrator(tenantName))); + .roles(Set.of(Role.developer(tenantName))); tester.assertResponse(deleteRequest, "{" + "\"error-code\":\"NOT_FOUND\"," + "\"message\":\"Could not delete secret store 'secret-foo': Secret store not found\"" + diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index bbba115b0a8..dc765e01d4e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -175,8 +175,8 @@ public class UserApiTest extends ControllerContainerCloudTest { // PUT in a new secret store for the tenant tester.assertResponse(request("/application/v4/tenant/my-tenant/secret-store/secret-foo", PUT) - .principal("admin@tenant") - .roles(Set.of(Role.administrator(id.tenant()))) + .principal("developer@tenant") + .roles(Set.of(Role.developer(id.tenant()))) .data("{\"awsId\":\"123\",\"role\":\"secret-role\",\"externalId\":\"abc\"}"), "{\"secretStores\":[{\"name\":\"secret-foo\",\"awsId\":\"123\",\"role\":\"secret-role\"}]}", 200); |