From b89b0f7c8150258ce5b177989bdd522e1c1f284e Mon Sep 17 00:00:00 2001 From: Morten Tokle Date: Sun, 3 Sep 2023 18:53:56 +0200 Subject: Revert "Override filter port bindings" --- .../vespa/model/container/http/xml/HttpBuilder.java | 20 +++----------------- .../model/container/xml/ContainerModelBuilder.java | 18 ++++++++---------- .../model/container/http/FilterBindingsTest.java | 21 +-------------------- .../model/container/http/FilterChainsTest.java | 4 +--- .../model/container/http/FilterConfigTest.java | 4 +--- .../model/container/xml/AccessControlTest.java | 4 ++-- 6 files changed, 16 insertions(+), 55 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java index d276bf3b850..ae13bed4bb4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java @@ -22,7 +22,6 @@ import org.w3c.dom.Element; import java.util.ArrayList; import java.util.List; import java.util.Optional; -import java.util.Set; import java.util.logging.Level; /** @@ -34,12 +33,6 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilderBase VALID_FILTER_CHAIN_TAG_NAMES = List.of(REQUEST_CHAIN_TAG_NAME, RESPONSE_CHAIN_TAG_NAME); - private final Set portBindingOverrides; - - public HttpBuilder(Set portBindingOverrides) { - super(); - this.portBindingOverrides = portBindingOverrides; - } @Override protected Http doBuild(DeployState deployState, TreeConfigProducer ancestor, Element spec) { @@ -51,7 +44,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilderBase readFilterBindings(Element filteringSpec, Set portBindingOverride) { + private List readFilterBindings(Element filteringSpec) { List result = new ArrayList<>(); for (Element child: XML.getChildren(filteringSpec)) { @@ -157,14 +150,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilderBase result.add(FilterBinding.create(toFilterBindingType(tagName), chainId, pattern))); - } + result.add(FilterBinding.create(toFilterBindingType(tagName), chainId, UserBindingPattern.fromPattern(binding))); } } } 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 459c54a2805..b603f9f0ba1 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 @@ -444,7 +444,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { protected void addHttp(DeployState deployState, Element spec, ApplicationContainerCluster cluster, ConfigModelContext context) { Element httpElement = XML.getChild(spec, "http"); if (httpElement != null) { - cluster.setHttp(buildHttp(deployState, cluster, httpElement, context)); + cluster.setHttp(buildHttp(deployState, cluster, httpElement)); } if (isHostedTenantApplication(context)) { addHostedImplicitHttpIfNotPresent(deployState, cluster); @@ -707,8 +707,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder { .configureHttpFilterChains(http); } - private Http buildHttp(DeployState deployState, ApplicationContainerCluster cluster, Element httpElement, ConfigModelContext context) { - Http http = new HttpBuilder(portBindingOverride(deployState, context)).build(deployState, cluster, httpElement); + private Http buildHttp(DeployState deployState, ApplicationContainerCluster cluster, Element httpElement) { + Http http = new HttpBuilder().build(deployState, cluster, httpElement); if (networking == Networking.disable) http.removeAllServers(); @@ -836,9 +836,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder { } private void addUserHandlers(DeployState deployState, ApplicationContainerCluster cluster, Element spec, ConfigModelContext context) { + var portBindingOverride = isHostedTenantApplication(context) ? getDataplanePorts(deployState) : Set.of(); for (Element component: XML.getChildren(spec, "handler")) { cluster.addComponent( - new DomHandlerBuilder(cluster, portBindingOverride(deployState, context)).build(deployState, cluster, component)); + new DomHandlerBuilder(cluster, portBindingOverride).build(deployState, cluster, component)); } } @@ -1171,14 +1172,11 @@ public class ContainerModelBuilder extends ConfigModelBuilder { ContainerDocumentApi.HandlerOptions documentApiOptions = DocumentApiOptionsBuilder.build(documentApiElement); Element ignoreUndefinedFields = XML.getChild(documentApiElement, "ignore-undefined-fields"); - return new ContainerDocumentApi(cluster, documentApiOptions, - "true".equals(XML.getValue(ignoreUndefinedFields)), portBindingOverride(deployState, context)); - } - - private Set portBindingOverride(DeployState deployState, ConfigModelContext context) { - return isHostedTenantApplication(context) + var portBindingOverride = isHostedTenantApplication(context) ? getDataplanePorts(deployState) : Set.of(); + return new ContainerDocumentApi(cluster, documentApiOptions, + "true".equals(XML.getValue(ignoreUndefinedFields)), portBindingOverride); } private ContainerDocproc buildDocproc(DeployState deployState, ApplicationContainerCluster cluster, Element spec) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterBindingsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterBindingsTest.java index 70a859af010..787a8255628 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterBindingsTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterBindingsTest.java @@ -14,8 +14,6 @@ import com.yahoo.vespa.model.container.xml.ContainerModelBuilder.Networking; import org.junit.jupiter.api.Test; import org.w3c.dom.Element; -import java.util.Set; - import static com.yahoo.collections.CollectionUtil.first; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -28,7 +26,7 @@ public class FilterBindingsTest extends DomBuilderTest { private static final BindingPattern MY_CHAIN_BINDING = UserBindingPattern.fromHttpPath("/my-chain-binding"); private Http buildHttp(Element xml) { - Http http = new HttpBuilder(Set.of()).build(root.getDeployState(), root, xml); + Http http = new HttpBuilder().build(root.getDeployState(), root, xml); root.freezeModelTopology(); http.validate(); return http; @@ -110,21 +108,4 @@ public class FilterBindingsTest extends DomBuilderTest { } } - @Test - void filter_binding_ports_are_overriden() { - Element xml = parse( - "", - " ", - " ", - " http://*/my-binding", - " ", - " ", - ""); - Http http = new HttpBuilder(Set.of(4443)).build(root.getDeployState(), root, xml); - root.freezeModelTopology(); - http.validate(); - FilterBinding binding = first(http.getBindings()); - assertEquals("my-request-chain", binding.chainId().getName()); - assertEquals("http://*:4443/my-binding", binding.binding().patternString()); - } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterChainsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterChainsTest.java index 1c60205039f..990896acb01 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterChainsTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterChainsTest.java @@ -9,8 +9,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.w3c.dom.Element; -import java.util.Set; - import static com.yahoo.collections.CollectionUtil.first; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -25,7 +23,7 @@ public class FilterChainsTest extends DomBuilderTest { @BeforeEach public void setupFilterChains() { - http = new HttpBuilder(Set.of()).build(root.getDeployState(), root, servicesXml()); + http = new HttpBuilder().build(root.getDeployState(), root, servicesXml()); root.freezeModelTopology(); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterConfigTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterConfigTest.java index a1f9661de14..76a3dcb2788 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterConfigTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterConfigTest.java @@ -8,8 +8,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.w3c.dom.Element; -import java.util.Set; - import static com.yahoo.collections.CollectionUtil.first; import static com.yahoo.vespa.model.container.http.FilterConfigProvider.configProviderId; import static org.junit.jupiter.api.Assertions.*; @@ -24,7 +22,7 @@ public class FilterConfigTest extends DomBuilderTest { @BeforeEach public void setupFilterChains() { - http = new HttpBuilder(Set.of()).build(root.getDeployState(), root, servicesXml()); + http = new HttpBuilder().build(root.getDeployState(), root, servicesXml()); root.freezeModelTopology(); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java index 740986bb000..697d2d422e8 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java @@ -225,7 +225,7 @@ public class AccessControlTest extends ContainerModelBuilderTestBase { "http://*:4443/metrics/v2/*"))); Set actualCustomChainBindings = getFilterBindings(http, ComponentId.fromString("my-custom-request-chain")); - assertTrue(actualCustomChainBindings.containsAll(List.of("http://*:4443/custom-handler/*", "http://*:4443/"))); + assertTrue(actualCustomChainBindings.containsAll(List.of("http://*/custom-handler/*", "http://*/"))); } @Test @@ -262,7 +262,7 @@ public class AccessControlTest extends ContainerModelBuilderTestBase { "http://*:4443/custom-handler/*"))); Set actualCustomChainBindings = getFilterBindings(http, ComponentId.fromString("my-custom-response-chain")); - assertTrue(actualCustomChainBindings.contains("http://*:4443/custom-handler/*")); + assertTrue(actualCustomChainBindings.contains("http://*/custom-handler/*")); } @Test -- cgit v1.2.3