From ebc7eb82969af107f5a54b29b8ef898cd759a997 Mon Sep 17 00:00:00 2001 From: Michal Arbet Date: Mon, 25 May 2026 09:56:17 +0200 Subject: set_configs: Add support for creating files and setting perms Change-Id: I74a6d05007488b69fa040c40d01a07afae369794 Needed-By: https://review.opendev.org/c/openstack/kolla-ansible/+/989850 Signed-off-by: Michal Arbet --- doc/source/admin/kolla_api.rst | 11 +++ docker/base/set_configs.py | 49 +++++++++-- tests/test_set_config.py | 181 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 235 insertions(+), 6 deletions(-) diff --git a/doc/source/admin/kolla_api.rst b/doc/source/admin/kolla_api.rst index b1fd6b140..a34cd819f 100644 --- a/doc/source/admin/kolla_api.rst +++ b/doc/source/admin/kolla_api.rst @@ -64,6 +64,17 @@ The `kolla_set_configs`_ script understands the following attributes: * **perm** (required): the unix permissions to set to the directory. Must be passed in the numeric octal form. +* **files**: create empty files inside the container if they do not + exist, and set their ownership and permissions. A list of dicts, each + containing the following attributes: + + * **path** (required): the path to the file to create. Parent directories + must already exist. Glob patterns are not supported. + * **owner** (required): the ``user:group`` to change ownership to. ``user`` + is synonymous to ``user:user``. Must be user and group names, not uid/gid. + * **perm** (required): the unix permissions to set to the file. + Must be passed in the numeric octal form. + * **permissions**: change the permissions and/or ownership of files or directories inside the container. A list of dicts, each containing the following attributes: diff --git a/docker/base/set_configs.py b/docker/base/set_configs.py index c0a4af76c..4b025d55a 100644 --- a/docker/base/set_configs.py +++ b/docker/base/set_configs.py @@ -258,7 +258,7 @@ class ConfigFile(object): def validate_config(config): required_keys = {'source', 'dest'} - required_dir_keys = {'path', 'owner', 'perm'} + required_path_keys = {'path', 'owner', 'perm'} if 'command' not in config: raise InvalidConfig('Config is missing required "command" key') @@ -274,11 +274,24 @@ def validate_config(config): raise InvalidConfig( 'Config needs preserve_properties or owner and perm') - for data in config.get('directories', list()): - if not set(data.keys()) >= required_dir_keys: - message = ('directories config is missing required keys: %s' - % (required_dir_keys - set(data.keys()))) - raise InvalidConfig(message) + for section in ('directories', 'files'): + for data in config.get(section, list()): + missing = required_path_keys - set(data.keys()) + if missing: + raise InvalidConfig( + '%s config is missing required keys: %s' + % (section, missing)) + + for key in required_path_keys: + if data.get(key) is None: + raise InvalidConfig( + '%s config key "%s" must not be null' + % (section, key)) + + if glob.has_magic(data['path']): + raise InvalidConfig( + '%s config does not support glob paths: %s' + % (section, data['path'])) def validate_source(data): @@ -375,6 +388,28 @@ def create_directories(config): handle_permissions({'permissions': [data]}) +def create_files(config): + if 'files' not in config: + return + LOG.info('Creating files') + for data in config['files']: + path = data['path'] + if os.path.islink(path): + raise OSError( + "Path is a symbolic link, expected regular file (%s)" % path) + if os.path.isdir(path): + raise OSError( + "Path is a directory, expected file (%s)" % path) + parent = os.path.dirname(path) + if parent and not os.path.isdir(parent): + raise OSError( + "Parent directory does not exist (%s)" % parent) + if not os.path.exists(path): + with open(path, 'a'): + pass + handle_permissions({'permissions': [data]}) + + def user_group(owner): if ':' in owner: user, group = owner.split(':', 1) @@ -611,6 +646,7 @@ def execute_config_strategy(config): LOG.info("Kolla config strategy set to: %s", config_strategy) if config_strategy == "COPY_ALWAYS": create_directories(config) + create_files(config) handle_defaults(config) copy_config(config) handle_permissions(config) @@ -621,6 +657,7 @@ def execute_config_strategy(config): exit_code=0) else: create_directories(config) + create_files(config) handle_defaults(config) copy_config(config) handle_permissions(config) diff --git a/tests/test_set_config.py b/tests/test_set_config.py index 45f71bdd4..50c38c087 100644 --- a/tests/test_set_config.py +++ b/tests/test_set_config.py @@ -990,6 +990,19 @@ class ValidateConfigDirectoriesTest(base.BaseTestCase): } set_configs.validate_config(config) + def test_validate_config_directories_glob_rejected(self): + config = { + 'command': '/bin/true', + 'directories': [{'path': '/var/log/*', + 'owner': 'myuser', + 'perm': '0755'}] + } + self.assertRaisesRegex( + set_configs.InvalidConfig, + 'directories config does not support glob paths', + set_configs.validate_config, + config) + class CreateDirectoriesTest(base.BaseTestCase): @@ -1062,6 +1075,174 @@ class CreateDirectoriesTest(base.BaseTestCase): self.assertEqual(mock_handle_permissions.call_count, 2) +class ValidateConfigFilesTest(base.BaseTestCase): + + def test_validate_config_files_missing_path(self): + config = { + 'command': '/bin/true', + 'files': [{'owner': 'myuser', 'perm': '0644'}] + } + self.assertRaisesRegex( + set_configs.InvalidConfig, + 'files config is missing required keys', + set_configs.validate_config, + config) + + def test_validate_config_files_glob_rejected(self): + config = { + 'command': '/bin/true', + 'files': [{'path': '/var/log/*.log', + 'owner': 'myuser', 'perm': '0644'}] + } + self.assertRaisesRegex( + set_configs.InvalidConfig, + 'files config does not support glob paths', + set_configs.validate_config, + config) + + def test_validate_config_files_valid(self): + config = { + 'command': '/bin/true', + 'files': [ + {'path': '/var/log/foo.log', + 'owner': 'myuser', 'perm': '0644'} + ] + } + set_configs.validate_config(config) + + def test_validate_config_files_null_path_rejected(self): + config = { + 'command': '/bin/true', + 'files': [{'path': None, + 'owner': 'myuser', + 'perm': '0644'}] + } + self.assertRaisesRegex( + set_configs.InvalidConfig, + 'files config key "path" must not be null', + set_configs.validate_config, + config) + + +class CreateFilesTest(base.BaseTestCase): + + @mock.patch.object(set_configs, 'handle_permissions') + @mock.patch('builtins.open', new_callable=mock.mock_open) + @mock.patch('os.path.exists', return_value=False) + @mock.patch('os.path.isdir', side_effect=[False, True]) + @mock.patch('os.path.islink', return_value=False) + def test_create_file(self, mock_islink, mock_isdir, mock_exists, + mock_open, mock_handle_permissions): + config = { + 'files': [ + {'path': '/var/log/foo.log', + 'owner': 'myuser', 'perm': '0644'} + ] + } + set_configs.create_files(config) + + mock_open.assert_called_once_with('/var/log/foo.log', 'a') + mock_handle_permissions.assert_called_once_with( + {'permissions': [ + {'path': '/var/log/foo.log', + 'owner': 'myuser', 'perm': '0644'} + ]} + ) + + @mock.patch.object(set_configs, 'handle_permissions') + @mock.patch('builtins.open', new_callable=mock.mock_open) + @mock.patch('os.path.exists', return_value=True) + @mock.patch('os.path.isdir', side_effect=[False, True]) + @mock.patch('os.path.islink', return_value=False) + def test_create_file_already_exists(self, mock_islink, mock_isdir, + mock_exists, mock_open, + mock_handle_permissions): + config = { + 'files': [ + {'path': '/var/log/foo.log', + 'owner': 'myuser', 'perm': '0644'} + ] + } + + set_configs.create_files(config) + + mock_open.assert_not_called() + mock_handle_permissions.assert_called_once_with( + {'permissions': [ + {'path': '/var/log/foo.log', + 'owner': 'myuser', 'perm': '0644'} + ]} + ) + + @mock.patch('os.path.islink', return_value=True) + def test_create_file_rejects_symlink(self, mock_islink): + + config = { + 'files': [ + {'path': '/var/log/foo.log', + 'owner': 'myuser', + 'perm': '0644'} + ] + } + + self.assertRaisesRegex( + OSError, + 'Path is a symbolic link', + set_configs.create_files, + config) + + @mock.patch('os.path.isdir', return_value=True) + @mock.patch('os.path.islink', return_value=False) + def test_create_file_rejects_directory(self, mock_islink, mock_isdir): + + config = { + 'files': [ + {'path': '/var/log/somefile', + 'owner': 'myuser', + 'perm': '0644'} + ] + } + + self.assertRaisesRegex( + OSError, + 'Path is a directory', + set_configs.create_files, + config) + + @mock.patch('os.path.isdir', side_effect=[False, False]) + @mock.patch('os.path.islink', return_value=False) + def test_create_file_rejects_missing_parent(self, mock_islink, + mock_isdir): + config = { + 'files': [ + {'path': '/var/log/foo.log', + 'owner': 'myuser', + 'perm': '0644'} + ] + } + + self.assertRaisesRegex( + OSError, + 'Parent directory does not exist', + set_configs.create_files, + config) + + @mock.patch.object(set_configs, 'handle_permissions') + @mock.patch('os.path.exists') + @mock.patch('os.path.isdir') + @mock.patch('os.path.islink') + def test_create_files_no_files_key(self, mock_islink, mock_isdir, + mock_exists, + mock_handle_permissions): + config = {'command': '/bin/true'} + set_configs.create_files(config) + + mock_islink.assert_not_called() + mock_isdir.assert_not_called() + mock_exists.assert_not_called() + mock_handle_permissions.assert_not_called() + + class ExecuteConfigCheckStateMismatchTest(base.BaseTestCase): @mock.patch.object(set_configs, 'get_defaults_state') -- cgit v1.2.3