diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2023-09-03 18:55:09 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-03 18:55:09 +0200 |
commit | d8dbd9ceb4aabf9b6bd2e548bec1f911b9a94002 (patch) | |
tree | b2db496cf7a96a1a88213e205f356e8fcd54820e | |
parent | 2d17d98b9be592444cd93d214a3049cc6c3fd7c6 (diff) | |
parent | b89b0f7c8150258ce5b177989bdd522e1c1f284e (diff) |
Merge pull request #28366 from vespa-engine/revert-28353-mortent/override-port-filtersv8.220.105
Revert "Override filter port bindings" MERGEOK
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<Ht static final String REQUEST_CHAIN_TAG_NAME = "request-chain"; static final String RESPONSE_CHAIN_TAG_NAME = "response-chain"; static final List<String> VALID_FILTER_CHAIN_TAG_NAMES = List.of(REQUEST_CHAIN_TAG_NAME, RESPONSE_CHAIN_TAG_NAME); - private final Set<Integer> portBindingOverrides; - - public HttpBuilder(Set<Integer> portBindingOverrides) { - super(); - this.portBindingOverrides = portBindingOverrides; - } @Override protected Http doBuild(DeployState deployState, TreeConfigProducer<AnyConfigProducer> ancestor, Element spec) { @@ -51,7 +44,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilderBase<Ht Element filteringElem = XML.getChild(spec, "filtering"); if (filteringElem != null) { filterChains = new FilterChainsBuilder().build(deployState, ancestor, filteringElem); - bindings = readFilterBindings(filteringElem, this.portBindingOverrides); + bindings = readFilterBindings(filteringElem); strictFiltering = XmlHelper.getOptionalAttribute(filteringElem, "strict-mode") .map(Boolean::valueOf); @@ -147,7 +140,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilderBase<Ht return Optional.of((ApplicationContainerCluster) currentProducer); } - private List<FilterBinding> readFilterBindings(Element filteringSpec, Set<Integer> portBindingOverride) { + private List<FilterBinding> readFilterBindings(Element filteringSpec) { List<FilterBinding> result = new ArrayList<>(); for (Element child: XML.getChildren(filteringSpec)) { @@ -157,14 +150,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilderBase<Ht for (Element bindingSpec: XML.getChildren(child, "binding")) { String binding = XML.getValue(bindingSpec); - if (portBindingOverride.isEmpty()) { - result.add(FilterBinding.create(toFilterBindingType(tagName), chainId, UserBindingPattern.fromPattern(binding))); - } else { - UserBindingPattern userBindingPattern = UserBindingPattern.fromPattern(binding); - portBindingOverride.stream() - .map(userBindingPattern::withOverriddenPort) - .forEach(pattern -> 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<ContainerModel> { 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<ContainerModel> { .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<ContainerModel> { } private void addUserHandlers(DeployState deployState, ApplicationContainerCluster cluster, Element spec, ConfigModelContext context) { + var portBindingOverride = isHostedTenantApplication(context) ? getDataplanePorts(deployState) : Set.<Integer>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<ContainerModel> { 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<Integer> portBindingOverride(DeployState deployState, ConfigModelContext context) { - return isHostedTenantApplication(context) + var portBindingOverride = isHostedTenantApplication(context) ? getDataplanePorts(deployState) : Set.<Integer>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>", - " <filtering>", - " <request-chain id='my-request-chain'>", - " <binding>http://*/my-binding</binding>", - " </request-chain>", - " </filtering>", - "</http>"); - 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<String> 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<String> actualCustomChainBindings = getFilterBindings(http, ComponentId.fromString("my-custom-response-chain")); - assertTrue(actualCustomChainBindings.contains("http://*:4443/custom-handler/*")); + assertTrue(actualCustomChainBindings.contains("http://*/custom-handler/*")); } @Test |