Code review comment for lp:~teknico/charms/precise/juju-gui/encrypt-api-env-connection

Reviewers: mp+141107_code.launchpad.net,

Message:
Please take a look.

Description:
Setup encrypted conn. to the API environment

Pass the same certificate and private key used by nginx to the
API environment, so that the websocket connection can use WSS.

This sets the code up, but HTTPS is still disabled, and WSS too.
To test this you need to enable HTTPS in config/nginx.conf.template,
WSS in config/config.js.template, and expose the 443 port in
hooks/start.

Also, this is not yet working while deploying manually, and needs
further testing. Do not land without checking first.

https://code.launchpad.net/~teknico/charms/precise/juju-gui/encrypt-api-env-connection/+merge/141107

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7007045/

Affected files:
   M HACKING.md
   A [revision details]
   M config.yaml
   M config/juju-api-agent.conf.template
   M config/juju-api-improv.conf.template
   M config/nginx.conf.template
   M hooks/start
   M hooks/utils.py
   M revision
   M tests/test_utils.py

Index: HACKING.md
=== modified file 'HACKING.md'
--- HACKING.md 2012-12-19 15:27:53 +0000
+++ HACKING.md 2012-12-21 18:06:41 +0000
@@ -114,7 +114,7 @@
  this (again, assuming you have set up your repo the way the functional
tests
  need them, as described above).

- juju deploy --repository=/path/to/charm/repo local:precise/juju-gui
+ juju deploy --repository=/path/to/charm/repo --upgrade
local:precise/juju-gui
      juju expose juju-gui

  Now you are working with a test run, as described in

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: config.yaml
=== modified file 'config.yaml'
--- config.yaml 2012-12-20 14:56:29 +0000
+++ config.yaml 2012-12-21 18:06:41 +0000
@@ -50,4 +50,4 @@
      description: |
        The path to the directory where the SSL certificates are stored.
      type: string
- default: /etc/ssl/private/juju-gui
+ default: /etc/ssl/private/juju-gui/

Index: config/juju-api-agent.conf.template
=== modified file 'config/juju-api-agent.conf.template'
--- config/juju-api-agent.conf.template 2012-11-29 13:23:28 +0000
+++ config/juju-api-agent.conf.template 2012-12-21 15:12:03 +0000
@@ -10,4 +10,5 @@
  # Use --nodaemon so that upstart can correctly retrieve the process ID.
  exec /usr/bin/python -m juju.agents.api --nodaemon --port %(port)s \
      --logfile /var/log/juju/api-agent.log \
- --session-file /var/run/juju/api-agent.zksession
+ --session-file /var/run/juju/api-agent.zksession \
+ --keys %(keys)s

Index: config/juju-api-improv.conf.template
=== modified file 'config/juju-api-improv.conf.template'
--- config/juju-api-improv.conf.template 2012-12-03 10:02:45 +0000
+++ config/juju-api-improv.conf.template 2012-12-21 15:12:03 +0000
@@ -8,4 +8,5 @@
  env PYTHONPATH=%(juju_dir)s:$PYTHONPATH

  exec /usr/bin/python %(juju_dir)s/improv.py --port %(port)s \
- -f %(juju_dir)s/%(staging_env)s.json
+ -f %(juju_dir)s/%(staging_env)s.json \
+ --keys %(keys)s

Index: config/nginx.conf.template
=== modified file 'config/nginx.conf.template'
--- config/nginx.conf.template 2012-12-20 18:02:44 +0000
+++ config/nginx.conf.template 2012-12-21 15:12:03 +0000
@@ -13,8 +13,8 @@
      root %(server_root)s;
      index index.html;
      # Uncomment to switch back to TLS connections.
- # ssl_certificate /etc/ssl/private/juju-gui/server.pem;
- # ssl_certificate_key /etc/ssl/private/juju-gui/server.key;
+ # ssl_certificate /etc/ssl/private/juju-gui/juju.crt;
+ # ssl_certificate_key /etc/ssl/private/juju-gui/juju.key;

      # Serve static assets.
      location ^~ /juju-ui/ {

Index: revision
=== modified file 'revision'
--- revision 2012-12-20 10:52:39 +0000
+++ revision 2012-12-21 18:06:41 +0000
@@ -1,1 +1,1 @@
-17
+18

Index: hooks/start
=== modified file 'hooks/start'
--- hooks/start 2012-12-20 18:02:44 +0000
+++ hooks/start 2012-12-21 15:12:03 +0000
@@ -33,9 +33,10 @@
      staging = config.get('staging')
      start_gui(juju_api_port, config['juju-gui-console-enabled'], staging)
      if staging:
- start_improv(juju_api_port, config['staging-environment'])
+ start_improv(juju_api_port, ssl_cert_path=config['ssl-cert-path'],
+ config_path=config['staging-environment'])
      else:
- start_agent(juju_api_port)
+ start_agent(juju_api_port, ssl_cert_path=config['ssl-cert-path'])
      open_ports(juju_api_port)

Index: hooks/utils.py
=== modified file 'hooks/utils.py'
--- hooks/utils.py 2012-12-20 14:56:29 +0000
+++ hooks/utils.py 2012-12-21 15:12:03 +0000
@@ -27,7 +27,6 @@
  import json
  import os
  import logging
-import shutil
  import tempfile

  from launchpadlib.launchpad import Launchpad
@@ -178,6 +177,7 @@

  def start_improv(juju_api_port, staging_env,
+ ssl_cert_path='/etc/ssl/private/juju-gui/',
                   config_path='/etc/init/juju-api-improv.conf'):
      """Start a simulated juju environment using ``improv.py``."""
      log('Setting up staging start up script.')
@@ -185,6 +185,7 @@
          'juju_dir': JUJU_DIR,
          'port': juju_api_port,
          'staging_env': staging_env,
+ 'keys': ssl_cert_path,
      }
      render_to_file('juju-api-improv.conf.template', context, config_path)
      log('Starting the staging backend.')
@@ -192,7 +193,8 @@
          service_control(IMPROV, START)

-def start_agent(juju_api_port,
config_path='/etc/init/juju-api-agent.conf'):
+def start_agent(juju_api_port, ssl_cert_path='/etc/ssl/private/juju-gui/',
+ config_path='/etc/init/juju-api-agent.conf'):
      """Start the Juju agent and connect to the current environment."""
      # Retrieve the Zookeeper address from the start up script.
      unit_dir = os.path.realpath(os.path.join(CURRENT_DIR, '..'))
@@ -203,6 +205,7 @@
          'juju_dir': JUJU_DIR,
          'port': juju_api_port,
          'zookeeper': zookeeper,
+ 'keys': ssl_cert_path,
      }
      render_to_file('juju-api-agent.conf.template', context, config_path)
      log('Starting API agent.')
@@ -321,9 +324,9 @@
              run('ln', '-s', juju_gui_site,
                  '/etc/nginx/sites-enabled/juju-gui'))
      # Generate the nginx SSL certificates, if needed.
- pem_path = os.path.join(ssl_cert_path, 'server.pem')
- key_path = os.path.join(ssl_cert_path, 'server.key')
- if not (os.path.exists(pem_path) and os.path.exists(key_path)):
+ crt_path = os.path.join(ssl_cert_path, 'juju.crt')
+ key_path = os.path.join(ssl_cert_path, 'juju.key')
+ if not (os.path.exists(crt_path) and os.path.exists(key_path)):
          if not os.path.exists(ssl_cert_path):
              os.makedirs(ssl_cert_path)
          # See http://superuser.com/questions/226192/openssl-without-prompt
@@ -332,4 +335,4 @@
              '-days', '365', '-nodes', '-x509', '-subj',
              # These are arbitrary test values for the certificate.
              '/C=GB/ST=Juju/L=GUI/O=Ubuntu/CN=juju.ubuntu.com',
- '-keyout', key_path, '-out', pem_path))
+ '-keyout', key_path, '-out', crt_path))

Index: tests/test_utils.py
=== modified file 'tests/test_utils.py'
--- tests/test_utils.py 2012-12-20 13:27:30 +0000
+++ tests/test_utils.py 2012-12-21 15:12:03 +0000
@@ -348,6 +348,7 @@

          self.destination_file = tempfile.NamedTemporaryFile()
          self.addCleanup(self.destination_file.close)
+ self.ssl_cert_path = 'ssl/cert/path'

      def tearDown(self):
          # Undo all of the monkey patching.
@@ -358,20 +359,23 @@
      def test_start_improv(self):
          port = '1234'
          staging_env = 'large'
- start_improv(port, staging_env, self.destination_file.name)
+ start_improv(port, staging_env, self.ssl_cert_path,
+ self.destination_file.name)
          conf = self.destination_file.read()
          self.assertTrue('--port %s' % port in conf)
          self.assertTrue(staging_env + '.json' in conf)
+ self.assertTrue(self.ssl_cert_path in conf)
          self.assertEqual(self.svc_ctl_call_count, 1)
          self.assertEqual(self.service_names, ['juju-api-improv'])
          self.assertEqual(self.actions, [charmhelpers.START])

      def test_start_agent(self):
          port = '1234'
- start_agent(port, self.destination_file.name)
+ start_agent(port, self.ssl_cert_path, self.destination_file.name)
          conf = self.destination_file.read()
          self.assertTrue('--port %s' % port in conf)
          self.assertTrue('JUJU_ZOOKEEPER=%s' % self.fake_zk_address in conf)
+ self.assertTrue(self.ssl_cert_path in conf)
          self.assertEqual(self.svc_ctl_call_count, 1)
          self.assertEqual(self.service_names, ['juju-api-agent'])
          self.assertEqual(self.actions, [charmhelpers.START])