From 3f716f7369230ab2a864b6bfcae44a952ca5bf90 Mon Sep 17 00:00:00 2001 From: Marco Donadoni Date: Thu, 28 Mar 2024 12:44:20 +0100 Subject: [PATCH] fix(gitlab): handle pagination of GitLab webhooks (#684) Closes #682 --- reana_server/config.py | 2 ++ reana_server/utils.py | 45 +++++++++++++++++++++++++----------------- tests/test_utils.py | 32 ++++++++++++++++++++++++++++-- tests/test_views.py | 1 + 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/reana_server/config.py b/reana_server/config.py index 472361f8..32e8eb64 100644 --- a/reana_server/config.py +++ b/reana_server/config.py @@ -328,6 +328,8 @@ def _get_rate_limit(env_variable: str, default: str) -> str: REANA_GITLAB_OAUTH_APP_SECRET = os.getenv("REANA_GITLAB_OAUTH_APP_SECRET", "CHANGE_ME") REANA_GITLAB_HOST = os.getenv("REANA_GITLAB_HOST", None) REANA_GITLAB_URL = "https://{}".format((REANA_GITLAB_HOST or "CHANGE ME")) +REANA_GITLAB_MAX_PER_PAGE = 100 +"""Maximum number of items that can be listed in a single GitLab's paginated response.""" # Workflow scheduler # ================== diff --git a/reana_server/utils.py b/reana_server/utils.py index 192bfddb..fe264fd5 100644 --- a/reana_server/utils.py +++ b/reana_server/utils.py @@ -17,7 +17,7 @@ import secrets import sys import shutil -from typing import Dict, List, Optional, Union +from typing import Any, Dict, List, Optional, Union, Generator from uuid import UUID, uuid4 import click @@ -67,6 +67,7 @@ ) from reana_server.config import ( ADMIN_USER_ID, + REANA_GITLAB_MAX_PER_PAGE, REANA_GITLAB_URL, REANA_HOSTNAME, REANA_USER_EMAIL_CONFIRMATION, @@ -500,6 +501,19 @@ def _format_gitlab_secrets(gitlab_response): } +def _unpaginate_gitlab_endpoint(url: str) -> Generator[Any, None, None]: + """Get all the paginated records of a given GitLab endpoint. + + :param url: Endpoint URL to the first page. + """ + while url: + logging.debug(f"Request to '{url}' while unpaginating GitLab endpoint") + response = requests.get(url) + response.raise_for_status() + yield from response.json() + url = response.links.get("next", {}).get("url") + + def _get_gitlab_hook_id(project_id, gitlab_token): """Return REANA hook id from a GitLab project if it is connected. @@ -511,27 +525,22 @@ def _get_gitlab_hook_id(project_id, gitlab_token): """ gitlab_hooks_url = ( REANA_GITLAB_URL - + "/api/v4/projects/{0}/hooks?access_token={1}".format(project_id, gitlab_token) + + f"/api/v4/projects/{project_id}/hooks?" + + f"per_page={REANA_GITLAB_MAX_PER_PAGE}&" + + f"access_token={gitlab_token}" ) - response = requests.get(gitlab_hooks_url) + create_workflow_url = url_for("workflows.create_workflow", _external=True) - if not response.ok: + try: + for hook in _unpaginate_gitlab_endpoint(gitlab_hooks_url): + if hook["url"] and hook["url"] == create_workflow_url: + return hook["id"] + except requests.HTTPError as e: logging.warning( - f"GitLab hook request failed with status code: {response.status_code}, " - f"content: {response.content}" + f"GitLab hook request failed with status code: {e.response.status_code}, " + f"content: {e.response.content}" ) - return None - - response_json = response.json() - create_workflow_url = url_for("workflows.create_workflow", _external=True) - return next( - ( - hook["id"] - for hook in response_json - if hook["url"] and hook["url"] == create_workflow_url - ), - None, - ) + return None class RequestStreamWithLen(object): diff --git a/tests/test_utils.py b/tests/test_utils.py index ef17d690..dee6abeb 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,5 @@ # This file is part of REANA. -# Copyright (C) 2021, 2022, 2023 CERN. +# Copyright (C) 2021, 2022, 2023, 2024 CERN. # # REANA is free software; you can redistribute it and/or modify it # under the terms of the MIT License; see LICENSE file for more details. @@ -7,11 +7,17 @@ """REANA-Server tests for utils module.""" import pathlib +from unittest.mock import call, patch, Mock import pytest from reana_commons.errors import REANAValidationError from reana_db.models import UserToken, UserTokenStatus, UserTokenType -from reana_server.utils import is_valid_email, filter_input_files, get_user_from_token +from reana_server.utils import ( + is_valid_email, + filter_input_files, + get_user_from_token, + _unpaginate_gitlab_endpoint, +) @pytest.mark.parametrize( @@ -81,3 +87,25 @@ def test_get_user_from_token_two_tokens(default_user, session): # Check that old revoked token does not work with pytest.raises(ValueError, match="revoked"): get_user_from_token(old_token.token) + + +@patch("requests.get") +def test_gitlab_pagination(mock_get): + """Test getting all paginated results from GitLab.""" + # simulating two pages + first_response = Mock() + first_response.ok = True + first_response.links = {"next": {"url": "next_url"}} + first_response.json.return_value = [1, 2] + + second_response = Mock() + second_response.ok = True + second_response.links = {} + second_response.json.return_value = [3, 4] + + mock_get.side_effect = [first_response, second_response] + + res = list(_unpaginate_gitlab_endpoint("first_url")) + + assert res == [1, 2, 3, 4] + assert mock_get.call_args_list == [call("first_url"), call("next_url")] diff --git a/tests/test_views.py b/tests/test_views.py index 03c6fde0..e9afcaf5 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -835,6 +835,7 @@ def test_gitlab_projects(app: Flask, default_user): mock_response_webhook = Mock() mock_response_webhook.ok = True mock_response_webhook.status_code = 200 + mock_response_webhook.links = {} mock_response_webhook.json.return_value = [ {"id": 1234, "url": "wrong_url"}, {