From ad5cd177a8df4440bb0258e39d0dbda57ca80a2a Mon Sep 17 00:00:00 2001 From: Niklas Stoffers <74815146+niklasstoffers@users.noreply.github.com> Date: Tue, 26 Dec 2023 17:49:46 +0100 Subject: [PATCH 1/4] Add linting, lint workflow and fix linter errors --- .flake8 | 4 ++ .github/workflows/lint.yml | 20 ++++++++++ .github/workflows/publish.yml | 7 ++++ .pre-commit-config.yaml | 8 +++- autoapprove/app.py | 2 +- autoapprove/app_info.py | 6 +-- autoapprove/bootstrapping/app_builder.py | 19 ++++----- autoapprove/config/commands/approval.py | 3 +- autoapprove/config/commands/command.py | 3 +- autoapprove/config/commands/commands.py | 3 +- autoapprove/config/commands/disapproval.py | 3 +- autoapprove/config/commands/merge.py | 3 +- .../config/commands/respondable_command.py | 3 +- autoapprove/config/config.py | 3 +- autoapprove/config/config_manager.py | 5 ++- autoapprove/config/environment.py | 3 +- autoapprove/config/environment_loader.py | 10 +++-- autoapprove/config/gitlab/gitlab.py | 3 +- autoapprove/config/gitlab/gitlab_role.py | 4 +- autoapprove/config/logging/console_handler.py | 3 +- autoapprove/config/logging/file_handler.py | 3 +- autoapprove/config/logging/handler.py | 3 +- autoapprove/config/logging/handlers.py | 3 +- autoapprove/config/logging/logging.py | 8 ++-- autoapprove/config/ssl.py | 3 +- autoapprove/config/uvicorn.py | 1 + .../validators/string_list_not_empty.py | 3 +- autoapprove/helpers/dump/json_dump.py | 3 +- autoapprove/helpers/logging/dependencies.py | 3 +- autoapprove/helpers/logging/factory.py | 3 +- autoapprove/helpers/logging/formatter.py | 2 +- autoapprove/helpers/logging/level.py | 3 +- autoapprove/helpers/type.py | 7 +++- autoapprove/main.py | 6 ++- .../middleware/exception_logger_middleware.py | 5 ++- autoapprove/routes/comment.py | 5 ++- autoapprove/security/cors.py | 3 +- autoapprove/security/https.py | 3 +- autoapprove/security/trusted_hosts.py | 3 +- autoapprove/services/auth/auth_service.py | 5 ++- .../gitlab/events/comment/comment_event.py | 3 +- .../events/comment/comment_event_service.py | 18 +++++---- .../events/comment/comment_event_type.py | 3 +- .../events/comment/models/merge_request.py | 3 +- .../comment/models/object_attributes.py | 3 +- .../gitlab/events/comment/models/project.py | 3 +- .../gitlab/events/comment/models/user.py | 3 +- autoapprove/services/gitlab/gitlab_client.py | 12 +++--- autoapprove/services/gitlab/gitlab_role.py | 3 +- .../services/gitlab/merge_request_status.py | 3 +- autoapprove/startup.py | 25 ++++++------ ci_lint.sh | 3 ++ ci_run_tests.sh | 1 + install-pre-commits.sh | 40 +++++++++++++++++++ tests/test_main.py | 3 +- 55 files changed, 225 insertions(+), 91 deletions(-) create mode 100644 .flake8 create mode 100644 .github/workflows/lint.yml create mode 100644 ci_lint.sh create mode 100644 install-pre-commits.sh diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000..87e9154 --- /dev/null +++ b/.flake8 @@ -0,0 +1,4 @@ +[flake8] +exclude = __pycache__,.pytest_cache,.venv,venv +max-complexity = 10 +max_line_length = 250 \ No newline at end of file diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..da42e04 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,20 @@ +name: LINT + +on: [push] + +jobs: + lint: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Wait for build + uses: lewagon/wait-on-check-action@v1.3.1 + with: + ref: ${{ github.ref }} + check-name: 'build' + repo-token: ${{ secrets.GITHUB_TOKEN }} + wait-interval: 5 + - name: Run lint + run: | + ./ci_run_lint.sh \ No newline at end of file diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4cec9d4..12ed301 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -21,6 +21,13 @@ jobs: check-name: 'test' repo-token: ${{ secrets.GITHUB_TOKEN }} wait-interval: 5 + - name: Wait for lint + uses: lewagon/wait-on-check-action@v1.3.1 + with: + ref: ${{ github.ref }} + check-name: 'lint' + repo-token: ${{ secrets.GITHUB_TOKEN }} + wait-interval: 5 - name: Download build artifacts uses: dawidd6/action-download-artifact@v3 with: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6fc17df..8eb7290 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,4 +4,10 @@ repos: hooks: - id: ggshield language_version: python3 - stages: [commit] \ No newline at end of file + stages: [commit] + - repo: https://github.com/PyCQA/flake8 + rev: 6.1.0 + hooks: + - id: flake8 + files: . + args: [--config, .flake8] \ No newline at end of file diff --git a/autoapprove/app.py b/autoapprove/app.py index 11c4f55..cb4769c 100644 --- a/autoapprove/app.py +++ b/autoapprove/app.py @@ -9,4 +9,4 @@ app.include_router(comment_router) logger: Logger = getLogger(__name__) -logger.info("Application startup complete") \ No newline at end of file +logger.info("Application startup complete") diff --git a/autoapprove/app_info.py b/autoapprove/app_info.py index 83695a5..074a437 100644 --- a/autoapprove/app_info.py +++ b/autoapprove/app_info.py @@ -1,3 +1,3 @@ -VERSION="0.1.0" -NAME="Gitlab Auto Approve" -SUMMARY="Gitlab webhook for automatically approving merge requests." \ No newline at end of file +VERSION = "0.1.0" +NAME = "Gitlab Auto Approve" +SUMMARY = "Gitlab webhook for automatically approving merge requests." diff --git a/autoapprove/bootstrapping/app_builder.py b/autoapprove/bootstrapping/app_builder.py index c7cede5..3703ebe 100644 --- a/autoapprove/bootstrapping/app_builder.py +++ b/autoapprove/bootstrapping/app_builder.py @@ -9,6 +9,7 @@ from middleware.exception_logger_middleware import ExceptionLoggerMiddleware from app_info import VERSION, NAME, SUMMARY + class AppBuilder(): config: Config logger: Logger @@ -16,18 +17,18 @@ class AppBuilder(): def with_config(self, config: Config) -> 'AppBuilder': self.config = config return self - + def __configure_logging(self): rootLogger: Logger = getLogger(None) config = self.config.logging if config.enable and config.handlers is not None: handlers = config.handlers - rootLogger = create_logger(rootLogger, - config.getLogLevel(), - handlers.console is not None and handlers.console.enable, - handlers.file is not None and handlers.file.enable, - handlers.file.logfile if handlers.file is not None else "") - + rootLogger = create_logger(rootLogger, + config.getLogLevel(), + handlers.console is not None and handlers.console.enable, + handlers.file is not None and handlers.file.enable, + handlers.file.logfile if handlers.file is not None else "") + def __configure_app(self, app: FastAPI): app.add_middleware(ExceptionLoggerMiddleware) @@ -38,7 +39,7 @@ def __configure_app(self, app: FastAPI): if self.config.ssl.enable: self.logger.info("Enabling HTTPS redirection") enable_https_redirection(app) - + origin = str(self.config.gitlab.host) self.logger.info('Enabling CORS with allowed origin set to "%s"', origin) enable_cors(app, origins=[origin]) @@ -56,4 +57,4 @@ def build(self) -> FastAPI: self.logger.info("Configuring app") self.__configure_app(app) - return app \ No newline at end of file + return app diff --git a/autoapprove/config/commands/approval.py b/autoapprove/config/commands/approval.py index 320feac..a1bf8d5 100644 --- a/autoapprove/config/commands/approval.py +++ b/autoapprove/config/commands/approval.py @@ -1,4 +1,5 @@ from config.commands.respondable_command import RespondableCommand + class Approval(RespondableCommand): - pass \ No newline at end of file + pass diff --git a/autoapprove/config/commands/command.py b/autoapprove/config/commands/command.py index 1fcc591..063ff7c 100644 --- a/autoapprove/config/commands/command.py +++ b/autoapprove/config/commands/command.py @@ -4,6 +4,7 @@ from config.validators.string_list_not_empty import string_list_not_empty from config.gitlab.gitlab_role import GitlabRole + class Command(ABC, BaseModel): keyword: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] ignore_case: bool @@ -11,4 +12,4 @@ class Command(ABC, BaseModel): only_for_members: list[str] | None = None requires_role: GitlabRole | None = None - _string_list_not_empty = validator('ONLY_FOR_MEMBERS', allow_reuse=True, check_fields=False)(string_list_not_empty) \ No newline at end of file + _string_list_not_empty = validator('ONLY_FOR_MEMBERS', allow_reuse=True, check_fields=False)(string_list_not_empty) diff --git a/autoapprove/config/commands/commands.py b/autoapprove/config/commands/commands.py index c493750..cdddc59 100644 --- a/autoapprove/config/commands/commands.py +++ b/autoapprove/config/commands/commands.py @@ -3,7 +3,8 @@ from config.commands.disapproval import Disapproval from config.commands.merge import Merge + class Commands(BaseModel): approval: Approval disapproval: Disapproval - merge: Merge \ No newline at end of file + merge: Merge diff --git a/autoapprove/config/commands/disapproval.py b/autoapprove/config/commands/disapproval.py index 202292d..f676687 100644 --- a/autoapprove/config/commands/disapproval.py +++ b/autoapprove/config/commands/disapproval.py @@ -1,4 +1,5 @@ from config.commands.respondable_command import RespondableCommand + class Disapproval(RespondableCommand): - pass \ No newline at end of file + pass diff --git a/autoapprove/config/commands/merge.py b/autoapprove/config/commands/merge.py index 8d56f1d..3c2fa32 100644 --- a/autoapprove/config/commands/merge.py +++ b/autoapprove/config/commands/merge.py @@ -1,4 +1,5 @@ from config.commands.respondable_command import RespondableCommand + class Merge(RespondableCommand): - pass \ No newline at end of file + pass diff --git a/autoapprove/config/commands/respondable_command.py b/autoapprove/config/commands/respondable_command.py index 1d673c5..0b015bb 100644 --- a/autoapprove/config/commands/respondable_command.py +++ b/autoapprove/config/commands/respondable_command.py @@ -3,5 +3,6 @@ from typing import Annotated from config.commands.command import Command + class RespondableCommand(Command, ABC): - message: Annotated[str | None, StringConstraints(strip_whitespace=True, min_length=1)] = None \ No newline at end of file + message: Annotated[str | None, StringConstraints(strip_whitespace=True, min_length=1)] = None diff --git a/autoapprove/config/config.py b/autoapprove/config/config.py index a1ee89b..6d9e521 100644 --- a/autoapprove/config/config.py +++ b/autoapprove/config/config.py @@ -6,6 +6,7 @@ from config.uvicorn import Uvicorn from config.logging.logging import Logging + class Config(BaseModel): gitlab: Gitlab trusted_hosts_only: bool @@ -13,4 +14,4 @@ class Config(BaseModel): ssl: SSL commands: Commands uvicorn: Uvicorn - logging: Logging \ No newline at end of file + logging: Logging diff --git a/autoapprove/config/config_manager.py b/autoapprove/config/config_manager.py index c7848fb..aeadb94 100644 --- a/autoapprove/config/config_manager.py +++ b/autoapprove/config/config_manager.py @@ -7,6 +7,7 @@ _config: Config | None = None + def _load_config(filename: str, logger: Logger) -> Config: try: logger.info('Loading configuration from file "%s"', filename) @@ -19,11 +20,13 @@ def _load_config(filename: str, logger: Logger) -> Config: logger.error('Failed to load configuration', exc_info=e) raise Exception("Failed to load configuration file. Please check your settings") + def init(filename: str, logger: Logger): global _config _config = _load_config(filename, logger) + def get_config() -> Config: if _config is None: raise Exception("Configuration has not been loaded") - return _config \ No newline at end of file + return _config diff --git a/autoapprove/config/environment.py b/autoapprove/config/environment.py index 61bf8ba..9711e37 100644 --- a/autoapprove/config/environment.py +++ b/autoapprove/config/environment.py @@ -1,5 +1,6 @@ from enum import Enum + class Environment(str, Enum): DEVELOPMENT = 'DEVELOPMENT' - PRODUCTION = 'PRODUCTION' \ No newline at end of file + PRODUCTION = 'PRODUCTION' diff --git a/autoapprove/config/environment_loader.py b/autoapprove/config/environment_loader.py index dba4212..b88a56e 100644 --- a/autoapprove/config/environment_loader.py +++ b/autoapprove/config/environment_loader.py @@ -4,15 +4,17 @@ from config.config import Config from helpers.type import is_of_type_or_generic_of_type, is_optional, unpack_optional + def _parse_as_list(value: str) -> list[str]: value = value.strip("[]") return [x.strip() for x in value.split(',') if len(x.strip()) > 0] + def _load_environment_variable(section: dict[str, Any], env_var_name: str, attribute: str, type: type) -> bool: value: str = environ.get(env_var_name, None) if value is None or len(value.strip()) == 0: return False - + value = value.strip() if is_optional(type): type = unpack_optional(type) @@ -23,6 +25,7 @@ def _load_environment_variable(section: dict[str, Any], env_var_name: str, attri section[attribute] = value return True + def _is_model(t: type) -> (bool, type): model_type: type = t if is_optional(t): @@ -31,13 +34,14 @@ def _is_model(t: type) -> (bool, type): return True, model_type return False, None + def _load_environment_helper(section: dict[str, Any], section_type: BaseModel, separator: str, prefix: str = "") -> bool: loaded_field = False for field_name, field_info in section_type.model_fields.items(): field_type = field_info.annotation is_model, model_type = _is_model(field_type) if is_model: - field_was_none = not field_name in section + field_was_none = field_name not in section if field_was_none: section[field_name] = dict() loaded_sub_field = _load_environment_helper(section[field_name], cast(BaseModel, model_type), separator, f"{prefix}{field_name}{separator}") @@ -49,4 +53,4 @@ def _load_environment_helper(section: dict[str, Any], section_type: BaseModel, s def load_environment(config: dict[str, Any], separator: str = "__"): - _load_environment_helper(config, Config, separator) \ No newline at end of file + _load_environment_helper(config, Config, separator) diff --git a/autoapprove/config/gitlab/gitlab.py b/autoapprove/config/gitlab/gitlab.py index ed0989d..e854077 100644 --- a/autoapprove/config/gitlab/gitlab.py +++ b/autoapprove/config/gitlab/gitlab.py @@ -1,7 +1,8 @@ from pydantic import BaseModel, StringConstraints, HttpUrl from typing import Annotated + class Gitlab(BaseModel): host: HttpUrl access_token: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] - webhook_token: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] \ No newline at end of file + webhook_token: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] diff --git a/autoapprove/config/gitlab/gitlab_role.py b/autoapprove/config/gitlab/gitlab_role.py index 650cc59..4252fb7 100644 --- a/autoapprove/config/gitlab/gitlab_role.py +++ b/autoapprove/config/gitlab/gitlab_role.py @@ -3,6 +3,7 @@ ROLE_MAPPINGS = dict() + class GitlabRole(str, Enum): NO_ACCESS = 'NO_ACCESS' MINIMAL_ACCESS = 'MINIMAL_ACCESS' @@ -15,10 +16,11 @@ class GitlabRole(str, Enum): def get_role(self): return ROLE_MAPPINGS[self] + ROLE_MAPPINGS[GitlabRole.NO_ACCESS] = Role.NO_ACCESS ROLE_MAPPINGS[GitlabRole.MINIMAL_ACCESS] = Role.MINIMAL_ACCESS ROLE_MAPPINGS[GitlabRole.GUEST] = Role.GUEST ROLE_MAPPINGS[GitlabRole.REPORTER] = Role.REPORTER ROLE_MAPPINGS[GitlabRole.DEVELOPER] = Role.DEVELOPER ROLE_MAPPINGS[GitlabRole.MAINTAINER] = Role.MAINTAINER -ROLE_MAPPINGS[GitlabRole.OWNER] = Role.OWNER \ No newline at end of file +ROLE_MAPPINGS[GitlabRole.OWNER] = Role.OWNER diff --git a/autoapprove/config/logging/console_handler.py b/autoapprove/config/logging/console_handler.py index 2d68ff3..249de0d 100644 --- a/autoapprove/config/logging/console_handler.py +++ b/autoapprove/config/logging/console_handler.py @@ -1,4 +1,5 @@ from config.logging.handler import Handler + class ConsoleHandler(Handler): - pass \ No newline at end of file + pass diff --git a/autoapprove/config/logging/file_handler.py b/autoapprove/config/logging/file_handler.py index 7a1d8d6..7088850 100644 --- a/autoapprove/config/logging/file_handler.py +++ b/autoapprove/config/logging/file_handler.py @@ -2,6 +2,7 @@ from pathlib import Path from config.logging.handler import Handler + class FileHandler(Handler): logfile: Path | None = None @@ -9,4 +10,4 @@ class FileHandler(Handler): def _validate(self) -> 'FileHandler': if self.enable and self.logfile is None: raise ValidationError("Log file must be specified when enabling file handler") - return self \ No newline at end of file + return self diff --git a/autoapprove/config/logging/handler.py b/autoapprove/config/logging/handler.py index 278eaf6..8de043e 100644 --- a/autoapprove/config/logging/handler.py +++ b/autoapprove/config/logging/handler.py @@ -1,4 +1,5 @@ from pydantic import BaseModel + class Handler(BaseModel): - enable: bool \ No newline at end of file + enable: bool diff --git a/autoapprove/config/logging/handlers.py b/autoapprove/config/logging/handlers.py index 4ff757a..7d4f904 100644 --- a/autoapprove/config/logging/handlers.py +++ b/autoapprove/config/logging/handlers.py @@ -2,6 +2,7 @@ from config.logging.file_handler import FileHandler from config.logging.console_handler import ConsoleHandler + class Handlers(BaseModel): console: ConsoleHandler | None = None - file: FileHandler | None = None \ No newline at end of file + file: FileHandler | None = None diff --git a/autoapprove/config/logging/logging.py b/autoapprove/config/logging/logging.py index 7f73671..99ad41d 100644 --- a/autoapprove/config/logging/logging.py +++ b/autoapprove/config/logging/logging.py @@ -1,14 +1,13 @@ from pydantic import BaseModel, model_validator, ValidationError from config.logging.handlers import Handlers -from logging import getLevelNamesMapping from helpers.logging.level import get_level_from_name + class Logging(BaseModel): enable: bool level: str | None = None handlers: Handlers | None = None _level: int | None = None - @model_validator(mode='after') def _validate(self) -> 'Logging': @@ -20,9 +19,8 @@ def _validate(self) -> 'Logging': if level is None: raise ValidationError("Invalid log level") self._level = level - + return self - def getLogLevel(self) -> int: - return self._level \ No newline at end of file + return self._level diff --git a/autoapprove/config/ssl.py b/autoapprove/config/ssl.py index 80974b8..c0025c8 100644 --- a/autoapprove/config/ssl.py +++ b/autoapprove/config/ssl.py @@ -1,5 +1,6 @@ from pydantic import BaseModel, model_validator, FilePath, ValidationError + class SSL(BaseModel): enable: bool key_file: FilePath | None = None @@ -9,4 +10,4 @@ class SSL(BaseModel): def _validate_certs(self) -> 'SSL': if self.enable and (self.key_file is None or self.cert_file is None): raise ValidationError("Certificate and key file are required with SSL enabled") - return self \ No newline at end of file + return self diff --git a/autoapprove/config/uvicorn.py b/autoapprove/config/uvicorn.py index a8e33b4..86f4106 100644 --- a/autoapprove/config/uvicorn.py +++ b/autoapprove/config/uvicorn.py @@ -1,6 +1,7 @@ from pydantic import BaseModel, StringConstraints, Field from typing import Annotated + class Uvicorn(BaseModel): reload: bool host: Annotated[str, StringConstraints(strip_whitespace=True, pattern="^((25[0-5]|(2[0-4]|1\\d|[1-9]|)\\d)\\.?\\b){4}$")] diff --git a/autoapprove/config/validators/string_list_not_empty.py b/autoapprove/config/validators/string_list_not_empty.py index d27e016..c717c6f 100644 --- a/autoapprove/config/validators/string_list_not_empty.py +++ b/autoapprove/config/validators/string_list_not_empty.py @@ -1,8 +1,9 @@ from pydantic import ValidationError + def string_list_not_empty(value: list[str] | None) -> list[str] | None: for i, entry in enumerate(value or []): value[i] = entry.strip() if len(entry) == 0: raise ValidationError("string cannot be empty") - return value \ No newline at end of file + return value diff --git a/autoapprove/helpers/dump/json_dump.py b/autoapprove/helpers/dump/json_dump.py index deefaf3..d54323a 100644 --- a/autoapprove/helpers/dump/json_dump.py +++ b/autoapprove/helpers/dump/json_dump.py @@ -1,5 +1,6 @@ import json from typing import Any + def dump(obj: Any): - return json.dumps(obj, default=lambda x: x.__dict__ if hasattr(x, "__dict__") else str(x), indent=4) \ No newline at end of file + return json.dumps(obj, default=lambda x: x.__dict__ if hasattr(x, "__dict__") else str(x), indent=4) diff --git a/autoapprove/helpers/logging/dependencies.py b/autoapprove/helpers/logging/dependencies.py index c9a284e..ddc26a7 100644 --- a/autoapprove/helpers/logging/dependencies.py +++ b/autoapprove/helpers/logging/dependencies.py @@ -1,7 +1,8 @@ from logging import Logger, getLogger from typing import Callable + def resolve_logger(name: str | None = None) -> Callable[[], Logger]: def resolver() -> Logger: return getLogger(name) - return resolver \ No newline at end of file + return resolver diff --git a/autoapprove/helpers/logging/factory.py b/autoapprove/helpers/logging/factory.py index 440cf73..ced6792 100644 --- a/autoapprove/helpers/logging/factory.py +++ b/autoapprove/helpers/logging/factory.py @@ -2,6 +2,7 @@ from helpers.logging.formatter import default_formatter from sys import stdout + def create_logger(logger: Logger, level: int, use_console: bool, use_file: bool, file: str) -> Logger: logger.setLevel(level) if use_console: @@ -15,4 +16,4 @@ def create_logger(logger: Logger, level: int, use_console: bool, use_file: bool, if not (use_console or use_file): logger.addHandler(NullHandler()) - return logger \ No newline at end of file + return logger diff --git a/autoapprove/helpers/logging/formatter.py b/autoapprove/helpers/logging/formatter.py index 34004bc..c985373 100644 --- a/autoapprove/helpers/logging/formatter.py +++ b/autoapprove/helpers/logging/formatter.py @@ -1,2 +1,2 @@ from uvicorn.logging import DefaultFormatter -default_formatter = DefaultFormatter(fmt="%(levelprefix)s [%(name)s] %(asctime)s %(message)s", datefmt="%Y-%m-%d %H:%M:%S") \ No newline at end of file +default_formatter = DefaultFormatter(fmt="%(levelprefix)s [%(name)s] %(asctime)s %(message)s", datefmt="%Y-%m-%d %H:%M:%S") diff --git a/autoapprove/helpers/logging/level.py b/autoapprove/helpers/logging/level.py index 8145f28..fbc97cf 100644 --- a/autoapprove/helpers/logging/level.py +++ b/autoapprove/helpers/logging/level.py @@ -1,7 +1,8 @@ from logging import getLevelNamesMapping + def get_level_from_name(name: str) -> int | None: mappings = getLevelNamesMapping() if name in mappings: return mappings[name] - return None \ No newline at end of file + return None diff --git a/autoapprove/helpers/type.py b/autoapprove/helpers/type.py index 5f493ea..6db02e2 100644 --- a/autoapprove/helpers/type.py +++ b/autoapprove/helpers/type.py @@ -1,13 +1,16 @@ from typing import Union, cast, Any, get_origin, get_args from types import UnionType + def is_of_type_or_generic_of_type(type: type, of_type: type): return type is of_type or get_origin(type) is of_type + def is_optional(t: type) -> bool: return type(t) is UnionType or get_origin(t) is Union and len(get_args(t)) == 2 and type(None) in get_args(t) - + + def unpack_optional(t: Any | None): args = cast(list, list(t.__args__)) args.remove(type(None)) - return args[0] \ No newline at end of file + return args[0] diff --git a/autoapprove/main.py b/autoapprove/main.py index 106e485..ed0830e 100644 --- a/autoapprove/main.py +++ b/autoapprove/main.py @@ -1,6 +1,7 @@ from argparse import ArgumentParser, Namespace from startup import Startup, StartupLoggerConfig + def parse_args() -> Namespace: parser = ArgumentParser() parser.add_argument('-c', '--config', default="config.yaml") @@ -10,6 +11,7 @@ def parse_args() -> Namespace: parser.add_argument('--startup-log-file', default="startup.log") return parser.parse_args() + def main(): args = parse_args() Startup() \ @@ -21,9 +23,9 @@ def main(): args.startup_log_file, args.startup_log_level ) - ) \ + ) \ .run() if __name__ == "__main__": - main() \ No newline at end of file + main() diff --git a/autoapprove/middleware/exception_logger_middleware.py b/autoapprove/middleware/exception_logger_middleware.py index c518463..0530875 100644 --- a/autoapprove/middleware/exception_logger_middleware.py +++ b/autoapprove/middleware/exception_logger_middleware.py @@ -2,16 +2,17 @@ from starlette.middleware.base import BaseHTTPMiddleware from logging import Logger, getLogger + class ExceptionLoggerMiddleware(BaseHTTPMiddleware): logger: Logger def __init__(self, app): super().__init__(app) self.logger = getLogger(__name__) - + async def dispatch(self, request: Request, call_next): try: return await call_next(request) except Exception as e: self.logger.error("Exception occured during request handling", exc_info=e) - return Response("Internal server error", status_code=500) \ No newline at end of file + return Response("Internal server error", status_code=500) diff --git a/autoapprove/routes/comment.py b/autoapprove/routes/comment.py index f85b4f0..59124cc 100644 --- a/autoapprove/routes/comment.py +++ b/autoapprove/routes/comment.py @@ -8,10 +8,11 @@ router = APIRouter(prefix="/comment", tags=["comment"]) + @router.post('/') -def comment_webhook(event: CommentEvent, comment_event_service: CommentEventService = Depends(get_service), logger: Logger = Depends(resolve_logger(__name__)), authorized = Depends(require_token)): +def comment_webhook(event: CommentEvent, comment_event_service: CommentEventService = Depends(get_service), logger: Logger = Depends(resolve_logger(__name__)), authorized=Depends(require_token)): try: comment_event_service.handle_comment_event(event) except Exception as e: logger.error("Failed to handle comment", exc_info=e) - raise Exception("Failed to handle comment event") \ No newline at end of file + raise Exception("Failed to handle comment event") diff --git a/autoapprove/security/cors.py b/autoapprove/security/cors.py index 192ff5a..dc11550 100644 --- a/autoapprove/security/cors.py +++ b/autoapprove/security/cors.py @@ -1,6 +1,7 @@ from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware + def enable_cors(app: FastAPI, origins: list[str]): app.add_middleware( CORSMiddleware, @@ -8,4 +9,4 @@ def enable_cors(app: FastAPI, origins: list[str]): allow_credentials=True, allow_methods=["*"], allow_headers=["*"] - ) \ No newline at end of file + ) diff --git a/autoapprove/security/https.py b/autoapprove/security/https.py index bb35468..ae130de 100644 --- a/autoapprove/security/https.py +++ b/autoapprove/security/https.py @@ -1,5 +1,6 @@ from fastapi import FastAPI from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware + def enable_https_redirection(app: FastAPI): - app.add_middleware(HTTPSRedirectMiddleware) \ No newline at end of file + app.add_middleware(HTTPSRedirectMiddleware) diff --git a/autoapprove/security/trusted_hosts.py b/autoapprove/security/trusted_hosts.py index 71a518c..132c5db 100644 --- a/autoapprove/security/trusted_hosts.py +++ b/autoapprove/security/trusted_hosts.py @@ -1,5 +1,6 @@ from fastapi import FastAPI from fastapi.middleware.trustedhost import TrustedHostMiddleware + def enable_trusted_hosts_only(app: FastAPI, allowed_hosts: list[str]): - app.add_middleware(TrustedHostMiddleware, allowed_hosts=allowed_hosts) \ No newline at end of file + app.add_middleware(TrustedHostMiddleware, allowed_hosts=allowed_hosts) diff --git a/autoapprove/services/auth/auth_service.py b/autoapprove/services/auth/auth_service.py index 2cb0129..15bb175 100644 --- a/autoapprove/services/auth/auth_service.py +++ b/autoapprove/services/auth/auth_service.py @@ -7,8 +7,9 @@ GITLAB_TOKEN_HEADER = 'X-Gitlab-Token' -async def require_token(x_gitlab_token: Annotated[str | None, Header(alias = GITLAB_TOKEN_HEADER)] = None, config: Config = Depends(get_config), logger: Logger = Depends(resolve_logger(__name__))): + +async def require_token(x_gitlab_token: Annotated[str | None, Header(alias=GITLAB_TOKEN_HEADER)] = None, config: Config = Depends(get_config), logger: Logger = Depends(resolve_logger(__name__))): if x_gitlab_token is None or x_gitlab_token != config.gitlab.webhook_token: logger.debug("Authorization error") raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) - logger.debug("Authorization successfull") \ No newline at end of file + logger.debug("Authorization successfull") diff --git a/autoapprove/services/gitlab/events/comment/comment_event.py b/autoapprove/services/gitlab/events/comment/comment_event.py index 65f0d6a..de5c8c4 100644 --- a/autoapprove/services/gitlab/events/comment/comment_event.py +++ b/autoapprove/services/gitlab/events/comment/comment_event.py @@ -6,6 +6,7 @@ from services.gitlab.events.comment.models.user import User from services.gitlab.events.comment.models.project import Project + class CommentEvent(BaseModel): user: User project: Project @@ -17,4 +18,4 @@ def type(self) -> CommentEventType: for type in CommentEventType: if getattr(self, type.value, None) is not None: return type - return CommentEventType.NONE \ No newline at end of file + return CommentEventType.NONE diff --git a/autoapprove/services/gitlab/events/comment/comment_event_service.py b/autoapprove/services/gitlab/events/comment/comment_event_service.py index 6c96b9d..9c393be 100644 --- a/autoapprove/services/gitlab/events/comment/comment_event_service.py +++ b/autoapprove/services/gitlab/events/comment/comment_event_service.py @@ -11,6 +11,7 @@ from helpers.dump.json_dump import dump as json_dump from services.gitlab.gitlab_role import GitlabRole + class CommentEventService(): gitlab_client: GitlabClient config: Config @@ -29,7 +30,7 @@ def __is_command_invocation(self, event: CommentEvent, command: Command) -> bool self.logger.debug('Performing ignore case match for command invocation') keyword = keyword.lower() message = message.lower() - + is_match: bool = False if command.strict_match: is_match = keyword == message @@ -37,11 +38,11 @@ def __is_command_invocation(self, event: CommentEvent, command: Command) -> bool else: is_match = keyword in message self.logger.debug(f'Performing match comparison of message "{message}" with result {is_match}') - + if is_match: self.logger.info(f'Invocation matches command signature for command "{type(command).__name__}"') return is_match - + def __can_invoke_command(self, event: CommentEvent, command: Command) -> bool: can_invoke: bool = True @@ -51,11 +52,11 @@ def __can_invoke_command(self, event: CommentEvent, command: Command) -> bool: project: Project = self.gitlab_client.get_project(event.project.id) if project is None: raise Exception(f"Unknown project with id {event.project.id}") - + project_member: ProjectMember = self.gitlab_client.get_project_member(project, event.user.id) if project_member is None: raise Exception(f"Unknown project member with id {event.user.id} for project with id {event.project.id}") - + can_invoke = self.gitlab_client.member_has_role(project_member, command.requires_role.get_role()) member_role: GitlabRole = self.gitlab_client.get_role_for_member(project_member) if can_invoke: @@ -82,7 +83,7 @@ def __handle_merge_comment(self, event: CommentEvent): project: Project = self.gitlab_client.get_project(event.project.id) if project is None: raise Exception(f"Project with id {event.project.id} not found") - + merge_request: ProjectMergeRequest = self.gitlab_client.get_merge_request(project, event.merge_request.iid) if merge_request is None: raise Exception(f"Merge request with iid {event.merge_request.iid} not found for project {event.project.id}") @@ -97,7 +98,6 @@ def __handle_merge_comment(self, event: CommentEvent): self.logger.info('Merging merge request') self.gitlab_client.merge(event.project.id, event.merge_request.iid, message=self.config.commands.merge.message) - def handle_comment_event(self, event: CommentEvent): if self.logger.isEnabledFor(DEBUG): self.logger.debug(f'Got comment event with data\n{json_dump(event)}') @@ -108,11 +108,13 @@ def handle_comment_event(self, event: CommentEvent): else: self.logger.info(f'Got comment event for unhandled type "{event.type}"') + service: CommentEventService | None = None + def get_service(gitlab_client: GitlabClient = Depends(get_client), config: Config = Depends(get_config), logger: Logger = Depends(resolve_logger(__name__))) -> CommentEventService: global service if service is None: logger.debug('Initializing comment event service') service = CommentEventService(gitlab_client, config, logger) - return service \ No newline at end of file + return service diff --git a/autoapprove/services/gitlab/events/comment/comment_event_type.py b/autoapprove/services/gitlab/events/comment/comment_event_type.py index b996314..de5ca7d 100644 --- a/autoapprove/services/gitlab/events/comment/comment_event_type.py +++ b/autoapprove/services/gitlab/events/comment/comment_event_type.py @@ -1,8 +1,9 @@ from enum import Enum + class CommentEventType(str, Enum): NONE: str = "none" COMMIT: str = "commit" MERGE_REQUEST: str = "merge_request" ISSUE: str = "issue" - SNIPPET: str = "snippet" \ No newline at end of file + SNIPPET: str = "snippet" diff --git a/autoapprove/services/gitlab/events/comment/models/merge_request.py b/autoapprove/services/gitlab/events/comment/models/merge_request.py index b7403b4..ee89e68 100644 --- a/autoapprove/services/gitlab/events/comment/models/merge_request.py +++ b/autoapprove/services/gitlab/events/comment/models/merge_request.py @@ -1,4 +1,5 @@ from pydantic import BaseModel + class MergeRequest(BaseModel): - iid: int \ No newline at end of file + iid: int diff --git a/autoapprove/services/gitlab/events/comment/models/object_attributes.py b/autoapprove/services/gitlab/events/comment/models/object_attributes.py index a7baf5b..df08df0 100644 --- a/autoapprove/services/gitlab/events/comment/models/object_attributes.py +++ b/autoapprove/services/gitlab/events/comment/models/object_attributes.py @@ -1,5 +1,6 @@ from pydantic import BaseModel, StringConstraints from typing import Annotated + class ObjectAttributes(BaseModel): - note: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] \ No newline at end of file + note: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] diff --git a/autoapprove/services/gitlab/events/comment/models/project.py b/autoapprove/services/gitlab/events/comment/models/project.py index 3336f8f..872faf1 100644 --- a/autoapprove/services/gitlab/events/comment/models/project.py +++ b/autoapprove/services/gitlab/events/comment/models/project.py @@ -1,4 +1,5 @@ from pydantic import BaseModel + class Project(BaseModel): - id: int \ No newline at end of file + id: int diff --git a/autoapprove/services/gitlab/events/comment/models/user.py b/autoapprove/services/gitlab/events/comment/models/user.py index cf26c87..fe10e8b 100644 --- a/autoapprove/services/gitlab/events/comment/models/user.py +++ b/autoapprove/services/gitlab/events/comment/models/user.py @@ -1,6 +1,7 @@ from pydantic import BaseModel, StringConstraints from typing import Annotated + class User(BaseModel): id: int - username: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] \ No newline at end of file + username: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] diff --git a/autoapprove/services/gitlab/gitlab_client.py b/autoapprove/services/gitlab/gitlab_client.py index 9204bb4..972e01e 100644 --- a/autoapprove/services/gitlab/gitlab_client.py +++ b/autoapprove/services/gitlab/gitlab_client.py @@ -8,6 +8,7 @@ from services.gitlab.gitlab_role import GitlabRole from services.gitlab.merge_request_status import MergeRequestStatus + class GitlabClient(): client: Gitlab logger: Logger @@ -35,10 +36,10 @@ def is_merge_request_approved_by(self, merge_request: ProjectMergeRequest, usern def get_current_username(self) -> str: return self.client.user.attributes["username"] - + def get_project_member(self, project: Project, user_id: int) -> ProjectMember | None: return project.members.get(user_id) - + def get_role_for_member(self, project_member: ProjectMember) -> GitlabRole: return GitlabRole(project_member.attributes['access_level']) @@ -48,7 +49,7 @@ def member_has_role(self, project_member: ProjectMember, role: GitlabRole, exact if exact_rule: return access_level == role return GitlabRole(access_level).is_higher_than(role) - + def approved_merge_request(self, merge_request: ProjectMergeRequest) -> bool: return self.is_merge_request_approved_by(merge_request, self.get_current_username()) @@ -61,7 +62,7 @@ def disapprove_merge_request(self, merge_request: ProjectMergeRequest, message: if message is not None: merge_request.notes.create({'body': message}) merge_request.unapprove() - + def is_mergeable_or_future_mergeable(self, merge_request: ProjectMergeRequest) -> bool: merge_status: str = merge_request.attributes['detailed_merge_status'] self.logger.debug(f'Checking merge status for merge request {merge_request.get_id()} with merge status "{merge_status}"') @@ -86,6 +87,7 @@ def cancel_merge(self, merge_request: ProjectMergeRequest, message: str | None = client: GitlabClient = None + def get_client(logger: Logger = Depends(resolve_logger(__name__)), config: Config = Depends(get_config)) -> GitlabClient: global client if client is None: @@ -98,4 +100,4 @@ def get_client(logger: Logger = Depends(resolve_logger(__name__)), config: Confi except Exception as e: logger.error('Failed to connect to gitlab', exc_info=e) raise Exception("Failed to connect to gitlab") - return client \ No newline at end of file + return client diff --git a/autoapprove/services/gitlab/gitlab_role.py b/autoapprove/services/gitlab/gitlab_role.py index 209d81c..8fae1c6 100644 --- a/autoapprove/services/gitlab/gitlab_role.py +++ b/autoapprove/services/gitlab/gitlab_role.py @@ -1,5 +1,6 @@ from enum import Enum + class GitlabRole(Enum): NO_ACCESS = 0 MINIMAL_ACCESS = 5 @@ -10,4 +11,4 @@ class GitlabRole(Enum): OWNER = 50 def is_higher_than(self, role: 'GitlabRole') -> bool: - return self > role \ No newline at end of file + return self > role diff --git a/autoapprove/services/gitlab/merge_request_status.py b/autoapprove/services/gitlab/merge_request_status.py index 232efc1..c61ec84 100644 --- a/autoapprove/services/gitlab/merge_request_status.py +++ b/autoapprove/services/gitlab/merge_request_status.py @@ -1,5 +1,6 @@ from enum import Enum + class MergeRequestStatus(str, Enum): BLOCKED = 'blocked_status' BROKEN_STATUS = 'broken_status' @@ -14,4 +15,4 @@ class MergeRequestStatus(str, Enum): NOT_APPROVED = 'not_approved' NOT_OPEN = 'not_open' POLICIES_DENIED = 'policies_denied' - JIRA_ASSOCIATION_MISSING = 'jira_association_missing' \ No newline at end of file + JIRA_ASSOCIATION_MISSING = 'jira_association_missing' diff --git a/autoapprove/startup.py b/autoapprove/startup.py index 3888d75..bd69461 100644 --- a/autoapprove/startup.py +++ b/autoapprove/startup.py @@ -6,6 +6,7 @@ from helpers.logging.level import get_level_from_name import uvicorn + class StartupLoggerConfig(): enable: bool enable_file_logs: bool @@ -23,6 +24,7 @@ def __init__(self, disable: bool, disable_file_logs: bool, file: str, log_level: raise Exception("Invalid log level") self._log_level = log_level + class Startup(): config_file: str startup_logger_config: StartupLoggerConfig @@ -30,11 +32,11 @@ class Startup(): def with_startup_logger(self, loggerConfig: StartupLoggerConfig) -> 'Startup': self.startup_logger_config = loggerConfig return self - + def with_config(self, config_file: str) -> 'Startup': self.config_file = config_file return self - + def __run_app(self, config: Config, logger: Logger): cert_file: str | None = None key_file: str | None = None @@ -45,26 +47,25 @@ def __run_app(self, config: Config, logger: Logger): key_file = config.ssl.key_file else: logger.info("Launching uvicorn") - - uvicorn.run("app:app", - host=config.uvicorn.host, + + uvicorn.run("app:app", + host=config.uvicorn.host, port=config.uvicorn.port, ssl_certfile=cert_file, ssl_keyfile=key_file, reload=config.uvicorn.reload, log_config="log_config.yaml" - ) + ) def __get_startup_logger(self) -> Logger: config = self.startup_logger_config startup_logger: Logger = Logger('$startup') - startup_logger = create_logger(startup_logger, - config._log_level, - config.enable, - config.enable and config.enable_file_logs, + startup_logger = create_logger(startup_logger, + config._log_level, + config.enable, + config.enable and config.enable_file_logs, config.file) return startup_logger - def run(self): logger = self.__get_startup_logger() @@ -75,4 +76,4 @@ def run(self): if logger.isEnabledFor(DEBUG): logger.debug("Running with configuration:\n%s", dump(config)) - self.__run_app(config, logger) \ No newline at end of file + self.__run_app(config, logger) diff --git a/ci_lint.sh b/ci_lint.sh new file mode 100644 index 0000000..a01577d --- /dev/null +++ b/ci_lint.sh @@ -0,0 +1,3 @@ +#!/bin/sh +pip install flake8==6.1.0 +flake8 . --config .flake8 \ No newline at end of file diff --git a/ci_run_tests.sh b/ci_run_tests.sh index 0bea163..a3c3cc2 100755 --- a/ci_run_tests.sh +++ b/ci_run_tests.sh @@ -1,3 +1,4 @@ +#!/bin/sh docker run --entrypoint /bin/sh -v "./tests:/tests" gitlab-auto-approve -c \ "pip install -r /tests/requirements.txt && \ pytest /tests" \ No newline at end of file diff --git a/install-pre-commits.sh b/install-pre-commits.sh new file mode 100644 index 0000000..bd8a541 --- /dev/null +++ b/install-pre-commits.sh @@ -0,0 +1,40 @@ +#!/bin/bash +echo "Installing necessary packages..." +pip_output=$(pip install pre-commit==3.6.0 2>&1) +ret=$? + +if [ $ret -ne 0 ]; then + echo "Package installation failed" + echo $pip_output +else + echo "Successfully installed necessary packages!" +fi + +echo "Please enter GitGuardian API key for ggshield pre-commit scan job" +stty -echo +read -p 'Gitguardian API key: ' gg_api_key; echo +stty echo + +export_global= +while ! [[ $export_global = "y" || $export_global = "N" ]]; do + read -p 'Do you want to add the API key as a global variable? Otherwise it has to be set each time environment variables are being reloaded [y/N]: ' export_global +done + +export GITGUARDIAN_API_KEY=$gg_api_key +if [ $export_global = "y" ]; then + echo "Exporting GitGuardian API key..." + echo "export GITGUARDIAN_API_KEY="$gg_api_key"">>~/.bashrc +else + echo "GitGuardian API key will not be exported" +fi + +echo "Installing pre-commit..." +install_output=$(pre-commit install 2>&1) +ret=$? + +if [ $ret -ne 0 ]; then + echo "Installing pre-commit hooks failed" + echo $install_output +else + echo "Successfully installed pre-commit hooks!" +fi diff --git a/tests/test_main.py b/tests/test_main.py index fdd8c41..dc75005 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,4 +1,5 @@ from autoapprove.helpers.type import is_optional + def test(): - assert is_optional(str | None) \ No newline at end of file + assert is_optional(str | None) From 38e216abc3a54ca162130ac599e94b1d8ab8681b Mon Sep 17 00:00:00 2001 From: Niklas Stoffers <74815146+niklasstoffers@users.noreply.github.com> Date: Tue, 26 Dec 2023 17:50:35 +0100 Subject: [PATCH 2/4] Add executable flag to lint script --- ci_lint.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 ci_lint.sh diff --git a/ci_lint.sh b/ci_lint.sh old mode 100644 new mode 100755 From c169ac83133420a67a5d16db31bf2768b9fb6d82 Mon Sep 17 00:00:00 2001 From: Niklas Stoffers <74815146+niklasstoffers@users.noreply.github.com> Date: Tue, 26 Dec 2023 17:52:44 +0100 Subject: [PATCH 3/4] Fix wrong script name error in lint workflow --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index da42e04..ba8ed7d 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -17,4 +17,4 @@ jobs: wait-interval: 5 - name: Run lint run: | - ./ci_run_lint.sh \ No newline at end of file + ./ci_lint.sh \ No newline at end of file From 667281937060a22da12ef4b233373fde1cc73ef6 Mon Sep 17 00:00:00 2001 From: Niklas Stoffers <74815146+niklasstoffers@users.noreply.github.com> Date: Tue, 26 Dec 2023 17:58:58 +0100 Subject: [PATCH 4/4] Fix code quality issues --- install-pre-commits.sh | 6 +++--- publish.sh | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/install-pre-commits.sh b/install-pre-commits.sh index bd8a541..e440c06 100644 --- a/install-pre-commits.sh +++ b/install-pre-commits.sh @@ -12,18 +12,18 @@ fi echo "Please enter GitGuardian API key for ggshield pre-commit scan job" stty -echo -read -p 'Gitguardian API key: ' gg_api_key; echo +read -rp 'Gitguardian API key: ' gg_api_key; echo stty echo export_global= while ! [[ $export_global = "y" || $export_global = "N" ]]; do - read -p 'Do you want to add the API key as a global variable? Otherwise it has to be set each time environment variables are being reloaded [y/N]: ' export_global + read -rp 'Do you want to add the API key as a global variable? Otherwise it has to be set each time environment variables are being reloaded [y/N]: ' export_global done export GITGUARDIAN_API_KEY=$gg_api_key if [ $export_global = "y" ]; then echo "Exporting GitGuardian API key..." - echo "export GITGUARDIAN_API_KEY="$gg_api_key"">>~/.bashrc + echo "export GITGUARDIAN_API_KEY=$gg_api_key">>~/.bashrc else echo "GitGuardian API key will not be exported" fi diff --git a/publish.sh b/publish.sh index ccffc33..40472d0 100644 --- a/publish.sh +++ b/publish.sh @@ -3,9 +3,9 @@ IMAGE_NAME=gitlab-auto-approve IMAGE_RELEASE_TAG=latest IMAGE_RELEASE_NAME=$IMAGE_NAME:$IMAGE_RELEASE_TAG -read -p 'Docker hub username: ' username +read -rp 'Docker hub username: ' username stty -echo -read -p 'Docker hub password: ' password; echo +read -rp 'Docker hub password: ' password; echo stty echo REGISTRY_IMAGE_NAME=$username/$IMAGE_RELEASE_NAME