Clients: allow upload without configuring the account

Motivation:

Commit 0032693 updated the client so that it was no longer necessary
for the client to specify the account; the support for this already
existed in the server.

Unfortunately, this patch was incomplete: it is still required that the
user configures the desired account when uploading data, even when the
user's identity maps to a single identity.

Moreover, failure to configure the account leads to an obscure error
message:

    can only concatenate str (not "NoneType") to str

This provides the user with no useful information on how to correct the
problem.  Enabling verbose output leads to the client providing arguably
incorrect information:

    This means the parameter you passed has a wrong type.

Modification:

The UploadClient class now discovers an account for the user's identity
if no account was specified.  It does this by making a whoami query
against the Rucio server, and using the supplied account.

If the server is unable to provide the account information for this user
then the operation will fail with a clear error message that provides
information on how to correct the situation.

Functional tests have been added to verify correct behaviour.  Note
that, due to issue rucio#7394, the tests are skipped for the M-VO deployment
scenarios; however, manual testing with a real-life M-VO deployment
(interTwin) show that this patch also works for that Rucio instance.

Result:

It is now possible to upload data without the client configuring the
desired account.  Under these circumstances, the default account for
that user identity is used.

Closes: rucio#7349
Signed-off-by: Paul Millar <paul.millar@desy.de>

lib/rucio/client/uploadclient.py

@@ -81,6 +81,16 @@ def __init__(
         self.tracing = tracing
         if not self.tracing:
             logger(logging.DEBUG, 'Tracing is turned off.')
+        if self.client.account is None:
+            self.logger(logging.DEBUG, 'No account specified, querying rucio.')
+            try:
+                acc = self.client.whoami()
+                if acc is None:
+                    raise InputValidationError('account not specified and rucio has no account with your identity')
+                self.client.account = acc['account']
+            except RucioException as e:
+                raise InputValidationError('account not specified and problem with rucio: %s' % e)
+            self.logger(logging.DEBUG, 'Discovered account as "%s"' % self.client.account)
         self.default_file_scope: Final[str] = 'user.' + self.client.account
         self.rses = {}
         self.rse_expressions = {}

tests/conftest.py

@@ -67,8 +67,10 @@ def pytest_make_parametrize_id(
 ) -> Optional[str]:
     if argname == 'file_config_mock':
         cfg = {}
-        for section, option, value in val['overrides']:
+        for section, option, value in val.get('overrides', []):
             cfg.setdefault(section, {})[option] = value
+        for section, option in val.get('removes', []):
+            cfg.setdefault(section, {})[option] = "[REMOVED]"
         return argname + str(cfg)
     if argname == 'core_config_mock':
         cfg = {}
@@ -604,20 +606,25 @@ def file_config_mock(request: pytest.FixtureRequest) -> "Iterator[ConfigParser]"
     """
     from unittest import mock
 
-    from rucio.common.config import Config, config_add_section, config_has_section, config_set
+    from rucio.common.config import Config, config_add_section, config_has_option, config_has_section, config_remove_option, config_set
 
     # Get the fixture parameters
     overrides = []
+    removes = []
     params = __get_fixture_param(request)
     if params:
         overrides = params.get("overrides", overrides)
+        removes = params.get("removes", removes)
 
     parser = Config().parser
     with mock.patch('rucio.common.config.get_config', side_effect=lambda: parser):
         for section, option, value in (overrides or []):
             if not config_has_section(section):
                 config_add_section(section)
             config_set(section, option, value)
+        for section, option in (removes or []):
+            if config_has_section(section) and config_has_option(section, option):
+                config_remove_option(section, option)
         yield parser
 
 

tests/test_upload.py

@@ -24,7 +24,7 @@
 from rucio.client.client import Client
 from rucio.client.uploadclient import UploadClient
 from rucio.common.checksum import adler32
-from rucio.common.config import config_add_section, config_set
+from rucio.common.config import config_add_section, config_has_option, config_set
 from rucio.common.constants import RseAttr
 from rucio.common.exception import InputValidationError, NoFilesUploaded, NotAllFilesUploaded, ResourceTemporaryUnavailable
 from rucio.common.utils import generate_uuid
@@ -55,8 +55,18 @@ def scope(vo, containerized_rses, test_scope, mock_scope):
     else:
         return str(mock_scope)
 
-
-def test_upload_single(rse, scope, upload_client, download_client, file_factory):
+@pytest.mark.parametrize("file_config_mock", [
+    { # Use rucio.cfg as-is.
+    },
+    pytest.param(
+    { # Remove "account" from the "[client]" section.
+        "removes": [
+            ('client', 'account')
+        ]
+    }, marks=pytest.mark.skipif('SUITE' in os.environ and os.environ['SUITE'] == 'multi_vo',
+                                reason="See https://github.com/rucio/rucio/issues/7394"))
+], indirect=True)
+def test_upload_single(file_config_mock, rse, scope, upload_client, download_client, file_factory):
     local_file = file_factory.file_generator()
     download_dir = file_factory.base_dir
     fn = os.path.basename(local_file)