From b44b2ce5fd913f56d781dba9efe7cb6006584a7d Mon Sep 17 00:00:00 2001 From: jonmv Date: Thu, 26 Jan 2023 09:18:25 +0100 Subject: Store whether load balancers are public, in routing policy --- .../persistence/RoutingPolicySerializer.java | 6 ++- .../hosted/controller/routing/RoutingPolicies.java | 3 +- .../hosted/controller/routing/RoutingPolicy.java | 17 +++++-- .../controller/deployment/DeploymentContext.java | 3 +- .../controller/integration/ConfigServerMock.java | 3 +- .../persistence/RoutingPolicySerializerTest.java | 58 ++++++++++++---------- .../controller/routing/RoutingPoliciesTest.java | 6 ++- 7 files changed, 60 insertions(+), 36 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java index 4d759056dfc..47b27aac79a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java @@ -50,6 +50,7 @@ public class RoutingPolicySerializer { private static final String agentField = "agent"; private static final String changedAtField = "changedAt"; private static final String statusField = "status"; + private static final String privateOnlyField = "private"; public Slime toSlime(List routingPolicies) { var slime = new Slime(); @@ -68,6 +69,7 @@ public class RoutingPolicySerializer { policy.applicationEndpoints().forEach(endpointId -> applicationEndpointsArray.addString(endpointId.id())); policyObject.setBool(loadBalancerActiveField, policy.status().isActive()); globalRoutingToSlime(policy.status().routingStatus(), policyObject.setObject(globalRoutingField)); + if ( ! policy.isPublic()) policyObject.setBool(privateOnlyField, true); }); return slime; } @@ -84,6 +86,7 @@ public class RoutingPolicySerializer { RoutingPolicyId id = new RoutingPolicyId(owner, ClusterSpec.Id.from(inspect.field(clusterField).asString()), ZoneId.from(inspect.field(zoneField).asString())); + boolean isPublic = ! inspect.field(privateOnlyField).asBool(); policies.add(new RoutingPolicy(id, SlimeUtils.optionalString(inspect.field(canonicalNameField)).map(DomainName::of), SlimeUtils.optionalString(inspect.field(ipAddressField)), @@ -91,7 +94,8 @@ public class RoutingPolicySerializer { instanceEndpoints, applicationEndpoints, new RoutingPolicy.Status(inspect.field(loadBalancerActiveField).asBool(), - globalRoutingFromSlime(inspect.field(globalRoutingField))))); + globalRoutingFromSlime(inspect.field(globalRoutingField))), + isPublic)); }); return Collections.unmodifiableList(policies); } 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 8be0b78acbd..1c4916b9bed 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 @@ -361,7 +361,8 @@ public class RoutingPolicies { var newPolicy = new RoutingPolicy(policyId, loadBalancer.hostname(), loadBalancer.ipAddress(), dnsZone, allocation.instanceEndpointsOf(loadBalancer), allocation.applicationEndpointsOf(loadBalancer), - new RoutingPolicy.Status(isActive(loadBalancer), RoutingStatus.DEFAULT)); + new RoutingPolicy.Status(isActive(loadBalancer), RoutingStatus.DEFAULT), + loadBalancer.isPublic()); // Preserve global routing status for existing policy if (existingPolicy != null) { newPolicy = newPolicy.with(newPolicy.status().with(existingPolicy.status().routingStatus())); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java index 3d43e42af27..acf3ba99a1a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java @@ -29,11 +29,12 @@ public record RoutingPolicy(RoutingPolicyId id, Optional dnsZone, Set instanceEndpoints, Set applicationEndpoints, - Status status) { + Status status, + boolean isPublic) { /** DO NOT USE. Public for serialization purposes */ public RoutingPolicy(RoutingPolicyId id, Optional canonicalName, Optional ipAddress, Optional dnsZone, - Set instanceEndpoints, Set applicationEndpoints, Status status) { + Set instanceEndpoints, Set applicationEndpoints, Status status, boolean isPublic) { this.id = Objects.requireNonNull(id, "id must be non-null"); this.canonicalName = Objects.requireNonNull(canonicalName, "canonicalName must be non-null"); this.ipAddress = Objects.requireNonNull(ipAddress, "ipAddress must be non-null"); @@ -41,10 +42,15 @@ public record RoutingPolicy(RoutingPolicyId id, this.instanceEndpoints = ImmutableSortedSet.copyOf(Objects.requireNonNull(instanceEndpoints, "instanceEndpoints must be non-null")); this.applicationEndpoints = ImmutableSortedSet.copyOf(Objects.requireNonNull(applicationEndpoints, "applicationEndpoints must be non-null")); this.status = Objects.requireNonNull(status, "status must be non-null"); + this.isPublic = isPublic; if (canonicalName.isEmpty() == ipAddress.isEmpty()) throw new IllegalArgumentException("Exactly 1 of canonicalName=%s and ipAddress=%s must be set".formatted( canonicalName.map(DomainName::value).orElse(""), ipAddress.orElse(""))); + if ( ! instanceEndpoints.isEmpty() && ! isPublic) + throw new IllegalArgumentException("Non-public zone endpoint cannot be part of any global endpoint, but was in: " + instanceEndpoints); + if ( ! applicationEndpoints.isEmpty() && ! isPublic) + throw new IllegalArgumentException("Non-public zone endpoint cannot be part of any application endpoint, but was in: " + applicationEndpoints); } /** The ID of this */ @@ -82,6 +88,11 @@ public record RoutingPolicy(RoutingPolicyId id, return status; } + /** Returns whether this has a load balancer which is available from public internet. */ + public boolean isPublic() { + return isPublic; + } + /** Returns whether this policy applies to given deployment */ public boolean appliesTo(DeploymentId deployment) { return id.owner().equals(deployment.applicationId()) && @@ -90,7 +101,7 @@ public record RoutingPolicy(RoutingPolicyId id, /** Returns a copy of this with status set to given status */ public RoutingPolicy with(Status status) { - return new RoutingPolicy(id, canonicalName, ipAddress, dnsZone, instanceEndpoints, applicationEndpoints, status); + return new RoutingPolicy(id, canonicalName, ipAddress, dnsZone, instanceEndpoints, applicationEndpoints, status, isPublic); } /** Returns the zone endpoints of this */ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java index 7edb458c154..95c22480c0f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java @@ -277,7 +277,8 @@ public class DeploymentContext { Optional.empty(), Set.of(EndpointId.of("default")), Set.of(), - new RoutingPolicy.Status(false, RoutingStatus.DEFAULT))); + new RoutingPolicy.Status(false, RoutingStatus.DEFAULT), + true)); tester.controller().curator().writeRoutingPolicies(instanceId, List.copyOf(policies.values())); return this; } 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..e2b421afe61 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 @@ -419,7 +419,8 @@ 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(new AllowedUrn(AccessType.awsPrivateLink, "arne")))), + true))); } Application application = applications.get(id); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java index 6285c5c4aac..8e0b1dd1d4e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java @@ -32,35 +32,38 @@ public class RoutingPolicySerializerTest { var instanceEndpoints = Set.of(EndpointId.of("r1"), EndpointId.of("r2")); var applicationEndpoints = Set.of(EndpointId.of("a1")); var id1 = new RoutingPolicyId(owner, - ClusterSpec.Id.from("my-cluster1"), - ZoneId.from("prod", "us-north-1")); + ClusterSpec.Id.from("my-cluster1"), + ZoneId.from("prod", "us-north-1")); var id2 = new RoutingPolicyId(owner, - ClusterSpec.Id.from("my-cluster2"), - ZoneId.from("prod", "us-north-2")); + ClusterSpec.Id.from("my-cluster2"), + ZoneId.from("prod", "us-north-2")); var policies = List.of(new RoutingPolicy(id1, - Optional.of(HostName.of("long-and-ugly-name")), - Optional.empty(), - Optional.of("zone1"), - instanceEndpoints, - applicationEndpoints, - new RoutingPolicy.Status(true, RoutingStatus.DEFAULT)), - new RoutingPolicy(id2, - Optional.of(HostName.of("long-and-ugly-name-2")), - Optional.empty(), - Optional.empty(), - instanceEndpoints, - Set.of(), - new RoutingPolicy.Status(false, - new RoutingStatus(RoutingStatus.Value.out, - RoutingStatus.Agent.tenant, - Instant.ofEpochSecond(123)))), - new RoutingPolicy(id1, - Optional.empty(), - Optional.of("127.0.0.1"), - Optional.of("zone2"), - instanceEndpoints, - applicationEndpoints, - new RoutingPolicy.Status(true, RoutingStatus.DEFAULT))); + Optional.of(HostName.of("long-and-ugly-name")), + Optional.empty(), + Optional.of("zone1"), + Set.of(), + Set.of(), + new RoutingPolicy.Status(true, RoutingStatus.DEFAULT), + false), + new RoutingPolicy(id2, + Optional.of(HostName.of("long-and-ugly-name-2")), + Optional.empty(), + Optional.empty(), + instanceEndpoints, + Set.of(), + new RoutingPolicy.Status(false, + new RoutingStatus(RoutingStatus.Value.out, + RoutingStatus.Agent.tenant, + Instant.ofEpochSecond(123))), + true), + new RoutingPolicy(id1, + Optional.empty(), + Optional.of("127.0.0.1"), + Optional.of("zone2"), + instanceEndpoints, + applicationEndpoints, + new RoutingPolicy.Status(true, RoutingStatus.DEFAULT), + true)); var serialized = serializer.fromSlime(owner, serializer.toSlime(policies)); assertEquals(policies.size(), serialized.size()); for (Iterator it1 = policies.iterator(), it2 = serialized.iterator(); it1.hasNext(); ) { @@ -73,6 +76,7 @@ public class RoutingPolicySerializerTest { assertEquals(expected.instanceEndpoints(), actual.instanceEndpoints()); assertEquals(expected.applicationEndpoints(), actual.applicationEndpoints()); assertEquals(expected.status(), actual.status()); + assertEquals(expected.isPublic(), actual.isPublic()); } } 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 9c1253c7520..2932860efaa 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 @@ -497,7 +497,8 @@ public class RoutingPoliciesTest { LoadBalancer.State.active, Optional.of("dns-zone-1"), Optional.empty(), - Optional.empty()); + Optional.empty(), + true); tester.controllerTester().configServer().putLoadBalancers(zone1, List.of(loadBalancer)); // Application redeployment preserves DNS record @@ -952,7 +953,8 @@ public class RoutingPoliciesTest { LoadBalancer.State.active, Optional.of("dns-zone-1").filter(__ -> lbHostname.isPresent()), Optional.empty(), - Optional.empty())); + Optional.empty(), + true)); } return loadBalancers; } -- cgit v1.2.3