From 7ce9ecfa462e7ddad858c3d224f418fcce317829 Mon Sep 17 00:00:00 2001 From: Hetang Modi <62056057+hetangmodi-crest@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:18:00 +0530 Subject: [PATCH] chore: restructure code and add test cases (#1329) **Issue number:** [ADDON-73368](https://splunk.atlassian.net/browse/ADDON-73368) ## Summary ### Changes > Restructuring of code - moving helper functions in `utils.py` and cleaning up duplicate code present elsewhere. ### User experience > No change, the build process would work as it did before. ## Checklist If your change doesn't seem to apply, please leave them unchecked. * [x] I have performed a self-review of this change * [x] Changes have been tested * [ ] Changes are documented * [x] PR title follows [conventional commit semantics](https://www.conventionalcommits.org/en/v1.0.0/) --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Artem Rys Co-authored-by: sgoral-splunk <138458044+sgoral-splunk@users.noreply.github.com> --- mkdocs.yml | 1 + .../alert_actions_helper.py | 64 ------ .../alert_actions_merge.py | 50 ----- .../alert_actions_py_gen.py | 4 +- .../generators/file_generator.py | 2 +- splunk_add_on_ucc_framework/utils.py | 117 +++++++++-- tests/unit/generators/test_doc_generator.py | 93 +++++++++ tests/unit/generators/test_file_generator.py | 189 ++++++++++++++++++ tests/unit/generators/test_init_.py | 20 ++ 9 files changed, 405 insertions(+), 135 deletions(-) delete mode 100644 splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_helper.py create mode 100644 tests/unit/generators/test_doc_generator.py create mode 100644 tests/unit/generators/test_file_generator.py create mode 100644 tests/unit/generators/test_init_.py diff --git a/mkdocs.yml b/mkdocs.yml index a8a9e438d..4c8d86881 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -43,6 +43,7 @@ nav: - Home: "index.md" - Quickstart: "quickstart.md" - ".conf files": "dot_conf_files.md" + - Generated files: "generated_files.md" - Inputs: - "inputs/index.md" - Introduction: "inputs/index.md" diff --git a/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_helper.py b/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_helper.py deleted file mode 100644 index 8b250c4e3..000000000 --- a/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_helper.py +++ /dev/null @@ -1,64 +0,0 @@ -# -# Copyright 2024 Splunk Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -import logging -from os import makedirs, remove, path - -import addonfactory_splunk_conf_parser_lib as conf_parser - -from splunk_add_on_ucc_framework.commands.modular_alert_builder.alert_actions_merge import ( - merge_conf_file, -) - -logger = logging.getLogger("ucc_gen") - - -def write_file(file_name: str, file_path: str, content: str, **kwargs: str) -> None: - """ - :param merge_mode: only supported for .conf and .conf.spec files. - """ - logger.debug('operation="write", object="%s" object_type="file"', file_path) - - merge_mode = kwargs.get("merge_mode", "stanza_overwrite") - do_merge = False - if file_name.endswith(".conf") or file_name.endswith(".conf.spec"): - do_merge = True - else: - logger.debug( - f"'{file_name}' is not going to be merged, only .conf and " - f".conf.spec files are supported." - ) - - new_file = None - if path.exists(file_path) and do_merge: - new_file = path.join(path.dirname(file_path), "new_" + file_name) - if new_file: - try: - with open(new_file, "w+") as fhandler: - fhandler.write(content) - merge_conf_file(new_file, file_path, merge_mode=merge_mode) - finally: - if path.exists(new_file): - remove(new_file) - else: - if not path.exists(path.dirname(file_path)): - makedirs(path.dirname(file_path)) - with open(file_path, "w+") as fhandler: - fhandler.write(content) - if do_merge: - parser = conf_parser.TABConfigParser() - parser.read(file_path) - with open(file_path, "w") as df: - parser.write(df) diff --git a/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_merge.py b/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_merge.py index 4b70fb847..9c1ac2454 100644 --- a/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_merge.py +++ b/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_merge.py @@ -14,8 +14,6 @@ # limitations under the License. # import logging -import os -from os.path import basename as bn from typing import Any import addonfactory_splunk_conf_parser_lib as conf_parser @@ -27,9 +25,6 @@ arf_consts as ac, ) -merge_deny_list = ["default.meta", "README.txt"] -merge_mode_config = {"app.conf": "item_overwrite"} - logger = logging.getLogger("ucc_gen") @@ -64,48 +59,3 @@ def remove_alert_from_conf_file(alert: Any, conf_file: str) -> None: with open(conf_file, "w") as cf: parser.write(cf) - - -def merge_conf_file( - src_file: str, dst_file: str, merge_mode: str = "stanza_overwrite" -) -> None: - if not os.path.isfile(src_file): - return - if not os.path.isfile(dst_file): - return - if bn(src_file) in merge_deny_list: - return - - sparser = conf_parser.TABConfigParser() - sparser.read(src_file) - src_dict = sparser.item_dict() - parser = conf_parser.TABConfigParser() - parser.read(dst_file) - dst_dict = parser.item_dict() - - if merge_mode == "stanza_overwrite": - for stanza, key_values in list(src_dict.items()): - if stanza not in dst_dict: - parser.add_section(stanza) - else: - parser.remove_section(stanza) - parser.add_section(stanza) - - for k, v in list(key_values.items()): - parser.set(stanza, k, v) - elif merge_mode == "item_overwrite": - for stanza, key_values in list(src_dict.items()): - if stanza not in dst_dict: - parser.add_section(stanza) - - for k, v in list(key_values.items()): - if v: - parser.set(stanza, k, v) - else: - parser.remove_option(stanza, k) - else: - # overwrite the whole file - parser.read(src_file) - - with open(dst_file, "w") as df: - parser.write(df) diff --git a/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_py_gen.py b/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_py_gen.py index 30a4a31a2..01b2c6602 100644 --- a/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_py_gen.py +++ b/splunk_add_on_ucc_framework/commands/modular_alert_builder/alert_actions_py_gen.py @@ -23,9 +23,7 @@ from splunk_add_on_ucc_framework.commands.modular_alert_builder import ( arf_consts as ac, ) -from splunk_add_on_ucc_framework.commands.modular_alert_builder.alert_actions_helper import ( - write_file, -) +from splunk_add_on_ucc_framework.utils import write_file logger = logging.getLogger("ucc_gen") diff --git a/splunk_add_on_ucc_framework/generators/file_generator.py b/splunk_add_on_ucc_framework/generators/file_generator.py index 64df89a47..011d78fb2 100644 --- a/splunk_add_on_ucc_framework/generators/file_generator.py +++ b/splunk_add_on_ucc_framework/generators/file_generator.py @@ -20,7 +20,7 @@ from jinja2 import Environment, FileSystemLoader, select_autoescape -from splunk_add_on_ucc_framework.commands.modular_alert_builder.alert_actions_helper import ( +from splunk_add_on_ucc_framework.utils import ( write_file, ) from splunk_add_on_ucc_framework.commands.rest_builder.global_config_builder_schema import ( diff --git a/splunk_add_on_ucc_framework/utils.py b/splunk_add_on_ucc_framework/utils.py index 6dedcf15c..32489fc6b 100644 --- a/splunk_add_on_ucc_framework/utils.py +++ b/splunk_add_on_ucc_framework/utils.py @@ -14,23 +14,27 @@ # limitations under the License. # import json -import os +import logging import shutil +from os import listdir, makedirs, path, remove, sep +from os.path import basename as bn +from os.path import dirname, exists, isdir, join from typing import Any, Dict +import addonfactory_splunk_conf_parser_lib as conf_parser import dunamai import jinja2 import yaml from splunk_add_on_ucc_framework import exceptions +logger = logging.getLogger("ucc_gen") + def get_j2_env() -> jinja2.Environment: # nosemgrep: splunk.autoescape-disabled, python.jinja2.security.audit.autoescape-disabled.autoescape-disabled return jinja2.Environment( - loader=jinja2.FileSystemLoader( - os.path.join(os.path.dirname(__file__), "templates") - ) + loader=jinja2.FileSystemLoader(join(dirname(__file__), "templates")) ) @@ -44,17 +48,15 @@ def recursive_overwrite(src: str, dest: str, ui_source_map: bool = False) -> Non ui_source_map (bool): flag that decides if source map files should be copied """ # TODO: move to shutil.copytree("src", "dst", dirs_exist_ok=True) when Python 3.8+. - if os.path.isdir(src): - if not os.path.isdir(dest): - os.makedirs(dest) - files = os.listdir(src) + if isdir(src): + if not isdir(dest): + makedirs(dest) + files = listdir(src) for f in files: - recursive_overwrite( - os.path.join(src, f), os.path.join(dest, f), ui_source_map - ) + recursive_overwrite(join(src, f), join(dest, f), ui_source_map) else: - if os.path.exists(dest): - os.remove(dest) + if exists(dest): + remove(dest) if (".js.map" not in dest) or ui_source_map: shutil.copy(src, dest) @@ -72,11 +74,11 @@ def get_os_path(path: str) -> str: """ if "\\\\" in path: - path = path.replace("\\\\", os.sep) + path = path.replace("\\\\", sep) else: - path = path.replace("\\", os.sep) - path = path.replace("/", os.sep) - return path.strip(os.sep) + path = path.replace("\\", sep) + path = path.replace("/", sep) + return path.strip(sep) def dump_json_config(config: Dict[Any, Any], file_path: str) -> None: @@ -104,3 +106,84 @@ def get_version_from_git() -> str: except ValueError: raise exceptions.CouldNotVersionFromGitException() return f"{version.base}{stage}{version.commit}" + + +def merge_conf_file( + src_file: str, dst_file: str, merge_mode: str = "stanza_overwrite" +) -> None: + merge_deny_list = ["default.meta", "README.txt"] + if bn(src_file) in merge_deny_list: + return + + sparser = conf_parser.TABConfigParser() + sparser.read(src_file) + src_dict = sparser.item_dict() + parser = conf_parser.TABConfigParser() + parser.read(dst_file) + dst_dict = parser.item_dict() + + if merge_mode == "stanza_overwrite": + for stanza, key_values in src_dict.items(): + if stanza not in dst_dict: + parser.add_section(stanza) + else: + parser.remove_section(stanza) + parser.add_section(stanza) + + for k, v in key_values.items(): + parser.set(stanza, k, v) + elif merge_mode == "item_overwrite": + for stanza, key_values in src_dict.items(): + if stanza not in dst_dict: + parser.add_section(stanza) + + for k, v in key_values.items(): + if v: + parser.set(stanza, k, v) + else: + parser.remove_option(stanza, k) + else: + # overwrite the whole file + parser.read(src_file) + + with open(dst_file, "w") as df: + parser.write(df) + + +def write_file(file_name: str, file_path: str, content: str, **kwargs: str) -> None: + """ + :param merge_mode: only supported for .conf and .conf.spec files. + """ + logger.debug('operation="write", object="%s" object_type="file"', file_path) + + merge_mode = kwargs.get("merge_mode", "stanza_overwrite") + do_merge = False + if file_name.endswith(".conf") or file_name.endswith(".conf.spec"): + do_merge = True + else: + logger.debug( + f"'{file_name}' is not going to be merged, only .conf and " + f".conf.spec files are supported." + ) + + new_file = None + if path.exists(file_path) and do_merge: + new_file = path.join(path.dirname(file_path), "new_" + file_name) + if new_file: + try: + with open(new_file, "w+") as fhandler: + fhandler.write(content) + merge_conf_file(new_file, file_path, merge_mode=merge_mode) + finally: + if path.exists(new_file): + remove(new_file) + else: + if not path.exists(path.dirname(file_path)): + makedirs(path.dirname(file_path)) + with open(file_path, "w+") as fhandler: + fhandler.write(content) + if do_merge: + parser = conf_parser.TABConfigParser() + parser.read(file_path) + with open(file_path, "w") as df: + parser.write(df) diff --git a/tests/unit/generators/test_doc_generator.py b/tests/unit/generators/test_doc_generator.py new file mode 100644 index 000000000..0b6ac311f --- /dev/null +++ b/tests/unit/generators/test_doc_generator.py @@ -0,0 +1,93 @@ +from unittest.mock import patch, mock_open, MagicMock +from splunk_add_on_ucc_framework.generators import doc_generator +from os.path import join +from splunk_add_on_ucc_framework.generators.file_const import FileClass + + +MOCKED_GEN_FILE_LIST = [ + FileClass("file1.conf", MagicMock(), "some/path", "Conf file"), + FileClass("file2.xml", MagicMock(), ["xml", "path"], "XML file"), +] + + +@patch( + "splunk_add_on_ucc_framework.generators.doc_generator.realpath", + return_value="/fake/dir", +) +@patch( + "splunk_add_on_ucc_framework.generators.doc_generator.dirname", + return_value="/fake/dir", +) +@patch("builtins.open", new_callable=mock_open) +@patch.object(doc_generator, "GEN_FILE_LIST", MOCKED_GEN_FILE_LIST) +def test_generate_docs(mock_open, mock_dirname, mock_realpath): + doc_generator.generate_docs() + + # Verify the correct file path is being opened + expected_path = join("/fake/dir", "docs", "generated_files.md") + mock_open.assert_called_once_with(expected_path, "w") + + # Check the content written to the file + written_content = "\n".join( + [ + "---", + "title: UCC framework generated files", + "---", + "", + "Below table describes the files generated by UCC framework", + "", + "## File Description", + "", + "| File Name | File Location | File Description |", + "| ------------ | ------------ | ----------------- |", + "| file1.conf | output/<YOUR_ADDON_NAME>/some/path | Conf file |", + "| file2.xml | output/<YOUR_ADDON_NAME>/xml/path | XML file |", + "\n", + ] + ) + + mock_open().write.assert_called_once_with(written_content) + + +@patch( + "splunk_add_on_ucc_framework.generators.doc_generator.realpath", + return_value="/fake/dir", +) +@patch( + "splunk_add_on_ucc_framework.generators.doc_generator.dirname", + return_value="/fake/dir", +) +@patch("builtins.open", new_callable=mock_open) +@patch.object(doc_generator, "GEN_FILE_LIST", []) +def test_generate_docs_empty_list(mock_open_file, mock_dirname, mock_realpath): + doc_generator.generate_docs() + + # Verify the correct file path is being opened + expected_path = join("/fake/dir", "docs", "generated_files.md") + mock_open_file.assert_called_once_with(expected_path, "w") + + # Check the content written to the file + written_content = "\n".join( + [ + "---", + "title: UCC framework generated files", + "---", + "", + "Below table describes the files generated by UCC framework", + "", + "## File Description", + "", + "| File Name | File Location | File Description |", + "| ------------ | ------------ | ----------------- |", + "\n", + ] + ) + + mock_open_file().write.assert_called_once_with(written_content) + + +# Test main function +@patch("splunk_add_on_ucc_framework.generators.doc_generator.generate_docs") +def test_main(mock_generate_docs): + doc_generator.main() + mock_generate_docs.assert_called_once() diff --git a/tests/unit/generators/test_file_generator.py b/tests/unit/generators/test_file_generator.py new file mode 100644 index 000000000..b6091c346 --- /dev/null +++ b/tests/unit/generators/test_file_generator.py @@ -0,0 +1,189 @@ +from splunk_add_on_ucc_framework.generators import FileGenerator +from splunk_add_on_ucc_framework.generators.file_generator import begin +from splunk_add_on_ucc_framework.global_config import GlobalConfig +from unittest.mock import patch, MagicMock +from pytest import raises, fixture +from tests.unit.helpers import get_testdata_file_path +from jinja2 import Template + + +@fixture +def global_config(): + return GlobalConfig(get_testdata_file_path("valid_config.json")) + + +@fixture +def input_dir(tmp_path): + return str(tmp_path / "input_dir") + + +@fixture +def output_dir(tmp_path): + return str(tmp_path / "output_dir") + + +@fixture +def ucc_dir(tmp_path): + return str(tmp_path / "ucc_dir") + + +@fixture +def ta_name(): + return "test_addon" + + +@fixture +def set_attr(): + return {"file_name": "file_path"} + + +def mocked__set_attribute(this, **kwargs): + this.attrib_1 = "value_1" + this.attrib_2 = "value_2" + + +@patch("splunk_add_on_ucc_framework.generators.FileGenerator._set_attributes") +def test_file_generator_init( + mock_set_attribute, global_config, input_dir, output_dir, ucc_dir, ta_name +): + file_gen = FileGenerator( + global_config, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + assert file_gen._global_config == global_config + assert file_gen._input_dir == input_dir + assert file_gen._output_dir == output_dir + assert file_gen._addon_name == "test_addon" + assert file_gen._template_dir == [f"{ucc_dir}/templates"] + + file_gen_none = FileGenerator( + None, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + assert file_gen_none._gc_schema is None + + +@patch("splunk_add_on_ucc_framework.generators.FileGenerator._set_attributes") +def test_get_output_dir(global_config, input_dir, output_dir, ucc_dir, ta_name): + file_gen = FileGenerator( + global_config, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + expected_output_dir = f"{output_dir}/test_addon" + assert file_gen._get_output_dir() == expected_output_dir + + +@patch("splunk_add_on_ucc_framework.generators.FileGenerator._set_attributes") +@patch( + "splunk_add_on_ucc_framework.generators.FileGenerator._get_output_dir", + return_value="tmp/path", +) +def test_get_file_output_path(global_config, input_dir, output_dir, ucc_dir, ta_name): + file_gen = FileGenerator( + global_config, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + + # Test with string + result = file_gen.get_file_output_path("output_file") + assert result == "tmp/path/output_file" + + # Test with list + result = file_gen.get_file_output_path(["dir1", "dir2", "output_file"]) + assert result == "tmp/path/dir1/dir2/output_file" + + # Test with invalid type + with raises(TypeError): + file_gen.get_file_output_path({"path": "/dummy/path"}) # type: ignore[arg-type] + + +@patch("splunk_add_on_ucc_framework.generators.FileGenerator._set_attributes") +@patch("jinja2.Environment.get_template") +def test_set_template_and_render( + mock_get_template, + mock_set_attribute, + global_config, + input_dir, + output_dir, + ucc_dir, + ta_name, +): + file_gen = FileGenerator( + global_config, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + + mock_get_template.return_value = Template("mock template") + + file_gen.set_template_and_render(["dir1"], "test.template") + mock_get_template.assert_called_once_with("test.template") + + +@patch("splunk_add_on_ucc_framework.generators.FileGenerator._set_attributes") +@patch("jinja2.Environment.get_template") +def test_set_template_and_render_invalid_file_name( + mock_get_template, + mock_set_attribute, + global_config, + input_dir, + output_dir, + ucc_dir, + ta_name, +): + file_gen = FileGenerator( + global_config, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + mock_get_template.return_value = Template("mock template") + # Test with invalid file name + with raises(AssertionError): + file_gen.set_template_and_render(["dir1"], "test.invalid") + + +@patch( + "splunk_add_on_ucc_framework.generators.file_generator.fc.GEN_FILE_LIST", + new_callable=list, +) +@patch("splunk_add_on_ucc_framework.generators.file_generator.logger") +def test_begin( + mock_logger, + mock_gen_file_list, + global_config, + input_dir, + output_dir, + ucc_dir, + ta_name, +): + mock_item = MagicMock() + mock_item.file_class.return_value.generate.return_value = { + "file1": "/path/to/file1" + } + + mock_gen_file_list.extend([mock_item]) + + result = begin( + global_config, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + + assert result == [{"file1": "/path/to/file1"}] + mock_logger.info.assert_called_once_with( + "Successfully generated 'file1' at '/path/to/file1'." + ) + + +def test__set_attributes_error(global_config, input_dir, output_dir, ucc_dir, ta_name): + """ + This tests that the exception provided in side_effect is raised too + """ + with raises(NotImplementedError): + FileGenerator( + global_config, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + + +@patch("splunk_add_on_ucc_framework.generators.FileGenerator._set_attributes") +def test_generate( + mock_set_attribute, global_config, input_dir, output_dir, ucc_dir, ta_name +): + """ + This tests that the exception provided in side_effect is raised too + """ + file_gen = FileGenerator( + global_config, input_dir, output_dir, ucc_dir=ucc_dir, addon_name=ta_name + ) + with raises(NotImplementedError): + file_gen.generate() diff --git a/tests/unit/generators/test_init_.py b/tests/unit/generators/test_init_.py new file mode 100644 index 000000000..01b48a693 --- /dev/null +++ b/tests/unit/generators/test_init_.py @@ -0,0 +1,20 @@ +def test___init__gen(): + expected_classes = [ + "FileGenerator", + "GEN_FILE_LIST", + "FileClass", + ] + expected_modules = [ + "file_const", + "file_generator", + ] + import splunk_add_on_ucc_framework.generators as gen + + assert gen.__all__ == expected_classes + + not_allowed = ["conf_files", "xml_files", "html_files", "doc_generator"] + for attrib in dir(gen): + if attrib.startswith("__") and attrib.endswith("__") or attrib in not_allowed: + # ignore the builtin modules + continue + assert attrib in expected_classes or attrib in expected_modules