From 0925449a0d68a7616bdd2dc097c205f464244655 Mon Sep 17 00:00:00 2001
From: Lukasz Nowak <luke@nexedi.com>
Date: Tue, 20 Nov 2018 13:28:20 +0100
Subject: [PATCH] caddy-frontend: Deduplicate local host names

custom_domain and server-alias on given slave do not have to clash, and
can be deduplicated during request parameter analysis.

/reviewed-on https://lab.nexedi.com/nexedi/slapos/merge_requests/444
---
 software/caddy-frontend/buildout.hash.cfg     |  4 +-
 .../instance-apache-replicate.cfg.in          |  7 +-
 .../templates/default-virtualhost.conf.in     | 10 +--
 software/caddy-frontend/test/test.py          | 79 +++++++++++++++++--
 ...est.TestSlave.test_file_list_log-CADDY.txt |  4 +
 ...tSlave.test_monitor_promise_list-CADDY.txt |  4 +
 6 files changed, 93 insertions(+), 15 deletions(-)

diff --git a/software/caddy-frontend/buildout.hash.cfg b/software/caddy-frontend/buildout.hash.cfg
index 5267fbc9e..5b19b7f39 100644
--- a/software/caddy-frontend/buildout.hash.cfg
+++ b/software/caddy-frontend/buildout.hash.cfg
@@ -26,7 +26,7 @@ md5sum = ab1795f92e32655d05c662c965d2b1f5
 
 [template-apache-replicate]
 filename = instance-apache-replicate.cfg.in
-md5sum = a0110d0ec69875946a16ac3a4b269eb8
+md5sum = 44d50bf8391b5a73b2ab72923efe6437
 
 [template-slave-list]
 filename = templates/apache-custom-slave-list.cfg.in
@@ -62,7 +62,7 @@ md5sum = f20d6c3d2d94fb685f8d26dfca1e822b
 
 [template-default-slave-virtualhost]
 filename = templates/default-virtualhost.conf.in
-md5sum = 9e00b6d981b9f93a486ef06a47345ebd
+md5sum = 669a93b7e21b99066a84494fec47906b
 
 [template-cached-slave-virtualhost]
 filename = templates/cached-virtualhost.conf.in
diff --git a/software/caddy-frontend/instance-apache-replicate.cfg.in b/software/caddy-frontend/instance-apache-replicate.cfg.in
index abf78054b..3f05435cf 100644
--- a/software/caddy-frontend/instance-apache-replicate.cfg.in
+++ b/software/caddy-frontend/instance-apache-replicate.cfg.in
@@ -71,6 +71,7 @@ context =
 {% set unauthorized_message = 'slave not authorized' %}
 {% for slave in slave_instance_list %}
 {%   set slave_error_list = [] %}
+{%   set slave_server_alias_unclashed = [] %}
 {#   BBB: apache_custom_https AND apache_custom_http #}
 {%   set custom_domain = slave.get('custom_domain') %}
 {%   if custom_domain and custom_domain in used_host_list %}
@@ -88,13 +89,17 @@ context =
 {%       if not validators.domain(clean_slave_alias) %}
 {%         do slave_error_list.append('server-alias %r not valid' % (slave_alias,)) %}
 {%       else %}
-{%         if slave_alias in used_host_list %}
+{%         if slave_alias in slave_server_alias_unclashed or slave_alias == custom_domain %}
+{#           optionally do something about reporting back that server-alias has been unclashed #}
+{%         elif slave_alias in used_host_list %}
 {%           do slave_error_list.append('server-alias %r clashes' % (slave_alias,)) %}
 {%         else %}
+{%           do slave_server_alias_unclashed.append(slave_alias) %}
 {%           do used_host_list.append(slave_alias) %}
 {%         endif %}
 {%       endif %}
 {%     endfor %}
+{%     do slave.__setitem__('server-alias', ' '.join(slave_server_alias_unclashed)) %}
 {%   endif %}
 {%   for key in ['caddy_custom_http', 'caddy_custom_https', 'apache_custom_http', 'apache_custom_https'] %}
 {%     if slave.get(key) %}
diff --git a/software/caddy-frontend/templates/default-virtualhost.conf.in b/software/caddy-frontend/templates/default-virtualhost.conf.in
index 3eae50a20..9c2a568fa 100644
--- a/software/caddy-frontend/templates/default-virtualhost.conf.in
+++ b/software/caddy-frontend/templates/default-virtualhost.conf.in
@@ -12,12 +12,10 @@
 {%- set disabled_cookie_list =  slave_parameter.get('disabled-cookie-list', '').split() %}
 {%- set https_only = ('' ~ slave_parameter.get('https-only', '')).lower() in TRUE_VALUES %}
 {%- set slave_type = slave_parameter.get('type', '') %}
-{%- set host_list = [] %}
-{%- for host in [slave_parameter.get('custom_domain')] + server_alias_list %}
-{%-   if host not in host_list %}
-{%-     do host_list.append(host) %}
-{%-   endif %}
-{%- endfor %}
+{%- set host_list = server_alias_list %}
+{%- if slave_parameter.get('custom_domain') not in host_list %}
+{%-   do host_list.append(slave_parameter.get('custom_domain')) %}
+{%- endif %}
 {%- set backend_url = slave_parameter.get('https-url', slave_parameter.get('url', '')) %}
 {%- set http_host_list = [] %}
 {%- set https_host_list = [] %}
diff --git a/software/caddy-frontend/test/test.py b/software/caddy-frontend/test/test.py
index 298e0fe29..cd791ed3d 100644
--- a/software/caddy-frontend/test/test.py
+++ b/software/caddy-frontend/test/test.py
@@ -589,6 +589,15 @@ http://apachecustomhttpsaccepted.example.com:%%(http_port)s {
         'url': cls.backend_url,
         'server-alias': '*.alias1.example.com',
       },
+      'server-alias-duplicated': {
+        'url': cls.backend_url,
+        'server-alias': 'alias3.example.com alias3.example.com',
+      },
+      'server-alias_custom_domain-duplicated': {
+        'url': cls.backend_url,
+        'custom_domain': 'alias4.example.com',
+        'server-alias': 'alias4.example.com alias4.example.com',
+      },
       'ssl-proxy-verify_ssl_proxy_ca_crt': {
         'url': cls.backend_https_url,
         'ssl-proxy-verify': True,
@@ -1124,6 +1133,66 @@ http://apachecustomhttpsaccepted.example.com:%%(http_port)s {
 
     self.assertEqualResultJson(result, 'Path', '/test-path')
 
+  def test_server_alias_duplicated(self):
+    parameter_dict = self.slave_connection_parameter_dict_dict[
+      'server-alias-duplicated']
+    self.assertLogAccessUrlWithPop(parameter_dict, 'server-alias-duplicated')
+    self.assertEqual(
+      {
+        'domain': 'serveraliasduplicated.example.com',
+        'replication_number': '1',
+        'url': 'http://serveraliasduplicated.example.com',
+        'site_url': 'http://serveraliasduplicated.example.com',
+        'secure_access': 'https://serveraliasduplicated.example.com',
+        'public-ipv4': LOCAL_IPV4,
+      },
+      parameter_dict
+    )
+
+    result = self.fakeHTTPSResult(
+      parameter_dict['domain'], parameter_dict['public-ipv4'], 'test-path')
+
+    self.assertEqual(
+      open('wildcard.example.com.crt').read(),
+      der2pem(result.peercert))
+
+    self.assertEqualResultJson(result, 'Path', '/test-path')
+
+    result = self.fakeHTTPSResult(
+      'alias3.example.com', parameter_dict['public-ipv4'], 'test-path')
+
+    self.assertEqual(
+      open('wildcard.example.com.crt').read(),
+      der2pem(result.peercert))
+
+    self.assertEqualResultJson(result, 'Path', '/test-path')
+
+  def test_server_alias_custom_domain_duplicated(self):
+    parameter_dict = self.slave_connection_parameter_dict_dict[
+      'server-alias_custom_domain-duplicated']
+    self.assertLogAccessUrlWithPop(
+      parameter_dict, 'server-alias_custom_domain-duplicated')
+    self.assertEqual(
+      {
+        'domain': 'alias4.example.com',
+        'replication_number': '1',
+        'url': 'http://alias4.example.com',
+        'site_url': 'http://alias4.example.com',
+        'secure_access': 'https://alias4.example.com',
+        'public-ipv4': LOCAL_IPV4,
+      },
+      parameter_dict
+    )
+
+    result = self.fakeHTTPSResult(
+      parameter_dict['domain'], parameter_dict['public-ipv4'], 'test-path')
+
+    self.assertEqual(
+      open('wildcard.example.com.crt').read(),
+      der2pem(result.peercert))
+
+    self.assertEqualResultJson(result, 'Path', '/test-path')
+
   @skip('Feature postponed')
   def test_check_error_log(self):
     # Caddy: Need to implement similar thing like check-error-on-apache-log
@@ -3450,10 +3519,9 @@ class TestDuplicateSiteKeyProtection(SlaveHttpFrontendTestCase, TestDataMixin):
       'rejected-slave-amount': '3',
       'slave-amount': '4',
       'rejected-slave-dict':
-      '{"_site_4": ["custom_domain \'duplicate.example.com\' clashes", '
-      '"server-alias \'duplicate.example.com\' clashes"], "_site_1": '
-      '["custom_domain \'duplicate.example.com\' clashes"], "_site_3": '
-      '["server-alias \'duplicate.example.com\' clashes"]}'
+      '{"_site_4": ["custom_domain \'duplicate.example.com\' clashes"], '
+      '"_site_1": ["custom_domain \'duplicate.example.com\' clashes"], '
+      '"_site_3": ["server-alias \'duplicate.example.com\' clashes"]}'
     }
 
     self.assertEqual(
@@ -3505,8 +3573,7 @@ class TestDuplicateSiteKeyProtection(SlaveHttpFrontendTestCase, TestDataMixin):
     self.assertEqual(
       {
         'request-error-list':
-        '["custom_domain \'duplicate.example.com\' clashes", "server-alias '
-        '\'duplicate.example.com\' clashes"]'
+        '["custom_domain \'duplicate.example.com\' clashes"]'
       },
       parameter_dict
     )
diff --git a/software/caddy-frontend/test/test_data/test.TestSlave.test_file_list_log-CADDY.txt b/software/caddy-frontend/test/test_data/test.TestSlave.test_file_list_log-CADDY.txt
index dcf05a80e..63d3e8b2f 100644
--- a/software/caddy-frontend/test/test_data/test.TestSlave.test_file_list_log-CADDY.txt
+++ b/software/caddy-frontend/test/test_data/test.TestSlave.test_file_list_log-CADDY.txt
@@ -39,9 +39,13 @@ TestSlave-1/var/log/httpd/_prefer-gzip-encoding-to-backend_access_log
 TestSlave-1/var/log/httpd/_prefer-gzip-encoding-to-backend_error_log
 TestSlave-1/var/log/httpd/_re6st-optimal-test_access_log
 TestSlave-1/var/log/httpd/_re6st-optimal-test_error_log
+TestSlave-1/var/log/httpd/_server-alias-duplicated_access_log
+TestSlave-1/var/log/httpd/_server-alias-duplicated_error_log
 TestSlave-1/var/log/httpd/_server-alias-wildcard_access_log
 TestSlave-1/var/log/httpd/_server-alias-wildcard_error_log
 TestSlave-1/var/log/httpd/_server-alias_access_log
+TestSlave-1/var/log/httpd/_server-alias_custom_domain-duplicated_access_log
+TestSlave-1/var/log/httpd/_server-alias_custom_domain-duplicated_error_log
 TestSlave-1/var/log/httpd/_server-alias_error_log
 TestSlave-1/var/log/httpd/_ssl-proxy-verify-unverified_access_log
 TestSlave-1/var/log/httpd/_ssl-proxy-verify-unverified_error_log
diff --git a/software/caddy-frontend/test/test_data/test.TestSlave.test_monitor_promise_list-CADDY.txt b/software/caddy-frontend/test/test_data/test.TestSlave.test_monitor_promise_list-CADDY.txt
index febced599..baad74fde 100644
--- a/software/caddy-frontend/test/test_data/test.TestSlave.test_monitor_promise_list-CADDY.txt
+++ b/software/caddy-frontend/test/test_data/test.TestSlave.test_monitor_promise_list-CADDY.txt
@@ -39,10 +39,14 @@ TestSlave-1/etc/monitor-promise/check-_prefer-gzip-encoding-to-backend-error-log
 TestSlave-1/etc/monitor-promise/check-_re6st-optimal-test-error-log-last-day
 TestSlave-1/etc/monitor-promise/check-_re6st-optimal-test-error-log-last-hour
 TestSlave-1/etc/monitor-promise/check-_re6st-optimal-test-re6st-optimal-test
+TestSlave-1/etc/monitor-promise/check-_server-alias-duplicated-error-log-last-day
+TestSlave-1/etc/monitor-promise/check-_server-alias-duplicated-error-log-last-hour
 TestSlave-1/etc/monitor-promise/check-_server-alias-error-log-last-day
 TestSlave-1/etc/monitor-promise/check-_server-alias-error-log-last-hour
 TestSlave-1/etc/monitor-promise/check-_server-alias-wildcard-error-log-last-day
 TestSlave-1/etc/monitor-promise/check-_server-alias-wildcard-error-log-last-hour
+TestSlave-1/etc/monitor-promise/check-_server-alias_custom_domain-duplicated-error-log-last-day
+TestSlave-1/etc/monitor-promise/check-_server-alias_custom_domain-duplicated-error-log-last-hour
 TestSlave-1/etc/monitor-promise/check-_ssl-proxy-verify-unverified-error-log-last-day
 TestSlave-1/etc/monitor-promise/check-_ssl-proxy-verify-unverified-error-log-last-hour
 TestSlave-1/etc/monitor-promise/check-_ssl-proxy-verify_ssl_proxy_ca_crt-error-log-last-day
-- 
2.30.9