Controller finds active deployment when validating secret store. Path includes secret store name

4 files changed, 92 insertions, 24 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 a53d5bddbfc..9ce0fa25e8a 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
@@ -268,7 +268,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     private HttpResponse handlePUT(Path path, HttpRequest request) {
         if (path.matches("/application/v4/tenant/{tenant}")) return updateTenant(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/info")) return updateTenantInfo(path.get("tenant"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/secret-store")) return addSecretStore(path.get("tenant"), request);
+        if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}")) return addSecretStore(path.get("tenant"), path.get("name"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), false, 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"), false, request);
         return ErrorResponse.notFoundError("Nothing at " + path);
@@ -277,6 +277,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     private HttpResponse handlePOST(Path path, HttpRequest request) {
         if (path.matches("/application/v4/tenant/{tenant}")) return createTenant(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/key")) return addDeveloperKey(path.get("tenant"), request);
+        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/{application}")) return createApplication(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), "default", false, request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), "default", true, request);
@@ -297,7 +298,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindexing")) return enableReindexing(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}/instance/{instance}/environment/{environment}/region/{region}/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}/instance/{instance}/environment/{environment}/region/{region}/suspend")) return suspend(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), true);
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/validate-parameter-store")) return validateParameterStore(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}")) 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);
@@ -583,25 +583,44 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     }
 
 
-    private HttpResponse validateParameterStore(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
+    private HttpResponse validateSecretStore(String tenantName, String name, HttpRequest request) {
         var tenant = TenantName.from(tenantName);
         if (controller.tenants().require(tenant).type() != Tenant.Type.cloud)
-            throw new IllegalArgumentException("Tenant '" + tenant + "' is not a cloud tenant");
+            return ErrorResponse.badRequest("Tenant '" + tenant + "' is not a cloud tenant");
 
-        var application = ApplicationId.from(tenantName, applicationName, instanceName);
-        var zone = requireZone(environment, region);
-        var deployment = new DeploymentId(application, zone);
+        var cloudTenant = (CloudTenant)controller.tenants().require(tenant);
+        var tenantSecretStore = cloudTenant.tenantSecretStores()
+                .stream()
+                .filter(secretStore -> secretStore.getName().equals(name))
+                .findFirst();
+        var deployment = getActiveDeployment(tenant);
 
-        var data = toSlime(request.getData()).get();
-        var awsId = mandatory("awsId", data).asString();
-        var name = mandatory("name", data).asString();
-        var role = mandatory("role", data).asString();
-        var tenantSecretStore = new TenantSecretStore(name, awsId, role);
+        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 + "'");
 
-        var response = controller.serviceRegistry().configServer().validateSecretStore(deployment, tenantSecretStore);
+        var response = controller.serviceRegistry().configServer().validateSecretStore(deployment.get(), tenantSecretStore.get());
         return new MessageResponse(response);
     }
 
+    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");
@@ -654,14 +673,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new SlimeJsonResponse(root);
     }
 
-    private HttpResponse addSecretStore(String tenantName, HttpRequest request) {
+    private HttpResponse addSecretStore(String tenantName, String name, HttpRequest request) {
         if (controller.tenants().require(TenantName.from(tenantName)).type() != Tenant.Type.cloud)
             throw new IllegalArgumentException("Tenant '" + tenantName + "' is not a cloud tenant");
 
         var data = toSlime(request.getData()).get();
         var awsId = mandatory("awsId", data).asString();
         var externalId = mandatory("externalId", data).asString();
-        var name = mandatory("name", data).asString();
         var role = mandatory("role", data).asString();
 
         var tenant = (CloudTenant) controller.tenants().require(TenantName.from(tenantName));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 35b0a7ba5b3..b669e942494 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -577,7 +577,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
 
     @Override
     public String validateSecretStore(DeploymentId deployment, TenantSecretStore tenantSecretStore) {
-        return "";
+        return deployment.toString() + " - " + tenantSecretStore.toString();
     }
 
     public static class Application {
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 8d6b4f1f629..eaae6f98819 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
@@ -1,12 +1,17 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.restapi.application;
 
+import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.TenantName;
+import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.flags.PermanentFlags;
+import com.yahoo.vespa.hosted.controller.LockedTenant;
+import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanId;
+import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore;
 import com.yahoo.vespa.hosted.controller.api.role.Role;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
@@ -20,6 +25,7 @@ import org.junit.Test;
 
 import javax.ws.rs.ForbiddenException;
 import java.util.Collections;
+import java.util.Optional;
 import java.util.Set;
 
 import static com.yahoo.application.container.handler.Request.Method.GET;
@@ -117,10 +123,9 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
     @Test
     public void test_secret_store_configuration() {
         var secretStoreRequest =
-                request("/application/v4/tenant/scoober/secret-store", PUT)
+                request("/application/v4/tenant/scoober/secret-store/some-name", PUT)
                         .data("{" +
                                 "\"awsId\": \"123\"," +
-                                "\"name\": \"some-name\"," +
                                 "\"role\": \"role-id\"," +
                                 "\"externalId\": \"321\"" +
                                 "}")
@@ -132,20 +137,52 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
                 "}", 400);
 
         secretStoreRequest =
-                request("/application/v4/tenant/scoober/secret-store", PUT)
+                request("/application/v4/tenant/scoober/secret-store/should-fail", PUT)
                         .data("{" +
-                                "\"awsId\": \"123\"," +
-                                "\"name\": \" \"," +
+                                "\"awsId\": \" \"," +
                                 "\"role\": \"role-id\"," +
                                 "\"externalId\": \"321\"" +
                                 "}")
                         .roles(Set.of(Role.administrator(tenantName)));
         tester.assertResponse(secretStoreRequest, "{" +
                 "\"error-code\":\"BAD_REQUEST\"," +
-                "\"message\":\"Secret store TenantSecretStore{name=' ', awsId='123', role='role-id'} is invalid\"" +
+                "\"message\":\"Secret store TenantSecretStore{name='should-fail', awsId=' ', role='role-id'} is invalid\"" +
                 "}", 400);
     }
 
+    @Test
+    public void validate_secret_store() {
+        var secretStoreRequest =
+                request("/application/v4/tenant/scoober/secret-store/secret-foo/validate", POST)
+                        .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/validate", POST)
+                        .roles(Set.of(Role.administrator(tenantName)));
+        tester.assertResponse(secretStoreRequest, "{" +
+                "\"error-code\":\"NOT_FOUND\"," +
+                "\"message\":\"No secret store 'secret-foo' configured for tenant 'scoober'\"" +
+                "}", 404);
+
+        tester.controller().tenants().lockOrThrow(tenantName, LockedTenant.Cloud.class, lockedTenant -> {
+            lockedTenant = lockedTenant.withSecretStore(new TenantSecretStore("secret-foo", "123", "some-role"));
+            tester.controller().tenants().store(lockedTenant);
+        });
+
+        // ConfigServerMock returns message on format deployment.toString() + " - " + tenantSecretStore.toString()
+        secretStoreRequest =
+                request("/application/v4/tenant/scoober/secret-store/secret-foo/validate", POST)
+                        .roles(Set.of(Role.administrator(tenantName)));
+        tester.assertResponse(secretStoreRequest, "{" +
+                "\"message\":\"scoober.albums in prod.us-central-1 - TenantSecretStore{name='secret-foo', awsId='123', role='some-role'}\"" +
+                "}", 200);
+    }
+
     private ApplicationPackageBuilder prodBuilder() {
         return new ApplicationPackageBuilder()
                 .instances("default")
@@ -167,4 +204,17 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
     private static Credentials credentials(String name) {
         return new Auth0Credentials(() -> name, Collections.emptySet());
     }
+
+    private void deployApplication() {
+        var applicationPackage = new ApplicationPackageBuilder()
+                .instances("default")
+                .globalServiceId("foo")
+                .region("us-central-1")
+                .build();
+
+        tester.controller().applications().deploy(ApplicationId.from("scoober", "albums", "default"),
+                ZoneId.from("prod", "us-central-1"),
+                Optional.of(applicationPackage),
+                new DeployOptions(true, Optional.empty(), false, false));
+    }
 }
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 b765b2c3afb..9e0d645583a 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
@@ -174,10 +174,10 @@ public class UserApiTest extends ControllerContainerCloudTest {
                               new File("second-developer-key.json"));
 
         // PUT in a new secret store for the tenant
-        tester.assertResponse(request("/application/v4/tenant/my-tenant/secret-store/", PUT)
+        tester.assertResponse(request("/application/v4/tenant/my-tenant/secret-store/secret-foo", PUT)
                         .principal("admin@tenant")
                         .roles(Set.of(Role.administrator(id.tenant())))
-                        .data("{\"name\":\"secret-foo\",\"awsId\":\"123\",\"role\":\"secret-role\",\"externalId\":\"abc\"}"),
+                        .data("{\"awsId\":\"123\",\"role\":\"secret-role\",\"externalId\":\"abc\"}"),
                 "{\"message\":\"Configured secret store: TenantSecretStore{name='secret-foo', awsId='123', role='secret-role'}\"}",
                 200);