Merge lp:~frankban/charms/precise/juju-gui/use-env-name into lp:~juju-gui/charms/precise/juju-gui/trunk

[Francesco Banconi]

Add jenv file path to the GUI login help text.

Change the GUI login message to give exact
jenv file to look in for the admin secret, if
the environment name is available in the hook's
context.

Tests: `make unittest`.

QA:
1) bootstrap a stable juju-core;
2) deploy this branch with `make deploy`;
3) open the GUI: the login message should point
   you generically to the path where to find
   the jenv files;
4) destroy the environment;

5) bootstrap juju-core trunk (at least revno 2289);
6) deploy this branch with `make deploy`;
7) open the GUI: the login message should point you
   to the exact path to the jenv file;

8) now execute "juju set juju-gui login-help=HELP!",
   and wait a couple of seconds;
9) open the GUI again and you should see the new message;
10) destroy the environment.

Done, thank you!

https://codereview.appspot.com/59560044/

[Francesco Banconi (frankban)]

Reviewers: mp+204503_code.launchpad.net,

Message:
Please take a look.

Description:
Add jenv file path to the GUI login help text.

Change the GUI login message to give exact
jenv file to look in for the admin secret, if
the environment name is available in the hook's
context.

Tests: `make unittest`.

QA:
1) bootstrap a stable juju-core;
2) deploy this branch with `make deploy`;
3) open the GUI: the login message should point
    you generically to the path where to find
    the jenv files;
4) destroy the environment;

5) bootstrap juju-core trunk (at least revno 2289);
6) deploy this branch with `make deploy`;
7) open the GUI: the login message should point you
    to the exact path to the jenv file;

8) now execute "juju set juju-gui login-help=HELP!",
    and wait a couple of seconds;
9) open the GUI again and you should see the new message;
10) destroy the environment.

Done, thank you!

https://code.launchpad.net/~frankban/charms/precise/juju-gui/use-env-name/+merge/204503

(do not edit description out of merge proposal)

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

Affected files (+50, -6 lines):
   A [revision details]
   M config.yaml
   M hooks/backend.py
   M hooks/utils.py
   M revision
   M tests/test_utils.py

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 2014-01-16 19:25:52 +0000
+++ config.yaml 2014-02-03 14:02:24 +0000
@@ -74,11 +74,9 @@
      type: string
    login-help:
      description: |
- The help text shown to the user on the login screen.
+ The help text shown to the user on the login screen. If not
provided, a
+ default message is used, suggesting how to find the login
credentials.
      type: string
- default: |
- The password is the admin-secret from the Juju environment. This can
- often be found by looking in ~/.juju/environments.yaml.
    read-only:
      description: |
        Whether or not the GUI is in read-only mode. Note that read-only
mode is

Index: revision
=== modified file 'revision'
--- revision 2014-01-22 09:07:34 +0000
+++ revision 2014-02-03 14:02:24 +0000
@@ -1,1 +1,1 @@
-103
+104

Index: hooks/backend.py
=== modified file 'hooks/backend.py'
--- hooks/backend.py 2014-01-28 11:01:55 +0000
+++ hooks/backend.py 2014-02-03 14:02:24 +0000
@@ -127,7 +127,7 @@
          build_dir = utils.compute_build_dir(
              config['juju-gui-debug'], config['serve-tests'])
          utils.write_gui_config(
- config['juju-gui-console-enabled'], config['login-help'],
+ config['juju-gui-console-enabled'], config.get('login-help'),
              config['read-only'], config['charmworld-url'],
              build_dir, secure=config['secure'], sandbox=config['sandbox'],
              ga_key=config['ga-key'],

Index: hooks/utils.py
=== modified file 'hooks/utils.py'
--- hooks/utils....

[Gary Poster (gary)]

LGTM and QA OK with consideration of suggested text changes. Thank you!

Gary

https://codereview.appspot.com/59560044/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/59560044/diff/1/hooks/utils.py#newcode372
hooks/utils.py:372: 'and searching for the admin-secret
field.'.format(env_name))
per discussion, I suggest changing this to "...the password field."

We could add the following at the end:

"Note that using juju-quickstart (https://launchpad.net/juju-quickstart)
can automate logging in, as well as other parts of installing and
starting Juju."

It would be nice if that could be a separate paragraph, but I suspect
that requires changes in the GUI, and so making a separate paragraph
should be another branch.

https://codereview.appspot.com/59560044/diff/1/hooks/utils.py#newcode380
hooks/utils.py:380: 'environment, and searching for the admin-secret
field.')
Per our QA exploration, I suggest something like the following.

The password for newer Juju clients can be found by locating the Juju
environment file placed in ~/.juju/environments/ with the same name as
the current environment. For example, if you have an environment named
"production," then the file is named
~/.juju/environments/production.jenv. Look for the "password" field in
the file, or if that is empty, for the "admin-secret". Remove the
quotes from the value, and use this to log in. The password for older
Juju clients (< 1.16) is in ~/.juju/environments.yaml, and listed as the
admin-secret for the environment you are using. Note that using
juju-quickstart (https://launchpad.net/juju-quickstart) can automate
logging in, as well as other parts of installing and starting Juju.

Again, paragraphs would be nice, but that may be for another time.

https://codereview.appspot.com/59560044/

[lp:~frankban/charms/precise/juju-gui/use-env-name]

159. By Francesco Banconi on 2014-02-03

Changes as per review.

[Francesco Banconi (frankban)]

*** Submitted:

Add jenv file path to the GUI login help text.

Change the GUI login message to give exact
jenv file to look in for the admin secret, if
the environment name is available in the hook's
context.

Tests: `make unittest`.

QA:
1) bootstrap a stable juju-core;
2) deploy this branch with `make deploy`;
3) open the GUI: the login message should point
    you generically to the path where to find
    the jenv files;
4) destroy the environment;

5) bootstrap juju-core trunk (at least revno 2289);
6) deploy this branch with `make deploy`;
7) open the GUI: the login message should point you
    to the exact path to the jenv file;

8) now execute "juju set juju-gui login-help=HELP!",
    and wait a couple of seconds;
9) open the GUI again and you should see the new message;
10) destroy the environment.

Done, thank you!

R=gary.poster
CC=
https://codereview.appspot.com/59560044

https://codereview.appspot.com/59560044/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/59560044/diff/1/hooks/utils.py#newcode372
hooks/utils.py:372: 'and searching for the admin-secret
field.'.format(env_name))
On 2014/02/03 15:39:45, gary.poster wrote:
> per discussion, I suggest changing this to "...the password field."

> We could add the following at the end:

> "Note that using juju-quickstart
(https://launchpad.net/juju-quickstart) can
> automate logging in, as well as other parts of installing and starting
Juju."

> It would be nice if that could be a separate paragraph, but I suspect
that
> requires changes in the GUI, and so making a separate paragraph should
be
> another branch.

Done.

https://codereview.appspot.com/59560044/diff/1/hooks/utils.py#newcode380
hooks/utils.py:380: 'environment, and searching for the admin-secret
field.')
On 2014/02/03 15:39:45, gary.poster wrote:
> Per our QA exploration, I suggest something like the following.

> The password for newer Juju clients can be found by locating the Juju
> environment file placed in ~/.juju/environments/ with the same name as
the
> current environment. For example, if you have an environment named
> "production," then the file is named
~/.juju/environments/production.jenv. Look
> for the "password" field in the file, or if that is empty, for the
> "admin-secret". Remove the quotes from the value, and use this to log
in. The
> password for older Juju clients (< 1.16) is in
~/.juju/environments.yaml, and
> listed as the admin-secret for the environment you are using. Note
that using
> juju-quickstart (https://launchpad.net/juju-quickstart) can automate
logging in,
> as well as other parts of installing and starting Juju.

> Again, paragraphs would be nice, but that may be for another time.

Done.

https://codereview.appspot.com/59560044/

[Francesco Banconi (frankban)]

Thank you!

https://codereview.appspot.com/59560044/

=== modified file 'config.yaml'
--- config.yaml	2014-01-16 19:25:52 +0000
+++ config.yaml	2014-02-03 16:30:44 +0000
@@ -74,11 +74,9 @@
     type: string
   login-help:
     description: |
-      The help text shown to the user on the login screen.
+      The help text shown to the user on the login screen. If not provided, a
+      default message is used, suggesting how to find the login credentials.
     type: string
-    default: |
-      The password is the admin-secret from the Juju environment.  This can
-      often be found by looking in ~/.juju/environments.yaml.
   read-only:
     description: |
       Whether or not the GUI is in read-only mode. Note that read-only mode is

=== modified file 'hooks/backend.py'
--- hooks/backend.py	2014-01-28 11:01:55 +0000
+++ hooks/backend.py	2014-02-03 16:30:44 +0000
@@ -127,7 +127,7 @@
         build_dir = utils.compute_build_dir(
             config['juju-gui-debug'], config['serve-tests'])
         utils.write_gui_config(
-            config['juju-gui-console-enabled'], config['login-help'],
+            config['juju-gui-console-enabled'], config.get('login-help'),
             config['read-only'], config['charmworld-url'],
             build_dir, secure=config['secure'], sandbox=config['sandbox'],
             ga_key=config['ga-key'],

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2014-01-16 19:25:52 +0000
+++ hooks/utils.py	2014-02-03 16:30:44 +0000
@@ -362,6 +362,35 @@
     else:
         log('Running in insecure mode! Port 80 will serve unencrypted.')
         protocol = 'ws'
+    # Set up the help message displayed by the GUI login view.
+    if login_help is None:
+        env_name = os.getenv('JUJU_ENV_NAME')
+        if env_name:
+            login_help = (
+                'The password is the admin-secret from the Juju environment. '
+                'This can be found by looking in ~/.juju/environments/{}.jenv '
+                'and searching for the password field. Note that using '
+                'juju-quickstart (https://launchpad.net/juju-quickstart) can '
+                'automate logging in, as well as other parts of installing '
+                'and starting Juju.'.format(env_name))
+        else:
+            # The Juju environment name is included in the hooks context
+            # starting from juju-core v1.18.
+            login_help = (
+                'The password for newer Juju clients can be found by locating '
+                'the Juju environment file placed in ~/.juju/environments/ '
+                'with the same name as the current environment. For example, '
+                'if you have an environment named "production", then the file '
+                'is named ~/.juju/environments/production.jenv. Look for the '
+                '"password" field in the file, or if that is empty, for the '
+                '"admin-secret". Remove the quotes from the value, and use '
+                'this to log in. The password for older Juju clients (< 1.16) '
+                'is in ~/.juju/environments.yaml, and listed as the '
+                'admin-secret for the environment you are using. Note that '
+                'using juju-quickstart '
+                '(https://launchpad.net/juju-quickstart) can automate logging '
+                'in, as well as other parts of installing and starting Juju.')
+
     context = {
         'raw_protocol': protocol,
         'address': unit_get('public-address'),

=== modified file 'revision'
--- revision	2014-01-22 09:07:34 +0000
+++ revision	2014-02-03 16:30:44 +0000
@@ -1,1 +1,1 @@
-103
+104

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2014-01-16 19:25:52 +0000
+++ tests/test_utils.py	2014-02-03 16:30:44 +0000
@@ -961,6 +961,34 @@
         self.assertIn('user: "user-admin"', js_conf)
         self.assertIn('password: "kumquat"', js_conf)
 
+    def test_write_gui_config_help_with_env_name(self):
+        # The login help message refers to the specific jenv file is the Juju
+        # environment name is available.
+        with mock.patch('os.environ', {'JUJU_ENV_NAME': 'my-env'}):
+            write_gui_config(
+                False, None, True, True, self.charmworld_url, self.build_dir,
+                config_js_path='config',)
+        js_conf = self.files['config']
+        expected_help = (
+            'login_help: "The password is the admin-secret from the Juju '
+            'environment. This can be found by looking in '
+            '~/.juju/environments/my-env.jenv and searching for the '
+            'password field.')
+        self.assertIn(expected_help, js_conf)
+
+    def test_write_gui_config_help_without_env_name(self):
+        # The login help points to the path where to find the jenv files.
+        write_gui_config(
+            False, None, True, True, self.charmworld_url, self.build_dir,
+            config_js_path='config',)
+        js_conf = self.files['config']
+        expected_help = (
+            'login_help: "The password for newer Juju clients can be found by '
+            'locating the Juju environment file placed in '
+            '~/.juju/environments/ with the same name as the current '
+            'environment.')
+        self.assertIn(expected_help, js_conf)
+
     def test_setup_haproxy_config_insecure(self):
         setup_haproxy_config(self.ssl_cert_path, secure=False)
         # The insecure approach eliminates the https redirect.