diff --git a/src/service.py b/src/service.py index 9acd2dbf..d68db3d1 100644 --- a/src/service.py +++ b/src/service.py @@ -1,9 +1,10 @@ """Exporter service helper.""" +import os from functools import wraps from logging import getLogger from pathlib import Path -from typing import Any, Callable, Dict, Tuple +from typing import Any, Callable, Dict, Optional, Tuple from charms.operator_libs_linux.v1 import systemd from jinja2 import Environment, FileSystemLoader @@ -55,13 +56,20 @@ def __init__(self, search_path: Path): self.config_template = self.environment.get_template(EXPORTER_CONFIG_TEMPLATE) self.service_template = self.environment.get_template(EXPORTER_SERVICE_TEMPLATE) - def _install(self, path: Path, content: str) -> bool: + def _install(self, path: Path, content: str, mode: Optional[int] = None) -> bool: """Install file.""" success = True try: logger.info("Writing file to %s.", path) - with open(path, "w", encoding="utf-8") as file: + fileobj = ( + os.fdopen(os.open(path, os.O_CREAT | os.O_WRONLY, mode), "w", encoding="utf-8") + if mode + # create file with default permissions based on default OS umask + else open(path, "w", encoding="utf-8") # pylint: disable=consider-using-with + ) + with fileobj as file: file.write(content) + except (NotADirectoryError, PermissionError) as err: logger.error(err) logger.info("Writing file to %s - Failed.", path) @@ -106,7 +114,7 @@ def render_config( REDFISH_PASSWORD=redfish_conn_params.get("password", ""), REDFISH_CLIENT_TIMEOUT=redfish_conn_params.get("timeout", ""), ) - return self._install(EXPORTER_CONFIG_PATH, content) + return self._install(EXPORTER_CONFIG_PATH, content, mode=0o600) def render_service(self, charm_dir: str, config_file: str) -> bool: """Render and install exporter service file.""" diff --git a/tests/functional/test_charm.py b/tests/functional/test_charm.py index c0f28d5f..3e330352 100644 --- a/tests/functional/test_charm.py +++ b/tests/functional/test_charm.py @@ -533,6 +533,14 @@ async def test_resource_clean_up(self, ops_test, app, unit, required_resources): class TestCharm: """Perform basic functional testing of the charm without having the actual hardware.""" + async def test_config_file_permissions(self, unit, ops_test): + """Check config file permissions are set correctly.""" + expected_file_mode = "600" + cmd = "stat -c '%a' /etc/hardware-exporter-config.yaml" + results = await run_command_on_unit(ops_test, unit.name, cmd) + assert results.get("return-code") == 0 + assert results.get("stdout").rstrip("\n") == expected_file_mode + async def test_config_changed_port(self, app, unit, ops_test): """Test changing the config option: exporter-port.""" new_port = "10001" diff --git a/tests/unit/test_exporter.py b/tests/unit/test_exporter.py index d521c732..d16023c4 100644 --- a/tests/unit/test_exporter.py +++ b/tests/unit/test_exporter.py @@ -57,6 +57,10 @@ def setUp(self): redfish_client_patcher.start() self.addCleanup(redfish_client_patcher.stop) + os_patcher = mock.patch.object(service, "os") + os_patcher.start() + self.addCleanup(os_patcher.stop) + @classmethod def setUpClass(cls): exporter_health_retry_count_patcher = mock.patch("charm.EXPORTER_HEALTH_RETRY_COUNT", 1) @@ -69,7 +73,7 @@ def setUpClass(cls): exporter_health_retry_timeout_patcher.start() cls.addClassCleanup(exporter_health_retry_timeout_patcher.stop) - def test_00_install_okay(self): + def test_install_okay(self): """Test exporter service is installed when charm is installed - okay.""" self.harness.begin() @@ -78,7 +82,7 @@ def test_00_install_okay(self): mock_open.assert_called() self.mock_systemd.daemon_reload.assert_called_once() - def test_01_install_failed_rendering(self): + def test_install_failed_rendering(self): """Test exporter service is failed to installed - failed to render.""" self.harness.begin() @@ -95,7 +99,7 @@ def test_01_install_failed_rendering(self): self.mock_systemd.daemon_reload.assert_not_called() @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_10_uninstall_okay(self, mock_service_exists): + def test_uninstall_okay(self, mock_service_exists): """Test exporter service is uninstalled when charm is removed - okay.""" self.harness.begin() @@ -105,7 +109,7 @@ def test_10_uninstall_okay(self, mock_service_exists): self.mock_systemd.daemon_reload.assert_called_once() @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_11_uninstall_failed(self, mock_service_exists): + def test_uninstall_failed(self, mock_service_exists): """Test exporter service is not uninstalled - failed to remove.""" self.harness.begin() @@ -116,7 +120,7 @@ def test_11_uninstall_failed(self, mock_service_exists): self.mock_systemd.daemon_reload.assert_not_called() @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_20_start_okay(self, mock_service_installed): + def test_start_okay(self, mock_service_installed): """Test exporter service started when relation is joined.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -127,7 +131,7 @@ def test_20_start_okay(self, mock_service_installed): self.mock_systemd.service_enable.assert_called_once() @mock.patch.object(pathlib.Path, "exists", return_value=False) - def test_21_start_failed(self, mock_service_not_installed): + def test_start_failed(self, mock_service_not_installed): """Test exporter service failed to started when relation is joined.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -136,7 +140,7 @@ def test_21_start_failed(self, mock_service_not_installed): self.mock_systemd.service_enable.assert_not_called() @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_22_start_defer_resource_not_ready(self, mock_service_installed): + def test_start_defer_resource_not_ready(self, mock_service_installed): """Test exporter service started when relation is joined.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -147,7 +151,7 @@ def test_22_start_defer_resource_not_ready(self, mock_service_installed): self.mock_systemd.service_enable.assert_not_called() @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_23_start_defer_exporter_not_ready(self, mock_service_installed): + def test_start_defer_exporter_not_ready(self, mock_service_installed): """Test exporter service started when relation is joined.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -158,7 +162,7 @@ def test_23_start_defer_exporter_not_ready(self, mock_service_installed): self.mock_systemd.service_enable.assert_not_called() @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_30_stop_okay(self, mock_service_installed): + def test_stop_okay(self, mock_service_installed): """Test exporter service is stopped when service is installed and relation is departed.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -170,7 +174,7 @@ def test_30_stop_okay(self, mock_service_installed): self.mock_systemd.service_disable.assert_called_once() @mock.patch.object(pathlib.Path, "exists", return_value=False) - def test_31_stop_failed(self, mock_service_not_installed): + def test_stop_failed(self, mock_service_not_installed): """Test exporter service failed to stop when service is not installed.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -191,7 +195,7 @@ def test_31_stop_failed(self, mock_service_not_installed): ] ) @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_40_check_health( + def test_check_health( self, failed, expected_status, @@ -211,7 +215,7 @@ def test_40_check_health( self.assertEqual(self.harness.charm.unit.status, expected_status) @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_40_check_health_exporter_crash(self, mock_service_installed): + def test_check_health_exporter_crash(self, mock_service_installed): """Test check_health function when service is installed but exporter crashes.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -225,7 +229,7 @@ def test_40_check_health_exporter_crash(self, mock_service_installed): self.harness.charm.on.update_status.emit() @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_50_check_relation_exists(self, mock_service_installed): + def test_check_relation_exists(self, mock_service_installed): """Test check_relation function when relation exists.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -237,7 +241,7 @@ def test_50_check_relation_exists(self, mock_service_installed): self.assertEqual(self.harness.charm.unit.status, ActiveStatus("Unit is ready")) @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_51_check_relation_not_exists(self, mock_service_installed): + def test_check_relation_not_exists(self, mock_service_installed): """Test check_relation function when relation does not exists.""" self.harness.begin() with mock.patch("builtins.open", new_callable=mock.mock_open) as _: @@ -249,7 +253,7 @@ def test_51_check_relation_not_exists(self, mock_service_installed): ) @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_52_too_many_relations(self, mock_service_installed): + def test_too_many_relations(self, mock_service_installed): """Test there too many relations.""" rid_1 = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") rid_2 = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") @@ -266,7 +270,7 @@ def test_52_too_many_relations(self, mock_service_installed): ) @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_60_config_changed_log_level_okay(self, mock_service_installed): + def test_config_changed_log_level_okay(self, mock_service_installed): """Test on_config_change function when exporter-log-level is changed.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -284,7 +288,7 @@ def test_60_config_changed_log_level_okay(self, mock_service_installed): ) @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_61_config_changed_not_okay(self, mock_service_installed): + def test_invalid_exporter_log_level(self, mock_service_installed): """Test on_config_change function when exporter-log-level is changed.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -309,8 +313,8 @@ def test_61_config_changed_not_okay(self, mock_service_installed): @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch.object(pathlib.Path, "exists", return_value=True) - def test_62_config_changed_not_okay(self, mock_service_installed, mock_exporter): - """Test on_config_change function when exporter-log-level is changed.""" + def test_render_config_fail(self, mock_service_installed, mock_exporter): + """Test on_config_change function when render config fails.""" rid = self.harness.add_relation(EXPORTER_RELATION_NAME, "grafana-agent") self.harness.begin() @@ -361,6 +365,7 @@ def test_render_config(self, mock_get_hw_tool_white_list): REDFISH_USERNAME="", REDFISH_CLIENT_TIMEOUT=10, ), + mode=0o600, ) @mock.patch( @@ -393,4 +398,5 @@ def test_render_config_redfish(self, mock_get_hw_tool_white_list): REDFISH_USERNAME="default_user", REDFISH_CLIENT_TIMEOUT="10", ), + mode=0o600, )