Merge lp:~rvb/maas/bug-1066938-rndc2 into lp:~maas-committers/maas/trunk

[Raphaël Badin]

This branch adds the inclusion of the default 'controls' statement so that the init scripts can control the bind server using the default RNDC key from localhost.

= Notes =

It turns out that if not controls statement is provided, "inet 127.0.0.1 port 953 allow { localhost; };" is included silently and this is used by the init scripts to control the bind server. Since MAAS adds a 'controls' statement to control the bind server, we also need to explicitly include the default 'controls' statement.

I've created a package locally and tested this fix:

Without the default 'controls' statement:

$ sudo /etc/init.d/bind9 restart
 * Stopping domain name service... bind9 rndc: connect failed: 127.0.0.1#953: connection refused
waiting for pid 14057 to die [ OK ]
 * Starting domain name service... bind9 [ OK ]

With the default 'controls' statement:

sudo /etc/init.d/bind9 restart
 * Stopping domain name service... bind9 waiting for pid 13819 to die [ OK ]
 * Starting domain name service... bind9 [ OK ]

Include the default 'controls' statement so that the init scripts can control the bind server using the default RNDC key from localhost.

[Gavin Panella (allenap)]

Looks good.

[1]

>  * Stopping domain name service... bind9 rndc: connect failed:
>    127.0.0.1#953: connection refused waiting for pid 14057 to die
>    [ OK ]

Do you know why it prints OK when it has failed?

[2]

+# Include the default RNDC controls (default RNDC key on port 953).
+DNS_DEFAULT_CONTROLS = True

Why is this in a Celery config file?

[3]

+DEFAULT_CONTROLS = """
+controls {
+    inet 127.0.0.1 port 953 allow { localhost; };
+};
+"""

Include a comment here, about what placed it, and why? Also, remove
leading new-line?

review: Approve

[Raphaël Badin (rvb)]

Thanks for the review!

> [1]
>
> >  * Stopping domain name service... bind9 rndc: connect failed:
> >    127.0.0.1#953: connection refused waiting for pid 14057 to die
> >    [ OK ]
>
> Do you know why it prints OK when it has failed?

Yeah, if stopping bind using a clean rndc command fails, the init script goes for the jugular: http://paste.ubuntu.com/1282827/

> [2]
>
> +# Include the default RNDC controls (default RNDC key on port 953).
> +DNS_DEFAULT_CONTROLS = True
>
> Why is this in a Celery config file?

Because all of this code is in the provisioningserver code. Basically all the DNS-related is in provisioningserver. Strickly speaking, this set-up stuff is not executed by a task though.

> [3]
>
> +DEFAULT_CONTROLS = """
> +controls {
> +    inet 127.0.0.1 port 953 allow { localhost; };
> +};
> +"""
>
> Include a comment here, about what placed it, and why? Also, remove
> leading new-line?

Added a comment, but I'd like to keep the leading new-line because this snippet is used with "content += snippet" and so the leading new-line is a safety net to avoid any change of fucking up the config file.

[Gavin Panella (allenap)]

> > [2]
> >
> > +# Include the default RNDC controls (default RNDC key on port 953).
> > +DNS_DEFAULT_CONTROLS = True
> >
> > Why is this in a Celery config file?
>
> Because all of this code is in the provisioningserver code.

Precisely. The provisioningserver code has a config file, pserv.yaml.
More configuration options drifting around in semi-related files makes
the software, as a whole, harder to control, to understand, to debug,
to document.

=== modified file 'etc/celeryconfig_common.py'
--- etc/celeryconfig_common.py	2012-10-05 16:33:37 +0000
+++ etc/celeryconfig_common.py	2012-10-16 10:48:29 +0000
@@ -27,6 +27,9 @@
 # server.
 DNS_RNDC_PORT = 954
 
+# Include the default RNDC controls (default RNDC key on port 953).
+DNS_DEFAULT_CONTROLS = True
+
 # DHCP leases file, as maintained by ISC dhcpd.
 DHCP_LEASES_FILE = '/var/lib/maas/dhcp/dhcpd.leases'
 

=== modified file 'etc/democeleryconfig_common.py'
--- etc/democeleryconfig_common.py	2012-09-28 13:37:39 +0000
+++ etc/democeleryconfig_common.py	2012-10-16 10:48:29 +0000
@@ -25,6 +25,11 @@
 DNS_RNDC_PORT = 9154
 
 
+# Do not include the default RNDC controls statement to avoid
+# a conflict while trying to listen on port 943.
+DNS_DEFAULT_CONTROLS = False
+
+
 DHCP_CONFIG_FILE = os.path.join(
     DEV_ROOT_DIRECTORY, 'run/dhcpd.conf')
 

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2012-10-16 09:31:20 +0000
+++ src/provisioningserver/dns/config.py	2012-10-16 10:48:29 +0000
@@ -57,7 +57,17 @@
     """Raised if there's a problem with a DNS config."""
 
 
-def generate_rndc(port=953, key_name='rndc-maas-key'):
+# Default 'controls' statement included in the configuration so that the
+# default RNDC can be used (by init scripts).
+DEFAULT_CONTROLS = """
+controls {
+    inet 127.0.0.1 port 953 allow { localhost; };
+};
+"""
+
+
+def generate_rndc(port=953, key_name='rndc-maas-key',
+                  include_default_controls=True):
     """Use `rndc-confgen` (from bind9utils) to generate a rndc+named
     configuration.
 
@@ -79,6 +89,8 @@
     named_start = rndc_content.index(start_marker) + len(start_marker)
     named_end = rndc_content.index(end_marker)
     named_conf = rndc_content[named_start:named_end].replace('\n# ', '\n')
+    if include_default_controls:
+        named_conf += DEFAULT_CONTROLS
     # Return a tuple of the two configurations.
     return rndc_content, named_conf
 
@@ -98,7 +110,8 @@
     conf.DNS_CONFIG_DIR.
     """
     rndc_content, named_content = generate_rndc(
-        conf.DNS_RNDC_PORT)
+        port=conf.DNS_RNDC_PORT,
+        include_default_controls=conf.DNS_DEFAULT_CONTROLS)
 
     target_file = get_rndc_conf_path()
     with open(target_file, "wb") as f:

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2012-10-16 04:05:58 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2012-10-16 10:48:29 +0000
@@ -35,6 +35,7 @@
     )
 from provisioningserver.dns import config
 from provisioningserver.dns.config import (
+    DEFAULT_CONTROLS,
     DNSConfig,
     DNSConfigDirectoryMissing,
     DNSConfigFail,
@@ -86,6 +87,16 @@
                 conf_content = stream.read()
                 self.assertIn(content, conf_content)
 
+    def test_rndc_config_includes_default_controls(self):
+        dns_conf_dir = self.make_dir()
+        self.patch(conf, 'DNS_CONFIG_DIR', dns_conf_dir)
+        self.patch(conf, 'DNS_DEFAULT_CONTROLS', True)
+        setup_rndc()
+        rndc_file = os.path.join(dns_conf_dir, MAAS_NAMED_RNDC_CONF_NAME)
+        with open(rndc_file, "rb") as stream:
+            conf_content = stream.read()
+            self.assertIn(DEFAULT_CONTROLS, conf_content)
+
     def test_execute_rndc_command_executes_command(self):
         recorder = FakeMethod()
         fake_dir = factory.getRandomString()