Merge lp:~frankban/juju-deployer/guiserver-auth into lp:juju-deployer

[Francesco Banconi]

GUI environment: use both username and password.

Use both credentials to authenticate to the Juju environment: do not assume the user name to always be user-admin.

[j.c.sackett (jcsackett)]

LGTM, Francesco. Thanks!

review: Approve (code)

[Brad Crittenden (bac)]

LGTM

review: Approve (code)

[Tim Van Steenburgh (tvansteenburgh)]

I'm +1 on this idea but have a suggestion for the implementation. The proposed solution unnecessarily breaks backward compatibility by changing the order of args, and by making username a required arg. This could be avoided by maintaining the existing order of args, and adding username as a kwarg with default value 'user-admin'.

review: Needs Fixing

=== modified file 'deployer/env/gui.py'
--- deployer/env/gui.py	2014-09-03 17:21:51 +0000
+++ deployer/env/gui.py	2014-12-17 14:20:19 +0000
@@ -12,21 +12,14 @@
     """A Juju environment for the juju-deployer.
 
     Add support for deployments via the Juju API and for authenticating with
-    the provided password.
+    the provided credentials.
     """
 
-    def __init__(self, endpoint, password):
+    def __init__(self, endpoint, username, password):
         super(GUIEnvironment, self).__init__('gui', endpoint=endpoint)
+        self._username = username
         self._password = password
 
-    def _get_token(self):
-        """Return the stored password.
-
-        This method is overridden so that the juju-deployer does not try to
-        parse the environments.yaml file in order to retrieve the admin-secret.
-        """
-        return self._password
-
     def connect(self):
         """Connect the API client to the Juju backend.
 
@@ -35,7 +28,7 @@
         """
         if self.client is None:
             self.client = EnvironmentClient(self.api_endpoint)
-            self.client.login(self._password)
+            self.client.login(self._password, user=self._username)
 
     def close(self):
         """Close the API connection.

=== modified file 'deployer/guiserver.py'
--- deployer/guiserver.py	2014-02-20 18:13:57 +0000
+++ deployer/guiserver.py	2014-12-17 14:20:19 +0000
@@ -72,9 +72,9 @@
         raise ValueError(error)
 
 
-def validate(apiurl, password, bundle):
+def validate(apiurl, username, password, bundle):
     """Validate a bundle."""
-    env = GUIEnvironment(apiurl, password)
+    env = GUIEnvironment(apiurl, username, password)
     env.connect()
     try:
         _validate(env, bundle)
@@ -82,9 +82,9 @@
         env.close()
 
 
-def import_bundle(apiurl, password, name, bundle, options):
+def import_bundle(apiurl, username, password, name, bundle, options):
     """Import a bundle."""
-    env = GUIEnvironment(apiurl, password)
+    env = GUIEnvironment(apiurl, username, password)
     deployment = GUIDeployment(name, bundle)
     importer = Importer(env, deployment, options)
     env.connect()

=== modified file 'deployer/tests/test_guienv.py'
--- deployer/tests/test_guienv.py	2014-09-03 17:21:51 +0000
+++ deployer/tests/test_guienv.py	2014-12-17 14:20:19 +0000
@@ -12,17 +12,19 @@
 class TestGUIEnvironment(unittest.TestCase):
 
     endpoint = 'wss://api.example.com:17070'
+    username = 'who'
     password = 'Secret!'
 
     def setUp(self):
-        self.env = GUIEnvironment(self.endpoint, self.password)
+        self.env = GUIEnvironment(self.endpoint, self.username, self.password)
 
     def test_connect(self, mock_client):
         # The environment uses the provided endpoint and password to connect
         # to the Juju API server.
         self.env.connect()
         mock_client.assert_called_once_with(self.endpoint)
-        mock_client().login.assert_called_once_with(self.password)
+        mock_client().login.assert_called_once_with(
+            self.password, user=self.username)
 
     def test_multiple_connections(self, mock_client):
         # The environment does not attempt a second connection if it is already

=== modified file 'deployer/tests/test_guiserver.py'
--- deployer/tests/test_guiserver.py	2014-09-29 12:14:05 +0000
+++ deployer/tests/test_guiserver.py	2014-12-17 14:20:19 +0000
@@ -107,6 +107,7 @@
     """Base set up for the functions that make use of the juju-deployer."""
 
     apiurl = 'wss://api.example.com:17070'
+    username = 'who'
     password = 'Secret!'
     name = 'mybundle'
     bundle = yaml.safe_load("""
@@ -144,7 +145,8 @@
         services, the environment is instantiated, connected, env.status is
         called and then the connection is closed.
         """
-        mock_environment.assert_called_once_with(self.apiurl, self.password)
+        mock_environment.assert_called_once_with(
+            self.apiurl, self.username, self.password)
         mock_env_instance = mock_environment()
         mock_env_instance.connect.assert_called_once_with()
         mock_env_instance.status.assert_called_once_with()
@@ -177,7 +179,8 @@
 
     def test_validation(self, mock_environment):
         # The validation is correctly run.
-        guiserver.validate(self.apiurl, self.password, self.bundle)
+        guiserver.validate(
+            self.apiurl, self.username, self.password, self.bundle)
         # The environment is correctly instantiated and used.
         self.check_environment_life(mock_environment)
 
@@ -185,7 +188,8 @@
         # The validation fails if the bundle includes a service name already
         # present in the Juju environment.
         with self.assert_overlapping_services(mock_environment):
-            guiserver.validate(self.apiurl, self.password, self.bundle)
+            guiserver.validate(
+                self.apiurl, self.username, self.password, self.bundle)
 
 
 @unittest.skipIf(TEST_OFFLINE,
@@ -211,7 +215,8 @@
     def import_bundle(self):
         """Call the import_bundle function."""
         guiserver.import_bundle(
-            self.apiurl, self.password, self.name, self.bundle, self.options)
+            self.apiurl, self.username, self.password, self.name, self.bundle,
+            self.options)
 
     def cleanup_series_path(self):
         """Remove the series path created by the Deployment object."""
@@ -270,7 +275,8 @@
         mock_sleep.assert_has_calls([mock.call(5.1), mock.call(60)])
         # If any of the calls below fails, then we have to change the
         # signatures of deployer.guiserver.GUIEnvironment methods.
-        mock_environment.assert_called_once_with(self.apiurl, self.password)
+        mock_environment.assert_called_once_with(
+            self.apiurl, self.username, self.password)
         mock_environment().assert_has_calls([
             mock.call.connect(),
             mock.call.status(),
@@ -309,8 +315,8 @@
         with self.patch_juju_home():
             with self.assertRaises(guiserver.DeploymentError) as cm:
                 guiserver.import_bundle(
-                    self.apiurl, self.password, self.name, bundle,
-                    self.options)
+                    self.apiurl, self.username, self.password, self.name,
+                    bundle, self.options)
         expected_errors = set([
             'Invalid config charm cs:precise/wordpress-20 no-such-option=42',
             'Invalid config charm cs:precise/mysql-28 bad=wolf',