Merge pull request #26102 from vespa-engine/bratseth/bcp-endpoints

Complete bcp handling

8 files changed, 397 insertions, 134 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java
index af369dc2672..8f88b1ba74c 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java
@@ -86,17 +86,30 @@ public class Bcp {
 
     public static class Group {
 
-        private final Duration deadline;
         private final List<RegionMember> members;
+        private final List<Endpoint> endpoints;
+        private final Duration deadline;
 
         public Group(List<RegionMember> members, Duration deadline) {
+            this(members, List.of(), deadline);
+        }
+
+        public Group(List<RegionMember> members, List<Endpoint> endpoints, Duration deadline) {
             this.members = List.copyOf(members);
+            this.endpoints = endpoints;
             this.deadline = deadline;
         }
 
         public List<RegionMember> members() { return members; }
 
         /**
+         * Returns the endpoints defined in this.
+         * These will be added to instances during XML import post processing
+         * and should not otherwise be exposed from here.
+         */
+        List<Endpoint> endpoints() { return endpoints; }
+
+        /**
          * Returns the max time until the other regions must be able to handle the additional traffic
          * when a region becomes unreachable, which by default is Duration.ZERO.
          */
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
index fd355d427a3..1582d03df9f 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
@@ -45,7 +45,7 @@ public class Endpoint {
         this.level = Objects.requireNonNull(level, "level must be non-null");
         this.targets = List.copyOf(Objects.requireNonNull(targets, "targets must be non-null"));
         if (endpointId().length() > endpointMaxLength || !endpointPattern.matcher(endpointId()).matches()) {
-            throw new IllegalArgumentException("Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, " +
+            throw new IllegalArgumentException("Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, " +
                                                "of length 1 to 12, and begin with a character; but got '" + endpointId() + "'");
         }
         if (targets.isEmpty()) throw new IllegalArgumentException("targets must be non-empty");
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
index be6be5566a8..91cb2f2622e 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
@@ -46,8 +46,10 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -136,8 +138,10 @@ public class DeploymentSpecXmlReader {
 
         List<Step> steps = new ArrayList<>();
         List<Endpoint> applicationEndpoints = new ArrayList<>();
+        Bcp defaultBcp;
         if ( ! containsTag(instanceTag, root)) { // deployment spec skipping explicit instance -> "default" instance
             steps.addAll(readInstanceContent("default", root, new HashMap<>(), root));
+            defaultBcp = Bcp.empty();
         }
         else {
             if (XML.getChildren(root).stream().anyMatch(child -> child.getTagName().equals(prodTag)))
@@ -154,6 +158,7 @@ public class DeploymentSpecXmlReader {
                 }
             }
             readEndpoints(root, Optional.empty(), steps, applicationEndpoints, Map.of());
+            defaultBcp = readBcp(root, Optional.empty(), steps, List.of(), Map.of());
         }
 
         return new DeploymentSpec(steps,
@@ -162,7 +167,7 @@ public class DeploymentSpecXmlReader {
                                   stringAttribute(athenzServiceAttribute, root).map(AthenzService::from),
                                   stringAttribute(cloudAccountAttribute, root).map(CloudAccount::from),
                                   applicationEndpoints,
-                                  readBcp(root),
+                                  defaultBcp,
                                   xmlForm,
                                   deprecatedElements);
     }
@@ -210,6 +215,8 @@ public class DeploymentSpecXmlReader {
         List<Endpoint> endpoints = new ArrayList<>();
         Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> zoneEndpoints = new LinkedHashMap<>();
         readEndpoints(instanceElement, Optional.of(instanceNameString), steps, endpoints, zoneEndpoints);
+        Bcp bcp = readBcp(instanceElement, Optional.of(instanceNameString), steps, endpoints, zoneEndpoints);
+        validateEndpoints(endpoints);
 
         // Build and return instances with these values
         Instant now = clock.instant();
@@ -230,11 +237,19 @@ public class DeploymentSpecXmlReader {
                                                              notifications,
                                                              endpoints,
                                                              zoneEndpoints,
-                                                             readBcp(instanceElement),
+                                                             bcp,
                                                              now))
                      .toList();
     }
 
+    private void validateEndpoints(List<Endpoint> endpoints) {
+        Set<String> endpointIds = new HashSet<>();
+        for (Endpoint endpoint : endpoints) {
+            if ( ! endpointIds.add(endpoint.endpointId()))
+                illegal("Endpoint id '" + endpoint.endpointId() + "' is specified multiple times");
+        }
+    }
+
     private List<Step> readSteps(Element stepTag, Map<String, String> prodAttributes, Element parentTag) {
         if (stepTag.getTagName().equals(instanceTag))
             return new ArrayList<>(readInstanceContent(stepTag.getAttribute(idAttribute), stepTag, prodAttributes, parentTag));
@@ -329,140 +344,161 @@ public class DeploymentSpecXmlReader {
         if (endpointsElement == null) return;
 
         Endpoint.Level level = instance.isEmpty() ? Endpoint.Level.application : Endpoint.Level.instance;
-        Map<String, Endpoint> endpointsById = new LinkedHashMap<>();
         Map<String, Map<RegionName, List<ZoneEndpoint>>> endpointsByZone = new LinkedHashMap<>();
-        XML.getChildren(endpointsElement, endpointTag).stream() // Read zone settings first.
-                .sorted(comparingInt(endpoint -> getZoneEndpointType(endpoint, level).isPresent() ? 0 : 1))
-                .forEach(endpointElement -> {
-            String containerId = requireStringAttribute("container-id", endpointElement);
-            Optional<String> endpointId = stringAttribute("id", endpointElement);
-            Optional<String> zoneEndpointType = getZoneEndpointType(endpointElement, level);
-            String msgPrefix = (level == Endpoint.Level.application ? "Application-level" : "Instance-level") +
-                               " endpoint '" + endpointId.orElse(Endpoint.DEFAULT_ID) + "': ";
-
-            if (zoneEndpointType.isPresent() && endpointId.isPresent())
-                illegal(msgPrefix + "cannot declare 'id' with type 'zone' or 'private'");
-
-            String invalidChild = level == Endpoint.Level.application ? "region" : "instance";
-            if ( ! XML.getChildren(endpointElement, invalidChild).isEmpty())
-                illegal(msgPrefix + "invalid element '" + invalidChild + "'");
-
-            boolean enabled = XML.attribute("enabled", endpointElement)
-                                 .map(value -> {
-                                     if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("zone"))
-                                         illegal(msgPrefix + "only endpoints of type 'zone' can specify 'enabled'");
-
-                                     return switch (value) {
-                                         case "true" -> true;
-                                         case "false" -> false;
-                                         default -> throw new IllegalArgumentException(msgPrefix + "invalid 'enabled' value; must be 'true' or 'false'");
-                                     };
-                                 }).orElse(true);
-
-            List<AllowedUrn> allowedUrns = new ArrayList<>();
-            for (var allow : XML.getChildren(endpointElement, "allow")) {
-                if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("private"))
-                    illegal(msgPrefix + "only endpoints of type 'private' can specify 'allow' children");
-
-                switch (requireStringAttribute("with", allow)) {
-                    case "aws-private-link" -> allowedUrns.add(new AllowedUrn(AccessType.awsPrivateLink, requireStringAttribute("arn", allow)));
-                    case "gcp-service-connect" -> allowedUrns.add(new AllowedUrn(AccessType.gcpServiceConnect, requireStringAttribute("project", allow)));
-                    default -> illegal("Private endpoint for container-id '" + containerId + "': " +
-                                       "invalid attribute 'with': '" + requireStringAttribute("with", allow) + "'");
-                }
-            }
+        for (Element endpointElement : XML.getChildren(endpointsElement, endpointTag).stream() // Read zone settings first.
+                .sorted(comparingInt(endpoint -> getZoneEndpointType(endpoint, level).isPresent() ? 0 : 1)).toList()) {
+            Optional<Endpoint> endpoint = readEndpoint(parent, endpointElement, level, instance, steps, List.of(), endpointsByZone);
+            endpoint.ifPresent(e -> endpoints.add(e));
+        }
+        validateAndConsolidate(endpointsByZone, zoneEndpoints);
+    }
 
-            List<Endpoint.Target> targets = new ArrayList<>();
-            if (level == Endpoint.Level.application) {
-                Optional<String> endpointRegion = stringAttribute("region", endpointElement);
-                int weightSum = 0;
-                for (var instanceElement : XML.getChildren(endpointElement, "instance")) {
-                    String instanceName = instanceElement.getTextContent();
-                    if (instanceName == null || instanceName.isBlank()) illegal(msgPrefix + "empty 'instance' element");
-                    Optional<String> instanceRegion = stringAttribute("region", instanceElement);
-                    if (endpointRegion.isPresent() == instanceRegion.isPresent())
-                        illegal(msgPrefix + "'region' attribute must be declared on either <endpoint> or <instance> tag");
-                    String weightFromAttribute = requireStringAttribute("weight", instanceElement);
-                    int weight;
-                    try {
-                        weight = Integer.parseInt(weightFromAttribute);
-                    } catch (NumberFormatException e) {
-                        throw new IllegalArgumentException(msgPrefix + "invalid weight value '" + weightFromAttribute + "'");
-                    }
-                    weightSum += weight;
-                    targets.add(new Endpoint.Target(RegionName.from(endpointRegion.orElseGet(instanceRegion::get)),
-                                                    InstanceName.from(instanceName),
-                                                    weight));
-                }
-                if (weightSum == 0) illegal(msgPrefix + "sum of all weights must be positive, got " + weightSum);
-            } else {
-                if (stringAttribute("region", endpointElement).isPresent()) illegal(msgPrefix + "invalid 'region' attribute");
-                Set<RegionName> regions = new LinkedHashSet<>();
-                for (var regionElement : XML.getChildren(endpointElement, "region")) {
-                    String region = regionElement.getTextContent();
-                    if (region == null || region.isBlank())
-                        illegal(msgPrefix + "empty 'region' element");
-                    if (   zoneEndpointType.isEmpty()
-                        && Stream.of(RegionName.from(region), null)
-                                 .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get)
-                                 .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream())
-                                 .anyMatch(endpoint -> ! endpoint.isPublicEndpoint()))
-                        illegal(msgPrefix + "targets zone endpoint in '" + region + "' with 'enabled' set to 'false'");
-                    if ( ! regions.add(RegionName.from(region)))
-                        illegal(msgPrefix + "duplicate 'region' element: '" + region + "'");
-                }
+    /**
+     * @param parentElement
+     * @param endpointElement
+     * @param level decide what this method is reading TODO: Split into different methods instead
+     * @param instance the instance this applies to, or empty if it does not apply to an instance (application endpoints)
+     * @param steps
+     * @param forRegions the regions this applies to (for bcp), or empty (otherwise) to read this from "region" subelements
+     * @param endpointsByZone a map containing any zone endpoints read by this
+     * @return the endpoint read, unless it is added to endspointsByZone instead *sob*
+     */
+    static Optional<Endpoint> readEndpoint(Element parentElement,
+                                           Element endpointElement,
+                                           Endpoint.Level level,
+                                           Optional<String> instance,
+                                           List<Step> steps,
+                                           Collection<RegionName> forRegions,
+                                           Map<String, Map<RegionName, List<ZoneEndpoint>>> endpointsByZone) {
+        String containerId = requireStringAttribute("container-id", endpointElement);
+        Optional<String> endpointId = stringAttribute("id", endpointElement);
+        Optional<String> zoneEndpointType = getZoneEndpointType(endpointElement, level);
+        String msgPrefix = (level == Endpoint.Level.application ? "Application-level" : "Instance-level") +
+                           " endpoint '" + endpointId.orElse(Endpoint.DEFAULT_ID) + "': ";
+
+        if (zoneEndpointType.isPresent() && endpointId.isPresent())
+            illegal(msgPrefix + "cannot declare 'id' with type 'zone' or 'private'");
+
+        String invalidChild = level == Endpoint.Level.application ? "region" : "instance";
+        if ( ! XML.getChildren(endpointElement, invalidChild).isEmpty())
+            illegal(msgPrefix + "invalid element '" + invalidChild + "'");
+
+        boolean enabled = XML.attribute("enabled", endpointElement)
+                             .map(value -> {
+                                 if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("zone"))
+                                     illegal(msgPrefix + "only endpoints of type 'zone' can specify 'enabled'");
+
+                                 return switch (value) {
+                                     case "true" -> true;
+                                     case "false" -> false;
+                                     default -> throw new IllegalArgumentException(msgPrefix + "invalid 'enabled' value; must be 'true' or 'false'");
+                                 };
+                             }).orElse(true);
+
+        List<AllowedUrn> allowedUrns = new ArrayList<>();
+        for (var allow : XML.getChildren(endpointElement, "allow")) {
+            if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("private"))
+                illegal(msgPrefix + "only endpoints of type 'private' can specify 'allow' children");
+
+            switch (requireStringAttribute("with", allow)) {
+                case "aws-private-link" -> allowedUrns.add(new AllowedUrn(AccessType.awsPrivateLink, requireStringAttribute("arn", allow)));
+                case "gcp-service-connect" -> allowedUrns.add(new AllowedUrn(AccessType.gcpServiceConnect, requireStringAttribute("project", allow)));
+                default -> illegal("Private endpoint for container-id '" + containerId + "': " +
+                                   "invalid attribute 'with': '" + requireStringAttribute("with", allow) + "'");
+            }
+        }
 
-                if (zoneEndpointType.isPresent()) {
-                    if (regions.isEmpty()) regions.add(null);
-                    ZoneEndpoint endpoint = switch (zoneEndpointType.get()) {
-                        case "zone" -> new ZoneEndpoint(enabled, false, List.of());
-                        case "private" -> new ZoneEndpoint(true, true, allowedUrns); // Doesn't turn off public visibility.
-                        default -> throw new IllegalArgumentException("unsupported zone endpoint type '" + zoneEndpointType.get() + "'");
-                    };
-                    for (RegionName region : regions) endpointsByZone.computeIfAbsent(containerId, __ -> new LinkedHashMap<>())
-                                                                     .computeIfAbsent(region, __ -> new ArrayList<>())
-                                                                     .add(endpoint);
-                }
-                else {
-                    if (regions.isEmpty()) {
-                        // No explicit targets given for instance level endpoint. Include all declared, enabled regions by default
-                        List<RegionName> declared =
-                                steps.stream()
-                                     .filter(step -> step.concerns(Environment.prod))
-                                     .flatMap(step -> step.zones().stream())
-                                     .flatMap(zone -> zone.region().stream())
-                                     .toList();
-                        if (declared.isEmpty()) illegal(msgPrefix + "no declared regions to target");
-
-                        declared.stream().filter(region -> Stream.of(region, null)
-                                                                 .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get)
-                                                                 .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream())
-                                                                 .allMatch(ZoneEndpoint::isPublicEndpoint))
-                                .forEach(regions::add);
-                    }
-                    if (regions.isEmpty()) illegal(msgPrefix + "all eligible zone endpoints have 'enabled' set to 'false'");
-                    InstanceName instanceName = instance.map(InstanceName::from).get();
-                    for (RegionName region : regions) targets.add(new Target(region, instanceName, 1));
+        List<Endpoint.Target> targets = new ArrayList<>();
+        if (level == Endpoint.Level.application) {
+            if ( ! forRegions.isEmpty()) throw new IllegalStateException("Illegal combination");
+            Optional<String> endpointRegion = stringAttribute("region", endpointElement);
+            int weightSum = 0;
+            for (var instanceElement : XML.getChildren(endpointElement, "instance")) {
+                String instanceName = instanceElement.getTextContent();
+                if (instanceName == null || instanceName.isBlank()) illegal(msgPrefix + "empty 'instance' element");
+                Optional<String> instanceRegion = stringAttribute("region", instanceElement);
+                if (endpointRegion.isPresent() == instanceRegion.isPresent())
+                    illegal(msgPrefix + "'region' attribute must be declared on either <endpoint> or <instance> tag");
+                String weightFromAttribute = requireStringAttribute("weight", instanceElement);
+                int weight;
+                try {
+                    weight = Integer.parseInt(weightFromAttribute);
+                } catch (NumberFormatException e) {
+                    throw new IllegalArgumentException(msgPrefix + "invalid weight value '" + weightFromAttribute + "'");
                 }
+                weightSum += weight;
+                targets.add(new Endpoint.Target(RegionName.from(endpointRegion.orElseGet(instanceRegion::get)),
+                                                InstanceName.from(instanceName),
+                                                weight));
+            }
+            if (weightSum == 0) illegal(msgPrefix + "sum of all weights must be positive, got " + weightSum);
+        } else {
+            if (stringAttribute("region", endpointElement).isPresent()) illegal(msgPrefix + "invalid 'region' attribute");
+
+            Set<RegionName> regions = new LinkedHashSet<>(forRegions);
+            List<Element> regionElements = XML.getChildren(endpointElement, "region");
+            if ( ! regions.isEmpty() && ! regionElements.isEmpty())
+                illegal("Endpoints in <" + parentElement.getTagName() + "> cannot contain <region> children");
+            for (var regionElement : XML.getChildren(endpointElement, "region")) {
+                String region = regionElement.getTextContent();
+                if (region == null || region.isBlank())
+                    illegal(msgPrefix + "empty 'region' element");
+                if (   zoneEndpointType.isEmpty()
+                       && Stream.of(RegionName.from(region), null)
+                                .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get)
+                                .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream())
+                                .anyMatch(endpoint -> ! endpoint.isPublicEndpoint()))
+                    illegal(msgPrefix + "targets zone endpoint in '" + region + "' with 'enabled' set to 'false'");
+                if ( ! regions.add(RegionName.from(region)))
+                    illegal(msgPrefix + "duplicate 'region' element: '" + region + "'");
             }
 
-            if (zoneEndpointType.isEmpty()) {
-                Endpoint endpoint = new Endpoint(endpointId.orElse(Endpoint.DEFAULT_ID), containerId, level, targets);
-                if (endpointsById.containsKey(endpoint.endpointId())) {
-                    illegal("Endpoint ID '" + endpoint.endpointId() + "' is specified multiple times");
+            if (zoneEndpointType.isPresent()) {
+                if (regions.isEmpty()) regions.add(null);
+                ZoneEndpoint endpoint = switch (zoneEndpointType.get()) {
+                    case "zone" -> new ZoneEndpoint(enabled, false, List.of());
+                    case "private" -> new ZoneEndpoint(true, true, allowedUrns); // Doesn't turn off public visibility.
+                    default -> throw new IllegalArgumentException("unsupported zone endpoint type '" + zoneEndpointType.get() + "'");
+                };
+                for (RegionName region : regions) endpointsByZone.computeIfAbsent(containerId, __ -> new LinkedHashMap<>())
+                                                                 .computeIfAbsent(region, __ -> new ArrayList<>())
+                                                                 .add(endpoint);
+            }
+            else {
+                if (regions.isEmpty()) {
+                    // No explicit targets given for instance level endpoint. Include all declared, enabled regions by default
+                    List<RegionName> declared =
+                            steps.stream()
+                                 .filter(step -> step.concerns(Environment.prod))
+                                 .flatMap(step -> step.zones().stream())
+                                 .flatMap(zone -> zone.region().stream())
+                                 .toList();
+                    if (declared.isEmpty()) illegal(msgPrefix + "no declared regions to target");
+
+                    declared.stream().filter(region -> Stream.of(region, null)
+                                                             .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get)
+                                                             .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream())
+                                                             .allMatch(ZoneEndpoint::isPublicEndpoint))
+                            .forEach(regions::add);
                 }
-                endpointsById.put(endpoint.endpointId(), endpoint);
+                if (regions.isEmpty()) illegal(msgPrefix + "all eligible zone endpoints have 'enabled' set to 'false'");
+                InstanceName instanceName = instance.map(InstanceName::from).get();
+                for (RegionName region : regions) targets.add(new Target(region, instanceName, 1));
             }
-        });
-        endpoints.addAll(endpointsById.values());
-        validateAndConsolidate(endpointsByZone, zoneEndpoints);
+        }
+
+        if (zoneEndpointType.isEmpty())
+            return Optional.of(new Endpoint(endpointId.orElse(Endpoint.DEFAULT_ID), containerId, level, targets));
+        return Optional.empty();
     }
 
-    static Bcp readBcp(Element element) {
-        Element bcpElement = XML.getChild(element, "bcp");
+    static Bcp readBcp(Element parent, Optional<String> instance, List<Step> steps,
+                       List<Endpoint> endpoints, Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> zoneEndpoints) {
+        Element bcpElement = XML.getChild(parent, "bcp");
         if (bcpElement == null) return Bcp.empty();
 
         List<Bcp.Group> groups = new ArrayList<>();
+        Map<String, Map<RegionName, List<ZoneEndpoint>>> endpointsByZone = new LinkedHashMap<>();
         for (Element groupElement : XML.getChildren(bcpElement, "group")) {
             List<Bcp.RegionMember> regions = new ArrayList<>();
             for (Element regionElement : XML.getChildren(groupElement, "region")) {
@@ -470,9 +506,22 @@ public class DeploymentSpecXmlReader {
                 double fraction = toDouble(XML.attribute("fraction", regionElement).orElse(null), "fraction").orElse(1.0);
                 regions.add(new Bcp.RegionMember(region, fraction));
             }
+            for (Element endpointElement : XML.getChildren(groupElement, "endpoint")) {
+                if (instance.isEmpty()) illegal("The default <bcp> element at the root cannot define endpoints");
+                Optional<Endpoint> endpoint = readEndpoint(groupElement,
+                                                           endpointElement,
+                                                           Endpoint.Level.instance,
+                                                           instance,
+                                                           steps,
+                                                           regions.stream().map(r -> r.region()).toList(),
+                                                           endpointsByZone);
+                endpoint.ifPresent(e -> endpoints.add(e));
+            }
+
             Duration deadline = XML.attribute("deadline", groupElement).map(value -> toDuration(value, "deadline")).orElse(Duration.ZERO);
-            groups.add(new Bcp.Group(regions, deadline));
+            groups.add(new Bcp.Group(regions, endpoints, deadline));
         }
+        validateAndConsolidate(endpointsByZone, zoneEndpoints);
         return new Bcp(groups);
     }
 
diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
index afa7e3e502b..89b7318739e 100644
--- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
+++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
@@ -1294,22 +1294,22 @@ public class DeploymentSpecTest {
     @Test
     public void invalidEndpoints() {
         assertInvalidEndpoints("<endpoint id='FOO' container-id='qrs'/>",
-                               "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'FOO'");
+                               "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'FOO'");
         assertInvalidEndpoints("<endpoint id='123' container-id='qrs'/>",
-                               "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got '123'");
+                               "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got '123'");
         assertInvalidEndpoints("<endpoint id='foo!' container-id='qrs'/>",
-                               "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo!'");
+                               "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo!'");
         assertInvalidEndpoints("<endpoint id='foo.bar' container-id='qrs'/>",
-                               "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo.bar'");
+                               "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo.bar'");
         assertInvalidEndpoints("<endpoint id='foo--bar' container-id='qrs'/>",
-                               "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo--bar'");
+                               "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo--bar'");
         assertInvalidEndpoints("<endpoint id='foo-' container-id='qrs'/>",
-                               "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo-'");
+                               "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo-'");
         assertInvalidEndpoints("<endpoint id='foooooooooooo' container-id='qrs'/>",
-                               "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foooooooooooo'");
+                               "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foooooooooooo'");
 
         assertInvalidEndpoints("<endpoint id='foo' container-id='qrs'/><endpoint id='foo' container-id='qrs'/>",
-                               "Endpoint ID 'foo' is specified multiple times");
+                               "Endpoint id 'foo' is specified multiple times");
         assertInvalidEndpoints("<endpoint id='default' type='zone' container-id='foo' />",
                                "Instance-level endpoint 'default': cannot declare 'id' with type 'zone' or 'private'");
         assertInvalidEndpoints("<endpoint id='default' type='private' container-id='foo' />",
diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithBcpTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithBcpTest.java
index 77aadc88be8..a78f53e2084 100644
--- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithBcpTest.java
+++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithBcpTest.java
@@ -6,6 +6,7 @@ import org.junit.Test;
 
 import java.io.StringReader;
 import java.time.Duration;
+import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
@@ -100,7 +101,9 @@ public class DeploymentSpecWithBcpTest {
                    </bcp>
                 </deployment>
             """);
+
         var spec = DeploymentSpec.fromXml(r);
+
         var betaBcp = spec.requireInstance("beta").bcp().orElse(spec.bcp());
         assertEquals(1, betaBcp.groups().size());
         var betaGroup = betaBcp.groups().get(0);
@@ -241,6 +244,164 @@ public class DeploymentSpecWithBcpTest {
         }
     }
 
+    @Test
+    public void endpointsDefinedInBcp() {
+        StringReader r = new StringReader("""
+                <deployment version='1.0'>
+                   <instance id='beta'>
+                      <prod>
+                         <region>us-east1</region>
+                         <region>us-east2</region>
+                      </prod>
+                      <bcp>
+                         <group>
+                            <endpoint id="foo" container-id="bar"/>
+                            <region>us-east1</region>
+                            <region>us-east2</region>
+                         </group>
+                      </bcp>
+                   </instance>
+                   <instance id='main'>
+                      <prod>
+                         <region>us-east1</region>
+                         <region>us-east2</region>
+                         <region>us-central1</region>
+                         <region>us-west1</region>
+                         <region>us-west2</region>
+                      </prod>
+                     <bcp>
+                       <group>
+                          <endpoint id="east" container-id="bar"/>
+                          <region>us-east1</region>
+                          <region>us-east2</region>
+                          <region fraction="0.3">us-central1</region>
+                       </group>
+                       <group>
+                          <endpoint id="west" container-id="bar"/>
+                          <region>us-west1</region>
+                          <region>us-west2</region>
+                          <region fraction="0.7">us-central1</region>
+                       </group>
+                     </bcp>
+                   </instance>
+                </deployment>
+            """);
+
+        var spec = DeploymentSpec.fromXml(r);
+
+        var betaEndpoints = spec.requireInstance("beta").endpoints();
+        assertEquals(1, betaEndpoints.size());
+        assertEquals("foo", betaEndpoints.get(0).endpointId());
+        assertEquals("bar", betaEndpoints.get(0).containerId());
+        assertEquals(List.of(RegionName.from("us-east1"), RegionName.from("us-east2")),
+                     betaEndpoints.get(0).regions());
+
+        var mainEndpoints = spec.requireInstance("main").endpoints();
+        assertEquals(2, mainEndpoints.size());
+        assertEquals("east", mainEndpoints.get(0).endpointId());
+        assertEquals(List.of(RegionName.from("us-east1"), RegionName.from("us-east2"), RegionName.from("us-central1")),
+                     mainEndpoints.get(0).regions());
+        assertEquals("west", mainEndpoints.get(1).endpointId());
+        assertEquals(List.of(RegionName.from("us-west1"), RegionName.from("us-west2"), RegionName.from("us-central1")),
+                     mainEndpoints.get(1).regions());
+    }
+
+    @Test
+    public void endpointsDefinedInBcpImplicitInstance() {
+        StringReader r = new StringReader("""
+                <deployment version='1.0'>
+                   <prod>
+                      <region>us-east1</region>
+                      <region>us-east2</region>
+                      <region>us-central1</region>
+                      <region>us-west1</region>
+                      <region>us-west2</region>
+                   </prod>
+                  <bcp>
+                    <group>
+                       <endpoint id="east" container-id="bar"/>
+                       <region>us-east1</region>
+                       <region>us-east2</region>
+                       <region fraction="0.3">us-central1</region>
+                    </group>
+                    <group>
+                       <endpoint id="west" container-id="bar"/>
+                       <region>us-west1</region>
+                       <region>us-west2</region>
+                       <region fraction="0.7">us-central1</region>
+                    </group>
+                  </bcp>
+                </deployment>
+            """);
+
+        var spec = DeploymentSpec.fromXml(r);
+
+        var mainEndpoints = spec.requireInstance("default").endpoints();
+        assertEquals(2, mainEndpoints.size());
+        assertEquals("east", mainEndpoints.get(0).endpointId());
+        assertEquals(List.of(RegionName.from("us-east1"), RegionName.from("us-east2"), RegionName.from("us-central1")),
+                     mainEndpoints.get(0).regions());
+        assertEquals("west", mainEndpoints.get(1).endpointId());
+        assertEquals(List.of(RegionName.from("us-west1"), RegionName.from("us-west2"), RegionName.from("us-central1")),
+                     mainEndpoints.get(1).regions());
+    }
+
+    @Test
+    public void endpointsDefinedInBcpValidation1() {
+        StringReader r = new StringReader("""
+                <deployment version='1.0'>
+                   <instance id='beta'>
+                      <prod>
+                         <region>us-east1</region>
+                         <region>us-east2</region>
+                      </prod>
+                   </instance>
+                   <bcp>
+                      <group>
+                         <endpoint id="foo" container-id="bar"/>
+                         <region>us-east1</region>
+                         <region>us-east2</region>
+                      </group>
+                   </bcp>
+                </deployment>
+            """);
+        try {
+            DeploymentSpec.fromXml(r);
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("The default <bcp> element at the root cannot define endpoints", Exceptions.toMessageString(e));
+        }
+    }
+
+    @Test
+    public void endpointsDefinedInBcpValidation2() {
+        StringReader r = new StringReader("""
+                <deployment version='1.0'>
+                   <instance id='beta'>
+                      <prod>
+                         <region>us-east1</region>
+                         <region>us-east2</region>
+                      </prod>
+                      <bcp>
+                         <group>
+                            <region>us-east1</region>
+                            <region>us-east2</region>
+                            <endpoint id="foo" container-id="bar">
+                               <region>us-east1</region>
+                            </endpoint>
+                         </group>
+                      </bcp>
+                   </instance>
+                </deployment>
+            """);
+        try {
+            DeploymentSpec.fromXml(r);
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("Endpoints in <group> cannot contain <region> children", Exceptions.toMessageString(e));
+        }
+
+    }
     private void assertTwoRegions(DeploymentSpec spec) {
         var bcp = spec.requireInstance("default").bcp().orElse(spec.bcp());
         assertEquals(1, bcp.groups().size());
diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc
index d63b8885a57..cc9db471120 100644
--- a/config-model/src/main/resources/schema/deployment.rnc
+++ b/config-model/src/main/resources/schema/deployment.rnc
@@ -22,6 +22,7 @@ StepExceptInstance =
     BlockChange* &
     Notifications? &
     Endpoints? &
+    Bcp? &
     Test? &
     Staging? &
     Dev? &
@@ -170,3 +171,18 @@ Endpoint = element endpoint {
 Endpoints = element endpoints {
     Endpoint+
 }
+
+Bcp = element bcp {
+    Group+
+}
+
+Group = element group {
+    attribute deadline { xsd:string }? &
+    Endpoint* &
+    MemberRegion+
+}
+
+MemberRegion = element region {
+    attribute fraction { xsd:double }? &
+    text
+}
diff --git a/config-model/src/test/schema-test-files/deployment-with-instances.xml b/config-model/src/test/schema-test-files/deployment-with-instances.xml
index f37ff9f6cc6..0c3409533d1 100644
--- a/config-model/src/test/schema-test-files/deployment-with-instances.xml
+++ b/config-model/src/test/schema-test-files/deployment-with-instances.xml
@@ -30,6 +30,18 @@
           </endpoint>
           <endpoint container-id="bar" />
       </endpoints>
+      <bcp>
+          <group deadline="60m">
+              <endpoint id="foo" container-id="baz"/>
+              <region>us-west-1</region>
+              <region fraction="0.5">us-central-1</region>
+          </group>
+          <group>
+              <region>us-north-1</region>
+              <region>us-south-2</region>
+              <region fraction="0.5">us-central-1</region>
+          </group>
+      </bcp>
     </instance>
 
     <delay hours='2'/>
diff --git a/config-model/src/test/schema-test-files/deployment.xml b/config-model/src/test/schema-test-files/deployment.xml
index 38145a1ac74..bc0f070d88c 100644
--- a/config-model/src/test/schema-test-files/deployment.xml
+++ b/config-model/src/test/schema-test-files/deployment.xml
@@ -26,4 +26,16 @@
         </endpoint>
         <endpoint container-id="bar" />
     </endpoints>
+    <bcp>
+        <group deadline="60m">
+            <endpoint id="foo" container-id="baz"/>
+            <region>us-west-1</region>
+            <region fraction="0.5">us-central-1</region>
+        </group>
+        <group>
+            <region>us-north-1</region>
+            <region>us-south-2</region>
+            <region fraction="0.5">us-central-1</region>
+        </group>
+    </bcp>
 </deployment>