Merge ~cgrabowski/maas:fix_writing_zonefile_with_correct_permissions into maas:master

Proposed by Christian Grabowski
Status: Merged
Approved by: Christian Grabowski
Approved revision: 5201a578aa693770ccbe9b22d94b720a1f7a0586
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~cgrabowski/maas:fix_writing_zonefile_with_correct_permissions
Merge into: maas:master
Diff against target: 114 lines (+38/-5)
3 files modified
src/provisioningserver/dns/tests/test_zoneconfig.py (+23/-0)
src/provisioningserver/dns/zoneconfig.py (+7/-1)
src/provisioningserver/utils/fs.py (+8/-4)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Jack Lloyd-Walters Approve
Review via email: mp+435611@code.launchpad.net

Commit message

allow MAAS to overwrite zonefile ownership from BIND

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix_writing_zonefile_with_correct_permissions lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 34993c2e1ac1b014f406e9a3015a861f0dbe4ac9

review: Approve
Revision history for this message
Jack Lloyd-Walters (lloydwaltersj) wrote :

LGTM, minor question inline

review: Approve
Revision history for this message
Christian Grabowski (cgrabowski) :
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix_writing_zonefile_with_correct_permissions lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 5201a578aa693770ccbe9b22d94b720a1f7a0586

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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 @@
7from itertools import chain7from itertools import chain
8import os.path8import os.path
9import random9import random
10from tempfile import mktemp
1011
11from netaddr import IPAddress, IPNetwork, IPRange12from netaddr import IPAddress, IPNetwork, IPRange
12from testtools.matchers import (13from testtools.matchers import (
@@ -29,9 +30,11 @@ from provisioningserver.dns.config import (
29 get_zone_file_config_dir,30 get_zone_file_config_dir,
30)31)
31from provisioningserver.dns.testing import patch_zone_file_config_path32from provisioningserver.dns.testing import patch_zone_file_config_path
33import provisioningserver.dns.zoneconfig
32from provisioningserver.dns.zoneconfig import (34from provisioningserver.dns.zoneconfig import (
33 DNSForwardZoneConfig,35 DNSForwardZoneConfig,
34 DNSReverseZoneConfig,36 DNSReverseZoneConfig,
37 DomainConfigBase,
35 DomainInfo,38 DomainInfo,
36)39)
3740
@@ -77,6 +80,26 @@ class HostnameRRsetMapping:
77 return self.__dict__ == other.__dict__80 return self.__dict__ == other.__dict__
7881
7982
83class TestDomainConfigBase(MAASTestCase):
84 def test_write_zone_file_sets_current_user_and_group(self):
85 render_dns_template = self.patch(
86 provisioningserver.dns.zoneconfig, "render_dns_template"
87 )
88 render_dns_template.return_value = ""
89 incremental_write = self.patch(
90 provisioningserver.dns.zoneconfig, "incremental_write"
91 )
92 tmp_file = mktemp()
93 DomainConfigBase.write_zone_file(tmp_file)
94 incremental_write.assert_called_once_with(
95 "".encode("utf-8"),
96 tmp_file,
97 mode=0o644,
98 uid=os.getuid(),
99 gid=os.getgid(),
100 )
101
102
80class TestDNSForwardZoneConfig(MAASTestCase):103class TestDNSForwardZoneConfig(MAASTestCase):
81 """Tests for DNSForwardZoneConfig."""104 """Tests for DNSForwardZoneConfig."""
82105
diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
index 3c6e383..8dec9b6 100644
--- a/src/provisioningserver/dns/zoneconfig.py
+++ b/src/provisioningserver/dns/zoneconfig.py
@@ -199,7 +199,13 @@ class DomainConfigBase:
199 for outfile in output_file:199 for outfile in output_file:
200 content = render_dns_template(cls.template_file_name, *parameters)200 content = render_dns_template(cls.template_file_name, *parameters)
201 with report_missing_config_dir():201 with report_missing_config_dir():
202 incremental_write(content.encode("utf-8"), outfile, mode=0o644)202 incremental_write(
203 content.encode("utf-8"),
204 outfile,
205 mode=0o644,
206 uid=os.getuid(),
207 gid=os.getgid(),
208 )
203 pass209 pass
204210
205211
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):
116 return temp_file116 return temp_file
117117
118118
119def atomic_write(content, filename, overwrite=True, mode=0o600):119def atomic_write(
120 content, filename, overwrite=True, mode=0o600, uid=None, gid=None
121):
120 """Write `content` into the file `filename` in an atomic fashion.122 """Write `content` into the file `filename` in an atomic fashion.
121123
122 This requires write permissions to the directory that `filename` is in.124 This requires write permissions to the directory that `filename` is in.
@@ -142,7 +144,9 @@ def atomic_write(content, filename, overwrite=True, mode=0o600):
142 raise # Something's seriously wrong.144 raise # Something's seriously wrong.
143 else:145 else:
144 try:146 try:
145 chown(temp_file, prev_stats.st_uid, prev_stats.st_gid)147 chown(
148 temp_file, uid or prev_stats.st_uid, gid or prev_stats.st_gid
149 )
146 except PermissionError as error:150 except PermissionError as error:
147 # if the permissions of `filename` couldn't be applied to151 # if the permissions of `filename` couldn't be applied to
148 # `temp_file` then the temporary file needs to be removed and then152 # `temp_file` then the temporary file needs to be removed and then
@@ -263,7 +267,7 @@ def atomic_symlink(source, link_name):
263 raise267 raise
264268
265269
266def incremental_write(content, filename, mode=0o600):270def incremental_write(content, filename, mode=0o600, uid=None, gid=None):
267 """Write the given `content` into the file `filename`. In the past, this271 """Write the given `content` into the file `filename`. In the past, this
268 would potentially change modification time to arbitrary values.272 would potentially change modification time to arbitrary values.
269273
@@ -283,7 +287,7 @@ def incremental_write(content, filename, mode=0o600):
283 # granularity are no longer supported by MAAS. The good news is that since287 # granularity are no longer supported by MAAS. The good news is that since
284 # 2.6, linux has supported nanosecond-granular time. As of bind9288 # 2.6, linux has supported nanosecond-granular time. As of bind9
285 # 1:9.10.3.dfsg.P2-5, BIND even uses it.289 # 1:9.10.3.dfsg.P2-5, BIND even uses it.
286 atomic_write(content, filename, mode=mode)290 atomic_write(content, filename, mode=mode, uid=uid, gid=gid)
287291
288292
289def _with_dev_python(*command):293def _with_dev_python(*command):

Subscribers

People subscribed via source and target branches