Merge pull request #24012 from vespa-engine/mpolden/validate-dev-pkg

Validate application package before direct deployment

4 files changed, 15 insertions, 4 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 259c091e074..92c3198175a 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
@@ -179,6 +179,11 @@ public class ApplicationController {
         });
     }
 
+    /** Validate the given application package */
+    public void validatePackage(ApplicationPackage applicationPackage, Application application) {
+        applicationPackageValidator.validate(application, applicationPackage, clock.instant());
+    }
+
     /** Returns the application with the given id, or null if it is not present */
     public Optional<Application> getApplication(TenantAndApplicationId id) {
         return curator.readApplication(id);
@@ -539,7 +544,7 @@ public class ApplicationController {
 
     /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */
     public void storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) {
-        applicationPackageValidator.validate(application.get(), applicationPackage, clock.instant());
+        validatePackage(applicationPackage, application.get());
 
         application = application.with(applicationPackage.deploymentSpec());
         application = application.with(applicationPackage.validationOverrides());
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index 31ac0606a1f..9a84cd7304d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -6,6 +6,7 @@ import com.yahoo.component.Version;
 import com.yahoo.component.VersionCompatibility;
 import com.yahoo.concurrent.UncheckedTimeoutException;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.transaction.Mutex;
 import com.yahoo.vespa.hosted.controller.Application;
@@ -702,7 +703,10 @@ public class JobController {
         controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> {
             if ( ! application.get().instances().containsKey(id.instance()))
                 application = controller.applications().withNewInstance(application, id);
-
+            // TODO(mpolden): Enable for public CD once all tests have been updated
+            if (controller.system() != SystemName.PublicCd) {
+                controller.applications().validatePackage(applicationPackage, application.get());
+            }
             controller.applications().store(application);
         });
 
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 0c84016ad06..dede0c8dec8 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
@@ -696,7 +696,8 @@ public class ControllerTest {
 
     @Test
     void testDevDeployment() {
-        ApplicationPackage applicationPackage = new ApplicationPackageBuilder().build();
+        // A package without deployment.xml is considered valid
+        ApplicationPackage applicationPackage = new ApplicationPackage(new byte[0]);
 
         // Create application
         var context = tester.newDeploymentContext();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
index 674424fbdd9..cfe25232408 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
@@ -413,7 +413,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
     @Test
     void create_application_on_deploy() {
         var application = ApplicationName.from("unique");
-        var applicationPackage = new ApplicationPackageBuilder().withoutAthenzIdentity().build();
+        var applicationPackage = new ApplicationPackageBuilder().trustDefaultCertificate().withoutAthenzIdentity().build();
 
         new ControllerTester(tester).upgradeSystem(new Version("6.1"));
         assertTrue(tester.controller().applications().getApplication(TenantAndApplicationId.from(tenantName, application)).isEmpty());
@@ -473,6 +473,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
 
     private void deployApplication() {
         var applicationPackage = new ApplicationPackageBuilder()
+                .trustDefaultCertificate()
                 .instances("default")
                 .globalServiceId("foo")
                 .region("aws-us-east-1c")