Use Hierarchical Machine Config (#711)

* Make all existing MachineConfig keys fully optional
* Added 'instrument_type' as a MachineConfig key, for use in hierarchical config files
* If MachineConfig.rsync_basepath or MachineConfig.default_model is None, default to 'Path().resolve()'
* Renamed 'from_file' to 'machine_config_from_file' for clarity
* Removed 'get_machine_config_for_instrument' from 'session_shared' and replaced it with 'get_machine_config' from 'murfey.util.config' instead
* Updated 'machine_config_from_file' function to parse and connstruct instrument machine configs hierarchically; it can load all the machine configs or just the specified one

Author: tieneupin

src/murfey/server/api/clem.py

@@ -83,7 +83,7 @@ def validate_and_sanitise(
     machine_config = get_machine_config(instrument_name=instrument_name)[
         instrument_name
     ]
-    rsync_basepath = machine_config.rsync_basepath.resolve()
+    rsync_basepath = (machine_config.rsync_basepath or Path("")).resolve()
 
     # Check that full file path doesn't contain unallowed characters
     #   Currently allows only:

src/murfey/server/api/file_io_frontend.py

@@ -14,6 +14,7 @@
     process_gain as _process_gain,
 )
 from murfey.server.murfey_db import murfey_db
+from murfey.util import secure_path
 from murfey.util.config import get_machine_config
 from murfey.util.db import Session
 
@@ -50,10 +51,23 @@ async def create_symlink(
     machine_config = get_machine_config(instrument_name=instrument_name)[
         instrument_name
     ]
-    symlink_full_path = machine_config.rsync_basepath / symlink_params.symlink
+    rsync_basepath = (machine_config.rsync_basepath or Path("")).resolve()
+    symlink_full_path = secure_path(
+        rsync_basepath / symlink_params.symlink, keep_spaces=True
+    )
+    # Verify that the symlink provided does not lead elsewhere
+    if not symlink_full_path.resolve().is_relative_to(rsync_basepath):
+        logger.warning(
+            "Symlink rejected because it will be created in a forbidden location"
+        )
+        return ""
+    # Remove and replace symlink if it exists are 'override' is set
     if symlink_full_path.is_symlink() and symlink_params.override:
         symlink_full_path.unlink()
+    # If a file/folder already exists using the desired symlink name, return empty string
     if symlink_full_path.exists():
         return ""
-    symlink_full_path.symlink_to(machine_config.rsync_basepath / symlink_params.target)
+    symlink_full_path.symlink_to(
+        secure_path(rsync_basepath / symlink_params.target, keep_spaces=True)
+    )
     return str(symlink_params.symlink)

src/murfey/server/api/file_io_instrument.py

@@ -57,7 +57,8 @@ def suggest_path(
         )
 
     # Construct the full path to where the dataset is to be saved
-    check_path = machine_config.rsync_basepath / base_path
+    rsync_basepath = (machine_config.rsync_basepath or Path("")).resolve()
+    check_path = rsync_basepath / base_path
 
     # Check previous year to account for the year rolling over during data collection
     if not check_path.parent.exists():
@@ -69,7 +70,7 @@ def suggest_path(
                 base_path_parts[year_idx] = str(int(part) - 1)
         base_path = "/".join(base_path_parts)
         check_path_prev = check_path
-        check_path = machine_config.rsync_basepath / base_path
+        check_path = rsync_basepath / base_path
 
         # If it's not in the previous year either, it's a genuine error
         if not check_path.parent.exists():
@@ -88,7 +89,7 @@ def suggest_path(
         check_path.mkdir(mode=0o750)
         if params.extra_directory:
             (check_path / secure_filename(params.extra_directory)).mkdir(mode=0o750)
-    return {"suggested_path": check_path.relative_to(machine_config.rsync_basepath)}
+    return {"suggested_path": check_path.relative_to(rsync_basepath)}
 
 
 class Dest(BaseModel):
@@ -107,7 +108,9 @@ def make_rsyncer_destination(session_id: int, destination: Dest, db=murfey_db):
     ]
     if not machine_config:
         raise ValueError("No machine configuration set when making rsyncer destination")
-    full_destination_path = machine_config.rsync_basepath / destination_path
+    full_destination_path = (
+        machine_config.rsync_basepath or Path("")
+    ).resolve() / destination_path
     for parent_path in full_destination_path.parents:
         parent_path.mkdir(mode=0o750, exist_ok=True)
     return destination
@@ -151,7 +154,7 @@ async def write_eer_fractionation_file(
         ) / secure_filename(fractionation_params.fractionation_file_name)
     else:
         file_path = (
-            Path(machine_config.rsync_basepath)
+            (machine_config.rsync_basepath or Path("")).resolve()
             / str(datetime.now().year)
             / secure_filename(visit_name)
             / machine_config.gain_directory_name

src/murfey/server/api/file_io_shared.py

@@ -37,8 +37,9 @@ async def process_gain(
         executables = machine_config.external_executables
     env = machine_config.external_environment
     safe_path_name = secure_filename(gain_reference_params.gain_ref.name)
+    rsync_basepath = machine_config.rsync_basepath or Path("")
     filepath = (
-        Path(machine_config.rsync_basepath)
+        rsync_basepath
         / str(datetime.now().year)
         / secure_filename(visit_name)
         / machine_config.gain_directory_name
@@ -48,7 +49,7 @@ async def process_gain(
     if not filepath.exists():
         filepath_prev = filepath
         filepath = (
-            Path(machine_config.rsync_basepath)
+            rsync_basepath
             / str(datetime.now().year - 1)
             / secure_filename(visit_name)
             / machine_config.gain_directory_name
@@ -80,14 +81,12 @@ async def process_gain(
         )
     if new_gain_ref and new_gain_ref_superres:
         return {
-            "gain_ref": new_gain_ref.relative_to(Path(machine_config.rsync_basepath)),
-            "gain_ref_superres": new_gain_ref_superres.relative_to(
-                Path(machine_config.rsync_basepath)
-            ),
+            "gain_ref": new_gain_ref.relative_to(rsync_basepath),
+            "gain_ref_superres": new_gain_ref_superres.relative_to(rsync_basepath),
         }
     elif new_gain_ref:
         return {
-            "gain_ref": new_gain_ref.relative_to(Path(machine_config.rsync_basepath)),
+            "gain_ref": new_gain_ref.relative_to(rsync_basepath),
             "gain_ref_superres": None,
         }
     else:

src/murfey/server/api/session_control.py

@@ -24,15 +24,14 @@
     get_foil_holes_from_grid_square as _get_foil_holes_from_grid_square,
     get_grid_squares as _get_grid_squares,
     get_grid_squares_from_dcg as _get_grid_squares_from_dcg,
-    get_machine_config_for_instrument,
     get_tiff_file as _get_tiff_file,
     get_upstream_file as _get_upstream_file,
     remove_session_by_id,
 )
 from murfey.server.ispyb import DB as ispyb_db, get_all_ongoing_visits
 from murfey.server.murfey_db import murfey_db
 from murfey.util import sanitise
-from murfey.util.config import MachineConfig
+from murfey.util.config import get_machine_config
 from murfey.util.db import (
     AutoProcProgram,
     ClientEnvironment,
@@ -80,8 +79,8 @@ async def get_current_timestamp():
 
 
 @router.get("/instruments/{instrument_name}/machine")
-def machine_info_by_instrument(instrument_name: str) -> Optional[MachineConfig]:
-    return get_machine_config_for_instrument(instrument_name)
+def machine_info_by_instrument(instrument_name: str):
+    return get_machine_config(instrument_name)[instrument_name]
 
 
 @router.get("/new_client_id/")

src/murfey/server/api/session_info.py

@@ -24,15 +24,14 @@
     get_foil_holes_from_grid_square as _get_foil_holes_from_grid_square,
     get_grid_squares as _get_grid_squares,
     get_grid_squares_from_dcg as _get_grid_squares_from_dcg,
-    get_machine_config_for_instrument,
     get_tiff_file as _get_tiff_file,
     get_upstream_file as _get_upstream_file,
     remove_session_by_id,
 )
 from murfey.server.ispyb import DB as ispyb_db, get_all_ongoing_visits
 from murfey.server.murfey_db import murfey_db
 from murfey.util import sanitise
-from murfey.util.config import MachineConfig
+from murfey.util.config import get_machine_config
 from murfey.util.db import (
     ClassificationFeedbackParameters,
     ClientEnvironment,
@@ -78,8 +77,8 @@ def connections_check():
 @router.get("/instruments/{instrument_name}/machine")
 def machine_info_by_instrument(
     instrument_name: MurfeyInstrumentName,
-) -> Optional[MachineConfig]:
-    return get_machine_config_for_instrument(instrument_name)
+):
+    return get_machine_config(instrument_name)[instrument_name]
 
 
 @router.get("/instruments/{instrument_name}/visits_raw", response_model=List[Visit])

src/murfey/server/api/session_shared.py

@@ -1,15 +1,14 @@
 import logging
-from functools import lru_cache
 from pathlib import Path
-from typing import Dict, List, Optional
+from typing import Dict, List
 
 from sqlmodel import select
 from sqlmodel.orm.session import Session as SQLModelSession
 from werkzeug.utils import secure_filename
 
 import murfey.server.prometheus as prom
 from murfey.util import safe_run, sanitise, secure_path
-from murfey.util.config import MachineConfig, from_file, get_machine_config, settings
+from murfey.util.config import get_machine_config
 from murfey.util.db import (
     DataCollection,
     DataCollectionGroup,
@@ -23,15 +22,6 @@
 logger = logging.getLogger("murfey.server.api.shared")
 
 
-@lru_cache(maxsize=5)
-def get_machine_config_for_instrument(instrument_name: str) -> Optional[MachineConfig]:
-    if settings.murfey_machine_configuration:
-        return from_file(Path(settings.murfey_machine_configuration), instrument_name)[
-            instrument_name
-        ]
-    return None
-
-
 def remove_session_by_id(session_id: int, db):
     session = db.exec(select(MurfeySession).where(MurfeySession.id == session_id)).one()
     sessions_for_visit = db.exec(

src/murfey/server/api/workflow.py

@@ -245,15 +245,14 @@ def start_dc(
     machine_config = get_machine_config(instrument_name=instrument_name)[
         instrument_name
     ]
+    rsync_basepath = (machine_config.rsync_basepath or Path("")).resolve()
     logger.info(
         f"Starting data collection on microscope {instrument_name!r} "
-        f"with basepath {sanitise(str(machine_config.rsync_basepath))} and directory {sanitise(dc_params.image_directory)}"
+        f"with basepath {sanitise(str(rsync_basepath))} and directory {sanitise(dc_params.image_directory)}"
     )
     dc_parameters = {
         "visit": visit_name,
-        "image_directory": str(
-            machine_config.rsync_basepath / dc_params.image_directory
-        ),
+        "image_directory": str(rsync_basepath / dc_params.image_directory),
         "start_time": str(datetime.now()),
         "voltage": dc_params.voltage,
         "pixel_size": str(float(dc_params.pixel_size_on_image) * 1e9),
@@ -744,7 +743,10 @@ async def request_tomography_preprocessing(
                 "fm_dose": proc_file.dose_per_frame,
                 "frame_count": proc_file.frame_count,
                 "gain_ref": (
-                    str(machine_config.rsync_basepath / proc_file.gain_ref)
+                    str(
+                        (machine_config.rsync_basepath or Path("")).resolve()
+                        / proc_file.gain_ref
+                    )
                     if proc_file.gain_ref and machine_config.data_transfer_enabled
                     else proc_file.gain_ref
                 ),
@@ -1060,7 +1062,7 @@ async def make_gif(
         instrument_name
     ]
     output_dir = (
-        Path(machine_config.rsync_basepath)
+        (machine_config.rsync_basepath or Path("")).resolve()
         / secure_filename(year)
         / secure_filename(visit_name)
         / "processed"

src/murfey/server/demo_api.py

@@ -43,8 +43,8 @@
 from murfey.util import sanitise_path
 from murfey.util.config import (
     MachineConfig,
-    from_file,
     get_hostname,
+    machine_config_from_file,
     security_from_file,
 )
 from murfey.util.db import (
@@ -93,7 +93,9 @@ class Settings(BaseSettings):
 machine_config: dict[str, MachineConfig] = {}
 if settings.murfey_machine_configuration:
     microscope = get_microscope()
-    machine_config = from_file(Path(settings.murfey_machine_configuration), microscope)
+    machine_config = machine_config_from_file(
+        Path(settings.murfey_machine_configuration), microscope
+    )
 
 
 # This will be the homepage for a given microscope.
@@ -114,19 +116,19 @@ async def root(request: Request):
 def machine_info() -> Optional[MachineConfig]:
     instrument_name = os.getenv("BEAMLINE")
     if settings.murfey_machine_configuration and instrument_name:
-        return from_file(Path(settings.murfey_machine_configuration), instrument_name)[
-            instrument_name
-        ]
+        return machine_config_from_file(
+            Path(settings.murfey_machine_configuration), instrument_name
+        )[instrument_name]
     return None
 
 
 @lru_cache(maxsize=5)
 @router.get("/instruments/{instrument_name}/machine")
 def machine_info_by_name(instrument_name: str) -> Optional[MachineConfig]:
     if settings.murfey_machine_configuration:
-        return from_file(Path(settings.murfey_machine_configuration), instrument_name)[
-            instrument_name
-        ]
+        return machine_config_from_file(
+            Path(settings.murfey_machine_configuration), instrument_name
+        )[instrument_name]
     return None
 
 

src/murfey/server/feedback.py

@@ -1100,7 +1100,9 @@ def _register_class_selection(message: dict, _db, demo: bool = False):
 def _find_initial_model(visit: str, machine_config: MachineConfig) -> Path | None:
     if machine_config.initial_model_search_directory:
         visit_directory = (
-            machine_config.rsync_basepath / str(datetime.now().year) / visit
+            (machine_config.rsync_basepath or Path("")).resolve()
+            / str(datetime.now().year)
+            / visit
         )
         possible_models = [
             p
@@ -1512,7 +1514,10 @@ def _flush_tomography_preprocessing(message: dict, _db):
                 "fm_dose": proc_params.dose_per_frame,
                 "frame_count": proc_params.frame_count,
                 "gain_ref": (
-                    str(machine_config.rsync_basepath / proc_params.gain_ref)
+                    str(
+                        (machine_config.rsync_basepath or Path("")).resolve()
+                        / proc_params.gain_ref
+                    )
                     if proc_params.gain_ref
                     else proc_params.gain_ref
                 ),
@@ -2042,7 +2047,10 @@ def feedback_callback(header: dict, message: dict, _db=murfey_db) -> None:
                     angpix=float(message["pixel_size_on_image"]) * 1e10,
                     dose_per_frame=message["dose_per_frame"],
                     gain_ref=(
-                        str(machine_config.rsync_basepath / message["gain_ref"])
+                        str(
+                            (machine_config.rsync_basepath or Path("")).resolve()
+                            / message["gain_ref"]
+                        )
                         if message["gain_ref"] and machine_config.data_transfer_enabled
                         else message["gain_ref"]
                     ),