Add deployment info to secret-store validation api

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&parameter-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&parameter-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);