Merge ~barryprice/charm-k8s-bind/+git/charm-k8s-bind:master into charm-k8s-bind:master

[Barry Price]

Add a charm option to allow recursion by default, and streamline the wrapper script.

These changes backported from (and compatible with) the WIP sidecar branch.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Haw Loeung (hloeung)]

LGTM

Alternatively, you could use bind's `include` to include a file with the `allow-recursion` rather than patch and unpatch.

The file will be empty/emptied to disable.

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 215704a032cea6cd5de965483218d231066153c1

diff --git a/Dockerfile b/Dockerfile
index 94856fb..81bc645 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -21,8 +21,10 @@ RUN apt-get update && apt-get -y dist-upgrade \
 
 # wrapper script will configure Bind based on env variables and run it
 # dns-check script will provide a readinessProbe
+# recursion.patch will allow traffic from local (RFC1918) networks
 COPY ./image-scripts/docker-wrapper.sh /usr/local/bin/
 COPY ./image-scripts/dns-check.sh /usr/local/bin/
+COPY ./image-scripts/recursion.patch /usr/local/share/
 RUN chmod 0755 /usr/local/bin/docker-wrapper.sh
 RUN chmod 0755 /usr/local/bin/dns-check.sh
 
diff --git a/config.yaml b/config.yaml
index 214ab0c..fcad409 100644
--- a/config.yaml
+++ b/config.yaml
@@ -35,6 +35,13 @@ options:
       If unset, bind will be deployed with the package defaults.
       e.g. http://github.com/foo/my-custom-bind-config
     default: ""
+  enable_rfc1918_recursion:
+    type: boolean
+    description: |
+      Enable recursive queries from hosts on RFC1918 networks.
+
+      Ignored if custom_config_repo is set.
+    default: True
   https_proxy:
     type: string
     description: |
diff --git a/image-scripts/docker-wrapper.sh b/image-scripts/docker-wrapper.sh
index 4f807e1..97c4997 100644
--- a/image-scripts/docker-wrapper.sh
+++ b/image-scripts/docker-wrapper.sh
@@ -1,25 +1,34 @@
 #!/bin/bash
+
 set -eu
 
+echo "Fresh wrapper run at $(date)";
+
 if [ -z "${BIND_CONFDIR-}" ]; then
-	# If BIND_CONFDIR wasn't set, use the package default
-	BIND_CONFDIR="/etc/bind";
+    # If BIND_CONFDIR wasn't set, use the package default
+    BIND_CONFDIR="/etc/bind";
 fi
 
 if [ -z "${CUSTOM_CONFIG_REPO-}" ]; then
-	echo "No custom repo set, will fall back to package default config";
+    echo "No custom repo set, will fall back to package default config";
+    if [ -z "${ENABLE_RFC1918_RECURSION-}" ]; then
+        echo "RFC1918 network recursion disabled, using stock config";
+        apt-get install --reinstall -o Dpkg::Options::="--force-confask,confnew,confmiss" bind9
+    else
+        echo "Enabling RFC1918 network recursion";
+        patch -p0 -b < /usr/local/share/recursion.patch -d /
+    fi
 else
-	echo "Pulling config from $CUSTOM_CONFIG_REPO";
-	if [ -d "${BIND_CONFDIR}" ]; then
-		mv "${BIND_CONFDIR}" "${BIND_CONFDIR}_$(date +"%Y-%m-%d_%H-%M-%S")";
-	fi
-	git clone "$CUSTOM_CONFIG_REPO" "$BIND_CONFDIR";
+    if [ -d "${BIND_CONFDIR}" ]; then
+        echo "Backing up old config";
+        mv "${BIND_CONFDIR}" "${BIND_CONFDIR}_$(date +"%Y-%m-%d_%H-%M-%S")";
+    fi
+    echo "Pulling config from $CUSTOM_CONFIG_REPO";
+    git clone "$CUSTOM_CONFIG_REPO" "$BIND_CONFDIR";
 fi
 
-if [ -d "${BIND_CONFDIR}" ]; then
-	exec "$@"
-else
-	echo "Something went wrong, ${BIND_CONFDIR} does not exist, not starting";
+if [ ! -d "${BIND_CONFDIR}" ]; then
+    echo "Something went wrong, ${BIND_CONFDIR} does not exist, not starting";
 fi
 
 /usr/sbin/named -g -u bind -c /etc/bind/named.conf
diff --git a/image-scripts/recursion.patch b/image-scripts/recursion.patch
new file mode 100644
index 0000000..7af8b38
--- /dev/null
+++ b/image-scripts/recursion.patch
@@ -0,0 +1,8 @@
+--- /etc/bind/named.conf.options.orig	2021-05-28 07:45:55.746342836 +0000
++++ /etc/bind/named.conf.options	2021-05-28 07:48:07.884023912 +0000
+@@ -21,4 +21,5 @@
+ 	dnssec-validation auto;
+
+ 	listen-on-v6 { any; };
++	allow-recursion { 127.0.0.1; 10.0.0.0/8; 172.16.0.0/12; 192.168.0.0/16; };
+ };
diff --git a/src/charm.py b/src/charm.py
index 14198c7..964922c 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -85,6 +85,9 @@ class BindK8sCharm(CharmBase):
         if config["custom_config_repo"].strip():
             pod_config["CUSTOM_CONFIG_REPO"] = config["custom_config_repo"]
 
+        if config["enable_rfc1918_recursion"]:
+            pod_config["ENABLE_RFC1918_RECURSION"] = 1
+
         if config["https_proxy"].strip():
             pod_config["http_proxy"] = config["https_proxy"]
             pod_config["https_proxy"] = config["https_proxy"]
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 3938f12..c9834d1 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -38,6 +38,17 @@ CONFIG_VALID = {
     'https_proxy': '',
 }
 
+CONFIG_VALID_WITHOUT_RECURSION = {
+    'bind_image_path': 'example.com/bind:v1',
+    'bind_image_username': '',
+    'bind_image_password': '',
+    'enable_rfc1918_recursion': False,
+    'container_config': '',
+    'container_secrets': '',
+    'custom_config_repo': '',
+    'https_proxy': '',
+}
+
 CONFIG_VALID_WITH_CREDS = {
     'bind_image_path': 'secure.example.com/bind:v1',
     'bind_image_username': 'test-user',
@@ -123,6 +134,26 @@ class TestBindK8s(unittest.TestCase):
                         {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'},
                         {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'},
                     ],
+                    'config': {'ENABLE_RFC1918_RECURSION': 1},
+                    'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}},
+                }
+            ],
+        }
+        self.assertEqual(self.harness.charm.make_pod_spec(), expected)
+
+    def test_make_pod_spec_without_recursion(self):
+        """Confirm that we generate the expected pod spec from config disabling recursion."""
+        self.harness.update_config(CONFIG_VALID_WITHOUT_RECURSION)
+        expected = {
+            'version': 2,
+            'containers': [
+                {
+                    'name': 'bind-k8s',
+                    'imageDetails': {'imagePath': 'example.com/bind:v1'},
+                    'ports': [
+                        {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'},
+                        {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'},
+                    ],
                     'config': {},
                     'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}},
                 }
@@ -147,7 +178,7 @@ class TestBindK8s(unittest.TestCase):
                         {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'},
                         {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'},
                     ],
-                    'config': {},
+                    'config': {'ENABLE_RFC1918_RECURSION': 1},
                     'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}},
                 }
             ],
@@ -167,7 +198,10 @@ class TestBindK8s(unittest.TestCase):
                         {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'},
                         {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'},
                     ],
-                    'config': {'magic_number': 123},
+                    'config': {
+                        'ENABLE_RFC1918_RECURSION': 1,
+                        'magic_number': 123,
+                    },
                     'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}},
                 }
             ],
@@ -187,7 +221,7 @@ class TestBindK8s(unittest.TestCase):
                         {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'},
                         {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'},
                     ],
-                    'config': {'magic_number': 123, 'secret_password': 'xyzzy'},
+                    'config': {'ENABLE_RFC1918_RECURSION': 1, 'magic_number': 123, 'secret_password': 'xyzzy'},
                     'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}},
                 }
             ],
@@ -209,6 +243,7 @@ class TestBindK8s(unittest.TestCase):
                     ],
                     'config': {
                         'CUSTOM_CONFIG_REPO': 'https://git.example.com/example-bind-config.git',
+                        'ENABLE_RFC1918_RECURSION': 1,
                         'http_proxy': 'http://webproxy.example.com:3128/',
                         'https_proxy': 'http://webproxy.example.com:3128/',
                     },