From bc263eaccb3f40d90497bbe92a27807fa71d60c4 Mon Sep 17 00:00:00 2001 From: Isaac Heist Date: Tue, 16 Apr 2024 09:47:08 -0700 Subject: [PATCH] chore: audited and updated core and observability modules (#519) secureli-453 Audited and updated core and observability modules ## Changes * Audited and updated core and observability modules * No change or update to functionality, just re-organize code per audit. ## Testing * All existing unit tests pass ## Clean Code Checklist - [x] Meets acceptance criteria for issue - [ ] New logic is covered with automated tests - [ ] Appropriate exception handling added - [ ] Thoughtful logging included - [ ] Documentation is updated - [ ] Follow-up work is documented in TODOs - [ ] TODOs have a ticket associated with them - [x] No commented-out code included --- .../modules/core/core_services/scanner.py | 11 +-- .../modules/core/core_services/updater.py | 11 +-- .../observability_services/logging.py | 67 +++---------------- secureli/modules/shared/models/logging.py | 38 +++++++++++ secureli/modules/shared/models/scan.py | 8 +++ secureli/modules/shared/models/update.py | 12 ++++ secureli/modules/shared/utilities.py | 10 +++ tests/actions/test_action.py | 2 +- tests/actions/test_update_action.py | 2 +- tests/modules/core/test_scanner_service.py | 7 +- 10 files changed, 85 insertions(+), 83 deletions(-) create mode 100644 secureli/modules/shared/models/update.py diff --git a/secureli/modules/core/core_services/scanner.py b/secureli/modules/core/core_services/scanner.py index c706207e..3e37ce0f 100644 --- a/secureli/modules/core/core_services/scanner.py +++ b/secureli/modules/core/core_services/scanner.py @@ -1,4 +1,3 @@ -from enum import Enum from typing import Optional from pathlib import Path @@ -9,14 +8,6 @@ from secureli.repositories.repo_settings import PreCommitSettings -class OutputParseErrors(str, Enum): - """ - Possible errors when parsing scan output - """ - - REPO_NOT_FOUND = "repo-not-found" - - class HooksScannerService: """ Scans the repo according to the repo's seCureLI config @@ -157,4 +148,4 @@ def _find_repo_from_id(self, hook_id: str, config: PreCommitSettings): if hook.id == hook_id: return repo_str - return OutputParseErrors.REPO_NOT_FOUND + return scan.OutputParseErrors.REPO_NOT_FOUND diff --git a/secureli/modules/core/core_services/updater.py b/secureli/modules/core/core_services/updater.py index 9ebcc202..120646a9 100644 --- a/secureli/modules/core/core_services/updater.py +++ b/secureli/modules/core/core_services/updater.py @@ -1,20 +1,11 @@ from typing import Optional from pathlib import Path -import pydantic from secureli.modules.shared.abstractions.pre_commit import PreCommitAbstraction +from secureli.modules.shared.models.update import UpdateResult from secureli.repositories.secureli_config import SecureliConfigRepository -class UpdateResult(pydantic.BaseModel): - """ - The results of calling scan_repo - """ - - successful: bool - output: Optional[str] = None - - class UpdaterService: """ Handles update operations diff --git a/secureli/modules/observability/observability_services/logging.py b/secureli/modules/observability/observability_services/logging.py index dcdfad00..cea76824 100644 --- a/secureli/modules/observability/observability_services/logging.py +++ b/secureli/modules/observability/observability_services/logging.py @@ -1,59 +1,14 @@ -import platform -from datetime import datetime -from enum import Enum from pathlib import Path from typing import Optional -from uuid import uuid4 - -import pydantic -from secureli.modules.shared.models.config import HookConfiguration -from secureli.modules.shared.models.logging import LogAction +import secureli.modules.shared.models.logging as LoggingModels import secureli.repositories.secureli_config as SecureliConfig + from secureli.modules.language_analyzer import language_support from secureli.repositories.secureli_config import SecureliConfigRepository from secureli.modules.shared import utilities -def generate_unique_id() -> str: - """ - A unique identifier representing the log entry, including various - bits specific to the user and environment - """ - origin_email_branch = f"{utilities.origin_url()}|{utilities.git_user_email()}|{utilities.current_branch_name()}" - return f"{uuid4()}|{origin_email_branch}" - - -class LogStatus(str, Enum): - """Whether the entry represents a successful or failing entry""" - - success = "SUCCESS" - failure = "FAILURE" - - -class LogFailure(pydantic.BaseModel): - """An extendable structure for log failures""" - - details: str - - -class LogEntry(pydantic.BaseModel): - """A distinct entry in the log captured following actions like scan and init""" - - id: str = generate_unique_id() - timestamp: datetime = datetime.utcnow() - username: str = utilities.git_user_email() - machineid: str = platform.uname().node - secureli_version: str = utilities.secureli_version() - languages: Optional[list[str]] - status: LogStatus - action: LogAction - hook_config: Optional[HookConfiguration] - failure: Optional[LogFailure] = None - total_failure_count: Optional[int] - failure_count_details: Optional[object] - - class LoggingService: """Enables capturing secureli KPI log entries to a local file for future upload""" @@ -65,7 +20,7 @@ def __init__( self.language_support = language_support self.secureli_config = secureli_config - def success(self, action: LogAction) -> LogEntry: + def success(self, action: LoggingModels.LogAction) -> LoggingModels.LogEntry: """ Capture that a successful conclusion has been reached for an action :param action: The action that succeeded @@ -76,8 +31,8 @@ def success(self, action: LogAction) -> LogEntry: if secureli_config.languages else None ) - log_entry = LogEntry( - status=LogStatus.success, + log_entry = LoggingModels.LogEntry( + status=LoggingModels.LogStatus.success, action=action, hook_config=hook_config, languages=secureli_config.languages if secureli_config.languages else None, @@ -88,11 +43,11 @@ def success(self, action: LogAction) -> LogEntry: def failure( self, - action: LogAction, + action: LoggingModels.LogAction, details: str, total_failure_count: Optional[int] = None, individual_failure_count: Optional[object] = None, - ) -> LogEntry: + ) -> LoggingModels.LogEntry: """ Capture a failure against an action, with details :param action: The action that failed @@ -106,10 +61,10 @@ def failure( if not secureli_config.languages else self.language_support.get_configuration(secureli_config.languages) ) - log_entry = LogEntry( - status=LogStatus.failure, + log_entry = LoggingModels.LogEntry( + status=LoggingModels.LogStatus.failure, action=action, - failure=LogFailure( + failure=LoggingModels.LogFailure( details=details, ), total_failure_count=total_failure_count, @@ -121,7 +76,7 @@ def failure( return log_entry - def _log(self, log_entry: LogEntry): + def _log(self, log_entry: LoggingModels.LogEntry): """Commit a log entry to the branch log file""" log_folder_path = Path(SecureliConfig.FOLDER_PATH / ".secureli/logs") path_to_log = log_folder_path / f"{utilities.current_branch_name()}" diff --git a/secureli/modules/shared/models/logging.py b/secureli/modules/shared/models/logging.py index 1594b305..f40f8328 100644 --- a/secureli/modules/shared/models/logging.py +++ b/secureli/modules/shared/models/logging.py @@ -1,4 +1,12 @@ +from datetime import datetime from enum import Enum +from typing import Optional + +import platform +import pydantic + +from secureli.modules.shared.models.config import HookConfiguration +from secureli.modules.shared import utilities class LogAction(str, Enum): @@ -9,3 +17,33 @@ class LogAction(str, Enum): build = "_BUILD" update = "UPDATE" publish = "PUBLISH" # "PUBLISH" does not correspond to a CLI action/subcommand + + +class LogStatus(str, Enum): + """Whether the entry represents a successful or failing entry""" + + success = "SUCCESS" + failure = "FAILURE" + + +class LogFailure(pydantic.BaseModel): + """An extendable structure for log failures""" + + details: str + + +class LogEntry(pydantic.BaseModel): + """A distinct entry in the log captured following actions like scan and init""" + + id: str = utilities.generate_unique_id() + timestamp: datetime = datetime.utcnow() + username: str = utilities.git_user_email() + machineid: str = platform.uname().node + secureli_version: str = utilities.secureli_version() + languages: Optional[list[str]] + status: LogStatus + action: LogAction + hook_config: Optional[HookConfiguration] + failure: Optional[LogFailure] = None + total_failure_count: Optional[int] + failure_count_details: Optional[object] diff --git a/secureli/modules/shared/models/scan.py b/secureli/modules/shared/models/scan.py index d86e785a..300787b3 100644 --- a/secureli/modules/shared/models/scan.py +++ b/secureli/modules/shared/models/scan.py @@ -39,3 +39,11 @@ class ScanResult(pydantic.BaseModel): successful: bool output: Optional[str] = None failures: list[ScanFailure] + + +class OutputParseErrors(str, Enum): + """ + Possible errors when parsing scan output + """ + + REPO_NOT_FOUND = "repo-not-found" diff --git a/secureli/modules/shared/models/update.py b/secureli/modules/shared/models/update.py new file mode 100644 index 00000000..6520ff1a --- /dev/null +++ b/secureli/modules/shared/models/update.py @@ -0,0 +1,12 @@ +from typing import Optional + +import pydantic + + +class UpdateResult(pydantic.BaseModel): + """ + The results of calling scan_repo + """ + + successful: bool + output: Optional[str] = None diff --git a/secureli/modules/shared/utilities.py b/secureli/modules/shared/utilities.py index ac04a6d2..b4153d09 100644 --- a/secureli/modules/shared/utilities.py +++ b/secureli/modules/shared/utilities.py @@ -1,6 +1,7 @@ import configparser import hashlib import os +from uuid import uuid4 import requests import subprocess @@ -174,3 +175,12 @@ def merge_scan_results(results: list[ScanResult]): return ScanResult( successful=final_successful, output=final_output, failures=final_failures ) + + +def generate_unique_id() -> str: + """ + A unique identifier representing the log entry, including various + bits specific to the user and environment + """ + origin_email_branch = f"{origin_url()}|{git_user_email()}|{current_branch_name()}" + return f"{uuid4()}|{origin_email_branch}" diff --git a/tests/actions/test_action.py b/tests/actions/test_action.py index 83d575de..d231c084 100644 --- a/tests/actions/test_action.py +++ b/tests/actions/test_action.py @@ -11,7 +11,7 @@ from secureli.modules.shared.models import language from secureli.modules.shared.models.scan import ScanFailure, ScanResult from secureli.repositories.secureli_config import SecureliConfig, VerifyConfigOutcome -from secureli.modules.core.core_services.updater import UpdateResult +from secureli.modules.shared.models.update import UpdateResult test_folder_path = Path("does-not-matter") diff --git a/tests/actions/test_update_action.py b/tests/actions/test_update_action.py index 08c51479..4ca8fc66 100644 --- a/tests/actions/test_update_action.py +++ b/tests/actions/test_update_action.py @@ -4,7 +4,7 @@ from secureli.actions.action import ActionDependencies from secureli.actions.update import UpdateAction -from secureli.modules.core.core_services.updater import UpdateResult +from secureli.modules.shared.models.update import UpdateResult test_folder_path = Path("does-not-matter") diff --git a/tests/modules/core/test_scanner_service.py b/tests/modules/core/test_scanner_service.py index 820bb9f1..ef8f4310 100644 --- a/tests/modules/core/test_scanner_service.py +++ b/tests/modules/core/test_scanner_service.py @@ -3,12 +3,9 @@ import pytest from secureli.modules.shared.abstractions.pre_commit import ExecuteResult -from secureli.modules.shared.models.scan import ScanMode +from secureli.modules.shared.models.scan import OutputParseErrors, ScanMode from secureli.repositories import repo_settings -from secureli.modules.core.core_services.scanner import ( - HooksScannerService, - OutputParseErrors, -) +from secureli.modules.core.core_services.scanner import HooksScannerService from pytest_mock import MockerFixture test_folder_path = Path(".")