Skip to content

Commit

Permalink
Clients: Download client does not filter out tape sources Fix rucio#5122
Browse files Browse the repository at this point in the history


The download client should not download files from a tape RSE if the user is not
privileged.

The code to check that had a bug introduced in rucio#4196, so the user could read
from tape RSEs without restriction. This commit fixes the issue.
  • Loading branch information
Joel Dierkes committed Dec 21, 2021
1 parent d59c237 commit 820d9d4
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/rucio/client/downloadclient.py
Expand Up @@ -1205,7 +1205,7 @@ def _resolve_and_merge_input_items(self, input_items):
for file_item in file_items:
unfiltered_sources = copy.copy(file_item['sources'])
for src in unfiltered_sources:
if src in tape_rses:
if src['rse'] in tape_rses:
file_item['sources'].remove(src)
if unfiltered_sources and not file_item['sources']:
logger(logging.WARNING, 'The requested DID {} only has replicas on tape. Direct download from tape is prohibited. '
Expand Down
19 changes: 18 additions & 1 deletion lib/rucio/tests/test_download.py
Expand Up @@ -43,7 +43,7 @@
from rucio.common.utils import generate_uuid
from rucio.core import did as did_core
from rucio.core import scope as scope_core
from rucio.core.rse import add_protocol
from rucio.core.rse import add_protocol, add_rse_attribute
from rucio.rse import rsemanager as rsemgr
from rucio.rse.protocols.posix import Default as PosixProtocol
from rucio.tests.common import skip_rse_tests_with_accounts, scope_name_generator, file_generator
Expand Down Expand Up @@ -723,3 +723,20 @@ def test_download_file_with_supported_protocol_from_config(rse_factory, did_fact
patch('rucio.rse.protocols.%s.Default.close' % supported_impl):
download_client.download_dids([{'did': did_str, 'impl': supported_impl}])
mock_get.assert_called()


@pytest.mark.noparallel(reason='Avoid recalculation of rse expression')
@pytest.mark.parametrize("caches_mock", [{"caches_to_mock": [
'rucio.core.rse_expression_parser.REGION',
]}], indirect=True)
def test_download_exclude_tape(rse_factory, did_factory, download_client, caches_mock):
"""Client: Do not download from a tape rse."""
rse, rse_id = rse_factory.make_posix_rse()
did = did_factory.upload_test_file(rse)
did_str = '%s:%s' % (did['scope'], did['name'])

add_rse_attribute(rse_id, "istape", True)

with TemporaryDirectory() as tmp_dir:
with pytest.raises(NoFilesDownloaded):
download_client.download_dids([{'did': did_str, 'base_dir': tmp_dir}])

0 comments on commit 820d9d4

Please sign in to comment.