diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2023-01-18 18:32:50 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-18 18:32:50 +0100 |
commit | ce73e7681cf25865bf6f417f176eea1c85f5efba (patch) | |
tree | bd83cefec54d52788217abddcac020dbc59ba0df /controller-server | |
parent | e0191b4d49048f9398395dc8c1c60dfcb383f705 (diff) |
Revert "Jonmv/private endpoints"
Diffstat (limited to 'controller-server')
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) |