Add implicit access control when application has explicit http

2 files changed, 39 insertions, 16 deletions

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 a67c98bca49..cd292da6fa1 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
@@ -324,7 +324,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
             cluster.setHttp(buildHttp(deployState, cluster, httpElement));
         }
         if (isHostedTenantApplication(context)) {
-            addHostedImplicitHttpIfNotPresent(deployState, cluster);
+            addHostedImplicitHttpIfNotPresent(cluster);
+            addHostedImplicitAccessControlIfNotPresent(deployState, cluster);
             addAdditionalHostedConnector(deployState, cluster);
         }
     }
@@ -356,12 +357,9 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         return deployState.isHosted() && context.getApplicationType() == ApplicationType.DEFAULT && !isTesterApplication;
     }
 
-    private static void addHostedImplicitHttpIfNotPresent(DeployState deployState, ApplicationContainerCluster cluster) {
+    private static void addHostedImplicitHttpIfNotPresent(ApplicationContainerCluster cluster) {
         if(cluster.getHttp() == null) {
-            Http http = deployState.getProperties().athenzDomain()
-                    .map(tenantDomain -> createHostedImplicitHttpWithAccessControl(deployState, tenantDomain, cluster))
-                    .orElseGet(() -> createHostedImplicitHttpWithoutAccessControl(cluster));
-            cluster.setHttp(http);
+            cluster.setHttp(new Http(new FilterChains(cluster)));
         }
         if(cluster.getHttp().getHttpServer().isEmpty()) {
             JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer"));
@@ -370,24 +368,20 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         }
     }
 
-    private static Http createHostedImplicitHttpWithAccessControl(
-            DeployState deployState, AthenzDomain tenantDomain, ApplicationContainerCluster cluster) {
+    private void addHostedImplicitAccessControlIfNotPresent(DeployState deployState, ApplicationContainerCluster cluster) {
+        Http http = cluster.getHttp();
+        if (http.getAccessControl().isPresent()) return; // access control added explicitly
+        AthenzDomain tenantDomain = deployState.getProperties().athenzDomain().orElse(null);
+        if (tenantDomain == null) return; // tenant domain not present, cannot add access control. this should eventually be a failure.
         AccessControl accessControl =
                 new AccessControl.Builder(tenantDomain.value(), deployState.getDeployLogger())
                         .setHandlers(cluster)
                         .readEnabled(false)
                         .writeEnabled(false)
                         .build();
-        FilterChains filterChains = new FilterChains(cluster);
-        filterChains.add(new Chain<>(FilterChains.emptyChainSpec(ACCESS_CONTROL_CHAIN_ID)));
-        Http http = new Http(filterChains);
+        http.getFilterChains().add(new Chain<>(FilterChains.emptyChainSpec(ACCESS_CONTROL_CHAIN_ID)));
         http.setAccessControl(accessControl);
         http.getBindings().addAll(accessControl.getBindings());
-        return http;
-    }
-
-    private static Http createHostedImplicitHttpWithoutAccessControl(ApplicationContainerCluster cluster) {
-        return new Http(new FilterChains(cluster));
     }
 
     private Http buildHttp(DeployState deployState, ApplicationContainerCluster cluster, Element httpElement) {
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 12365fb773c..8fcd743cb2d 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
@@ -46,6 +46,7 @@ import com.yahoo.vespa.model.container.SecretStore;
 import com.yahoo.vespa.model.container.component.Component;
 import com.yahoo.vespa.model.container.http.AccessControl;
 import com.yahoo.vespa.model.container.http.ConnectorFactory;
+import com.yahoo.vespa.model.container.http.Http;
 import com.yahoo.vespa.model.content.utils.ContentClusterUtils;
 import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg;
 import org.hamcrest.Matchers;
@@ -857,6 +858,34 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
         assertThat(accessControl.domain, equalTo(tenantDomain.value()));
     }
 
+    @Test
+    public void access_control_is_implicitly_added_for_hosted_apps_with_existing_http_element() {
+        Element clusterElem = DomBuilderTest.parse(
+                "<container version='1.0'>",
+                "  <http>",
+                "    <server port='" + getDefaults().vespaWebServicePort() + "' id='main' />",
+                "    <filtering>",
+                "      <filter id='outer' />",
+                "      <request-chain id='myChain'>",
+                "        <filter id='inner' />",
+                "      </request-chain>",
+                "    </filtering>",
+                "  </http>",
+                nodesXml,
+                "</container>" );
+        AthenzDomain tenantDomain = AthenzDomain.from("my-tenant-domain");
+        DeployState state = new DeployState.Builder().properties(
+                new TestProperties()
+                        .setAthenzDomain(tenantDomain)
+                        .setHostedVespa(true))
+                .build();
+        createModel(root, state, null, clusterElem);
+        Http http = ((ApplicationContainer) root.getProducer("container/container.0")).getHttp();
+        assertThat(http.getAccessControl().isPresent(), is(true));
+        assertThat(http.getFilterChains().hasChain(AccessControl.ACCESS_CONTROL_CHAIN_ID), is(true));
+        assertThat(http.getFilterChains().hasChain(ComponentId.fromString("myChain")), is(true));
+    }
+
 
     private Element generateContainerElementWithRenderer(String rendererId) {
         return DomBuilderTest.parse(