diff --git a/renku/core/errors.py b/renku/core/errors.py index b20f2ad039..bb005f3054 100644 --- a/renku/core/errors.py +++ b/renku/core/errors.py @@ -458,6 +458,10 @@ class InvalidTemplateError(TemplateError): """Raised when using a non-valid template.""" +class TemplateMissingReferenceError(TemplateError): + """Raised when using a non-valid template.""" + + class TemplateUpdateError(TemplateError): """Raised when a project couldn't be updated from its template.""" diff --git a/renku/core/template/template.py b/renku/core/template/template.py index 0334d3c862..c70c4cdb3e 100644 --- a/renku/core/template/template.py +++ b/renku/core/template/template.py @@ -420,6 +420,10 @@ def fetch(cls, source: Optional[str], reference: Optional[str]) -> "RepositoryTe try: repository = clone_repository(url=source, path=path, checkout_revision=reference, install_lfs=False) except errors.GitError as e: + if "Cannot checkout reference" in str(e): + raise errors.TemplateMissingReferenceError( + f"Cannot find the reference '{reference}' in the template repository from {source}" + ) from e raise errors.InvalidTemplateError(f"Cannot clone template repository from {source}") from e version = repository.head.commit.hexsha diff --git a/renku/core/template/usecase.py b/renku/core/template/usecase.py index e6bf2f1292..22eca78690 100644 --- a/renku/core/template/usecase.py +++ b/renku/core/template/usecase.py @@ -132,7 +132,15 @@ def update_template( "break your project" ) - templates_source = fetch_templates_source(source=template_metadata.source, reference=template_metadata.reference) + try: + templates_source = fetch_templates_source( + source=template_metadata.source, reference=template_metadata.reference + ) + except errors.TemplateMissingReferenceError as e: + message = f"{str(e)}; You can still manually update the template and set a difference reference." + raise errors.TemplateUpdateError(message) + except errors.InvalidTemplateError: + raise errors.TemplateUpdateError("Template cannot be fetched.") update_available, latest_reference = templates_source.is_update_available( id=template_metadata.id, reference=template_metadata.reference, version=template_metadata.version diff --git a/renku/ui/service/errors.py b/renku/ui/service/errors.py index 48f917c91a..5ae2f53521 100644 --- a/renku/ui/service/errors.py +++ b/renku/ui/service/errors.py @@ -342,6 +342,29 @@ def __init__(self, exception=None): super().__init__(exception=exception) +class UserProjectTemplateReferenceError(ServiceError): + """The project's template original reference cannot be found anymore. + + The reference has probably been removed, either on purpose or as a side effect of a + forced push. + """ + + code = SVC_ERROR_USER + 141 + userMessage = ( + "The project's template original reference has been removed or overwritten." + " Manually changing it in a session may fix the problem." + " Further details: {message}." + ) + devMessage = "Template reference is not available anymore. Details: {message}." + + def __init__(self, exception): + super().__init__( + userMessage=self.userMessage.format(message=str(exception)), + devMessage=self.devMessage.format(message=str(exception)), + exception=exception, + ) + + class ProgramInvalidGenericFieldsError(ServiceError): """One or more fields are unexpected. @@ -668,6 +691,26 @@ def __init__(self, exception=None): super().__init__(exception=exception) +class IntermittentProjectTemplateUnavailable(ServiceError): + """The reference template for the project is currently unavailable. + + It may be a temporary issue in accessing the remote template, or it may have been deleted, + moved, or otherwise not-accessible. + """ + + code = SVC_ERROR_INTERMITTENT + 140 + userMessage = ( + "The reference template for the project is currently unavailable." + " It may be a temporary problem, or the template may not be accessible anymore." + ) + devMessage = ( + "Error accessing the project template. This may be temporary, or the project may not be accessible anymore." + ) + + def __init__(self, exception=None): + super().__init__(exception=exception) + + class IntermittentTimeoutError(ServiceError): """An operation timed out.""" diff --git a/renku/ui/service/views/cache.py b/renku/ui/service/views/cache.py index 64a379a611..a7a2a1cc45 100644 --- a/renku/ui/service/views/cache.py +++ b/renku/ui/service/views/cache.py @@ -28,7 +28,11 @@ from renku.ui.service.gateways.gitlab_api_provider import GitlabAPIProvider from renku.ui.service.views.api_versions import V0_9, V1_0, V1_1, VersionedBlueprint from renku.ui.service.views.decorators import accepts_json, optional_identity, requires_cache, requires_identity -from renku.ui.service.views.error_handlers import handle_common_except, handle_migration_errors +from renku.ui.service.views.error_handlers import ( + handle_common_except, + handle_migration_read_errors, + handle_migration_write_errors, +) from renku.ui.service.views.v0_9.cache import add_v0_9_specific_endpoints from renku.ui.service.views.v1_0.cache import add_v1_0_specific_endpoints @@ -156,7 +160,7 @@ def list_projects_view(user_data, cache): @cache_blueprint.route("/cache.migrate", methods=["POST"], provide_automatic_options=False, versions=[V1_1]) @handle_common_except -@handle_migration_errors +@handle_migration_write_errors @accepts_json @requires_cache @requires_identity @@ -187,6 +191,7 @@ def migrate_project_view(user_data, cache): "/cache.migrations_check", methods=["GET"], provide_automatic_options=False, versions=[V1_0, V1_1] ) @handle_common_except +@handle_migration_read_errors @requires_cache @optional_identity def migration_check_project_view(user_data, cache): diff --git a/renku/ui/service/views/error_handlers.py b/renku/ui/service/views/error_handlers.py index 5a3c8fb2d1..583a79ffdf 100644 --- a/renku/ui/service/views/error_handlers.py +++ b/renku/ui/service/views/error_handlers.py @@ -36,6 +36,7 @@ MigrationRequired, ParameterError, RenkuException, + TemplateMissingReferenceError, TemplateUpdateError, UninitializedProject, ) @@ -44,6 +45,7 @@ IntermittentDatasetExistsError, IntermittentFileNotExistsError, IntermittentProjectIdError, + IntermittentProjectTemplateUnavailable, IntermittentRedisError, IntermittentSettingExistsError, IntermittentTimeoutError, @@ -65,6 +67,7 @@ UserNonRenkuProjectError, UserOutdatedProjectError, UserProjectCreationError, + UserProjectTemplateReferenceError, UserRepoBranchInvalidError, UserRepoNoAccessError, UserRepoUrlInvalidError, @@ -375,8 +378,25 @@ def decorated_function(*args, **kwargs): return decorated_function -def handle_migration_errors(f): - """Wrapper which handles migrations exceptions.""" +def handle_migration_read_errors(f): + """Wrapper which handles migrations read exceptions.""" + # noqa + @wraps(f) + def decorated_function(*args, **kwargs): + """Represents decorated function.""" + try: + return f(*args, **kwargs) + except TemplateMissingReferenceError as e: + raise UserProjectTemplateReferenceError(e) + except (InvalidTemplateError, TemplateUpdateError) as e: + raise IntermittentProjectTemplateUnavailable(e) + + return decorated_function + + +@handle_migration_read_errors +def handle_migration_write_errors(f): + """Wrapper which handles migrations write exceptions.""" # noqa @wraps(f) def decorated_function(*args, **kwargs): diff --git a/renku/ui/service/views/v1_0/cache.py b/renku/ui/service/views/v1_0/cache.py index b8913b2434..4bb2c4e5a8 100644 --- a/renku/ui/service/views/v1_0/cache.py +++ b/renku/ui/service/views/v1_0/cache.py @@ -22,11 +22,11 @@ from renku.ui.service.serializers.v1_0.cache import ProjectMigrateResponseRPC_1_0 from renku.ui.service.views.api_versions import V0_9, V1_0 from renku.ui.service.views.decorators import accepts_json, requires_cache, requires_identity -from renku.ui.service.views.error_handlers import handle_common_except, handle_migration_errors +from renku.ui.service.views.error_handlers import handle_common_except, handle_migration_write_errors @handle_common_except -@handle_migration_errors +@handle_migration_write_errors @accepts_json @requires_cache @requires_identity diff --git a/tests/cli/test_template.py b/tests/cli/test_template.py index 15e2494557..3e5dfb399c 100644 --- a/tests/cli/test_template.py +++ b/tests/cli/test_template.py @@ -245,6 +245,16 @@ def test_template_update_latest_version(runner, client): assert "Template is up-to-date" in result.output +@pytest.mark.integration +def test_template_update_missing_repo(runner, client_with_template): + """Test update with a none-existing template repository fails with expected error.""" + result = runner.invoke(cli, ["template", "update"]) + + assert 1 == result.exit_code + assert "Template cannot be fetched" in result.output + assert not client_with_template.repository.is_dirty() + + @pytest.mark.integration def test_template_update_dry_run(runner, client): """Test update dry-run doesn't make any changes.""" diff --git a/tests/core/management/test_template.py b/tests/core/management/test_template.py index 41b426ddfd..ce0cdbfc55 100644 --- a/tests/core/management/test_template.py +++ b/tests/core/management/test_template.py @@ -56,7 +56,7 @@ def test_template_fetch_invalid_git_url(): @pytest.mark.vcr def test_template_fetch_invalid_git_reference(): """Test fetching a template from an invalid reference.""" - with pytest.raises(errors.InvalidTemplateError): + with pytest.raises(errors.TemplateMissingReferenceError): fetch_templates_source(source=TEMPLATES_URL, reference="invalid-ref") @@ -85,6 +85,13 @@ def test_template_update_files(client_with_template, templates_source, client_da assert file.read_text() != files_before[file] +def test_template_update_source_failure(client_with_template, client_database_injection_manager): + """Test template update with broken template source.""" + with client_database_injection_manager(client_with_template): + with pytest.raises(errors.TemplateUpdateError): + update_template(force=False, interactive=False, dry_run=False) + + @pytest.mark.parametrize( "action, content_type", [ diff --git a/tests/service/views/test_cache_views.py b/tests/service/views/test_cache_views.py index 86090bb20f..fb9664d975 100644 --- a/tests/service/views/test_cache_views.py +++ b/tests/service/views/test_cache_views.py @@ -26,11 +26,17 @@ from renku.core.dataset.context import DatasetContext from renku.domain_model.git import GitURL +from renku.domain_model.template import TemplateMetadata from renku.infrastructure.gateway.dataset_gateway import DatasetGateway from renku.infrastructure.repository import Repository -from renku.ui.service.errors import IntermittentFileExistsError, UserAnonymousError +from renku.ui.service.errors import ( + IntermittentFileExistsError, + IntermittentProjectTemplateUnavailable, + UserAnonymousError, + UserProjectTemplateReferenceError, +) from renku.ui.service.serializers.headers import JWT_TOKEN_SECRET -from tests.utils import retry_failed +from tests.utils import assert_rpc_response, retry_failed @pytest.mark.service @@ -681,18 +687,62 @@ def test_check_migrations_remote(svc_client, identity_headers, it_remote_repo_ur @pytest.mark.service @pytest.mark.integration -def test_check_no_migrations(svc_client_with_repo): - """Check if migrations are not required.""" - svc_client, headers, project_id, _ = svc_client_with_repo - - response = svc_client.get("/cache.migrations_check", query_string=dict(project_id=project_id), headers=headers) +def test_mirgate_wrong_template_failure(svc_client_with_repo, template, monkeypatch): + """Check if migrations gracefully fail when the project template is not available.""" + import renku.core.template.usecase + from renku.core.template.template import fetch_templates_source - assert 200 == response.status_code + svc_client, headers, project_id, _ = svc_client_with_repo - assert not response.json["result"]["core_compatibility_status"]["migration_required"] - assert not response.json["result"]["template_status"]["newer_template_available"] - assert not response.json["result"]["dockerfile_renku_status"]["automated_dockerfile_update"] - assert response.json["result"]["project_supported"] + class DummyTemplateMetadata(TemplateMetadata): + def __init__(self, metadata, immutable_files): + super().__init__(metadata=metadata, immutable_files=immutable_files) + + def set_fake_source(self, value): + """Toggle source between fake and real""" + self.fake_source = value + + @property + def source(self): + """Template source.""" + template_url = template["url"] + if self.fake_source: + return f"{template_url}FAKE_URL" + return template_url + + @property + def reference(self): + """Template reference.""" + return "FAKE_REF" + + def dummy_check_for_template_update(client): + metadata = DummyTemplateMetadata.from_client(client=client) + metadata.set_fake_source(fake_source) + templates_source = fetch_templates_source(source=metadata.source, reference=metadata.reference) + update_available, latest_reference = templates_source.is_update_available( + id=metadata.id, reference=metadata.reference, version=metadata.version + ) + return update_available, metadata.allow_update, metadata.reference, latest_reference + + # NOTE: fake URL and fake REF + fake_source = True + with monkeypatch.context() as monkey: + monkey.setattr(renku.command.migrate, "check_for_template_update", dummy_check_for_template_update) + + response = svc_client.get("/cache.migrations_check", query_string=dict(project_id=project_id), headers=headers) + + assert_rpc_response(response, "error") + assert IntermittentProjectTemplateUnavailable.code == response.json["error"]["code"] + + # NOTE: valid URL but fake REF + fake_source = False + with monkeypatch.context() as monkey: + monkey.setattr(renku.command.migrate, "check_for_template_update", dummy_check_for_template_update) + + response = svc_client.get("/cache.migrations_check", query_string=dict(project_id=project_id), headers=headers) + + assert_rpc_response(response, "error") + assert UserProjectTemplateReferenceError.code == response.json["error"]["code"] @pytest.mark.service