Skip to content

Commit

Permalink
Tests/Rule move_rule with invalid Source Replica Expression; Fix ru…
Browse files Browse the repository at this point in the history
…cio#4845

Currently, a misleading error message is raised to the user upon
attempting to move a rule which has an invalid Source Replica
Expression. An invalid Source Replica Expression is an RSE expression
which evaluates to an empty set, which can occur due to a variety of
reasons.

The misleading error message informs the user that the provided `RSE
expression is considered invalid`, this is misleading, since the error
does not stem from the RSE expression provided to the `move_rule`
command. Instead, the failure lies in parsing the Source Replica
Expression (which may come as an attribute of the rule prior to
moving/updating it, or be provided by the user as an additional
argument). This failure is then not communicated to the user in a clear
manner.

The solution is to introduce a new exception
`InvalidSourceReplicaExpression` and raise it within
`core/rule.py::add_rule`, should the attempt to parse the Source Replica
Expression with `parse_expression` fail.

This commit also includes a test added to `test_rule.py::TestCore` which
simulates two ways in which the Source Replica Expression might become
invalid:
1) By deleting the only RSE the Source Replica Expression points to.
2) By matching the RSE to the Source Replica Expression via an RSE
   attribute but then deleting this RSE attribute from the RSE.
In both cases the Source Replica Expression will evaluate to an empty
set.

The test makes use of the `caches_mock`-fixture to override the internal
cache of `rse_expression_parser`. Since the parser caches previously
parsed expressions, access to the cache is necessary to clear the cache
after altering the RSE (either by deleting the RSE or by deleting an
attribute of the RSE).
  • Loading branch information
ThePhisch committed Feb 14, 2023
1 parent e852b16 commit 887b69b
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 3 deletions.
22 changes: 22 additions & 0 deletions lib/rucio/common/exception.py
Expand Up @@ -1069,3 +1069,25 @@ def __init__(self, package, *args, **kwargs):
super(PolicyPackageVersionError, self).__init__(*args, **kwargs)
self._message = 'Policy package %s is not compatible with this Rucio version' % package
self.error_code = 103


class ServerSSLCertificateExpiredException(RucioException):
"""
Server SSL Certificate has expired, auth is no longer possible
"""

def __init__(self, enddatetime: Optional[str], *args, **kwargs):
super(ServerSSLCertificateExpiredException, self).__init__(*args, **kwargs)
self._message = f'Server SSL Certificate has expired on {enddatetime}, auth is no longer possible'
self.error_code = 104


class InvalidSourceReplicaExpression(RucioException):
"""
Source Replica Expression Considered Invalid
"""

def __init__(self, *args, **kwargs):
super(InvalidSourceReplicaExpression, self).__init__(*args, **kwargs)
self._message = 'Provided Source Replica expression is considered invalid.'
self.error_code = 105
8 changes: 6 additions & 2 deletions lib/rucio/core/rule.py
Expand Up @@ -47,7 +47,8 @@
ReplicationRuleCreationTemporaryFailed, InsufficientTargetRSEs, RucioException,
InvalidRuleWeight, StagingAreaRuleRequiresLifetime, DuplicateRule,
InvalidObject, RSEWriteBlocked, RuleReplaceFailed, RequestNotFound,
ManualRuleApprovalBlocked, UnsupportedOperation, UndefinedPolicy, InvalidValueForKey)
ManualRuleApprovalBlocked, UnsupportedOperation, UndefinedPolicy, InvalidValueForKey,
InvalidSourceReplicaExpression)
from rucio.common.schema import validate_schema
from rucio.common.types import InternalScope, InternalAccount
from rucio.common.utils import str_to_date, sizefmt, chunks
Expand Down Expand Up @@ -151,7 +152,10 @@ def add_rule(dids, account, copies, rse_expression, grouping, weight, lifetime,
raise ManualRuleApprovalBlocked()

if source_replica_expression:
source_rses = parse_expression(source_replica_expression, filter_={'vo': vo}, session=session)
try:
source_rses = parse_expression(source_replica_expression, filter_={'vo': vo}, session=session)
except InvalidRSEExpression:
raise InvalidSourceReplicaExpression
else:
source_rses = []

Expand Down
74 changes: 73 additions & 1 deletion lib/rucio/tests/test_rule.py
Expand Up @@ -13,6 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from collections import namedtuple
import json
import random
import string
Expand All @@ -26,7 +27,7 @@
from rucio.common.config import config_get_bool
from rucio.common.exception import (RucioException, RuleNotFound, AccessDenied, InsufficientAccountLimit, DuplicateRule, RSEWriteBlocked,
RSEOverQuota, RuleReplaceFailed, ManualRuleApprovalBlocked, InputValidationError,
UnsupportedOperation, InvalidValueForKey)
UnsupportedOperation, InvalidValueForKey, InvalidSourceReplicaExpression)
from rucio.common.policy import get_policy
from rucio.common.schema import get_schema_value
from rucio.common.types import InternalAccount, InternalScope
Expand All @@ -53,8 +54,10 @@
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from typing import Tuple
from rucio.tests.temp_factories import TemporaryDidFactory, TemporaryRSEFactory

LOG = getLogger(__name__)
RSE_namedtuple = namedtuple('RSE_namedtuple', ['name', 'id'])


def create_files(nrfiles, scope, rse_id, bytes_=1):
Expand Down Expand Up @@ -153,6 +156,75 @@ def setup_class(request, vo, root_account, jdoe_account, rse_factory):
@pytest.mark.usefixtures('setup_class')
class TestCore:

@pytest.mark.parametrize("caches_mock", [{"caches_to_mock": [
'rucio.core.rse_expression_parser.REGION'
]}], indirect=True)
@pytest.mark.parametrize(
"delete_rse", [True, False]
)
def test_move_rule_invalid_source_replica_expression(
self,
caches_mock,
delete_rse: bool,
did_factory: "TemporaryDidFactory",
rse_factory: "TemporaryRSEFactory",
root_account: "InternalAccount"
):
"""
REPLICATION RULE (CORE): Move a rule using an invalid source replica expression.
Parametrised so that there are two different ways for the source replica
expression to become invalid:
1) by deleting the source RSE
2) by referring to the source RSE via an RSE attribute and then deleting
said RSE attribute from the source RSE
in both cases the source replica expression is invalidated since it
evaluates to an empty set.
"""

from dogpile.cache.api import NO_VALUE
from hashlib import sha256

# create RSEs A, B, C (structured into namedtuples)
rseA, rseB, rseC = (
RSE_namedtuple(*rse_factory.make_mock_rse()) for _ in range(3)
)
rsekey, rseval = tag_generator(), tag_generator()
add_rse_attribute(rseA.id, rsekey, rseval)

# create DID (dataset is easiest)
dataset = did_factory.make_dataset()

# create source replica expression
if delete_rse:
SRE = rseA.name
else:
SRE = f"{rsekey}={rseval}"

# add rule binding DID to RSE-B, source replica expression matches RSE-A
rule = {
"account": root_account,
"copies": 1,
"rse_expression": rseB.name,
"source_replica_expression": SRE
}
ruledict = add_rules([dataset], [rule])
rule_id = ruledict[(dataset["scope"], dataset["name"])][0]

# delete RSE-A (or otherwise unmatching from source replica expr)
if delete_rse:
from rucio.core.rse import del_rse
del_rse(rseA.id)
else:
del_rse_attribute(rseA.id, rsekey)

# must remove cached entries for the Source Replica Expression
caches_mock[0].set(sha256(SRE.encode()).hexdigest(), NO_VALUE)

# move-rule asserts correct error
with pytest.raises(InvalidSourceReplicaExpression):
move_rule(rule_id, rseC.name)

def test_add_rule_to_file_ask_approval(self, vo, mock_scope, jdoe_account):
""" REPLICATION RULE (CORE): Add a replication rule, asking approval"""
rse = rse_name_generator()
Expand Down

0 comments on commit 887b69b

Please sign in to comment.