Merge lp:~dpb/charms/precise/apache2/trunk into lp:charms/apache2

[David Britton]

- Fill out README around the reverseproxy use case.
- some minor code-cleanup in hooks.py
- strip disallowed characters from the jinja2 template data
- Add in ssl_key, ssl_cert to allow passing in base64 encoded versions of these files
- Add in ability to generate a self-signed certificate for testing
- Default servername to the public-address of the unit (which juju in turn falls back to private-address)

[Brandon Holtsclaw (imbrandon)]

Changes all look good to me, I'd have like it if this was broken up into several MP of related changes in the future to make it easier to review but other than that Thanks!

review: Approve

[lp:~dpb/charms/precise/apache2/trunk]

35. By Brandon Holtsclaw on 2013-02-03

Merge of MP# 146242 - davidpbritton

=== modified file '.bzrignore'
--- .bzrignore	2012-10-26 02:41:40 +0000
+++ .bzrignore	2013-02-01 23:20:27 +0000
@@ -1,2 +1,4 @@
 revision
 basenode/
+*.crt
+*.key

=== renamed file 'README' => 'README.md'
--- README	2013-01-28 19:09:50 +0000
+++ README.md	2013-02-01 23:20:27 +0000
@@ -1,5 +1,5 @@
 Juju charm apache
-=====================
+=================
 
 The Apache Software Foundation's goal is to build a secure, efficient
 and extensible HTTP server as standards-compliant open source
@@ -9,46 +9,60 @@
 filtering, many flexible authentication schemes, and more.
 
 How to deploy the charm
---------------------------
-juju deploy apache2
-juju set apache2 "vhost_http_template=$(base64 < vhost.tmpl)"
-and or
-juju set apache2 "vhost_https_template=$(base64 < vhost.tmpl)"
+-----------------------
+    juju deploy apache2
+    juju set apache2 "vhost_http_template=$(base64 < vhost.tmpl)"
+    # and / or
+    juju set apache2 "vhost_https_template=$(base64 < vhost.tmpl)"
+    juju add-relation apache2:reverseproxy haproxy:website
 
 Vhost templates
---------------------------
+---------------
 The charm expects a jinja2 template to be passed in. The variables in
 the template should relate to the services that apache will be proxying
-to (obviously no variables need to be specified if no proxying is needed).
+-- obviously no variables need to be specified if no proxying is needed.
 
 The charm will create the service variable, with the unit_name, when
 the reverseproxy relationship is joined and present this to the template
-at which point the vhost will be generated from the template again.
+at which point the vhost will be generated from the template again.  
+All config settings are also available to the template.
 
-For example to access squid then the {{{ squid }}} variable should be used.
+For example to access squid then the {{ squid }} variable should be used.
 This will be populated with the hostname:port of the squid service. The
 individual hostname and port can also be accessed via squid_hostname and
 squid_port.
-Note: If an alias is used when deploying a charm then the alias name needs
-      to be used.
-
-If the joining charm also present an all_services variable which contains
-a list of services it provides in yaml format then variables for each
-service will be created of the format unitname_stanza. For example if
-haproxy contains stanzas named gunicorn and solr these can be accessed
-via {{{ haproxy_gunicorn }}} and {{ haproxy_solr }}}.
-Note: Currently the haproxy does not seem to be generating individual stanzas 
-      correctly
+
+Note: The service name should be used, not the charm name.  If deploying
+      a charm with a different service name, use that instaed.
+
+The joining charm may also set an all_services variable which contains
+a list of services it provides in yaml format (list of associative arrays):
+
+    # ... in haproxy charm, website-relation-joined
+    relation-set all_services="
+      - {service_name: gunicorn, service_port: 80}
+      - {service_name: solr, service_port: 8080}
+      - {service_name: my-webapp, service_port: 9090}
+    "
+
+then variables for each service would be available to the jinja2 template
+in <juju_service_name>_<sub_service_name>.  In our example above 
+haproxy contains stanzas named gunicorn, solr and my-webapp.  These are
+accessed as {{ haproxy_gunicorn }}, {{ haproxy_solr }} and
+{{ haproxy_mywebapp }} respectively.  If any unsupported characters are in
+your juju service name or the service names exposed through "all_services",
+they will be stripped.
 
 For example a vhost that will pass all traffic on to an haproxy instance:
-<VirtualHost *:80>
-	ServerName radiotiptop.org.uk
-
-	CustomLog /var/log/apache2/radiotiptop-access.log combined
-	ErrorLog /var/log/apache2/radiotiptop-error.log
-
+
+    <VirtualHost *:80>
+    	ServerName radiotiptop.org.uk
+    
+    	CustomLog /var/log/apache2/radiotiptop-access.log combined
+    	ErrorLog /var/log/apache2/radiotiptop-error.log
+    
         DocumentRoot /srv/radiotiptop/www/root
-
+    
         ProxyRequests off
         <Proxy *>
             Order Allow,Deny
@@ -58,33 +72,48 @@
             ErrorDocument 502 /offline.html
             ErrorDocument 503 /offline.html
         </Proxy>
-
+    
         ProxyPreserveHost off
         ProxyPassReverse / http://{{ haproxy_gunicorn }}/
-
+    
         RewriteEngine on
-
+    
         RewriteRule ^/$ /index.html [L]
         RewriteRule ^/(.*)$ http://{{ haproxy_gunicorn }}/$1 [P,L]
-
-</VirtualHost>
+    </VirtualHost>
 
 Certs, keys and chains
---------------------------
+----------------------
 ssl_keylocation, ssl_certlocation and ssl_chainlocation are file names in the 
-data directory.
+charm /data directory.  If found, they will be copied as follows:
+
+  - /etc/ssl/private/<ssl_keylocation>
+  - /etc/ssl/certs/<ssl_certlocation>
+  - /etc/ssl/certs/<ssl_chainlocation>
+
+ssl_key and ssl_cert can also be specified which are are assumed to be
+base64 encoded.  If specified, they will be written to appropriate directories
+given the values in ssl_keylocation and ssl_certlocation as listed above.
+
+ssl_cert may also be set to SELFSIGNED, which will generate a certificate.
+This, of course, is mostly useful for testing and staging purposes.  The
+generated certifcate/key will be placed according to ssl_certlocation and
+ssl_keylocation as listed above.
 
 {enable,disable}_modules
---------------------------
-Lists of modules to be enabled or disabled. If a module to be enabled cannot be found
-then the charm will attempt to install it.
-
+------------------------
+Space separated list of modules to be enabled or disabled. If a module to
+be enabled cannot be found then the charm will attempt to install it.
 
 TODO:
-* Method to deliver site content. This maybe by converting the charm to a subordinate
-  and making it the master charms problem
-* Delivery of SSL key. Implement secure method for delivering key.
-* Tuning. No tuning options are present. Convert apache2.conf to a template and expose
-  config options
-* Testing. I have only tested the relationship setup with 1 haproxy instance. Needs testing against
-  multiple instances 
+-----
+
+  * Document the use of balancer, nrpe, logging and website-cache
+  * Method to deliver site content. This maybe by converting the charm to a
+    subordinate and making it the master charms problem
+  * Implement secure method for delivering key.  Juju will likely need to provide
+    this.
+  * Tuning. No tuning options are present. Convert apache2.conf to a template
+    and expose config options
+  * Testing. I have only tested the relationship setup with 1 haproxy instance.
+    Needs testing against multiple instances 

=== modified file 'config.yaml'
--- config.yaml	2013-02-01 15:56:52 +0000
+++ config.yaml	2013-02-01 23:20:27 +0000
@@ -1,8 +1,8 @@
 options:
   servername:
     type: string
-    default: 'myvhost'
-    description: ServerName for vhost
+    default: ''
+    description: ServerName for vhost, defaults to the units public-address
   vhost_http_template:
     type: string
     default: ''
@@ -35,15 +35,28 @@
   ssl_keylocation:
     type: string
     default: ''
-    description: Location of ssl key
+    description: |
+        Name and location of ssl keyfile in charm/data directory.
+        If not found, will ignore.  Basename of this file will be used
+        as the basename of the key rooted at /etc/ssl/private.  Can
+        be used in conjuntion with the ssl_key parameter to specify
+        the key as a configuration setting.
   ssl_certlocation:
     type: string
     default: ''
-    description: Location of ssl cert
+    description: |
+        Name and location of ssl certificate in charm/data directory.
+        If not found, will ignore.  Basename of this file will be used
+        as the basename of the cert rooted at /etc/ssl/certs.  Can
+        be used in conjunction with the ssl_cert parameter to specify
+        the cert as a configuration setting.
   ssl_chainlocation:
     type: string
     default: ''
-    description: Location of ssl chain file
+    description: |
+        Name and location of the ssl chain file.  Basename of this file
+        will be used as the basename of the chain file rooted at
+        /etc/ssl/certs.
   lb_balancer_timeout:
     type: int
     default: 60
@@ -108,6 +121,24 @@
     description: >
       Use daily extension like YYYMMDD instead of simply adding a number
     default: True
+  use_rsyslog:
+    type: boolean
+    description: >-
+      Change logging behaviour to log both access and error logs via rsyslog
+    default: False
+  ssl_cert:
+    type: string
+    description: |
+        base64 encoded server certificate.  If the keyword 'SELFSIGNED'
+        is used, the certificate and key will be autogenerated as
+        self-signed.
+    default: ''
+  ssl_key:
+    type: string
+    description: |
+        base64 encoded server certificate key.  If ssl_cert is
+        specified as SELFSIGNED, this will be ignored.
+    default: ''
   server_tokens:
     type: string
     description: Security setting. Set to one of  Full  OS  Minimal  Minor  Major  Prod

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2013-02-01 15:56:52 +0000
+++ hooks/hooks.py	2013-02-01 23:20:27 +0000
@@ -41,6 +41,12 @@
 
 
 #------------------------------------------------------------------------------
+# juju_log:  Convenience wrapper around juju-log
+#------------------------------------------------------------------------------
+def juju_log(msg="MARK"):
+    subprocess.call(['juju-log', str(msg)])
+
+#------------------------------------------------------------------------------
 # open_port:  Convenience function to open a port in juju to
 #             expose a service
 #------------------------------------------------------------------------------
@@ -75,8 +81,12 @@
             config_cmd_line.append(scope)
         config_cmd_line.append('--format=json')
         config_data = json.loads(subprocess.check_output(config_cmd_line))
+        if not config_data["servername"]:
+            config_data["servername"] = run(
+                    ["unit-get", "public-address"]).rstrip()
+
     except Exception, e:
-        subprocess.call(['juju-log', str(e)])
+        juju_log(e)
         config_data = None
     finally:
         return(config_data)
@@ -101,7 +111,7 @@
         if unit_name is not None:
             relation_cmd_line.append('-')
             relation_cmd_line.append(unit_name)
-        subprocess.call(['juju-log', 'Calling: %s' % relation_cmd_line])
+        juju_log('Calling: %s' % relation_cmd_line)
         relation_data = json.loads(subprocess.check_output(relation_cmd_line))
     except Exception:
         relation_data = None
@@ -114,7 +124,7 @@
         relation_cmd_line = ['relation-ids', '--format=json']
         if relation_name is not None:
             relation_cmd_line.append(relation_name)
-        subprocess.call(['juju-log', 'Calling: %s' % relation_cmd_line])
+        juju_log('Calling: %s' % relation_cmd_line)
         relation_ids = json.loads(subprocess.check_output(relation_cmd_line))
     except Exception:
         relation_ids = None
@@ -127,7 +137,7 @@
         relation_cmd_line = ['relation-list', '--format=json']
         if relation_id is not None:
             relation_cmd_line.extend(('-r', relation_id))
-        subprocess.call(['juju-log', 'Calling: %s' % relation_cmd_line])
+        juju_log('Calling: %s' % relation_cmd_line)
         relations = json.loads(subprocess.check_output(relation_cmd_line))
     except Exception:
         relations = None
@@ -209,15 +219,12 @@
     if module is None:
         return(True)
     if os.path.exists("/etc/apache2/mods-enabled/%s.load" % (module)):
-        subprocess.call(['juju-log', "Module already loaded"])
+        juju_log("Module already loaded: %s" % module)
         return(True)
     if not os.path.exists("/etc/apache2/mods-available/%s.load" % (module)):
         retVal = apt_get_install("libapache2-mod-%s" % (module))
         if retVal != 0:
-            subprocess.call(
-                ['juju-log',
-                 "Installing module %s failed" % (module)
-                ])
+            juju_log("Installing module %s failed" % module)
             return(False)
     retVal = subprocess.call(['/usr/sbin/a2enmod', module])
     if retVal != 0:
@@ -231,7 +238,7 @@
     if module is None:
         return(True)
     if not os.path.exists("/etc/apache2/mods-enabled/%s.load" % (module)):
-        subprocess.call(['juju-log', "Module already disabled"])
+        juju_log("Module already disabled: %s" % module)
         return(True)
     retVal = subprocess.call(['/usr/sbin/a2dismod', module])
     if retVal != 0:
@@ -271,8 +278,14 @@
     for unit_name in relation_data.keys():
         if 'port' not in relation_data[unit_name]:
             return reverseproxy_data
+
+        # unit_name: <service-name>-<unit_number>
+        # jinja2 templates require python-type variables, remove all characters
+        # that do not comply
         unit_type = re.sub(r'(.*)-[0-9]*', r'\1', unit_name)
-        subprocess.call(['juju-log', 'unit_type: %s' % unit_type])
+        unit_type = re.sub('[^a-zA-Z0-9_]*', '', unit_type)
+        juju_log('unit_type: %s' % unit_type)
+
         host = relation_data[unit_name]['private-address']
         for config_setting in relation_data[unit_name].keys():
             config_key = '%s_%s' % (unit_type, config_setting)
@@ -293,7 +306,7 @@
     relation_data = all_relation_data_get(relation_name='balancer')
     config_data = config_get()
     if relation_data is None or len(relation_data) == 0:
-        subprocess.call(['juju-log', 'No relation data exiting'])
+        juju_log('No relation data exiting')
         return
     unit_dict = {}
     for unit_name in relation_data.keys():
@@ -320,6 +333,7 @@
         }
         template = template_env.get_template(
             'balancer.template').render(templ_vars)
+        juju_log("Writing file: %s with data: %s" % (balancer_host_file, templ_vars))
         with open(balancer_host_file, 'w') as balancer_config:
             balancer_config.write(str(template))
 
@@ -327,13 +341,13 @@
 def update_nrpe_checks():
     config_data = config_get()
     if 'nagios_check_http_params' not in config_data or len(config_data['nagios_check_http_params']) == 0:
-        subprocess.call(['juju-log', "No vhost check data, exiting"])
+        juju_log("No nrpe configuration, skipping")
         return
     try:
         nagios_uid = pwd.getpwnam('nagios').pw_uid
         nagios_gid = grp.getgrnam('nagios').gr_gid
     except:
-        subprocess.call(['juju-log', "Nagios user not setup, exiting"])
+        juju_log("Nagios user not setup, exiting")
         return
 
     unit_name = os.environ['JUJU_UNIT_NAME'].replace('/', '-')
@@ -347,8 +361,7 @@
         os.mkdir(nagios_logdir)
         os.chown(nagios_logdir, nagios_uid, nagios_gid)
     if not os.path.exists(nagios_exportdir):
-        subprocess.call(['juju-log', 'Exiting as %s is not accessible'
-                         % (nagios_exportdir)])
+        juju_log('Exiting as %s is not accessible' % nagios_exportdir)
         return
     for f in os.listdir(nagios_exportdir):
         if re.search('.*check_vhost.cfg', f):
@@ -454,6 +467,7 @@
     ports = {'http': 80, 'https': 443}
     for proto in ports.keys():
         template_var = 'vhost_%s_template' % (proto)
+        template_data = dict(config_data.items() + relationship_data.items())
         close_port(ports[proto])
         if template_var in config_data:
             vhost_name = '%s_%s' % (config_data['servername'], proto)
@@ -461,34 +475,59 @@
             from jinja2 import Template
             template = Template(
                 str(base64.b64decode(config_data[template_var])))
+            juju_log("Writing file: %s with data: %s" % (vhost_file, template_data))
             with open(vhost_file, 'w') as vhost:
-                vhost.write(
-                    str(template.render(
-                        dict(config_data.items() +
-                        relationship_data.items()))))
+                vhost.write(str(template.render(template_data)))
             open_port(ports[proto])
         subprocess.call(['/usr/sbin/a2ensite', vhost_name])
 
+    cert_file = None
     if config_data['ssl_certlocation']:
+        source = os.path.join(
+            os.environ['CHARM_DIR'], 'data', config_data['ssl_certlocation'])
         cert_file = '/etc/ssl/certs/%s' % \
             (config_data['ssl_certlocation'].rpartition('/')[2])
-        shutil.copy(os.path.join(os.environ['CHARM_DIR'], 'data',
-                                 config_data['ssl_certlocation']), cert_file)
+        if os.path.exists(source):
+            shutil.copy(source, cert_file)
+        else:
+            juju_log("Certificate not found, ignoring: %s" % source)
 
+    chain_file = None
     if config_data['ssl_chainlocation']:
         chain_file = '/etc/ssl/certs/%s' % \
             (config_data['ssl_chainlocation'].rpartition('/')[2])
         shutil.copy(os.path.join(os.environ['CHARM_DIR'], 'data',
                                  config_data['ssl_chainlocation']), chain_file)
 
+    key_file = None
     if config_data['ssl_keylocation']:
+        source = os.path.join(
+            os.environ['CHARM_DIR'], 'data', config_data['ssl_keylocation'])
         key_file = '/etc/ssl/private/%s' % \
             (config_data['ssl_keylocation'].rpartition('/')[2])
-        shutil.copy(os.path.join(os.environ['CHARM_DIR'], 'data',
-                                 config_data['ssl_keylocation']), key_file)
-        os.chmod(key_file, 0440)
-        os.chown(key_file, pwd.getpwnam('root').pw_uid,
-                 grp.getgrnam('ssl-cert').gr_gid)
+        if os.path.exists(source):
+            shutil.copy(source, key_file)
+            os.chmod(key_file, 0440)
+            os.chown(key_file,
+                pwd.getpwnam('root').pw_uid, grp.getgrnam('ssl-cert').gr_gid)
+        else:
+            juju_log("Key file not found, ignoring: %s" % source)
+
+    if config_data['ssl_cert'] and cert_file is not None:
+        if config_data['ssl_cert'] == "SELFSIGNED" and key_file is not None:
+            config_data['ssl_key'] = ""
+            gen_cert = os.path.join(os.environ['CHARM_DIR'], 'scripts',
+                    'gen-selfsigned-cert')
+            run([gen_cert, key_file, cert_file])
+        else:
+            juju_log("Writing cert from config ssl_cert: %s" % cert_file)
+            with open(cert_file, 'w') as f:
+                f.write(str(base64.b64decode(config_data['ssl_cert'])))
+
+    if config_data['ssl_key'] and key_file is not None:
+        juju_log("Writing key from config ssl_key: %s" % key_file)
+        with open(key_file, 'w') as f:
+            f.write(str(base64.b64decode(config_data['ssl_key'])))
 
     # Disable the default website because we don't want people to see the
     # "It works!" page on production services and remove the

=== modified file 'revision'
--- revision	2013-01-17 16:32:08 +0000
+++ revision	2013-02-01 23:20:27 +0000
@@ -1,1 +1,1 @@
-1
+2

=== added directory 'scripts'
=== added file 'scripts/gen-selfsigned-cert'
--- scripts/gen-selfsigned-cert	1970-01-01 00:00:00 +0000
+++ scripts/gen-selfsigned-cert	2013-02-01 23:20:27 +0000
@@ -0,0 +1,54 @@
+#!/bin/bash
+# 
+# Simple helper to generate a certificate
+# $1 = key location
+# $2 = certificate location
+# $3 = public-address (will use unit-get if unspecified)
+# $4 = private-address (will use unit-get if unspecified)
+
+KEY_FILE=${1:-key}
+CERT_FILE=${2:-cert}
+PUBLIC=${3:-$(unit-get public-address)}
+PRIVATE=${4:-$(unit-get private-address)}
+juju-log "network data: $PUBLIC $PRIVATE"
+juju-log "Generating cert: $CERT_FILE / key: $KEY_FILE"
+
+gen_certificate() {
+    CN=$PUBLIC
+    tmpfile=$(mktemp /tmp/XXXXXX) || exit 1
+    cat > $tmpfile <<EOF
+RANDFILE                = /dev/urandom
+
+[ req ]
+default_bits            = 1024
+default_keyfile         = privkey.pem
+distinguished_name      = req_distinguished_name
+prompt                  = no
+policy                  = policy_anything
+x509_extensions         = v3_ca
+
+[ req_distinguished_name ]
+commonName              = $CN
+
+[ v3_ca ]
+# Extensions to add to a certificate request
+subjectAltName          = @alt_names
+
+[alt_names]
+DNS.1   = $PUBLIC
+DNS.2   = $PRIVATE
+EOF
+    cat $tmpfile
+
+    openssl req \
+        -new -x509 -nodes -days 3650 \
+        -config $tmpfile \
+        -keyout $KEY_FILE
+    chmod 0440 $KEYFILE
+    chown root:ssl-cert $KEYFILE
+
+    rm -f $tmpfile
+}
+
+cert=$(gen_certificate)
+echo "$cert" > $CERT_FILE