Merge lp:~mpontillo/maas/dns-template-changes-1.7 into lp:maas/1.7

[Mike Pontillo]

Allow users to define allow-* parameters in named.conf.options; prevent writing them to named.conf.options.inside.maas in that case. (Merge from trunk revision 4082)

Backport of https://code.launchpad.net/~mpontillo/maas/dns-template-changes-trunk/+merge/264216

[Mike Pontillo (mpontillo)]

Self-approving backport.

review: Approve

=== modified file 'etc/maas/templates/dns/named.conf.options.inside.maas.template'
--- etc/maas/templates/dns/named.conf.options.inside.maas.template	2015-07-03 08:57:15 +0000
+++ etc/maas/templates/dns/named.conf.options.inside.maas.template	2015-07-09 23:21:59 +0000
@@ -6,7 +6,13 @@
 };
 {{endif}}
 
+
+{{if not upstream_allow_query}}
 allow-query { any; };
+{{endif}}
+{{if not upstream_allow_recursion}}
 allow-recursion { trusted; };
+{{endif}}
+{{if not upstream_allow_query_cache}}
 allow-query-cache { trusted; };
-
+{{endif}}

=== modified file 'src/maasserver/management/commands/edit_named_options.py'
--- src/maasserver/management/commands/edit_named_options.py	2015-07-08 00:26:38 +0000
+++ src/maasserver/management/commands/edit_named_options.py	2015-07-09 23:21:59 +0000
@@ -33,7 +33,7 @@
     CommandError,
     )
 from maasserver.models import Config
-from maasserver.utils.isc import (
+from provisioningserver.utils.isc import (
     ISCParseException,
     make_isc_string,
     parse_isc_string,

=== modified file 'src/maasserver/tests/test_commands_edit_named_options.py'
--- src/maasserver/tests/test_commands_edit_named_options.py	2015-07-08 00:26:38 +0000
+++ src/maasserver/tests/test_commands_edit_named_options.py	2015-07-09 23:21:59 +0000
@@ -30,12 +30,12 @@
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils import get_one
-from maasserver.utils.isc import (
+from provisioningserver.dns.config import MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME
+from provisioningserver.utils.isc import (
     make_isc_string,
     parse_isc_string,
     read_isc_file,
     )
-from provisioningserver.dns.config import MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME
 from testtools.matchers import (
     Contains,
     Equals,

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2014-11-17 11:02:09 +0000
+++ src/provisioningserver/dns/config.py	2015-07-09 23:21:59 +0000
@@ -31,10 +31,12 @@
 
 from provisioningserver.utils import locate_config
 from provisioningserver.utils.fs import atomic_write
+from provisioningserver.utils.isc import read_isc_file
 from provisioningserver.utils.shell import call_and_check
 import tempita
 
 
+NAMED_CONF_OPTIONS = 'named.conf.options'
 MAAS_NAMED_CONF_NAME = 'named.conf.maas'
 MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME = 'named.conf.options.inside.maas'
 MAAS_NAMED_RNDC_CONF_NAME = 'named.conf.rndc.maas'
@@ -53,6 +55,18 @@
         return setting
 
 
+def get_bind_config_dir():
+    """Location of bind configuration files."""
+    setting = os.getenv(
+        "MAAS_BIND_CONFIG_DIR",
+        locate_config(os.path.pardir, "bind"))
+    if isinstance(setting, bytes):
+        fsenc = sys.getfilesystemencoding()
+        return setting.decode(fsenc)
+    else:
+        return setting
+
+
 def get_dns_rndc_port():
     """RNDC port to be configured by MAAS to communicate with BIND."""
     setting = os.getenv("MAAS_DNS_RNDC_PORT", "954")
@@ -192,6 +206,25 @@
     # specify it. If it's not set, the substitution will fail with the default
     # template that uses this value.
     kwargs.setdefault("upstream_dns")
+
+    # Parse the options file and make sure MAAS doesn't define any options
+    # that the user has already customized.
+    allow_user_override_options = [
+        "allow-query",
+        "allow-recursion",
+        "allow-query-cache",
+    ]
+
+    try:
+        parsed_options = read_isc_file(
+            compose_bind_config_path(NAMED_CONF_OPTIONS))
+    except IOError:
+        parsed_options = {}
+
+    options = parsed_options.get('options', {})
+    for option in allow_user_override_options:
+        kwargs['upstream_' + option.replace('-', '_')] = option in options
+
     try:
         rendered = template.substitute(kwargs)
     except NameError as error:
@@ -206,6 +239,11 @@
     return os.path.join(get_dns_config_dir(), filename)
 
 
+def compose_bind_config_path(filename):
+    """Return the full path for a DNS config or zone file."""
+    return os.path.join(get_bind_config_dir(), filename)
+
+
 def render_dns_template(template_name, *parameters):
     """Generate contents for a DNS configuration or zone file.
 

=== modified file 'src/provisioningserver/dns/testing.py'
--- src/provisioningserver/dns/testing.py	2014-09-24 09:32:18 +0000
+++ src/provisioningserver/dns/testing.py	2015-07-09 23:21:59 +0000
@@ -32,6 +32,8 @@
         config_dir = config_dir.encode(fsenc)
     testcase.useFixture(
         EnvironmentVariable(b"MAAS_DNS_CONFIG_DIR", config_dir))
+    testcase.useFixture(
+        EnvironmentVariable(b"MAAS_BIND_CONFIG_DIR", config_dir))
     return config_dir.decode(fsenc)
 
 

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2015-07-03 09:30:27 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2015-07-09 23:21:59 +0000
@@ -39,6 +39,7 @@
     MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME,
     MAAS_NAMED_RNDC_CONF_NAME,
     MAAS_RNDC_CONF_NAME,
+    NAMED_CONF_OPTIONS,
     render_dns_template,
     report_missing_config_dir,
     set_up_options_conf,
@@ -54,12 +55,16 @@
     DNSReverseZoneConfig,
     )
 from provisioningserver.utils import locate_config
+from provisioningserver.utils.isc import read_isc_file
 from testtools.matchers import (
+    AllMatch,
     Contains,
     ContainsAll,
     EndsWith,
+    Equals,
     FileContains,
     FileExists,
+    Is,
     IsInstance,
     MatchesAll,
     Not,
@@ -70,6 +75,47 @@
 from twisted.python.filepath import FilePath
 
 
+NAMED_CONF_OPTIONS_CONTENTS = dedent("""\
+    options {
+        forwarders {
+            8.8.8.8;
+            8.8.4.4;
+        };
+        dnssec-validation auto;
+        allow-query { any; };
+        allow-recursion { trusted; };
+        allow-query-cache { trusted; };
+        auth-nxdomain no;
+        listen-on-v6 { any; };
+    };
+    """)
+
+NAMED_CONF_OPTIONS_WITH_ALLOW_QUERY_CONTENTS = dedent("""\
+    options {
+        forwarders {
+            8.8.8.8;
+            8.8.4.4;
+        };
+        dnssec-validation auto;
+        allow-query { any; };
+        auth-nxdomain no;
+        listen-on-v6 { any; };
+    };
+    """)
+
+NAMED_CONF_OPTIONS_NO_ALLOW_CONTENTS = dedent("""\
+    options {
+        forwarders {
+            8.8.8.8;
+            8.8.4.4;
+        };
+        dnssec-validation auto;
+        auth-nxdomain no;
+        listen-on-v6 { any; };
+    };
+    """)
+
+
 class TestHelpers(MAASTestCase):
 
     def test_get_dns_config_dir_defaults_to_etc_bind_maas(self):
@@ -90,6 +136,24 @@
                 IsInstance(unicode),
             ))
 
+    def test_get_bind_config_dir_defaults_to_etc_bind_maas(self):
+        self.useFixture(EnvironmentVariable("MAAS_BIND_CONFIG_DIR"))
+        self.assertThat(
+            config.get_bind_config_dir(), MatchesAll(
+                SamePath(locate_config("../bind")),
+                IsInstance(unicode),
+            ))
+
+    def test_get_bind_config_dir_checks_environ_first(self):
+        directory = self.make_dir()
+        self.useFixture(EnvironmentVariable(
+            "MAAS_BIND_CONFIG_DIR", directory.encode("ascii")))
+        self.assertThat(
+            config.get_bind_config_dir(), MatchesAll(
+                SamePath(directory),
+                IsInstance(unicode),
+            ))
+
     def test_get_dns_root_port_defaults_to_954(self):
         self.useFixture(EnvironmentVariable("MAAS_DNS_RNDC_PORT"))
         self.assertEqual(954, config.get_dns_rndc_port())
@@ -145,6 +209,59 @@
             ])
         )
 
+    def test_set_up_options_conf_write_config_assumes_no_overrides(self):
+        dns_conf_dir = patch_dns_config_path(self)
+        set_up_options_conf()
+        target_file = os.path.join(
+            dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
+        target = read_isc_file(target_file)
+        self.assertThat([
+            target['allow-query']['any'],
+            target['allow-recursion']['trusted'],
+            target['allow-query-cache']['trusted'],
+        ], AllMatch(Equals(True)))
+
+    def test_set_up_options_conf_write_config_allows_overrides(self):
+        dns_conf_dir = patch_dns_config_path(self)
+        factory.make_file(
+            location=dns_conf_dir, name=NAMED_CONF_OPTIONS,
+            contents=NAMED_CONF_OPTIONS_CONTENTS)
+        set_up_options_conf()
+        target_file = os.path.join(
+            dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
+        target = read_isc_file(target_file)
+        self.assertThat([
+            target.get('allow-query'),
+            target.get('allow-recursion'),
+            target.get('allow-query-cache'),
+        ], AllMatch(Is(None)))
+
+    def test_set_up_options_conf_write_config_allows_zero_overrides(self):
+        dns_conf_dir = patch_dns_config_path(self)
+        factory.make_file(
+            location=dns_conf_dir, name=NAMED_CONF_OPTIONS,
+            contents=NAMED_CONF_OPTIONS_NO_ALLOW_CONTENTS)
+        set_up_options_conf()
+        target_file = os.path.join(
+            dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
+        target = read_isc_file(target_file)
+        self.assertThat([
+            target['allow-query']['any'],
+            target['allow-recursion']['trusted'],
+            target['allow-query-cache']['trusted'],
+        ], AllMatch(Equals(True)))
+
+    def test_set_up_options_conf_write_config_allows_single_override(self):
+        dns_conf_dir = patch_dns_config_path(self)
+        factory.make_file(
+            location=dns_conf_dir, name=NAMED_CONF_OPTIONS,
+            contents=NAMED_CONF_OPTIONS_WITH_ALLOW_QUERY_CONTENTS)
+        set_up_options_conf()
+        target_file = os.path.join(
+            dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
+        target = read_isc_file(target_file)
+        self.assertIsNone(target.get('allow-query'))
+
     def test_set_up_options_conf_handles_no_upstream_dns(self):
         dns_conf_dir = patch_dns_config_path(self)
         set_up_options_conf()

=== renamed file 'src/maasserver/utils/isc.py' => 'src/provisioningserver/utils/isc.py'
--- src/maasserver/utils/isc.py	2015-07-08 00:26:38 +0000
+++ src/provisioningserver/utils/isc.py	2015-07-09 23:21:59 +0000
@@ -144,6 +144,8 @@
             # If there are more than 1 'keywords' at new_char_list[index]
             # ex - "recursion no;"
             elif len(new_char_list[index].split()) >= 2:
+                if type(dictionary_fragment) == list:
+                    raise ISCParseException("Syntax error")
                 dictionary_fragment[
                     new_char_list[index].split()[0]] = (
                     ' '.join(new_char_list[index].split()[1:]))
@@ -155,7 +157,7 @@
             elif new_char_list[index] not in ['{', ';', '}']:
                 key = new_char_list[index]
                 if type(dictionary_fragment) == list:
-                    raise ISCParseException("Dictionary expected; got a list")
+                    raise ISCParseException("Syntax error")
                 dictionary_fragment[key] = ''
                 index += 1
             index += 1

=== renamed file 'src/maasserver/utils/tests/test_isc.py' => 'src/provisioningserver/utils/tests/test_isc.py'
--- src/maasserver/utils/tests/test_isc.py	2015-07-08 00:26:38 +0000
+++ src/provisioningserver/utils/tests/test_isc.py	2015-07-09 23:21:59 +0000
@@ -18,13 +18,13 @@
 from collections import OrderedDict
 from textwrap import dedent
 
-from maasserver.utils.isc import (
+from maastesting.testcase import MAASTestCase
+from provisioningserver.utils.isc import (
     ISCParseException,
     make_isc_string,
     parse_isc_string,
     read_isc_file,
     )
-from maastesting.testcase import MAASTestCase
 from testtools import ExpectedException
 
 
@@ -220,6 +220,10 @@
         with ExpectedException(ISCParseException):
             parse_isc_string("forwarders {{}a;;b}")
 
+    def test_parse_forgotten_semicolons_throw_iscparseexception(self):
+        with ExpectedException(ISCParseException):
+            parse_isc_string("a { b; } { c; } d e;")
+
     def test_read_isc_file(self):
         testdata = dedent("""\
             acl canonical-int-ns { 91.189.90.151; 91.189.89.192;  };