diff options
author | jonmv <venstad@gmail.com> | 2023-01-18 18:59:51 +0100 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2023-01-18 18:59:51 +0100 |
commit | 669ae1bc6572c198e609bc20cacbdc592e8e2731 (patch) | |
tree | 0ecf6e5d73433301117b6ecebdd241eec472460a /config-model | |
parent | c47ed544a31a6b56f518901247212a47d8eb9d31 (diff) |
Revert "Merge pull request #25624 from vespa-engine/revert-25617-jonmv/private-endpoints"
This reverts commit c47ed544a31a6b56f518901247212a47d8eb9d31, reversing
changes made to e0191b4d49048f9398395dc8c1c60dfcb383f705.
Diffstat (limited to 'config-model')
9 files changed, 193 insertions, 62 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java index 95d97bc9e87..756646beddb 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java +++ b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java @@ -80,7 +80,7 @@ public class ConfigModelRepo implements ConfigModelRepoAdder, Serializable, Iter ConfigModelRegistry configModelRegistry) throws IOException { Element userServicesElement = getServicesFromApp(deployState.getApplicationPackage()); readConfigModels(root, userServicesElement, deployState, vespaModel, configModelRegistry); - builder.postProc(deployState.getDeployLogger(), root, this); + builder.postProc(deployState, root, this); } private Element getServicesFromApp(ApplicationPackage applicationPackage) throws IOException { diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 7cb0672699f..49194a5d1bb 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -37,7 +37,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private ApplicationId applicationId = ApplicationId.defaultId(); private List<ConfigServerSpec> configServerSpecs = Collections.emptyList(); private boolean hostedVespa = false; - private Zone zone; + private Zone zone = Zone.defaultZone(); private final Set<ContainerEndpoint> endpoints = Collections.emptySet(); private boolean useDedicatedNodeForLogserver = false; private double defaultTermwiseLimit = 1.0; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java index 2cf32f1e8ff..421e3a2902c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java @@ -22,6 +22,6 @@ public abstract class VespaModelBuilder { * @param producerRoot the root producer. * @param configModelRepo a {@link com.yahoo.config.model.ConfigModelRepo instance} */ - public abstract void postProc(DeployLogger deployLogger, AbstractConfigProducer producerRoot, ConfigModelRepo configModelRepo); + public abstract void postProc(DeployState deployState, AbstractConfigProducer producerRoot, ConfigModelRepo configModelRepo); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java index a31e3fcce71..aac968f9038 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.model.builder.xml.dom; import com.yahoo.collections.Pair; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.provision.ZoneEndpoint; import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.CloudAccount; @@ -11,7 +12,6 @@ import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NodeResources; import com.yahoo.text.XML; import com.yahoo.vespa.model.HostResource; @@ -256,13 +256,13 @@ public class NodesSpecification { ClusterSpec.Id clusterId, DeployLogger logger, boolean stateful) { - return provision(hostSystem, clusterType, clusterId, LoadBalancerSettings.empty, logger, stateful); + return provision(hostSystem, clusterType, clusterId, ZoneEndpoint.defaultEndpoint, logger, stateful); } public Map<HostResource, ClusterMembership> provision(HostSystem hostSystem, ClusterSpec.Type clusterType, ClusterSpec.Id clusterId, - LoadBalancerSettings loadBalancerSettings, + ZoneEndpoint zoneEndpoint, DeployLogger logger, boolean stateful) { if (combinedId.isPresent()) @@ -272,7 +272,7 @@ public class NodesSpecification { .exclusive(exclusive) .combinedId(combinedId.map(ClusterSpec.Id::from)) .dockerImageRepository(dockerImageRepo) - .loadBalancerSettings(loadBalancerSettings) + .loadBalancerSettings(zoneEndpoint) .stateful(stateful) .build(); return hostSystem.allocateHosts(cluster, Capacity.from(min, max, required, canFail, cloudAccount), logger); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java index cb3c43074fc..e4e56dcaaca 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.builder.xml.dom; +import ai.vespa.validation.Validation; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.ApplicationConfigProducerRoot; import com.yahoo.config.model.ConfigModelRepo; @@ -22,8 +23,15 @@ import com.yahoo.vespa.model.container.docproc.ContainerDocproc; import com.yahoo.vespa.model.content.Content; import com.yahoo.vespa.model.search.SearchCluster; import org.w3c.dom.Element; + +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Pattern; /** * Builds Vespa model components using the w3c dom api @@ -43,10 +51,12 @@ public class VespaDomBuilder extends VespaModelBuilder { public static final String VESPAMALLOC = "vespamalloc"; // Intended for vespa engineers public static final String VESPAMALLOC_DEBUG = "vespamalloc-debug"; // Intended for vespa engineers public static final String VESPAMALLOC_DEBUG_STACKTRACE = "vespamalloc-debug-stacktrace"; // Intended for vespa engineers - private static final String CPU_SOCKET_ATTRIB_NAME = "cpu-socket"; public static final String CPU_SOCKET_AFFINITY_ATTRIB_NAME = "cpu-socket-affinity"; public static final String Allocated_MEMORY_ATTRIB_NAME = "allocated-memory"; + private static final String CPU_SOCKET_ATTRIB_NAME = "cpu-socket"; + private static final Pattern clusterPattern = Pattern.compile("([a-z0-9]|[a-z0-9][a-z0-9_-]{0,61}[a-z0-9])"); + public static final Logger log = Logger.getLogger(VespaDomBuilder.class.getPackage().toString()); @@ -232,13 +242,14 @@ public class VespaDomBuilder extends VespaModelBuilder { * @param root root config producer * @param configModelRepo a {@link ConfigModelRepo} */ - public void postProc(DeployLogger deployLogger, AbstractConfigProducer root, ConfigModelRepo configModelRepo) { + public void postProc(DeployState deployState, AbstractConfigProducer root, ConfigModelRepo configModelRepo) { setContentSearchClusterIndexes(configModelRepo); createDocprocMBusServersAndClients(configModelRepo); + if (deployState.isHosted()) validateContainerClusterIds(configModelRepo); } private void createDocprocMBusServersAndClients(ConfigModelRepo pc) { - for (ContainerCluster cluster: ContainerModel.containerClusters(pc)) { + for (ContainerCluster<?> cluster: ContainerModel.containerClusters(pc)) { addServerAndClientsForChains(cluster.getDocproc()); } } @@ -248,6 +259,19 @@ public class VespaDomBuilder extends VespaModelBuilder { docproc.getChains().addServersAndClientsForChains(); } + private void validateContainerClusterIds(ConfigModelRepo pc) { + Map<String, String> normalizedClusterIds = new LinkedHashMap<>(); + for (ContainerCluster<?> cluster: ContainerModel.containerClusters(pc)) { + if (cluster.getHttp() == null) continue; + String name = cluster.getName(); + Validation.requireMatch(name, "container cluster name", clusterPattern); + String clashing = normalizedClusterIds.put(name.replaceAll("_", "-"), name); + if (clashing != null) throw new IllegalArgumentException("container clusters '" + clashing + "' and '" + name + + "' have clashing endpoint names, when '_' is replaced " + + "with '-' to form valid domain names"); + } + } + /** * For some reason, search clusters need to be enumerated. * @param configModelRepo a {@link ConfigModelRepo} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index d0a03be2869..4c7bad575d2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -11,6 +11,8 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.model.api.ApplicationClusterEndpoint; import com.yahoo.config.model.api.ConfigServerSpec; @@ -29,10 +31,11 @@ import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.LoadBalancerSettings; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.Zone; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.container.logging.FileConnectionLog; import com.yahoo.io.IOUtils; @@ -847,11 +850,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } } - private LoadBalancerSettings loadBalancerSettings(Element loadBalancerElement) { - List<String> allowedUrnElements = XML.getChildren(XML.getChild(loadBalancerElement, "private-access"), - "allow-urn") - .stream().map(XML::getValue).toList(); - return new LoadBalancerSettings(allowedUrnElements); + private ZoneEndpoint zoneEndpoint(ConfigModelContext context, ClusterSpec.Id cluster) { + InstanceName instance = context.properties().applicationId().instance(); + ZoneId zone = ZoneId.from(context.properties().zone().environment(), + context.properties().zone().region()); + DeploymentSpec spec = context.getApplicationPackage().getDeployment() + .map(new DeploymentSpecXmlReader(false)::read) + .orElse(DeploymentSpec.empty); + return spec.zoneEndpoint(instance, zone, cluster); } private static Map<String, String> getEnvironmentVariables(Element environmentVariables) { @@ -940,11 +946,12 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { private List<ApplicationContainer> createNodesFromNodeCount(ApplicationContainerCluster cluster, Element containerElement, Element nodesElement, ConfigModelContext context) { NodesSpecification nodesSpecification = NodesSpecification.from(new ModelElement(nodesElement), context); - LoadBalancerSettings loadBalancerSettings = loadBalancerSettings(XML.getChild(containerElement, "load-balancer")); + ClusterSpec.Id clusterId = ClusterSpec.Id.from(cluster.name()); + ZoneEndpoint zoneEndpoint = zoneEndpoint(context, clusterId); Map<HostResource, ClusterMembership> hosts = nodesSpecification.provision(cluster.getRoot().hostSystem(), ClusterSpec.Type.container, - ClusterSpec.Id.from(cluster.getName()), - loadBalancerSettings, + clusterId, + zoneEndpoint, log, getZooKeeper(containerElement) != null); return createNodesFromHosts(hosts, cluster, context.getDeployState()); diff --git a/config-model/src/main/resources/schema/containercluster.rnc b/config-model/src/main/resources/schema/containercluster.rnc index 933ec528c42..81455084ad2 100644 --- a/config-model/src/main/resources/schema/containercluster.rnc +++ b/config-model/src/main/resources/schema/containercluster.rnc @@ -6,8 +6,7 @@ ContainerCluster = element container { ContainerServices & DocumentBinding* & NodesOfContainerCluster? & - ClientAuthorize? & - LoadBalancer? + ClientAuthorize? } ContainerServices = @@ -312,16 +311,6 @@ NodesOfContainerCluster = element nodes { ) } -LoadBalancer = element load-balancer { - element private-access { - element allow-urn { - xsd:string - }* - }? -} - - - #DOCUMENT BINDINGS: DocumentBinding = element document { diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 444f66a92ab..d63b8885a57 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -150,12 +150,21 @@ EndpointInstance = element instance { text } +AllowedUrn = element allow { + attribute with { xsd:string } & + attribute arn { xsd:string }? & + attribute project { xsd:string }? +} + Endpoint = element endpoint { attribute id { xsd:string }? & attribute container-id { xsd:string } & attribute region { xsd:string }? & + attribute type { xsd:string }? & + attribute enabled { xsd:boolean }? & EndpointRegion* & - EndpointInstance* + EndpointInstance* & + AllowedUrn* } Endpoints = element endpoints { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index addf4dffde2..f0c39ecc920 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -22,6 +22,9 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provisioning.FlavorsConfig; import com.yahoo.container.ComponentsConfig; import com.yahoo.container.QrConfig; @@ -57,7 +60,6 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.logging.Level; -import java.util.stream.Collectors; import static com.yahoo.config.model.test.TestUtil.joinLines; import static com.yahoo.test.LinePatternMatcher.containsLineWithPattern; @@ -179,6 +181,63 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } @Test + void container_cluster_with_invalid_name_throws_exception_when_hosted() throws IOException, SAXException { + String servicesXml = """ + <services version='1.0'> + <container id='C-1' version='1.0'> + <nodes count='1' /> + </container> + </services> + """; + + assertEquals("container cluster name must match '([a-z0-9]|[a-z0-9][a-z0-9_-]{0,61}[a-z0-9])', but got: 'C-1'", + assertThrows(IllegalArgumentException.class, + () -> + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(true)) + .build())) + .getMessage()); + + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(false)) + .build()); + } + + @Test + void two_clusters_with_clashing_cluster_names_throws_exception_when_hosted() throws IOException, SAXException { + String servicesXml = """ + <services version='1.0'> + <container id='c-1' version='1.0'> + <nodes count='1' /> + </container> + <container id='c_1' version='1.0'> + <nodes count='1' /> + </container> + </services> + """; + + assertEquals("container clusters 'c-1' and 'c_1' have clashing endpoint names, when '_' is replaced with '-' to form valid domain names", + assertThrows(IllegalArgumentException.class, + () -> + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(true)) + .build())) + .getMessage()); + + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(false)) + .build()); + } + + @Test void two_clusters_without_explicit_port_throws_exception() { Element cluster1Elem = DomBuilderTest.parse( "<container id='cluster1' version='1.0'>", @@ -198,53 +257,96 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { @Test void load_balancers_can_be_set() throws IOException, SAXException { - // No load-balancer or nodes elements - verifyAllowedUrns(""); + // No endpoints + verifyAllowedUrns("", Environment.prod, "eu", ZoneEndpoint.defaultEndpoint); - // No load-balancer element - verifyAllowedUrns("<nodes count='2' />"); + // No non-default settings + verifyAllowedUrns(""" + <endpoint type='zone' container-id='default' /> + """, + Environment.prod, + "eu", + ZoneEndpoint.defaultEndpoint); - // No nodes element + // No allowed urns verifyAllowedUrns(""" - <load-balancer> - <private-access> - <allow-urn>foo</allow-urn> - <allow-urn>bar</allow-urn> - </private-access> - </load-balancer> - """); - - // Both load-balancer and nodes + <endpoint type='private' container-id='default' /> + """, + Environment.prod, + "eu", + new ZoneEndpoint(true, true, List.of())); + + // Various settings + verifyAllowedUrns(""" + <endpoint type='zone' container-id='default' enabled='false' /> + <endpoint type='private' container-id='default'> + <region>eu</region> + <allow with='aws-private-link' arn='barn' /> + <allow with='gcp-service-connect' project='nine' /> + </endpoint> + """, + Environment.prod, + "eu", + new ZoneEndpoint(false, true, List.of(new AllowedUrn(AccessType.awsPrivateLink, "barn"), + new AllowedUrn(AccessType.gcpServiceConnect, "nine")))); + + // Various settings, but wrong region verifyAllowedUrns(""" - <load-balancer> - <private-access> - <allow-urn>foo</allow-urn> - <allow-urn>bar</allow-urn> - </private-access> - </load-balancer> - <nodes count='2' /> + <endpoint type='zone' container-id='default' enabled='false' /> + <endpoint type='private' container-id='default'> + <region>eu</region> + <allow with='aws-private-link' arn='barn' /> + <allow with='gcp-service-connect' project='nine' /> + </endpoint> """, - "foo", "bar"); + Environment.prod, + "us", + ZoneEndpoint.defaultEndpoint); + + // Various settings, but wrong environment + verifyAllowedUrns(""" + <endpoint type='zone' container-id='default' enabled='false' /> + <endpoint type='private' container-id='default'> + <region>eu</region> + <allow with='aws-private-link' arn='barn' /> + <allow with='gcp-service-connect' project='nine' /> + </endpoint> + """, + Environment.dev, + "eu", + ZoneEndpoint.defaultEndpoint); } - private void verifyAllowedUrns(String containerXml, String... expectedAllowedUrns) throws IOException, SAXException { + private void verifyAllowedUrns(String endpointsTag, Environment environment, String region, ZoneEndpoint expected) throws IOException, SAXException { String servicesXml = """ <container id='default' version='1.0'> - %s + <nodes count='2' /> </container> - """.formatted(containerXml); - ApplicationPackage applicationPackage = new MockApplicationPackage.Builder().withServices(servicesXml).build(); + """; + String deploymentXml = """ + <deployment version='1.0'> + <prod> + <region>eu</region> + </prod> + <endpoints> + %s + </endpoints> + </deployment> + """.formatted(endpointsTag); + ApplicationPackage applicationPackage = new MockApplicationPackage.Builder().withServices(servicesXml).withDeploymentSpec(deploymentXml).build(); InMemoryProvisioner provisioner = new InMemoryProvisioner(true, false, "host1.yahoo.com", "host2.yahoo.com"); VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() .modelHostProvisioner(provisioner) .provisioned(provisioner.startProvisionedRecording()) .applicationPackage(applicationPackage) - .properties(new TestProperties().setMultitenant(true).setHostedVespa(true)) + .properties(new TestProperties().setMultitenant(true) + .setHostedVespa(true) + .setZone(new Zone(environment, RegionName.from(region)))) .build()); assertEquals(2, model.hostSystem().getHosts().size()); assertEquals(1, provisioner.provisionedClusters().size()); - assertEquals(List.of(expectedAllowedUrns), - provisioner.provisionedClusters().iterator().next().loadBalancerSettings().allowedUrns()); + assertEquals(expected, + provisioner.provisionedClusters().iterator().next().zoneEndpoint()); } @Test |