Land with changes. This branch looks good Nicola: it's nice to see everything's working with TLS enabled. Tests pass. Please take a look at my comments below: especially the config-changed issue needs to be addressed IMHO.
https://codereview.appspot.com/7007045/diff/6002/HACKING.md File HACKING.md (right): https://codereview.appspot.com/7007045/diff/6002/HACKING.md#newcode128 HACKING.md:128: /var/lib/juju/units/[NAME OF UNIT]. There is a charm.log file to investigate, Good catch, thank you. https://codereview.appspot.com/7007045/diff/6002/hooks/config-changed File hooks/config-changed (right): https://codereview.appspot.com/7007045/diff/6002/hooks/config-changed#newcode88 hooks/config-changed:88: start_improv( The improv and the api agent services now depend on the certificates path, and need to be restarted if the ssl-cert-path is changed. AFAICT, currently this is not handled here. E.g., if the user run ``juju set juju-gui ssl-cert-path=/tmp/juju-gui``, the services are not restarted and the GUI ends up in a broken state. https://codereview.appspot.com/7007045/diff/6002/hooks/config-changed#newcode106 hooks/config-changed:106: start_agent( See above. https://codereview.appspot.com/7007045/diff/6002/hooks/utils.py File hooks/utils.py (right): https://codereview.appspot.com/7007045/diff/6002/hooks/utils.py#newcode59 hooks/utils.py:59: SSL_CERT_PATH = '/etc/ssl/juju-gui' Here the config default is redefined, and this value is used as default for the functions below needing the tls keys path. Is this really required? I believe that the default value is never used, i.e. each time we call start_improv, start_agent etc., we always pass the value taken from config. Moreover, it seems to me that if, in the future, we want to change the default path, we will need to change the same value in two different places. https://codereview.appspot.com/7007045/diff/6002/hooks/utils.py#newcode339 hooks/utils.py:339: crt_path = os.path.join(ssl_cert_path, 'juju.crt') I like how you renamed the keys. https://codereview.appspot.com/7007045/ -- https://code.launchpad.net/~teknico/charms/precise/juju-gui/encrypt-api-env-connection/+merge/141107 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~teknico/charms/precise/juju-gui/encrypt-api-env-connection into lp:~juju-gui/charms/precise/juju-gui/trunk. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
