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

[Martin Packman]

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?

[Aaron Bentley (abentley)]

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

review: Needs Fixing

[lp:~gz/charms/trusty/apache2-reverseproxy/gzip_encoded_extensions]

23. By Martin Packman on 2015-07-03

Address review by abentley

[Martin Packman (gz)]

Pushed up a change with the things you mentioned.

[Aaron Bentley (abentley)]

One small change requested inline.

review: Approve

[lp:~gz/charms/trusty/apache2-reverseproxy/gzip_encoded_extensions]

24. By Martin Packman on 2015-07-03

Use custom InvalidConfig as suggested by abentley in review

=== modified file 'README.md'
--- README.md	2014-09-23 19:44:39 +0000
+++ README.md	2015-07-03 19:36:09 +0000
@@ -41,3 +41,11 @@
     juju deploy cs:~abentley/trusty/apache2-reverseproxy data-proxy
     juju set data-proxy domain_name=data.org external_host=data.example.org:80
     juju add-relation data-proxy:extension apache2
+
+# Marking certain extensions as gzip encoded
+
+When proxying a site, the 'gzip_encoded_extensions' key can be set to add a
+Content-Encoding header to files with the given extensions. This can make it
+possible for user agents to display the content inline. For example:
+
+    juju set data-proxy gzip_encoded_extensions=".log.gz .txt.gz"
 
=== modified file 'config.yaml'
--- config.yaml	2014-10-14 13:04:52 +0000
+++ config.yaml	2015-07-03 19:36:09 +0000
@@ -29,3 +29,7 @@
     type: string
     default: "80"
     description: "Space-separated list of ports to proxy http on."
+  gzip_encoded_extensions:
+    type: string
+    default: ""
+    description: "Space-separated list of extensions that are gzip encoded."
 
=== modified file 'scripts/config-changed'
--- scripts/config-changed	2015-05-11 20:36:31 +0000
+++ scripts/config-changed	2015-07-03 19:36:09 +0000
@@ -2,6 +2,7 @@
 # Copyright 2014 Aaron Bentley
 import errno
 import os
+import re
 import urllib2
 
 import hookenv
@@ -36,6 +37,27 @@
       SSLCertificateFile /etc/ssl/certs/{ssl_certlocation}
       SSLCertificateKeyFile /etc/ssl/private/{ssl_keylocation}
 """
+extra_add_gzip_encoding_template = """\
+      <LocationMatch "{extension_re}">
+          SetEnv no-gzip
+          Header set Content-Encoding gzip
+      </LocationMatch>
+"""
+
+
+class InvalidConfig(Exception):
+    """Raise to indicate the given charm config was invalid."""
+
+    def __init__(self, key, value):
+        self.key = key
+        self.value = value
+
+    def __str__(self):
+        return "Invalid {} config value: {!r}".format(self.key, self.value)
+
+    def __repr__(self):
+        return "{}({!r}, {!r})".format(
+            self.__class__.__name, self.key, self.value)
 
 
 def get_hosts(config):
@@ -57,7 +79,7 @@
 
 
 def generate_vhost_config(domain, proxy_port, hosts, preserve_host, doc_root,
-                          key='', cert='', handle_503=False):
+                          key='', cert='', handle_503=False, gzip_exts=''):
     if preserve_host:
         preserve_host = 'On'
     else:
@@ -73,6 +95,12 @@
         proxy_proto = 'https'
         extra += https_extra_template.format(ssl_keylocation=key,
                                              ssl_certlocation=cert)
+    if gzip_exts:
+        if re.match(r'(?:\.\w+ ?)+$', gzip_exts) is None:
+            raise InvalidConfig("gzip_encoded_extensions", gzip_exts)
+        exts = gzip_exts.rstrip().split()
+        match = '.*({})'.format('|'.join(re.escape(s) for s in exts))
+        extra += extra_add_gzip_encoding_template.format(extension_re=match)
     return template.format(
         domain_name=domain, host=hosts[0][0], port=hosts[0][1],
         proxy_proto=proxy_proto, proxy_port=proxy_port,
@@ -80,7 +108,7 @@
 
 
 def generate_config(config, domain, hosts, http_ports, doc_root, handle_503,
-                    key, cert, use_ssl):
+                    key, cert, use_ssl, gzip_exts):
     if len(hosts) == 0:
         return ''
     preserve_host = bool(config.get('preserve_host'))
@@ -88,7 +116,7 @@
     for port in http_ports:
         site_configs.append(
             generate_vhost_config(domain, port, hosts, preserve_host, doc_root,
-            handle_503=handle_503)
+            handle_503=handle_503, gzip_exts=gzip_exts)
             )
     if use_ssl:
         site_configs.append(generate_vhost_config(
@@ -131,13 +159,19 @@
     key = config.get('ssl_keylocation', '')
     cert = config.get('ssl_certlocation', '')
     use_ssl = bool('' not in (key, cert))
+    gzip_exts = config.get('gzip_encoded_extensions', '')
     site_modules = ['proxy', 'proxy_http', 'headers']
     if use_ssl:
         site_modules.append('ssl')
         ports.append(443)
-    relation_settings['site_config'] = generate_config(
-        config, domain, hosts, http_ports, doc_root, handle_503, key, cert,
-        use_ssl)
+    try:
+        relation_settings['site_config'] = generate_config(
+            config, domain, hosts, http_ports, doc_root, handle_503, key, cert,
+            use_ssl, gzip_exts)
+    except InvalidConfig as e:
+        hookenv.log('Failed to generate site config: {}'.format(e),
+                    hookenv.ERROR)
+        relation_settings['enabled'] = False
     relation_settings['ports'] = ' '.join(str(p) for p in ports)
     relation_settings['site_modules'] = ' '.join(site_modules)
     for rid in hookenv.relation_ids("apache-website"):