Merge pull request #9052 from vespa-engine/bjorncs/binding-warning

Warn on 'https' bindings during deploy 

6 files changed, 70 insertions, 38 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java
index 071411845ad..4de50fc2dde 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java
@@ -4,11 +4,11 @@ package com.yahoo.vespa.model.container.http;
 import com.google.common.collect.ImmutableList;
 import com.yahoo.component.ComponentId;
 import com.yahoo.component.ComponentSpecification;
+import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.component.FileStatusHandlerComponent;
 import com.yahoo.vespa.model.container.component.Handler;
 import com.yahoo.vespa.model.container.component.Servlet;
-import com.yahoo.vespa.model.container.http.Http.Binding;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -47,10 +47,12 @@ public final class AccessControl {
         private final Set<String> excludeBindings = new LinkedHashSet<>();
         private Collection<Handler<?>> handlers = Collections.emptyList();
         private Collection<Servlet> servlets = Collections.emptyList();
+        private final DeployLogger logger;
 
-        public Builder(String domain, String applicationId) {
+        public Builder(String domain, String applicationId, DeployLogger logger) {
             this.domain = domain;
             this.applicationId = applicationId;
+            this.logger = logger;
         }
 
         public Builder readEnabled(boolean readEnabled) {
@@ -85,7 +87,7 @@ public final class AccessControl {
 
         public AccessControl build() {
             return new AccessControl(domain, applicationId, writeEnabled, readEnabled,
-                                     excludeBindings, vespaDomain, servlets, handlers);
+                                     excludeBindings, vespaDomain, servlets, handlers, logger);
         }
     }
 
@@ -97,6 +99,7 @@ public final class AccessControl {
     private final Set<String> excludedBindings;
     private final Collection<Handler<?>> handlers;
     private final Collection<Servlet> servlets;
+    private final DeployLogger logger;
 
     private AccessControl(String domain,
                           String applicationId,
@@ -105,7 +108,8 @@ public final class AccessControl {
                           Set<String> excludedBindings,
                           Optional<String> vespaDomain,
                           Collection<Servlet> servlets,
-                          Collection<Handler<?>> handlers) {
+                          Collection<Handler<?>> handlers,
+                          DeployLogger logger) {
         this.domain = domain;
         this.applicationId = applicationId;
         this.readEnabled = readEnabled;
@@ -114,6 +118,7 @@ public final class AccessControl {
         this.vespaDomain = vespaDomain;
         this.handlers = handlers;
         this.servlets = servlets;
+        this.logger = logger;
     }
 
     public List<Binding> getBindings() {
@@ -125,14 +130,14 @@ public final class AccessControl {
         return handlers.stream()
                         .filter(this::shouldHandlerBeProtected)
                         .flatMap(handler -> handler.getServerBindings().stream())
-                        .map(AccessControl::accessControlBinding);
+                        .map(binding -> accessControlBinding(binding, logger));
     }
 
     private Stream<Binding> getServletBindings() {
         return servlets.stream()
                 .filter(this::shouldServletBeProtected)
                 .flatMap(AccessControl::servletBindings)
-                .map(AccessControl::accessControlBinding);
+                .map(binding -> accessControlBinding(binding, logger));
     }
 
     private boolean shouldHandlerBeProtected(Handler<?> handler) {
@@ -148,8 +153,8 @@ public final class AccessControl {
         return servletBindings(servlet).noneMatch(excludedBindings::contains);
     }
 
-    private static Binding accessControlBinding(String binding) {
-        return new Binding(new ComponentSpecification(ACCESS_CONTROL_CHAIN_ID.stringValue()), binding);
+    private static Binding accessControlBinding(String binding, DeployLogger logger) {
+        return Binding.create(new ComponentSpecification(ACCESS_CONTROL_CHAIN_ID.stringValue()), binding, logger);
     }
 
     private static Stream<String> servletBindings(Servlet servlet) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/Binding.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/Binding.java
new file mode 100644
index 00000000000..09ac5f580ed
--- /dev/null
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/Binding.java
@@ -0,0 +1,38 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.model.container.http;
+
+import com.yahoo.component.ComponentSpecification;
+import com.yahoo.config.application.api.DeployLogger;
+
+import java.util.logging.Level;
+
+/**
+ * @author bjorncs
+ */
+public class Binding {
+    private final ComponentSpecification filterId;
+    private final String binding;
+
+    private Binding(ComponentSpecification filterId, String binding) {
+        this.filterId = filterId;
+        this.binding = binding;
+    }
+
+    public static Binding create(ComponentSpecification filterId, String binding, DeployLogger logger) {
+        if (binding.startsWith("https://")) {
+            logger.log(Level.WARNING, String.format(
+                    "For binding '%s' on '%s': 'https' bindings are deprecated, " +
+                            "use 'http' instead to bind to both http and https traffic.",
+                    binding, filterId));
+        }
+        return new Binding(filterId, binding);
+    }
+
+    public ComponentSpecification filterId() {
+        return filterId;
+    }
+
+    public String binding() {
+        return binding;
+    }
+}
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/Http.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/Http.java
index 9e85a889075..9a67871e683 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/Http.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/Http.java
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.container.http;
 
-import com.yahoo.component.ComponentSpecification;
 import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.jdisc.http.ServerConfig;
@@ -20,16 +19,6 @@ import java.util.Optional;
  */
 public class Http extends AbstractConfigProducer<AbstractConfigProducer<?>> implements ServerConfig.Producer {
 
-    public static class Binding {
-        public final ComponentSpecification filterId;
-        public final String binding;
-
-        public Binding(ComponentSpecification filterId, String binding) {
-            this.filterId = filterId;
-            this.binding = binding;
-        }
-    }
-
     private FilterChains filterChains;
     private JettyHttpServer httpServer;
     public final List<Binding> bindings;
@@ -91,8 +80,8 @@ public class Http extends AbstractConfigProducer<AbstractConfigProducer<?>> impl
         for (final Binding binding : bindings) {
             builder.filter(
                     new ServerConfig.Filter.Builder()
-                            .id(binding.filterId.stringValue())
-                            .binding(binding.binding));
+                            .id(binding.filterId().stringValue())
+                            .binding(binding.binding()));
         }
     }
 
@@ -110,8 +99,8 @@ public class Http extends AbstractConfigProducer<AbstractConfigProducer<?>> impl
             ComponentRegistry<Chain<Filter>> chains = filterChains.allChains();
 
             for (Binding binding: bindings) {
-                if (filters.getComponent(binding.filterId) == null && chains.getComponent(binding.filterId) == null)
-                    throw new RuntimeException("Can't find filter " + binding.filterId + " for binding " + binding.binding);
+                if (filters.getComponent(binding.filterId()) == null && chains.getComponent(binding.filterId()) == null)
+                    throw new RuntimeException("Can't find filter " + binding.filterId() + " for binding " + binding.binding());
             }
         }
     }
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 08268f5085d..8cf430741f0 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
@@ -17,7 +17,7 @@ import com.yahoo.vespa.model.container.component.chain.Chain;
 import com.yahoo.vespa.model.container.http.AccessControl;
 import com.yahoo.vespa.model.container.http.FilterChains;
 import com.yahoo.vespa.model.container.http.Http;
-import com.yahoo.vespa.model.container.http.Http.Binding;
+import com.yahoo.vespa.model.container.http.Binding;
 import org.w3c.dom.Element;
 
 import java.util.ArrayList;
@@ -41,7 +41,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
         Element filteringElem = XML.getChild(spec, "filtering");
         if (filteringElem != null) {
             filterChains = new FilterChainsBuilder().build(deployState, ancestor, filteringElem);
-            bindings = readFilterBindings(filteringElem);
+            bindings = readFilterBindings(filteringElem, deployState.getDeployLogger());
 
             Element accessControlElem = XML.getChild(filteringElem, "access-control");
             if (accessControlElem != null) {
@@ -65,7 +65,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
         String application = XmlHelper.getOptionalChildValue(accessControlElem, "application")
                 .orElse(getDeployedApplicationId(deployState, ancestor).value());
 
-        AccessControl.Builder builder = new AccessControl.Builder(accessControlElem.getAttribute("domain"), application);
+        AccessControl.Builder builder = new AccessControl.Builder(accessControlElem.getAttribute("domain"), application, deployState.getDeployLogger());
 
         getContainerCluster(ancestor).ifPresent(cluster -> {
             builder.setHandlers(cluster.getHandlers());
@@ -106,7 +106,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
         return Optional.of((ApplicationContainerCluster) currentProducer);
     }
 
-    private List<Binding> readFilterBindings(Element filteringSpec) {
+    private List<Binding> readFilterBindings(Element filteringSpec, DeployLogger logger) {
         List<Binding> result = new ArrayList<>();
 
         for (Element child: XML.getChildren(filteringSpec)) {
@@ -116,7 +116,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
 
                 for (Element bindingSpec: XML.getChildren(child, "binding")) {
                     String binding = XML.getValue(bindingSpec);
-                    result.add(new Binding(chainId, binding));
+                    result.add(Binding.create(chainId, binding, logger));
                 }
             }
         }
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 4d04a1be614..b6d63af180d 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
@@ -52,9 +52,9 @@ public class FilterBindingsTest extends DomBuilderTest {
                 "</http>");
         Http http = buildHttp(xml);
 
-        Http.Binding binding = first(http.bindings);
-        assertThat(binding.filterId.getName(), is("my-request-chain"));
-        assertThat(binding.binding, is(MY_CHAIN_BINDING));
+        Binding binding = first(http.bindings);
+        assertThat(binding.filterId().getName(), is("my-request-chain"));
+        assertThat(binding.binding(), is(MY_CHAIN_BINDING));
 
         Chain<Filter> myChain = http.getFilterChains().allChains().getComponent("my-request-chain");
         assertNotNull("Missing chain", myChain);
@@ -72,9 +72,9 @@ public class FilterBindingsTest extends DomBuilderTest {
                 "</http>");
         Http http = buildHttp(xml);
 
-        Http.Binding binding = first(http.bindings);
-        assertThat(binding.filterId.getName(), is("my-response-chain"));
-        assertThat(binding.binding, is(MY_CHAIN_BINDING));
+        Binding binding = first(http.bindings);
+        assertThat(binding.filterId().getName(), is("my-response-chain"));
+        assertThat(binding.binding(), is(MY_CHAIN_BINDING));
 
         Chain<Filter> myChain = http.getFilterChains().allChains().getComponent("my-response-chain");
         assertNotNull("Missing chain", myChain);
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 f5df3df0070..5433c7659cc 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
@@ -8,7 +8,7 @@ import com.yahoo.container.jdisc.state.StateHandler;
 import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.http.AccessControl;
 import com.yahoo.vespa.model.container.http.Http;
-import com.yahoo.vespa.model.container.http.Http.Binding;
+import com.yahoo.vespa.model.container.http.Binding;
 import com.yahoo.vespa.model.container.http.xml.HttpBuilder;
 import com.yahoo.vespa.model.container.jersey.Jersey2Servlet;
 import org.junit.Test;
@@ -140,8 +140,8 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
                    missingRequiredBindings.isEmpty());
 
         FORBIDDEN_HANDLER_BINDINGS.forEach(forbiddenBinding -> http.getBindings().forEach(
-                binding -> assertFalse("Access control chain was bound to: " + binding.binding,
-                                       binding.binding.contains(forbiddenBinding))));
+                binding -> assertFalse("Access control chain was bound to: " + binding.binding(),
+                                       binding.binding().contains(forbiddenBinding))));
     }
 
     @Test
@@ -256,7 +256,7 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
 
     private boolean containsBinding(Collection<Binding> bindings, String binding) {
         for (Binding b : bindings) {
-            if (b.binding.contains(binding))
+            if (b.binding().contains(binding))
                 return true;
         }
         return false;