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

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