Revert "Jonmv/private endpoints"

7 files changed, 13 insertions, 151 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 2693fdcbd7c..4159b099387 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -946,7 +946,7 @@ public class ApplicationController {
         // Either the user is member of the domain admin role, or is given the "launch" privilege on the service.
         Optional<AthenzUser> athenzUser = getUser(deployer);
         if (athenzUser.isPresent()) {
-            // We only need to validate the root and instance in deployment.xml. Dev/perf entries are found at the instance level as well.
+            // We only need to validate the root and instance in deployment.xml. Not possible to add dev or perf tags to deployment.xml
             var zone = zoneId.orElseThrow(() -> new IllegalArgumentException("Unable to evaluate access, no zone provided in deployment"));
             var serviceToLaunch = instanceName
                     .flatMap(instance -> applicationPackage.deploymentSpec().instance(instance))
@@ -954,7 +954,7 @@ public class ApplicationController {
                     .or(() -> applicationPackage.deploymentSpec().athenzService())
                     .map(service -> new AthenzService(identityDomain.get(), service.value()));
 
-            if (serviceToLaunch.isPresent()) {
+            if(serviceToLaunch.isPresent()) {
                 if (
                         ! ((AthenzFacade) accessControl).canLaunch(athenzUser.get(), serviceToLaunch.get()) && // launch privilege
                         ! ((AthenzFacade) accessControl).hasTenantAdminAccess(athenzUser.get(), identityDomain.get()) // tenant admin
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java
index 842d99abe71..bce9d44f8c6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java
@@ -4,28 +4,26 @@ package com.yahoo.vespa.hosted.controller.application.pkg;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentInstanceSpec;
 import com.yahoo.config.application.api.DeploymentSpec;
-import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone;
 import com.yahoo.config.application.api.Endpoint;
 import com.yahoo.config.application.api.Endpoint.Level;
 import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.CloudName;
-import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.RegionName;
-import com.yahoo.config.provision.ZoneEndpoint;
-import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn;
 import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.application.EndpointId;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -152,10 +150,10 @@ public class ApplicationPackageValidator {
 
     /** Verify endpoint configuration of given application package */
     private void validateEndpointChange(Application application, ApplicationPackage applicationPackage, Instant instant) {
-        for (DeploymentInstanceSpec instance : applicationPackage.deploymentSpec().instances()) {
-            validateGlobalEndpointChanges(application, instance.name(), applicationPackage, instant);
-            validateZoneEndpointChanges(application, instance.name(), applicationPackage, instant);
-        }
+        applicationPackage.deploymentSpec().instances().forEach(instance -> validateEndpointChange(application,
+                                                                                                   instance.name(),
+                                                                                                   applicationPackage,
+                                                                                                   instant));
     }
 
     /** Verify that compactable endpoint parts (instance name and endpoint ID) do not clash */
@@ -178,7 +176,7 @@ public class ApplicationPackageValidator {
     }
 
     /** Verify changes to endpoint configuration by comparing given application package to the existing one, if any */
-    private void validateGlobalEndpointChanges(Application application, InstanceName instanceName, ApplicationPackage applicationPackage, Instant instant) {
+    private void validateEndpointChange(Application application, InstanceName instanceName, ApplicationPackage applicationPackage, Instant instant) {
         var validationId = ValidationId.globalEndpointChange;
         if (applicationPackage.validationOverrides().allows(validationId, instant)) return;
 
@@ -202,43 +200,6 @@ public class ApplicationPackageValidator {
                                            ". " + ValidationOverrides.toAllowMessage(validationId));
     }
 
-    /** Verify changes to endpoint configuration by comparing given application package to the existing one, if any */
-    private void validateZoneEndpointChanges(Application application, InstanceName instance, ApplicationPackage applicationPackage, Instant now) {
-        ValidationId validationId = ValidationId.zoneEndpointChange;
-        if (applicationPackage.validationOverrides().allows(validationId, now)) return;;
-
-        String prefix = validationId + ": application '" + application.id() +
-                        (instance.isDefault() ? "" : "." + instance.value()) + "' ";
-        DeploymentInstanceSpec spec = applicationPackage.deploymentSpec().requireInstance(instance);
-        for (DeclaredZone zone : spec.zones()) {
-            if (zone.environment() == Environment.prod) {
-                Map<ClusterSpec.Id, ZoneEndpoint> newEndpoints = spec.zoneEndpoints(ZoneId.from(zone.environment(), zone.region().get()));
-                application.deploymentSpec().instance(instance)                                     // If old spec has this instance ...
-                           .filter(oldSpec -> oldSpec.concerns(zone.environment(), zone.region()))  // ... and deploys to this zone ...
-                           .map(oldSpec -> oldSpec.zoneEndpoints(ZoneId.from(zone.environment(), zone.region().get())))
-                           .ifPresent(oldEndpoints -> {                                             // ... then we compare the endpoints present in both.
-                               oldEndpoints.forEach((cluster, oldEndpoint) -> {
-                                   ZoneEndpoint newEndpoint = newEndpoints.getOrDefault(cluster, ZoneEndpoint.defaultEndpoint);
-                                   if ( ! newEndpoint.allowedUrns().containsAll(oldEndpoint.allowedUrns()))
-                                       throw new IllegalArgumentException(prefix + "allows access to cluster '" + cluster.value() +
-                                                                          "' in '" + zone.region().get().value() + "' to " +
-                                                                          oldEndpoint.allowedUrns().stream().map(AllowedUrn::toString).collect(joining(", ", "[", "]")) +
-                                                                          ", but does not include all these in the new deployment spec. " +
-                                                                          "Deploying with the new settings will allow access to " +
-                                                                          (newEndpoint.allowedUrns().isEmpty() ? "no one" : newEndpoint.allowedUrns().stream().map(AllowedUrn::toString).collect(joining(", ", "[", "]"))));
-                               });
-                               newEndpoints.forEach((cluster, newEndpoint) -> {
-                                    ZoneEndpoint oldEndpoint = oldEndpoints.getOrDefault(cluster, ZoneEndpoint.defaultEndpoint);
-                                    if (oldEndpoint.isPublicEndpoint() && ! newEndpoint.isPublicEndpoint())
-                                        throw new IllegalArgumentException(prefix + "has a public endpoint for cluster '" + cluster.value() +
-                                                                           "' in '" + zone.region().get().value() + "', but the new deployment spec " +
-                                                                           "disables this");
-                               });
-                           });
-            }
-        }
-    }
-
     /** Returns whether newEndpoints contains all destinations in endpoints */
     private static boolean containsAllDestinationsOf(List<Endpoint> endpoints, List<Endpoint> newEndpoints) {
         var containsAllRegions = true;
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index 73c64be3e47..674d71001e3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -619,7 +619,7 @@ public class JobController {
         submission.applicationPackage().deploymentSpec().majorVersion().ifPresent(explicitMajor -> {
             if ( ! controller.readVersionStatus().isOnCurrentMajor(new Version(explicitMajor)))
                 controller.notificationsDb().setNotification(NotificationSource.from(id), Type.submission, Notification.Level.warning,
-                                                             "Vespa " + explicitMajor + " will soon reach end of life, upgrade to Vespa " + (explicitMajor + 1) + " now: " +
+                                                             "Vespa " + explicitMajor + " will soon be end of life, upgrade to Vespa " + (explicitMajor + 1) + " now: " +
                                                              "https://cloud.vespa.ai/en/vespa" + (explicitMajor + 1) + "-release-notes.html"); // ∠( ᐛ 」∠)＿
         });
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 3f68be611eb..c9ad34b5082 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -23,7 +23,6 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn;
 import com.yahoo.config.provision.zone.RoutingMethod;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.container.handler.metrics.JsonResponse;
@@ -1979,15 +1978,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
             lbObject.setString("cluster", lb.cluster().value());
             lb.service().ifPresent(service -> {
                 lbObject.setString("serviceId", service.id()); // Really the "serviceName", but this is what the user needs >_<
-                Cursor urnsArray = lbObject.setArray("allowedUrns");
-                for (AllowedUrn urn : service.allowedUrns()) {
-                    Cursor urnObject = urnsArray.addObject();
-                    urnObject.setString("type", switch (urn.type()) {
-                        case awsPrivateLink -> "aws-private-link";
-                        case gcpServiceConnect -> "gcp-service-connect";
-                    });
-                    urnObject.setString("urn", urn.urn());
-                }
+                service.allowedUrns().forEach(lbObject.setArray("allowedUrns")::addString);
                 Cursor endpointsArray = lbObject.setArray("endpoints");
                 controller.serviceRegistry().vpcEndpointService()
                           .getConnections(new ClusterId(id, lb.cluster()),
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index db96c265363..95b81dffaed 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -1118,94 +1118,6 @@ public class ControllerTest {
     }
 
     @Test
-    void testZoneEndpointChanges() {
-        DeploymentContext app = tester.newDeploymentContext();
-        // Set up app with default settings.
-        app.submit(ApplicationPackageBuilder.fromDeploymentXml("""
-                                                               <deployment>
-                                                                 <prod>
-                                                                   <region>us-east-3</region>
-                                                                 </prod>
-                                                               </deployment>"""));
-
-        assertEquals("zone-endpoint-change: application 'tenant.application' has a public endpoint for cluster 'foo' in 'us-east-3', but the new deployment spec disables this",
-                     assertThrows(IllegalArgumentException.class,
-                                  () -> app.submit(ApplicationPackageBuilder.fromDeploymentXml("""
-                                                                                               <deployment>
-                                                                                                 <prod>
-                                                                                                   <region>us-east-3</region>
-                                                                                                 </prod>
-                                                                                                 <endpoints>
-                                                                                                   <endpoint type='zone' container-id='foo' enabled='false' />
-                                                                                                 </endpoints>
-                                                                                               </deployment>""")))
-                             .getMessage());
-
-        // Disabling endpoints is OK with override.
-        app.submit(ApplicationPackageBuilder.fromDeploymentXml("""
-                                                               <deployment>
-                                                                 <prod>
-                                                                   <region>us-east-3</region>
-                                                                 </prod>
-                                                                 <endpoints>
-                                                                   <endpoint type='zone' container-id='foo' enabled='false' />
-                                                                 </endpoints>
-                                                               </deployment>""",
-                                                               ValidationId.zoneEndpointChange));
-
-        // Enabling endpoints again is OK, as is adding a private endpoint with some URN.
-        app.submit(ApplicationPackageBuilder.fromDeploymentXml("""
-                                                               <deployment>
-                                                                 <prod>
-                                                                   <region>us-east-3</region>
-                                                                 </prod>
-                                                                 <endpoints>
-                                                                   <endpoint type='private' container-id='foo'>
-                                                                     <allow with='aws-private-link' arn='yarn' />
-                                                                   </endpoint>
-                                                                 </endpoints>
-                                                               </deployment>""",
-                                                               ValidationId.zoneEndpointChange));
-
-        // Changing URNs is guarded.
-        assertEquals("zone-endpoint-change: application 'tenant.application' allows access to cluster 'foo' in 'us-east-3' to " +
-                     "['yarn' through 'aws-private-link'], but does not include all these in the new deployment spec. " +
-                     "Deploying with the new settings will allow access to ['yarn' through 'gcp-service-connect']",
-                     assertThrows(IllegalArgumentException.class,
-                                  () -> app.submit(ApplicationPackageBuilder.fromDeploymentXml("""
-                                                                                               <deployment>
-                                                                                                 <prod>
-                                                                                                   <region>us-east-3</region>
-                                                                                                 </prod>
-                                                                                                 <endpoints>
-                                                                                                   <endpoint type='private' container-id='foo'>
-                                                                                                     <allow with='gcp-service-connect' project='yarn' />
-                                                                                                   </endpoint>
-                                                                                                 </endpoints>
-                                                                                               </deployment>""")))
-                             .getMessage());
-
-        // Changing cluster, effectively removing old URNs, is also guarded.
-        assertEquals("zone-endpoint-change: application 'tenant.application' allows access to cluster 'foo' in 'us-east-3' to " +
-                     "['yarn' through 'aws-private-link'], but does not include all these in the new deployment spec. " +
-                     "Deploying with the new settings will allow access to no one",
-                     assertThrows(IllegalArgumentException.class,
-                                  () -> app.submit(ApplicationPackageBuilder.fromDeploymentXml("""
-                                                                                               <deployment>
-                                                                                                 <prod>
-                                                                                                   <region>us-east-3</region>
-                                                                                                 </prod>
-                                                                                                 <endpoints>
-                                                                                                   <endpoint type='private' container-id='bar'>
-                                                                                                     <allow with='aws-private-link' arn='yarn' />
-                                                                                                   </endpoint>
-                                                                                                 </endpoints>
-                                                                                               </deployment>""")))
-                             .getMessage());
-    }
-
-
-        @Test
     void testReadableApplications() {
         var db = new MockCuratorDb(tester.controller().system());
         var tester = new DeploymentTester(new ControllerTester(db));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 6f859ff3d15..93b804805c3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -16,8 +16,6 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn;
-import com.yahoo.config.provision.ZoneEndpoint.AccessType;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.flags.json.FlagData;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics;
@@ -419,7 +417,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
                                                                    LoadBalancer.State.active,
                                                                    Optional.of("dns-zone-1"),
                                                                    Optional.empty(),
-                                                                   Optional.of(new PrivateServiceInfo("service", List.of(new AllowedUrn(AccessType.awsPrivateLink, "arne")))))));
+                                                                   Optional.of(new PrivateServiceInfo("service", List.of("arne"))))));
         }
 
         Application application = applications.get(id);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index be405c7b876..d40485ff5c0 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -701,7 +701,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/private-service", GET)
                                       .userIdentity(USER_ID),
                               """
-                              {"loadBalancers":[{"cluster":"default","serviceId":"service","allowedUrns":[{"type":"aws-private-link","urn":"arne"}],"endpoints":[{"endpointId":"endpoint-1","state":"available"}]}]}""");
+                              {"loadBalancers":[{"cluster":"default","serviceId":"service","allowedUrns":["arne"],"endpoints":[{"endpointId":"endpoint-1","state":"available"}]}]}""");
 
         // GET service/state/v1
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/service/storagenode/host.com/state/v1/?foo=bar", GET)