Merged in remote-valve-config (pull request #162)

Properly configure remoteIpValve

* Fix typos

* Fix more typos

* Fix unit tests

* Fix docker compose wait

* Rename remoteipvalve tests

* Fix docker compose yaml

* Add missing quotes

* Remove sleep

* Fix formatting

* More formatting


Approved-by: Yifei Zhang

config/server.xml.j2

@@ -40,23 +40,28 @@
           <Valve className="org.apache.catalina.valves.StuckThreadDetectionValve"
                  threshold="60"/>
         {% if ((atl_tomcat_access_log == 'true') or
-            (atl_tomcat_access_log is not defined and (confluence_version.split('.') | map('int') | list) >= ('7.11.0'.split('.') | map('int') | list)) ) %}
+                     (atl_tomcat_access_log is not defined and (confluence_version.split('.') | map('int') | list) >= ('7.11.0'.split('.') | map('int') | list)) ) %}
           <Valve className="org.apache.catalina.valves.AccessLogValve"
-                 requestAttributesEnabled="true"
                  directory="logs"
                  prefix="confluence_access"
                  suffix=".log"
                  rotatable="true"
                  pattern="%h %{X-AUSERNAME}o %t &quot;%r&quot; %s %b %D %U %I &quot;%{User-Agent}i&quot;" 
+                 requestAttributesEnabled="{{ atl_tomcat_requestattributesenabled | default('false') }}"
                  maxDays="{{ atl_tomcat_access_logs_maxdays | default('-1') }}"/>
+        {%- endif %}
+        {%- if atl_tomcat_trustedproxies is defined or atl_tomcat_internalproxies is defined %}
           <Valve className="org.apache.catalina.valves.RemoteIpValve"
-                 proxiesHeader="x-forwarded-by"
+              {%- if atl_tomcat_trustedproxies %}
-                 internalProxies="{{ atl_tomcat_proxy_internal_ips | default('') }}"
+                trustedProxies="{{ atl_tomcat_trustedproxies }}"
-                 remoteIpHeader="x-forwarded-for"
+              {%- endif %}
-                 protocolHeader="x-forwarded-proto"
+              {%- if atl_tomcat_internalproxies %}
-                 requestAttributesEnabled="true"
+                internalProxies="{{ atl_tomcat_internalproxies }}"
-                 resolveHosts="false" />
+              {%- endif %}
-        {% endif %}
+                remoteIpHeader="x-forwarded-for"
+                proxiesHeader="x-forwarded-by"
+                protocolHeader="x-forwarded-proto"/>
+        {%- endif %}
         </Context>
 
         <Context path="${confluence.context.path}/synchrony-proxy"

func-tests/docker-compose.yml

@@ -24,13 +24,12 @@ services:
       - postgresql
     ports:
       - '8090:8090'
+    entrypoint: ["/bin/bash", "-c"]
     command: >
-      bash -c '
+      'command -v microdnf &> /dev/null && microdnf install -y nmap || { apt-get update -y && apt-get install -y netcat; } &&
-              command -v microdnf &> /dev/null && microdnf install -y nmap || { apt-get update -y && apt-get install -y netcat; } &&
+      /opt/atlassian/support/waitport postgresql 5432 &&
-              /opt/atlassian/support/waitport postgresql 5432 &&
+      chown -R confluence.confluence  /var/atlassian/application-data/confluence/ &&
-              chown -R confluence.confluence  /var/atlassian/application-data/confluence/ &&
+      /entrypoint.py'
-              /entrypoint.py
-      '
 
   smoketests:
     build:

tests/test_image.py

@@ -165,36 +165,45 @@ def test_server_xml_params(docker_cli, image):
 def test_server_xml_access_log_enabled(docker_cli, image):
     environment = {
         'ATL_TOMCAT_ACCESS_LOG': 'true',
-        'ATL_TOMCAT_PROXY_INTERNAL_IPS': '192.168.1.1',
-        'CONFLUENCE_VERSION': '7.10.0',
     }
 
     container = run_image(docker_cli, image, environment=environment)
     _jvm = wait_for_proc(container, get_bootstrap_proc(container))
 
     xml = parse_xml(container, f'{get_app_install_dir(container)}/conf/server.xml')
-    value = xml.find('.//Context/Valve[@className="org.apache.catalina.valves.RemoteIpValve"]')
+    value = xml.find('.//Context/Valve[@className="org.apache.catalina.valves.AccessLogValve"]')
-    assert value.get('internalProxies') == environment.get('ATL_TOMCAT_PROXY_INTERNAL_IPS')
+    assert value.get('requestAttributesEnabled') == 'false'
+
+def test_server_xml_access_log_request_attributes(docker_cli, image):
+    environment = {
+        'ATL_TOMCAT_ACCESS_LOG': 'true',
+        'ATL_TOMCAT_REQUESTATTRIBUTESENABLED': 'true',
+    }
+
+    container = run_image(docker_cli, image, environment=environment)
+    _jvm = wait_for_proc(container, get_bootstrap_proc(container))
+
+    xml = parse_xml(container, f'{get_app_install_dir(container)}/conf/server.xml')
+    value = xml.find('.//Context/Valve[@className="org.apache.catalina.valves.AccessLogValve"]')
+    assert value.get('requestAttributesEnabled') == 'true'
+
 
 def test_server_xml_access_log_disabled(docker_cli, image):
     environment = {
         'ATL_TOMCAT_ACCESS_LOG': 'false',
-        'ATL_TOMCAT_PROXY_INTERNAL_IPS': '192.168.1.1',
-        'CONFLUENCE_VERSION': '7.12.0',
     }
 
     container = run_image(docker_cli, image, environment=environment)
     _jvm = wait_for_proc(container, get_bootstrap_proc(container))
 
     xml = parse_xml(container, f'{get_app_install_dir(container)}/conf/server.xml')
-    value = xml.find('.//Context/Valve[@className="org.apache.catalina.valves.RemoteIpValve"]')
+    value = xml.find('.//Context/Valve[@className="org.apache.catalina.valves.AccessLogValve"]')
     assert value is None
 
-def test_server_xml_access_log_default_ver_lt_7_11(docker_cli, image):
+def test_server_xml_internal_proxies(docker_cli, image):
     environment = {
-        #'ATL_TOMCAT_ACCESS_LOG': Not defined,
+        'ATL_TOMCAT_ACCESS_LOG': 'true',
-        'ATL_TOMCAT_PROXY_INTERNAL_IPS': '192.168.1.1',
+        'ATL_TOMCAT_INTERNALPROXIES': '192.168.1.1',
-        'CONFLUENCE_VERSION': "7.10.0",
     }
 
     container = run_image(docker_cli, image, environment=environment)
@@ -202,13 +211,12 @@ def test_server_xml_access_log_default_ver_lt_7_11(docker_cli, image):
 
     xml = parse_xml(container, f'{get_app_install_dir(container)}/conf/server.xml')
     value = xml.find('.//Context/Valve[@className="org.apache.catalina.valves.RemoteIpValve"]')
-    assert value is None
+    assert value.get('internalProxies') == environment.get('ATL_TOMCAT_INTERNALPROXIES')
 
-def test_server_xml_access_log_default_ver_gt_7_11(docker_cli, image):
+def test_server_xml_trusted_proxies(docker_cli, image):
     environment = {
-        #'ATL_TOMCAT_ACCESS_LOG': Not defined,
+        'ATL_TOMCAT_ACCESS_LOG': 'true',
-        'ATL_TOMCAT_PROXY_INTERNAL_IPS': '192.168.1.1',
+        'ATL_TOMCAT_TRUSTEDPROXIES': '192.168.1.2',
-        'CONFLUENCE_VERSION': '7.12.0',
     }
 
     container = run_image(docker_cli, image, environment=environment)
@@ -216,7 +224,23 @@ def test_server_xml_access_log_default_ver_gt_7_11(docker_cli, image):
 
     xml = parse_xml(container, f'{get_app_install_dir(container)}/conf/server.xml')
     value = xml.find('.//Context/Valve[@className="org.apache.catalina.valves.RemoteIpValve"]')
-    assert value.get('internalProxies') == environment.get('ATL_TOMCAT_PROXY_INTERNAL_IPS')
+    assert value.get('trustedProxies') == environment.get('ATL_TOMCAT_TRUSTEDPROXIES')
+
+def test_server_xml_internal_trusted_proxies(docker_cli, image):
+    environment = {
+        'ATL_TOMCAT_ACCESS_LOG': 'true',
+        'ATL_TOMCAT_INTERNALPROXIES': '192.168.1.1',
+        'ATL_TOMCAT_TRUSTEDPROXIES': '192.168.1.2',
+    }
+
+    container = run_image(docker_cli, image, environment=environment)
+    _jvm = wait_for_proc(container, get_bootstrap_proc(container))
+
+    xml = parse_xml(container, f'{get_app_install_dir(container)}/conf/server.xml')
+    value = xml.find('.//Context/Valve[@className="org.apache.catalina.valves.RemoteIpValve"]')
+    assert value.get('internalProxies') == environment.get('ATL_TOMCAT_INTERNALPROXIES')
+    assert value.get('trustedProxies') == environment.get('ATL_TOMCAT_TRUSTEDPROXIES')
+
 
 def test_seraph_defaults(docker_cli, image):
     container = run_image(docker_cli, image)