From 6070488dc485b1c481f74329f30fad369973b604 Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@yahooinc.com>
Date: Fri, 22 Sep 2023 15:27:10 +0200
Subject: Fail when config has url and is not in public (no matter exclusive
 nodes or not)

---
 .../application/validation/UrlConfigValidator.java | 39 ++++++++++++----------
 .../validation/UrlConfigValidatorTest.java         |  7 +++-
 2 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java
index 64856e0a61f..9b26c63dba7 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java
@@ -19,33 +19,36 @@ public class UrlConfigValidator extends Validator {
         if (! state.isHostedTenantApplication(model.getAdmin().getApplicationType())) return;
 
         model.getContainerClusters().forEach((__, cluster) -> {
-            var isExclusive = model.hostSystem().getHosts()
-                    .stream()
-                    .map(hostResource -> hostResource.spec().membership())
-                    .filter(Optional::isPresent)
-                    .map(Optional::get)
-                    .filter(membership -> membership.cluster().id().equals(cluster.id()))
-                    .anyMatch(membership -> membership.cluster().isExclusive());
-
-            if (! isExclusive)
-                validateS3UlsInConfig(state, cluster);
+            var isExclusive = hasExclusiveNodes(model, cluster);
+            validateS3UlsInConfig(state, cluster, isExclusive);
         });
     }
 
-    private static void validateS3UlsInConfig(DeployState state, ApplicationContainerCluster cluster) {
-        var match = state.getFileRegistry().export().stream()
-                .filter(fileReference -> fileReference.relativePath.startsWith("s3://"))
-                .findFirst();
+    private static boolean hasExclusiveNodes(VespaModel model, ApplicationContainerCluster cluster) {
+        return model.hostSystem().getHosts()
+                .stream()
+                .map(hostResource -> hostResource.spec().membership())
+                .filter(Optional::isPresent)
+                .map(Optional::get)
+                .filter(membership -> membership.cluster().id().equals(cluster.id()))
+                .anyMatch(membership -> membership.cluster().isExclusive());
+    }
 
-        if (match.isPresent()) {
+    private static void validateS3UlsInConfig(DeployState state, ApplicationContainerCluster cluster, boolean isExclusive) {
+        if (hasUrlInConfig(state)) {
             // TODO: Would be even better if we could add which config/field the url is set for in the error message
             String message = "Found s3:// urls in config for container cluster " + cluster.getName();
-            if (state.zone().system().isPublic())
+            if ( ! state.zone().system().isPublic())
+                throw new IllegalArgumentException(message + ". This is only supported in public systems");
+            else if ( ! isExclusive)
                 throw new IllegalArgumentException(message + ". Nodes in the cluster need to be 'exclusive'," +
                                                            " see https://cloud.vespa.ai/en/reference/services#nodes");
-            else
-                throw new IllegalArgumentException(message + ". This is only supported in public systems");
         }
     }
 
+    private static boolean hasUrlInConfig(DeployState state) {
+        return state.getFileRegistry().export().stream()
+                .anyMatch(fileReference -> fileReference.relativePath.startsWith("s3://"));
+    }
+
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java
index 0f5f081d2b0..cef4d8c27dd 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java
@@ -29,13 +29,18 @@ public class UrlConfigValidatorTest {
 
     @Test
     void failsWhenContainerNodesNotExclusive() throws IOException, SAXException {
-        runValidatorOnApp(true, SystemName.Public);  // Exclusive nodes => success
+        runValidatorOnApp(true, SystemName.Public);  // Exclusive nodes in public => success
 
         assertEquals("Found s3:// urls in config for container cluster default. This is only supported in public systems",
                      assertThrows(IllegalArgumentException.class,
                                   () -> runValidatorOnApp(false, SystemName.main))
                              .getMessage());
 
+        assertEquals("Found s3:// urls in config for container cluster default. This is only supported in public systems",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> runValidatorOnApp(true, SystemName.main))
+                             .getMessage());
+
         assertEquals("Found s3:// urls in config for container cluster default. Nodes in the cluster need to be 'exclusive',"
                      + " see https://cloud.vespa.ai/en/reference/services#nodes",
                      assertThrows(IllegalArgumentException.class,
-- 
cgit v1.2.3