Merge lp:~gz/charms/trusty/apache2-reverseproxy/gzip_encoded_extensions into lp:~abentley/charms/trusty/apache2-reverseproxy/apache-website-interface

Proposed by Martin Packman
Status: Merged
Merged at revision: 22
Proposed branch: lp:~gz/charms/trusty/apache2-reverseproxy/gzip_encoded_extensions
Merge into: lp:~abentley/charms/trusty/apache2-reverseproxy/apache-website-interface
Diff against target: 130 lines (+52/-6)
3 files modified
README.md (+8/-0)
config.yaml (+4/-0)
scripts/config-changed (+40/-6)
To merge this branch: bzr merge lp:~gz/charms/trusty/apache2-reverseproxy/gzip_encoded_extensions
Reviewer Review Type Date Requested Status
Aaron Bentley Approve
Review via email: mp+261139@code.launchpad.net

Description of the change

Adds a new configuration option to serve gzip encoded files with the correct content-encoding header

Two main things of interest:

* We should arguably set the content encoding in S3. This branch does not preclude doing that as well. Unfortunately s3cmd doesn't make it easy, and we'd have to fix up the previous files. See:

<https://github.com/s3tools/s3cmd/issues/396>

Which also raises the other side, if we have Content-Encoding set in S3, we really want the reverse proxy to *always* set accept gzip, and handle the vary/not vary stuff locally - rather than passing through to S3 and letting them decode and transfer uncompressed data.

* Rather than juse using mod_mime AddEncoding, this branch takes a more convoluted approach so it's possible to limit on multiple extensions. This should address Aaron's previous concern as we can then configure just ".log.gz" to be marked, we we know is safe to be displayed in browser.

In the LocationMatch block, we're first disabling mod_deflate (which otherwise futzes with things), then just setting the Content-Encoding header. No Vary business going on, we'd prefer that browsers and other tools get the encoded version.

It would be nice to add some unit tests, we can maybe refactor a bit later?

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

Please add docs to the README. See also inline comments.

review: Needs Fixing
23. By Martin Packman

Address review by abentley

Revision history for this message
Martin Packman (gz) wrote :

Pushed up a change with the things you mentioned.

Revision history for this message
Aaron Bentley (abentley) wrote :

One small change requested inline.

review: Approve
24. By Martin Packman

Use custom InvalidConfig as suggested by abentley in review

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README.md'
2--- README.md 2014-09-23 19:44:39 +0000
3+++ README.md 2015-07-03 19:36:09 +0000
4@@ -41,3 +41,11 @@
5 juju deploy cs:~abentley/trusty/apache2-reverseproxy data-proxy
6 juju set data-proxy domain_name=data.org external_host=data.example.org:80
7 juju add-relation data-proxy:extension apache2
8+
9+# Marking certain extensions as gzip encoded
10+
11+When proxying a site, the 'gzip_encoded_extensions' key can be set to add a
12+Content-Encoding header to files with the given extensions. This can make it
13+possible for user agents to display the content inline. For example:
14+
15+ juju set data-proxy gzip_encoded_extensions=".log.gz .txt.gz"
16
17=== modified file 'config.yaml'
18--- config.yaml 2014-10-14 13:04:52 +0000
19+++ config.yaml 2015-07-03 19:36:09 +0000
20@@ -29,3 +29,7 @@
21 type: string
22 default: "80"
23 description: "Space-separated list of ports to proxy http on."
24+ gzip_encoded_extensions:
25+ type: string
26+ default: ""
27+ description: "Space-separated list of extensions that are gzip encoded."
28
29=== modified file 'scripts/config-changed'
30--- scripts/config-changed 2015-05-11 20:36:31 +0000
31+++ scripts/config-changed 2015-07-03 19:36:09 +0000
32@@ -2,6 +2,7 @@
33 # Copyright 2014 Aaron Bentley
34 import errno
35 import os
36+import re
37 import urllib2
38
39 import hookenv
40@@ -36,6 +37,27 @@
41 SSLCertificateFile /etc/ssl/certs/{ssl_certlocation}
42 SSLCertificateKeyFile /etc/ssl/private/{ssl_keylocation}
43 """
44+extra_add_gzip_encoding_template = """\
45+ <LocationMatch "{extension_re}">
46+ SetEnv no-gzip
47+ Header set Content-Encoding gzip
48+ </LocationMatch>
49+"""
50+
51+
52+class InvalidConfig(Exception):
53+ """Raise to indicate the given charm config was invalid."""
54+
55+ def __init__(self, key, value):
56+ self.key = key
57+ self.value = value
58+
59+ def __str__(self):
60+ return "Invalid {} config value: {!r}".format(self.key, self.value)
61+
62+ def __repr__(self):
63+ return "{}({!r}, {!r})".format(
64+ self.__class__.__name, self.key, self.value)
65
66
67 def get_hosts(config):
68@@ -57,7 +79,7 @@
69
70
71 def generate_vhost_config(domain, proxy_port, hosts, preserve_host, doc_root,
72- key='', cert='', handle_503=False):
73+ key='', cert='', handle_503=False, gzip_exts=''):
74 if preserve_host:
75 preserve_host = 'On'
76 else:
77@@ -73,6 +95,12 @@
78 proxy_proto = 'https'
79 extra += https_extra_template.format(ssl_keylocation=key,
80 ssl_certlocation=cert)
81+ if gzip_exts:
82+ if re.match(r'(?:\.\w+ ?)+$', gzip_exts) is None:
83+ raise InvalidConfig("gzip_encoded_extensions", gzip_exts)
84+ exts = gzip_exts.rstrip().split()
85+ match = '.*({})'.format('|'.join(re.escape(s) for s in exts))
86+ extra += extra_add_gzip_encoding_template.format(extension_re=match)
87 return template.format(
88 domain_name=domain, host=hosts[0][0], port=hosts[0][1],
89 proxy_proto=proxy_proto, proxy_port=proxy_port,
90@@ -80,7 +108,7 @@
91
92
93 def generate_config(config, domain, hosts, http_ports, doc_root, handle_503,
94- key, cert, use_ssl):
95+ key, cert, use_ssl, gzip_exts):
96 if len(hosts) == 0:
97 return ''
98 preserve_host = bool(config.get('preserve_host'))
99@@ -88,7 +116,7 @@
100 for port in http_ports:
101 site_configs.append(
102 generate_vhost_config(domain, port, hosts, preserve_host, doc_root,
103- handle_503=handle_503)
104+ handle_503=handle_503, gzip_exts=gzip_exts)
105 )
106 if use_ssl:
107 site_configs.append(generate_vhost_config(
108@@ -131,13 +159,19 @@
109 key = config.get('ssl_keylocation', '')
110 cert = config.get('ssl_certlocation', '')
111 use_ssl = bool('' not in (key, cert))
112+ gzip_exts = config.get('gzip_encoded_extensions', '')
113 site_modules = ['proxy', 'proxy_http', 'headers']
114 if use_ssl:
115 site_modules.append('ssl')
116 ports.append(443)
117- relation_settings['site_config'] = generate_config(
118- config, domain, hosts, http_ports, doc_root, handle_503, key, cert,
119- use_ssl)
120+ try:
121+ relation_settings['site_config'] = generate_config(
122+ config, domain, hosts, http_ports, doc_root, handle_503, key, cert,
123+ use_ssl, gzip_exts)
124+ except InvalidConfig as e:
125+ hookenv.log('Failed to generate site config: {}'.format(e),
126+ hookenv.ERROR)
127+ relation_settings['enabled'] = False
128 relation_settings['ports'] = ' '.join(str(p) for p in ports)
129 relation_settings['site_modules'] = ' '.join(site_modules)
130 for rid in hookenv.relation_ids("apache-website"):

Subscribers

People subscribed via source and target branches

to all changes: