Merge pull request #11806 from vespa-engine/jvenstad/verify-instance-names-from-deployment-spec

Validate instances names from all sources

4 files changed, 16 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 a5821812ce9..dfc9574fcd7 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
@@ -156,7 +156,7 @@ public class ApplicationController {
                     else
                         for (InstanceName instance : application.get().deploymentSpec().instanceNames())
                             if ( ! application.get().instances().keySet().contains(instance))
-                                application = application.withNewInstance(instance);
+                                application = withNewInstance(application, id.instance(instance));
                     store(application);
                 });
                 count++;
@@ -316,7 +316,7 @@ public class ApplicationController {
         });
     }
 
-    private LockedApplication withNewInstance(LockedApplication application, ApplicationId id) {
+    public LockedApplication withNewInstance(LockedApplication application, ApplicationId id) {
         if (id.instance().isTester())
             throw new IllegalArgumentException("'" + id + "' is a tester application!");
         InstanceId.validate(id.instance().value());
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
index 3c5bc3b98ad..35eb18ae5d8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
@@ -89,7 +89,7 @@ public class LockedApplication {
                                projectId, latestVersion, instances.values());
     }
 
-    public LockedApplication withNewInstance(InstanceName instance) {
+    LockedApplication withNewInstance(InstanceName instance) {
         var instances = new HashMap<>(this.instances);
         instances.put(instance, new Instance(id.instance(instance)));
         return new LockedApplication(lock, id, createdAt, deploymentSpec, 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 c6f54b1fe5c..ba61e8813e1 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
@@ -430,7 +430,7 @@ public class JobController {
 
         controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> {
             if ( ! application.get().instances().containsKey(id.instance()))
-                application = application.withNewInstance(id.instance());
+                application = controller.applications().withNewInstance(application, id);
 
             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 bc98775c00e..1b92a3df03a 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
@@ -117,6 +117,18 @@ public class ControllerTest {
 
         assertEquals(4, context.instanceJobs().size());
 
+        // Instance with uppercase characters is not allowed.
+        applicationPackage = new ApplicationPackageBuilder()
+                .instances("hellO")
+                .build();
+        try {
+            context.submit(applicationPackage);
+            fail("Expected exception due to illegal deployment spec.");
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("Invalid id 'hellO'. Tenant, application and instance names must start with a letter, may contain no more than 20 characters, and may only contain lowercase letters, digits or dashes, but no double-dashes.", e.getMessage());
+        }
+
         // Production zone for which there is no JobType is not allowed.
         applicationPackage = new ApplicationPackageBuilder()
                 .environment(Environment.prod)