Merge "Adds optional host parameter to users."

reddwarfclient/cli.py

@@ -140,6 +140,7 @@ class UserCommands(common.AuthedCommandsBase):
               'id',
               'database',
               'databases',
+              'hostname',
               'name',
               'password',
              ]
@@ -149,19 +150,20 @@ class UserCommands(common.AuthedCommandsBase):
         self._require('id', 'name', 'password', 'databases')
         self._make_list('databases')
         databases = [{'name': dbname} for dbname in self.databases]
-        users = [{'name': self.name, 'password': self.password,
+        users = [{'name': self.name, 'host': self.hostname,
-                  'databases': databases}]
+                  'password': self.password, 'databases': databases}]
         self.dbaas.users.create(self.id, users)
 
     def delete(self):
         """Delete the specified user"""
         self._require('id', 'name')
-        self.dbaas.users.delete(self.id, self.name)
+        self.dbaas.users.delete(self.id, self.name, self.hostname)
 
     def get(self):
         """Get a single user."""
         self._require('id', 'name')
-        self._pretty_print(self.dbaas.users.get, self.id, self.name)
+        self._pretty_print(self.dbaas.users.get, self.id,
+                           self.name, self.hostname)
 
     def list(self):
         """List all the users for an instance"""
@@ -171,25 +173,29 @@ class UserCommands(common.AuthedCommandsBase):
     def access(self):
         """Show all databases the user has access to."""
         self._require('id', 'name')
-        self._pretty_list(self.dbaas.users.list_access, self.id, self.name)
+        self._pretty_list(self.dbaas.users.list_access, self.id,
+                          self.name, self.hostname)
 
     def grant(self):
         """Allow an existing user permissions to access one or more
         databases."""
         self._require('id', 'name', 'databases')
         self._make_list('databases')
-        self.dbaas.users.grant(self.id, self.name, self.databases)
+        self.dbaas.users.grant(self.id, self.name, self.databases,
+                               self.hostname)
 
     def revoke(self):
         """Revoke from an existing user access permissions to a database."""
         self._require('id', 'name', 'database')
-        self.dbaas.users.revoke(self.id, self.name, self.database)
+        self.dbaas.users.revoke(self.id, self.name, self.database,
+                                self.hostname)
 
     def change_password(self):
         """Change the password of a single user."""
         self._require('id', 'name', 'password')
         users = [{'name': self.name,
-                 'password': self.password}]
+                  'host': self.hostname,
+                  'password': self.password}]
         self.dbaas.users.change_passwords(self.id, users)
 
 

reddwarfclient/common.py

@@ -23,6 +23,8 @@ from reddwarfclient import client
 from reddwarfclient.xml import ReddwarfXmlClient
 from reddwarfclient import exceptions
 
+from urllib import quote
+
 
 def methods_of(obj):
     """Get all callable methods of an object that don't start with underscore
@@ -68,6 +70,15 @@ def limit_url(url, limit=None, marker=None):
     return url + query
 
 
+def quote_user_host(user, host):
+    quoted = ''
+    if host:
+        quoted = quote("%s@%s" % (user, host))
+    else:
+        quoted = quote("%s@%%" % user)
+    return quoted.replace('.', '%2e')
+
+
 class CliOptions(object):
     """A token object containing the user, apikey and token which
        is pickleable."""

reddwarfclient/users.py

@@ -18,9 +18,9 @@ from reddwarfclient import databases
 from reddwarfclient.common import check_for_exceptions
 from reddwarfclient.common import limit_url
 from reddwarfclient.common import Paginated
+from reddwarfclient.common import quote_user_host
 import exceptions
 import urlparse
-from urllib import quote
 
 
 class User(base.Resource):
@@ -46,8 +46,9 @@ class Users(base.ManagerWithFind):
         resp, body = self.api.client.post(url, body=body)
         check_for_exceptions(resp, body)
 
-    def delete(self, instance_id, user):
+    def delete(self, instance_id, username, hostname=None):
         """Delete an existing user in the specified instance"""
+        user = quote_user_host(username, hostname)
         url = "/instances/%s/users/%s" % (instance_id, user)
         resp, body = self.api.client.delete(url)
         check_for_exceptions(resp, body)
@@ -78,41 +79,41 @@ class Users(base.ManagerWithFind):
         return self._list("/instances/%s/users" % base.getid(instance),
                           "users", limit, marker)
 
-    def get(self, instance_id, user):
+    def get(self, instance_id, username, hostname=None):
         """
         Get a single User from the instance's Database.
 
         :rtype: :class:`User`.
         """
-        username = quote(user)
+        user = quote_user_host(username, hostname)
-        url = "/instances/%s/users/%s" % (instance_id, username)
+        url = "/instances/%s/users/%s" % (instance_id, user)
         return self._get(url, "user")
 
-    def list_access(self, instance, user):
+    def list_access(self, instance, username, hostname=None):
         """Show all databases the given user has access to. """
         instance_id = base.getid(instance)
-        username = quote(user)
+        user = quote_user_host(username, hostname)
-        url = "/instances/%(instance_id)s/users/%(username)s/databases"
+        url = "/instances/%(instance_id)s/users/%(user)s/databases"
         resp, body = self.api.client.get(url % locals())
         check_for_exceptions(resp, body)
         if not body:
             raise Exception("Call to %s did not return to a body" % url)
         return [databases.Database(self, db) for db in body['databases']]
 
-    def grant(self, instance, user, databases):
+    def grant(self, instance, username, databases, hostname=None):
         """Allow an existing user permissions to access a database."""
         instance_id = base.getid(instance)
-        username = quote(user)
+        user = quote_user_host(username, hostname)
-        url = "/instances/%(instance_id)s/users/%(username)s/databases"
+        url = "/instances/%(instance_id)s/users/%(user)s/databases"
         dbs = {'databases': [{'name': db} for db in databases]}
         resp, body = self.api.client.put(url % locals(), body=dbs)
         check_for_exceptions(resp, body)
 
-    def revoke(self, instance, user, database):
+    def revoke(self, instance, username, database, hostname=None):
         """Revoke from an existing user access permissions to a database."""
         instance_id = base.getid(instance)
-        username = quote(user)
+        user = quote_user_host(username, hostname)
-        url = ("/instances/%(instance_id)s/users/%(username)s/"
+        url = ("/instances/%(instance_id)s/users/%(user)s/"
                "databases/%(database)s")
         resp, body = self.api.client.delete(url % locals())
         check_for_exceptions(resp, body)

tests/test_users.py

@@ -56,20 +56,48 @@ class UsersTest(TestCase):
 
         return Mock(side_effect=side_effect_func)
 
+    def _build_fake_user(self, name, hostname=None, password=None,
+                         databases=None):
+        return {'name': name,
+                'password': password if password else 'password',
+                'host': hostname,
+                'databases': databases if databases else [],
+               }
+
     def test_create(self):
         self.users.api.client.post = self._get_mock_method()
         self._resp.status = 200
-        self.users.create(23, 'user1')
+        user = self._build_fake_user('user1')
+
+        self.users.create(23, [user])
         self.assertEqual('/instances/23/users', self._url)
-        self.assertEqual({"users": 'user1'}, self._body)
+        self.assertEqual({"users": [user]}, self._body)
+
+        # Even if host isn't supplied originally,
+        # the default is supplied.
+        del user['host']
+        self.users.create(23, [user])
+        self.assertEqual('/instances/23/users', self._url)
+        user['host'] = '%'
+        self.assertEqual({"users": [user]}, self._body)
+
+        # If host is supplied, of course it's put into the body.
+        user['host'] = '127.0.0.1'
+        self.users.create(23, [user])
+        self.assertEqual({"users": [user]}, self._body)
+
+        # Make sure that response of 400 is recognized as an error.
+        user['host'] = '%'
         self._resp.status = 400
-        self.assertRaises(Exception, self.users.create, 12, ['user1'])
+        self.assertRaises(Exception, self.users.create, 12, [user])
 
     def test_delete(self):
         self.users.api.client.delete = self._get_mock_method()
         self._resp.status = 200
         self.users.delete(27, 'user1')
-        self.assertEqual('/instances/27/users/user1', self._url)
+        # The client appends the host to remove ambiguity.
+        # urllib.unquote('%40%25') == '@%'
+        self.assertEqual('/instances/27/users/user1%40%25', self._url)
         self._resp.status = 400
         self.assertRaises(Exception, self.users.delete, 34, 'user1')