Support deploy key only with instance, major version only without in app/v4

6 files changed, 35 insertions, 31 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 c58bc5b97b1..5c629173f3d 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
@@ -819,6 +819,23 @@ public class ApplicationController {
     /**
      * Acquire a locked application to modify and store, or throw an exception if no application has the given id.
      *
+     * WARNING: Uses only the "default" instance.
+     *
+     * @param applicationId ID of the application to lock and require.
+     * @param action Function which acts on the locked application.
+     * @throws IllegalArgumentException when application does not exist.
+     */
+    public void lockApplicationOrThrow(TenantAndApplicationId applicationId, Consumer<LockedApplication> action) {
+        ApplicationId instanceId = applicationId.defaultInstance();
+        try (Lock lock = lock(applicationId);
+             Lock oldLock = lock(instanceId)) {
+            action.accept(new LockedApplication(requireApplication(instanceId), lock));
+        }
+    }
+
+    /**
+     * Acquire a locked application to modify and store, or throw an exception if no application has the given id.
+     *
      * @param applicationId ID of the application to lock and require.
      * @param action Function which acts on the locked application.
      * @throws IllegalArgumentException when application does not exist.
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedInstance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedInstance.java
index 4de1b7ac7ac..a650c7109da 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedInstance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedInstance.java
@@ -188,27 +188,6 @@ public class LockedInstance {
                                   metrics, pemDeployKey, rotations, rotationStatus);
     }
 
-    public LockedInstance withOwnershipIssueId(IssueId issueId) {
-        return new LockedInstance(lock, id, createdAt, deploymentSpec, validationOverrides, deployments,
-                                  deploymentJobs, change, outstandingChange, Optional.ofNullable(issueId), owner,
-                                  majorVersion, metrics, pemDeployKey, rotations, rotationStatus);
-    }
-
-    public LockedInstance withOwner(User owner) {
-        return new LockedInstance(lock, id, createdAt, deploymentSpec, validationOverrides, deployments,
-                                  deploymentJobs, change, outstandingChange, ownershipIssueId,
-                                  Optional.ofNullable(owner), majorVersion, metrics, pemDeployKey,
-                                  rotations, rotationStatus);
-    }
-
-    /** Set a major version for this, or set to null to remove any major version override */
-    public LockedInstance withMajorVersion(Integer majorVersion) {
-        return new LockedInstance(lock, id, createdAt, deploymentSpec, validationOverrides, deployments,
-                                  deploymentJobs, change, outstandingChange, ownershipIssueId, owner,
-                                     majorVersion == null ? OptionalInt.empty() : OptionalInt.of(majorVersion),
-                                  metrics, pemDeployKey, rotations, rotationStatus);
-    }
-
     public LockedInstance with(ApplicationMetrics metrics) {
         return new LockedInstance(lock, id, createdAt, deploymentSpec, validationOverrides, deployments,
                                   deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion,
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java
index d94004eda2b..d829364a421 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java
@@ -108,7 +108,7 @@ public class ApplicationOwnershipConfirmer extends Maintainer {
                     .forEach(application -> {
                     IssueId ownershipIssueId = application.ownershipIssueId().get();
                     ownershipIssues.getConfirmedOwner(ownershipIssueId).ifPresent(owner -> {
-                        controller().applications().lockIfPresent(application.id(), lockedApplication ->
+                        controller().applications().lockApplicationIfPresent(application.id(), lockedApplication ->
                                 controller().applications().store(lockedApplication.withOwner(owner)));
                     });
                 });
@@ -130,7 +130,7 @@ public class ApplicationOwnershipConfirmer extends Maintainer {
     }
 
     protected void store(IssueId issueId, ApplicationId applicationId) {
-        controller().applications().lockIfPresent(applicationId, application ->
+        controller().applications().lockApplicationIfPresent(applicationId, application ->
                 controller().applications().store(application.withOwnershipIssueId(issueId)));
     }
 }
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 7585d372bc1..d352c560097 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
@@ -64,6 +64,7 @@ import com.yahoo.vespa.hosted.controller.application.Endpoint;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.application.RoutingPolicy;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
+import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel;
 import com.yahoo.vespa.hosted.controller.deployment.TestConfigSerializer;
@@ -261,7 +262,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     }
 
     private HttpResponse handlePATCH(Path path, HttpRequest request) {
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return patchApplication(path.get("tenant"), path.get("application"), "default", request);
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return patchApplication(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return patchApplication(path.get("tenant"), path.get("application"), path.get("instance"), request);
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
@@ -363,24 +364,31 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new SlimeJsonResponse(slime);
     }
 
-    private HttpResponse patchApplication(String tenantName, String applicationName, String instanceName, HttpRequest request) {
+    private HttpResponse patchApplication(String tenantName, String applicationName, HttpRequest request) {
         Inspector requestObject = toSlime(request.getData()).get();
         StringJoiner messageBuilder = new StringJoiner("\n").setEmptyValue("No applicable changes.");
-        controller.applications().lockOrThrow(ApplicationId.from(tenantName, applicationName, instanceName), application -> {
+        controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(tenantName, applicationName), application -> {
             Inspector majorVersionField = requestObject.field("majorVersion");
             if (majorVersionField.valid()) {
                 Integer majorVersion = majorVersionField.asLong() == 0 ? null : (int) majorVersionField.asLong();
                 application = application.withMajorVersion(majorVersion);
                 messageBuilder.add("Set major version to " + (majorVersion == null ? "empty" : majorVersion));
             }
+            controller.applications().store(application);
+        });
+        return new MessageResponse(messageBuilder.toString());
+    }
 
+    private HttpResponse patchApplication(String tenantName, String applicationName, String instanceName, HttpRequest request) {
+        Inspector requestObject = toSlime(request.getData()).get();
+        StringJoiner messageBuilder = new StringJoiner("\n").setEmptyValue("No applicable changes.");
+        controller.applications().lockOrThrow(ApplicationId.from(tenantName, applicationName, instanceName), application -> {
             Inspector pemDeployKeyField = requestObject.field("pemDeployKey");
             if (pemDeployKeyField.valid()) {
                 String pemDeployKey = pemDeployKeyField.type() == Type.NIX ? null : pemDeployKeyField.asString();
                 application = application.withPemDeployKey(pemDeployKey);
                 messageBuilder.add("Set pem deploy key to " + (pemDeployKey == null ? "empty" : pemDeployKey));
             }
-
             controller.applications().store(application);
         });
         return new MessageResponse(messageBuilder.toString());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
index 498c20e69c5..6c1e0f7abb2 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
@@ -958,7 +958,7 @@ public class UpgraderTest {
         // Setup applications
         Application canary0 = tester.createAndDeploy("canary0", 1, "canary");
         Application default0 = tester.createAndDeploy("default0", 2, default0ApplicationPackage);
-        tester.applications().lockOrThrow(default0.id(), a -> tester.applications().store(a.withMajorVersion(6)));
+        tester.applications().lockApplicationOrThrow(default0.id(), a -> tester.applications().store(a.withMajorVersion(6)));
         assertEquals(OptionalInt.of(6), tester.applications().get(default0.id()).get().majorVersion());
 
         // New major version is released
@@ -1285,7 +1285,7 @@ public class UpgraderTest {
         tester.upgradeSystem(version2);
 
         // App 2 is allowed on new major and upgrades
-        tester.controller().applications().lockIfPresent(app2.id(), app -> tester.applications().store(app.withMajorVersion(7)));
+        tester.controller().applications().lockApplicationIfPresent(app2.id(), app -> tester.applications().store(app.withMajorVersion(7)));
         tester.upgrader().maintain();
         assertEquals(version2, tester.controller().applications().require(app2.id()).change().platform().get());
 
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 6eaccc3038b..677bd36e4f2 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
@@ -345,7 +345,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               "{\"message\":\"Set major version to 7\"}");
 
         // PATCH in a pem deploy key
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2", PATCH)
+        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/instance/default", PATCH)
                                       .userIdentity(USER_ID)
                                       .data("{\"pemDeployKey\":\"-----BEGIN PUBLIC KEY-----\n∠( ᐛ 」∠)＿\n-----END PUBLIC KEY-----\"}"),
                               "{\"message\":\"Set pem deploy key to -----BEGIN PUBLIC KEY-----\\n∠( ᐛ 」∠)＿\\n-----END PUBLIC KEY-----\"}");
@@ -362,7 +362,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               "{\"message\":\"Set major version to empty\"}");
 
         // PATCH in removal of the pem deploy key
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2", PATCH)
+        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/instance/default", PATCH)
                                       .userIdentity(USER_ID)
                                       .data("{\"pemDeployKey\":null}"),
                               "{\"message\":\"Set pem deploy key to empty\"}");