diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000..33ac2a07 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,59 @@ +# For most projects, this workflow file will not need changing; you simply need +# to commit it to your repository. +# +# You may wish to alter this file to override the set of languages analyzed, +# or to provide custom queries or build logic. +# +# ******** NOTE ******** +# We have attempted to detect the languages in your repository. Please check +# the `language` matrix defined below to confirm you have the correct set of +# supported CodeQL languages. +# +name: "CodeQL" + +on: + push: + branches: ["main", "development"] + pull_request: + branches: ["main", "development"] + schedule: + - cron: "0 0 * * 1" + +permissions: + contents: read + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + + strategy: + fail-fast: false + matrix: + language: ["javascript", "python"] + + steps: + - name: Harden Runner + uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4 + with: + egress-policy: audit + + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Initialize CodeQL + uses: github/codeql-action/init@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2 + with: + languages: ${{ matrix.language }} + + - name: Autobuild + uses: github/codeql-action/autobuild@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2 + with: + category: "/language:${{matrix.language}}" diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml new file mode 100644 index 00000000..1b495dbc --- /dev/null +++ b/.github/workflows/dependency-review.yml @@ -0,0 +1,27 @@ +# Dependency Review Action +# +# This Action will scan dependency manifest files that change as part of a Pull Request, +# surfacing known-vulnerable versions of the packages declared or updated in the PR. +# Once installed, if the workflow run is marked as required, +# PRs introducing known-vulnerable packages will be blocked from merging. +# +# Source repository: https://github.com/actions/dependency-review-action +name: 'Dependency Review' +on: [pull_request] + +permissions: + contents: read + +jobs: + dependency-review: + runs-on: ubuntu-latest + steps: + - name: Harden Runner + uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4 + with: + egress-policy: audit + + - name: 'Checkout Repository' + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - name: 'Dependency Review' + uses: actions/dependency-review-action@3b139cfc5fae8b618d3eae3675e383bb1769c019 # v4.5.0 diff --git a/.github/workflows/github-actions-ansible-lint.yml b/.github/workflows/github-actions-ansible-lint.yml index a7ad2e38..b2fe8a73 100644 --- a/.github/workflows/github-actions-ansible-lint.yml +++ b/.github/workflows/github-actions-ansible-lint.yml @@ -19,7 +19,7 @@ jobs: - name: Setup Python uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 #v5.4.0 with: - python-version: '3.x' + python-version: '3.10' - name: Install dependencies run: | diff --git a/.github/workflows/github-actions-code-coverage.yml b/.github/workflows/github-actions-code-coverage.yml index 3bda5b78..ebc7c601 100644 --- a/.github/workflows/github-actions-code-coverage.yml +++ b/.github/workflows/github-actions-code-coverage.yml @@ -19,7 +19,7 @@ jobs: - name: Setup Python uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 #v5.4.0 with: - python-version: '3.x' + python-version: '3.10' - name: Install dependencies run: | @@ -30,6 +30,10 @@ jobs: run: | pytest --cov=src/ --cov-fail-under=85 --cov-report=xml tests/ + - name: Run pylint + run: | + pylint --load-plugins=pylint.extensions.docparams --fail-under=9 --disable=R $(git ls-files '*.py') --rcfile=./pyproject.toml + - name: Check code formatting with black run: | black --check src/ tests/ --config pyproject.toml diff --git a/.github/workflows/ossf-scoreboard.yml b/.github/workflows/ossf-scoreboard.yml new file mode 100644 index 00000000..9b69a86a --- /dev/null +++ b/.github/workflows/ossf-scoreboard.yml @@ -0,0 +1,51 @@ +# This workflow uses actions that are not certified by GitHub. They are provided +# by a third-party and are governed by separate terms of service, privacy +# policy, and support documentation. + +name: Scorecard supply-chain security +on: + branch_protection_rule: + schedule: + - cron: '32 4 * * 5' + push: + branches: [ "main" ] + +permissions: read-all + +jobs: + analysis: + name: Scorecard analysis + runs-on: ubuntu-latest + permissions: + security-events: write + id-token: write + + steps: + - name: Harden Runner + uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4 + with: + egress-policy: audit + + - name: "Checkout code" + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + + - name: "Run analysis" + uses: ossf/scorecard-action@62b2cac7ed8198b15735ed49ab1e5cf35480ba46 # v2.4.0 + with: + results_file: results.sarif + results_format: sarif + publish_results: true + + - name: "Upload artifact" + uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 + with: + name: SARIF file + path: results.sarif + retention-days: 5 + + - name: "Upload to code-scanning" + uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2 + with: + sarif_file: results.sarif diff --git a/.github/workflows/trivy.yml b/.github/workflows/trivy.yml new file mode 100644 index 00000000..737362b0 --- /dev/null +++ b/.github/workflows/trivy.yml @@ -0,0 +1,43 @@ +--- + name: trivy + + on: + pull_request: + types: [ 'opened', 'reopened', 'synchronize' ] + merge_group: + workflow_dispatch: + + + permissions: + actions: read + contents: read + security-events: write + + jobs: + build: + name: 'trivy scan' + runs-on: ubuntu-latest + steps: + - name: Harden Runner + uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4 + with: + egress-policy: audit + + - name: Checkout code + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Run Trivy vulnerability scanner (file system) + uses: aquasecurity/trivy-action@18f2510ee396bbf400402947b394f2dd8c87dbb0 # 0.29.0 + with: + scan-type: 'fs' + ignore-unfixed: true + scan-ref: . + format: 'sarif' + scanners: 'vuln,secret,config' + output: report-fs.sarif + + - name: Upload Trivy report (fs) GitHub Security + uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2 + with: + sarif_file: report-fs.sarif + category: 'fs' diff --git a/pyproject.toml b/pyproject.toml index 5c40f1e3..175a8762 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,43 +7,62 @@ load-plugins = ["pylint.extensions.docparams"] [tool.pylint.basic] argument-naming-style = "snake_case" attr-naming-style = "snake_case" -bad-names = ["foo", "bar", "baz", "toto", "tutu", "tata"] class-naming-style = "PascalCase" -docstring-min-length = 10 function-naming-style = "snake_case" variable-naming-style = "snake_case" - +module-naming-style = "snake_case" +bad-names = ["foo", "bar", "baz", "toto", "tutu", "tata"] +docstring-min-length = 10 [tool.pylint.format] max-line-length = 100 max-module-lines = 1000 +[tool.pylint.docs] +docstring-style = "sphinx" +default-docstring-type = "sphinx" +accept-no-param-doc = false +accept-no-raise-doc = false +accept-no-return-doc = false +accept-no-yields-doc = false + [tool.pylint."messages control"] enable = [ - "C0116", - "C0115", - "C0114", - "C0301", - "E1101", - "W0611", + "missing-module-docstring", + "missing-class-docstring", + "wrong-exception-operation", + "wrong-spelling-in-comment", + "wrong-spelling-in-docstring", + "missing-any-param-doc", + "missing-format-attribute", + "missing-kwoa", + "missing-param-doc", + "missing-parentheses-for-call-in-test", + "missing-raises-doc", + "missing-return-doc", + "missing-return-type-doc", + "missing-timeout", + "missing-type-doc", + "missing-yield-doc", + "missing-yield-type-doc", + "trailing-newlines", + "trailing-whitespace", ] disable = [ + "C0199", # empty-first-line-docstring "W0702", # bare-except "W0703", # broad-except "W4901", # global-statement - "R0902", # too-many-instance-attributes - "R0903", # too-few-public-methods - "R1702", - "R0801", - "W0108" + "W0108", # lambda + "W0622", # redefined-builtin id + "E0015", + "E0401", # import-error + "E0611", # no-name-in-module ] [tool.pylint.design] max-args = 5 -[tool.pylint.docs] -docstring-min-length = 10 - [tool.pylint.variables] init-import = false dummy-variables-rgx = "_.*|dummy" @@ -56,3 +75,20 @@ enable = [ "E1101", "W0611", ] + +[tool.pylint.tests] +disable = [ + "W0702", # bare-except + "W0703", # broad-except + "W4901", # global-statement + "R0902", # too-many-instance-attributes + "R0903", # too-few-public-methods + "R1702", # too-many-nested-blocks + "R0801", # duplicate-code + "W0108", # lambda + "E0401", # import-error + "W0613", # unused-argument + "W0212", # protected-access + "W0107", # unnecessary-pass + "C0103" # invalid-name +] diff --git a/requirements.txt b/requirements.txt index fed477ef..80285b1c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,7 @@ ansible-compat==24.6.1 ansible-core==2.17.7 ansible-lint==24.6.1 ansible-runner==2.4.0 +astroid==2.9.0 attrs==25.1.0 azure-common==1.1.28 azure-core==1.32.0 @@ -27,13 +28,17 @@ ijson==3.3.0 importlib_metadata==8.6.1 iniconfig==2.0.0 isodate==0.7.2 +isort==5.13.2 Jinja2==3.1.6 jsonschema==4.23.0 jsonschema-specifications==2024.10.1 +lazy-object-proxy==1.10.0 lockfile==0.12.2 markdown-it-py==3.0.0 MarkupSafe==3.0.2 +mccabe==0.6.1 mdurl==0.1.2 +mock==5.1.0 msal==1.31.1 msal-extensions==1.2.0 mypy-extensions==1.0.0 @@ -49,6 +54,7 @@ ptyprocess==0.7.0 pycparser==2.22 Pygments==2.19.1 PyJWT==2.10.1 +pylint==2.12.2 pytest==8.3.4 pytest-cov==6.0.0 pytest-mock==3.14.0 @@ -66,10 +72,12 @@ ruamel.yaml.clib==0.2.12 six==1.17.0 subprocess-tee==0.4.2 tenacity==9.0.0 +toml==0.10.2 tomli==2.2.1 typing_extensions==4.12.2 tzdata==2025.1 urllib3==2.2.2 wcmatch==10.0 +wrapt==1.13.3 yamllint==1.35.1 zipp==3.21.0 diff --git a/src/module_utils/get_cluster_status.py b/src/module_utils/get_cluster_status.py index 75b50984..b727d0b0 100644 --- a/src/module_utils/get_cluster_status.py +++ b/src/module_utils/get_cluster_status.py @@ -117,8 +117,8 @@ def run(self) -> Dict[str, str]: self.result["message"] = "Pacemaker cluster isn't stable" self.log(logging.WARNING, self.result["message"]) - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) self.result["end"] = datetime.now() self.result["status"] = TestStatus.SUCCESS.value diff --git a/src/module_utils/sap_automation_qa.py b/src/module_utils/sap_automation_qa.py index a4496fa7..2e9f7c6d 100644 --- a/src/module_utils/sap_automation_qa.py +++ b/src/module_utils/sap_automation_qa.py @@ -3,7 +3,6 @@ and setup base variables for the test case running in the sap-automation-qa """ -import os from abc import ABC from enum import Enum import sys @@ -153,12 +152,12 @@ def execute_command_subprocess(self, command: str, shell_command: bool = False) stdout = command_output.stdout.decode("utf-8") stderr = command_output.stderr.decode("utf-8") return stdout if not stderr else stderr - except subprocess.TimeoutExpired as e: - self.handle_error(e, "Command timed out") - except subprocess.CalledProcessError as e: - self.handle_error(e, e.stderr.decode("utf-8").strip()) - except Exception as e: - self.handle_error(e, "") + except subprocess.TimeoutExpired as ex: + self.handle_error(ex, "Command timed out") + except subprocess.CalledProcessError as ex: + self.handle_error(ex, ex.stderr.decode("utf-8").strip()) + except Exception as ex: + self.handle_error(ex, "") return "" def parse_xml_output(self, xml_output: str) -> Optional[ET.Element]: diff --git a/src/modules/check_indexserver.py b/src/modules/check_indexserver.py index 94167f68..03ae5607 100644 --- a/src/modules/check_indexserver.py +++ b/src/modules/check_indexserver.py @@ -117,11 +117,11 @@ def check_indexserver(self) -> None: "indexserver_enabled": "no", } ) - except Exception as e: + except Exception as ex: self.result.update( { "status": TestStatus.ERROR.value, - "message": f"Exception occurred while checking indexserver configuration: {e}", + "message": f"Exception occurred while checking indexserver configuration: {ex}", "details": {}, "indexserver_enabled": "no", } diff --git a/src/modules/filesystem_freeze.py b/src/modules/filesystem_freeze.py index d659a33c..f9fb993b 100644 --- a/src/modules/filesystem_freeze.py +++ b/src/modules/filesystem_freeze.py @@ -21,11 +21,6 @@ class FileSystemFreeze(SapAutomationQA): Class to run the test case when the filesystem is frozen. """ - def __init__( - self, - ): - super().__init__() - def _find_filesystem(self) -> str: """ Find the filesystem mounted on /hana/shared. @@ -39,8 +34,8 @@ def _find_filesystem(self) -> str: parts = line.split() if len(parts) > 1 and parts[1] == "/hana/shared": return parts[0] - except FileNotFoundError as e: - self.handle_error(e) + except FileNotFoundError as ex: + self.handle_error(ex) return None def run(self) -> Dict[str, Any]: diff --git a/src/modules/get_azure_lb.py b/src/modules/get_azure_lb.py index 56bb50d3..2a95dfd7 100644 --- a/src/modules/get_azure_lb.py +++ b/src/modules/get_azure_lb.py @@ -47,9 +47,9 @@ def _create_network_client(self): self.network_client = NetworkManagementClient( self.credential, self.module_params["subscription_id"] ) - except Exception as e: - self.handle_error(e) - self.result["message"] += f"Failed to create network client object. {e} \n" + except Exception as ex: + self.handle_error(ex) + self.result["message"] += f"Failed to create network client object. {ex} \n" def get_load_balancers(self) -> list: """ @@ -66,9 +66,9 @@ def get_load_balancers(self) -> list: if lb.location.lower() == self.module_params["region"].lower() ] - except Exception as e: - self.handle_error(e) - self.result["message"] += f"Failed to create network client object. {e} \n" + except Exception as ex: + self.handle_error(ex) + self.result["message"] += f"Failed to create network client object. {ex} \n" def get_load_balancers_details(self) -> dict: """ @@ -139,11 +139,11 @@ def check_parameters(entity, parameters_dict, entity_type): self.constants["RULES"], "load_balancing_rule", ) - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) self.result[ "message" - ] += f"Failed to validate load balancer rule parameters. {e} \n" + ] += f"Failed to validate load balancer rule parameters. {ex} \n" continue for probe in found_load_balancer["probes"]: @@ -153,11 +153,11 @@ def check_parameters(entity, parameters_dict, entity_type): self.constants["PROBES"], "probes", ) - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) self.result[ "message" - ] += f"Failed to validate load balancer probe parameters. {e} \n" + ] += f"Failed to validate load balancer probe parameters. {ex} \n" continue failed_parameters = [ @@ -179,8 +179,8 @@ def check_parameters(entity, parameters_dict, entity_type): else: self.result["message"] += "No load balancer found" - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) def run_module(): diff --git a/src/modules/get_cluster_status_scs.py b/src/modules/get_cluster_status_scs.py index 79e0255c..4954e46b 100644 --- a/src/modules/get_cluster_status_scs.py +++ b/src/modules/get_cluster_status_scs.py @@ -5,7 +5,6 @@ Python script to get and validate the status of an SCS cluster. """ -import logging import xml.etree.ElementTree as ET from typing import Dict, Any from ansible.module_utils.basic import AnsibleModule diff --git a/src/modules/get_package_list.py b/src/modules/get_package_list.py index 4042004c..6c2031b6 100644 --- a/src/modules/get_package_list.py +++ b/src/modules/get_package_list.py @@ -57,8 +57,8 @@ def format_packages(self) -> Dict[str, Any]: for package in PACKAGE_LIST if package["key"] in self.package_facts_list ] - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) self.result["status"] = TestStatus.SUCCESS.value return self.result diff --git a/src/modules/get_pcmk_properties_db.py b/src/modules/get_pcmk_properties_db.py index f1b00737..f1863b90 100644 --- a/src/modules/get_pcmk_properties_db.py +++ b/src/modules/get_pcmk_properties_db.py @@ -85,7 +85,14 @@ def __init__( def _get_expected_value(self, category, name): """ - Get expected value for basic configuration parameters. + Get expected value for a given configuration parameter. + + :param category: The category of the configuration parameter. + :type category: str + :param name: The name of the configuration parameter. + :type name: str + :return: The expected value for the configuration parameter. + :rtype: str """ _, defaults_key = self.BASIC_CATEGORIES[category] @@ -96,7 +103,18 @@ def _get_expected_value(self, category, name): def _get_resource_expected_value(self, resource_type, section, param_name, op_name=None): """ - Get expected value for resource-specific configuration parameters. + Get expected value for a given resource configuration parameter. + + :param resource_type: The type of the resource. + :type resource_type: str + :param section: The section of the resource configuration. + :type section: str + :param param_name: The name of the configuration parameter. + :type param_name: str + :param op_name: The name of the operation (if applicable), defaults to None + :type op_name: str, optional + :return: The expected value for the resource configuration parameter. + :rtype: str """ resource_defaults = self.constants["RESOURCE_DEFAULTS"].get(self.os_type, {}) @@ -125,7 +143,24 @@ def _create_parameter( op_name=None, ): """ - Create a Parameters object for a given configuration parameter. + Create a parameter dictionary for the given configuration. + + :param category: The category of the configuration parameter. + :type category: str + :param name: The name of the configuration parameter. + :type name: str + :param value: The value of the configuration parameter. + :type value: str + :param expected_value: The expected value for the configuration parameter, defaults to None + :type expected_value: str, optional + :param id: The ID of the configuration parameter, defaults to None + :type id: str, optional + :param subcategory: The subcategory of the configuration parameter, defaults to None + :type subcategory: str, optional + :param op_name: The name of the operation (if applicable), defaults to None + :type op_name: str, optional + :return: A dictionary representing the parameter. + :rtype: dict """ if expected_value is None: if category in self.RESOURCE_CATEGORIES: @@ -157,7 +192,18 @@ def _create_parameter( def _parse_nvpair_elements(self, elements, category, subcategory=None, op_name=None): """ - Parse nvpair elements and return a list of Parameters objects. + Parse nvpair elements and create parameter dictionaries. + + :param elements: List of nvpair elements to parse. + :type elements: list + :param category: The category of the configuration parameter. + :type category: str + :param subcategory: The subcategory of the configuration parameter, defaults to None + :type subcategory: str, optional + :param op_name: The name of the operation (if applicable), defaults to None + :type op_name: str, optional + :return: A list of parameter dictionaries. + :rtype: list """ parameters = [] for nvpair in elements: @@ -175,7 +221,10 @@ def _parse_nvpair_elements(self, elements, category, subcategory=None, op_name=N def _parse_os_parameters(self): """ - Parse OS-specific parameters + Parse and validate OS-specific configuration parameters. + + :return: A list of parameter dictionaries containing validation results. + :rtype: list """ parameters = [] @@ -203,6 +252,9 @@ def _parse_os_parameters(self): def _parse_global_ini_parameters(self): """ Parse global.ini parameters + + :return: A list of parameter dictionaries containing validation results. + :rtype: list """ parameters = [] global_ini_defaults = self.constants["GLOBAL_INI"].get(self.os_type, {}) @@ -240,6 +292,15 @@ def _parse_global_ini_parameters(self): def _parse_basic_config(self, element, category, subcategory=None): """ Parse basic configuration parameters + + :param element: The XML element to parse. + :type element: xml.etree.ElementTree.Element + :param category: The category of the configuration parameter. + :type category: str + :param subcategory: The subcategory of the configuration parameter, defaults to None + :type subcategory: str, optional + :return: A list of parameter dictionaries. + :rtype: list """ parameters = [] for nvpair in element.findall(".//nvpair"): @@ -257,6 +318,13 @@ def _parse_basic_config(self, element, category, subcategory=None): def _parse_resource(self, element, category): """ Parse resource-specific configuration parameters + + :param element: The XML element to parse. + :type element: xml.etree.ElementTree.Element + :param category: The category of the resource. + :type category: str + :return: A list of parameter dictionaries. + :rtype: list """ parameters = [] @@ -279,18 +347,18 @@ def _parse_resource(self, element, category): ) ) - ops = element.find(".//operations") - if ops is not None: - for op in ops.findall(".//op"): + operations = element.find(".//operations") + if operations is not None: + for operation in operations.findall(".//op"): for op_type in ["timeout", "interval"]: parameters.append( self._create_parameter( category=category, subcategory="operations", - id=op.get("id", ""), + id=operation.get("id", ""), name=op_type, - op_name=op.get("name", ""), - value=op.get(op_type, ""), + op_name=operation.get("name", ""), + value=operation.get(op_type, ""), ) ) return parameters @@ -298,6 +366,11 @@ def _parse_resource(self, element, category): def _parse_constraints(self, root): """ Parse constraints configuration parameters + + :param root: The XML root element to parse. + :type root: xml.etree.ElementTree.Element + :return: A list of parameter dictionaries. + :rtype: list """ parameters = [] for element in root: @@ -346,10 +419,10 @@ def parse_ha_cluster_config(self): xpath = self.BASIC_CATEGORIES[self.category][0] for element in root.findall(xpath): parameters.extend(self._parse_basic_config(element, self.category)) - except Exception as e: + except Exception as ex: self.result[ "message" - ] += f"Failed to get {self.category} configuration: {str(e)}" + ] += f"Failed to get {self.category} configuration: {str(ex)}" continue elif self.category == "resources": @@ -358,28 +431,28 @@ def parse_ha_cluster_config(self): elements = root.findall(xpath) for element in elements: parameters.extend(self._parse_resource(element, sub_category)) - except Exception as e: + except Exception as ex: self.result[ "message" - ] += f"Failed to get resources configuration for {self.category}: {str(e)}" + ] += f"Failed to get resources configuration for {self.category}: {str(ex)}" continue elif self.category == "constraints": try: parameters.extend(self._parse_constraints(root)) - except Exception as e: - self.result["message"] += f"Failed to get constraints configuration: {str(e)}" + except Exception as ex: + self.result["message"] += f"Failed to get constraints configuration: {str(ex)}" continue try: parameters.extend(self._parse_os_parameters()) - except Exception as e: - self.result["message"] += f"Failed to get OS parameters: {str(e)} \n" + except Exception as ex: + self.result["message"] += f"Failed to get OS parameters: {str(ex)} \n" try: parameters.extend(self._parse_global_ini_parameters()) - except Exception as e: - self.result["message"] += f"Failed to get global.ini parameters: {str(e)} \n" + except Exception as ex: + self.result["message"] += f"Failed to get global.ini parameters: {str(ex)} \n" failed_parameters = [ param diff --git a/src/modules/get_pcmk_properties_scs.py b/src/modules/get_pcmk_properties_scs.py index c3d5ff6b..edb54aca 100644 --- a/src/modules/get_pcmk_properties_scs.py +++ b/src/modules/get_pcmk_properties_scs.py @@ -82,6 +82,13 @@ def __init__( def _get_expected_value(self, category, name): """ Get expected value for basic configuration parameters. + + :param category: Category of the parameter + :type category: str + :param name: Name of the parameter + :type name: str + :return: Expected value of the parameter + :rtype: str """ _, defaults_key = self.BASIC_CATEGORIES[category] @@ -93,6 +100,17 @@ def _get_expected_value(self, category, name): def _get_resource_expected_value(self, resource_type, section, param_name, op_name=None): """ Get expected value for resource-specific configuration parameters. + + :param resource_type: Type of the resource (e.g., stonith, ipaddr) + :type resource_type: str + :param section: Section of the resource (e.g., meta_attributes, operations) + :type section: str + :param param_name: Name of the parameter + :type param_name: str + :param op_name: Name of the operation (if applicable) + :type op_name: str + :return: Expected value of the parameter + :rtype: str """ resource_defaults = ( self.constants["RESOURCE_DEFAULTS"].get(self.os_type, {}).get(resource_type, {}) @@ -119,6 +137,23 @@ def _create_parameter( ): """ Create a Parameters object for a given configuration parameter. + + :param category: Category of the parameter + :type category: str + :param name: Name of the parameter + :type name: str + :param value: Value of the parameter + :type value: str + :param expected_value: Expected value of the parameter + :type expected_value: str + :param id: ID of the parameter (optional) + :type id: str + :param subcategory: Subcategory of the parameter (optional) + :type subcategory: str + :param op_name: Operation name (optional) + :type op_name: str + :return: Parameters object + :rtype: Parameters """ if expected_value is None: if category in self.RESOURCE_CATEGORIES or category in ["ascs", "ers"]: @@ -151,6 +186,17 @@ def _create_parameter( def _parse_nvpair_elements(self, elements, category, subcategory=None, op_name=None): """ Parse nvpair elements and return a list of Parameters objects. + + :param elements: List of XML elements to parse + :type elements: List[ElementTree.Element] + :param category: Category of the parameters + :type category: str + :param subcategory: Subcategory of the parameters + :type subcategory: str + :param op_name: Operation name (if applicable) + :type op_name: str + :return: List of Parameters objects + :rtype: List[Parameters] """ parameters = [] for nvpair in elements: @@ -169,6 +215,13 @@ def _parse_nvpair_elements(self, elements, category, subcategory=None, op_name=N def _parse_resource(self, element, category): """ Parse resource-specific configuration parameters + + :param element: XML element to parse + :type element: ElementTree.Element + :param category: Resource category (e.g., stonith, ipaddr) + :type category: str + :return: List of Parameters objects for the resource + :rtype: List[Parameters] """ parameters = [] @@ -183,18 +236,18 @@ def _parse_resource(self, element, category): ) ) - ops = element.find(".//operations") - if ops is not None: - for op in ops.findall(".//op"): + operations = element.find(".//operations") + if operations is not None: + for operation in operations.findall(".//op"): for op_type in ["timeout", "interval"]: parameters.append( self._create_parameter( category=category, subcategory="operations", - id=op.get("id", ""), + id=operation.get("id", ""), name=op_type, - op_name=op.get("name", ""), - value=op.get(op_type, ""), + op_name=operation.get("name", ""), + value=operation.get(op_type, ""), ) ) return parameters @@ -202,6 +255,15 @@ def _parse_resource(self, element, category): def _parse_basic_config(self, element, category, subcategory=None): """ Parse basic configuration parameters + + :param element: XML element to parse + :type element: ElementTree.Element + :param category: Category of the parameters + :type category: str + :param subcategory: Subcategory of the parameters + :type subcategory: str + :return: List of Parameters objects for basic configuration + :rtype: List[Parameters] """ parameters = [] for nvpair in element.findall(".//nvpair"): @@ -219,6 +281,9 @@ def _parse_basic_config(self, element, category, subcategory=None): def _parse_os_parameters(self): """ Parse OS-specific parameters + + :return: List of Parameters objects for OS parameters + :rtype: List[Parameters] """ parameters = [] @@ -246,6 +311,11 @@ def _parse_os_parameters(self): def _parse_constraints(self, root): """ Parse constraints configuration parameters + + :param root: XML root element + :type root: ElementTree.Element + :return: List of Parameters objects for constraints + :rtype: List[Parameters] """ parameters = [] for element in root: @@ -292,10 +362,10 @@ def parse_ha_cluster_config(self): xpath = self.BASIC_CATEGORIES[self.category][0] for element in root.findall(xpath): parameters.extend(self._parse_basic_config(element, self.category)) - except Exception as e: + except Exception as ex: self.result[ "message" - ] += f"Failed to get {self.category} configuration: {str(e)}" + ] += f"Failed to get {self.category} configuration: {str(ex)}" continue elif self.category == "resources": @@ -314,10 +384,10 @@ def parse_ha_cluster_config(self): for element in group.findall(".//primitive[@type='SAPInstance']"): parameters.extend(self._parse_resource(element, "ers")) - except Exception as e: + except Exception as ex: self.result[ "message" - ] += f"Failed to get resources configuration for {self.category}: {str(e)}" + ] += f"Failed to get resources configuration for {self.category}: {str(ex)}" continue elif self.category == "constraints": @@ -329,8 +399,8 @@ def parse_ha_cluster_config(self): try: parameters.extend(self._parse_os_parameters()) - except Exception as e: - self.result["message"] += f"Failed to get OS parameters: {str(e)} \n" + except Exception as ex: + self.result["message"] += f"Failed to get OS parameters: {str(ex)} \n" failed_parameters = [ param diff --git a/src/modules/location_constraints.py b/src/modules/location_constraints.py index 9371ea18..9bf4fd5d 100644 --- a/src/modules/location_constraints.py +++ b/src/modules/location_constraints.py @@ -6,7 +6,7 @@ """ import xml.etree.ElementTree as ET -from typing import List, Dict, Any +from typing import List from ansible.module_utils.basic import AnsibleModule try: @@ -64,8 +64,8 @@ def location_constraints_exists(self) -> List[ET.Element]: xml_output = self.execute_command_subprocess(CONSTRAINTS) self.result["details"] = xml_output return ET.fromstring(xml_output).findall(".//rsc_location") if xml_output else [] - except Exception as e: - self.handle_exception(e) + except Exception as ex: + self.handle_exception(ex) def run_module() -> None: diff --git a/src/modules/log_parser.py b/src/modules/log_parser.py index 757acb52..dd4bd588 100644 --- a/src/modules/log_parser.py +++ b/src/modules/log_parser.py @@ -119,8 +119,8 @@ def parse_logs(self) -> None: ) except FileNotFoundError as ex: self.handle_error(ex) - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) def run_module() -> None: diff --git a/src/modules/render_html_report.py b/src/modules/render_html_report.py index e042cde9..37577012 100644 --- a/src/modules/render_html_report.py +++ b/src/modules/render_html_report.py @@ -10,8 +10,8 @@ import os from datetime import datetime from typing import Dict, Any, List -from ansible.module_utils.basic import AnsibleModule import jinja2 +from ansible.module_utils.basic import AnsibleModule try: from ansible.module_utils.sap_automation_qa import SapAutomationQA, TestStatus @@ -55,12 +55,12 @@ def read_log_file(self) -> List[Dict[str, Any]]: try: with open(log_file_path, "r", encoding="utf-8") as log_file: return [json.loads(line) for line in log_file.readlines()] - except FileNotFoundError as e: + except FileNotFoundError as ex: self.log( logging.ERROR, f"Log file {log_file_path} not found.", ) - self.handle_error(e) + self.handle_error(ex) return [] def render_report(self, test_case_results: List[Dict[str, Any]]) -> None: @@ -91,8 +91,8 @@ def render_report(self, test_case_results: List[Dict[str, Any]]) -> None: ) self.result["report_path"] = report_path self.result["status"] = TestStatus.SUCCESS.value - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) def run_module() -> None: diff --git a/src/modules/send_telemetry_data.py b/src/modules/send_telemetry_data.py index 0c2b1a84..86ef793b 100644 --- a/src/modules/send_telemetry_data.py +++ b/src/modules/send_telemetry_data.py @@ -210,8 +210,8 @@ def write_log_file(self) -> None: "data_sent": True, } ) - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) def send_telemetry_data(self) -> None: """ @@ -249,8 +249,8 @@ def send_telemetry_data(self) -> None: "data_sent": True, } ) - except Exception as e: - self.handle_error(e) + except Exception as ex: + self.handle_error(ex) else: self.log( logging.ERROR, diff --git a/src/roles/ha_db_hana/tasks/files/constants.yaml b/src/roles/ha_db_hana/tasks/files/constants.yaml index 36e0dc14..76281302 100644 --- a/src/roles/ha_db_hana/tasks/files/constants.yaml +++ b/src/roles/ha_db_hana/tasks/files/constants.yaml @@ -244,14 +244,14 @@ RESOURCE_DEFAULTS: interleave: "true" operations: monitor: - interval: "120" - timeout: "120" + interval: "20s" + timeout: "120s" start: - interval: "0" - timeout: "120" + interval: "0s" + timeout: "60s" stop: - interval: "0" - timeout: "120" + interval: "0s" + timeout: "60s" azurelb: meta_attributes: diff --git a/tests/module_utils/get_cluster_status_test.py b/tests/module_utils/get_cluster_status_test.py index 4610a0ed..b6d6b14d 100644 --- a/tests/module_utils/get_cluster_status_test.py +++ b/tests/module_utils/get_cluster_status_test.py @@ -5,9 +5,9 @@ Unit tests for the get_cluster_status module. """ -import pytest -from typing import Dict, Any import xml.etree.ElementTree as ET +from typing import Dict, Any +import pytest from src.module_utils.get_cluster_status import BaseClusterStatusChecker @@ -60,6 +60,9 @@ class TestBaseClusterStatusChecker: def base_checker(self): """ Fixture for creating a testable BaseClusterStatusChecker instance. + + :return: Instance of TestableBaseClusterChecker. + :rtype: TestableBaseClusterChecker """ return TestableBaseClusterChecker(ansible_os_family="REDHAT") @@ -108,9 +111,7 @@ def test_validate_cluster_basic_status_success(self, mocker, base_checker): :param base_checker: Instance of TestableBaseClusterChecker. :type base_checker: TestableBaseClusterChecker """ - mock_execute = mocker.patch.object( - base_checker, "execute_command_subprocess", return_value="active" - ) + mocker.patch.object(base_checker, "execute_command_subprocess", return_value="active") xml_str = """ @@ -125,7 +126,7 @@ def test_validate_cluster_basic_status_success(self, mocker, base_checker): """ cluster_xml = ET.fromstring(xml_str) - result = base_checker._validate_cluster_basic_status(cluster_xml) + base_checker._validate_cluster_basic_status(cluster_xml) assert base_checker.result["pacemaker_status"] == "running" @@ -138,9 +139,7 @@ def test_validate_cluster_basic_status_insufficient_nodes(self, mocker, base_che :param base_checker: Instance of TestableBaseClusterChecker. :type base_checker: TestableBaseClusterChecker """ - mock_execute = mocker.patch.object( - base_checker, "execute_command_subprocess", return_value="active" - ) + mocker.patch.object(base_checker, "execute_command_subprocess", return_value="active") xml_str = """ @@ -154,22 +153,17 @@ def test_validate_cluster_basic_status_insufficient_nodes(self, mocker, base_che """ cluster_xml = ET.fromstring(xml_str) - result = base_checker._validate_cluster_basic_status(cluster_xml) + base_checker._validate_cluster_basic_status(cluster_xml) assert "insufficient nodes" in base_checker.result["message"] - def test_validate_cluster_basic_status_offline_node(self, mocker, base_checker): + def test_validate_cluster_basic_status_offline_node(self, base_checker): """ Test _validate_cluster_basic_status method with an offline node. - :param mocker: Mocking library to patch methods. - :type mocker: mocker.MockerFixture :param base_checker: Instance of TestableBaseClusterChecker. :type base_checker: TestableBaseClusterChecker """ - mock_execute = mocker.patch.object( - base_checker, "execute_command_subprocess", return_value="active" - ) xml_str = """ @@ -184,7 +178,7 @@ def test_validate_cluster_basic_status_offline_node(self, mocker, base_checker): """ cluster_xml = ET.fromstring(xml_str) - result = base_checker._validate_cluster_basic_status(cluster_xml) + base_checker._validate_cluster_basic_status(cluster_xml) assert "node2 is not online" in base_checker.result["message"] diff --git a/tests/module_utils/sap_automation_qa_test.py b/tests/module_utils/sap_automation_qa_test.py index 88c89f4b..f1f61d88 100644 --- a/tests/module_utils/sap_automation_qa_test.py +++ b/tests/module_utils/sap_automation_qa_test.py @@ -68,13 +68,16 @@ def test_init(self): sap_qa = SapAutomationQA() assert sap_qa.result["status"] == TestStatus.NOT_STARTED.value assert sap_qa.result["message"] == "" - assert sap_qa.result["details"] == [] - assert sap_qa.result["logs"] == [] + assert not sap_qa.result["details"] + assert not sap_qa.result["logs"] assert sap_qa.result["changed"] is False def test_setup_logger(self, monkeypatch): """ Test the setup_logger method. + + :param monkeypatch: Monkeypatch fixture for mocking. + :type monkeypatch: pytest.MonkeyPatch """ def mock_get_logger(name): @@ -88,14 +91,19 @@ def mock_get_logger(name): """ return MockLogger(name) - with monkeypatch.context() as m: - m.setattr("src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( + "src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger + ) sap_qa = SapAutomationQA() assert sap_qa.logger.name == "sap-automation-qa" def test_add_log(self, monkeypatch): """ Test the add_log method. + + :param monkeypatch: Monkeypatch fixture for mocking. + :type monkeypatch: pytest.MonkeyPatch """ def mock_get_logger(name): @@ -109,8 +117,10 @@ def mock_get_logger(name): """ return MockLogger(name) - with monkeypatch.context() as m: - m.setattr("src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( + "src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger + ) sap_qa = SapAutomationQA() sap_qa.log(1, "Test log") assert sap_qa.result["logs"] == ["Test log"] @@ -118,6 +128,9 @@ def mock_get_logger(name): def test_handle_error(self, monkeypatch): """ Test the handle_error method. + + :param monkeypatch: Monkeypatch fixture for mocking. + :type monkeypatch: pytest.MonkeyPatch """ def mock_get_logger(name): @@ -131,8 +144,10 @@ def mock_get_logger(name): """ return MockLogger(name) - with monkeypatch.context() as m: - m.setattr("src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( + "src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger + ) sap_qa = SapAutomationQA() sap_qa.handle_error(FileNotFoundError("Test error")) assert sap_qa.result["status"] == TestStatus.ERROR.value @@ -142,6 +157,9 @@ def mock_get_logger(name): def test_execute_command_subprocess(self, monkeypatch): """ Test the execute_command_subprocess method. + + :param monkeypatch: Monkeypatch fixture for mocking. + :type monkeypatch: pytest.MonkeyPatch """ def mock_get_logger(name): @@ -155,8 +173,10 @@ def mock_get_logger(name): """ return MockLogger(name) - with monkeypatch.context() as m: - m.setattr("src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( + "src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger + ) sap_qa = SapAutomationQA() command = "echo 'Hello World'" result = sap_qa.execute_command_subprocess(command) @@ -165,6 +185,9 @@ def mock_get_logger(name): def test_parse_xml_output(self, monkeypatch): """ Test the parse_xml_output method. + + :param monkeypatch: Monkeypatch fixture for mocking. + :type monkeypatch: pytest.MonkeyPatch """ def mock_get_logger(name): @@ -178,8 +201,10 @@ def mock_get_logger(name): """ return MockLogger(name) - with monkeypatch.context() as m: - m.setattr("src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( + "src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger + ) sap_qa = SapAutomationQA() xml_output = "" result = sap_qa.parse_xml_output(xml_output=xml_output) @@ -188,6 +213,9 @@ def mock_get_logger(name): def test_get_test_status(self, monkeypatch): """ Test the get_test_status method. + + :param monkeypatch: Monkeypatch fixture for mocking. + :type monkeypatch: pytest.MonkeyPatch """ def mock_get_logger(name): @@ -201,8 +229,10 @@ def mock_get_logger(name): """ return MockLogger(name) - with monkeypatch.context() as m: - m.setattr("src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( + "src.module_utils.sap_automation_qa.logging.getLogger", mock_get_logger + ) sap_qa = SapAutomationQA() result = sap_qa.get_result() assert isinstance(result, dict) diff --git a/tests/modules/check_indexserver_test.py b/tests/modules/check_indexserver_test.py index 4a808c32..290e4ffd 100644 --- a/tests/modules/check_indexserver_test.py +++ b/tests/modules/check_indexserver_test.py @@ -21,6 +21,14 @@ def fake_open_factory(file_content): """ def fake_open(*args, **kwargs): + """ + Fake open function that returns a StringIO object. + + :param *args: Positional arguments. + :param **kwargs: Keyword arguments. + :return: Instance of StringIO with file content. + :rtype: io.StringIO + """ return io.StringIO("\n".join(file_content)) return fake_open @@ -44,8 +52,8 @@ def test_redhat_indexserver_success(self, monkeypatch): "path=/usr/share/SAPHanaSR/srHook", "dummy=dummy", ] - with monkeypatch.context() as m: - m.setattr("builtins.open", fake_open_factory(file_lines)) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("builtins.open", fake_open_factory(file_lines)) checker = IndexServerCheck(database_sid="TEST", os_distribution="redhat") checker.check_indexserver() result = checker.get_result() @@ -69,8 +77,8 @@ def test_suse_indexserver_success(self, monkeypatch): "path=/usr/share/SAPHanaSR", "dummy=dummy", ] - with monkeypatch.context() as m: - m.setattr("builtins.open", fake_open_factory(file_lines)) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("builtins.open", fake_open_factory(file_lines)) checker = IndexServerCheck(database_sid="TEST", os_distribution="suse") checker.check_indexserver() result = checker.get_result() @@ -107,8 +115,8 @@ def test_indexserver_not_configured(self, monkeypatch): "path=WrongPath", "dummy=dummy", ] - with monkeypatch.context() as m: - m.setattr("builtins.open", fake_open_factory(file_lines)) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("builtins.open", fake_open_factory(file_lines)) index_server_check = IndexServerCheck(database_sid="HDB", os_distribution="redhat") index_server_check.check_indexserver() result = index_server_check.get_result() @@ -133,8 +141,8 @@ def fake_open(*args, **kwargs): """ raise FileNotFoundError("File not found") - with monkeypatch.context() as m: - m.setattr("builtins.open", fake_open) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("builtins.open", fake_open) index_server_check = IndexServerCheck(database_sid="HDB", os_distribution="redhat") index_server_check.check_indexserver() result = index_server_check.get_result() @@ -176,8 +184,8 @@ def exit_json(self, **kwargs): nonlocal mock_result mock_result = kwargs - with monkeypatch.context() as m: - m.setattr("src.modules.check_indexserver.AnsibleModule", MockAnsibleModule) - m.setattr("builtins.open", fake_open_factory(file_lines)) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("src.modules.check_indexserver.AnsibleModule", MockAnsibleModule) + monkey_patch.setattr("builtins.open", fake_open_factory(file_lines)) main() assert mock_result["status"] == TestStatus.ERROR.value diff --git a/tests/modules/filesystem_freeze_test.py b/tests/modules/filesystem_freeze_test.py index 54f14c78..c42a233a 100644 --- a/tests/modules/filesystem_freeze_test.py +++ b/tests/modules/filesystem_freeze_test.py @@ -16,30 +16,37 @@ def fake_open_factory(file_content): :param file_content: Content to be returned by the fake open function. :type file_content: list + :return: Fake open function. + :rtype: function """ def fake_open(*args, **kwargs): + """ + Fake open function that returns a StringIO object. + + :return: StringIO object with the content. + :rtype: io.StringIO + """ return io.StringIO("\n".join(file_content)) return fake_open -@pytest.fixture -def filesystem_freeze(): - """ - Fixture for creating a FileSystemFreeze instance. - - :return: FileSystemFreeze instance - :rtype: FileSystemFreeze - """ - return FileSystemFreeze() - - class TestFileSystemFreeze: """ Class to test the FileSystemFreeze class. """ + @pytest.fixture + def filesystem_freeze(self): + """ + Fixture for creating a FileSystemFreeze instance. + + :return: FileSystemFreeze instance + :rtype: FileSystemFreeze + """ + return FileSystemFreeze() + def test_file_system_exists(self, monkeypatch, filesystem_freeze): """ Test the run method when the filesystem exists. @@ -49,11 +56,13 @@ def test_file_system_exists(self, monkeypatch, filesystem_freeze): :param filesystem_freeze: FileSystemFreeze instance. :type filesystem_freeze: FileSystemFreeze """ - with monkeypatch.context() as m: - m.setattr( + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( "builtins.open", fake_open_factory(["/dev/sda1 /hana/shared ext4 rw,relatime 0 0"]) ) - m.setattr(filesystem_freeze, "execute_command_subprocess", lambda x: "output") + monkey_patch.setattr( + filesystem_freeze, "execute_command_subprocess", lambda x: "output" + ) filesystem_freeze.run() result = filesystem_freeze.get_result() @@ -74,8 +83,8 @@ def test_file_system_not_exists(self, monkeypatch, filesystem_freeze): :type filesystem_freeze: FileSystemFreeze """ - with monkeypatch.context() as m: - m.setattr( + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( "builtins.open", fake_open_factory(["/dev/sda1 /hana/log ext4 rw,relatime 0 0"]) ) filesystem_freeze.run() @@ -109,12 +118,12 @@ def exit_json(self, **kwargs): nonlocal mock_result mock_result = kwargs - with monkeypatch.context() as m: - m.setattr("src.modules.filesystem_freeze.AnsibleModule", MockAnsibleModule) - m.setattr( + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("src.modules.filesystem_freeze.AnsibleModule", MockAnsibleModule) + monkey_patch.setattr( "builtins.open", fake_open_factory(["/dev/sda1 /hana/shared ext4 rw,relatime 0 0"]) ) - m.setattr( + monkey_patch.setattr( "src.modules.filesystem_freeze.FileSystemFreeze.execute_command_subprocess", lambda self, cmd: "command output", ) @@ -145,8 +154,8 @@ def exit_json(self, **kwargs): nonlocal mock_result mock_result = kwargs - with monkeypatch.context() as m: - m.setattr("src.modules.filesystem_freeze.AnsibleModule", MockAnsibleModule) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("src.modules.filesystem_freeze.AnsibleModule", MockAnsibleModule) main() diff --git a/tests/modules/get_azure_lb_test.py b/tests/modules/get_azure_lb_test.py index c83f0475..da96acd1 100644 --- a/tests/modules/get_azure_lb_test.py +++ b/tests/modules/get_azure_lb_test.py @@ -3,7 +3,6 @@ """ import pytest -import json from src.modules.get_azure_lb import AzureLoadBalancer, main @@ -48,52 +47,54 @@ def as_dict(self): } -@pytest.fixture -def azure_lb(mocker): - """ - Fixture for creating an AzureLoadBalancer instance. - - :return: AzureLoadBalancer instance - :rtype: AzureLoadBalancer - """ - patched_client = mocker.patch("src.modules.get_azure_lb.NetworkManagementClient") - patched_client.return_value.load_balancers.list_all.return_value = [ - LoadBalancer("test1", "127.0.0.0"), - LoadBalancer("test", "127.0.0.1"), - ] - return AzureLoadBalancer( - module_params={ - "subscription_id": "test", - "region": "test", - "inbound_rules": repr( - [ - { - "backendPort": "0", - "frontendPort": "0", - "protocol": "All", - "privateIpAddress": "127.0.0.1", - } - ] - ), - "constants": { - "AZURE_LOADBALANCER": { - "RULES": {"idle_timeout_in_minutes": 4, "enable_floating_ip": False}, - "PROBES": { - "interval_in_seconds": 5, - "number_of_probes": 3, - "timeout_in_seconds": 4, - }, - } - }, - } - ) - - class TestAzureLoadBalancer: """ Test cases for the AzureLoadBalancer class. """ + @pytest.fixture + def azure_lb(self, mocker): + """ + Fixture for creating an AzureLoadBalancer instance. + + :param mocker: Mocking library for Python. + :type mocker: _mocker.MagicMock + + :return: AzureLoadBalancer instance + :rtype: AzureLoadBalancer + """ + patched_client = mocker.patch("src.modules.get_azure_lb.NetworkManagementClient") + patched_client.return_value.load_balancers.list_all.return_value = [ + LoadBalancer("test1", "127.0.0.0"), + LoadBalancer("test", "127.0.0.1"), + ] + return AzureLoadBalancer( + module_params={ + "subscription_id": "test", + "region": "test", + "inbound_rules": repr( + [ + { + "backendPort": "0", + "frontendPort": "0", + "protocol": "All", + "privateIpAddress": "127.0.0.1", + } + ] + ), + "constants": { + "AZURE_LOADBALANCER": { + "RULES": {"idle_timeout_in_minutes": 4, "enable_floating_ip": False}, + "PROBES": { + "interval_in_seconds": 5, + "number_of_probes": 3, + "timeout_in_seconds": 4, + }, + } + }, + } + ) + def test_get_load_balancers(self, azure_lb): """ Test the get_load_balancers method. @@ -118,6 +119,9 @@ def test_get_load_balancers_details(self, azure_lb): def test_main(self, monkeypatch): """ Test the main function. + + :param monkeypatch: Monkeypatch fixture for mocking. + :type monkeypatch: pytest.MonkeyPatch """ mock_result = {} @@ -141,7 +145,7 @@ def exit_json(self, **kwargs): nonlocal mock_result mock_result = kwargs - with monkeypatch.context() as m: - m.setattr("src.modules.get_azure_lb.AnsibleModule", MockAnsibleModule) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("src.modules.get_azure_lb.AnsibleModule", MockAnsibleModule) main() assert mock_result["status"] == "FAILED" diff --git a/tests/modules/get_cluster_status_db_test.py b/tests/modules/get_cluster_status_db_test.py index 7b93d1a7..4075252e 100644 --- a/tests/modules/get_cluster_status_db_test.py +++ b/tests/modules/get_cluster_status_db_test.py @@ -5,8 +5,8 @@ Unit tests for the get_cluster_status_db module. """ -import pytest import xml.etree.ElementTree as ET +import pytest from src.modules.get_cluster_status_db import HanaClusterStatusChecker, run_module @@ -19,6 +19,9 @@ class TestHanaClusterStatusChecker: def hana_checker(self): """ Fixture for creating a HanaClusterStatusChecker instance. + + :return: Instance of HanaClusterStatusChecker. + :rtype: HanaClusterStatusChecker """ return HanaClusterStatusChecker(database_sid="TEST", ansible_os_family="REDHAT") @@ -31,10 +34,11 @@ def test_get_automation_register(self, mocker, hana_checker): :param hana_checker: Instance of HanaClusterStatusChecker. :type hana_checker: HanaClusterStatusChecker """ - mock_execute = mocker.patch.object( + mocker.patch.object( hana_checker, "execute_command_subprocess", - return_value='', + return_value='', ) hana_checker._get_automation_register() @@ -50,7 +54,7 @@ def test_get_automation_register_exception(self, mocker, hana_checker): :param hana_checker: Instance of HanaClusterStatusChecker. :type hana_checker: HanaClusterStatusChecker """ - mock_execute = mocker.patch.object( + mocker.patch.object( hana_checker, "execute_command_subprocess", side_effect=Exception("Test error") ) diff --git a/tests/modules/get_cluster_status_scs_test.py b/tests/modules/get_cluster_status_scs_test.py index 0331f7fd..186ea0ca 100644 --- a/tests/modules/get_cluster_status_scs_test.py +++ b/tests/modules/get_cluster_status_scs_test.py @@ -5,8 +5,8 @@ Unit tests for the get_cluster_status_scs module. """ -import pytest import xml.etree.ElementTree as ET +import pytest from src.modules.get_cluster_status_scs import SCSClusterStatusChecker, run_module @@ -19,6 +19,9 @@ class TestSCSClusterStatusChecker: def scs_checker(self): """ Fixture for creating a SCSClusterStatusChecker instance. + + :return: Instance of SCSClusterStatusChecker. + :rtype: SCSClusterStatusChecker """ return SCSClusterStatusChecker(sap_sid="TST", ansible_os_family="REDHAT") diff --git a/tests/modules/get_package_list_test.py b/tests/modules/get_package_list_test.py index 02645897..5367282f 100644 --- a/tests/modules/get_package_list_test.py +++ b/tests/modules/get_package_list_test.py @@ -9,24 +9,32 @@ from src.modules.get_package_list import PackageListFormatter, main -@pytest.fixture -def package_facts_list(): +class TestPackageListFormatter: """ - Fixture for creating a package_facts_list. - - :return: package_facts_list - :rtype: dict + Test cases for the PackageListFormatter class. """ - return { - "corosynclib": [{"version": "2.4.5", "release": "1.el7", "arch": "x86_64"}], - "corosync": [{"version": "2.4.5", "release": "1.el7", "arch": "x86_64"}], - } + @pytest.fixture + def package_facts_list(self): + """ + Fixture for creating a package_facts_list. + + :return: package_facts_list + :rtype: dict + """ + return { + "corosynclib": [{"version": "2.4.5", "release": "1.el7", "arch": "x86_64"}], + "corosync": [{"version": "2.4.5", "release": "1.el7", "arch": "x86_64"}], + } -class TestPackageListFormatter: def test_format_packages(self, mocker, package_facts_list): """ Test the format_packages method of the PackageListFormatter class. + + :param mocker: Mocking library for Python. + :type mocker: _mocker.MagicMock + :param package_facts_list: Fixture for creating a package_facts_list. + :type package_facts_list: dict """ mock_ansible_module = mocker.patch("src.modules.get_package_list.AnsibleModule") mock_ansible_module.return_value.params = {"package_facts_list": package_facts_list} @@ -52,12 +60,9 @@ def test_format_packages(self, mocker, package_facts_list): assert result["details"] == expected_details assert result["status"] == "PASSED" - def test_format_packages_no_packages(self, monkeypatch): + def test_format_packages_no_packages(self): """ Test the format_packages method of the PackageListFormatter class with no packages. - - :param monkeypatch: Monkeypatch fixture for modifying built-in functions. - :type monkeypatch: pytest.MonkeyPatch """ empty_facts = {} formatter = PackageListFormatter(empty_facts) @@ -77,6 +82,10 @@ def test_main_method(self, monkeypatch): mock_result = {} class MockAnsibleModule: + """ + Mock class to simulate AnsibleModule behavior. + """ + def __init__(self, *args, **kwargs): self.params = { "package_facts_list": { @@ -89,8 +98,8 @@ def exit_json(self, **kwargs): nonlocal mock_result mock_result = kwargs - with monkeypatch.context() as m: - m.setattr("src.modules.get_package_list.AnsibleModule", MockAnsibleModule) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("src.modules.get_package_list.AnsibleModule", MockAnsibleModule) main() assert mock_result["status"] == "PASSED" assert len(mock_result["details"]) == 2 diff --git a/tests/modules/get_pcmk_properties_db_test.py b/tests/modules/get_pcmk_properties_db_test.py index 9c0372eb..fdb635cd 100644 --- a/tests/modules/get_pcmk_properties_db_test.py +++ b/tests/modules/get_pcmk_properties_db_test.py @@ -136,6 +136,14 @@ def fake_open_factory(file_content): """ def fake_open(*args, **kwargs): + """ + Fake open function that returns a StringIO object. + + :param *args: Positional arguments. + :param **kwargs: Keyword arguments. + :return: Instance of StringIO with file content. + :rtype: io.StringIO + """ return io.StringIO("\n".join(file_content)) return fake_open @@ -171,12 +179,16 @@ def validator(self, monkeypatch, mock_xml_outputs): :type monkeypatch: pytest.MonkeyPatch :param mock_xml_outputs: Mock XML outputs. :type mock_xml_outputs: dict + :return: HAClusterValidator instance. + :rtype: HAClusterValidator """ def mock_execute_command(*args, **kwargs): """ Mock function to replace execute_command_subprocess. + :param *args: Positional arguments. + :param **kwargs: Keyword arguments. :return: Mocked command output. :rtype: str """ @@ -220,6 +232,10 @@ def test_main_method(self, monkeypatch): mock_result = {} class MockAnsibleModule: + """ + Mock class for AnsibleModule. + """ + def __init__(self, *args, **kwargs): self.params = { "sid": "PRD", diff --git a/tests/modules/get_pcmk_properties_scs_test.py b/tests/modules/get_pcmk_properties_scs_test.py index b853f059..4349c5c9 100644 --- a/tests/modules/get_pcmk_properties_scs_test.py +++ b/tests/modules/get_pcmk_properties_scs_test.py @@ -136,6 +136,14 @@ def fake_open_factory(file_content): """ def fake_open(*args, **kwargs): + """ + Fake open function that returns a StringIO object. + + :param *args: Positional arguments. + :param **kwargs: Keyword arguments. + :return: _description_ + :rtype: _type_ + """ return io.StringIO("\n".join(file_content)) return fake_open @@ -171,12 +179,16 @@ def validator(self, monkeypatch, mock_xml_outputs): :type monkeypatch: pytest.MonkeyPatch :param mock_xml_outputs: Mock XML outputs. :type mock_xml_outputs: dict + :return: HAClusterValidator instance. + :rtype: HAClusterValidator """ def mock_execute_command(*args, **kwargs): """ Mock function to replace execute_command_subprocess. + :param *args: Positional arguments. + :param **kwargs: Keyword arguments. :return: Mocked command output. :rtype: str """ @@ -220,6 +232,10 @@ def test_main_method(self, monkeypatch): mock_result = {} class MockAnsibleModule: + """ + Mock class to simulate AnsibleModule behavior. + """ + def __init__(self, *args, **kwargs): self.params = { "sid": "PRD", diff --git a/tests/modules/location_constraints_test.py b/tests/modules/location_constraints_test.py index 91ffb7dc..32934aae 100644 --- a/tests/modules/location_constraints_test.py +++ b/tests/modules/location_constraints_test.py @@ -16,43 +16,40 @@ """ -@pytest.fixture -def location_constraints_string(): - """ - Fixture for providing a sample location constraints XML. - - :return: A sample location constraints XML. - :rtype: str +class TestLocationConstraints: """ - return LC_STR - - -@pytest.fixture -def location_constraints_xml(): + Test cases for the LocationConstraintsManager class. """ - Fixture for providing a sample location constraints XML. - :return: A sample location constraints XML. - :rtype: list[xml.etree.ElementTree.Element] - """ - return ET.fromstring(LC_STR).findall(".//rsc_location") + @pytest.fixture + def location_constraints_string(self): + """ + Fixture for providing a sample location constraints XML. + :return: A sample location constraints XML. + :rtype: str + """ + return LC_STR -@pytest.fixture -def location_constraints_manager(): - """ - Fixture for creating a LocationConstraintsManager instance. + @pytest.fixture + def location_constraints_xml(self): + """ + Fixture for providing a sample location constraints XML. - :return: LocationConstraintsManager instance - :rtype: LocationConstraintsManager - """ - return LocationConstraintsManager(ansible_os_family="SUSE") + :return: A sample location constraints XML. + :rtype: list[xml.etree.ElementTree.Element] + """ + return ET.fromstring(LC_STR).findall(".//rsc_location") + @pytest.fixture + def location_constraints_manager(self): + """ + Fixture for creating a LocationConstraintsManager instance. -class TestLocationConstraints: - """ - Test cases for the LocationConstraintsManager class. - """ + :return: LocationConstraintsManager instance + :rtype: LocationConstraintsManager + """ + return LocationConstraintsManager(ansible_os_family="SUSE") def test_location_constraints_exists_success( self, @@ -143,7 +140,9 @@ def exit_json(self, **kwargs): """ mock_result.update(kwargs) - with monkeypatch.context() as m: - m.setattr("src.modules.location_constraints.AnsibleModule", MockAnsibleModule) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr( + "src.modules.location_constraints.AnsibleModule", MockAnsibleModule + ) main() assert mock_result["status"] == "INFO" diff --git a/tests/modules/log_parser_test.py b/tests/modules/log_parser_test.py index 781d4f39..5ee16063 100644 --- a/tests/modules/log_parser_test.py +++ b/tests/modules/log_parser_test.py @@ -10,39 +10,41 @@ from src.modules.log_parser import LogParser, PCMK_KEYWORDS, SYS_KEYWORDS, main -@pytest.fixture -def log_parser_redhat(): +class TestLogParser: """ - Fixture for creating a LogParser instance. - - :return: LogParser instance - :rtype: LogParser + Test cases for the LogParser class. """ - return LogParser( - start_time="2025-01-01 00:00:00", - end_time="2025-01-01 23:59:59", - log_file="test_log_file.log", - ansible_os_family="REDHAT", - ) + @pytest.fixture + def log_parser_redhat(self): + """ + Fixture for creating a LogParser instance. -@pytest.fixture -def log_parser_suse(): - """ - Fixture for creating a LogParser instance. + :return: LogParser instance + :rtype: LogParser + """ + return LogParser( + start_time="2025-01-01 00:00:00", + end_time="2025-01-01 23:59:59", + log_file="test_log_file.log", + ansible_os_family="REDHAT", + ) - :return: LogParser instance - :rtype: LogParser - """ - return LogParser( - start_time="2023-01-01 00:00:00", - end_time="2023-01-01 23:59:59", - log_file="test_log_file.log", - ansible_os_family="SUSE", - ) + @pytest.fixture + def log_parser_suse(self): + """ + Fixture for creating a LogParser instance. + :return: LogParser instance + :rtype: LogParser + """ + return LogParser( + start_time="2023-01-01 00:00:00", + end_time="2023-01-01 23:59:59", + log_file="test_log_file.log", + ansible_os_family="SUSE", + ) -class TestLogParser: def test_parse_logs_success(self, mocker, log_parser_redhat): """ Test the parse_logs method for successful log parsing. @@ -133,6 +135,10 @@ def test_main_redhat(self, monkeypatch): mock_result = {} class MockAnsibleModule: + """ + Mock AnsibleModule for testing. + """ + def __init__(self, argument_spec, supports_check_mode): self.params = { "start_time": "2023-01-01 00:00:00", @@ -145,7 +151,7 @@ def __init__(self, argument_spec, supports_check_mode): def exit_json(self, **kwargs): mock_result.update(kwargs) - with monkeypatch.context() as m: - m.setattr("src.modules.log_parser.AnsibleModule", MockAnsibleModule) + with monkeypatch.context() as monkey_patch: + monkey_patch.setattr("src.modules.log_parser.AnsibleModule", MockAnsibleModule) main() assert mock_result["status"] == "FAILED" diff --git a/tests/modules/render_html_report_test.py b/tests/modules/render_html_report_test.py index 0ed12f73..c933f9e8 100644 --- a/tests/modules/render_html_report_test.py +++ b/tests/modules/render_html_report_test.py @@ -9,44 +9,42 @@ from src.modules.render_html_report import HTMLReportRenderer, main -@pytest.fixture -def module_params(): - """ - Fixture for providing sample module parameters. - - :return: Sample module parameters. - :rtype: dict +class TestHTMLReportRenderer: """ - return { - "test_group_invocation_id": "12345", - "test_group_name": "test_group", - "report_template": "report_template.html", - "workspace_directory": "/tmp", - } - - -@pytest.fixture -def html_report_renderer(module_params): + Test cases for the HTMLReportRenderer class. """ - Fixture for creating an HTMLReportRenderer instance. - :param module_params: Sample module parameters. - :type module_params: dict - :return: HTMLReportRenderer instance. - :rtype: HTMLReportRenderer - """ - return HTMLReportRenderer( - module_params["test_group_invocation_id"], - module_params["test_group_name"], - module_params["report_template"], - module_params["workspace_directory"], - ) + @pytest.fixture + def module_params(self): + """ + Fixture for providing sample module parameters. + :return: Sample module parameters. + :rtype: dict + """ + return { + "test_group_invocation_id": "12345", + "test_group_name": "test_group", + "report_template": "report_template.html", + "workspace_directory": "/tmp", + } + + @pytest.fixture + def html_report_renderer(self, module_params): + """ + Fixture for creating an HTMLReportRenderer instance. -class TestHTMLReportRenderer: - """ - Test cases for the HTMLReportRenderer class. - """ + :param module_params: Sample module parameters. + :type module_params: dict + :return: HTMLReportRenderer instance. + :rtype: HTMLReportRenderer + """ + return HTMLReportRenderer( + module_params["test_group_invocation_id"], + module_params["test_group_name"], + module_params["report_template"], + module_params["workspace_directory"], + ) def test_render_report(self, mocker, html_report_renderer): """ diff --git a/tests/modules/send_telemetry_data_test.py b/tests/modules/send_telemetry_data_test.py index 7735cba1..c539f4fb 100644 --- a/tests/modules/send_telemetry_data_test.py +++ b/tests/modules/send_telemetry_data_test.py @@ -11,75 +11,74 @@ from src.modules.send_telemetry_data import TelemetryDataSender, main -@pytest.fixture -def module_params(): - """ - Fixture for providing sample module parameters. - - :return: Sample module parameters. - :rtype: dict +class TestTelemetryDataSender: """ - return { - "test_group_json_data": {"TestGroupInvocationId": "12345"}, - "telemetry_data_destination": "azureloganalytics", - "laws_workspace_id": "workspace_id", - "laws_shared_key": base64.b64encode(b"shared_key").decode("utf-8"), - "telemetry_table_name": "telemetry_table", - "adx_database_name": "adx_database", - "adx_cluster_fqdn": "adx_cluster", - "adx_client_id": "adx_client", - "workspace_directory": "/tmp", - } - - -@pytest.fixture -def module_params_adx(): + Test cases for the TelemetryDataSender class. """ - Fixture for providing sample module parameters. - :return: Sample module parameters. - :rtype: dict - """ - return { - "test_group_json_data": {"TestGroupInvocationId": "12345"}, - "telemetry_data_destination": "azuredataexplorer", - "laws_workspace_id": "workspace_id", - "laws_shared_key": base64.b64encode(b"shared_key").decode("utf-8"), - "telemetry_table_name": "telemetry_table", - "adx_database_name": "adx_database", - "adx_cluster_fqdn": "adx_cluster", - "adx_client_id": "adx_client", - "workspace_directory": "/tmp", - } - - -@pytest.fixture -def telemetry_data_sender(module_params): - """ - Fixture for creating a TelemetryDataSender instance. + @pytest.fixture + def module_params(self): + """ + Fixture for providing sample module parameters. - :param module_params: Sample module parameters. - :type module_params: dict - :return: TelemetryDataSender instance. - :rtype: TelemetryDataSender - """ - return TelemetryDataSender(module_params) + :return: Sample module parameters. + :rtype: dict + """ + return { + "test_group_json_data": {"TestGroupInvocationId": "12345"}, + "telemetry_data_destination": "azureloganalytics", + "laws_workspace_id": "workspace_id", + "laws_shared_key": base64.b64encode(b"shared_key").decode("utf-8"), + "telemetry_table_name": "telemetry_table", + "adx_database_name": "adx_database", + "adx_cluster_fqdn": "adx_cluster", + "adx_client_id": "adx_client", + "workspace_directory": "/tmp", + } + @pytest.fixture + def module_params_adx(self): + """ + Fixture for providing sample module parameters. -@pytest.fixture -def telemetry_data_sender_adx(module_params_adx): - """ - Fixture for creating a TelemetryDataSender instance. + :return: Sample module parameters. + :rtype: dict + """ + return { + "test_group_json_data": {"TestGroupInvocationId": "12345"}, + "telemetry_data_destination": "azuredataexplorer", + "laws_workspace_id": "workspace_id", + "laws_shared_key": base64.b64encode(b"shared_key").decode("utf-8"), + "telemetry_table_name": "telemetry_table", + "adx_database_name": "adx_database", + "adx_cluster_fqdn": "adx_cluster", + "adx_client_id": "adx_client", + "workspace_directory": "/tmp", + } - :param module_params_adx: Sample module parameters. - :type module_params_adx: dict - :return: TelemetryDataSender instance. - :rtype: TelemetryDataSender - """ - return TelemetryDataSender(module_params_adx) + @pytest.fixture + def telemetry_data_sender(self, module_params): + """ + Fixture for creating a TelemetryDataSender instance. + :param module_params: Sample module parameters. + :type module_params: dict + :return: TelemetryDataSender instance. + :rtype: TelemetryDataSender + """ + return TelemetryDataSender(module_params) -class TestTelemetryDataSender: + @pytest.fixture + def telemetry_data_sender_adx(self, module_params_adx): + """ + Fixture for creating a TelemetryDataSender instance. + + :param module_params_adx: Sample module parameters. + :type module_params_adx: dict + :return: TelemetryDataSender instance. + :rtype: TelemetryDataSender + """ + return TelemetryDataSender(module_params_adx) def test_send_telemetry_data_to_azuredataexplorer(self, mocker, telemetry_data_sender): """ @@ -90,7 +89,6 @@ def test_send_telemetry_data_to_azuredataexplorer(self, mocker, telemetry_data_s :param telemetry_data_sender: TelemetryDataSender instance. :type telemetry_data_sender: TelemetryDataSender """ - mock_pandas = mocker.patch("pandas.DataFrame") mock_kusto = mocker.patch("azure.kusto.ingest.QueuedIngestClient.ingest_from_dataframe") mock_kusto.return_value = "response" @@ -187,7 +185,11 @@ def test_main_method(self, monkeypatch): mock_result = {} class MockAnsibleModule: - def __init__(self, argument_spec, supports_check_mode): + """ + Mock class to simulate AnsibleModule behavior. + """ + + def __init__(self, *args, **kwargs): self.params = { "test_group_json_data": {"TestGroupInvocationId": "12345"}, "telemetry_data_destination": "azureloganalyticss",