From 7e2a3d4765f32cab3cc0c328b3525c98d4e96ea8 Mon Sep 17 00:00:00 2001 From: Lorenzo Cavazzi <43481553+lorenzo-cavazzi@users.noreply.github.com> Date: Wed, 27 Apr 2022 13:02:33 +0200 Subject: [PATCH] feat(service): restore optimized migration check (#2854) - handle the GitLab library specific errors. - fall back to full repository checkout in case of unforeseen errors. fix #2546 --- .../controllers/cache_migrations_check.py | 20 ++++++----- renku/ui/service/errors.py | 2 +- .../service/gateways/gitlab_api_provider.py | 32 +++++++++++++++-- renku/ui/service/views/error_handlers.py | 6 +++- tests/fixtures/config.py | 8 +++-- tests/service/fixtures/service_projects.py | 8 +++++ tests/service/views/test_cache_views.py | 34 +++++++++++++++++++ 7 files changed, 94 insertions(+), 16 deletions(-) diff --git a/renku/ui/service/controllers/cache_migrations_check.py b/renku/ui/service/controllers/cache_migrations_check.py index cb338ff2ab..c9d886a52f 100644 --- a/renku/ui/service/controllers/cache_migrations_check.py +++ b/renku/ui/service/controllers/cache_migrations_check.py @@ -21,7 +21,7 @@ from pathlib import Path from renku.command.migrate import migrations_check -from renku.core.errors import RenkuException +from renku.core.errors import AuthenticationError, ProjectNotFound, RenkuException from renku.core.util.contexts import click_context from renku.ui.service.controllers.api.abstract import ServiceCtrl from renku.ui.service.controllers.api.mixins import RenkuOperationMixin @@ -71,11 +71,15 @@ def renku_op(self): def to_response(self): """Execute controller flow and serialize to service response.""" - return result_response(self.RESPONSE_SERIALIZER, self.execute_op()) + if "project_id" in self.context: + result = self.execute_op() + else: + # NOTE: use quick flow but fallback to regular flow in case of unexpected exceptions + try: + result = self._fast_op_without_cache() + except (AuthenticationError, ProjectNotFound): + raise + except BaseException: + result = self.execute_op() - # TODO: Enable this optimization. See https://github.com/SwissDataScienceCenter/renku-python/issues/2546 - # if "project_id" in self.context: - # # use regular flow using cache - # return result_response(self.RESPONSE_SERIALIZER, self.execute_op()) - # - # return result_response(self.RESPONSE_SERIALIZER, self._fast_op_without_cache()) + return result_response(self.RESPONSE_SERIALIZER, result) diff --git a/renku/ui/service/errors.py b/renku/ui/service/errors.py index 5ae2f53521..dc2ec26a8c 100644 --- a/renku/ui/service/errors.py +++ b/renku/ui/service/errors.py @@ -611,7 +611,7 @@ class IntermittentAuthenticationError(ServiceError): This may happen for a number of reasons. Triggering a new login will likely fix it. """ - code = SVC_ERROR_USER + 30 + code = SVC_ERROR_INTERMITTENT + 30 userMessage = "Invalid user credentials. Please try to log out and in again." devMessage = "Authentication error. Check the Sentry exception to inspect the headers" diff --git a/renku/ui/service/gateways/gitlab_api_provider.py b/renku/ui/service/gateways/gitlab_api_provider.py index 5ce83d11c1..52611dfd33 100644 --- a/renku/ui/service/gateways/gitlab_api_provider.py +++ b/renku/ui/service/gateways/gitlab_api_provider.py @@ -22,13 +22,25 @@ import gitlab +from renku.core import errors from renku.core.util.os import delete_file from renku.domain_model.git import GitURL from renku.ui.service.interfaces.git_api_provider import IGitAPIProvider class GitlabAPIProvider(IGitAPIProvider): - """Interface a Git API Provider.""" + """GitLab API provider abstraction layer. + + Args: + paths: List of files to download. + target_folder: Folder to use to download the files. + remote: Remote repository URL. + token: User bearer token. + ref: optional reference to checkout, + Raises: + errors.ProjectNotFound: If the remote URL is not accessible. + errors.AuthenticationError: If the bearer token is invalid in any way. + """ def download_files_from_api( self, @@ -45,8 +57,22 @@ def download_files_from_api( target_folder = Path(target_folder) git_data = GitURL.parse(remote) - gl = gitlab.Gitlab(git_data.instance_url, private_token=token) - project = gl.projects.get(f"{git_data.owner}/{git_data.name}") + try: + gl = gitlab.Gitlab(git_data.instance_url, private_token=token) + project = gl.projects.get(f"{git_data.owner}/{git_data.name}") + except gitlab.GitlabAuthenticationError: + # NOTE: Invalid or expired tokens fail even on public projects. Let's give it a try without tokens + try: + gl = gitlab.Gitlab(git_data.instance_url) + project = gl.projects.get(f"{git_data.owner}/{git_data.name}") + except gitlab.GitlabAuthenticationError as e: + raise errors.AuthenticationError from e + except gitlab.GitlabGetError as e: + # NOTE: better to re-raise this as a core error since it's a common case + if "project not found" in getattr(e, "error_message", "").lower(): + raise errors.ProjectNotFound from e + else: + raise result_paths = [] diff --git a/renku/ui/service/views/error_handlers.py b/renku/ui/service/views/error_handlers.py index 8bfd2c532e..17184be107 100644 --- a/renku/ui/service/views/error_handlers.py +++ b/renku/ui/service/views/error_handlers.py @@ -26,6 +26,7 @@ from requests import RequestException from renku.core.errors import ( + AuthenticationError, DatasetExistsError, DatasetImageError, DockerfileUpdateError, @@ -35,6 +36,7 @@ MigrationError, MigrationRequired, ParameterError, + ProjectNotFound, RenkuException, TemplateMissingReferenceError, TemplateUpdateError, @@ -126,7 +128,7 @@ def decorated_function(*args, **kwargs): """Represents decorated function.""" try: return f(*args, **kwargs) - except (ExpiredSignatureError, ImmatureSignatureError, InvalidIssuedAtError) as e: + except (AuthenticationError, ExpiredSignatureError, ImmatureSignatureError, InvalidIssuedAtError) as e: raise IntermittentAuthenticationError(e) return decorated_function @@ -392,6 +394,8 @@ def decorated_function(*args, **kwargs): raise UserProjectTemplateReferenceError(e) except (InvalidTemplateError, TemplateUpdateError) as e: raise IntermittentProjectTemplateUnavailable(e) + except ProjectNotFound as e: + raise UserRepoUrlInvalidError(e) return decorated_function diff --git a/tests/fixtures/config.py b/tests/fixtures/config.py index 3d16b16b2f..dc305ffdd2 100644 --- a/tests/fixtures/config.py +++ b/tests/fixtures/config.py @@ -21,12 +21,14 @@ import pytest +IT_REMOTE_REPO_URL = os.getenv( + "IT_REMOTE_REPOSITORY", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-integration-test" +) IT_PROTECTED_REMOTE_REPO_URL = os.getenv( "IT_PROTECTED_REMOTE_REPO", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-it-protected.git" ) - -IT_REMOTE_REPO_URL = os.getenv( - "IT_REMOTE_REPOSITORY", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-integration-test" +IT_PUBLIC_REMOTE_REPO_URL = os.getenv( + "IT_PUBLIC_REMOTE_REPO", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-it-public" ) IT_REMOTE_NON_RENKU_REPO_URL = os.getenv( "IT_REMOTE_NON_RENKU_REPO_URL", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-it-non-renku" diff --git a/tests/service/fixtures/service_projects.py b/tests/service/fixtures/service_projects.py index 907a865af9..9391a795b9 100644 --- a/tests/service/fixtures/service_projects.py +++ b/tests/service/fixtures/service_projects.py @@ -63,6 +63,14 @@ def it_remote_repo_url(): return IT_REMOTE_REPO_URL +@pytest.fixture(scope="module") +def it_remote_public_renku_repo_url(): + """Returns a remote path to a public integration test repository.""" + from tests.fixtures.config import IT_PUBLIC_REMOTE_REPO_URL + + return IT_PUBLIC_REMOTE_REPO_URL + + @pytest.fixture(scope="module") def it_remote_public_repo_url(): """Returns a remote path to a public integration test repository.""" diff --git a/tests/service/views/test_cache_views.py b/tests/service/views/test_cache_views.py index fb9664d975..78788b3230 100644 --- a/tests/service/views/test_cache_views.py +++ b/tests/service/views/test_cache_views.py @@ -34,6 +34,7 @@ IntermittentProjectTemplateUnavailable, UserAnonymousError, UserProjectTemplateReferenceError, + UserRepoUrlInvalidError, ) from renku.ui.service.serializers.headers import JWT_TOKEN_SECRET from tests.utils import assert_rpc_response, retry_failed @@ -685,6 +686,39 @@ def test_check_migrations_remote(svc_client, identity_headers, it_remote_repo_ur assert response.json["result"]["core_renku_version"] +@pytest.mark.service +@pytest.mark.integration +def test_check_migrations_remote_errors( + svc_client, identity_headers, it_remote_repo_url, it_remote_public_renku_repo_url +): + """Check migrations throws the correct errors.""" + + # NOTE: repo doesn't exist + fake_url = f"{it_remote_repo_url}FAKE_URL" + response = svc_client.get("/cache.migrations_check", query_string=dict(git_url=fake_url), headers=identity_headers) + + assert_rpc_response(response, "error") + assert UserRepoUrlInvalidError.code == response.json["error"]["code"] + + # NOTE: token errors + response = svc_client.get( + "/cache.migrations_check", query_string=dict(git_url=it_remote_repo_url), headers=identity_headers + ) + assert_rpc_response(response) + + identity_headers["Authorization"] = "Bearer 123abc" + response = svc_client.get( + "/cache.migrations_check", query_string=dict(git_url=it_remote_repo_url), headers=identity_headers + ) + assert_rpc_response(response, "error") + assert UserRepoUrlInvalidError.code == response.json["error"]["code"] + + response = svc_client.get( + "/cache.migrations_check", query_string=dict(git_url=it_remote_public_renku_repo_url), headers=identity_headers + ) + assert_rpc_response(response) + + @pytest.mark.service @pytest.mark.integration def test_mirgate_wrong_template_failure(svc_client_with_repo, template, monkeypatch):