Merge pull request #28786 from vespa-engine/mpolden/generate-all-auth-methods

Ensure endpoint is generated for all requested auth methods

3 files changed, 96 insertions, 16 deletions

diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java
index a6d3b435dcb..2bc11adddf7 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java
@@ -28,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock;
 import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
-import com.yahoo.vespa.hosted.controller.maintenance.EndpointCertificateMaintainer;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -296,7 +295,7 @@ public class EndpointCertificatesTest {
         // Initial certificate is requested directly from provider
         Optional<EndpointCertificate> certFromProvider = endpointCertificates.get(instance, prodZone, DeploymentSpec.empty);
         assertTrue(certFromProvider.isPresent());
-        assertFalse(certFromProvider.get().randomizedId().isPresent());
+        assertFalse(certFromProvider.get().generatedId().isPresent());
 
         // Pooled certificates become available
         tester.flagSource().withBooleanFlag(Flags.RANDOMIZED_ENDPOINT_NAMES.id(), true);
@@ -315,8 +314,8 @@ public class EndpointCertificatesTest {
             String certId = "pool-cert-1";
             addCertificateToPool(certId, UnassignedCertificate.State.ready);
             Optional<EndpointCertificate> cert = endpointCertificates.get(instance, prodZone, DeploymentSpec.empty);
-            assertEquals(certId, cert.get().randomizedId().get());
-            assertEquals(certId, tester.curator().readAssignedCertificate(TenantAndApplicationId.from(instance.id()), Optional.empty()).get().certificate().randomizedId().get(), "Certificate is assigned at application-level");
+            assertEquals(certId, cert.get().generatedId().get());
+            assertEquals(certId, tester.curator().readAssignedCertificate(TenantAndApplicationId.from(instance.id()), Optional.empty()).get().certificate().generatedId().get(), "Certificate is assigned at application-level");
             assertTrue(tester.controller().curator().readUnassignedCertificate(certId).isEmpty(), "Certificate is removed from pool");
             assertEquals(clock.instant().getEpochSecond(), cert.get().lastRequested());
         }
@@ -326,8 +325,8 @@ public class EndpointCertificatesTest {
             addCertificateToPool(certId, UnassignedCertificate.State.ready);
             ZoneId devZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.dev).zones().stream().findFirst().orElseThrow().getId();
             Optional<EndpointCertificate> cert = endpointCertificates.get(instance, devZone, DeploymentSpec.empty);
-            assertEquals(certId, cert.get().randomizedId().get());
-            assertEquals(certId, tester.curator().readAssignedCertificate(instance.id()).get().certificate().randomizedId().get(), "Certificate is assigned at instance-level");
+            assertEquals(certId, cert.get().generatedId().get());
+            assertEquals(certId, tester.curator().readAssignedCertificate(instance.id()).get().certificate().generatedId().get(), "Certificate is assigned at instance-level");
             assertTrue(tester.controller().curator().readUnassignedCertificate(certId).isEmpty(), "Certificate is removed from pool");
             assertEquals(clock.instant().getEpochSecond(), cert.get().lastRequested());
         }
@@ -338,7 +337,7 @@ public class EndpointCertificatesTest {
         // Initial certificate is requested directly from provider
         Optional<EndpointCertificate> certFromProvider = endpointCertificates.get(instance, prodZone, DeploymentSpec.empty);
         assertTrue(certFromProvider.isPresent());
-        assertFalse(certFromProvider.get().randomizedId().isPresent());
+        assertFalse(certFromProvider.get().generatedId().isPresent());
 
         // Simulate endpoint certificate maintainer to assign random id
         TenantAndApplicationId tenantAndApplicationId = TenantAndApplicationId.from(instance.id());
@@ -346,7 +345,7 @@ public class EndpointCertificatesTest {
         Optional<AssignedCertificate> assignedCertificate = tester.controller().curator().readAssignedCertificate(tenantAndApplicationId, instanceName);
         assertTrue(assignedCertificate.isPresent());
         String assignedRandomId = "randomid";
-        AssignedCertificate updated = assignedCertificate.get().with(assignedCertificate.get().certificate().withRandomizedId(assignedRandomId));
+        AssignedCertificate updated = assignedCertificate.get().with(assignedCertificate.get().certificate().withGeneratedId(assignedRandomId));
         tester.controller().curator().writeAssignedCertificate(updated);
 
         // Pooled certificates become available
@@ -358,12 +357,12 @@ public class EndpointCertificatesTest {
 
         // Request cert for app
         Optional<EndpointCertificate> cert = endpointCertificates.get(instance, prodZone, DeploymentSpec.empty);
-        assertEquals(assignedRandomId, cert.get().randomizedId().get());
+        assertEquals(assignedRandomId, cert.get().generatedId().get());
 
         // Pooled cert remains unassigned
         List<String> unassignedCertificateIds = tester.curator().readUnassignedCertificates().stream()
                 .map(UnassignedCertificate::certificate)
-                .map(EndpointCertificate::randomizedId)
+                .map(EndpointCertificate::generatedId)
                 .map(Optional::get)
                 .toList();
         assertEquals(List.of(certId), unassignedCertificateIds);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java
index 647c809231e..2f996bac897 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java
@@ -41,7 +41,6 @@ import java.util.stream.Stream;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.devUsEast1;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.perfUsEast3;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsCentral1;
-import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsEast3;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsWest1;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.stagingTest;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.systemTest;
@@ -138,7 +137,7 @@ public class EndpointCertificateMaintainerTest {
         tester.clock().advance(Duration.ofDays(3));
         secretStore.setSecret(assignedCertificate.certificate().keyName(), "foo", 1);
         secretStore.setSecret(assignedCertificate.certificate().certName(), "bar", 1);
-        tester.controller().serviceRegistry().endpointCertificateProvider().requestCaSignedCertificate("preprovisioned." + assignedCertificate.certificate().randomizedId().get(), assignedCertificate.certificate().requestedDnsSans(), Optional.of(assignedCertificate.certificate()), "rsa_2048", false);
+        tester.controller().serviceRegistry().endpointCertificateProvider().requestCaSignedCertificate("preprovisioned." + assignedCertificate.certificate().generatedId().get(), assignedCertificate.certificate().requestedDnsSans(), Optional.of(assignedCertificate.certificate()), "rsa_2048", false);
 
         // We should now pick up the new key and cert version + uuid, but not force trigger deployment yet
         assertEquals(0.0, maintainer.maintain(), 0.0000001);
@@ -206,7 +205,7 @@ public class EndpointCertificateMaintainerTest {
         assertTrue(applicationCertificate.isPresent());
         Optional<AssignedCertificate> instanceCertificate = tester.curator().readAssignedCertificate(TenantAndApplicationId.from(app), Optional.of(app.instance()));
         assertTrue(instanceCertificate.isPresent());
-        assertEquals(instanceCertificate.get().certificate().randomizedId(), applicationCertificate.get().certificate().randomizedId());
+        assertEquals(instanceCertificate.get().certificate().generatedId(), applicationCertificate.get().certificate().generatedId());
 
         // Verify the 3 wildcard random names are same in all certs
         List<String> appWildcardSans = applicationCertificate.get().certificate().requestedDnsSans();
@@ -226,13 +225,13 @@ public class EndpointCertificateMaintainerTest {
         assertEquals(1, tester.curator().readAssignedCertificates().size());
         maintainer.maintain();
 
-        String randomId = tester.curator().readAssignedCertificate(instance1).get().certificate().randomizedId().get();
+        String randomId = tester.curator().readAssignedCertificate(instance1).get().certificate().generatedId().get();
 
         deployToAssignCert(deploymentTester, instance2, List.of(productionUsWest1), Optional.of("instance1,instance2"));
         maintainer.maintain();
         assertEquals(3, tester.curator().readAssignedCertificates().size());
 
-        assertEquals(randomId, tester.curator().readAssignedCertificate(instance1).get().certificate().randomizedId().get());
+        assertEquals(randomId, tester.curator().readAssignedCertificate(instance1).get().certificate().generatedId().get());
     }
 
     @Test
@@ -247,7 +246,7 @@ public class EndpointCertificateMaintainerTest {
 
         // Verify certificate is assigned random id and 3 new names
         Optional<AssignedCertificate> assignedCertificate = tester.curator().readAssignedCertificate(devApp);
-        assertTrue(assignedCertificate.get().certificate().randomizedId().isPresent());
+        assertTrue(assignedCertificate.get().certificate().generatedId().isPresent());
         List<String> newRequestedSans = assignedCertificate.get().certificate().requestedDnsSans();
         List<String> randomizedNames = newRequestedSans.stream().filter(san -> !originalRequestedSans.contains(san)).toList();
         assertEquals(3, randomizedNames.size());
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 22523103208..3405009714d 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
@@ -1206,6 +1206,70 @@ public class RoutingPoliciesTest {
     }
 
     @Test
+    public void generated_endpoints_enable_token() {
+        var tester = new RoutingPoliciesTester(SystemName.Public);
+        var context = tester.newDeploymentContext("tenant1", "app1", "default");
+        tester.controllerTester().flagSource().withBooleanFlag(Flags.RANDOMIZED_ENDPOINT_NAMES.id(), true);
+        tester.controllerTester().flagSource().withBooleanFlag(Flags.LEGACY_ENDPOINTS.id(), false);
+        addCertificateToPool("cafed00d", UnassignedCertificate.State.ready, tester);
+
+        // Deploy application without token
+        var zone1 = ZoneId.from("prod", "aws-us-east-1c");
+        ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region())
+                                                                           .container("c0", AuthMethod.mtls)
+                                                                           .endpoint("foo", "c0")
+                                                                           .build();
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c"));
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c"));
+        tester.provisionLoadBalancers(1, context.instanceId(), zone1);
+        context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy();
+        assertEquals(List.of("a9c8c045.cafed00d.g.vespa-app.cloud",
+                             "ebd395b6.cafed00d.z.vespa-app.cloud",
+                             "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"),
+                     tester.recordNames());
+
+        // Re-deploy with token enabled
+        applicationPackage = applicationPackageBuilder().region(zone1.region())
+                                                        .container("c0", AuthMethod.mtls, AuthMethod.token)
+                                                        .endpoint("foo", "c0")
+                                                        .build();
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c"));
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c"));
+        context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy();
+        // Additional zone- and global-scoped endpoints are added (token)
+        assertEquals(List.of("a9c8c045.cafed00d.g.vespa-app.cloud",
+                             "b7e79800.cafed00d.z.vespa-app.cloud",
+                             "c60d3149.cafed00d.g.vespa-app.cloud",
+                             "ebd395b6.cafed00d.z.vespa-app.cloud",
+                             "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"),
+                     tester.recordNames());
+
+        // Add new endpoint is generated for an additional global endpoint
+        applicationPackage = applicationPackageBuilder().region(zone1.region())
+                                                        .container("c0", AuthMethod.mtls, AuthMethod.token)
+                                                        .endpoint("foo", "c0")
+                                                        .endpoint("bar", "c0")
+                                                        .build();
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c"));
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c"));
+        context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy();
+        List<String> expectedRecords = List.of("a9c8c045.cafed00d.g.vespa-app.cloud",
+                                               "aa7591aa.cafed00d.g.vespa-app.cloud",
+                                               "b7e79800.cafed00d.z.vespa-app.cloud",
+                                               "c60d3149.cafed00d.g.vespa-app.cloud",
+                                               "d467800f.cafed00d.g.vespa-app.cloud",
+                                               "ebd395b6.cafed00d.z.vespa-app.cloud",
+                                               "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud");
+        assertEquals(expectedRecords, tester.recordNames());
+
+        // No change on redeployment
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c"));
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c"));
+        context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy();
+        assertEquals(expectedRecords, tester.recordNames());
+    }
+
+    @Test
     public void generated_endpoints_only() {
         var tester = new RoutingPoliciesTester(SystemName.Public);
         var context = tester.newDeploymentContext("tenant1", "app1", "default");
@@ -1216,6 +1280,7 @@ public class RoutingPoliciesTest {
 
         // Deploy application
         var zone1 = ZoneId.from("prod", "aws-us-east-1c");
+        var zone2 = ZoneId.from("prod", "aws-eu-west-1a");
         ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region())
                                                                            .container("c0", AuthMethod.mtls)
                                                                            .endpoint("foo", "c0")
@@ -1232,6 +1297,23 @@ public class RoutingPoliciesTest {
                              "ebd395b6.cafed00d.z.vespa-app.cloud",
                              "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"),
                      tester.recordNames());
+
+        // Another zone is added to global endpoint
+        applicationPackage = applicationPackageBuilder().region(zone1.region())
+                                                        .region(zone2.region())
+                                                        .container("c0", AuthMethod.mtls)
+                                                        .endpoint("foo", "c0")
+                                                        .build();
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c"));
+        tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c"));
+        tester.provisionLoadBalancers(1, context.instanceId(), zone2);
+        context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy();
+        assertEquals(List.of("a6414896.cafed00d.aws-eu-west-1.w.vespa-app.cloud",
+                             "a9c8c045.cafed00d.g.vespa-app.cloud",
+                             "cbff1506.cafed00d.z.vespa-app.cloud",
+                             "ebd395b6.cafed00d.z.vespa-app.cloud",
+                             "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"),
+                     tester.recordNames());
     }
 
     @Test