Merge pull request #11033 from vespa-engine/mpolden/app-package-download

Add support for downloading application package

8 files changed, 144 insertions, 19 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationStore.java
index 5cd29fcd48e..44db38c3ec2 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationStore.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationStore.java
@@ -6,6 +6,8 @@ import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneId;
 
+import java.util.Optional;
+
 /**
  * Store for the application and tester packages.
  *
@@ -19,6 +21,12 @@ public interface ApplicationStore {
     /** Returns the tenant application package of the given version. */
     byte[] get(TenantName tenant, ApplicationName application, ApplicationVersion applicationVersion);
 
+    /** Find application package by given build number */
+    default Optional<byte[]> find(TenantName tenant, ApplicationName application, long buildNumber) {
+        // TODO(mpolden): Remove default once all implemenations catch up
+        return Optional.empty();
+    }
+
     /** Stores the given tenant application package of the given version. */
     void put(TenantName tenant, ApplicationName application, ApplicationVersion applicationVersion, byte[] applicationPackage);
 
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
index 31caa192bda..89f0a3c3382 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
@@ -75,6 +75,7 @@ enum PathGroup {
     applicationInfo(Matcher.tenant,
                     Matcher.application,
                     Optional.of("/api"),
+                    "/application/v4/tenant/{tenant}/application/{application}/package",
                     "/application/v4/tenant/{tenant}/application/{application}/deploying/{*}",
                     "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/deploying/{*}",
                     "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/job/{*}",
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 5b11fe1a311..34fe73176c9 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
@@ -19,9 +19,9 @@ import com.yahoo.io.IOUtils;
 import com.yahoo.restapi.ErrorResponse;
 import com.yahoo.restapi.MessageResponse;
 import com.yahoo.restapi.Path;
-import com.yahoo.security.KeyUtils;
 import com.yahoo.restapi.ResourceResponse;
 import com.yahoo.restapi.SlimeJsonResponse;
+import com.yahoo.security.KeyUtils;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
@@ -107,6 +107,7 @@ import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.OptionalLong;
 import java.util.Scanner;
 import java.util.Set;
 import java.util.StringJoiner;
@@ -204,6 +205,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         if (path.matches("/application/v4/tenant/{tenant}/cost/{month}")) return tenantCost(path.get("tenant"), path.get("month"), 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}/package")) return applicationPackage(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return deploying(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deploying(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/metering")) return metering(path.get("tenant"), path.get("application"), request);
@@ -409,6 +411,38 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new SlimeJsonResponse(slime);
     }
 
+    private HttpResponse applicationPackage(String tenantName, String applicationName, HttpRequest request) {
+        var tenantAndApplication = TenantAndApplicationId.from(tenantName, applicationName);
+        var applicationId = ApplicationId.from(tenantName, applicationName, InstanceName.defaultName().value());
+
+        long buildNumber;
+        var requestedBuild = Optional.ofNullable(request.getProperty("build")).map(build -> {
+            try {
+                return Long.parseLong(build);
+            } catch (NumberFormatException e) {
+                throw new IllegalArgumentException("Invalid build number", e);
+            }
+        });
+        if (requestedBuild.isEmpty()) { // Fall back to latest build
+            var application = controller.applications().requireApplication(tenantAndApplication);
+            var latestBuild = application.latestVersion().map(ApplicationVersion::buildNumber).orElse(OptionalLong.empty());
+            if (latestBuild.isEmpty()) {
+                throw new NotExistsException("No application package has been submitted for '" + tenantAndApplication + "'");
+            }
+            buildNumber = latestBuild.getAsLong();
+        } else {
+            buildNumber = requestedBuild.get();
+        }
+        var applicationPackage = controller.applications().applicationStore().find(tenantAndApplication.tenant(), tenantAndApplication.application(), buildNumber);
+        var filename = tenantAndApplication + "-build" + buildNumber + ".zip";
+        if (applicationPackage.isEmpty()) {
+            throw new NotExistsException("No application package found for '" +
+                                         tenantAndApplication +
+                                         "' with build number " + buildNumber);
+        }
+        return new ZipResponse(filename, applicationPackage.get());
+    }
+
     private HttpResponse application(String tenantName, String applicationName, HttpRequest request) {
         Slime slime = new Slime();
         toSlime(slime.setObject(), getApplication(tenantName, applicationName), request);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ZipResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ZipResponse.java
new file mode 100644
index 00000000000..6e9715d59a7
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ZipResponse.java
@@ -0,0 +1,34 @@
+// Copyright 2019 Oath Inc. 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.container.jdisc.HttpResponse;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * A HTTP response containing a named ZIP file.
+ *
+ * @author mpolden
+ */
+public class ZipResponse extends HttpResponse {
+
+    private final byte[] zipContent;
+
+    public ZipResponse(String filename, byte[] zipContent) {
+        super(200);
+        this.zipContent = zipContent;
+        this.headers().add("Content-Disposition", "attachment; filename=\"" + filename + "\"");
+    }
+
+    @Override
+    public String getContentType() {
+        return "application/zip";
+    }
+
+    @Override
+    public void render(OutputStream outputStream) throws IOException {
+        outputStream.write(zipContent);
+    }
+
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java
index 48d573edd05..d915fe06720 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java
@@ -11,10 +11,10 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationV
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId;
 
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 
 import static java.util.Objects.requireNonNull;
-import static org.junit.Assert.assertFalse;
 
 /**
  * Threadsafe.
@@ -40,6 +40,14 @@ public class ApplicationStoreMock implements ApplicationStore {
     }
 
     @Override
+    public Optional<byte[]> find(TenantName tenant, ApplicationName application, long buildNumber) {
+        return store.getOrDefault(appId(tenant, application), Map.of()).entrySet().stream()
+                    .filter(kv -> kv.getKey().buildNumber().orElse(Long.MIN_VALUE) == buildNumber)
+                    .map(Map.Entry::getValue)
+                    .findFirst();
+    }
+
+    @Override
     public void put(TenantName tenant, ApplicationName application, ApplicationVersion applicationVersion, byte[] applicationPackage) {
         store.putIfAbsent(appId(tenant, application), new ConcurrentHashMap<>());
         store.get(appId(tenant, application)).put(applicationVersion, applicationPackage);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
index 80e52f373d7..8a1bbcd09d5 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
@@ -125,7 +125,7 @@ public class ContainerControllerTester {
     }
 
     public void assertResponse(Request request, String expectedResponse, int expectedStatusCode) {
-        containerTester.assertResponse(request, expectedResponse, expectedStatusCode);
+        containerTester.assertResponse(() -> request, expectedResponse, expectedStatusCode);
     }
 
     /*
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
index 7f0b07e4c93..300eddd6291 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
@@ -22,10 +22,12 @@ import java.io.File;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.charset.CharacterCodingException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.time.Instant;
 import java.util.Optional;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.regex.Pattern;
 
@@ -119,27 +121,26 @@ public class ContainerTester {
     }
 
     public void assertResponse(Supplier<Request> request, String expectedResponse) {
-        assertResponse(request.get(), expectedResponse, 200);
+        assertResponse(request, expectedResponse, 200);
     }
 
     public void assertResponse(Request request, String expectedResponse) {
-        assertResponse(request, expectedResponse, 200);
+        assertResponse(() -> request, expectedResponse, 200);
     }
 
     public void assertResponse(Supplier<Request> request, String expectedResponse, int expectedStatusCode) {
-        assertResponse(request.get(), expectedResponse, expectedStatusCode);
+        assertResponse(request,
+                       (response) -> assertEquals(expectedResponse, new String(response.getBody(), StandardCharsets.UTF_8)),
+                       expectedStatusCode);
     }
 
-    public void assertResponse(Request request, String expectedResponse, int expectedStatusCode) {
+    public void assertResponse(Supplier<Request> requestSupplier, Consumer<Response> responseAssertion, int expectedStatusCode) {
+        var request = requestSupplier.get();
         FilterResult filterResult = invokeSecurityFilters(request);
         request = filterResult.request;
         Response response = filterResult.response != null ? filterResult.response : container.handleRequest(request);
-        try {
-            assertEquals(expectedResponse, response.getBodyAsString());
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
         assertEquals("Status code", expectedStatusCode, response.getStatus());
+        responseAssertion.accept(response);
     }
 
     // Hack to run request filters as part of the request processing chain.
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 238a55c0d76..09fd804ea0c 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
@@ -94,6 +94,7 @@ import static com.yahoo.application.container.handler.Request.Method.GET;
 import static com.yahoo.application.container.handler.Request.Method.PATCH;
 import static com.yahoo.application.container.handler.Request.Method.POST;
 import static com.yahoo.application.container.handler.Request.Method.PUT;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -545,9 +546,10 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               "{\"message\":\"Requested restart of tenant1.application1.instance1 in dev.us-central-1\"}");
 
         // POST a 'restart application' command with a host filter (other filters not supported yet)
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/restart?hostname=host1", POST)
+        tester.serviceRegistry().configServerMock().nodeRepository().addFixedNodes(ZoneId.from("prod", "us-central-1"));
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/restart?hostname=hostA", POST)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
-                              "{\"error-code\":\"INTERNAL_SERVER_ERROR\",\"message\":\"No node with the hostname host1 is known.\"}", 500);
+                              "{\"message\":\"Requested restart of tenant1.application1.instance1 in prod.us-central-1\"}", 200);
 
         // GET suspended
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/suspended", GET)
@@ -631,6 +633,14 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .data(createApplicationSubmissionData(applicationPackage)),
                               "{\"message\":\"Application package version: 1.0.43-d00d, source revision of repository 'repo', branch 'master' with commit 'd00d', by a@b, built against 6.1 at 1970-01-01T00:00:01Z\"}");
 
+        // GET application package
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/package", GET).userIdentity(HOSTED_VESPA_OPERATOR),
+                              (response) -> {
+                                  assertEquals("attachment; filename=\"tenant1.application1-build43.zip\"", response.getHeaders().getFirst("Content-Disposition"));
+                                  assertArrayEquals(applicationPackage.zippedContent(), response.getBody());
+                              },
+                              200);
+
         // Second attempt has a service under a different domain than the tenant of the application, and fails.
         ApplicationPackage packageWithServiceForWrongDomain = new ApplicationPackageBuilder()
                 .instances("instance1")
@@ -658,6 +668,22 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .data(createApplicationSubmissionData(packageWithService)),
                               "{\"message\":\"Application package version: 1.0.44-d00d, source revision of repository 'repo', branch 'master' with commit 'd00d', by a@b, built against 6.1 at 1970-01-01T00:00:01Z\"}");
 
+        // GET last submitted application package
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/package", GET).userIdentity(HOSTED_VESPA_OPERATOR),
+                              (response) -> {
+                                  assertEquals("attachment; filename=\"tenant1.application1-build44.zip\"", response.getHeaders().getFirst("Content-Disposition"));
+                                  assertArrayEquals(packageWithService.zippedContent(), response.getBody());
+                              },
+                              200);
+
+        // GET application package for previous build
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/package?build=43", GET).userIdentity(HOSTED_VESPA_OPERATOR),
+                              (response) -> {
+                                  assertEquals("attachment; filename=\"tenant1.application1-build43.zip\"", response.getHeaders().getFirst("Content-Disposition"));
+                                  assertArrayEquals(applicationPackage.zippedContent(), response.getBody());
+                              },
+                              200);
+
         // Fourth attempt has a wrong content hash in a header, and fails.
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/submit", POST)
                                       .screwdriverIdentity(SCREWDRIVER_ID)
@@ -1138,6 +1164,21 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         ConfigServerMock configServer = serviceRegistry().configServerMock();
         configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Failed to prepare application", ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, null));
+
+        // GET non-existent application package
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/package", GET).userIdentity(HOSTED_VESPA_OPERATOR),
+                              "{\"error-code\":\"NOT_FOUND\",\"message\":\"No application package has been submitted for 'tenant1.application1'\"}",
+                              404);
+
+        // GET non-existent application package of specific build
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/package?build=42", GET).userIdentity(HOSTED_VESPA_OPERATOR),
+                              "{\"error-code\":\"NOT_FOUND\",\"message\":\"No application package found for 'tenant1.application1' with build number 42\"}",
+                              404);
+
+        // GET non-existent application package of invalid build
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/package?build=foobar", GET).userIdentity(HOSTED_VESPA_OPERATOR),
+                              "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid build number: For input string: \\\"foobar\\\"\"}",
+                              400);
         
         // POST (deploy) an application with an invalid application package
         MultiPartStreamer entity = createApplicationDeployData(applicationPackageInstance1, true);
@@ -1555,10 +1596,9 @@ public class ApplicationApiTest extends ControllerContainerTest {
         job.type(JobType.systemTest).submit();
 
         // Notifying about job started not by the controller fails
-        Request request = request("/application/v4/tenant/tenant1/application/application1/jobreport", POST)
+        var request = request("/application/v4/tenant/tenant1/application/application1/jobreport", POST)
                 .data(asJson(job.type(JobType.systemTest).report()))
-                .userIdentity(HOSTED_VESPA_OPERATOR)
-                .get();
+                .userIdentity(HOSTED_VESPA_OPERATOR);
         tester.assertResponse(request, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Notified of completion " +
                                        "of system-test for tenant1.application1, but that has not been triggered; last was " +
                                        controllerTester.controller().applications().requireInstance(app.id().defaultInstance()).deploymentJobs().jobStatus().get(JobType.systemTest).lastTriggered().get().at() + "\"}", 400);
@@ -1566,8 +1606,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         // Notifying about unknown job fails
         request = request("/application/v4/tenant/tenant1/application/application1/jobreport", POST)
                 .data(asJson(job.type(JobType.productionUsEast3).report()))
-                .userIdentity(HOSTED_VESPA_OPERATOR)
-                .get();
+                .userIdentity(HOSTED_VESPA_OPERATOR);
         tester.assertResponse(request, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Notified of completion " +
                                        "of production-us-east-3 for tenant1.application1, but that has not been triggered; last was never\"}",
                               400);