Merge lp:~stub/charms/trusty/postgresql/unrestricted-admin into lp:charms/trusty/postgresql

[Stuart Bishop]

This was a regression in the recent rewrite.

Clients connected by a db-admin relation need to be able to connect as any valid user, not just the one they were provided with. This is used by charms such as pgbouncer.

[Review Queue (review-queue)]

The results (PASS) are in and available here: http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/1577/

review: Approve (automated testing)

[Review Queue (review-queue)]

The results (PASS) are in and available here: http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/1560/

review: Approve (automated testing)

[Chris Glass (tribaal)]

This was confirmed to work by relating the newest revision with the landscape server charm. The previous revision fails for the same relation.

+1

review: Approve

=== modified file 'hooks/service.py'
--- hooks/service.py	2015-11-19 21:14:49 +0000
+++ hooks/service.py	2015-11-27 07:59:08 +0000
@@ -390,13 +390,15 @@
                     postgresql.quote_identifier(addr),
                     'md5', '# {}'.format(relinfo))
 
-    # Admin clients need access to all databases as the relation users.
+    # Admin clients need access to all databases as any user, not just the
+    # relation user. Most clients will just use the user provided them,
+    # but proxies such as pgbouncer need to open connections as the accounts
+    # it creates.
     for rel in rels['db-admin'].values():
         if 'user' in rel.local:
             for relinfo in rel.values():
                 addr = postgresql.addr_to_range(relinfo['private-address'])
-                add('host', 'all',
-                    postgresql.quote_identifier(rel.local['user']),
+                add('host', 'all', 'all',
                     postgresql.quote_identifier(addr),
                     'md5', '# {}'.format(relinfo))
 

=== modified file 'tests/test_integration.py'
--- tests/test_integration.py	2015-11-02 12:12:14 +0000
+++ tests/test_integration.py	2015-11-27 07:59:08 +0000
@@ -203,7 +203,8 @@
         status = self.deployment.get_status()
         return set(status['services']['postgresql']['units'].keys())
 
-    def connect(self, unit=None, admin=False, database=None):
+    def connect(self, unit=None, admin=False, database=None,
+                user=None, password=None):
         '''
         A psycopg2 connection to a PostgreSQL unit via our client.
 
@@ -266,7 +267,8 @@
 
         return psycopg2.connect(
             port=local_port, host='localhost', database=database,
-            user=relinfo['user'], password=relinfo['password'])
+            user=user or relinfo['user'],
+            password=password or relinfo['password'])
 
     def test_db_relation(self):
         for unit in self.units:
@@ -287,7 +289,17 @@
                 # db-admin relations can connect to any database.
                 con = self.connect(unit, admin=True, database='postgres')
                 cur = con.cursor()
-                cur.execute('SELECT * FROM pg_stat_activity')
+                newuser = str(uuid.uuid1())
+                newpass = str(uuid.uuid1())
+                cur.execute("""CREATE USER "{}" SUPERUSER PASSWORD '{}'"""
+                            .format(newuser, newpass))
+                con.commit()
+
+                # db-admin relations can connect as any user to any database.
+                con = self.connect(unit, admin=True, database='postgres',
+                                   user=newuser, password=newpass)
+                cur = con.cursor()
+                cur.execute('select * from pg_stat_activity')
                 cur.fetchone()
 
     def test_admin_addresses(self):

=== modified file 'tests/test_pg_hba_conf.py'
--- tests/test_pg_hba_conf.py	2015-11-19 21:24:47 +0000
+++ tests/test_pg_hba_conf.py	2015-11-27 07:59:08 +0000
@@ -82,7 +82,7 @@
         rels = Relations()
         rels['db-admin'].add_unit('unit/1', local=({'user': 'user'}))
         content = generate_pg_hba_conf('', defaultdict(str), rels)
-        self.assertIn('host all "user" "1.2.3.4/32" md5', content)
+        self.assertIn('host all all "1.2.3.4/32" md5', content)
 
     def test_master_relation(self):
         rels = Relations()