Merge pull request #10889 from vespa-engine/jvenstad/respond-with-all-keys

Respons with new set of keys when changing them

7 files changed, 80 insertions, 20 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 74010c4e41d..24819fda261 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
@@ -380,9 +380,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         Principal user = request.getJDiscRequest().getUserPrincipal();
         String pemDeveloperKey = toSlime(request.getData()).get().field("key").asString();
         PublicKey developerKey = KeyUtils.fromPemEncodedPublicKey(pemDeveloperKey);
-        controller.tenants().lockOrThrow(TenantName.from(tenantName), LockedTenant.Cloud.class, tenant ->
-                controller.tenants().store(tenant.withDeveloperKey(developerKey, user)));
-        return new MessageResponse("Set developer key " + pemDeveloperKey + " for " + user);
+        Slime root = new Slime();
+        controller.tenants().lockOrThrow(TenantName.from(tenantName), LockedTenant.Cloud.class, tenant -> {
+            tenant = tenant.withDeveloperKey(developerKey, user);
+            toSlime(root.setObject().setArray("keys"), tenant.get().developerKeys());
+            controller.tenants().store(tenant);
+        });
+        return new SlimeJsonResponse(root);
     }
 
     private HttpResponse removeDeveloperKey(String tenantName, HttpRequest request) {
@@ -392,27 +396,49 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         String pemDeveloperKey = toSlime(request.getData()).get().field("key").asString();
         PublicKey developerKey = KeyUtils.fromPemEncodedPublicKey(pemDeveloperKey);
         Principal user = ((CloudTenant) controller.tenants().require(TenantName.from(tenantName))).developerKeys().get(developerKey);
-        controller.tenants().lockOrThrow(TenantName.from(tenantName), LockedTenant.Cloud.class, tenant ->
-                controller.tenants().store(tenant.withoutDeveloperKey(developerKey)));
-        return new MessageResponse("Removed developer key " + pemDeveloperKey + " for " + user);
+        Slime root = new Slime();
+        controller.tenants().lockOrThrow(TenantName.from(tenantName), LockedTenant.Cloud.class, tenant -> {
+            tenant = tenant.withoutDeveloperKey(developerKey);
+            toSlime(root.setObject().setArray("keys"), tenant.get().developerKeys());
+            controller.tenants().store(tenant);
+        });
+        return new SlimeJsonResponse(root);
+    }
+
+    private void toSlime(Cursor keysArray, Map<PublicKey, Principal> keys) {
+        keys.forEach((key, principal) -> {
+            Cursor keyObject = keysArray.addObject();
+            keyObject.setString("key", KeyUtils.toPem(key));
+            keyObject.setString("user", principal.getName());
+        });
     }
 
     private HttpResponse addDeployKey(String tenantName, String applicationName, HttpRequest request) {
         String pemDeployKey = toSlime(request.getData()).get().field("key").asString();
         PublicKey deployKey = KeyUtils.fromPemEncodedPublicKey(pemDeployKey);
-        controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(tenantName, applicationName), application ->
-                controller.applications().store(application.withDeployKey(deployKey)));
-
-        return new MessageResponse("Added deploy key " + pemDeployKey);
+        Slime root = new Slime();
+        controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(tenantName, applicationName), application -> {
+            application = application.withDeployKey(deployKey);
+            application.get().deployKeys().stream()
+                       .map(KeyUtils::toPem)
+                       .forEach(root.setObject().setArray("keys")::addString);
+            controller.applications().store(application);
+        });
+        return new SlimeJsonResponse(root);
     }
 
     private HttpResponse removeDeployKey(String tenantName, String applicationName, HttpRequest request) {
         String pemDeployKey = toSlime(request.getData()).get().field("key").asString();
         PublicKey deployKey = KeyUtils.fromPemEncodedPublicKey(pemDeployKey);
-        controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(tenantName, applicationName), application ->
-                controller.applications().store(application.withoutDeployKey(deployKey)));
-
-        return new MessageResponse("Removed deploy key " + pemDeployKey);
+        Slime root = new Slime();
+        controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(tenantName, applicationName), application -> {
+            application = application.withoutDeployKey(deployKey);
+            application.get().deployKeys().stream()
+                       .map(KeyUtils::toPem)
+                       .forEach(root.setObject().setArray("keys")::addString);
+            controller.applications().store(application);
+        });
+        return new SlimeJsonResponse(root);
     }
 
     private HttpResponse patchApplication(String tenantName, String applicationName, HttpRequest request) {
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 7cacd91a5c4..fb59a0e51d8 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
@@ -354,7 +354,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/key", POST)
                                       .userIdentity(USER_ID)
                                       .data("{\"key\":\"" + pemPublicKey + "\"}"),
-                              "{\"message\":\"Added deploy key " + quotedPemPublicKey + "\"}");
+                              "{\"keys\":[\"-----BEGIN PUBLIC KEY-----\\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEuKVFA8dXk43kVfYKzkUqhEY2rDT9\\nz/4jKSTHwbYR8wdsOSrJGVEUPbS2nguIJ64OJH7gFnxM6sxUVj+Nm2HlXw==\\n-----END PUBLIC KEY-----\\n\"]}");
 
         // PATCH in a pem deploy key at deprecated path
         tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/instance/default", PATCH)
@@ -377,7 +377,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/key", DELETE)
                                       .userIdentity(USER_ID)
                                       .data("{\"key\":\"" + pemPublicKey + "\"}"),
-                              "{\"message\":\"Removed deploy key " + quotedPemPublicKey + "\"}");
+                              "{\"keys\":[]}");
 
         tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2", GET)
                                       .userIdentity(USER_ID),
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 f2410c47908..b1f5f33b960 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
@@ -143,14 +143,14 @@ public class UserApiTest extends ControllerContainerCloudTest {
         tester.assertResponse(request("/application/v4/tenant/my-tenant/application/my-app/key", POST)
                                       .roles(Set.of(Role.tenantOperator(id.tenant())))
                                       .data("{\"key\":\"" + pemPublicKey + "\"}"),
-                              "{\"message\":\"Added deploy key " + quotedPemPublicKey + "\"}");
+                              new File("first-deploy-key.json"));
 
         // POST a pem developer key
         tester.assertResponse(request("/application/v4/tenant/my-tenant/key", POST)
                                       .user("joe@dev")
                                       .roles(Set.of(Role.tenantOperator(id.tenant())))
                                       .data("{\"key\":\"" + pemPublicKey + "\"}"),
-                              "{\"message\":\"Set developer key " + quotedPemPublicKey + " for joe@dev\"}");
+                              new File("first-developer-key.json"));
 
         // POST the same pem developer key for a different user is forbidden
         tester.assertResponse(request("/application/v4/tenant/my-tenant/key", POST)
@@ -165,7 +165,7 @@ public class UserApiTest extends ControllerContainerCloudTest {
                                       .user("operator@tenant")
                                       .roles(Set.of(Role.tenantOperator(id.tenant())))
                                       .data("{\"key\":\"" + otherPemPublicKey + "\"}"),
-                              "{\"message\":\"Set developer key " + otherQuotedPemPublicKey + " for operator@tenant\"}");
+                              new File("both-developer-keys.json"));
 
         // GET tenant information with keys
         tester.assertResponse(request("/application/v4/tenant/my-tenant/")
@@ -176,7 +176,7 @@ public class UserApiTest extends ControllerContainerCloudTest {
         tester.assertResponse(request("/application/v4/tenant/my-tenant/key", DELETE)
                                       .roles(Set.of(Role.tenantOperator(id.tenant())))
                                       .data("{\"key\":\"" + pemPublicKey + "\"}"),
-                              "{\"message\":\"Removed developer key " + quotedPemPublicKey + " for joe@dev\"}");
+                              new File("second-developer-key.json"));
 
         // DELETE an application role is allowed for an application admin.
         tester.assertResponse(request("/user/v1/tenant/my-tenant/application/my-app", DELETE)
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/both-developer-keys.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/both-developer-keys.json
new file mode 100644
index 00000000000..2ff1c29fe29
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/both-developer-keys.json
@@ -0,0 +1,12 @@
+{
+  "keys": [
+    {
+      "key": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEuKVFA8dXk43kVfYKzkUqhEY2rDT9\nz/4jKSTHwbYR8wdsOSrJGVEUPbS2nguIJ64OJH7gFnxM6sxUVj+Nm2HlXw==\n-----END PUBLIC KEY-----\n",
+      "user": "joe@dev"
+    },
+    {
+      "key": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFELzPyinTfQ/sZnTmRp5E4Ve/sbE\npDhJeqczkyFcT2PysJ5sZwm7rKPEeXDOhzTPCyRvbUqc2SGdWbKUGGa/Yw==\n-----END PUBLIC KEY-----\n",
+      "user": "operator@tenant"
+    }
+  ]
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/first-deploy-key.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/first-deploy-key.json
new file mode 100644
index 00000000000..1c86877b77d
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/first-deploy-key.json
@@ -0,0 +1,5 @@
+{
+  "keys": [
+    "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEuKVFA8dXk43kVfYKzkUqhEY2rDT9\nz/4jKSTHwbYR8wdsOSrJGVEUPbS2nguIJ64OJH7gFnxM6sxUVj+Nm2HlXw==\n-----END PUBLIC KEY-----\n"
+  ]
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/first-developer-key.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/first-developer-key.json
new file mode 100644
index 00000000000..b7d48f283f3
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/first-developer-key.json
@@ -0,0 +1,9 @@
+{
+  "keys": [
+    {
+      "key": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEuKVFA8dXk43kVfYKzkUqhEY2rDT9\nz/4jKSTHwbYR8wdsOSrJGVEUPbS2nguIJ64OJH7gFnxM6sxUVj+Nm2HlXw==\n-----END PUBLIC KEY-----\n",
+      "user": "joe@dev"
+    }
+  ]
+}
+
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/second-developer-key.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/second-developer-key.json
new file mode 100644
index 00000000000..f7d90f31116
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/second-developer-key.json
@@ -0,0 +1,8 @@
+{
+  "keys": [
+    {
+      "key": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFELzPyinTfQ/sZnTmRp5E4Ve/sbE\npDhJeqczkyFcT2PysJ5sZwm7rKPEeXDOhzTPCyRvbUqc2SGdWbKUGGa/Yw==\n-----END PUBLIC KEY-----\n",
+      "user": "operator@tenant"
+    }
+  ]
+}