sts-sponsors team mailing list archive

[Christian Grabowski <mp+435611@xxxxxxxxxxxxxxxxxx>]

Christian Grabowski has proposed merging ~cgrabowski/maas:fix_writing_zonefile_with_correct_permissions into maas:master.

Commit message:
allow MAAS to overwrite zonefile ownership from BIND



Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~cgrabowski/maas/+git/maas/+merge/435611
-- 
Your team MAAS Maintainers is requested to review the proposed merge of ~cgrabowski/maas:fix_writing_zonefile_with_correct_permissions into maas:master.

diff --git a/src/provisioningserver/dns/tests/test_zoneconfig.py b/src/provisioningserver/dns/tests/test_zoneconfig.py
index 24e18e1..10ee0f7 100644
--- a/src/provisioningserver/dns/tests/test_zoneconfig.py
+++ b/src/provisioningserver/dns/tests/test_zoneconfig.py
@@ -7,6 +7,7 @@
 from itertools import chain
 import os.path
 import random
+from tempfile import mktemp
 
 from netaddr import IPAddress, IPNetwork, IPRange
 from testtools.matchers import (
@@ -29,9 +30,11 @@ from provisioningserver.dns.config import (
     get_zone_file_config_dir,
 )
 from provisioningserver.dns.testing import patch_zone_file_config_path
+import provisioningserver.dns.zoneconfig
 from provisioningserver.dns.zoneconfig import (
     DNSForwardZoneConfig,
     DNSReverseZoneConfig,
+    DomainConfigBase,
     DomainInfo,
 )
 
@@ -77,6 +80,26 @@ class HostnameRRsetMapping:
         return self.__dict__ == other.__dict__
 
 
+class TestDomainConfigBase(MAASTestCase):
+    def test_write_zone_file_sets_current_user_and_group(self):
+        render_dns_template = self.patch(
+            provisioningserver.dns.zoneconfig, "render_dns_template"
+        )
+        render_dns_template.return_value = ""
+        incremental_write = self.patch(
+            provisioningserver.dns.zoneconfig, "incremental_write"
+        )
+        tmp_file = mktemp()
+        DomainConfigBase.write_zone_file(tmp_file)
+        incremental_write.assert_called_once_with(
+            "".encode("utf-8"),
+            tmp_file,
+            mode=0o644,
+            uid=os.getuid(),
+            gid=os.getgid(),
+        )
+
+
 class TestDNSForwardZoneConfig(MAASTestCase):
     """Tests for DNSForwardZoneConfig."""
 
diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
index 3c6e383..6ab01a8 100644
--- a/src/provisioningserver/dns/zoneconfig.py
+++ b/src/provisioningserver/dns/zoneconfig.py
@@ -197,9 +197,16 @@ class DomainConfigBase:
         if not isinstance(output_file, list):
             output_file = [output_file]
         for outfile in output_file:
+            print(render_dns_template)
             content = render_dns_template(cls.template_file_name, *parameters)
             with report_missing_config_dir():
-                incremental_write(content.encode("utf-8"), outfile, mode=0o644)
+                incremental_write(
+                    content.encode("utf-8"),
+                    outfile,
+                    mode=0o644,
+                    uid=os.getuid(),
+                    gid=os.getgid(),
+                )
         pass
 
 
diff --git a/src/provisioningserver/utils/fs.py b/src/provisioningserver/utils/fs.py
index 37428ab..d8fcc56 100644
--- a/src/provisioningserver/utils/fs.py
+++ b/src/provisioningserver/utils/fs.py
@@ -116,7 +116,9 @@ def _write_temp_file(content, filename):
         return temp_file
 
 
-def atomic_write(content, filename, overwrite=True, mode=0o600):
+def atomic_write(
+    content, filename, overwrite=True, mode=0o600, uid=None, gid=None
+):
     """Write `content` into the file `filename` in an atomic fashion.
 
     This requires write permissions to the directory that `filename` is in.
@@ -142,7 +144,9 @@ def atomic_write(content, filename, overwrite=True, mode=0o600):
             raise  # Something's seriously wrong.
     else:
         try:
-            chown(temp_file, prev_stats.st_uid, prev_stats.st_gid)
+            chown(
+                temp_file, uid or prev_stats.st_uid, gid or prev_stats.st_gid
+            )
         except PermissionError as error:
             # if the permissions of `filename` couldn't be applied to
             # `temp_file` then the temporary file needs to be removed and then
@@ -263,7 +267,7 @@ def atomic_symlink(source, link_name):
         raise
 
 
-def incremental_write(content, filename, mode=0o600):
+def incremental_write(content, filename, mode=0o600, uid=None, gid=None):
     """Write the given `content` into the file `filename`.  In the past, this
     would potentially change modification time to arbitrary values.
 
@@ -283,7 +287,7 @@ def incremental_write(content, filename, mode=0o600):
     # granularity are no longer supported by MAAS.  The good news is that since
     # 2.6, linux has supported nanosecond-granular time.  As of bind9
     # 1:9.10.3.dfsg.P2-5, BIND even uses it.
-    atomic_write(content, filename, mode=mode)
+    atomic_write(content, filename, mode=mode, uid=uid, gid=gid)
 
 
 def _with_dev_python(*command):