Merge ~cgrabowski/maas:backport_fix_writing_zonefile_with_correct_permissions_to_3.3 into maas:3.3

Proposed by Christian Grabowski
Status: Merged
Approved by: Christian Grabowski
Approved revision: 6b6816ec4aab78ac011f243a9c4bf9a70f17d40e
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~cgrabowski/maas:backport_fix_writing_zonefile_with_correct_permissions_to_3.3
Merge into: maas:3.3
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
Christian Grabowski Approve
Review via email: mp+435670@code.launchpad.net

Commit message

allow MAAS to overwrite zonefile ownership from BIND
(cherry picked from commit 46ea08c76a366732fd30062399173ee310709ff1)

To post a comment you must log in.
Revision history for this message
Christian Grabowski (cgrabowski) wrote :

self-approving backport

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/provisioningserver/dns/tests/test_zoneconfig.py b/src/provisioningserver/dns/tests/test_zoneconfig.py
2index 24e18e1..10ee0f7 100644
3--- a/src/provisioningserver/dns/tests/test_zoneconfig.py
4+++ b/src/provisioningserver/dns/tests/test_zoneconfig.py
5@@ -7,6 +7,7 @@
6 from itertools import chain
7 import os.path
8 import random
9+from tempfile import mktemp
10
11 from netaddr import IPAddress, IPNetwork, IPRange
12 from testtools.matchers import (
13@@ -29,9 +30,11 @@ from provisioningserver.dns.config import (
14 get_zone_file_config_dir,
15 )
16 from provisioningserver.dns.testing import patch_zone_file_config_path
17+import provisioningserver.dns.zoneconfig
18 from provisioningserver.dns.zoneconfig import (
19 DNSForwardZoneConfig,
20 DNSReverseZoneConfig,
21+ DomainConfigBase,
22 DomainInfo,
23 )
24
25@@ -77,6 +80,26 @@ class HostnameRRsetMapping:
26 return self.__dict__ == other.__dict__
27
28
29+class TestDomainConfigBase(MAASTestCase):
30+ def test_write_zone_file_sets_current_user_and_group(self):
31+ render_dns_template = self.patch(
32+ provisioningserver.dns.zoneconfig, "render_dns_template"
33+ )
34+ render_dns_template.return_value = ""
35+ incremental_write = self.patch(
36+ provisioningserver.dns.zoneconfig, "incremental_write"
37+ )
38+ tmp_file = mktemp()
39+ DomainConfigBase.write_zone_file(tmp_file)
40+ incremental_write.assert_called_once_with(
41+ "".encode("utf-8"),
42+ tmp_file,
43+ mode=0o644,
44+ uid=os.getuid(),
45+ gid=os.getgid(),
46+ )
47+
48+
49 class TestDNSForwardZoneConfig(MAASTestCase):
50 """Tests for DNSForwardZoneConfig."""
51
52diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
53index 3c6e383..8dec9b6 100644
54--- a/src/provisioningserver/dns/zoneconfig.py
55+++ b/src/provisioningserver/dns/zoneconfig.py
56@@ -199,7 +199,13 @@ class DomainConfigBase:
57 for outfile in output_file:
58 content = render_dns_template(cls.template_file_name, *parameters)
59 with report_missing_config_dir():
60- incremental_write(content.encode("utf-8"), outfile, mode=0o644)
61+ incremental_write(
62+ content.encode("utf-8"),
63+ outfile,
64+ mode=0o644,
65+ uid=os.getuid(),
66+ gid=os.getgid(),
67+ )
68 pass
69
70
71diff --git a/src/provisioningserver/utils/fs.py b/src/provisioningserver/utils/fs.py
72index 37428ab..d8fcc56 100644
73--- a/src/provisioningserver/utils/fs.py
74+++ b/src/provisioningserver/utils/fs.py
75@@ -116,7 +116,9 @@ def _write_temp_file(content, filename):
76 return temp_file
77
78
79-def atomic_write(content, filename, overwrite=True, mode=0o600):
80+def atomic_write(
81+ content, filename, overwrite=True, mode=0o600, uid=None, gid=None
82+):
83 """Write `content` into the file `filename` in an atomic fashion.
84
85 This requires write permissions to the directory that `filename` is in.
86@@ -142,7 +144,9 @@ def atomic_write(content, filename, overwrite=True, mode=0o600):
87 raise # Something's seriously wrong.
88 else:
89 try:
90- chown(temp_file, prev_stats.st_uid, prev_stats.st_gid)
91+ chown(
92+ temp_file, uid or prev_stats.st_uid, gid or prev_stats.st_gid
93+ )
94 except PermissionError as error:
95 # if the permissions of `filename` couldn't be applied to
96 # `temp_file` then the temporary file needs to be removed and then
97@@ -263,7 +267,7 @@ def atomic_symlink(source, link_name):
98 raise
99
100
101-def incremental_write(content, filename, mode=0o600):
102+def incremental_write(content, filename, mode=0o600, uid=None, gid=None):
103 """Write the given `content` into the file `filename`. In the past, this
104 would potentially change modification time to arbitrary values.
105
106@@ -283,7 +287,7 @@ def incremental_write(content, filename, mode=0o600):
107 # granularity are no longer supported by MAAS. The good news is that since
108 # 2.6, linux has supported nanosecond-granular time. As of bind9
109 # 1:9.10.3.dfsg.P2-5, BIND even uses it.
110- atomic_write(content, filename, mode=mode)
111+ atomic_write(content, filename, mode=mode, uid=uid, gid=gid)
112
113
114 def _with_dev_python(*command):

Subscribers

People subscribed via source and target branches