From 986ef6c6801d3f010e7481e3ef7f4a0ed8630296 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 20 Jul 2022 18:42:34 +0200 Subject: [PATCH 1/5] Check visible projects in heartbeat (fixes #126) --- src/app/api.py | 5 ++--- src/app/monitor.py | 16 +++++++++----- src/jbi/services.py | 38 +++++++++++++++++++++++++++++----- tests/unit/app/test_monitor.py | 24 +++++++++++++++++++++ 4 files changed, 70 insertions(+), 13 deletions(-) diff --git a/src/app/api.py b/src/app/api.py index 87ed96eb..3f9496fb 100644 --- a/src/app/api.py +++ b/src/app/api.py @@ -23,7 +23,7 @@ from src.jbi.bugzilla import BugzillaWebhookRequest from src.jbi.models import Actions from src.jbi.runner import IgnoreInvalidRequestError, execute_action -from src.jbi.services import get_jira +from src.jbi.services import jira_visible_projects SRC_DIR = Path(__file__).parents[1] @@ -117,8 +117,7 @@ def get_whiteboard_tag( @app.get("/jira_projects/") def get_jira_projects(): """API for viewing projects that are currently accessible by API""" - jira = get_jira() - visible_projects: List[Dict] = jira.projects(included_archived=None) + visible_projects: List[Dict] = jira_visible_projects() return [project["key"] for project in visible_projects] diff --git a/src/app/monitor.py b/src/app/monitor.py index 6eed9b30..6f8d4e53 100644 --- a/src/app/monitor.py +++ b/src/app/monitor.py @@ -1,9 +1,10 @@ """ Router dedicated to Dockerflow APIs """ -from fastapi import APIRouter, Response +from fastapi import APIRouter, Depends, Response -from src.app import environment +from src.app import configuration, environment +from src.jbi.models import Actions from src.jbi.services import jbi_service_health_map api_router = APIRouter(tags=["Monitor"]) @@ -11,10 +12,15 @@ @api_router.get("/__heartbeat__") @api_router.head("/__heartbeat__") -def heartbeat(response: Response): +def heartbeat( + response: Response, actions: Actions = Depends(configuration.get_actions) +): """Return status of backing services, as required by Dockerflow.""" - health_map = jbi_service_health_map() - if not all(health["up"] for health in health_map.values()): + health_map = jbi_service_health_map(actions) + health_checks = [] + for health in health_map.values(): + health_checks.extend(health.values()) + if not all(health_checks): response.status_code = 503 return health_map diff --git a/src/jbi/services.py b/src/jbi/services.py index 5dcd5ae3..c823ccba 100644 --- a/src/jbi/services.py +++ b/src/jbi/services.py @@ -1,13 +1,17 @@ """Services and functions that can be used to create custom actions""" -from typing import TypedDict +import logging +from typing import Dict, List, TypedDict import bugzilla as rh_bugzilla from atlassian import Jira from src.app import environment +from src.jbi.models import Actions settings = environment.get_settings() +logger = logging.getLogger(__name__) + ServiceHealth = TypedDict("ServiceHealth", {"up": bool}) @@ -22,6 +26,12 @@ def get_jira(): ) +def jira_visible_projects(jira=None) -> List[Dict]: + """Return list of projects that are visible with the configured Jira credentials""" + jira = jira or get_jira() + return jira.projects(included_archived=None) + + def get_bugzilla(): """Get bugzilla service""" return rh_bugzilla.Bugzilla( @@ -36,17 +46,35 @@ def bugzilla_check_health() -> ServiceHealth: return health -def jira_check_health() -> ServiceHealth: +def jira_check_health(actions: Actions) -> ServiceHealth: """Check health for Jira Service""" jira = get_jira() server_info = jira.get_server_info(True) - health: ServiceHealth = {"up": server_info is not None} + health: ServiceHealth = { + "up": server_info is not None, + } + if server_info is None: + return health + + visible_projects = {project["key"] for project in jira_visible_projects(jira)} + configured_projects = { + action.parameters["jira_project_key"] + for action in actions.__root__ + if "jira_project_key" in action.parameters + } + missing_projects = configured_projects - visible_projects + if missing_projects: + logger.error( + "Projects %s are not visible with configured credentials", missing_projects + ) + + health["all_projects_are_visible"] = not missing_projects return health -def jbi_service_health_map(): +def jbi_service_health_map(actions: Actions): """Returns dictionary of health check's for Bugzilla and Jira Services""" return { "bugzilla": bugzilla_check_health(), - "jira": jira_check_health(), + "jira": jira_check_health(actions), } diff --git a/tests/unit/app/test_monitor.py b/tests/unit/app/test_monitor.py index b7db093a..ce23d982 100644 --- a/tests/unit/app/test_monitor.py +++ b/tests/unit/app/test_monitor.py @@ -63,6 +63,7 @@ def test_read_heartbeat_bugzilla_services_fails( """/__heartbeat__ returns 503 when one service is unavailable.""" mocked_bugzilla().logged_in = False mocked_jira().get_server_info.return_value = {} + mocked_jira().projects.return_value = [{"key": "MR2"}, {"key": "JST"}] resp = anon_client.get("/__heartbeat__") @@ -70,6 +71,7 @@ def test_read_heartbeat_bugzilla_services_fails( assert resp.json() == { "jira": { "up": True, + "all_projects_are_visible": True, }, "bugzilla": { "up": False, @@ -81,6 +83,7 @@ def test_read_heartbeat_success(anon_client, mocked_jira, mocked_bugzilla): """/__heartbeat__ returns 200 when checks succeed.""" mocked_bugzilla().logged_in = True mocked_jira().get_server_info.return_value = {} + mocked_jira().projects.return_value = [{"key": "MR2"}, {"key": "JST"}] resp = anon_client.get("/__heartbeat__") @@ -88,6 +91,26 @@ def test_read_heartbeat_success(anon_client, mocked_jira, mocked_bugzilla): assert resp.json() == { "jira": { "up": True, + "all_projects_are_visible": True, + }, + "bugzilla": { + "up": True, + }, + } + + +def test_jira_heartbeat_visible_projects(anon_client, mocked_jira, mocked_bugzilla): + """/__heartbeat__ fails if configured projects don't match.""" + mocked_bugzilla().logged_in = True + mocked_jira().get_server_info.return_value = {} + + resp = anon_client.get("/__heartbeat__") + + assert resp.status_code == 503 + assert resp.json() == { + "jira": { + "up": True, + "all_projects_are_visible": False, }, "bugzilla": { "up": True, @@ -99,6 +122,7 @@ def test_head_heartbeat(anon_client, mocked_jira, mocked_bugzilla): """/__heartbeat__ support head requests""" mocked_bugzilla().logged_in = True mocked_jira().get_server_info.return_value = {} + mocked_jira().projects.return_value = [{"key": "MR2"}, {"key": "JST"}] resp = anon_client.head("/__heartbeat__") From ea9834ae796c8ac4b9c1cb8b613393fda09852c1 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 20 Jul 2022 18:54:07 +0200 Subject: [PATCH 2/5] Heartbeat can now have arbitrary keys --- src/jbi/services.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/jbi/services.py b/src/jbi/services.py index c823ccba..fa8f0825 100644 --- a/src/jbi/services.py +++ b/src/jbi/services.py @@ -1,6 +1,6 @@ """Services and functions that can be used to create custom actions""" import logging -from typing import Dict, List, TypedDict +from typing import Dict, List import bugzilla as rh_bugzilla from atlassian import Jira @@ -13,7 +13,7 @@ logger = logging.getLogger(__name__) -ServiceHealth = TypedDict("ServiceHealth", {"up": bool}) +ServiceHealth = Dict[str, bool] def get_jira(): @@ -29,7 +29,8 @@ def get_jira(): def jira_visible_projects(jira=None) -> List[Dict]: """Return list of projects that are visible with the configured Jira credentials""" jira = jira or get_jira() - return jira.projects(included_archived=None) + projects: List[Dict] = jira.projects(included_archived=None) + return projects def get_bugzilla(): From 763f3ab9aee44eaf30ca0461d330966191e60e34 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 21 Jul 2022 10:52:34 +0200 Subject: [PATCH 3/5] Add convencience methods in Actions model --- src/jbi/models.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/jbi/models.py b/src/jbi/models.py index 7eb326bc..d48cfd88 100644 --- a/src/jbi/models.py +++ b/src/jbi/models.py @@ -6,7 +6,7 @@ import warnings from inspect import signature from types import ModuleType -from typing import Any, Callable, Dict, List, Literal, Mapping, Optional, Union +from typing import Any, Callable, Dict, List, Literal, Mapping, Optional, Set, Union from pydantic import EmailStr, Extra, Field, root_validator, validator from pydantic_yaml import YamlModel @@ -70,6 +70,9 @@ def by_tag(self) -> Mapping[str, Action]: """Build mapping of actions by lookup tag.""" return {action.whiteboard_tag: action for action in self.__root__} + def __iter__(self): + return iter(self.__root__) + def __len__(self): return len(self.__root__) @@ -80,6 +83,15 @@ def get(self, tag: Optional[str]) -> Optional[Action]: """Lookup actions by whiteboard tag""" return self.by_tag.get(tag.lower()) if tag else None + @functools.cached_property + def configured_jira_projects_keys(self) -> Set[str]: + """Return the list of Jira project keys from all configured actions""" + return { + action.parameters["jira_project_key"] + for action in self.__root__ + if "jira_project_key" in action.parameters + } + @validator("__root__") def validate_actions( # pylint: disable=no-self-argument cls, actions: List[Action] From 1da55f6559ec3457caec97eb26496042fe4301f5 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 21 Jul 2022 10:53:18 +0200 Subject: [PATCH 4/5] Make services functions private --- src/jbi/services.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/jbi/services.py b/src/jbi/services.py index fa8f0825..aade3838 100644 --- a/src/jbi/services.py +++ b/src/jbi/services.py @@ -40,14 +40,14 @@ def get_bugzilla(): ) -def bugzilla_check_health() -> ServiceHealth: +def _bugzilla_check_health() -> ServiceHealth: """Check health for Bugzilla Service""" bugzilla = get_bugzilla() health: ServiceHealth = {"up": bugzilla.logged_in} return health -def jira_check_health(actions: Actions) -> ServiceHealth: +def _jira_check_health(actions: Actions) -> ServiceHealth: """Check health for Jira Service""" jira = get_jira() server_info = jira.get_server_info(True) @@ -76,6 +76,6 @@ def jira_check_health(actions: Actions) -> ServiceHealth: def jbi_service_health_map(actions: Actions): """Returns dictionary of health check's for Bugzilla and Jira Services""" return { - "bugzilla": bugzilla_check_health(), - "jira": jira_check_health(actions), + "bugzilla": _bugzilla_check_health(), + "jira": _jira_check_health(actions), } From 995b75b15be67f8fda6d654eb714005929e05581 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 21 Jul 2022 10:53:40 +0200 Subject: [PATCH 5/5] Refactor healthcheck from Graham's feedback --- src/jbi/services.py | 23 ++++++++++------------- tests/unit/app/test_monitor.py | 2 ++ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/jbi/services.py b/src/jbi/services.py index aade3838..c0c2ca37 100644 --- a/src/jbi/services.py +++ b/src/jbi/services.py @@ -51,26 +51,23 @@ def _jira_check_health(actions: Actions) -> ServiceHealth: """Check health for Jira Service""" jira = get_jira() server_info = jira.get_server_info(True) + is_up = server_info is not None health: ServiceHealth = { - "up": server_info is not None, + "up": is_up, + "all_projects_are_visible": is_up and _all_jira_projects_visible(jira, actions), } - if server_info is None: - return health + return health + +def _all_jira_projects_visible(jira, actions: Actions) -> bool: visible_projects = {project["key"] for project in jira_visible_projects(jira)} - configured_projects = { - action.parameters["jira_project_key"] - for action in actions.__root__ - if "jira_project_key" in action.parameters - } - missing_projects = configured_projects - visible_projects + missing_projects = actions.configured_jira_projects_keys - visible_projects if missing_projects: logger.error( - "Projects %s are not visible with configured credentials", missing_projects + "Jira projects %s are not visible with configured credentials", + missing_projects, ) - - health["all_projects_are_visible"] = not missing_projects - return health + return not missing_projects def jbi_service_health_map(actions: Actions): diff --git a/tests/unit/app/test_monitor.py b/tests/unit/app/test_monitor.py index ce23d982..d19e7db4 100644 --- a/tests/unit/app/test_monitor.py +++ b/tests/unit/app/test_monitor.py @@ -32,6 +32,7 @@ def test_read_heartbeat_all_services_fail(anon_client, mocked_jira, mocked_bugzi assert resp.json() == { "jira": { "up": False, + "all_projects_are_visible": False, }, "bugzilla": { "up": False, @@ -50,6 +51,7 @@ def test_read_heartbeat_jira_services_fails(anon_client, mocked_jira, mocked_bug assert resp.json() == { "jira": { "up": False, + "all_projects_are_visible": False, }, "bugzilla": { "up": True,