From 15a6e0868097ec8a6ef97b9fde59a9486270fc37 Mon Sep 17 00:00:00 2001 From: Jack Date: Tue, 21 Feb 2023 23:39:43 +0800 Subject: Set ownership for new folders in Write Files Module (#1980) The parent directory would be created automatically if it does not exist. But the ownership of newly-created parent directory would always be root. With this change, it would be set the same as `owner`. LP: #1990513 --- cloudinit/config/cc_write_files.py | 4 ++- .../config/schemas/schema-cloud-config-v1.json | 2 +- cloudinit/util.py | 38 ++++++++++++++++++++-- .../integration_tests/modules/test_write_files.py | 29 +++++++++++++++++ tests/unittests/test_util.py | 20 ++++++++++++ tools/.github-cla-signers | 1 + 6 files changed, 90 insertions(+), 4 deletions(-) diff --git a/cloudinit/config/cc_write_files.py b/cloudinit/config/cc_write_files.py index f59b2d54..a517d044 100644 --- a/cloudinit/config/cc_write_files.py +++ b/cloudinit/config/cc_write_files.py @@ -182,7 +182,9 @@ def write_files(name, files, owner: str): (u, g) = util.extract_usergroup(f_info.get("owner", owner)) perms = decode_perms(f_info.get("permissions"), DEFAULT_PERMS) omode = "ab" if util.get_cfg_option_bool(f_info, "append") else "wb" - util.write_file(path, contents, omode=omode, mode=perms) + util.write_file( + path, contents, omode=omode, mode=perms, user=u, group=g + ) util.chownbyname(path, u, g) diff --git a/cloudinit/config/schemas/schema-cloud-config-v1.json b/cloudinit/config/schemas/schema-cloud-config-v1.json index 8bd7a80c..10636e6d 100644 --- a/cloudinit/config/schemas/schema-cloud-config-v1.json +++ b/cloudinit/config/schemas/schema-cloud-config-v1.json @@ -3151,7 +3151,7 @@ "owner": { "type": "string", "default": "root:root", - "description": "Optional owner:group to chown on the file. Default: ``root:root``" + "description": "Optional owner:group to chown on the file and new directories. Default: ``root:root``" }, "permissions": { "type": "string", diff --git a/cloudinit/util.py b/cloudinit/util.py index 53a72b13..8ba3e2b6 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -35,6 +35,7 @@ from base64 import b64decode, b64encode from collections import deque, namedtuple from errno import EACCES, ENOENT from functools import lru_cache, total_ordering +from pathlib import Path from typing import Callable, Deque, Dict, List, TypeVar from urllib import parse @@ -1791,12 +1792,41 @@ def json_dumps(data): ) -def ensure_dir(path, mode=None): +def get_non_exist_parent_dir(path): + """Get the last directory in a path that does not exist. + + Example: when path=/usr/a/b and /usr/a does not exis but /usr does, + return /usr/a + """ + p_path = os.path.dirname(path) + # Check if parent directory of path is root + if p_path == os.path.dirname(p_path): + return path + else: + if os.path.isdir(p_path): + return path + else: + return get_non_exist_parent_dir(p_path) + + +def ensure_dir(path, mode=None, user=None, group=None): if not os.path.isdir(path): + # Get non existed parent dir first before they are created. + non_existed_parent_dir = get_non_exist_parent_dir(path) # Make the dir and adjust the mode with SeLinuxGuard(os.path.dirname(path), recursive=True): os.makedirs(path) chmod(path, mode) + # Change the ownership + if user or group: + chownbyname(non_existed_parent_dir, user, group) + # if path=/usr/a/b/c and non_existed_parent_dir=/usr, + # then sub_relative_dir=PosixPath('a/b/c') + sub_relative_dir = Path(path.split(non_existed_parent_dir)[1][1:]) + sub_path = Path(non_existed_parent_dir) + for part in sub_relative_dir.parts: + sub_path = sub_path.joinpath(part) + chownbyname(sub_path, user, group) else: # Just adjust the mode chmod(path, mode) @@ -2133,6 +2163,8 @@ def write_file( preserve_mode=False, *, ensure_dir_exists=True, + user=None, + group=None, ): """ Writes a file with the given content and sets the file mode as specified. @@ -2147,6 +2179,8 @@ def write_file( @param ensure_dir_exists: If True (the default), ensure that the directory containing `filename` exists before writing to the file. + @param user: The user to set on the file. + @param group: The group to set on the file. """ if preserve_mode: @@ -2156,7 +2190,7 @@ def write_file( pass if ensure_dir_exists: - ensure_dir(os.path.dirname(filename)) + ensure_dir(os.path.dirname(filename), user=user, group=group) if "b" in omode.lower(): content = encode_text(content) write_type = "bytes" diff --git a/tests/integration_tests/modules/test_write_files.py b/tests/integration_tests/modules/test_write_files.py index a713b9c5..e6bb8625 100644 --- a/tests/integration_tests/modules/test_write_files.py +++ b/tests/integration_tests/modules/test_write_files.py @@ -51,6 +51,13 @@ write_files: owner: 'myuser' permissions: '0644' append: true +- path: '/home/testuser/subdir1/subdir2/my-file' + content: | + echo 'hello world!' + defer: true + owner: 'myuser' + permissions: '0644' + append: true """.format( B64_CONTENT.decode("ascii") ) @@ -97,3 +104,25 @@ class TestWriteFiles: class_client.restart() out = class_client.read_from_file("/home/testuser/my-file") assert "echo 'hello world!'" == out + + def test_write_files_deferred_with_newly_created_dir(self, class_client): + """Test that newly created directory works as expected. + + Users get created after write_files module runs, so ensure that + with `defer: true`, the file and directories gets written with correct + ownership. + """ + out = class_client.read_from_file( + "/home/testuser/subdir1/subdir2/my-file" + ) + assert "echo 'hello world!'" == out + assert ( + class_client.execute( + 'stat -c "%U %a" /home/testuser/subdir1/subdir2' + ) + == "myuser 755" + ) + assert ( + class_client.execute('stat -c "%U %a" /home/testuser/subdir1') + == "myuser 755" + ) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 1be778c6..07142a86 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -14,6 +14,7 @@ import shutil import stat import tempfile from collections import deque +from pathlib import Path from textwrap import dedent from unittest import mock from urllib.parse import urlparse @@ -1755,6 +1756,25 @@ class TestWriteFile(helpers.TestCase): self.assertTrue(os.path.isdir(dirname)) self.assertTrue(os.path.isfile(path)) + def test_dir_ownership(self): + """Verifiy that directories is created with appropriate ownership.""" + dirname = os.path.join(self.tmp, "subdir", "subdir2") + path = os.path.join(dirname, "NewFile.txt") + contents = "Hey there" + user = "foo" + group = "foo" + + with mock.patch.object( + util, "chownbyname", return_value=None + ) as mockobj: + util.write_file(path, contents, user=user, group=group) + + calls = [ + mock.call(os.path.join(self.tmp, "subdir"), user, group), + mock.call(Path(dirname), user, group), + ] + mockobj.assert_has_calls(calls, any_order=False) + def test_dir_is_not_created_if_ensure_dir_false(self): """Verify directories are not created if ensure_dir_exists is False.""" dirname = os.path.join(self.tmp, "subdir") diff --git a/tools/.github-cla-signers b/tools/.github-cla-signers index 69e199b1..d8cca015 100644 --- a/tools/.github-cla-signers +++ b/tools/.github-cla-signers @@ -141,4 +141,5 @@ xiaoge1001 xnox yangzz-97 yawkat +zhan9san zhuzaifangxuele -- cgit v1.2.1