Merge lp:~bac/juju-quickstart/auth into lp:juju-quickstart

[Brad Crittenden]

Get admin_secret from juju-generated jenv file.

As of Juju v 1.17 the admin-secret is stored in a generated file, located in
$JUJU_HOME/environments/<environment>.jenv.

If an admin-secret is provided in environments.yaml, it is used and also
stored in the jenv file. If none is given, then juju fabricates one and
stores it in the jenv file.

The admin-secret *field* is marked as optional now. For the case where a
local environment is used, Quickstart previously would generate an
admin-secret. That behavior is maintained by forcing 'admin-secret' to be in
the missing_fields list. This approach ensures backwards compatibility.

Note that the Juju-generated admin-secret is not available until after
bootstrapping the environment. Due to this behavior, when using juju v1.16
and not admin-secret is configured in the environments.yaml file, the failure
will not be reported until after bootstrapping. This approach seemed better
than introducing hard version checking.

https://codereview.appspot.com/57900043/

[Brad Crittenden (bac)]

Reviewers: mp+203631_code.launchpad.net,

Message:
Please take a look.

Description:
Get admin_secret from juju-generated jenv file.

As of Juju v 1.17 the admin-secret is stored in a generated file,
located in
$JUJU_HOME/environments/<environment>.jenv.

If an admin-secret is provided in environments.yaml, it is used and also
stored in the jenv file. If none is given, then juju fabricates one and
stores it in the jenv file.

The admin-secret *field* is marked as optional now. For the case where
a
local environment is used, Quickstart previously would generate an
admin-secret. That behavior is maintained by forcing 'admin-secret' to
be in
the missing_fields list. This approach ensures backwards compatibility.

Note that the Juju-generated admin-secret is not available until after
bootstrapping the environment. Due to this behavior, when using juju
v1.16
and not admin-secret is configured in the environments.yaml file, the
failure
will not be reported until after bootstrapping. This approach seemed
better
than introducing hard version checking.

https://code.launchpad.net/~bac/juju-quickstart/auth/+merge/203631

(do not edit description out of merge proposal)

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

Affected files (+174, -31 lines):
   A [revision details]
   M quickstart/manage.py
   M quickstart/models/envs.py
   M quickstart/tests/cli/test_views.py
   M quickstart/tests/models/test_envs.py
   M quickstart/tests/test_manage.py

[Francesco Banconi (frankban)]

This branch is nice Brad, thank you.
LGTM with minor changes and a less trivial suggestion I'd like you to
consider.

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py#newcode294
quickstart/manage.py:294: At bootstrap, juju (v1.17 and later) writes
the admin-secret to a
IIRC Jenv files are generated starting form v1.16.

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py#newcode309
quickstart/manage.py:309: raise ValueError('admin-secret not found in
{}.'.format(filename))
Please encode('utf-8') the ValueError message.

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py#newcode316
quickstart/manage.py:316: - admin_secret: the password to use to access
the Juju API;
... or None if the admin-secret field is not found in the environments
YAML file... or similar?

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py#newcode419
quickstart/manage.py:419: options.admin_secret = _get_admin_secret(
It seems _get_admin_secret can raise exceptions we never catch. I am
considering a different strategy for this, please let me know what you
think about the following:
We always try to parse the jenv file, even if we already have an
admin_secret. Subsequently, we make the following check:
- if the password is found in the jenv, we use that, done.
- if the password is not found in the jenv:
   - if we already have an admin secret, we use that;
   - othrwise we exit with a (nice) error.
As a side note, this can involve moving the _get_admin_secret function
to the app module.
AFAICT this way we achieve two goals:
- we are still backward compatible with old versions of Juju;
- we support a use case we don't currently handle (in trunk): the case
when a user has a jenv enabled juju-core, runs quickstart to bootstrap
an environment, then runs quickstart interactively to explicitly provide
an admin-secret, and then re-invoke quickstart again on the same
environment (say, in order to deploy a bundle or reopen the GUI). Now
the passwords from the environments file and from the jenv are not the
same, but since we made the latter override the former, everything works
well.
How does it sound?

Another, less important, suggestion follows.
Rather than modifying options, we might just define admin_secret in this
scope, e.g.:

admin_secret = options.admin_secret
if admin_secret is None:
    admin_secret = ...

And then s/options.admin_secret/admin_secret/ below.
This way we keep the (very weak) separation between setup (which sets up
options) and run (which just uses them). <shrug>

https://codereview.appspot.com/57900043/diff/1/quickstart/models/envs.py
File quickstart/models/envs.py (right):

https://codereview.appspot.com/57900043/diff/1/quickstart/models/envs.py#newcode177
quickstart/models/envs.py:177: contents = _load_file(env_file)
Very nice.

https://codereview.appspot.com/57900043/

[lp:~bac/juju-quickstart/auth]

51. By Brad Crittenden on 2014-01-29

Code reorg from review. Handle exceptions more appropriately.

52. By Brad Crittenden on 2014-01-29

lint

[Brad Crittenden (bac)]

Please take a look.

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py#newcode294
quickstart/manage.py:294: At bootstrap, juju (v1.17 and later) writes
the admin-secret to a
On 2014/01/29 11:48:40, frankban wrote:
> IIRC Jenv files are generated starting form v1.16.

Done.

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py#newcode309
quickstart/manage.py:309: raise ValueError('admin-secret not found in
{}.'.format(filename))
On 2014/01/29 11:48:40, frankban wrote:
> Please encode('utf-8') the ValueError message.

Done.

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py#newcode316
quickstart/manage.py:316: - admin_secret: the password to use to access
the Juju API;
On 2014/01/29 11:48:40, frankban wrote:
> ... or None if the admin-secret field is not found in the environments
YAML
> file... or similar?

Done.

https://codereview.appspot.com/57900043/diff/1/quickstart/manage.py#newcode419
quickstart/manage.py:419: options.admin_secret = _get_admin_secret(
Good idea. I've moved the definition of get_admin_secret to app.

https://codereview.appspot.com/57900043/

[Brad Crittenden (bac)]

*** Submitted:

Get admin_secret from juju-generated jenv file.

As of Juju v 1.17 the admin-secret is stored in a generated file,
located in
$JUJU_HOME/environments/<environment>.jenv.

If an admin-secret is provided in environments.yaml, it is used and also
stored in the jenv file. If none is given, then juju fabricates one and
stores it in the jenv file.

The admin-secret *field* is marked as optional now. For the case where
a
local environment is used, Quickstart previously would generate an
admin-secret. That behavior is maintained by forcing 'admin-secret' to
be in
the missing_fields list. This approach ensures backwards compatibility.

Note that the Juju-generated admin-secret is not available until after
bootstrapping the environment. Due to this behavior, when using juju
v1.16
and not admin-secret is configured in the environments.yaml file, the
failure
will not be reported until after bootstrapping. This approach seemed
better
than introducing hard version checking.

R=frankban
CC=
https://codereview.appspot.com/57900043

https://codereview.appspot.com/57900043/

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2014-01-17 15:25:22 +0000
+++ quickstart/app.py	2014-01-29 14:27:01 +0000
@@ -35,6 +35,7 @@
     utils,
     watchers,
 )
+from quickstart.models import envs
 
 
 class ProgramExit(Exception):
@@ -496,3 +497,25 @@
         env.deploy_bundle(bundle_yaml, name=bundle_name, bundle_id=bundle_id)
     except jujuclient.EnvError as err:
         raise ProgramExit('bad API server response: {}'.format(err.message))
+
+
+def get_admin_secret(env_name, juju_home):
+    """Read the admin-secret from the generated environment file.
+
+    At bootstrap, juju (v1.16 and later) writes the admin-secret to a
+    generated file located in JUJU_HOME.  Return the value.
+    Raise a ValueError if:
+        - the environment file is not found;
+        - the environment file contents are not parsable by YAML;
+        - the YAML contents are not properly structured;
+        - the admin-secret is not found.
+    """
+    filename = '{}.jenv'.format(env_name)
+    juju_env_file = os.path.expanduser(
+        os.path.join(juju_home, 'environments', filename))
+    jenv_db = envs.load_generated(juju_env_file)
+    try:
+        return jenv_db['admin-secret']
+    except KeyError:
+        msg = 'admin-secret not found in {}'.format(juju_env_file)
+        raise ValueError(msg.encode('utf-8'))

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2014-01-17 18:31:16 +0000
+++ quickstart/manage.py	2014-01-29 14:27:01 +0000
@@ -237,8 +237,10 @@
             '  - using "juju switch" to select the default environment;\n'
             '  - setting the default environment in {}.'.format(env_file)
         )
+
     # Retrieve the environment database (or create an in-memory empty one).
     env_db = _retrieve_env_db(parser, env_file if env_file_exists else None)
+
     # Validate the environment.
     env_type_db = envs.get_env_type_db()
     if interactive:
@@ -250,7 +252,7 @@
         # selected environment before proceeding.
         env_data = _retrieve_env_data(parser, env_type_db, env_db, env_name)
     # Update the options namespace with the new values.
-    options.admin_secret = env_data['admin-secret']
+    options.admin_secret = env_data.get('admin-secret')
     options.env_file = env_file
     options.env_name = env_data['name']
     options.env_type = env_data['type']
@@ -290,7 +292,8 @@
     """Set up the application options and logger.
 
     Return the options as a namespace containing the following attributes:
-        - admin_secret: the password to use to access the Juju API;
+        - admin_secret: the password to use to access the Juju API or None if
+          no admin-secret is present in the $JUJU_HOME/environment.yaml file;
         - bundle: the optional bundle (path or URL) to be deployed;
         - charm_url: the Juju GUI charm URL or None if not specified;
         - debug: whether debug mode is activated;
@@ -389,10 +392,22 @@
     already_bootstrapped, bsn_series = app.bootstrap(
         options.env_name, is_local=is_local, debug=options.debug)
 
+    # The admin secret was not in the environments file.  Retrieve it from
+    # the Juju-generated file.
+    # Find the Juju-generated environment file.
+    try:
+        admin_secret = app.get_admin_secret(
+            options.env_name, settings.JUJU_HOME)
+    except ValueError as err:
+        admin_secret = options.admin_secret
+        if admin_secret is None:
+            msg = err.message + ' or {}.'.format(options.env_file)
+            raise app.ProgramExit(msg.encode('utf-8'))
+
     print('retrieving the Juju API address')
     api_url = app.get_api_url(options.env_name)
     print('connecting to {}'.format(api_url))
-    env = app.connect(api_url, options.admin_secret)
+    env = app.connect(api_url, admin_secret)
 
     # It is not possible to deploy on the bootstrap node if we are using the
     # local provider, or if the bootstrap node series is not compatible with
@@ -406,10 +421,10 @@
     address = app.watch(env, unit_name)
     env.close()
     url = 'https://{}'.format(address)
-    print('url: {}\npassword: {}'.format(url, options.admin_secret))
+    print('url: {}\npassword: {}'.format(url, admin_secret))
     gui_api_url = 'wss://{}:443/ws'.format(address)
     print('connecting to the Juju GUI server')
-    gui_env = app.connect(gui_api_url, options.admin_secret)
+    gui_env = app.connect(gui_api_url, admin_secret)
 
     # Handle bundle deployment.
     if options.bundle is not None:

=== modified file 'quickstart/models/envs.py'
--- quickstart/models/envs.py	2014-01-20 12:02:10 +0000
+++ quickstart/models/envs.py	2014-01-29 14:27:01 +0000
@@ -134,6 +134,23 @@
     return {'environments': {}}
 
 
+def _load_file(env_file):
+    """Load the given file and return the YAML-parsed contents."""
+    # Load the Juju environments file.
+    try:
+        environments_file = open(env_file.encode('utf-8'))
+    except IOError as err:
+        msg = b'unable to open environments file: {}'.format(err)
+        raise ValueError(msg)
+    # Parse the Juju environments file.
+    try:
+        contents = serializers.yaml_load(environments_file)
+    except Exception as err:
+        msg = b'unable to parse environments file {}: {}'
+        raise ValueError(msg.format(env_file.encode('utf-8'), err))
+    return contents
+
+
 def load(env_file):
     """Load and parse the provided Juju environments.yaml file.
 
@@ -157,24 +174,13 @@
         - the environment file contents are not parsable by YAML;
         - the YAML contents are not properly structured.
     """
-    # Load the Juju environments file.
-    try:
-        environments_file = open(env_file.encode('utf-8'))
-    except IOError as err:
-        msg = b'unable to open environments file: {}'.format(err)
-        raise ValueError(msg)
-    # Parse the Juju environments file.
-    try:
-        contents = serializers.yaml_load(environments_file)
-    except Exception as err:
-        msg = b'unable to parse environments file {}: {}'
-        raise ValueError(msg.format(env_file.encode('utf-8'), err))
+    contents = _load_file(env_file)
     if contents is None:
         return create_empty_env_db()
     # Retrieve the environment list.
     try:
         env_contents = contents.get('environments', {}).items()
-    except AttributeError as err:
+    except AttributeError:
         msg = 'invalid YAML contents in {}: {}'.format(env_file, contents)
         raise ValueError(msg.encode('utf-8'))
     environments = {}
@@ -193,6 +199,18 @@
     return env_db
 
 
+def load_generated(env_file, section='bootstrap-config'):
+    """Given the path to a YAML file, load the file and return the section."""
+    contents = _load_file(env_file)
+    try:
+        section_contents = contents[section]
+    except (KeyError, TypeError):
+        msg = 'invalid YAML contents in {}: {}'.format(env_file, contents)
+        raise ValueError(msg.encode('utf-8'))
+
+    return section_contents
+
+
 def save(env_file, env_db, backup_function=None):
     """Save the given env_db to the provided environments.yaml file.
 
@@ -331,8 +349,12 @@
     # Retrieve a list of missing required fields.
     missing_fields = [
         field for field in env_metadata['fields']
-        if field.required and field.name not in env_data
-    ]
+        if field.required and field.name not in env_data]
+    # The following fields are not generally required but should be
+    # automatically generated for a local environment.
+    forced_to_be_generated = ('admin-secret', )
+    missing_fields.extend([field for field in env_metadata['fields']
+                           if field.name in forced_to_be_generated])
     # Assume all missing fields can be automatically generated.
     env_data.update((field.name, field.generate()) for field in missing_fields)
     return env_data
@@ -375,7 +397,7 @@
         'name', label='environment name', required=True,
         help='the environment name to use with Juju (arbitrary string)')
     admin_secret_field = fields.AutoGeneratedPasswordField(
-        'admin-secret', label='admin secret', required=True,
+        'admin-secret', label='admin secret', required=False,
         help='the password used to authenticate to the environment')
     default_series_field = fields.ChoiceField(
         'default-series', choices=settings.JUJU_DEFAULT_SERIES,
@@ -636,7 +658,7 @@
     then those keys are assigned a generic default field (fields.Field).
     """
     data = copy.deepcopy(env_data)
-    # Start building a list of (field, value) pairs preserving fields order.
+    # Start building a list of (field, value) pairs preserving field order.
     field_value_pairs = [
         (field, data.pop(field.name, None))
         for field in env_metadata['fields']

=== modified file 'quickstart/tests/cli/test_views.py'
--- quickstart/tests/cli/test_views.py	2014-01-16 11:17:42 +0000
+++ quickstart/tests/cli/test_views.py	2014-01-29 14:27:01 +0000
@@ -649,7 +649,7 @@
 
     def test_save_invalid_form_data(self):
         # Errors are displayed if the user tries to save invalid data.
-        changes = {'admin-secret': ''}
+        changes = {'name': ''}
         with self.patch_create_form(changes=changes) as mock_create_form:
             self.call_view(env_name='ec2-west')
             self.assertTrue(mock_create_form.called)
@@ -661,7 +661,7 @@
         # The form has been re-rendered passing the errors.
         self.assertEqual(2, mock_create_form.call_count)
         self.assertEqual(
-            {'admin-secret': 'a value is required for the admin secret field'},
+            {'name': 'a value is required for the environment name field'},
             mock_create_form.errors)
 
     def test_save_invalid_new_name(self):

=== modified file 'quickstart/tests/models/test_envs.py'
--- quickstart/tests/models/test_envs.py	2014-01-20 12:02:10 +0000
+++ quickstart/tests/models/test_envs.py	2014-01-29 14:27:01 +0000
@@ -98,27 +98,32 @@
         self.assertEqual({'environments': {}}, env_db)
 
 
-class TestLoad(
+class TestLoadFile(
         helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin,
         unittest.TestCase):
 
     def test_no_file(self):
         # A ValueError is raised if the environments file is not found.
         expected = (
-            'unable to open environments file: '
+            "unable to open environments file: "
             "[Errno 2] No such file or directory: '/no/such/file.yaml'"
         )
         with self.assert_value_error(expected):
-            envs.load('/no/such/file.yaml')
+            envs._load_file('/no/such/file.yaml')
 
     def test_invalid_yaml(self):
         # A ValueError is raised if the environments file is not a valid YAML.
         env_file = self.make_env_file(':')
         with self.assertRaises(ValueError) as context_manager:
-            envs.load(env_file)
+            envs._load_file(env_file)
         expected = 'unable to parse environments file {}'.format(env_file)
         self.assertIn(expected, bytes(context_manager.exception))
 
+
+class TestLoad(
+        helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin,
+        unittest.TestCase):
+
     def test_empty_file(self):
         # An empty environments database is returned if the file is empty.
         env_file = self.make_env_file('')
@@ -199,6 +204,60 @@
         self.assertEqual(expected, env_db)
 
 
+class TestLoadGenerated(
+        helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin,
+        unittest.TestCase):
+
+    def test_empty_file(self):
+        # A ValueError is raised if the environments file is empty.
+        env_file = self.make_env_file('')
+        expected = 'invalid YAML contents in {}: None'.format(env_file)
+        with self.assert_value_error(expected):
+            envs.load_generated(env_file)
+
+    def test_invalid_yaml_contents(self):
+        # A ValueError is raised if the environments file is not well formed.
+        env_file = self.make_env_file('a-string')
+        expected = 'invalid YAML contents in {}: a-string'.format(env_file)
+        with self.assert_value_error(expected):
+            envs.load_generated(env_file)
+
+    def test_section_not_found(self):
+        expected = {
+            'shoehorn-config': {
+                'admin-secret': 'Secret!',
+                'type': 'ec2'},
+        }
+        yaml_contents = expected.copy()
+        env_file = self.make_env_file(yaml.safe_dump(yaml_contents))
+        expected = 'invalid YAML contents in {}: {}'.format(
+            env_file, yaml_contents)
+        with self.assert_value_error(expected):
+            envs.load_generated(env_file)
+
+    def test_successful_default_section(self):
+        expected = {
+            'bootstrap-config': {
+                'admin-secret': 'Secret!',
+                'type': 'ec2'},
+        }
+        yaml_contents = expected.copy()
+        env_file = self.make_env_file(yaml.safe_dump(yaml_contents))
+        env_db = envs.load_generated(env_file)
+        self.assertEqual(expected['bootstrap-config'], env_db)
+
+    def test_successful_specified_section(self):
+        expected = {
+            'my-config': {
+                'admin-secret': 'Secret!',
+                'type': 'ec2'},
+        }
+        yaml_contents = expected.copy()
+        env_file = self.make_env_file(yaml.safe_dump(yaml_contents))
+        env_db = envs.load_generated(env_file, section='my-config')
+        self.assertEqual(expected['my-config'], env_db)
+
+
 class TestSave(helpers.EnvFileTestsMixin, unittest.TestCase):
 
     def setUp(self):
@@ -661,7 +720,7 @@
         env_metadata = self.env_type_db['__fallback__']
         expected = [
             'type', 'name', 'admin-secret', 'default-series', 'is-default']
-        expected_required = ['type', 'name', 'admin-secret', 'is-default']
+        expected_required = ['type', 'name', 'is-default']
         self.assert_fields(expected, env_metadata)
         self.assert_required_fields(expected_required, env_metadata)
 
@@ -673,7 +732,7 @@
             'type', 'name', 'admin-secret', 'default-series', 'root-dir',
             'storage-port', 'shared-storage-port', 'network-bridge',
             'is-default']
-        expected_required = ['type', 'name', 'admin-secret', 'is-default']
+        expected_required = ['type', 'name', 'is-default']
         self.assert_fields(expected, env_metadata)
         self.assert_required_fields(expected_required, env_metadata)
 
@@ -685,7 +744,7 @@
             'type', 'name', 'admin-secret', 'default-series', 'access-key',
             'secret-key', 'control-bucket', 'region', 'is-default']
         expected_required = [
-            'type', 'name', 'admin-secret', 'access-key', 'secret-key',
+            'type', 'name', 'access-key', 'secret-key',
             'control-bucket', 'is-default']
         self.assert_fields(expected, env_metadata)
         self.assert_required_fields(expected_required, env_metadata)
@@ -700,7 +759,7 @@
             'region', 'auth-mode', 'username', 'password', 'access-key',
             'secret-key', 'is-default']
         expected_required = [
-            'type', 'name', 'admin-secret', 'use-floating-ip',
+            'type', 'name', 'use-floating-ip',
             'control-bucket', 'auth-url', 'tenant-name', 'region',
             'is-default']
         self.assert_fields(expected, env_metadata)
@@ -715,7 +774,7 @@
             'management-subscription-id', 'management-certificate-path',
             'storage-account-name', 'is-default']
         expected_required = [
-            'type', 'name', 'admin-secret', 'location',
+            'type', 'name', 'location',
             'management-subscription-id', 'management-certificate-path',
             'storage-account-name', 'is-default']
         self.assert_fields(expected, env_metadata)

=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py	2014-01-17 14:05:03 +0000
+++ quickstart/tests/test_app.py	2014-01-29 14:27:01 +0000
@@ -1311,3 +1311,23 @@
         with self.assertRaises(ValueError) as context_manager:
             app.deploy_bundle(env, self.yaml, self.name, None)
         self.assertIs(error, context_manager.exception)
+
+
+class TestGetAdminSecret(unittest.TestCase):
+
+    def test_no_admin_secret(self):
+        with mock.patch('quickstart.manage.envs.load_generated',
+                        lambda x: {}):
+            with self.assertRaises(ValueError) as exc:
+                app.get_admin_secret('local', '/home/bac/.juju')
+        expected = (
+            u'admin-secret not found in '
+            '/home/bac/.juju/environments/local.jenv')
+        self.assertIn(expected, bytes(exc.exception))
+
+    def test_success(self):
+        expected = 'superchunk'
+        with mock.patch('quickstart.manage.envs.load_generated',
+                        lambda x: {'admin-secret': expected}):
+            secret = app.get_admin_secret('local', '~bac/.juju')
+        self.assertEqual(expected, secret)

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2014-01-13 15:46:27 +0000
+++ quickstart/tests/test_manage.py	2014-01-29 14:27:01 +0000
@@ -38,6 +38,7 @@
 from quickstart.cli import views
 from quickstart.models import envs
 from quickstart.tests import helpers
+from quickstart import app
 
 
 class TestDescriptionAction(unittest.TestCase):
@@ -393,8 +394,7 @@
             self.parser, self.env_type_db, self.env_db, 'local-with-errors')
         self.parser.error.assert_called_once_with(
             'cannot use the local-with-errors environment:\n'
-            'the storage port field requires an integer value\n'
-            'a value is required for the admin secret field')
+            'the storage port field requires an integer value')
 
 
 class TestSetupEnv(helpers.EnvFileTestsMixin, unittest.TestCase):
@@ -658,12 +658,24 @@
         options.update(kwargs)
         return mock.Mock(**options)
 
+    @staticmethod
+    def mock_get_admin_secret_success(name, home):
+        return 'jenv secret'
+
+    @staticmethod
+    def mock_get_admin_secret_error(name, home):
+        fn = '{}.jenv'.format(name)
+        path = os.path.join(home, 'environments', fn)
+        msg = 'admin-secret not found in {}'.format(path)
+        raise ValueError(msg.encode('utf-8'))
+
     def test_no_bundle(self, mock_app, mock_open):
         # The application runs correctly if no bundle is provided.
         token = 'AUTHTOKEN'
         mock_app.create_auth_token.return_value = token
         mock_app.watch.return_value = '1.2.3.4'
         mock_app.bootstrap.return_value = (True, 'precise')
+        mock_app.get_admin_secret = self.mock_get_admin_secret_error
         options = self.make_options()
         manage.run(options)
         mock_app.ensure_dependencies.assert_called()
@@ -722,3 +734,41 @@
         options = self.make_options(open_browser=False)
         manage.run(options)
         self.assertFalse(mock_open.called)
+
+    def test_admin_secret_fetched(self, mock_app, mock_open):
+        # If an admin secret is fetched from jenv it is used, even if one is
+        # found in environments.yaml, as set in options.admin_secret.
+        mock_app.get_admin_secret = self.mock_get_admin_secret_success
+        mock_app.bootstrap.return_value = (True, 'precise')
+        options = self.make_options(admin_secret='secret in environments.yaml')
+        manage.run(options)
+        mock_app.connect.assert_has_calls([
+            mock.call(mock_app.get_api_url(), 'jenv secret'),
+        ])
+
+    def test_admin_secret_from_environments_yaml(self, mock_app, mock_open):
+        # If an admin secret is not fetched from jenv, then the one from
+        # environments.yaml is used, as found in options.admin_secret.
+        mock_app.get_admin_secret = self.mock_get_admin_secret_error
+        mock_app.bootstrap.return_value = (True, 'precise')
+        options = self.make_options(admin_secret='secret in environments.yaml')
+        manage.run(options)
+        mock_app.connect.assert_has_calls([
+            mock.call(mock_app.get_api_url(), 'secret in environments.yaml'),
+        ])
+
+    def test_no_admin_secret_found(self, mock_app, mock_open):
+        # If admin-secret cannot be found anywhere a ProgramExit is called.
+        mock_app.ProgramExit = app.ProgramExit
+        mock_app.get_admin_secret = self.mock_get_admin_secret_error
+        mock_app.bootstrap.return_value = (True, 'precise')
+        options = self.make_options(
+            admin_secret=None,
+            env_name='local',
+            env_file='environments.yaml')
+        with self.assertRaises(app.ProgramExit) as context:
+            manage.run(options)
+        expected = (
+            u'admin-secret not found in ~/.juju/environments/local.jenv '
+            'or environments.yaml.')
+        self.assertEqual(expected, context.exception.message)