Merge pull request #25020 from vespa-engine/jonmv/vpc-endpoint-services

Jonmv/vpc endpoint services

12 files changed, 59 insertions, 40 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index 798ca55bdfa..79d8512e447 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -842,7 +842,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
 
     private LoadBalancerSettings loadBalancerSettings(Element loadBalancerElement) {
         List<String> allowedUrnElements = XML.getChildren(XML.getChild(loadBalancerElement, "private-access"),
-                                                          "allowed-urn")
+                                                          "allow-urn")
                                              .stream().map(XML::getValue).toList();
         return new LoadBalancerSettings(allowedUrnElements);
     }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
index fafd4bb1ff5..6ef7b9f5b56 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
@@ -208,8 +208,8 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
         verifyAllowedUrns("""
                           <load-balancer>
                             <private-access>
-                              <allowed-urn>foo</allowed-urn>
-                              <allowed-urn>bar</allowed-urn>
+                              <allow-urn>foo</allow-urn>
+                              <allow-urn>bar</allow-urn>
                             </private-access>
                           </load-balancer>
                           """);
@@ -218,8 +218,8 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
         verifyAllowedUrns("""
                           <load-balancer>
                             <private-access>
-                              <allowed-urn>foo</allowed-urn>
-                              <allowed-urn>bar</allowed-urn>
+                              <allow-urn>foo</allow-urn>
+                              <allow-urn>bar</allow-urn>
                             </private-access>
                           </load-balancer>
                           <nodes count='2' />
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
index 7a633367fc0..afa3c0a4062 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
@@ -34,10 +34,10 @@ public class ClusterMembership {
             for (int i = 4; i < components.length; i++) {
                 String component = components[i];
                 switch (component) {
-                    case "exclusive": exclusive = true; break;
-                    case "retired": retired = true; break;
-                    case "stateful": stateful = true; break;
-                    default: combinedId = Optional.of(component); break;
+                    case "exclusive" -> exclusive = true;
+                    case "retired" -> retired = true;
+                    case "stateful" -> stateful = true;
+                    default -> combinedId = Optional.of(component);
                 }
             }
         }
diff --git a/configserver/src/test/apps/hosted/services.xml b/configserver/src/test/apps/hosted/services.xml
index f1435d8cc4f..22bccee9f5a 100644
--- a/configserver/src/test/apps/hosted/services.xml
+++ b/configserver/src/test/apps/hosted/services.xml
@@ -15,6 +15,11 @@
     </http>
     <search/>
     <nodes count='1'/>
+    <load-balancer>
+      <private-access>
+        <allow-urn>burn</allow-urn>
+      </private-access>
+    </load-balancer>
   </container>
 
   <content id="music" version="1.0">
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java
index fde3c85bdab..36e42fcf541 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java
@@ -24,11 +24,11 @@ public class LoadBalancerInstance {
     private final Set<Integer> ports;
     private final Set<String> networks;
     private final Set<Real> reals;
-    private final LoadBalancerSettings settings;
+    private final Optional<LoadBalancerSettings> settings;
     private final CloudAccount cloudAccount;
 
     public LoadBalancerInstance(Optional<DomainName> hostname, Optional<String> ipAddress, Optional<DnsZone> dnsZone, Set<Integer> ports,
-                                Set<String> networks, Set<Real> reals, LoadBalancerSettings settings, CloudAccount cloudAccount) {
+                                Set<String> networks, Set<Real> reals, Optional<LoadBalancerSettings> settings, CloudAccount cloudAccount) {
         this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null");
         this.ipAddress = Objects.requireNonNull(ipAddress, "ip must be non-null");
         this.dnsZone = Objects.requireNonNull(dnsZone, "dnsZone must be non-null");
@@ -74,8 +74,8 @@ public class LoadBalancerInstance {
         return reals;
     }
 
-    /** Static user-configured settings of this load balancer */
-    public LoadBalancerSettings settings() {
+    /** Static user-configured settings of this load balancer, if set */
+    public Optional<LoadBalancerSettings> settings() {
         return settings;
     }
 
@@ -89,6 +89,11 @@ public class LoadBalancerInstance {
         return new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, cloudAccount);
     }
 
+    /** Returns a copy of this with the given settings */
+    public LoadBalancerInstance withSettings(LoadBalancerSettings settings) {
+        return new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, Optional.of(settings), cloudAccount);
+    }
+
     private static Set<Integer> requirePorts(Set<Integer> ports) {
         Objects.requireNonNull(ports, "ports must be non-null");
         if (ports.isEmpty()) {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerSpec.java
index c4a6f6b2a37..dca6d434330 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerSpec.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerSpec.java
@@ -8,6 +8,7 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.LoadBalancerSettings;
 
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 
 /**
@@ -20,7 +21,7 @@ public class LoadBalancerSpec {
     private final ApplicationId application;
     private final ClusterSpec.Id cluster;
     private final Set<Real> reals;
-    private final LoadBalancerSettings settings;
+    private final Optional<LoadBalancerSettings> settings;
     private final CloudAccount cloudAccount;
 
     public LoadBalancerSpec(ApplicationId application, ClusterSpec.Id cluster, Set<Real> reals,
@@ -28,7 +29,7 @@ public class LoadBalancerSpec {
         this.application = Objects.requireNonNull(application);
         this.cluster = Objects.requireNonNull(cluster);
         this.reals = ImmutableSortedSet.copyOf(Objects.requireNonNull(reals));
-        this.settings = Objects.requireNonNull(settings);
+        this.settings = Optional.ofNullable(settings);
         this.cloudAccount = Objects.requireNonNull(cloudAccount);
     }
 
@@ -48,7 +49,7 @@ public class LoadBalancerSpec {
     }
 
     /** Static user-configured settings for this load balancer. */
-    public LoadBalancerSettings settings() {
+    public Optional<LoadBalancerSettings> settings() {
         return settings;
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java
index d9032bd0a5b..13407a83f53 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java
@@ -29,7 +29,8 @@ public class SharedLoadBalancerService implements LoadBalancerService {
 
     @Override
     public LoadBalancerInstance create(LoadBalancerSpec spec, boolean force) {
-        if ( ! spec.settings().isEmpty()) throw new IllegalArgumentException("custom load balancer settings are not supported with " + getClass());
+        if (spec.settings().isPresent() && ! spec.settings().get().isEmpty())
+            throw new IllegalArgumentException("custom load balancer settings are not supported with " + getClass());
         return new LoadBalancerInstance(Optional.of(DomainName.of(vipHostname)),
                                         Optional.empty(),
                                         Optional.empty(),
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
index 0264d0df837..bcb9c2ac2a7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
@@ -112,7 +112,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer {
                 attempts.add(1);
                 LOG.log(Level.INFO, () -> "Removing reals from inactive load balancer " + lb.id() + ": " + Sets.difference(lb.instance().get().reals(), reals));
                 service.create(new LoadBalancerSpec(lb.id().application(), lb.id().cluster(), reals,
-                                                    lb.instance().get().settings(), lb.instance().get().cloudAccount()),
+                                                    lb.instance().get().settings().get(), lb.instance().get().cloudAccount()),
                                true);
                 db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals))), lb.state());
             } catch (Exception e) {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
index 9eccb21b756..22f6ec97107 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
@@ -75,8 +75,8 @@ public class LoadBalancerSerializer {
         }));
         loadBalancer.instance()
                     .map(LoadBalancerInstance::settings)
-                    .filter(settings -> ! settings.isEmpty())
-                    .ifPresent(settings -> settings.allowedUrns().forEach(root.setObject(settingsField)
+                    .filter(settings -> ! settings.get().isEmpty())
+                    .ifPresent(settings -> settings.get().allowedUrns().forEach(root.setObject(settingsField)
                                                                               .setArray(allowedUrnsField)::addString));
         loadBalancer.instance()
                     .map(LoadBalancerInstance::cloudAccount)
@@ -112,7 +112,7 @@ public class LoadBalancerSerializer {
         LoadBalancerSettings settings = loadBalancerSettings(object.field(settingsField));
         CloudAccount cloudAccount = optionalString(object.field(cloudAccountField), CloudAccount::from).orElse(CloudAccount.empty);
         Optional<LoadBalancerInstance> instance = hostname.isEmpty() && ipAddress.isEmpty() ? Optional.empty() :
-                Optional.of(new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, cloudAccount));
+                Optional.of(new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, Optional.of(settings), cloudAccount));
 
         return new LoadBalancer(LoadBalancerId.fromSerializedForm(object.field(idField).asString()),
                                 instance,
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
index c9c2199a234..17fc83bce80 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
@@ -109,18 +109,18 @@ public class LoadBalancerProvisioner {
      * Calling this when no load balancer has been prepared for given cluster is a no-op.
      */
     public void activate(Set<ClusterSpec> clusters, NodeList newActive, ApplicationTransaction transaction) {
-        Map<ClusterSpec.Id, ClusterSpec> activatingClusters = clusters.stream()
-                                                                      .collect(groupingBy(LoadBalancerProvisioner::effectiveId,
-                                                                                          reducing(null, (o, n) -> o == null || o.type() != Type.container ? n : o)));
+        Set<ClusterSpec.Id> activatingClusters = clusters.stream()
+                                                         .map(LoadBalancerProvisioner::effectiveId)
+                                                         .collect(Collectors.toSet());
         for (var cluster : loadBalancedClustersOf(newActive).entrySet()) {
-            if (!activatingClusters.containsKey(cluster.getKey())) continue;
+            if ( ! activatingClusters.contains(cluster.getKey())) continue;
 
             Node clusterNode = cluster.getValue().first().get();
-            if (!shouldProvision(transaction.application(), clusterNode.type(), clusterNode.allocation().get().membership().cluster().type())) continue;
-            activate(transaction, cluster.getKey(), activatingClusters.get(cluster.getKey()).loadBalancerSettings(), cluster.getValue());
+            if ( ! shouldProvision(transaction.application(), clusterNode.type(), clusterNode.allocation().get().membership().cluster().type())) continue;
+            activate(transaction, cluster.getKey(), cluster.getValue());
         }
         // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed
-        var surplusLoadBalancers = surplusLoadBalancersOf(transaction.application(), activatingClusters.keySet());
+        var surplusLoadBalancers = surplusLoadBalancersOf(transaction.application(), activatingClusters);
         deactivate(surplusLoadBalancers, transaction.nested());
     }
 
@@ -188,7 +188,7 @@ public class LoadBalancerProvisioner {
     private void prepare(LoadBalancerId id, NodeList nodes, LoadBalancerSettings loadBalancerSettings, CloudAccount cloudAccount) {
         Instant now = nodeRepository.clock().instant();
         Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id);
-        Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer, loadBalancerSettings, cloudAccount);
+        Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer, loadBalancerSettings, cloudAccount, false);
         LoadBalancer newLoadBalancer;
         LoadBalancer.State fromState = null;
         if (loadBalancer.isEmpty()) {
@@ -207,14 +207,17 @@ public class LoadBalancerProvisioner {
         requireInstance(id, instance, cloudAccount);
     }
 
-    private void activate(ApplicationTransaction transaction, ClusterSpec.Id cluster, LoadBalancerSettings loadBalancerSettings, NodeList nodes) {
+    private void activate(ApplicationTransaction transaction, ClusterSpec.Id cluster, NodeList nodes) {
         Instant now = nodeRepository.clock().instant();
         LoadBalancerId id = new LoadBalancerId(transaction.application(), cluster);
         Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id);
         if (loadBalancer.isEmpty()) throw new IllegalArgumentException("Could not activate load balancer that was never prepared: " + id);
         if (loadBalancer.get().instance().isEmpty()) throw new IllegalArgumentException("Activating " + id + ", but prepare never provisioned a load balancer instance");
 
-        Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer, loadBalancerSettings, loadBalancer.get().instance().get().cloudAccount());
+        Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer,
+                                                                    loadBalancer.get().instance().get().settings().orElseThrow(),
+                                                                    loadBalancer.get().instance().get().cloudAccount(),
+                                                                    true);
         LoadBalancer.State state = instance.isPresent() ? LoadBalancer.State.active : loadBalancer.get().state();
         LoadBalancer newLoadBalancer = loadBalancer.get().with(instance).with(state, now);
         db.writeLoadBalancers(List.of(newLoadBalancer), loadBalancer.get().state(), transaction.nested());
@@ -225,7 +228,8 @@ public class LoadBalancerProvisioner {
     private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, NodeList nodes,
                                                              Optional<LoadBalancer> currentLoadBalancer,
                                                              LoadBalancerSettings loadBalancerSettings,
-                                                             CloudAccount cloudAccount) {
+                                                             CloudAccount cloudAccount,
+                                                             boolean activate) {
         boolean shouldDeactivateRouting = deactivateRouting.with(FetchVector.Dimension.APPLICATION_ID,
                                                                  id.application().serializedForm())
                                                            .value();
@@ -235,11 +239,14 @@ public class LoadBalancerProvisioner {
         } else {
             reals = realsOf(nodes);
         }
-        if (isUpToDate(currentLoadBalancer, reals, loadBalancerSettings)) return currentLoadBalancer.get().instance();
+        LoadBalancerSettings settingsToUse = activate ? loadBalancerSettings : null;
+        if (isUpToDate(currentLoadBalancer, reals, settingsToUse))
+            return currentLoadBalancer.get().instance().map(instance -> instance.withSettings(loadBalancerSettings));
         log.log(Level.INFO, () -> "Provisioning instance for " + id + ", targeting: " + reals);
         try {
-            return Optional.of(service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals, loadBalancerSettings, cloudAccount),
-                                              shouldDeactivateRouting || allowEmptyReals(currentLoadBalancer)));
+            return Optional.of(service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals, settingsToUse, cloudAccount),
+                                              shouldDeactivateRouting || allowEmptyReals(currentLoadBalancer))
+                                      .withSettings(loadBalancerSettings));
         } catch (Exception e) {
             log.log(Level.WARNING, e, () -> "Could not (re)configure " + id + ", targeting: " +
                                             reals + ". The operation will be retried on next deployment");
@@ -294,12 +301,12 @@ public class LoadBalancerProvisioner {
         return loadBalancer.instance().isEmpty() || loadBalancer.instance().get().cloudAccount().equals(cloudAccount);
     }
 
-    /** Returns whether load balancer has given reals and settings */
+    /** Returns whether load balancer has given reals, and settings if specified*/
     private static boolean isUpToDate(Optional<LoadBalancer> loadBalancer, Set<Real> reals, LoadBalancerSettings loadBalancerSettings) {
         if (loadBalancer.isEmpty()) return false;
         if (loadBalancer.get().instance().isEmpty()) return false;
         return    loadBalancer.get().instance().get().reals().equals(reals)
-               && loadBalancer.get().instance().get().settings().equals(loadBalancerSettings);
+               && (loadBalancerSettings == null || loadBalancer.get().instance().get().settings().get().equals(loadBalancerSettings));
     }
 
     /** Returns whether to allow given load balancer to have no reals */
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java
index 1e0385d152a..307c9db38c6 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java
@@ -45,7 +45,7 @@ public class LoadBalancerSerializerTest {
                                                                     new Real(DomainName.of("real-2"),
                                                                              "127.0.0.2",
                                                                              4080)),
-                                                    new LoadBalancerSettings(List.of("123")),
+                                                    Optional.of(new LoadBalancerSettings(List.of("123"))),
                                                     CloudAccount.from("012345678912"))),
                                             LoadBalancer.State.active,
                                             now);
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
index 4635eb6bc7c..5679201c77b 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
@@ -321,14 +321,14 @@ public class LoadBalancerProvisionerTest {
         tester.activate(app1, prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1"))));
         LoadBalancerList loadBalancers = tester.nodeRepository().loadBalancers().list();
         assertEquals(1, loadBalancers.size());
-        assertEquals(LoadBalancerSettings.empty, loadBalancers.first().get().instance().get().settings());
+        assertEquals(LoadBalancerSettings.empty, loadBalancers.first().get().instance().get().settings().get());
 
         // Next deployment contains new settings
         LoadBalancerSettings settings = new LoadBalancerSettings(List.of("alice", "bob"));
         tester.activate(app1, prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1"), Optional.empty(), settings)));
         loadBalancers = tester.nodeRepository().loadBalancers().list();
         assertEquals(1, loadBalancers.size());
-        assertEquals(settings, loadBalancers.first().get().instance().get().settings());
+        assertEquals(settings, loadBalancers.first().get().instance().get().settings().get());
     }