Merge pull request #27996 from vespa-engine/mortent/remove-restricted-dpbindings2

remove restricted dpbindings

12 files changed, 53 insertions, 62 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
index 1ab3cc30db7..1774b4f81d9 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
@@ -108,7 +108,7 @@ public interface ModelContext {
         @ModelFeatureFlag(owners = {"hmusum"}) default Architecture adminClusterArchitecture() { return Architecture.getDefault(); }
         @ModelFeatureFlag(owners = {"tokle"}) default boolean enableProxyProtocolMixedMode() { return true; }
         @ModelFeatureFlag(owners = {"arnej"}) default String logFileCompressionAlgorithm(String defVal) { return defVal; }
-        @ModelFeatureFlag(owners = {"tokle"}) default boolean useRestrictedDataPlaneBindings() { return false; }
+        @ModelFeatureFlag(owners = {"tokle"}, removeAfter = "8.210") default boolean useRestrictedDataPlaneBindings() { return true; }
         @ModelFeatureFlag(owners = {"arnej, bjorncs"}) default boolean enableGlobalPhase() { return true; }
         @ModelFeatureFlag(owners = {"baldersheim"}, comment = "Select summary decode type") default String summaryDecodePolicy() { return "eager"; }
         @ModelFeatureFlag(owners = {"hmusum"}) default boolean allowMoreThanOneContentGroupDown(ClusterSpec.Id id) { return false; }
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 b06d3572fcb..9f23c9b7231 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
@@ -82,7 +82,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     private int mbus_network_threads = 1;
     private int heapSizePercentage = ApplicationContainerCluster.defaultHeapSizePercentageOfAvailableMemory;
     private Architecture adminClusterNodeResourcesArchitecture = Architecture.getDefault();
-    private boolean useRestrictedDataPlaneBindings = false;
     private Optional<CloudAccount> cloudAccount = Optional.empty();
     private boolean allowUserFilters = true;
     private boolean allowMoreThanOneContentGroupDown = false;
@@ -141,7 +140,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public int rpcEventsBeforeWakeup() { return rpc_events_before_wakeup; }
     @Override public String queryDispatchPolicy() { return queryDispatchPolicy; }
     @Override public String summaryDecodePolicy() { return summaryDecodePolicy; }
-    @Override public boolean useRestrictedDataPlaneBindings() { return useRestrictedDataPlaneBindings; }
     @Override public Optional<CloudAccount> cloudAccount() { return cloudAccount; }
     @Override public boolean allowUserFilters() { return allowUserFilters; }
     @Override public boolean enableGlobalPhase() { return true; } // Enable global-phase by default for unit tests only
@@ -366,11 +364,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
         return this;
     }
 
-    public TestProperties setUseRestrictedDataPlaneBindings(boolean useRestrictedDataPlaneBindings) {
-        this.useRestrictedDataPlaneBindings = useRestrictedDataPlaneBindings;
-        return this;
-    }
-
     public TestProperties setCloudAccount(CloudAccount cloudAccount) {
         this.cloudAccount = Optional.ofNullable(cloudAccount);
         return this;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java
index f4aa4f649bd..f869d578dcb 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java
@@ -57,7 +57,7 @@ class UriBindingsValidator extends Validator {
         if (binding instanceof SystemBindingPattern) return;
 
         // Allow binding to port if we are restricting data plane bindings
-        if (!binding.matchesAnyPort() && !deployState.featureFlags().useRestrictedDataPlaneBindings()) {
+        if (!binding.matchesAnyPort()) {
                 throw new IllegalArgumentException(createErrorMessage(binding, "binding with port is not allowed"));
         }
         if (!binding.host().equals(BindingPattern.WILDCARD_PATTERN)) {
@@ -73,7 +73,7 @@ class UriBindingsValidator extends Validator {
     }
 
     private static String createErrorMessage(BindingPattern binding, String message) {
-        return String.format("For binding '%s': %s", binding.patternString(), message);
+        return String.format("For binding '%s': %s", binding.originalPatternString(), message);
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomHandlerBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomHandlerBuilder.java
index 9b5a1429cb7..d674a56007f 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomHandlerBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomHandlerBuilder.java
@@ -48,7 +48,7 @@ public class DomHandlerBuilder extends VespaDomBuilder.DomConfigProducerBuilderB
     @Override
     protected Handler doBuild(DeployState deployState, TreeConfigProducer<AnyConfigProducer> parent, Element handlerElement) {
         Handler handler = createHandler(handlerElement);
-        var ports = deployState.isHosted() && deployState.featureFlags().useRestrictedDataPlaneBindings()
+        var ports = deployState.isHosted()
                 ? portBindingOverride : Set.<Integer>of();
 
         for (Element xmlBinding : XML.getChildren(handlerElement, "binding"))
@@ -64,7 +64,7 @@ public class DomHandlerBuilder extends VespaDomBuilder.DomConfigProducerBuilderB
         UserBindingPattern bindingPattern = UserBindingPattern.fromPattern(path);
         if (portBindingOverride.isEmpty()) return Set.of(bindingPattern);
         return portBindingOverride.stream()
-                .map(bindingPattern::withPort)
+                .map(bindingPattern::withOverriddenPort)
                 .toList();
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
index a5a567b18f8..0795fdf41d6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
@@ -92,7 +92,7 @@ public class ContainerDocumentApi {
         UserBindingPattern bindingPattern = UserBindingPattern.fromPattern(path);
         if (ports.isEmpty()) return List.of(bindingPattern);
         return ports.stream()
-                .map(p -> (BindingPattern)bindingPattern.withPort(p))
+                .map(p -> (BindingPattern)bindingPattern.withOverriddenPort(p))
                 .toList();
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/BindingPattern.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/BindingPattern.java
index c3dae7e4c8a..f580a0a2cc9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/BindingPattern.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/BindingPattern.java
@@ -63,11 +63,21 @@ public abstract class BindingPattern implements Comparable<BindingPattern> {
         return builder.append(path).toString();
     }
 
+    public String originalPatternString() {
+        StringBuilder builder = new StringBuilder(scheme).append("://").append(host);
+        originalPort().ifPresent(port -> builder.append(':').append(port));
+        return builder.append(path).toString();
+    }
+
     /** Compares the underlying pattern string for equality */
     public boolean hasSamePattern(BindingPattern other) { return this.patternString().equals(other.patternString()); }
 
     /** Returns true if pattern will match any port (if present) in uri **/
-    public boolean matchesAnyPort() { return port().filter(p -> !p.equals(WILDCARD_PATTERN)).isEmpty(); }
+    public boolean matchesAnyPort() { return originalPort().filter(p -> !p.equals(WILDCARD_PATTERN)).isEmpty(); }
+
+    public Optional<String> originalPort() {
+        return port();
+    }
 
     @Override
     public boolean equals(Object o) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/UserBindingPattern.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/UserBindingPattern.java
index 182eca835c1..e27dfe69f00 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/UserBindingPattern.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/UserBindingPattern.java
@@ -1,6 +1,9 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.container.component;
 
+import java.util.Objects;
+import java.util.Optional;
+
 /**
  * A {@link BindingPattern} which is constructed directly from a user provided 'binding' element from services.xml.
  *
@@ -8,12 +11,30 @@ package com.yahoo.vespa.model.container.component;
  */
 public class UserBindingPattern extends BindingPattern {
 
-    private UserBindingPattern(String scheme, String host, String port, String path) { super(scheme, host, port, path); }
-    private UserBindingPattern(String binding) { super(binding); }
+    private final Optional<String> originalPort;
+
+    private UserBindingPattern(String scheme, String host, String port, String path) {
+        super(scheme, host, port, path);
+        this.originalPort = null;
+    }
+    private UserBindingPattern(String scheme, String host, String port, Optional<String> originalPort, String path) {
+        super(scheme, host, port, path);
+        this.originalPort = originalPort;
+    }
+    private UserBindingPattern(String binding) {
+        super(binding);
+        this.originalPort = null;
+    }
 
     public static UserBindingPattern fromHttpPath(String path) { return new UserBindingPattern("http", "*", null, path); }
     public static UserBindingPattern fromPattern(String binding) { return new UserBindingPattern(binding); }
-    public UserBindingPattern withPort(int port) { return new UserBindingPattern(scheme(), host(), Integer.toString(port), path()); }
+    public UserBindingPattern withOverriddenPort(int port) { return new UserBindingPattern(scheme(), host(), Integer.toString(port), port(), path()); }
+
+    public Optional<String> originalPort() {
+        return Objects.isNull(originalPort)
+                ? port()
+                : originalPort;
+    }
 
     @Override
     public String toString() {
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 31f8eba48bf..80b676159cb 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
@@ -1114,7 +1114,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
 
     private void addSearchHandler(DeployState deployState, ApplicationContainerCluster cluster, Element searchElement, ConfigModelContext context) {
         var bindingPatterns = List.<BindingPattern>of(SearchHandler.DEFAULT_BINDING);
-        if (isHostedTenantApplication(context) && deployState.featureFlags().useRestrictedDataPlaneBindings()) {
+        if (isHostedTenantApplication(context)) {
             bindingPatterns = SearchHandler.bindingPattern(getDataplanePorts(deployState));
         }
         SearchHandler searchHandler = new SearchHandler(cluster,
@@ -1136,7 +1136,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
 
     private List<BindingPattern> toBindingList(DeployState deployState, ConfigModelContext context, List<Element> bindingElements) {
         List<BindingPattern> result = new ArrayList<>();
-        var portOverride = isHostedTenantApplication(context) && deployState.featureFlags().useRestrictedDataPlaneBindings() ? getDataplanePorts(deployState) : Set.<Integer>of();
+        var portOverride = isHostedTenantApplication(context) ? getDataplanePorts(deployState) : Set.<Integer>of();
         for (Element element: bindingElements) {
             String text = element.getTextContent().trim();
             if (!text.isEmpty())
@@ -1149,7 +1149,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         UserBindingPattern bindingPattern = UserBindingPattern.fromPattern(path);
         if (portBindingOverride.isEmpty()) return Set.of(bindingPattern);
         return portBindingOverride.stream()
-                .map(bindingPattern::withPort)
+                .map(bindingPattern::withOverriddenPort)
                 .toList();
     }
 
@@ -1160,7 +1160,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
 
         ContainerDocumentApi.HandlerOptions documentApiOptions = DocumentApiOptionsBuilder.build(documentApiElement);
         Element ignoreUndefinedFields = XML.getChild(documentApiElement, "ignore-undefined-fields");
-        var portBindingOverride = deployState.featureFlags().useRestrictedDataPlaneBindings() && isHostedTenantApplication(context)
+        var portBindingOverride = isHostedTenantApplication(context)
                 ? getDataplanePorts(deployState)
                 : Set.<Integer>of();
         return new ContainerDocumentApi(cluster, documentApiOptions,
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UriBindingsValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UriBindingsValidatorTest.java
index ff9596f2062..a56b268eeab 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UriBindingsValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UriBindingsValidatorTest.java
@@ -57,12 +57,6 @@ public class UriBindingsValidatorTest {
         runUriBindingValidator(true, createServicesXmlWithHandler("http://*/my-handler"));
     }
 
-    @Test
-    void allows_portbinding_when_restricting_data_plane() throws IOException, SAXException {
-        runUriBindingValidator(new TestProperties().setHostedVespa(true).setUseRestrictedDataPlaneBindings(true), createServicesXmlWithHandler("http://*:4443/my-handler"));
-    }
-
-    @Test
     void allows_user_binding_with_wildcard_port() throws IOException, SAXException {
         runUriBindingValidator(true, createServicesXmlWithHandler("http://*:*/my-handler"));
     }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/HandlerBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/HandlerBuilderTest.java
index 6d61610a84f..fac07c6c6e6 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/HandlerBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/HandlerBuilderTest.java
@@ -1,5 +1,6 @@
 package com.yahoo.vespa.model.container.xml;
 
+import com.yahoo.config.model.ConfigModelContext;
 import com.yahoo.config.model.builder.xml.test.DomBuilderTest;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.deploy.TestProperties;
@@ -110,36 +111,15 @@ public class HandlerBuilderTest extends ContainerModelBuilderTestBase {
     @Test
     void restricts_default_bindings_in_hosted_vespa() {
         DeployState deployState = new DeployState.Builder()
-                .properties(new TestProperties().setHostedVespa(true).setUseRestrictedDataPlaneBindings(true))
+                .properties(new TestProperties().setHostedVespa(true))
                 .build();
         verifyDefaultBindings(deployState, "http://*:4443");
     }
 
     @Test
-    void does_not_restrict_default_bindings_in_hosted_vespa_when_disabled() {
-        DeployState deployState = new DeployState.Builder()
-                .properties(new TestProperties().setHostedVespa(true).setUseRestrictedDataPlaneBindings(false))
-                .build();
-        verifyDefaultBindings(deployState, "http://*");
-    }
-
-    @Test
-    void does_not_restrict_infrastructure() {
-        DeployState deployState = new DeployState.Builder()
-
-                .properties(
-                        new TestProperties()
-                                .setApplicationId(ApplicationId.defaultId())
-                                .setHostedVespa(true)
-                                .setUseRestrictedDataPlaneBindings(false))
-                .build();
-        verifyDefaultBindings(deployState, "http://*");
-    }
-
-    @Test
     void restricts_custom_bindings_in_hosted_vespa() {
         DeployState deployState = new DeployState.Builder()
-                .properties(new TestProperties().setHostedVespa(true).setUseRestrictedDataPlaneBindings(true))
+                .properties(new TestProperties().setHostedVespa(true))
                 .build();
         verifyCustomSearchBindings(deployState, "http://*:4443");
     }
@@ -147,7 +127,7 @@ public class HandlerBuilderTest extends ContainerModelBuilderTestBase {
     @Test
     void does_not_restrict_default_bindings_in_self_hosted() {
         DeployState deployState = new DeployState.Builder()
-                .properties(new TestProperties().setHostedVespa(false).setUseRestrictedDataPlaneBindings(false))
+                .properties(new TestProperties().setHostedVespa(false))
                 .build();
         verifyDefaultBindings(deployState, "http://*");
     }
@@ -155,12 +135,15 @@ public class HandlerBuilderTest extends ContainerModelBuilderTestBase {
     @Test
     void does_not_restrict_custom_bindings_in_self_hosted() {
         DeployState deployState = new DeployState.Builder()
-                .properties(new TestProperties().setHostedVespa(false).setUseRestrictedDataPlaneBindings(false))
+                .properties(new TestProperties().setHostedVespa(false))
                 .build();
         verifyCustomSearchBindings(deployState, "http://*");
     }
 
     private void verifyDefaultBindings(DeployState deployState, String bindingPrefix) {
+        verifyDefaultBindings(deployState, bindingPrefix, ConfigModelContext.ApplicationType.DEFAULT);
+    }
+    private void verifyDefaultBindings(DeployState deployState, String bindingPrefix, ConfigModelContext.ApplicationType applicationType) {
         Element clusterElem = DomBuilderTest.parse(
                 "<container id='default' version='1.0'>",
                 "  <search/>",
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
index d815ea3328a..1ebfef77a51 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
@@ -195,7 +195,6 @@ public class ModelContextImpl implements ModelContext {
         private final int mbus_cpp_events_before_wakeup;
         private final int rpc_num_targets;
         private final int rpc_events_before_wakeup;
-        private final boolean useRestrictedDataPlaneBindings;
         private final int heapPercentage;
         private final boolean enableGlobalPhase;
         private final String summaryDecodePolicy;
@@ -239,7 +238,6 @@ public class ModelContextImpl implements ModelContext {
             this.rpc_events_before_wakeup = flagValue(source, appId, version, Flags.RPC_EVENTS_BEFORE_WAKEUP);
             this.queryDispatchPolicy = flagValue(source, appId, version, Flags.QUERY_DISPATCH_POLICY);
             this.queryDispatchWarmup = flagValue(source, appId, version, PermanentFlags.QUERY_DISPATCH_WARMUP);
-            this.useRestrictedDataPlaneBindings = flagValue(source, appId, version, Flags.RESTRICT_DATA_PLANE_BINDINGS);
             this.heapPercentage = flagValue(source, appId, version, PermanentFlags.HEAP_SIZE_PERCENTAGE);
             this.enableGlobalPhase = flagValue(source, appId, version, Flags.ENABLE_GLOBAL_PHASE);
             this.summaryDecodePolicy = flagValue(source, appId, version, Flags.SUMMARY_DECODE_POLICY);
@@ -293,7 +291,6 @@ public class ModelContextImpl implements ModelContext {
             }
             return defVal;
         }
-        @Override public boolean useRestrictedDataPlaneBindings() { return useRestrictedDataPlaneBindings; }
         @Override public boolean enableGlobalPhase() { return enableGlobalPhase; }
         @Override public boolean allowMoreThanOneContentGroupDown(ClusterSpec.Id id) { return allowMoreThanOneContentGroupDown.test(id); }
         @Override public boolean enableDataplaneProxy() { return enableDataplaneProxy; }
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index 436104c590c..d5eadf45b08 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -274,13 +274,6 @@ public class Flags {
             APPLICATION_ID,HOSTNAME,NODE_TYPE,TENANT_ID,VESPA_VERSION
     );
 
-    public static final UnboundBooleanFlag RESTRICT_DATA_PLANE_BINDINGS = defineFeatureFlag(
-            "restrict-data-plane-bindings", false,
-            List.of("mortent"), "2022-09-08", "2023-09-01",
-            "Use restricted data plane bindings",
-            "Takes effect at redeployment",
-            APPLICATION_ID);
-
     public static final UnboundBooleanFlag ENABLE_OTELCOL = defineFeatureFlag(
             "enable-otel-collector", false,
             List.of("olaa"), "2022-09-23", "2023-09-01",