launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29406
[Merge] ~ilasc/content-cache-charm:enable-follow-redirects into content-cache-charm:master
Ioana Lasc has proposed merging ~ilasc/content-cache-charm:enable-follow-redirects into content-cache-charm:master.
Commit message:
Add ability to follow redirections
Requested reviews:
Canonical IS Reviewers (canonical-is-reviewers)
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~ilasc/content-cache-charm/+git/content-cache-charm/+merge/433376
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/content-cache-charm:enable-follow-redirects into content-cache-charm:master.
diff --git a/.gitignore b/.gitignore
index 3b821e7..a885ad6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,3 +13,4 @@ deps/
htmlcov/
repo-info
revision
+.idea/
diff --git a/config.yaml b/config.yaml
index cc17d36..0fecb0b 100644
--- a/config.yaml
+++ b/config.yaml
@@ -109,6 +109,16 @@ options:
Use the SO_REUSEPORT socket option, allowing a better load balance on the nginx workers.
Can result in performance drop due to increased latency on heavy workload.
See https://www.nginx.com/blog/socket-sharding-nginx-release-1-9-1/.
+ upstream_name:
+ default: ""
+ type: string
+ description: >
+ A name to use for the upstream server.
+ upstream_address:
+ default: ""
+ type: string
+ description: >
+ IP and port for the upstream server, e.g. 10.26.60.1:5000.
sites:
default: ""
type: string
diff --git a/lib/nginx.py b/lib/nginx.py
index 261597c..6007050 100644
--- a/lib/nginx.py
+++ b/lib/nginx.py
@@ -104,6 +104,8 @@ class NginxConf:
for location, loc_conf in locations.items():
conf[location] = deepcopy(loc_conf)
lc = conf[location]
+ if lc.get('proxy_intercept_errors'):
+ lc['proxy_intercept_errors'] = 'on'
backend_port = lc.get('backend_port')
if backend_port:
backend_path = lc.get('backend-path')
@@ -177,6 +179,8 @@ class NginxConf:
'reuseport': conf['reuseport'],
'site': conf['site'],
'site_name': conf['site_name'],
+ 'upstream_name': conf['upstream_name'],
+ 'upstream_address': conf['upstream_address'],
}
template = self.jinja_env.get_template('templates/nginx_cfg.tmpl')
return template.render(data)
diff --git a/reactive/content_cache.py b/reactive/content_cache.py
index 9e7e0ad..2ed420b 100644
--- a/reactive/content_cache.py
+++ b/reactive/content_cache.py
@@ -186,6 +186,9 @@ def configure_nginx(conf_path=None):
# access.
conf['listen_address'] = '127.0.0.1'
conf['reuseport'] = config['reuseport']
+ conf['upstream_name'] = config['upstream_name']
+ conf['upstream_address'] = config['upstream_address']
+
changed = False
for site, site_conf in sites.items():
conf['site'] = site
diff --git a/templates/nginx_cfg.tmpl b/templates/nginx_cfg.tmpl
index 0cda5be..a8bb066 100644
--- a/templates/nginx_cfg.tmpl
+++ b/templates/nginx_cfg.tmpl
@@ -1,5 +1,11 @@
proxy_cache_path {{cache_path}}/{{site}} use_temp_path=off levels=1:2 keys_zone={{keys_zone}}:10m {% if cache_inactive_time %}inactive={{cache_inactive_time}} {% endif %}max_size={{cache_max_size}};
+{%- if upstream_name %}
+upstream {{upstream_name}} {
+ server {{upstream_address}};
+}
+{% endif %}
+
server {
server_name {{site_name}};
listen {% if address %}{{address}}:{% endif %}{{port}}{% if reuseport %} reuseport{% endif %};
@@ -20,7 +26,8 @@ server {
location {% if conf['modifier'] %}{{conf['modifier']}} {% endif %}{{ location }} {
{%- if conf['backend'] %}
proxy_http_version 1.1;
- proxy_pass {{conf['backend']}};
+ {%- if not upstream_name %}
+ proxy_pass {{conf['backend']}};{% endif %}
proxy_set_header Host "{{site_name}}";
# Removed the following headers to avoid cache poisoning.
proxy_set_header Forwarded "";
@@ -43,6 +50,17 @@ server {
{%- endif %}
{%- endfor %}
proxy_force_ranges {{conf['force_ranges']}};
+{%- else %}
+{%- if upstream_name %}
+{%- if location == "@handle_redirects" %}
+ set $saved_redirect_location '$upstream_http_location';
+ proxy_pass {{conf['proxy_pass']}};
+{%- else %}
+ proxy_pass {{conf['proxy_pass']}};
+ proxy_intercept_errors {{conf['proxy_intercept_errors']}};
+ error_page 301 302 303 307 = @handle_redirects;
+{%- endif %}
+{%- endif %}
{%- endif %}
{%- if conf['origin-headers'] %}
diff --git a/tests/unit/files/config_nginx_redirect.txt b/tests/unit/files/config_nginx_redirect.txt
new file mode 100644
index 0000000..5c27eb0
--- /dev/null
+++ b/tests/unit/files/config_nginx_redirect.txt
@@ -0,0 +1,16 @@
+upstream lp_archive:
+ server:
+ - 10.26.60.1:5000
+
+site1.local:
+ port: 8085
+
+ locations:
+ '/':
+ proxy_pass: http://lp_archive/file
+ proxy_intercept_errors: 'on'
+ error_page:
+ - 301 302 303 307 = @handle_redirects
+ '@handle_redirects':
+ set $saved_redirect_location: '$upstream_http_location'
+ proxy_pass: $saved_redirect_location
diff --git a/tests/unit/files/config_nginx_redirect_rendered.txt b/tests/unit/files/config_nginx_redirect_rendered.txt
new file mode 100644
index 0000000..dce9879
--- /dev/null
+++ b/tests/unit/files/config_nginx_redirect_rendered.txt
@@ -0,0 +1,29 @@
+proxy_cache_path /var/lib/nginx/proxy/site1.local use_temp_path=off levels=1:2 keys_zone=a5586980e57a-cache:10m inactive=2h max_size=1g;
+upstream lp_archive {
+ server 10.26.60.1:5000;
+}
+
+
+server {
+ server_name site1.local;
+ listen 6081;
+
+ port_in_redirect off;
+ absolute_redirect off;
+
+ location / {
+ proxy_pass http://lp_archive/file;
+ proxy_intercept_errors on;
+ error_page 301 302 303 307 = @handle_redirects;
+ }
+
+
+ location @handle_redirects {
+ set $saved_redirect_location '$upstream_http_location';
+ proxy_pass $saved_redirect_location;
+ }
+
+
+ access_log /var/log/nginx/site1.local-access.log content_cache;
+ error_log /var/log/nginx/site1.local-error.log;
+}
diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
index 5f03e07..9ee7d2c 100644
--- a/tests/unit/test_content_cache.py
+++ b/tests/unit/test_content_cache.py
@@ -223,6 +223,8 @@ class TestCharm(unittest.TestCase):
'enable_cache_lock': True,
'enable_prometheus_metrics': False,
'reuseport': False,
+ 'upstream_name': '',
+ 'upstream_address': '',
'sites': ngx_config,
'worker_connections': 768,
'worker_processes': 0,
@@ -302,6 +304,8 @@ site1.local:
'cache_max_size': '1g',
'cache_path': '/var/lib/nginx/proxy',
'reuseport': False,
+ 'upstream_name': '',
+ 'upstream_address': '',
'sites': config,
'sites_secrets': secrets,
'worker_connections': 768,
@@ -352,6 +356,8 @@ site1.local:
'cache_max_size': '1g',
'cache_path': '/var/lib/nginx/proxy',
'reuseport': False,
+ 'upstream_name': '',
+ 'upstream_address': '',
'sites': config,
'worker_connections': 768,
'worker_processes': 0,
@@ -370,6 +376,8 @@ site1.local:
'cache_max_size': '20g',
'cache_path': '/srv/cache',
'reuseport': False,
+ 'upstream_name': '',
+ 'upstream_address': '',
'sites': config,
'worker_connections': 768,
'worker_processes': 0,
@@ -389,6 +397,8 @@ site1.local:
'cache_max_size': '',
'cache_path': '/srv/cache',
'reuseport': False,
+ 'upstream_name': '',
+ 'upstream_address': '',
'sites': config,
'worker_connections': 768,
'worker_processes': 0,
@@ -1422,6 +1432,8 @@ site1.local:
'enable_prometheus_metrics': True,
'metrics_listen_address': '127.0.0.2',
'reuseport': False,
+ 'upstream_name': '',
+ 'upstream_address': '',
'sites': ngx_config,
'worker_connections': 768,
'worker_processes': 0,
@@ -1540,6 +1552,8 @@ site1.local:
'enable_prometheus_metrics': True,
'metrics_listen_address': '127.0.0.2',
'reuseport': True,
+ 'upstream_name': '',
+ 'upstream_address': '',
'sites': ngx_config,
'worker_connections': 768,
'worker_processes': 0,
diff --git a/tests/unit/test_nginx.py b/tests/unit/test_nginx.py
index f51aaf6..0a39929 100644
--- a/tests/unit/test_nginx.py
+++ b/tests/unit/test_nginx.py
@@ -52,6 +52,9 @@ class TestLibNginx(unittest.TestCase):
conf['enable_prometheus_metrics'] = False
conf['listen_address'] = '127.0.0.1'
conf['reuseport'] = False
+ conf['upstream_name'] = ''
+ conf['upstream_address'] = ''
+
# From the given YAML-formatted list of sites, check that each individual
# Nginx config rendered matches what's in tests/unit/files.
port = BASE_LISTEN_PORT - 1
@@ -79,6 +82,50 @@ class TestLibNginx(unittest.TestCase):
self.assertEqual(ngx_conf.render(conf), output)
+ def test_nginx_config_redirect(self):
+ """Test parsing a YAML-formatted site for following redirect."""
+ ngx_conf = nginx.NginxConf(unit='mock-content-cache/0')
+
+ with open('tests/unit/files/config_nginx_redirect.txt', 'r', encoding='utf-8') as f:
+ sites = yaml.safe_load(f.read())
+ # 'configs' is special and used to host YAML anchors so let's remove it
+ sites.pop('configs', '')
+
+ conf = {}
+ conf['cache_path'] = '/var/lib/nginx/proxy'
+ conf['enable_prometheus_metrics'] = False
+ conf['listen_address'] = '127.0.0.1'
+ conf['reuseport'] = False
+ conf['upstream_name'] = 'lp_archive'
+ conf['upstream_address'] = '10.26.60.1:5000'
+
+ # From the given YAML-formatted list of sites, check that each individual
+ # Nginx config rendered matches what's in tests/unit/files.
+ port = BASE_LISTEN_PORT - 1
+ backend_port = BASE_BACKEND_PORT - 1
+ for site, site_conf in sites.items():
+ port += 1
+ conf['site'] = site
+ conf['site_name'] = site_conf.get('site-name') or site
+ conf['listen_port'] = port
+ conf['listen_address'] = site_conf.get('listen_address')
+ conf['upstream'] = site_conf.get('server', {})
+ conf['cache_inactive_time'] = site_conf.get('cache-inactive-time', '2h')
+ conf['cache_max_size'] = site_conf.get('cache-max-size', '1g')
+ conf['extra_configs'] = site_conf.get('extra-configs', [])
+ conf['locations'] = site_conf.get('locations', {})
+
+ for location, loc_conf in conf['locations'].items():
+ if loc_conf.get('backends'):
+ backend_port += 1
+ loc_conf['backend_port'] = backend_port
+
+ output_file = 'tests/unit/files/config_nginx_redirect_rendered.txt'
+ with open(output_file, 'r', encoding='utf-8') as f:
+ output = f.read()
+
+ self.assertEqual(ngx_conf.render(conf), output)
+
def test_nginx_config_write_sites(self):
"""Test writing out sites to individual Nginx site config files."""
ngx_conf = nginx.NginxConf(self.tmpdir)
@@ -143,6 +190,8 @@ class TestLibNginx(unittest.TestCase):
'enable_prometheus_metrics': True,
'listen_address': '127.0.0.1',
'reuseport': False,
+ 'upstream_name': '',
+ 'upstream_address': '',
}
for site, site_conf in sites.items():
conf['site'] = site
References
