Revert "Merge pull request #25776 from vespa-engine/jonmv/revert-private-endpoints"

This reverts commit 350b36dd88baef7548c0066b01ea1e328eb78f3f, reversing
changes made to 8a006bc9ca202713ec54c7961a9256790c87d10d.

6 files changed, 48 insertions, 26 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java
index c19aebcda6e..e74e3068674 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java
@@ -62,8 +62,8 @@ public class LoadBalancerServiceMock implements LoadBalancerService {
                 Collections.singleton(4443),
                 ImmutableSet.of("10.2.3.0/24", "10.4.5.0/24"),
                 spec.reals(),
-                spec.settings().orElse(ZoneEndpoint.defaultEndpoint),
-                spec.settings().map(__ -> PrivateServiceId.of("service")),
+                spec.settings(),
+                spec.settings().isPrivateEndpoint() ? Optional.of(PrivateServiceId.of("service")) : Optional.empty(),
                 spec.cloudAccount());
         instances.put(id, instance);
         return instance;
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 e0ef6739542..6be8bd71553 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
@@ -21,7 +21,7 @@ public class LoadBalancerSpec {
     private final ApplicationId application;
     private final ClusterSpec.Id cluster;
     private final Set<Real> reals;
-    private final Optional<ZoneEndpoint> settings;
+    private final ZoneEndpoint settings;
     private final CloudAccount cloudAccount;
 
     public LoadBalancerSpec(ApplicationId application, ClusterSpec.Id cluster, Set<Real> reals,
@@ -29,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 = Optional.ofNullable(settings);
+        this.settings = Objects.requireNonNull(settings);
         this.cloudAccount = Objects.requireNonNull(cloudAccount);
     }
 
@@ -49,7 +49,7 @@ public class LoadBalancerSpec {
     }
 
     /** Static user-configured settings for this load balancer. */
-    public Optional<ZoneEndpoint> settings() {
+    public ZoneEndpoint 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 0722bb8e1dc..fa672c0fd1e 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,13 +29,15 @@ public class SharedLoadBalancerService implements LoadBalancerService {
 
     @Override
     public LoadBalancerInstance create(LoadBalancerSpec spec, boolean force) {
+        if ( ! spec.settings().isPublicEndpoint())
+            throw new IllegalArgumentException("non-public endpoints is not supported with " + getClass());
         return new LoadBalancerInstance(Optional.of(DomainName.of(vipHostname)),
                                         Optional.empty(),
                                         Optional.empty(),
                                         Set.of(4443),
                                         Set.of(),
                                         spec.reals(),
-                                        spec.settings().orElse(ZoneEndpoint.defaultEndpoint),
+                                        spec.settings(),
                                         Optional.empty(),
                                         spec.cloudAccount());
     }
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 92fdb1d2e52..fc03ddcc3b0 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
@@ -81,7 +81,7 @@ public class LoadBalancerProvisioner {
      * Prepare a load balancer for given application and cluster.
      * <p>
      * If a load balancer for the cluster already exists, it will be reconfigured based on the currently allocated
-     * nodes. It's state will remain unchanged.
+     * nodes. Its state will remain unchanged.
      * <p>
      * If no load balancer exists, a new one will be provisioned in {@link LoadBalancer.State#reserved}.
      * <p>
@@ -93,7 +93,7 @@ public class LoadBalancerProvisioner {
             ClusterSpec.Id clusterId = effectiveId(cluster);
             LoadBalancerId loadBalancerId = requireNonClashing(new LoadBalancerId(application, clusterId));
             NodeList nodes = nodesOf(clusterId, application);
-            prepare(loadBalancerId, nodes, requestedNodes.cloudAccount());
+            prepare(loadBalancerId, nodes, cluster.zoneEndpoint(), requestedNodes.cloudAccount());
         }
     }
 
@@ -189,10 +189,10 @@ public class LoadBalancerProvisioner {
         return loadBalancerId;
     }
 
-    private void prepare(LoadBalancerId id, NodeList nodes, CloudAccount cloudAccount) {
+    private void prepare(LoadBalancerId id, NodeList nodes, ZoneEndpoint zoneEndpoint, CloudAccount cloudAccount) {
         Instant now = nodeRepository.clock().instant();
         Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id);
-        Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer, null, cloudAccount);
+        Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer, zoneEndpoint, cloudAccount, true);
         LoadBalancer newLoadBalancer;
         LoadBalancer.State fromState = null;
         if (loadBalancer.isEmpty()) {
@@ -206,9 +206,19 @@ public class LoadBalancerProvisioner {
             // provision a new one in the wanted account
             newLoadBalancer = newLoadBalancer.with(LoadBalancer.State.removable, now);
         }
+        if (!hasCorrectVisibility(newLoadBalancer, zoneEndpoint)) {
+            // We have a load balancer, but with the wrong visibility. Load balancer must be removed before we can
+            // provision a new one with the wanted visibility
+            newLoadBalancer = newLoadBalancer.with(LoadBalancer.State.removable, now);
+        }
         // Always store the load balancer. LoadBalancerExpirer will remove unwanted ones
         db.writeLoadBalancer(newLoadBalancer, fromState);
-        requireInstance(id, instance, cloudAccount);
+        requireInstance(id, newLoadBalancer, cloudAccount, zoneEndpoint);
+    }
+
+    private static boolean hasCorrectVisibility(LoadBalancer newLoadBalancer, ZoneEndpoint zoneEndpoint) {
+        return    newLoadBalancer.instance().isEmpty()
+               || newLoadBalancer.instance().get().settings().isPublicEndpoint() == zoneEndpoint.isPublicEndpoint();
     }
 
     private void activate(ApplicationTransaction transaction, ClusterSpec.Id cluster, ZoneEndpoint settings, NodeList nodes) {
@@ -218,18 +228,19 @@ public class LoadBalancerProvisioner {
         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, settings, loadBalancer.get().instance().get().cloudAccount());
+        Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer, settings, loadBalancer.get().instance().get().cloudAccount(), false);
         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());
-        requireInstance(id, instance, loadBalancer.get().instance().get().cloudAccount());
+        requireInstance(id, newLoadBalancer, loadBalancer.get().instance().get().cloudAccount(), settings);
     }
 
     /** Provision or reconfigure a load balancer instance, if necessary */
     private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, NodeList nodes,
                                                              Optional<LoadBalancer> currentLoadBalancer,
                                                              ZoneEndpoint zoneEndpoint,
-                                                             CloudAccount cloudAccount) {
+                                                             CloudAccount cloudAccount,
+                                                             boolean preparing) {
         boolean shouldDeactivateRouting = deactivateRouting.with(FetchVector.Dimension.APPLICATION_ID,
                                                                  id.application().serializedForm())
                                                            .value();
@@ -239,15 +250,15 @@ public class LoadBalancerProvisioner {
         } else {
             reals = realsOf(nodes);
         }
-        if (isUpToDate(currentLoadBalancer, reals, zoneEndpoint))
+        if (isUpToDate(currentLoadBalancer, reals, zoneEndpoint, preparing))
             return currentLoadBalancer.get().instance();
         log.log(Level.INFO, () -> "Provisioning instance for " + id + ", targeting: " + reals);
         try {
             // Override settings at activation, otherwise keep existing ones.
-            ZoneEndpoint settings = zoneEndpoint != null ? zoneEndpoint
-                                                         : currentLoadBalancer.flatMap(LoadBalancer::instance)
-                                                                                              .map(LoadBalancerInstance::settings)
-                                                                                              .orElse(null);
+            ZoneEndpoint settings = ! preparing ? zoneEndpoint
+                                                : currentLoadBalancer.flatMap(LoadBalancer::instance)
+                                                                     .map(LoadBalancerInstance::settings)
+                                                                     .orElse(zoneEndpoint);
             LoadBalancerInstance created = service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals, settings, cloudAccount),
                                                           shouldDeactivateRouting || allowEmptyReals(currentLoadBalancer));
             if (created.serviceId().isEmpty() && currentLoadBalancer.flatMap(LoadBalancer::instance).flatMap(LoadBalancerInstance::serviceId).isPresent())
@@ -307,12 +318,12 @@ public class LoadBalancerProvisioner {
         return loadBalancer.instance().isEmpty() || loadBalancer.instance().get().cloudAccount().equals(cloudAccount);
     }
 
-    /** Returns whether load balancer has given reals, and settings if specified*/
-    private static boolean isUpToDate(Optional<LoadBalancer> loadBalancer, Set<Real> reals, ZoneEndpoint zoneEndpoint) {
+    /** Returns whether load balancer has given reals, and settings if specified */
+    private static boolean isUpToDate(Optional<LoadBalancer> loadBalancer, Set<Real> reals, ZoneEndpoint zoneEndpoint, boolean preparing) {
         if (loadBalancer.isEmpty()) return false;
         if (loadBalancer.get().instance().isEmpty()) return false;
         return    loadBalancer.get().instance().get().reals().equals(reals)
-               && (zoneEndpoint == null || loadBalancer.get().instance().get().settings().equals(zoneEndpoint));
+               && (preparing || loadBalancer.get().instance().get().settings().equals(zoneEndpoint));
     }
 
     /** Returns whether to allow given load balancer to have no reals */
@@ -331,14 +342,17 @@ public class LoadBalancerProvisioner {
         return reachable;
     }
 
-    private static void requireInstance(LoadBalancerId id, Optional<LoadBalancerInstance> instance, CloudAccount cloudAccount) {
-        if (instance.isEmpty()) {
+    private static void requireInstance(LoadBalancerId id, LoadBalancer loadBalancer, CloudAccount cloudAccount, ZoneEndpoint zoneEndpoint) {
+        if (loadBalancer.instance().isEmpty()) {
             // Signal that load balancer is not ready yet
             throw new LoadBalancerServiceException("Could not (re)configure " + id + ". The operation will be retried on next deployment");
         }
-        if (!instance.get().cloudAccount().equals(cloudAccount)) {
+        if (!inAccount(cloudAccount, loadBalancer)) {
             throw new LoadBalancerServiceException("Could not (re)configure " + id + " due to change in cloud account. The operation will be retried on next deployment");
         }
+        if (!hasCorrectVisibility(loadBalancer, zoneEndpoint)) {
+            throw new LoadBalancerServiceException("Could not (re)configure " + id + " due to change in load balancer visibility. The operation will be retried on next deployment");
+        }
     }
 
     private static ClusterSpec.Id effectiveId(ClusterSpec cluster) {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
index 15a799c06d8..bf5b735c4a0 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
@@ -89,6 +89,7 @@ public class LoadBalancersResponse extends SlimeJsonResponse {
                     }
                 }
                 instance.serviceId().ifPresent(serviceId -> lbObject.setString("serviceId", serviceId.value()));
+                lbObject.setBool("public", instance.settings().isPublicEndpoint());
             });
             lb.instance()
               .map(LoadBalancerInstance::cloudAccount)
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
index 29914993bae..6cbb2ba1fb4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
@@ -21,6 +21,8 @@ import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.config.provision.ZoneEndpoint;
+import com.yahoo.config.provision.ZoneEndpoint.AccessType;
+import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn;
 import com.yahoo.transaction.Mutex;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.curator.mock.MockCurator;
@@ -190,7 +192,10 @@ public class MockNodeRepository extends NodeRepository {
         activate(provisioner.prepare(zoneApp, zoneCluster, Capacity.fromRequiredNodeType(NodeType.host), null), zoneApp, provisioner);
 
         ApplicationId app1Id = ApplicationId.from(TenantName.from("tenant1"), ApplicationName.from("application1"), InstanceName.from("instance1"));
-        ClusterSpec cluster1Id = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("id1")).vespaVersion("6.42").loadBalancerSettings(new ZoneEndpoint(List.of("arne"))).build();
+        ClusterSpec cluster1Id = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("id1"))
+                                            .vespaVersion("6.42")
+                                            .loadBalancerSettings(new ZoneEndpoint(false, true, List.of(new AllowedUrn(AccessType.awsPrivateLink, "arne"))))
+                                            .build();
         activate(provisioner.prepare(app1Id,
                                      cluster1Id,
                                      Capacity.from(new ClusterResources(2, 1, new NodeResources(2, 8, 50, 1)),