Merge lp:~ahasenack/charms/precise/postgresql/nicer-fix-to-avoid-cnames-in-pg_hba into lp:charms/postgresql

[Andreas Hasenack]

Use a much cheaper call to gethostbyname() instead of calling out to "dig" to avoid using CNAMEs in pg_hba.conf.

Use a much cheaper call to gethostbyname() instead of calling out to "dig" to avoid using CNAMEs in pg_hba.conf.

[Stuart Bishop (stub)]

This is going to be a permanent fix as I understanding, as charms supoprting MaaS that need this sort of IP filtering (Most services? Few services?) will be required to do the CNAME dereferencing themselves. This will mean MaaS needs to guarantee that the CNAME will never be changed, and always point to the same A record. This still allows MaaS to change the IP addresses of machines, as the CNAME points to an A record and the IP address associated with that A record may be changed.

For a permanent fix, I don't think we should have the bare except in here. I think that if the DNS lookups fail, it is a good reason to have the hook fail.

This code isn't quite doing the same thing as the dig lookup. The dig lookup is only dereferencing CNAMES to an A record. The address this A record points to can change, and everything will still be happy. The new lookup deferences both CNAMES and A records to IP addresses. If a juju provider supports changing the IP address of a machine it could do so by giving a DNS name rather than an IP address as the private-address. It can then change where that name points to without needing to kick off all the -changed hooks, as the standard DNS TTL mechanism will sort things out. If a juju provider did this, it would break the proposed use of socket.gethostbyname to replace dig.

I'd vote to leave it like it is, and just remove the exception handler since the dig lookup seems stable. This is not performance critical.

review: Needs Information

[Andreas Hasenack (ahasenack)]

Apparently things changed overnight. Now the MAAS team is saying they are fixing the bug. Let's put this on hold.

[Stuart Bishop (stub)]

https://bugs.launchpad.net/juju-core/+bug/1215579 means my worries may be unfounded.

[Cory Johns (johnsca)]

Touching base on this, since it hasn't been updated in some time. The MAAS bug is now in Fixed Released, and the juju-core bug is In Progress.

My understanding is that this proposal is now essentially moot and we should be able to (now or soon) remove the workaround entirely. Is this correct? Should this MP be closed?

review: Needs Information

[Andreas Hasenack (ahasenack)]

MAAS 1.6 doesn't use CNAMEs anymore, this can be dropped.

[Andreas Hasenack (ahasenack)]

In fact, I should probably make a new MP to drop the current code that calls out to dig, but let me run that test case again first to be sure.

Unmerged revisions

86. By Andreas Hasenack on 2014-02-05

Since we are returning an IP this time, mask it with /32.

85. By Andreas Hasenack on 2014-02-05

Use gethostbyname instead of calling out to dig, much simpler/better.

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-02-03 20:56:40 +0000
+++ hooks/hooks.py	2014-02-05 20:52:52 +0000
@@ -476,17 +476,13 @@
             return "%s/32" % addr
         except socket.error:
             # It's not an IP address.
-            # XXX workaround for MAAS bug
-            # https://bugs.launchpad.net/maas/+bug/1250435
             # If it's a CNAME, use the A record it points to.
             # If it fails for some reason, return the original address
             try:
-                output = run("dig +short -t CNAME %s" % addr, True).strip()
+                remote_ip = socket.gethostbyname(addr)
             except:
                 return addr
-            if len(output) != 0:
-                return output.rstrip(".")  # trailing dot
-            return addr
+            return "%s/32" % remote_ip
 
     config_data = hookenv.config()
     allowed_units = set()