Replace ScrewdriverBuildJob with a boolean

6 files changed, 26 insertions, 77 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 4e55b4857e9..db749713483 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -270,8 +270,7 @@ public class ApplicationController {
                     .map(app -> new LockedApplication(app, lock))
                     .orElseGet(() -> new LockedApplication(createApplication(applicationId, Optional.empty()), lock));
 
-            boolean canDeployDirectly =   ! options.screwdriverBuildJob.map(job1 -> job1.screwdriverId).isPresent()
-                                        ||  zone.environment().isManuallyDeployed();
+            boolean canDeployDirectly = options.deployDirectly || zone.environment().isManuallyDeployed();
             boolean preferOldestVersion = options.deployCurrentVersion;
 
             // Determine versions to use.
@@ -430,7 +429,7 @@ public class ApplicationController {
     }
 
     private DeployOptions withVersion(Version version, DeployOptions options) {
-        return new DeployOptions(options.screwdriverBuildJob,
+        return new DeployOptions(options.deployDirectly,
                                  Optional.of(version),
                                  options.ignoreValidationErrors,
                                  options.deployCurrentVersion);
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 d8ed99f116c..d9848577f0a 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
@@ -35,19 +35,13 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.EnvironmentResource;
 import com.yahoo.vespa.hosted.controller.api.application.v4.TenantResource;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.GitRevision;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.ScrewdriverBuildJob;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RefeedAction;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RestartAction;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ServiceInfo;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
-import com.yahoo.vespa.hosted.controller.api.identifiers.GitBranch;
-import com.yahoo.vespa.hosted.controller.api.identifiers.GitCommit;
-import com.yahoo.vespa.hosted.controller.api.identifiers.GitRepository;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
 import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
-import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
@@ -627,7 +621,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse updateTenant(String tenantName, HttpRequest request) {
         Optional<AthenzTenant> existingTenant = controller.tenants().athenzTenant(TenantName.from(tenantName));
-        if ( ! existingTenant.isPresent()) return ErrorResponse.notFoundError("Tenant '" + tenantName + "' does not exist");;
+        if ( ! existingTenant.isPresent()) return ErrorResponse.notFoundError("Tenant '" + tenantName + "' does not exist");
 
         Inspector requestData = toSlime(request.getData()).get();
         AthenzTenant updatedTenant = existingTenant.get()
@@ -738,7 +732,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         verifyApplicationIdentityConfiguration(tenantName, applicationPackage);
 
         // TODO: get rid of the json object
-        DeployOptions deployOptionsJsonClass = new DeployOptions(screwdriverBuildJobFromSlime(deployOptions.field("screwdriverBuildJob")),
+        DeployOptions deployOptionsJsonClass = new DeployOptions(deployOptions.field("deployDirectly").asBool(),
                                                                  optional("vespaVersion", deployOptions).map(Version::new),
                                                                  deployOptions.field("ignoreValidationErrors").asBool(),
                                                                  deployOptions.field("deployCurrentVersion").asBool());
@@ -1096,21 +1090,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
             array.addString(string);
     }
 
-    // TODO: get rid of the json object
-    private Optional<ScrewdriverBuildJob> screwdriverBuildJobFromSlime(Inspector object) {
-        if ( ! object.valid() ) return Optional.empty();
-        Optional<ScrewdriverId> screwdriverId = optional("screwdriverId", object).map(ScrewdriverId::new);
-        return Optional.of(new ScrewdriverBuildJob(screwdriverId.orElse(null),
-                                                   gitRevisionFromSlime(object.field("gitRevision"))));
-    }
-
-    // TODO: get rid of the json object
-    private GitRevision gitRevisionFromSlime(Inspector object) {
-        return new GitRevision(optional("repository", object).map(GitRepository::new).orElse(null),
-                               optional("branch", object).map(GitBranch::new).orElse(null),
-                               optional("commit", object).map(GitCommit::new).orElse(null));
-    }
-
     private String readToString(InputStream stream) {
         Scanner scanner = new Scanner(stream).useDelimiter("\\A");
         if ( ! scanner.hasNext()) return null;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 2506f35a08b..0de153fc3f9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -14,7 +14,6 @@ import com.yahoo.vespa.athenz.api.NToken;
 import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.ScrewdriverBuildJob;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId;
 import com.yahoo.vespa.hosted.controller.api.integration.BuildService;
@@ -794,7 +793,7 @@ public class ControllerTest {
         // Direct deploy is allowed when project ID is missing
         ZoneId zone = ZoneId.from("prod", "cd-us-central-1");
         // Same options as used in our integration tests
-        DeployOptions options = new DeployOptions(Optional.empty(), Optional.empty(), false,
+        DeployOptions options = new DeployOptions(true, Optional.empty(), false,
                                                   false);
         tester.controller().applications().deploy(app.id(), zone, Optional.of(applicationPackage), options);
 
@@ -870,8 +869,7 @@ public class ControllerTest {
         ControllerTester tester = new ControllerTester();
         String application = "this_application_name_is_far_too_long_and_has_underscores";
         ZoneId zone = ZoneId.from("test", "us-east-1");
-        DeployOptions options = new DeployOptions(Optional.of(new ScrewdriverBuildJob(new ScrewdriverId("123"),
-                                                                                      null)),
+        DeployOptions options = new DeployOptions(false,
                                                   Optional.empty(),
                                                   false,
                                                   false);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
index 52225a4c62e..98189613bd0 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
@@ -11,11 +11,6 @@ import com.yahoo.vespa.athenz.api.AthenzDomain;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.curator.mock.MockCurator;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.GitRevision;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.ScrewdriverBuildJob;
-import com.yahoo.vespa.hosted.controller.api.identifiers.GitBranch;
-import com.yahoo.vespa.hosted.controller.api.identifiers.GitCommit;
-import com.yahoo.vespa.hosted.controller.api.identifiers.GitRepository;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
 import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId;
@@ -240,12 +235,10 @@ public final class ControllerTester {
     }
 
     public void deploy(Application application, ZoneId zone, Optional<ApplicationPackage> applicationPackage, boolean deployCurrentVersion) {
-        ScrewdriverId app1ScrewdriverId = new ScrewdriverId(String.valueOf(application.deploymentJobs().projectId().getAsLong()));
-        GitRevision app1RevisionId = new GitRevision(new GitRepository("repo"), new GitBranch("master"), new GitCommit("commit1"));
         controller().applications().deploy(application.id(),
                                            zone,
                                            applicationPackage,
-                                           new DeployOptions(Optional.of(new ScrewdriverBuildJob(app1ScrewdriverId, app1RevisionId)), Optional.empty(), false, deployCurrentVersion));
+                                           new DeployOptions(false, Optional.empty(), false, deployCurrentVersion));
     }
 
     /** Used by ApplicationSerializerTest to avoid breaking encapsulation. Should not be used by anything else */
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 e776f613880..51437be2832 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
@@ -12,8 +12,6 @@ import com.yahoo.vespa.hosted.controller.ArtifactRepositoryMock;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.TestIdentities;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.GitRevision;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.ScrewdriverBuildJob;
 import com.yahoo.vespa.hosted.controller.api.identifiers.GitBranch;
 import com.yahoo.vespa.hosted.controller.api.identifiers.GitCommit;
 import com.yahoo.vespa.hosted.controller.api.identifiers.GitRepository;
@@ -88,10 +86,8 @@ public class ContainerControllerTester {
     }
 
     public Application deploy(Application application, ApplicationPackage applicationPackage, ZoneId zone, long projectId) {
-        ScrewdriverId app1ScrewdriverId = new ScrewdriverId(String.valueOf(projectId));
-        GitRevision app1RevisionId = new GitRevision(new GitRepository("repo"), new GitBranch("master"), new GitCommit("commit1"));
         controller().applications().deploy(application.id(), zone, Optional.of(applicationPackage),
-                                           new DeployOptions(Optional.of(new ScrewdriverBuildJob(app1ScrewdriverId, app1RevisionId)), Optional.empty(), false, false));
+                                           new DeployOptions(false, Optional.empty(), false, false));
         return application;
     }
 
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 196794df068..407263a7973 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
@@ -9,6 +9,7 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.TenantName;
+import com.yahoo.io.IOUtils;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
@@ -178,7 +179,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         addUserToHostedOperatorRole(HostedAthenzIdentities.from(HOSTED_VESPA_OPERATOR));
 
         // POST (deploy) an application to a zone - manual user deployment
-        HttpEntity entity = createApplicationDeployData(applicationPackage, Optional.empty());
+        HttpEntity entity = createApplicationDeployData(applicationPackage, true);
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default/deploy", POST)
                                       .data(entity)
                                       .userIdentity(USER_ID),
@@ -201,7 +202,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         // ... systemtest
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default/", POST)
-                                      .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)))
+                                      .data(createApplicationDeployData(applicationPackage, false))
                                       .screwdriverIdentity(SCREWDRIVER_ID),
                               new File("deploy-result.json"));
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default", DELETE)
@@ -215,7 +216,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         // ... staging
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default/", POST)
-                                      .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)))
+                                      .data(createApplicationDeployData(applicationPackage, false))
                                       .screwdriverIdentity(SCREWDRIVER_ID),
                               new File("deploy-result.json"));
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default", DELETE)
@@ -228,7 +229,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         // ... prod zone
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/", POST)
-                                      .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)))
+                                      .data(createApplicationDeployData(applicationPackage, false))
                                       .screwdriverIdentity(SCREWDRIVER_ID),
                               new File("deploy-result.json"));
         controllerTester.jobCompletion(DeploymentJobs.JobType.productionCorpUsEast1)
@@ -418,7 +419,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                        new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId("application1"));
 
         // POST (deploy) an application to a prod zone - allowed when project ID is not specified
-        HttpEntity entity = createApplicationDeployData(applicationPackage, Optional.empty());
+        HttpEntity entity = createApplicationDeployData(applicationPackage, true);
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/deploy", POST)
                                       .data(entity)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
@@ -446,7 +447,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               new File("create-user-response.json"));
 
         // POST (deploy) an application to a dev zone
-        HttpEntity entity = createApplicationDeployData(applicationPackage, Optional.empty());
+        HttpEntity entity = createApplicationDeployData(applicationPackage, true);
         tester.assertResponse(request("/application/v4/tenant/by-new-user/application/application1/environment/dev/region/cd-us-central-1/instance/default", POST)
                                       .data(entity)
                                       .userIdentity(userId),
@@ -483,7 +484,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                 .build();
         ApplicationId id = ApplicationId.from("tenant1", "application1", "default");
         long projectId = 1;
-        HttpEntity deployData = createApplicationDeployData(Optional.empty(), Optional.of(projectId));
+        HttpEntity deployData = createApplicationDeployData(Optional.empty(), false);
         startAndTestChange(controllerTester, id, projectId, applicationPackage, deployData, 100);
 
         // us-east-3
@@ -617,7 +618,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Failed to prepare application", ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, null));
         
         // POST (deploy) an application with an invalid application package
-        HttpEntity entity = createApplicationDeployData(applicationPackage, Optional.empty());
+        HttpEntity entity = createApplicationDeployData(applicationPackage, true);
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default/deploy", POST)
                                       .data(entity)
                                       .userIdentity(USER_ID),
@@ -742,7 +743,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               200);
 
         // Deploy to an authorized zone by a user tenant is disallowed
-        HttpEntity entity = createApplicationDeployData(applicationPackage, Optional.empty());
+        HttpEntity entity = createApplicationDeployData(applicationPackage, true);
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default/deploy", POST)
                                       .data(entity)
                                       .userIdentity(USER_ID),
@@ -803,7 +804,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         controllerTester.authorize(ATHENZ_TENANT_DOMAIN, screwdriverId, ApplicationAction.deploy, application);
 
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default/", POST)
-                                      .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)))
+                                      .data(createApplicationDeployData(applicationPackage, false))
                                       .screwdriverIdentity(screwdriverId),
                               "{\"error-code\":\"FORBIDDEN\",\"message\":\"Athenz domain in deployment.xml: [invalid.domain] must match tenant domain: [domain1]\"}",
                               403);
@@ -835,7 +836,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                         .uploadArtifact(applicationPackage)
                         .submit();
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default/", POST)
-                                      .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId)))
+                                      .data(createApplicationDeployData(applicationPackage, false))
                                       .screwdriverIdentity(screwdriverId),
                               new File("deploy-result.json"));
 
@@ -954,39 +955,22 @@ public class ApplicationApiTest extends ControllerContainerTest {
         }
     }
 
-    private HttpEntity createApplicationDeployData(ApplicationPackage applicationPackage, Optional<Long> screwdriverJobId) {
-        return createApplicationDeployData(Optional.of(applicationPackage), screwdriverJobId);
+    private HttpEntity createApplicationDeployData(ApplicationPackage applicationPackage, boolean deployDirectly) {
+        return createApplicationDeployData(Optional.of(applicationPackage), deployDirectly);
     }
 
-    private HttpEntity createApplicationDeployData(Optional<ApplicationPackage> applicationPackage, Optional<Long> screwdriverJobId) {
+    private HttpEntity createApplicationDeployData(Optional<ApplicationPackage> applicationPackage, boolean deployDirectly) {
         MultipartEntityBuilder builder = MultipartEntityBuilder.create();
-        builder.addTextBody("deployOptions", deployOptions(screwdriverJobId), ContentType.APPLICATION_JSON);
+        builder.addTextBody("deployOptions", deployOptions(deployDirectly), ContentType.APPLICATION_JSON);
         applicationPackage.ifPresent(ap -> builder.addBinaryBody("applicationZip", ap.zippedContent()));
         return builder.build();
     }
     
-    private String deployOptions(Optional<Long> screwdriverJobId) {
-        if (screwdriverJobId.isPresent()) // deployment from screwdriver
+    private String deployOptions(boolean deployDirectly) {
             return "{\"vespaVersion\":null," +
                     "\"ignoreValidationErrors\":false," +
-                    "\"screwdriverBuildJob\":{\"screwdriverId\":\"" + screwdriverJobId.get() + "\"," +
-                                             "\"gitRevision\":{\"repository\":\"repository1\"," +
-                                                              "\"branch\":\"master\"," +
-                                                              "\"commit\":\"commit1\"" +
-                                                              "}" +
-                                             "}" +
+                    "\"deployDirectly\":" + deployDirectly +
                     "}";
-        else // This is ugly and evil, but tentatively replicates the existing behavor from the client on user deployments
-            return "{\"vespaVersion\":null," +
-                   "\"ignoreValidationErrors\":false," +
-                   "\"screwdriverBuildJob\":{\"screwdriverId\":null," +
-                                            "\"gitRevision\":{\"repository\":null," +
-                                                             "\"branch\":null," +
-                                                             "\"commit\":null" +
-                                                             "}" +
-                                            "}" +
-                   "}";
-            
     }
 
     private static class RequestBuilder implements Supplier<Request> {