Move operator API

5 files changed, 67 insertions, 36 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 30f5f6462ae..d7b805c0949 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
@@ -309,7 +309,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         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}/access/support")) return allowSupportAccess(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}/access/support/grant")) return grantAccess(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);
@@ -995,26 +994,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         return new SlimeJsonResponse(SupportAccessSerializer.toSlime(allowed, false, Optional.of(now)));
     }
 
-    private HttpResponse grantAccess(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
-        DeploymentId deployment = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region));
-        Principal principal = requireUserPrincipal(request);
-        Instant now = controller.clock().instant();
-
-        Inspector requestObject = toSlime(request.getData()).get();
-        X509Certificate certificate = X509CertificateUtils.fromPem(requestObject.field("certificate").asString());
-
-        // Register grant
-        SupportAccess supportAccess = controller.supportAccess().registerGrant(deployment, principal.getName(), certificate);
-
-        // Trigger deployment to include operator cert
-        JobType jobType = JobType.from(controller.system(), deployment.zoneId())
-                .orElseThrow(() -> new IllegalStateException("No job found to trigger for " + deployment.toUserFriendlyString()));
-
-        String jobName = controller.applications().deploymentTrigger()
-                .reTrigger(deployment.applicationId(), jobType).type().jobName();
-        return new MessageResponse(String.format("Operator %s granted access and job %s triggered", principal.getName(), jobName));
-    }
-
     private HttpResponse disallowSupportAccess(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         DeploymentId deployment = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region));
         Principal principal = requireUserPrincipal(request);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
index 2b2087fccf4..9222f83ae1d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
@@ -10,22 +10,30 @@ import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
 import com.yahoo.io.IOUtils;
 import com.yahoo.restapi.ErrorResponse;
+import com.yahoo.restapi.MessageResponse;
 import com.yahoo.restapi.Path;
 import com.yahoo.restapi.ResourceResponse;
+import com.yahoo.security.X509CertificateUtils;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.SlimeUtils;
 import com.yahoo.vespa.athenz.api.AthenzUser;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.auditlog.AuditLoggingRequestHandler;
 import com.yahoo.vespa.hosted.controller.maintenance.ControllerMaintenance;
 import com.yahoo.vespa.hosted.controller.maintenance.Upgrader;
+import com.yahoo.vespa.hosted.controller.support.access.SupportAccess;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence;
 import com.yahoo.yolean.Exceptions;
 
+import javax.ws.rs.InternalServerErrorException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UncheckedIOException;
+import java.security.Principal;
+import java.security.cert.X509Certificate;
+import java.time.Instant;
 import java.util.Optional;
 import java.util.Scanner;
 import java.util.logging.Level;
@@ -83,6 +91,7 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler {
         Path path = new Path(request.getUri());
         if (path.matches("/controller/v1/jobs/upgrader/confidence/{version}")) return overrideConfidence(request, path.get("version"));
         if (path.matches("/controller/v1/access/requests/{user}")) return approveMembership(request, path.get("user"));
+        if (path.matches("/controller/v1/access/grants/{user}")) return grantAccess(request, path.get("user"));
         return notFound(path);
     }
 
@@ -92,8 +101,34 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler {
         Inspector inspector = SlimeUtils.jsonToSlime(jsonBytes).get();
         ApplicationId applicationId = ApplicationId.fromSerializedForm(inspector.field("applicationId").asString());
         ZoneId zone = ZoneId.from(inspector.field("zone").asString());
-        controller.supportAccess().allowDataplaneMembership(athenzUser, new DeploymentId(applicationId, zone));
-        return new AccessRequestResponse(controller.serviceRegistry().accessControlService().listMembers());
+        if(controller.supportAccess().allowDataplaneMembership(athenzUser, new DeploymentId(applicationId, zone))) {
+            return new AccessRequestResponse(controller.serviceRegistry().accessControlService().listMembers());
+        } else {
+            return new MessageResponse(400, "Unable to approve membership request");
+        }
+    }
+
+    private HttpResponse grantAccess(HttpRequest request, String user) {
+        Principal principal = requireUserPrincipal(request);
+        Instant now = controller.clock().instant();
+
+        byte[] jsonBytes = toJsonBytes(request.getData());
+        Inspector requestObject = SlimeUtils.jsonToSlime(jsonBytes).get();
+        X509Certificate certificate = X509CertificateUtils.fromPem(requestObject.field("certificate").asString());
+        ApplicationId applicationId = ApplicationId.fromSerializedForm(requestObject.field("applicationId").asString());
+        ZoneId zone = ZoneId.from(requestObject.field("zone").asString());
+        DeploymentId deployment = new DeploymentId(applicationId, zone);
+
+        // Register grant
+        SupportAccess supportAccess = controller.supportAccess().registerGrant(deployment, principal.getName(), certificate);
+
+        // Trigger deployment to include operator cert
+        JobType jobType = JobType.from(controller.system(), deployment.zoneId())
+                .orElseThrow(() -> new IllegalStateException("No job found to trigger for " + deployment.toUserFriendlyString()));
+
+        String jobName = controller.applications().deploymentTrigger()
+                .reTrigger(deployment.applicationId(), jobType).type().jobName();
+        return new MessageResponse(String.format("Operator %s granted access and job %s triggered", principal.getName(), jobName));
     }
 
     private HttpResponse delete(HttpRequest request) {
@@ -161,4 +196,9 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler {
         }
     }
 
+    private static Principal requireUserPrincipal(HttpRequest request) {
+        Principal principal = request.getJDiscRequest().getUserPrincipal();
+        if (principal == null) throw new InternalServerErrorException("Expected a user principal");
+        return principal;
+    }
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java
index 585cd609f53..ccee1b4af43 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java
@@ -92,14 +92,14 @@ public class SupportAccessControl {
                 .collect(Collectors.toUnmodifiableList());
     }
 
-    public void allowDataplaneMembership(AthenzUser identity, DeploymentId deploymentId) {
+    public boolean allowDataplaneMembership(AthenzUser identity, DeploymentId deploymentId) {
         Instant instant = controller.clock().instant();
         SupportAccess supportAccess = forDeployment(deploymentId);
         SupportAccess.CurrentStatus currentStatus = supportAccess.currentStatus(instant);
         if(currentStatus.state() == ALLOWED) {
-            controller.serviceRegistry().accessControlService().approveDataPlaneAccess(identity, currentStatus.allowedUntil().orElse(instant.plus(MAX_SUPPORT_ACCESS_TIME)));
+            return controller.serviceRegistry().accessControlService().approveDataPlaneAccess(identity, currentStatus.allowedUntil().orElse(instant.plus(MAX_SUPPORT_ACCESS_TIME)));
         } else {
-            throw new IllegalStateException("Not allowed");
+            return false;
         }
     }
 }
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 f1e54033792..ce7b4a6123b 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
@@ -1531,13 +1531,16 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         // Grant access to support user
         X509Certificate support_cert = grantCertificate(now, now.plusSeconds(3600));
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/access/support/grant", POST)
-                                      .data("{\"certificate\":\""+X509CertificateUtils.toPem(support_cert)+"\"}")
+        String grantPayload= "{\n" +
+                             "  \"applicationId\": \"tenant1:application1:instance1\",\n" +
+                             "  \"zone\": \"prod.us-west-1\",\n" +
+                             "  \"certificate\":\""+X509CertificateUtils.toPem(support_cert)+ "\"\n" +
+                             "}";
+        tester.assertResponse(request("/controller/v1/access/grants/"+HOSTED_VESPA_OPERATOR.id(), POST)
+                                      .data(grantPayload)
                                       .userIdentity(HOSTED_VESPA_OPERATOR),
                               "{\"message\":\"Operator user.johnoperator granted access and job production-us-west-1 triggered\"}");
 
-        //tester.controller().supportAccess().registerGrant(app.deploymentIdIn(zone), "user.andreer", support_cert);
-
         // GET shows grant
         String grantResponse = allowedResponse.replaceAll("\"grants\":\\[]",
                 "\"grants\":[{\"requestor\":\"user.johnoperator\",\"notBefore\":\"" + serializeInstant(now) + "\",\"notAfter\":\"" + serializeInstant(now.plusSeconds(3600)) + "\"}]");
@@ -1547,11 +1550,9 @@ public class ApplicationApiTest extends ControllerContainerTest {
         );
 
         // DELETE removes access
-        System.out.println("grantresponse:\n"+grantResponse+"\n");
         String disallowedResponse = grantResponse
                 .replaceAll("ALLOWED\".*?}", "NOT_ALLOWED\"}")
                 .replace("history\":[", "history\":[{\"state\":\"disallowed\",\"at\":\""+ serializeInstant(now) +"\",\"by\":\"user.myuser\"},");
-        System.out.println("disallowedResponse:\n"+disallowedResponse+"\n");
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/access/support", DELETE)
                         .userIdentity(USER_ID),
                 disallowedResponse, 200
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
index 74b609a495e..fc83c58cc67 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
@@ -9,6 +9,7 @@ import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.athenz.api.AthenzUser;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.flags.PermanentFlags;
+import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.MockAccessControlService;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Application;
 import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapshot;
@@ -169,13 +170,23 @@ public class ControllerApiTest extends ControllerContainerTest {
 
     @Test
     public void testApproveMembership() {
-        // TODO Migrate test to use MockZmsClient
+        ApplicationId applicationId = ApplicationId.from("tenant", "app", "instance");
+        DeploymentId deployment = new DeploymentId(applicationId, ZoneId.defaultId());
+        String requestBody = "{\n" +
+                             " \"applicationId\": \"" + deployment.applicationId().serializedForm() + "\",\n" +
+                             " \"zone\": \"" + deployment.zoneId().value() + "\"\n" +
+                             "}";
+
         MockAccessControlService accessControlService = (MockAccessControlService) tester.serviceRegistry().accessControlService();
-        tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/access/requests/"+hostedOperator.getName(), "", Request.Method.POST),
-                              "{\"members\":[]}");
+        tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/access/requests/"+hostedOperator.getName(), requestBody, Request.Method.POST),
+                              "{\"message\":\"Unable to approve membership request\"}", 400);
 
         accessControlService.addPendingMember(hostedOperator);
-        tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/access/requests/"+hostedOperator.getName(), "", Request.Method.POST),
+        tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/access/requests/"+hostedOperator.getName(), requestBody, Request.Method.POST),
+                              "{\"message\":\"Unable to approve membership request\"}", 400);
+
+        tester.controller().supportAccess().allow(deployment, Instant.now().plus(Duration.ofHours(1)), "tenantx");
+        tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/access/requests/"+hostedOperator.getName(), requestBody, Request.Method.POST),
                               "{\"members\":[\"user.alice\"]}");
     }
 }