Add legacy names also when cert has random id (#28741)

* Add legacy names also when cert has random id

* Fix test

2 files changed, 69 insertions, 4 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java
index d661fa189b9..33af58a9790 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java
@@ -11,6 +11,7 @@ import com.yahoo.transaction.Mutex;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.flags.BooleanFlag;
 import com.yahoo.vespa.flags.FetchVector;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.flags.StringFlag;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -57,6 +58,7 @@ public class EndpointCertificates {
     private final EndpointCertificateValidator certificateValidator;
     private final BooleanFlag useAlternateCertProvider;
     private final StringFlag endpointCertificateAlgo;
+    private final BooleanFlag assignLegacyNames;
     private final static Duration GCP_CERTIFICATE_EXPIRY_TIME = Duration.ofDays(100); // 100 days, 10 more than notAfter time
 
     public EndpointCertificates(Controller controller, EndpointCertificateProvider certificateProvider,
@@ -64,6 +66,7 @@ public class EndpointCertificates {
         this.controller = controller;
         this.useAlternateCertProvider = PermanentFlags.USE_ALTERNATIVE_ENDPOINT_CERTIFICATE_PROVIDER.bindTo(controller.flagSource());
         this.endpointCertificateAlgo = PermanentFlags.ENDPOINT_CERTIFICATE_ALGORITHM.bindTo(controller.flagSource());
+        this.assignLegacyNames = Flags.LEGACY_ENDPOINTS.bindTo(controller.flagSource());
         this.curator = controller.curator();
         this.clock = controller.clock();
         this.certificateProvider = certificateProvider;
@@ -175,9 +178,12 @@ public class EndpointCertificates {
         }
 
         // Re-provision certificate if it is missing SANs for the zone we are deploying to
-        // Skip this validation for now if the cert has a randomized id
+        // Skip this validation for now if the cert has a randomized id and should not provision legacy names
         Optional<EndpointCertificate> currentCertificate = assignedCertificate.map(AssignedCertificate::certificate);
-        var requiredSansForZone = currentCertificate.get().randomizedId().isEmpty() ?
+        boolean legacyNames = assignLegacyNames.with(FetchVector.Dimension.INSTANCE_ID, instance.id().serializedForm())
+                .with(FetchVector.Dimension.APPLICATION_ID, instance.id().toSerializedFormWithoutInstance()).value();
+
+        var requiredSansForZone = legacyNames || currentCertificate.get().randomizedId().isEmpty() ?
                 controller.routing().certificateDnsNames(deployment, deploymentSpec) :
                 List.<String>of();
 
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 e15e7d6792c..cbc69e52119 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
@@ -26,6 +26,7 @@ import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
 import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import java.time.Duration;
@@ -38,10 +39,13 @@ 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;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -248,9 +252,64 @@ public class EndpointCertificateMaintainerTest {
         assertEquals(3, randomizedNames.size());
     }
 
+    @Test
+    void deploy_to_other_manual_zone_refreshes_cert() {
+        String devSan = "*.foo.manual.tenant.us-east-1.dev.vespa.oath.cloud";
+        String perfSan = "*.foo.manual.tenant.us-east-3.perf.vespa.oath.cloud";
+
+        var devApp = ApplicationId.from("tenant", "manual", "foo");
+        DeploymentTester deploymentTester = new DeploymentTester(tester);
+        deployToAssignCert(deploymentTester, devApp, List.of(devUsEast1), Optional.empty());
+        assertEquals(1, tester.curator().readAssignedCertificates().size());
+        maintainer.maintain();
+        Optional<AssignedCertificate> devCertificate = tester.curator().readAssignedCertificate(TenantAndApplicationId.from(devApp), Optional.of(devApp.instance()));
+        List<String> devSans = devCertificate.get().certificate().requestedDnsSans();
+        Assertions.assertThat(devSans).contains(devSan);
+        Assertions.assertThat(devSans).doesNotContain(perfSan);
+
+        // Deploy to perf and verify that the certs are refreshed
+        deployToAssignCert(deploymentTester, devApp, List.of(perfUsEast3), Optional.empty());
+        Optional<AssignedCertificate> devAndPerfCertificate = tester.curator().readAssignedCertificate(TenantAndApplicationId.from(devApp), Optional.of(devApp.instance()));
+        List<String> devAndPerfSans = devAndPerfCertificate.get().certificate().requestedDnsSans();
+
+        assertNotEquals(devSans, devAndPerfSans);
+        Assertions.assertThat(devAndPerfSans).contains(devSan);
+        Assertions.assertThat(devAndPerfSans).contains(perfSan);
+    }
+
+    @Test
+    void deploy_to_other_prod_zone_refreshes_cert() {
+        String westSan = "*.prod.tenant.us-west-1.vespa.oath.cloud";
+        String centralSan = "*.prod.tenant.us-central-1.vespa.oath.cloud";
+
+        var prodApp = ApplicationId.from("tenant", "prod", "default");
+        DeploymentTester deploymentTester = new DeploymentTester(tester);
+        deployToAssignCert(deploymentTester, prodApp, List.of(systemTest, stagingTest, productionUsWest1), Optional.empty());
+        assertEquals(1, tester.curator().readAssignedCertificates().size());
+        maintainer.maintain();
+        Optional<AssignedCertificate> usWestCert = tester.curator().readAssignedCertificate(TenantAndApplicationId.from(prodApp), Optional.of(prodApp.instance()));
+        List<String> usWestSans = usWestCert.get().certificate().requestedDnsSans();
+        Assertions.assertThat(usWestSans).contains(westSan);
+        Assertions.assertThat(usWestSans).doesNotContain(centralSan);
+
+        // Deploy to perf and verify that the certs are refreshed
+        deployToAssignCert(deploymentTester, prodApp, List.of(systemTest, stagingTest, productionUsWest1, productionUsCentral1), Optional.empty());
+        Optional<AssignedCertificate> usCentralWestCert = tester.curator().readAssignedCertificate(TenantAndApplicationId.from(prodApp), Optional.of(prodApp.instance()));
+        List<String> usCentralWestSans = usCentralWestCert.get().certificate().requestedDnsSans();
+        assertNotEquals(usWestSans, usCentralWestSans);
+        Assertions.assertThat(usCentralWestSans).contains(westSan);
+        Assertions.assertThat(usCentralWestSans).contains(centralSan);
+    }
+
+    private void deploy() {
+
+    }
+
     private void deployToAssignCert(DeploymentTester tester, ApplicationId applicationId, List<JobType> jobTypes, Optional<String> instances) {
-        var applicationPackageBuilder = new ApplicationPackageBuilder()
-                .region("us-west-1");
+
+        var applicationPackageBuilder = new ApplicationPackageBuilder();
+        jobTypes.stream().filter(JobType::isProduction).map(job -> job.zone().region().value()).forEach(applicationPackageBuilder::region);
+
         instances.map(applicationPackageBuilder::instances);
         var applicationPackage = applicationPackageBuilder.build();