Set private DNS for existing VPC endpoints services

10 files changed, 134 insertions, 12 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
index 83106e75627..fd355d427a3 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
@@ -11,9 +11,10 @@ import java.util.stream.Collectors;
 
 /**
  * Represents an application- or instance-level endpoint in deployments.xml.
- *
- * - An instance-level endpoint is global and can span multiple regions within a single instance.
- * - An application-level endpoint points can span multiple instances within a single region.
+ * <p>
+ * - An instance-level endpoint is global and may span multiple regions within a single instance.
+ * - An application-level endpoint may span multiple instances within a single region, or
+ *   even multiple instances across multiple regions, depending on the name service used for the cloud.
  *
  * @author ogronnesby
  * @author mpolden
@@ -44,7 +45,8 @@ public class Endpoint {
         this.level = Objects.requireNonNull(level, "level must be non-null");
         this.targets = List.copyOf(Objects.requireNonNull(targets, "targets must be non-null"));
         if (endpointId().length() > endpointMaxLength || !endpointPattern.matcher(endpointId()).matches()) {
-            throw new IllegalArgumentException("Invalid endpoint ID: '" + endpointId() + "'");
+            throw new IllegalArgumentException("Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, " +
+                                               "of length 1 to 12, and begin with a character; but got '" + endpointId() + "'");
         }
         if (targets.isEmpty()) throw new IllegalArgumentException("targets must be non-empty");
         for (int i = 0; i < targets.size(); i++) {
@@ -66,7 +68,7 @@ public class Endpoint {
         }
     }
 
-    /** The unique identifer of this */
+    /** The unique identifier of this */
     public String endpointId() {
         return endpointId;
     }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/ServiceRegistry.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/ServiceRegistry.java
index e49e9c7998e..923304d4c55 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/ServiceRegistry.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/ServiceRegistry.java
@@ -20,6 +20,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationS
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ArtifactRepository;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.VpcEndpointService;
 import com.yahoo.vespa.hosted.controller.api.integration.entity.EntityService;
 import com.yahoo.vespa.hosted.controller.api.integration.horizon.HorizonClient;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever;
@@ -57,6 +58,8 @@ public interface ServiceRegistry {
 
     NameService nameService();
 
+    VpcEndpointService vpcEndpointService();
+
     Mailer mailer();
 
     EndpointCertificateProvider endpointCertificateProvider();
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
index 9a9270bdf7f..aff58989231 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
@@ -8,6 +8,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.stream.Collectors;
 
 /**
@@ -17,7 +18,7 @@ import java.util.stream.Collectors;
  */
 public class MemoryNameService implements NameService {
 
-    private final Set<Record> records = new TreeSet<>();
+    private final Set<Record> records = new ConcurrentSkipListSet<>();
 
     public Set<Record> records() {
         return Collections.unmodifiableSet(records);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java
index 344ffad80e9..e20c30d2745 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java
@@ -3,9 +3,11 @@ package com.yahoo.vespa.hosted.controller.dns;
 
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 
 /**
  * Create or update a record.
@@ -29,6 +31,11 @@ public class CreateRecord implements NameServiceRequest {
     }
 
     @Override
+    public Optional<RecordName> name() {
+        return Optional.of(record.name());
+    }
+
+    @Override
     public void dispatchTo(NameService nameService) {
         List<Record> records = nameService.findRecords(record.type(), record.name());
         records.forEach(r -> {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java
index b97fdde560e..88e4f351f1f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java
@@ -9,6 +9,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -39,6 +40,11 @@ public class CreateRecords implements NameServiceRequest {
     }
 
     @Override
+    public Optional<RecordName> name() {
+        return Optional.of(name);
+    }
+
+    @Override
     public void dispatchTo(NameService nameService) {
         switch (type) {
             case ALIAS -> {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java
index d42b9efbdb3..42ee8a7d2d5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java
@@ -2,6 +2,9 @@
 package com.yahoo.vespa.hosted.controller.dns;
 
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
+
+import java.util.Optional;
 
 /**
  * Interface for requests to a {@link NameService}.
@@ -10,6 +13,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
  */
 public interface NameServiceRequest {
 
+    Optional<RecordName> name();
+
     /** Send this to given name service */
     void dispatchTo(NameService nameService);
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
index b0d16126600..4e2eb03f01d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller.routing;
 
 import ai.vespa.http.DomainName;
+import com.yahoo.concurrent.UncheckedTimeoutException;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.zone.RoutingMethod;
@@ -9,6 +10,7 @@ import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.transaction.Mutex;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.api.identifiers.ClusterId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.AliasTarget;
@@ -27,7 +29,9 @@ import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceRequest;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
+import com.yahoo.yolean.UncheckedInterruptedException;
 
+import java.time.Instant;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -349,7 +353,7 @@ public class RoutingPolicies {
             if (existingPolicy != null) {
                 newPolicy = newPolicy.with(newPolicy.status().with(existingPolicy.status().routingStatus()));
             }
-            updateZoneDnsOf(newPolicy);
+            updateZoneDnsOf(newPolicy, allocation);
             policies.put(newPolicy.id(), newPolicy);
         }
         RoutingPolicyList updated = RoutingPolicyList.copyOf(policies.values());
@@ -358,16 +362,42 @@ public class RoutingPolicies {
     }
 
     /** Update zone DNS record for given policy */
-    private void updateZoneDnsOf(RoutingPolicy policy) {
+    private void updateZoneDnsOf(RoutingPolicy policy, LoadBalancerAllocation allocation) {
         for (var endpoint : policy.zoneEndpointsIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry())) {
             var name = RecordName.from(endpoint.dnsName());
             var record = policy.canonicalName().isPresent() ?
                     new Record(Record.Type.CNAME, name, RecordData.fqdn(policy.canonicalName().get().value())) :
                     new Record(Record.Type.A, name, RecordData.from(policy.ipAddress().orElseThrow()));
             nameServiceForwarderIn(policy.id().zone()).createRecord(record, Priority.normal);
+            setPrivateDns(endpoint, allocation);
         }
     }
 
+    private void setPrivateDns(Endpoint endpoint, LoadBalancerAllocation allocation) {
+        controller.serviceRegistry().vpcEndpointService()
+                  .setPrivateDns(DomainName.of(endpoint.dnsName()),
+                                 new ClusterId(allocation.deployment, endpoint.cluster()),
+                                 controller.applications().decideCloudAccountOf(allocation.deployment, allocation.deploymentSpec))
+                  .ifPresent(challenge -> {
+                      try {
+                          nameServiceForwarderIn(allocation.deployment.zoneId()).createTxt(challenge.name(), List.of(challenge.data()), Priority.high);
+                          Instant doom = controller.clock().instant().plusSeconds(30);
+                          while (controller.clock().instant().isBefore(doom)) {
+                              if (controller.curator().readNameServiceQueue().requests().stream()
+                                            .noneMatch(request -> request.name().equals(Optional.of(challenge.name())))) {
+                                  challenge.trigger().run();
+                                  return;
+                              }
+                              Thread.sleep(100);
+                          }
+                          throw new UncheckedTimeoutException("timed out waiting for DNS challenge to be processed");
+                      }
+                      catch (InterruptedException e) {
+                          throw new UncheckedInterruptedException("interrupted waiting for DNS challenge to be processed", e, true);
+                      }
+                  });
+    }
+
     /**
      * Remove policies and zone DNS records unreferenced by given load balancers
      *
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
index 46c731e6e49..eb16ecaab81 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
@@ -1,10 +1,12 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.integration;
 
+import ai.vespa.http.DomainName;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.component.Version;
 import com.yahoo.component.annotation.Inject;
+import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
@@ -28,6 +30,11 @@ import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCe
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateValidator;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateValidatorMock;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.MockVpcEndpointService;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.VpcEndpointService;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.VpcEndpointService.DnsChallenge;
 import com.yahoo.vespa.hosted.controller.api.integration.entity.MemoryEntityService;
 import com.yahoo.vespa.hosted.controller.api.integration.horizon.HorizonClient;
 import com.yahoo.vespa.hosted.controller.api.integration.horizon.MockHorizonClient;
@@ -64,6 +71,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
     private final ZoneRegistryMock zoneRegistryMock;
     private final ConfigServerMock configServerMock;
     private final MemoryNameService memoryNameService = new MemoryNameService();
+    private final MockVpcEndpointService vpcEndpointService = new MockVpcEndpointService();
     private final MockMailer mockMailer = new MockMailer();
     private final EndpointCertificateMock endpointCertificateMock = new EndpointCertificateMock(clock);
     private final EndpointCertificateValidatorMock endpointCertificateValidatorMock = new EndpointCertificateValidatorMock();
@@ -201,6 +209,11 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
     }
 
     @Override
+    public MockVpcEndpointService vpcEndpointService() {
+        return vpcEndpointService;
+    }
+
+    @Override
     public ZoneRegistryMock zoneRegistry() {
         return zoneRegistryMock;
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
index 7b00e7040b5..867e03258f9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
@@ -9,6 +9,7 @@ import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.AthenzService;
+import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
@@ -21,8 +22,11 @@ import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.Record.Type;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.VpcEndpointService;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.VpcEndpointService.DnsChallenge;
 import com.yahoo.vespa.hosted.controller.application.Endpoint;
 import com.yahoo.vespa.hosted.controller.application.EndpointId;
 import com.yahoo.vespa.hosted.controller.application.EndpointList;
@@ -47,11 +51,16 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author mortent
@@ -502,6 +511,48 @@ public class RoutingPoliciesTest {
     }
 
     @Test
+    void private_dns_for_vpc_endpoint() {
+        // Challenge answered for endpoint
+        RoutingPoliciesTester tester = new RoutingPoliciesTester();
+        Map<RecordName, RecordData> challenges = new ConcurrentHashMap<>();
+        tester.tester.controllerTester().serviceRegistry().vpcEndpointService().delegate =
+                (name, cluster, account) -> {
+                    RecordName recordName = RecordName.from("challenge--" + name.value());
+                    if (challenges.containsKey(recordName)) return Optional.empty();
+                    RecordData recordData = RecordData.from(account.map(CloudAccount::value).orElse("system"));
+                    return Optional.of(new DnsChallenge(recordName, recordData, () -> challenges.put(recordName, recordData)));
+                };
+
+        DeploymentContext app = tester.newDeploymentContext("t", "a", "default");
+        ApplicationPackage appPackage = applicationPackageBuilder().region(zone3.region()).build();
+        app.submit(appPackage);
+
+        AtomicBoolean done = new AtomicBoolean();
+        new Thread(() -> {
+            while ( ! done.get()) {
+                app.flushDnsUpdates();
+                try { Thread.sleep(10); } catch (InterruptedException e) { break; }
+            }
+        }).start();
+        app.deploy();
+        done.set(true);
+
+        assertEquals(Set.of(new Record(Type.CNAME,
+                                       RecordName.from("a.t.aws-us-east-1a.vespa.oath.cloud"),
+                                       RecordData.from("lb-0--t.a.default--prod.aws-us-east-1a.")),
+                            new Record(Type.TXT,
+                                       RecordName.from("challenge--a.t.aws-us-east-1a.vespa.oath.cloud"),
+                                       RecordData.from("system")),
+                            new Record(Type.TXT,
+                                       RecordName.from("challenge--a.t.us-east-1.test.vespa.oath.cloud"),
+                                       RecordData.from("system")),
+                            new Record(Type.TXT,
+                                       RecordName.from("challenge--a.t.us-east-3.staging.vespa.oath.cloud"),
+                                       RecordData.from("system"))),
+                     tester.controllerTester().nameService().records());
+    }
+
+    @Test
     void set_global_endpoint_status() {
         var tester = new RoutingPoliciesTester();
         var context = tester.newDeploymentContext("tenant1", "app1", "default");
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java
index 42400b82ab6..8eda57b0476 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java
@@ -327,7 +327,6 @@ class SingletonManager {
             Instant ourDoom = doom.get();
             boolean shouldBeActive = ourDoom != null && ourDoom != INVALID && ! clock.instant().isAfter(ourDoom);
             if ( ! active && shouldBeActive) {
-                logger.log(INFO, "Activating singleton for ID: " + id);
                 try {
                     active = true;
                     if ( ! singletons.isEmpty()) metrics.activation(singletons.peek()::activate);
@@ -338,7 +337,6 @@ class SingletonManager {
                 }
             }
             if (active && ! shouldBeActive) {
-                logger.log(INFO, "Deactivating singleton for ID: " + id);
                 logger.log(FINE, () -> "Doom value is " + doom);
                 try  {
                     if ( ! singletons.isEmpty()) metrics.deactivation(singletons.peek()::deactivate);
@@ -415,6 +413,7 @@ class SingletonManager {
                 Instant start = clock.instant();
                 boolean failed = false;
                 metric.add(ACTIVATION, 1, context);
+                logger.log(INFO, "Activating singleton for ID: " + id);
                 try {
                     activation.run();
                 }
@@ -423,7 +422,9 @@ class SingletonManager {
                     throw e;
                 }
                 finally {
-                    metric.set(ACTIVATION_MILLIS, Duration.between(start, clock.instant()).toMillis(), context);
+                    long durationMillis = Duration.between(start, clock.instant()).toMillis();
+                    metric.set(ACTIVATION_MILLIS, durationMillis, context);
+                    logger.log(INFO, "Activation completed in %.3f seconds".formatted(durationMillis * 1e-3));
                     if (failed) metric.add(ACTIVATION_FAILURES, 1, context);
                     else isActive = true;
                     ping();
@@ -434,6 +435,7 @@ class SingletonManager {
                 Instant start = clock.instant();
                 boolean failed = false;
                 metric.add(DEACTIVATION, 1, context);
+                logger.log(INFO, "Deactivating singleton for ID: " + id);
                 try {
                     deactivation.run();
                 }
@@ -442,7 +444,9 @@ class SingletonManager {
                     throw e;
                 }
                 finally {
-                    metric.set(DEACTIVATION_MILLIS, Duration.between(start, clock.instant()).toMillis(), context);
+                    long durationMillis = Duration.between(start, clock.instant()).toMillis();
+                    metric.set(DEACTIVATION_MILLIS, durationMillis, context);
+                    logger.log(INFO, "Deactivation completed in %.3f seconds".formatted(durationMillis * 1e-3));
                     if (failed) metric.add(DEACTIVATION_FAILURES, 1, context);
                     isActive = false;
                     ping();