Policies: standardise permission policy check logic rucio#7206

Author: jamesp-epcc

lib/rucio/common/policy.py

@@ -13,28 +13,37 @@
 # limitations under the License.
 
 import json
+import logging
 import os
 from configparser import NoOptionError, NoSectionError
 from functools import wraps
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
 from dogpile.cache import make_region
 from dogpile.cache.api import NoValue
 
 from rucio.common.config import config_get
 from rucio.common.exception import UndefinedPolicy
 
+if TYPE_CHECKING:
+    from rucio.common.types import LoggerFunction
+
 REGION = make_region().configure('dogpile.cache.memory',
                                  expiration_time=900)
 
 
-def get_policy() -> str:
+def get_policy(logger: 'LoggerFunction' = logging.log) -> str:
     policy = REGION.get('policy')
     if isinstance(policy, NoValue):
         try:
             policy = config_get('policy', 'permission')
         except (NoOptionError, NoSectionError):
-            policy = 'atlas'
+            try:
+                policy = config_get('permission', 'policy')
+            except (NoOptionError, NoSectionError):
+                policy = 'def'
+                logger(logging.WARNING, "Policy not specified, falling back to 'def'")
+        policy = os.environ.get('POLICY', policy)
         REGION.set('policy', policy)
     return policy
 

lib/rucio/core/permission/__init__.py

@@ -21,6 +21,7 @@
 import rucio.core.permission.generic
 from rucio.common import config, exception
 from rucio.common.plugins import check_policy_package_version
+from rucio.common.policy import get_policy
 
 if TYPE_CHECKING:
     from typing import Optional
@@ -43,17 +44,8 @@
 if not multivo:
     generic_fallback = 'generic'
 
-    if config.config_has_section('permission'):
-        try:
-            fallback_policy = config.config_get('permission', 'policy')
-        except (NoOptionError, NoSectionError):
-            fallback_policy = generic_fallback
-    elif config.config_has_section('policy'):
-        try:
-            fallback_policy = config.config_get('policy', 'permission')
-        except (NoOptionError, NoSectionError):
-            fallback_policy = generic_fallback
-    else:
+    fallback_policy = get_policy()
+    if fallback_policy == 'def':
         fallback_policy = generic_fallback
 
     if config.config_has_section('policy'):

lib/rucio/transfertool/fts3.py

@@ -33,6 +33,7 @@
 from rucio.common.config import config_get, config_get_bool, config_get_int, config_get_list
 from rucio.common.constants import FTS_COMPLETE_STATE, FTS_JOB_TYPE, FTS_STATE, RseAttr
 from rucio.common.exception import DuplicateFileTransferSubmission, TransferToolTimeout, TransferToolWrongAnswer
+from rucio.common.policy import get_policy
 from rucio.common.stopwatch import Stopwatch
 from rucio.common.utils import APIEncoder, chunks, deep_merge_dict
 from rucio.core.monitor import MetricManager
@@ -1369,10 +1370,7 @@ def set_se_config(
 
         params_dict = {storage_element: {'operations': {}, 'se_info': {}}}
         if staging is not None:
-            try:
-                policy = config_get('policy', 'permission')
-            except Exception:
-                self.logger(logging.WARNING, 'Could not get policy from config')
+            policy = get_policy()
             params_dict[storage_element]['operations'] = {policy: {'staging': staging}}
         # A lot of try-excepts to avoid dictionary overwrite's,
         # see https://stackoverflow.com/questions/27118687/updating-nested-dictionaries-when-data-has-existing-key/27118776

lib/rucio/web/ui/flask/bp.py

@@ -15,13 +15,14 @@
 
 from flask import Blueprint, make_response, render_template, request
 
-from rucio.common.config import config_get, config_get_bool
+from rucio.common.config import config_get_bool
+from rucio.common.policy import get_policy
 from rucio.gateway.authentication import get_auth_token_x509
 from rucio.web.rest.flaskapi.v1.common import generate_http_error_flask
 from rucio.web.ui.flask.common.utils import AUTH_ISSUERS, SAML_SUPPORT, USERPASS_SUPPORT, authenticate, finalize_auth, get_token, oidc_auth, saml_auth, userpass_auth, x509token_auth
 
 MULTI_VO = config_get_bool('common', 'multi_vo', raise_exception=False, default=False)
-POLICY = config_get('policy', 'permission')
+POLICY = get_policy()
 ATLAS_URLS = ()
 OTHER_URLS = ()
 

lib/rucio/web/ui/flask/common/utils.py

@@ -26,6 +26,7 @@
 from rucio.common.config import config_get, config_get_bool
 from rucio.common.exception import CannotAuthenticate
 from rucio.common.extra import import_extras
+from rucio.common.policy import get_policy
 from rucio.core import identity as identity_core
 from rucio.core import vo as vo_core
 from rucio.db.sqla.constants import AccountType, IdentityType
@@ -236,7 +237,7 @@ def access_granted(valid_token_dict, template, title):
     :param template: the template name that should be rendered
     :returns: rendered base temmplate with template content
     """
-    policy = config_get('policy', 'permission')
+    policy = get_policy()
     return render_template(template, token=valid_token_dict['token'], account=valid_token_dict['account'], vo=valid_token_dict['vo'], policy=policy, title=title)
 
 

tests/test_lifetime.py

@@ -33,6 +33,7 @@
 
 @pytest.mark.noparallel(reason='Race conditions with the other tests in test_lifetime.py, they all modify and use the config.')
 @skip_multivo(reason='only valid for ATLAS')
+@pytest.mark.skipif(os.environ.get('POLICY') != 'atlas', reason='ATLAS-specific test')
 def test_lifetime_creation_core(root_account, rse_factory, mock_scope, did_factory):
     """
     Test the creation of a lifetime exception on the core side
@@ -101,6 +102,7 @@ def test_lifetime_creation_core(root_account, rse_factory, mock_scope, did_facto
 
 @pytest.mark.noparallel(reason='Race conditions with the other tests in test_lifetime.py, they all modify and use the config.')
 @skip_multivo(reason='only valid for ATLAS')
+@pytest.mark.skipif(os.environ.get('POLICY') != 'atlas', reason='ATLAS-specific test')
 def test_lifetime_truncate_expiration(root_account, rse_factory, mock_scope, did_factory, rucio_client):
     """
     Test the duration of a lifetime exception is truncated if max_extension is defined
@@ -134,6 +136,7 @@ def test_lifetime_truncate_expiration(root_account, rse_factory, mock_scope, did
 
 @pytest.mark.noparallel(reason='Race conditions with the other tests in test_lifetime.py, they all modify and use the config.')
 @skip_multivo(reason='only valid for ATLAS')
+@pytest.mark.skipif(os.environ.get('POLICY') != 'atlas', reason='ATLAS-specific test')
 def test_lifetime_creation_client(root_account, rse_factory, mock_scope, did_factory, rucio_client):
     """
     Test the creation of a lifetime exception on the client side and that the exception can be listed with the client
@@ -210,6 +213,7 @@ def test_lifetime_creation_client(root_account, rse_factory, mock_scope, did_fac
 @skip_multivo(reason='only valid for ATLAS')
 @pytest.mark.dirty
 @pytest.mark.noparallel(reason='Uses daemons. Write a configuration file')
+@pytest.mark.skipif(os.environ.get('POLICY') != 'atlas', reason='ATLAS-specific test')
 def test_atropos(root_account, rse_factory, mock_scope, did_factory, rucio_client):
     """
     Test the behaviour of atropos

tests/test_rule.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import json
+import os
 import random
 import string
 from collections import namedtuple
@@ -41,7 +42,6 @@
     RuleReplaceFailed,
     UnsupportedOperation,
 )
-from rucio.common.policy import get_policy
 from rucio.common.schema import get_schema_value
 from rucio.common.types import InternalAccount, InternalScope
 from rucio.common.utils import generate_uuid as uuid
@@ -928,12 +928,9 @@ def test_add_rule_with_ignore_availability(self, vo, mock_scope, did_factory, jd
             for filtered_lock in [lock for lock in get_replica_locks(scope=file['scope'], name=file['name'])]:
                 assert (filtered_lock['state'] == LockState.STUCK)
 
+    @pytest.mark.skipif(os.environ.get('POLICY') != 'atlas', reason='ATLAS-specific test')
     def test_delete_rule_country_admin(self, vo, mock_scope, did_factory, jdoe_account):
         """ REPLICATION RULE (CORE): Delete a rule with a country admin account"""
-        if get_policy() != 'atlas':
-            LOG.info("Skipping atlas-specific test")
-            return
-
         rse = rse_name_generator()
         rse_id = add_rse(rse, vo=vo)
         add_rse_attribute(rse_id, RseAttr.COUNTRY, 'test')
@@ -1024,12 +1021,9 @@ def test_move_rule_with_arguments(self, mock_scope, did_factory, jdoe_account):
         pytest.raises(RuleReplaceFailed, move_rule, rule_id, self.rse3)
         pytest.raises(RucioException, move_rule, 'foo', self.rse3)
 
+    @pytest.mark.skipif(os.environ.get('POLICY') != 'atlas', reason='ATLAS-specific test')
     def test_add_rule_with_scratchdisk(self, vo, mock_scope, did_factory, jdoe_account):
         """ REPLICATION RULE (CORE): Add a replication rule for scratchdisk"""
-        if get_policy() != 'atlas':
-            LOG.info("Skipping atlas-specific test")
-            return
-
         rse = rse_name_generator()
         rse_id = add_rse(rse, vo=vo)
         add_rse_attribute(rse_id, RseAttr.TYPE, 'SCRATCHDISK')

tests/test_undertaker.py

@@ -12,12 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 from datetime import datetime, timedelta
 from logging import getLogger
 
 import pytest
 
-from rucio.common.policy import get_policy
 from rucio.common.types import InternalScope
 from rucio.core.account_limit import set_local_account_limit
 from rucio.core.did import add_dids, attach_dids, get_did, list_expired_dids, set_metadata
@@ -100,12 +100,9 @@ def test_list_expired_dids_with_locked_rules(self, rse_factory, vo, mock_scope,
         for did in list_expired_dids(limit=1000):
             assert (did['scope'], did['name']) != (dsn['scope'], dsn['name'])
 
+    @pytest.mark.skipif(os.environ.get('POLICY') != 'atlas', reason='ATLAS-specific test')
     def test_atlas_archival_policy(self, vo, mock_scope, root_account, jdoe_account):
         """ UNDERTAKER (CORE): Test the atlas archival policy. """
-        if get_policy() != 'atlas':
-            LOG.info("Skipping atlas-specific test")
-            return
-
         nbdatasets = 5
         nbfiles = 5