Skip to content

Commit

Permalink
Removed check_permissions_fl
Browse files Browse the repository at this point in the history
  • Loading branch information
nmassey001 committed Oct 2, 2024
1 parent 8883a12 commit 3e3bf9a
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 68 deletions.
14 changes: 7 additions & 7 deletions nlds/rabbit/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,13 @@ class State(Enum):
# Generic states
INITIALISING = -1
ROUTING = 0
COMPLETE = 100
FAILED = 101
COMPLETE_WITH_ERRORS = 102
COMPLETE_WITH_WARNINGS = 103
# PUT workflow states
SPLITTING = 1
INDEXING = 2
CATALOG_PUTTING = 3
TRANSFER_PUTTING = 4
CATALOG_ROLLBACK = 5
CATALOG_UPDATE = 6
CATALOG_UPDATING = 6
# GET workflow states
CATALOG_GETTING = 10
ARCHIVE_GETTING = 11
Expand All @@ -42,6 +38,11 @@ class State(Enum):
CATALOG_ARCHIVE_REMOVING = 40
CATALOG_DELETE_ROLLBACK = 41
CATALOG_RESTORING = 42
# Complete states
COMPLETE = 100
FAILED = 101
COMPLETE_WITH_ERRORS = 102
COMPLETE_WITH_WARNINGS = 103
# Initial state for searching for sub-states
SEARCHING = 1000

Expand All @@ -57,8 +58,7 @@ def has_name(cls, name):
def get_final_states(cls):
final_states = (
cls.TRANSFER_GETTING,
#cls.TRANSFER_PUTTING,
cls.CATALOG_UPDATE,
cls.CATALOG_UPDATING,
cls.CATALOG_ARCHIVE_UPDATING,
cls.CATALOG_ROLLBACK,
cls.CATALOG_ARCHIVE_REMOVING,
Expand Down
30 changes: 10 additions & 20 deletions nlds/rabbit/statting_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,12 @@ class StattingConsumer(RMQC):
_FILELIST_MAX_LENGTH = "filelist_max_length"
_MESSAGE_MAX_SIZE = "message_threshold"
_PRINT_TRACEBACKS = "print_tracebacks_fl"
_CHECK_PERMISSIONS = "check_permissions_fl"
_CHECK_FILESIZE = "check_filesize_fl"

# The corresponding default values
DEFAULT_CONSUMER_CONFIG = {
_FILELIST_MAX_LENGTH: 1000,
_MESSAGE_MAX_SIZE: 1000, # in kB
_CHECK_PERMISSIONS: True,
_CHECK_FILESIZE: True,
}

Expand Down Expand Up @@ -159,37 +157,29 @@ def set_ids(
self.gids = gids

def check_path_access(
self,
path: pth.Path,
stat_result: NamedTuple = None,
access: int = os.R_OK,
check_permissions_fl: bool = True,
self, path: pth.Path, stat_result: NamedTuple = None, access: int = os.R_OK
) -> bool:
"""Checks that the given path is accessible, either by checking for its
existence or, if the check_permissions_fl is set, by doing a permissions
check on the file's bitmask. This requires a stat of the file, so one
must be provided via stat_result else one is performed. The uid and gid
of the user must be set in the object as well, usually by having
performed RabbitMQConsumer.set_ids() beforehand.
existence and by doing a permissions check on the file's bitmask. This requires
a stat of the file, so one must be provided via stat_result else one is
performed. The uid and gid of the user must be set in the object as well,
usually by having performed RabbitMQConsumer.set_ids() beforehand.
"""
if check_permissions_fl and (
self.uid is None or self.gids is None or not isinstance(self.gids, list)
):
if (self.uid is None or self.gids is None or not isinstance(self.gids, list)):
raise ValueError("uid and gid not set properly.")

if not isinstance(path, pth.Path):
raise ValueError("No valid path object was given.")

if not path.exists():
# Can't access or stat something that doesn't exist
return False
elif check_permissions_fl:
check_path = False
else:
# If no stat result is passed through then get our own
if stat_result is None:
stat_result = path.stat()
return check_permissions(
check_path = check_permissions(
self.uid, self.gids, access=access, stat_result=stat_result
)
else:
return True
return check_path
5 changes: 0 additions & 5 deletions nlds/templates/server_config.j2
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
"print_tracebacks_fl": "{{ index_q_print_tracebacks_fl }}",
"filelist_max_length": "{{ index_q_filelist_max_length }}",
"message_threshold": "{{ index_q_message_threshold }}",
"check_permissions_fl": "{{ index_q_check_permissions_fl }}",
"check_filesize_fl": "{{ index_q_check_filesize_fl }}",
"max_filesize": "{{ index_q_max_filesize }}"
},
Expand Down Expand Up @@ -82,15 +81,13 @@
"logging": "{{ transfer_put_q_logging_dict }}",
"print_tracebacks_fl": "{{ transfer_put_q_print_tracebacks_fl }}",
"filelist_max_length": "{{ transfer_put_q_filelist_max_length }}",
"check_permissions_fl": "{{ transfer_put_q_check_permissions_fl }}",
"tenancy": "{{ transfer_put_q_tenancy }}",
"require_secure_fl": "{{ transfer_put_q_require_secure_fl }}"
},
"transfer_get_q": {
"logging": "{{ transfer_put_q_logging_dict }}",
"print_tracebacks_fl": "{{ transfer_put_q_print_tracebacks_fl }}",
"filelist_max_length": "{{ transfer_put_q_filelist_max_length }}",
"check_permissions_fl": "{{ transfer_put_q_check_permissions_fl }}",
"tenancy": "{{ transfer_put_q_tenancy }}",
"require_secure_fl": "{{ transfer_put_q_require_secure_fl }}",
"chown_fl": "{{ transfer_put_q_chown_fl }}",
Expand All @@ -104,7 +101,6 @@
"logging": "{{ archive_put_q_logging_dict }}",
"print_tracebacks_fl": "{{ archive_put_q_print_tracebacks_fl }}",
"tenancy": "{{ archive_put_q_tenancy }}",
"check_permissions_fl": "{{ archive_put_q_check_permissions_fl }}",
"require_secure_fl": "{{ archive_put_q_require_secure_fl }}",
"tape_url": "{{ archive_put_q_tape_url }}",
"tape_pool": "{{ archive_put_q_tape_pool }}",
Expand All @@ -115,7 +111,6 @@
"logging": "{{ archive_get_q_logging_dict }}",
"print_tracebacks_fl": "{{ archive_get_q_print_tracebacks_fl }}",
"tenancy": "{{ archive_get_q_tenancy }}",
"check_permissions_fl": "{{ archive_get_q_check_permissions_fl }}",
"require_secure_fl": "{{ archive_get_q_require_secure_fl }}",
"tape_url": "{{ archive_get_q_tape_url }}",
"tape_pool": "{{ archive_get_q_tape_pool }}",
Expand Down
5 changes: 2 additions & 3 deletions nlds_processors/archiver/archive_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,8 @@ def callback(self, ch, method, properties, body, connection):

# Set uid and gid from message contents if configured to check
# permissions
if self.check_permissions_fl:
self.log("Check permissions flag is set, setting uid and gids now.")
self.set_ids(body_dict)
self.log("Setting uid and gids now.", RK.LOG_INFOx)
self.set_ids(body_dict)

try:
access_key, secret_key, tenancy = self.get_objectstore_config(body_dict)
Expand Down
7 changes: 5 additions & 2 deletions nlds_processors/catalog/catalog_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ def _catalog_update(self, body: Dict, rk_origin: str) -> None:
self.completelist,
routing_key=rk_complete,
body_json=body,
state=State.CATALOG_UPDATE,
state=State.CATALOG_UPDATING,
)
# FAILED
if len(self.failedlist) > 0:
Expand Down Expand Up @@ -607,6 +607,7 @@ def _catalog_get(self, body: Dict, rk_origin: str) -> None:
# reset the lists
self.completelist.clear()
self.failedlist.clear()
self.tapelist.clear()

# start the database transactions, reset lists and
self.catalog.start_session()
Expand Down Expand Up @@ -678,7 +679,9 @@ def _catalog_get(self, body: Dict, rk_origin: str) -> None:

# NEED RETRIEVING FROM TAPE
if len(self.tapelist) > 0:
raise NotImplementedError
for f in self.tapelist:
print(f.original_path, f.tape_name)
#raise NotImplementedError

# NRM - TODO - sort out the logic here
# FAILED
Expand Down
15 changes: 5 additions & 10 deletions nlds_processors/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,13 @@ class IndexerConsumer(StattingConsumer):
_FILELIST_MAX_LENGTH = "filelist_max_length"
_MESSAGE_MAX_SIZE = "message_threshold"
_PRINT_TRACEBACKS = "print_tracebacks_fl"
_CHECK_PERMISSIONS = "check_permissions_fl"
_CHECK_FILESIZE = "check_filesize_fl"
_MAX_FILESIZE = "max_filesize"

DEFAULT_CONSUMER_CONFIG = {
_FILELIST_MAX_LENGTH: 1000,
_MESSAGE_MAX_SIZE: 16 * 1000 * 1000, # in kB
_PRINT_TRACEBACKS: False,
_CHECK_PERMISSIONS: True,
_CHECK_FILESIZE: True,
_MAX_FILESIZE: (500 * 1000 * 1000), # in kB, default=500GB
}
Expand All @@ -54,7 +52,6 @@ def __init__(self, queue=DEFAULT_QUEUE_NAME):
self.filelist_max_len = self.load_config_value(self._FILELIST_MAX_LENGTH)
self.message_max_size = self.load_config_value(self._MESSAGE_MAX_SIZE)
self.print_tracebacks_fl = self.load_config_value(self._PRINT_TRACEBACKS)
self.check_permissions_fl = self.load_config_value(self._CHECK_PERMISSIONS)
self.check_filesize_fl = self.load_config_value(self._CHECK_FILESIZE)
self.max_filesize = self.load_config_value(self._MAX_FILESIZE)

Expand Down Expand Up @@ -97,18 +94,16 @@ def _scan(
rk_parts: List[str],
body_json: Dict[str, Any],
) -> None:
# First change user and group so file permissions can be
# checked. This should be deactivated when testing locally.
if self.check_permissions_fl:
self.set_ids(body_json)
# First change user and group so file permissions can be checked
self.set_ids(body_json)

# Append routing info and then run the index
body_json = self.append_route_info(body_json)
self.log("Starting index scan", RK.LOG_INFO)

# Index the entirety of the passed filelist and check for
# permissions. The size of the packet will also be evaluated
# and used to send lists of roughly equal size.
# Index the entirety of the passed filelist and check for permissions. The size
# of the packet will also be evaluated and used to send lists of roughly equal
# size.
self.index(filelist, rk_parts[0], body_json)
self.log(f"Scan finished.", RK.LOG_INFO)

Expand Down
13 changes: 3 additions & 10 deletions nlds_processors/transferers/base_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,13 @@ class BaseTransferConsumer(StattingConsumer, ABC):

_TENANCY = "tenancy"
_REQUIRE_SECURE = "require_secure_fl"
_CHECK_PERMISSIONS = "check_permissions_fl"
_PRINT_TRACEBACKS = "print_tracebacks_fl"
_MAX_RETRIES = "max_retries"
_FILELIST_MAX_LENGTH = "filelist_max_length"
_REMOVE_ROOT_SLASH = "remove_root_slash_fl"
DEFAULT_CONSUMER_CONFIG = {
_TENANCY: None,
_REQUIRE_SECURE: True,
_CHECK_PERMISSIONS: True,
_PRINT_TRACEBACKS: False,
_MAX_RETRIES: 5,
_FILELIST_MAX_LENGTH: 1000,
Expand All @@ -49,7 +47,6 @@ def __init__(self, queue=DEFAULT_QUEUE_NAME):

self.tenancy = self.load_config_value(self._TENANCY)
self.require_secure_fl = self.load_config_value(self._REQUIRE_SECURE)
self.check_permissions_fl = self.load_config_value(self._CHECK_PERMISSIONS)
self.print_tracebacks_fl = self.load_config_value(self._PRINT_TRACEBACKS)
self.max_retries = self.load_config_value(self._MAX_RETRIES)
self.filelist_max_len = self.load_config_value(self._FILELIST_MAX_LENGTH)
Expand Down Expand Up @@ -101,10 +98,8 @@ def callback(self, ch, method, properties, body, connection):
self.log("Objectstore config unobtainable, exiting callback.", RK.LOG_ERROR)
return

# Set uid and gid from message contents if configured to check
# permissions
if self.check_permissions_fl:
self.set_ids(body_json)
# Set uid and gid from message contents
self.set_ids(body_json)

# Append route info to message to track the route of the message
body_json = self.append_route_info(body_json)
Expand Down Expand Up @@ -173,9 +168,7 @@ def get_objectstore_config(self, body_dict) -> Tuple:
def check_path_access(
self, path: pth.Path, stat_result: NamedTuple = None, access: int = os.R_OK
) -> bool:
return super().check_path_access(
path, stat_result, access, self.check_permissions_fl
)
return super().check_path_access(path, stat_result, access)

@abstractmethod
def transfer(
Expand Down
6 changes: 3 additions & 3 deletions nlds_processors/transferers/get_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,13 @@ def _transfer_files(
# If bucketname inserted into object path (i.e. from catalogue) then
# extract both
try:
bucket_name, object_name = (
self._get_and_check_bucket_name_object_name(path_details)
)
self.log(
f"Attempting to get file {object_name} from {bucket_name}",
RK.LOG_DEBUG,
)
bucket_name, object_name = (
self._get_and_check_bucket_name_object_name(path_details)
)
download_path = self._get_download_path(path_details, target_path)
self._transfer(bucket_name, object_name, download_path)
except TransferError as e:
Expand Down
2 changes: 1 addition & 1 deletion nlds_processors/transferers/put_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def _transfer_files(

# If check_permissions active then check again that file exists and
# is accessible.
if self.check_permissions_fl and not self.check_path_access(item_path):
if not self.check_path_access(item_path):
reason = f"Path:{path_details.path} is inaccessible."
self.log(reason, RK.LOG_DEBUG)
path_details.failure_reason = reason
Expand Down
7 changes: 0 additions & 7 deletions tests/nlds_processors_test/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ def test_check_path_access(monkeypatch, default_indexer):
# Point to non-existent test file
p = Path("test.py")

print(default_indexer.check_permissions_fl)
# Should fail with uninitialised uid and gid
with pytest.raises(ValueError):
default_indexer.check_path_access(p, access=os.R_OK)
Expand All @@ -155,9 +154,6 @@ def test_check_path_access(monkeypatch, default_indexer):
default_indexer.gids = [100]
assert default_indexer.check_path_access(p, access=os.R_OK) == False

# Set permissions checking to false for now
default_indexer.check_permissions_fl = False

monkeypatch.setattr(Path, "exists", lambda _: True)
mp_exists = Path("test.py")
assert default_indexer.check_path_access(mp_exists) == True
Expand All @@ -166,9 +162,6 @@ def test_check_path_access(monkeypatch, default_indexer):
mp_no_exists = Path("test.py")
assert default_indexer.check_path_access(mp_no_exists) == False

# Now we test if checking permissions will work
default_indexer.check_permissions_fl = True

# Will need a mock stat result
StatResult = namedtuple("StatResult", "st_mode st_uid st_gid")
sr = StatResult(int(0o100400), 0, 0)
Expand Down

0 comments on commit 3e3bf9a

Please sign in to comment.