diff options
author | Martin Polden <mpolden@mpolden.no> | 2023-07-07 10:01:10 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-07 10:01:10 +0200 |
commit | 85c6a1ef03591bc1d696ef0b1a73ade7e8731bc7 (patch) | |
tree | 7110e162dab5697d6e5645508850e8c2fc40b8d9 | |
parent | 767e01520450c288b5e8161c08fa2a9ddcfff1df (diff) | |
parent | ed9fc052b2d9c96ce8961e05699f7f974ac1af92 (diff) |
Merge pull request #27665 from vespa-engine/mpolden/generated-declared-endpoints
Generate endpoint names for all scopes
9 files changed, 198 insertions, 90 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index ceac681255b..65f4c851e3c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -17,6 +17,7 @@ import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificate; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; @@ -29,6 +30,7 @@ import com.yahoo.vespa.hosted.controller.application.EndpointList; import com.yahoo.vespa.hosted.controller.application.GeneratedEndpoint; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.certificate.AssignedCertificate; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; import com.yahoo.vespa.hosted.controller.routing.RoutingId; import com.yahoo.vespa.hosted.controller.routing.RoutingPolicies; @@ -118,13 +120,22 @@ public class RoutingController { /** Read and return zone-scoped endpoints for given deployment */ public EndpointList readEndpointsOf(DeploymentId deployment) { - boolean addTokenEndpoint = createTokenEndpoint.with(FetchVector.Dimension.APPLICATION_ID, deployment.applicationId().serializedForm()).value(); + boolean addTokenEndpoint = tokenEndpointEnabled(deployment.applicationId()); Set<Endpoint> endpoints = new LinkedHashSet<>(); - // To discover the cluster name for a zone-scoped endpoint, we need to read routing policies + // To discover the cluster name for a zone-scoped endpoint, we need to read the routing policy for (var policy : routingPolicies.read(deployment)) { RoutingMethod routingMethod = controller.zoneRegistry().routingMethod(policy.id().zone()); endpoints.addAll(policy.zoneEndpointsIn(controller.system(), routingMethod, addTokenEndpoint)); - endpoints.add(policy.regionEndpointIn(controller.system(), routingMethod)); + endpoints.add(policy.regionEndpointIn(controller.system(), routingMethod, Optional.empty())); + for (var ge : policy.generatedEndpoints()) { + boolean include = switch (ge.authMethod()) { + case token -> addTokenEndpoint; + case mtls -> true; + }; + if (include) { + endpoints.add(policy.regionEndpointIn(controller.system(), routingMethod, Optional.of(ge))); + } + } } return EndpointList.copyOf(endpoints); } @@ -142,7 +153,7 @@ public class RoutingController { /** Returns endpoints declared in {@link DeploymentSpec} for given application */ public EndpointList declaredEndpointsOf(Application application) { - // TODO(mpolden): Add generated endpoints for global and application scopes. Requires reading routing polices here + List<GeneratedEndpoint> generatedEndpoints = readGeneratedEndpoints(application); Set<Endpoint> endpoints = new LinkedHashSet<>(); DeploymentSpec deploymentSpec = application.deploymentSpec(); for (var spec : deploymentSpec.instances()) { @@ -154,7 +165,7 @@ public class RoutingController { .map(zone -> new DeploymentId(instance, ZoneId.from(Environment.prod, zone.region().get()))) .toList(); RoutingId routingId = RoutingId.of(instance, EndpointId.defaultId()); - endpoints.addAll(computeGlobalEndpoints(routingId, ClusterSpec.Id.from(clusterId), deployments)); + endpoints.addAll(computeGlobalEndpoints(routingId, ClusterSpec.Id.from(clusterId), deployments, generatedEndpoints)); }); // Add endpoints declared with current syntax spec.endpoints().forEach(declaredEndpoint -> { @@ -163,7 +174,7 @@ public class RoutingController { .map(region -> new DeploymentId(instance, ZoneId.from(Environment.prod, region))) .toList(); - endpoints.addAll(computeGlobalEndpoints(routingId, ClusterSpec.Id.from(declaredEndpoint.containerId()), deployments)); + endpoints.addAll(computeGlobalEndpoints(routingId, ClusterSpec.Id.from(declaredEndpoint.containerId()), deployments, generatedEndpoints)); }); } // Add application endpoints @@ -175,13 +186,16 @@ public class RoutingController { ZoneId zone = deployments.keySet().iterator().next().zoneId(); // Where multiple zones are possible, they all have the same routing method. RoutingMethod routingMethod = usesSharedRouting(zone) ? RoutingMethod.sharedLayer4 : RoutingMethod.exclusive; - endpoints.add(Endpoint.of(application.id()) - .targetApplication(EndpointId.of(declaredEndpoint.endpointId()), - ClusterSpec.Id.from(declaredEndpoint.containerId()), - deployments) - .routingMethod(routingMethod) - .on(Port.fromRoutingMethod(routingMethod)) - .in(controller.system())); + Endpoint.EndpointBuilder builder = Endpoint.of(application.id()) + .targetApplication(EndpointId.of(declaredEndpoint.endpointId()), + ClusterSpec.Id.from(declaredEndpoint.containerId()), + deployments) + .routingMethod(routingMethod) + .on(Port.fromRoutingMethod(routingMethod)); + endpoints.add(builder.in(controller.system())); + for (var ge : generatedEndpoints) { + endpoints.add(builder.generatedFrom(ge).in(controller.system())); + } } return EndpointList.copyOf(endpoints); } @@ -196,6 +210,10 @@ public class RoutingController { if (!directEndpoints.isEmpty()) { zoneEndpoints = directEndpoints; // Use only direct endpoints if we have any } + EndpointList generatedEndpoints = zoneEndpoints.generated(); + if (!generatedEndpoints.isEmpty()) { + zoneEndpoints = generatedEndpoints; // Use generated endpoints if we have any + } if ( ! zoneEndpoints.isEmpty()) { endpoints.put(deployment.zoneId(), zoneEndpoints.asList()); } @@ -353,7 +371,7 @@ public class RoutingController { .map(region -> new DeploymentId(instance.id(), ZoneId.from(Environment.prod, region))) .toList(); endpointsToRemove.addAll(computeGlobalEndpoints(RoutingId.of(instance.id(), rotation.endpointId()), - rotation.clusterId(), deployments)); + rotation.clusterId(), deployments, readGeneratedEndpoints(application))); } endpointsToRemove.forEach(endpoint -> controller.nameServiceForwarder() .removeRecords(Record.Type.CNAME, @@ -362,12 +380,16 @@ public class RoutingController { Optional.of(application.id()))); } - /** Generate endpoints for all authenticaiton methods, using given application part */ + /** Generate endpoints for all authentication methods, using given application part */ public List<GeneratedEndpoint> generateEndpoints(String applicationPart, ApplicationId instance) { - boolean enabled = randomizedEndpoints.with(FetchVector.Dimension.APPLICATION_ID, instance.serializedForm()).value(); - if (!enabled) { + if (!randomizedEndpointsEnabled(instance)) { return List.of(); } + return generateEndpoints(applicationPart); + } + + + private List<GeneratedEndpoint> generateEndpoints(String applicationPart) { return Arrays.stream(Endpoint.AuthMethod.values()) .map(method -> new GeneratedEndpoint(GeneratedEndpoint.createPart(controller.random(true)), applicationPart, @@ -375,6 +397,23 @@ public class RoutingController { .toList(); } + /** This is only suitable for use in declared endpoints, which ignore the randomly generated cluster part */ + private List<GeneratedEndpoint> readGeneratedEndpoints(Application application) { + boolean includeTokenEndpoint = application.productionInstances().values().stream() + .map(Instance::id) + .anyMatch(this::tokenEndpointEnabled); + Optional<String> randomizedId = controller.curator().readAssignedCertificate(application.id(), Optional.empty()) + .map(AssignedCertificate::certificate) + .flatMap(EndpointCertificate::randomizedId); + if (randomizedId.isEmpty()) { + return List.of(); + } + return generateEndpoints(randomizedId.get()).stream().filter(endpoint -> switch (endpoint.authMethod()) { + case token -> includeTokenEndpoint; + case mtls -> true; + }).toList(); + } + /** * Assigns one or more global rotations to given application, if eligible. The given application is implicitly * stored, ensuring that the assigned rotation(s) are persisted when this returns. @@ -412,7 +451,7 @@ public class RoutingController { } /** Compute global endpoints for given routing ID, application and deployments */ - private List<Endpoint> computeGlobalEndpoints(RoutingId routingId, ClusterSpec.Id cluster, List<DeploymentId> deployments) { + private List<Endpoint> computeGlobalEndpoints(RoutingId routingId, ClusterSpec.Id cluster, List<DeploymentId> deployments, List<GeneratedEndpoint> generatedEndpoints) { var endpoints = new ArrayList<Endpoint>(); var directMethods = 0; var availableRoutingMethods = routingMethodsOfAll(deployments); @@ -421,15 +460,26 @@ public class RoutingController { throw new IllegalArgumentException("Invalid routing methods for " + routingId + ": Exceeded maximum " + "direct methods"); } - endpoints.add(Endpoint.of(routingId.instance()) - .target(routingId.endpointId(), cluster, deployments) - .on(Port.fromRoutingMethod(method)) - .routingMethod(method) - .in(controller.system())); + Endpoint.EndpointBuilder builder = Endpoint.of(routingId.instance()) + .target(routingId.endpointId(), cluster, deployments) + .on(Port.fromRoutingMethod(method)) + .routingMethod(method); + endpoints.add(builder.in(controller.system())); + for (var ge : generatedEndpoints) { + endpoints.add(builder.generatedFrom(ge).in(controller.system())); + } } return endpoints; } + public boolean tokenEndpointEnabled(ApplicationId instance) { + return createTokenEndpoint.with(FetchVector.Dimension.APPLICATION_ID, instance.serializedForm()).value(); + } + + public boolean randomizedEndpointsEnabled(ApplicationId instance) { + return randomizedEndpoints.with(FetchVector.Dimension.APPLICATION_ID, instance.serializedForm()).value(); + } + /** Whether legacy global DNS names should be available for given application */ private static boolean requiresLegacyNames(DeploymentSpec deploymentSpec, InstanceName instanceName) { return deploymentSpec.instance(instanceName) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index a3381819778..1a4095001ff 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -45,11 +45,11 @@ public class Endpoint { private final boolean legacy; private final RoutingMethod routingMethod; private final AuthMethod authMethod; - private final boolean generated; + private final Optional<GeneratedEndpoint> generated; private Endpoint(TenantAndApplicationId application, Optional<InstanceName> instanceName, EndpointId id, ClusterSpec.Id cluster, URI url, List<Target> targets, Scope scope, Port port, boolean legacy, - RoutingMethod routingMethod, boolean certificateName, AuthMethod authMethod, boolean generated) { + RoutingMethod routingMethod, boolean certificateName, AuthMethod authMethod, Optional<GeneratedEndpoint> generated) { Objects.requireNonNull(application, "application must be non-null"); Objects.requireNonNull(instanceName, "instanceName must be non-null"); Objects.requireNonNull(cluster, "cluster must be non-null"); @@ -59,6 +59,7 @@ public class Endpoint { Objects.requireNonNull(port, "port must be non-null"); Objects.requireNonNull(routingMethod, "routingMethod must be non-null"); Objects.requireNonNull(authMethod, "authMethod must be non-null"); + Objects.requireNonNull(generated, "generated must be non-null"); this.id = requireEndpointId(id, scope, certificateName); this.cluster = requireCluster(cluster, certificateName); this.instance = requireInstance(instanceName, scope); @@ -139,7 +140,7 @@ public class Endpoint { } /** Returns whether this endpoint is generated by the system */ - public boolean generated() { + public Optional<GeneratedEndpoint> generated() { return generated; } @@ -198,11 +199,11 @@ public class Endpoint { } private static String generatedPart(GeneratedEndpoint generated, String name, Scope scope, String separator) { - if (scope.multiDeployment()) { + return switch (scope) { // Endpoints with these scopes have a name part that is explicitly configured through deployment.xml - return sanitize(namePart(name, separator)) + generated.applicationPart(); - } - return generated.clusterPart() + separator + generated.applicationPart(); + case weighted, global, application -> sanitize(namePart(name, separator)) + generated.applicationPart(); + case zone -> generated.clusterPart() + separator + generated.applicationPart(); + }; } private static String sanitize(String part) { // TODO: Reject reserved words @@ -218,7 +219,7 @@ public class Endpoint { String scopeSymbol = scopeSymbol(scope, system, generated); if (scope == Scope.global) return scopeSymbol; if (scope == Scope.application) return scopeSymbol; - if (generated.isPresent()) return scopeSymbol; + if (scope == Scope.zone && generated.isPresent()) return scopeSymbol; ZoneId zone = targets.stream().map(target -> target.deployment.zoneId()).min(comparing(ZoneId::value)).get(); String region = zone.region().value(); @@ -596,7 +597,7 @@ public class Endpoint { } /** Sets the generated ID to use when building this */ - public EndpointBuilder generatedEndpoint(GeneratedEndpoint generated) { + public EndpointBuilder generatedFrom(GeneratedEndpoint generated) { this.generated = Optional.of(generated); this.authMethod = generated.authMethod(); return this; @@ -633,7 +634,7 @@ public class Endpoint { routingMethod, certificateName, authMethod, - generated.isPresent()); + generated); } private Scope requireUnset(Scope scope) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java index 5026fea7847..e554bb2361a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java @@ -49,7 +49,7 @@ public class EndpointList extends AbstractFilteringList<Endpoint, EndpointList> endpoint.instance().get().equals(instance)); } - /** Returns the subset of endpoints which target all of the given deployments */ + /** Returns the subset of endpoints which target all the given deployments */ public EndpointList targets(List<DeploymentId> deployments) { return matching(endpoint -> endpoint.deployments().containsAll(deployments)); } @@ -66,7 +66,7 @@ public class EndpointList extends AbstractFilteringList<Endpoint, EndpointList> /** Returns the subset of endpoints generated by the system */ public EndpointList generated() { - return matching(Endpoint::generated); + return matching(endpoint -> endpoint.generated().isPresent()); } /** Returns the subset of endpoints that require a rotation */ 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 12beaa635ac..bc4dc74afff 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,7 +11,6 @@ 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; @@ -36,7 +35,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; -import static com.yahoo.vespa.hosted.controller.certificate.UnassignedCertificate.*; +import static com.yahoo.vespa.hosted.controller.certificate.UnassignedCertificate.State; /** * Looks up stored endpoint certificate, provisions new certificates if none is found, @@ -56,7 +55,6 @@ public class EndpointCertificates { private final Clock clock; private final EndpointCertificateProvider certificateProvider; private final EndpointCertificateValidator certificateValidator; - private final BooleanFlag useRandomizedCert; private final BooleanFlag useAlternateCertProvider; private final StringFlag endpointCertificateAlgo; private final static Duration GCP_CERTIFICATE_EXPIRY_TIME = Duration.ofDays(100); // 100 days, 10 more than notAfter time @@ -64,7 +62,6 @@ public class EndpointCertificates { public EndpointCertificates(Controller controller, EndpointCertificateProvider certificateProvider, EndpointCertificateValidator certificateValidator) { this.controller = controller; - this.useRandomizedCert = Flags.RANDOMIZED_ENDPOINT_NAMES.bindTo(controller.flagSource()); this.useAlternateCertProvider = PermanentFlags.USE_ALTERNATIVE_ENDPOINT_CERTIFICATE_PROVIDER.bindTo(controller.flagSource()); this.endpointCertificateAlgo = PermanentFlags.ENDPOINT_CERTIFICATE_ALGORITHM.bindTo(controller.flagSource()); this.curator = controller.curator(); @@ -140,7 +137,7 @@ public class EndpointCertificates { } private Optional<EndpointCertificate> getOrProvision(Instance instance, ZoneId zone, DeploymentSpec deploymentSpec) { - if (useRandomizedCert.with(FetchVector.Dimension.APPLICATION_ID, instance.id().serializedForm()).value()) { + if (controller.routing().randomizedEndpointsEnabled(instance.id())) { return Optional.of(assignFromPool(instance, zone)); } Optional<AssignedCertificate> assignedCertificate = curator.readAssignedCertificate(TenantAndApplicationId.from(instance.id()), Optional.of(instance.id().instance())); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java index d1311006cde..6c7ee4d0d85 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java @@ -8,7 +8,6 @@ import com.yahoo.restapi.RestApiException; import com.yahoo.restapi.StringResponse; 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; @@ -42,15 +41,15 @@ public class EndpointCertificatesHandler extends ThreadedHttpRequestHandler { private final CuratorDb curator; private final BooleanFlag useAlternateCertProvider; private final StringFlag endpointCertificateAlgo; - private final BooleanFlag useRandomizedCert; + private final Controller controller; public EndpointCertificatesHandler(Executor executor, ServiceRegistry serviceRegistry, CuratorDb curator, Controller controller) { super(executor); this.endpointCertificateProvider = serviceRegistry.endpointCertificateProvider(); this.curator = curator; + this.controller = controller; this.useAlternateCertProvider = PermanentFlags.USE_ALTERNATIVE_ENDPOINT_CERTIFICATE_PROVIDER.bindTo(controller.flagSource()); this.endpointCertificateAlgo = PermanentFlags.ENDPOINT_CERTIFICATE_ALGORITHM.bindTo(controller.flagSource()); - this.useRandomizedCert = Flags.RANDOMIZED_ENDPOINT_NAMES.bindTo(controller.flagSource()); } public HttpResponse handle(HttpRequest request) { @@ -74,7 +73,7 @@ public class EndpointCertificatesHandler extends ThreadedHttpRequestHandler { public StringResponse reRequestEndpointCertificateFor(String instanceId, boolean ignoreExisting) { ApplicationId applicationId = ApplicationId.fromFullString(instanceId); - if (useRandomizedCert.with(FetchVector.Dimension.APPLICATION_ID, instanceId).value()) { + if (controller.routing().randomizedEndpointsEnabled(applicationId)) { throw new IllegalArgumentException("Cannot re-request certificate. " + instanceId + " is assigned certificate from a pool"); } try (var lock = curator.lock(TenantAndApplicationId.from(applicationId))) { 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 a8fc0b8dffb..30c832a7747 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 @@ -7,9 +7,6 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.transaction.Mutex; -import com.yahoo.vespa.flags.BooleanFlag; -import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.identifiers.ClusterId; @@ -63,12 +60,10 @@ public class RoutingPolicies { private final Controller controller; private final CuratorDb db; - private final BooleanFlag createTokenEndpoint; public RoutingPolicies(Controller controller) { this.controller = Objects.requireNonNull(controller, "controller must be non-null"); this.db = controller.curator(); - this.createTokenEndpoint = Flags.ENABLE_DATAPLANE_PROXY.bindTo(controller.flagSource()); try (var lock = db.lockRoutingPolicies()) { // Update serialized format for (var policy : db.readRoutingPolicies().entrySet()) { db.writeRoutingPolicies(policy.getKey(), policy.getValue()); @@ -191,7 +186,7 @@ public class RoutingPolicies { if (endpoint.scope() != Endpoint.Scope.global) throw new IllegalArgumentException("Endpoint " + endpoint + " is not global"); if (deployment.isPresent() && !endpoint.deployments().contains(deployment.get())) return; - Collection<RegionEndpoint> regionEndpoints = computeRegionEndpoints(policies, inactiveZones); + Collection<RegionEndpoint> regionEndpoints = computeRegionEndpoints(endpoint, policies, inactiveZones); Set<AliasTarget> latencyTargets = new LinkedHashSet<>(); Set<AliasTarget> inactiveLatencyTargets = new LinkedHashSet<>(); for (var regionEndpoint : regionEndpoints) { @@ -243,12 +238,15 @@ public class RoutingPolicies { } /** Compute region endpoints and their targets from given policies */ - private Collection<RegionEndpoint> computeRegionEndpoints(List<RoutingPolicy> policies, Set<ZoneId> inactiveZones) { + private Collection<RegionEndpoint> computeRegionEndpoints(Endpoint parent, List<RoutingPolicy> policies, Set<ZoneId> inactiveZones) { + if (!parent.scope().multiDeployment()) { + throw new IllegalArgumentException(parent + " has unexpected scope"); + } Map<Endpoint, RegionEndpoint> endpoints = new LinkedHashMap<>(); for (var policy : policies) { if (policy.dnsZone().isEmpty() && policy.canonicalName().isPresent()) continue; if (controller.zoneRegistry().routingMethod(policy.id().zone()) != RoutingMethod.exclusive) continue; - Endpoint endpoint = policy.regionEndpointIn(controller.system(), RoutingMethod.exclusive); + Endpoint endpoint = policy.regionEndpointIn(controller.system(), RoutingMethod.exclusive, parent.generated()); var zonePolicy = db.readZoneRoutingPolicy(policy.id().zone()); long weight = 1; if (isConfiguredOut(zonePolicy, policy, inactiveZones)) { @@ -396,7 +394,7 @@ public class RoutingPolicies { /** Update zone DNS record for given policy */ private void updateZoneDnsOf(RoutingPolicy policy, LoadBalancer loadBalancer, DeploymentId deploymentId) { - boolean addTokenEndpoint = createTokenEndpoint.with(FetchVector.Dimension.APPLICATION_ID, deploymentId.applicationId().serializedForm()).value(); + boolean addTokenEndpoint = controller.routing().tokenEndpointEnabled(deploymentId.applicationId()); for (var endpoint : policy.zoneEndpointsIn(controller.system(), RoutingMethod.exclusive, addTokenEndpoint)) { var name = RecordName.from(endpoint.dnsName()); var record = policy.canonicalName().isPresent() ? @@ -471,7 +469,7 @@ public class RoutingPolicies { * @return the updated policies */ private RoutingPolicyList removePoliciesUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList instancePolicies, @SuppressWarnings("unused") Mutex lock) { - boolean addTokenEndpoint = createTokenEndpoint.with(FetchVector.Dimension.APPLICATION_ID, allocation.deployment.applicationId().serializedForm()).value(); + boolean addTokenEndpoint = controller.routing().tokenEndpointEnabled(allocation.deployment.applicationId()); Map<RoutingPolicyId, RoutingPolicy> newPolicies = new LinkedHashMap<>(instancePolicies.asMap()); Set<RoutingPolicyId> activeIds = allocation.asPolicyIds(); RoutingPolicyList removable = instancePolicies.deployment(allocation.deployment) 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 e1ce7c2c451..0233e7502ef 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 @@ -128,20 +128,22 @@ public record RoutingPolicy(RoutingPolicyId id, endpoints.add(tokenEndpoint); } for (var generatedEndpoint : generatedEndpoints) { - GeneratedEndpoint endpointToInclude = switch (generatedEndpoint.authMethod()) { - case token -> includeTokenEndpoint ? generatedEndpoint : null; - case mtls -> generatedEndpoint; + boolean include = switch (generatedEndpoint.authMethod()) { + case token -> includeTokenEndpoint; + case mtls -> true; }; - if (endpointToInclude != null) { - endpoints.add(builder.generatedEndpoint(endpointToInclude).in(system)); + if (include) { + endpoints.add(builder.generatedFrom(generatedEndpoint).in(system)); } } return endpoints; } /** Returns the region endpoint of this */ - public Endpoint regionEndpointIn(SystemName system, RoutingMethod routingMethod) { - return endpoint(routingMethod).targetRegion(id.cluster(), id.zone()).in(system); + public Endpoint regionEndpointIn(SystemName system, RoutingMethod routingMethod, Optional<GeneratedEndpoint> generated) { + Endpoint.EndpointBuilder builder = endpoint(routingMethod).targetRegion(id.cluster(), id.zone()); + generated.ifPresent(builder::generatedFrom); + return builder.in(system); } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java index 23c029845bb..477aca86b9c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java @@ -264,6 +264,13 @@ public class EndpointTest { .targetRegion(ClusterSpec.Id.from("c1"), prodZone) .routingMethod(RoutingMethod.exclusive) .on(Port.tls()) + .in(SystemName.Public), + "https://c1.cafed00d.us-north-2.w.vespa-app.cloud/", + Endpoint.of(instance1) + .targetRegion(ClusterSpec.Id.from("c1"), prodZone) + .routingMethod(RoutingMethod.exclusive) + .generatedFrom(new GeneratedEndpoint("deadbeef", "cafed00d", Endpoint.AuthMethod.mtls)) + .on(Port.tls()) .in(SystemName.Public) ); tests.forEach((expected, endpoint) -> assertEquals(expected, endpoint.url().toString())); @@ -351,26 +358,26 @@ public class EndpointTest { var tests = Map.of( // Zone endpoint in main, unlike named endpoints, this includes the scope symbol 'z' "cafed00d.deadbeef.z.vespa.oath.cloud", - Endpoint.of(instance1).target(ClusterSpec.Id.from("c1"), deployment).generatedEndpoint(ge) + Endpoint.of(instance1).target(ClusterSpec.Id.from("c1"), deployment).generatedFrom(ge) .routingMethod(RoutingMethod.sharedLayer4).on(Port.tls()).in(SystemName.main), // Zone endpoint in public "cafed00d.deadbeef.z.vespa-app.cloud", - Endpoint.of(instance1).target(ClusterSpec.Id.from("c1"), deployment).generatedEndpoint(ge) + Endpoint.of(instance1).target(ClusterSpec.Id.from("c1"), deployment).generatedFrom(ge) .routingMethod(RoutingMethod.exclusive).on(Port.tls()).in(SystemName.Public), // Global endpoint in public "foo.deadbeef.g.vespa-app.cloud", Endpoint.of(instance1).target(EndpointId.of("foo"), ClusterSpec.Id.from("c1"), List.of(deployment)) - .generatedEndpoint(ge) + .generatedFrom(ge) .routingMethod(RoutingMethod.exclusive).on(Port.tls()).in(SystemName.Public), // Global endpoint in public, with default ID "deadbeef.g.vespa-app.cloud", Endpoint.of(instance1).target(EndpointId.defaultId(), ClusterSpec.Id.from("c1"), List.of(deployment)) - .generatedEndpoint(ge) + .generatedFrom(ge) .routingMethod(RoutingMethod.exclusive).on(Port.tls()).in(SystemName.Public), // Application endpoint in public "bar.deadbeef.a.vespa-app.cloud", Endpoint.of(TenantAndApplicationId.from(instance1)).targetApplication(EndpointId.of("bar"), deployment) - .generatedEndpoint(ge) + .generatedFrom(ge) .routingMethod(RoutingMethod.exclusive).on(Port.tls()).in(SystemName.Public) ); tests.forEach((expected, endpoint) -> assertEquals(expected, endpoint.dnsName())); 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 f6ea43f9dd9..f6ed8fd7323 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 @@ -11,6 +11,7 @@ import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.RoutingMethod; @@ -202,16 +203,21 @@ public class RoutingPoliciesTest { context.flushDnsUpdates(); // Weight of inactive zone is set to zero - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L, - zone3, 1L, - zone4, 0L)); + ApplicationId application2 = context.instanceId(); + EndpointId endpointId2 = EndpointId.of("r0"); + Map<ZoneId, Long> zoneWeights1 = ImmutableMap.of(zone1, 1L, + zone3, 1L, + zone4, 0L); + tester.assertTargets(application2, endpointId2, ClusterSpec.Id.from("c0"), 0, zoneWeights1); // Other zone in shared region is set out. Entire record group for the region is removed as all zones in the // region are out (weight sum = 0) tester.routingPolicies().setRoutingStatus(context.deploymentIdIn(zone3), RoutingStatus.Value.out, RoutingStatus.Agent.tenant); context.flushDnsUpdates(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L)); + ApplicationId application1 = context.instanceId(); + EndpointId endpointId1 = EndpointId.of("r0"); + tester.assertTargets(application1, endpointId1, ClusterSpec.Id.from("c0"), 0, ImmutableMap.of(zone1, 1L)); // Everything is set back in tester.routingPolicies().setRoutingStatus(context.deploymentIdIn(zone3), RoutingStatus.Value.in, @@ -219,9 +225,12 @@ public class RoutingPoliciesTest { tester.routingPolicies().setRoutingStatus(context.deploymentIdIn(zone4), RoutingStatus.Value.in, RoutingStatus.Agent.tenant); context.flushDnsUpdates(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L, - zone3, 1L, - zone4, 1L)); + ApplicationId application = context.instanceId(); + EndpointId endpointId = EndpointId.of("r0"); + Map<ZoneId, Long> zoneWeights = ImmutableMap.of(zone1, 1L, + zone3, 1L, + zone4, 1L); + tester.assertTargets(application, endpointId, ClusterSpec.Id.from("c0"), 0, zoneWeights); } @Test @@ -1009,7 +1018,7 @@ public class RoutingPoliciesTest { } @Test - public void generated_zone_endpoints() { + public void generated_endpoints() { var tester = new RoutingPoliciesTester(SystemName.Public); var context = tester.newDeploymentContext("tenant1", "app1", "default"); tester.controllerTester().flagSource().withBooleanFlag(Flags.RANDOMIZED_ENDPOINT_NAMES.id(), true); @@ -1020,24 +1029,47 @@ public class RoutingPoliciesTest { var zone1 = ZoneId.from("prod", "aws-us-east-1c"); var zone2 = ZoneId.from("prod", "aws-eu-west-1a"); ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region()) - .region(zone2.region()) - .build(); + .region(zone2.region()) + .endpoint("foo", "c0") + .applicationEndpoint("bar", "c0", Map.of(zone1.region().value(), Map.of(InstanceName.defaultName(), 1))) + .build(); tester.provisionLoadBalancers(clustersPerZone, context.instanceId(), zone1, zone2); context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); // Deployment creates generated zone names List<String> expectedRecords = List.of( - "a9c8c045.cafed00d.z.vespa-app.cloud", + // save me, jebus! + "b22ab332.cafed00d.z.vespa-app.cloud", + "bar.app1.tenant1.a.vespa-app.cloud", + "bar.cafed00d.a.vespa-app.cloud", + "c0.app1.tenant1.aws-eu-west-1.w.vespa-app.cloud", "c0.app1.tenant1.aws-eu-west-1a.z.vespa-app.cloud", + "c0.app1.tenant1.aws-us-east-1.w.vespa-app.cloud", "c0.app1.tenant1.aws-us-east-1c.z.vespa-app.cloud", - "e144a11b.cafed00d.z.vespa-app.cloud" + "c0.cafed00d.aws-eu-west-1.w.vespa-app.cloud", + "c0.cafed00d.aws-us-east-1.w.vespa-app.cloud", + "dd0971b4.cafed00d.z.vespa-app.cloud", + "foo.app1.tenant1.g.vespa-app.cloud", + "foo.cafed00d.g.vespa-app.cloud" ); assertEquals(expectedRecords, tester.recordNames()); assertEquals(2, tester.policiesOf(context.instanceId()).size()); for (var zone : List.of(zone1, zone2)) { - EndpointList endpoints = tester.controllerTester().controller().routing().readEndpointsOf(context.deploymentIdIn(zone)); + EndpointList endpoints = tester.controllerTester().controller().routing().readEndpointsOf(context.deploymentIdIn(zone)).scope(Endpoint.Scope.zone); assertEquals(1, endpoints.generated().size()); } + // Ordinary endpoints point to expected targets + tester.assertTargets(context.instanceId(), EndpointId.of("foo"), ClusterSpec.Id.from("c0"), 0, + Map.of(zone1, 1L, zone2, 1L)); + tester.assertTargets(context.application().id(), EndpointId.of("bar"), ClusterSpec.Id.from("c0"), 0, + Map.of(context.deploymentIdIn(zone1), 1)); + // Generated endpoints point to expected targets + tester.assertTargets(context.instanceId(), EndpointId.of("foo"), ClusterSpec.Id.from("c0"), 0, + Map.of(zone1, 1L, zone2, 1L), + true); + tester.assertTargets(context.application().id(), EndpointId.of("bar"), ClusterSpec.Id.from("c0"), 0, + Map.of(context.deploymentIdIn(zone1), 1), + true); // Next deployment does not change generated names context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); @@ -1191,15 +1223,25 @@ public class RoutingPoliciesTest { tester.controllerTester().flagSource().withBooleanFlag(Flags.ENABLE_DATAPLANE_PROXY.id(), enabled); } - /** Assert that an application endpoint points to given targets and weights */ private void assertTargets(TenantAndApplicationId application, EndpointId endpointId, ClusterSpec.Id cluster, int loadBalancerId, Map<DeploymentId, Integer> deploymentWeights) { + assertTargets(application, endpointId, cluster, loadBalancerId, deploymentWeights, false); + } + + /** Assert that an application endpoint points to given targets and weights */ + private void assertTargets(TenantAndApplicationId application, EndpointId endpointId, ClusterSpec.Id cluster, + int loadBalancerId, Map<DeploymentId, Integer> deploymentWeights, boolean generated) { Map<String, List<DeploymentId>> deploymentsByDnsName = new HashMap<>(); for (var deployment : deploymentWeights.keySet()) { EndpointList applicationEndpoints = tester.controller().routing().readDeclaredEndpointsOf(application) .named(endpointId, Endpoint.Scope.application) .targets(deployment) .cluster(cluster); + if (generated) { + applicationEndpoints = applicationEndpoints.generated(); + } else { + applicationEndpoints = applicationEndpoints.not().generated(); + } assertEquals(1, applicationEndpoints.size(), "Expected a single endpoint with ID '" + endpointId + "'"); @@ -1218,9 +1260,14 @@ public class RoutingPoliciesTest { }); } - /** Assert that an instance endpoint points to given targets and weights */ private void assertTargets(ApplicationId instance, EndpointId endpointId, ClusterSpec.Id cluster, int loadBalancerId, Map<ZoneId, Long> zoneWeights) { + assertTargets(instance, endpointId, cluster, loadBalancerId, zoneWeights, false); + } + + /** Assert that a global endpoint points to given zones and weights */ + private void assertTargets(ApplicationId instance, EndpointId endpointId, ClusterSpec.Id cluster, + int loadBalancerId, Map<ZoneId, Long> zoneWeights, boolean generated) { Set<String> latencyTargets = new HashSet<>(); Map<String, List<ZoneId>> zonesByRegionEndpoint = new HashMap<>(); for (var zone : zoneWeights.keySet()) { @@ -1228,7 +1275,12 @@ public class RoutingPoliciesTest { EndpointList regionEndpoints = tester.controller().routing().readEndpointsOf(deployment) .cluster(cluster) .scope(Endpoint.Scope.weighted); - Endpoint regionEndpoint = regionEndpoints.first().orElseThrow(() -> new IllegalArgumentException("No region endpoint found for " + cluster + " in " + deployment)); + if (generated) { + regionEndpoints = regionEndpoints.generated(); + } else { + regionEndpoints = regionEndpoints.not().generated(); + } + Endpoint regionEndpoint = regionEndpoints.first().orElseThrow(() -> new IllegalArgumentException("No" + (generated ? " generated" : "") + " region endpoint found for " + cluster + " in " + deployment)); zonesByRegionEndpoint.computeIfAbsent(regionEndpoint.dnsName(), (k) -> new ArrayList<>()) .add(zone); } @@ -1246,16 +1298,22 @@ public class RoutingPoliciesTest { latencyTargets.add(latencyTarget); }); List<DeploymentId> deployments = zoneWeights.keySet().stream().map(z -> new DeploymentId(instance, z)).toList(); - String globalEndpoint = tester.controller().routing().readDeclaredEndpointsOf(instance) - .named(endpointId, Endpoint.Scope.global) - .targets(deployments) - .primary() + EndpointList global = tester.controller().routing().readDeclaredEndpointsOf(instance) + .named(endpointId, Endpoint.Scope.global) + .targets(deployments); + if (generated) { + global = global.generated(); + } else { + global = global.not().generated(); + } + String globalEndpoint = global.primary() .map(Endpoint::dnsName) .orElse("<none>"); assertEquals(latencyTargets, Set.copyOf(aliasDataOf(globalEndpoint)), "Global endpoint " + globalEndpoint + " points to expected latency targets"); } + /** Assert that a global endpoint points to given zones */ private void assertTargets(ApplicationId application, EndpointId endpointId, int loadBalancerId, ZoneId... zones) { Map<ZoneId, Long> zoneWeights = new LinkedHashMap<>(); for (var zone : zones) { @@ -1264,10 +1322,6 @@ public class RoutingPoliciesTest { assertTargets(application, endpointId, ClusterSpec.Id.from("c" + loadBalancerId), loadBalancerId, zoneWeights); } - private void assertTargets(ApplicationId application, EndpointId endpointId, int loadBalancerId, Map<ZoneId, Long> zoneWeights) { - assertTargets(application, endpointId, ClusterSpec.Id.from("c" + loadBalancerId), loadBalancerId, zoneWeights); - } - } } |