From 6ee427ad5a26d376b8c0d7dc800bc09b848299a9 Mon Sep 17 00:00:00 2001 From: d33bs Date: Mon, 27 Jul 2026 16:44:11 -0600 Subject: [PATCH] new collaboration reconfig and enhancements --- .env.example | 6 + .gitignore | 3 + README.md | 87 ++++++++++++-- config/omero/scan_dirs.yml | 7 ++ config/omero/users.example.yml | 25 +++++ config/omero/users.yml | 15 --- docs/src/operations.md | 55 ++++++++- scripts/import_scan.py | 200 +++++++++++++++++++++++++++++++-- scripts/scan_dirs.py | 80 ++++++++++--- scripts/show_access_url.py | 3 + scripts/sync_users.py | 100 +++++++++++++---- scripts/validate.py | 2 +- tests/test_scripts.py | 198 +++++++++++++++++++++++++++++++- 13 files changed, 707 insertions(+), 74 deletions(-) create mode 100644 config/omero/users.example.yml delete mode 100644 config/omero/users.yml diff --git a/.env.example b/.env.example index 5df4d21..2ac231e 100644 --- a/.env.example +++ b/.env.example @@ -2,9 +2,15 @@ POSTGRES_DB=omero POSTGRES_USER=omero POSTGRES_PASSWORD=omero-change-me OMERO_ROOT_PASSWORD=omero-root-change-me +OMERO_USER_HABOMERO_PASSWORD=habomero-change-me +OMERO_USER_TEST_PASSWORD=test-change-me +OMERO_USER_WAY_MCKINSEY_PASSWORD=way-mckinsey-change-me OMERO_SERVER_TAG=latest OMERO_WEB_TAG=latest OMERO_WEB_PORT=4080 +# Optional LAN hostname printed by `uv run poe show-url`. +# For mDNS on Linux, use habomero.local after configuring the host with Avahi. +OMERO_PUBLIC_HOSTNAME=habomero.local # OMERO server session timing (milliseconds) # Default 1 hour inactivity timeout; allow up to 24h idle requests. OMERO_SESSIONS_TIMEOUT_MS=3600000 diff --git a/.gitignore b/.gitignore index aba79b4..4f3ff3e 100644 --- a/.gitignore +++ b/.gitignore @@ -217,3 +217,6 @@ __marimo__/ # Streamlit .streamlit/secrets.toml + +# Local habomero configuration with credentials +config/omero/users.yml diff --git a/README.md b/README.md index d74a253..1fd9962 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ flowchart TD D --> E[Quick health check] E --> F[Open OMERO.web] - G[scan_dirs.yml + users.yml] --> D + G[scan_dirs.yml + local users.yml] --> D H[Source image directories] --> D I[Browser at /webclient] --> F ``` @@ -36,26 +36,41 @@ flowchart TD uv sync --group dev ``` -3. Create environment file: +3. Create local configuration files from templates: ```bash cp .env.example .env +cp config/omero/users.example.yml config/omero/users.yml ``` -4. Configure scan directories (project-relative only): +4. Set local secrets and user access: + +```bash +vim .env +vim config/omero/users.yml +``` + +`config/omero/users.yml` is ignored by Git. Keep real OMERO user passwords in +`.env` and reference them from `users.yml` with `password_env`. + +5. Configure and mount scan directories: ```bash vim config/omero/scan_dirs.yml uv run poe scan-dirs ``` -5. Prepare local directories: +Every path in `scan_directories` must exist locally before `scan-dirs` or +startup tasks run. For SMB shares, mount them at the local paths configured in +`config/omero/scan_dirs.yml`. + +6. Prepare local directories: ```bash uv run poe provision ``` -6. Start stack: +7. Start stack: ```bash uv run poe up @@ -63,14 +78,13 @@ uv run poe up `poe up` prints both localhost and local-network URLs for copy/paste sharing. -7. Configure user access allowlist: +8. Sync the configured OMERO users and groups: ```bash -vim config/omero/users.yml uv run poe sync-users ``` -8. Validate and inspect: +9. Validate and inspect: ```bash uv run poe validate @@ -79,9 +93,53 @@ uv run poe logs ``` OMERO.web is exposed at `http://localhost:${OMERO_WEB_PORT:-4080}`. +For a Linux server on a local network, see [LAN hostname setup](#lan-hostname-setup) +to make the service reachable as `habomero.local` or `habomero`. All `poe` operations are restricted to project-root execution and fail if run from another directory. +## LAN hostname setup + +The repo can print a hostname URL via `OMERO_PUBLIC_HOSTNAME`, but the hostname +itself has to be provided by the Linux host or your network. + +For mDNS on a Linux server, which gives most macOS/Linux clients +`http://habomero.local:${OMERO_WEB_PORT:-4080}/webclient/`: + +```bash +sudo hostnamectl set-hostname habomero +sudo apt-get update +sudo apt-get install -y avahi-daemon +sudo systemctl enable --now avahi-daemon +``` + +Allow OMERO.web and mDNS through the host firewall if one is enabled: + +```bash +sudo ufw allow 4080/tcp +sudo ufw allow 5353/udp +``` + +Then set this in the server's local `.env`: + +```bash +OMERO_PUBLIC_HOSTNAME=habomero.local +``` + +For the shorter `http://habomero:${OMERO_WEB_PORT:-4080}/webclient/`, configure +your router/DHCP DNS to resolve `habomero` to the server's LAN IP, or add a +hosts-file entry on each client: + +```text +192.168.1.50 habomero +``` + +After DNS or mDNS is configured, run: + +```bash +uv run poe show-url +``` + ## Operations - One-command local run: @@ -95,10 +153,19 @@ scan directory (`OMERO_SCAN_DIR` -> `/scan/inbox`) using the first user in `config/omero/users.yml`. Imports mirror directory hierarchy in OMERO Folders (folders map to folders, images map to images). If `shared_group` is set in `config/omero/scan_dirs.yml`, -all configured users are joined to that group and imported content is visible -to all group members. Set `import_mode: inplace` in +configured users are joined to that group by default and imported content is +visible to all group members. Set `join_shared_group: false` on restricted +users that should not see shared content. Set `import_mode: inplace` in `config/omero/scan_dirs.yml` to avoid duplicate storage by importing file references instead of copying pixel data. +`config/omero/users.yml` is local-only and ignored by Git; commit changes to +`config/omero/users.example.yml` instead, and use `password_env` entries with +real password values in `.env`. +Scan roots may also be configured as mappings with `path` and `group`; imports +for that root run in the configured OMERO group instead of the global shared +group. Set `delete_omero_missing_files: true` to delete tracked OMERO Images +when their source files are no longer present after a successful source-root +scan. This only deletes OMERO records, not source files. - Production-style full-dataset parallel ingest with periodic rescan: diff --git a/config/omero/scan_dirs.yml b/config/omero/scan_dirs.yml index 68041d9..ee73c43 100644 --- a/config/omero/scan_dirs.yml +++ b/config/omero/scan_dirs.yml @@ -2,6 +2,8 @@ allow_external_paths: true scan_directories: - ~/mnt/bandicoot/RxRx19a - ~/mnt/bandicoot/CFReT_subtyping_data + - path: ~/mnt/Way_McKinsey_Cardiac_Fibrosis + group: way_mckinsey_cardiac_fibrosis # Optional shared OMERO group for imports. All configured users are joined # to this group by sync-users, and imports use this group context. @@ -18,6 +20,11 @@ omero_folder_root: scan-root # - inplace: keep data at source path and import by reference (no duplication) import_mode: inplace +# When true, imported OMERO Images are deleted if their source files are no +# longer found during a successful scan of the configured source root. Source +# files are never deleted by this cleanup. +delete_omero_missing_files: true + # Safety controls for high-latency/remote sources: # Maximum number of files to attempt in a single import-scan run. # Set to 0 for no cap. diff --git a/config/omero/users.example.yml b/config/omero/users.example.yml new file mode 100644 index 0000000..81e83f0 --- /dev/null +++ b/config/omero/users.example.yml @@ -0,0 +1,25 @@ +users: + - username: habomero + first_name: Habomero + last_name: Service + group: lab + extra_groups: + - way_mckinsey_cardiac_fibrosis + email: habomero@example.org + institution: Local Lab + password_env: OMERO_USER_HABOMERO_PASSWORD + - username: test + first_name: Test + last_name: User + group: lab + email: test@example.org + institution: Local Lab + password_env: OMERO_USER_TEST_PASSWORD + - username: way_mckinsey + first_name: Way McKinsey + last_name: Cardiac Fibrosis + group: way_mckinsey_cardiac_fibrosis + join_shared_group: false + email: way_mckinsey@example.org + institution: Local Lab + password_env: OMERO_USER_WAY_MCKINSEY_PASSWORD diff --git a/config/omero/users.yml b/config/omero/users.yml deleted file mode 100644 index 27b1224..0000000 --- a/config/omero/users.yml +++ /dev/null @@ -1,15 +0,0 @@ -users: - - username: habomero - first_name: Habomero - last_name: Service - group: lab - email: habomero@example.org - institution: Local Lab - password: habomero - - username: test - first_name: Test - last_name: User - group: lab - email: test@example.org - institution: Local Lab - password: test diff --git a/docs/src/operations.md b/docs/src/operations.md index fe81bda..c9babdc 100644 --- a/docs/src/operations.md +++ b/docs/src/operations.md @@ -4,6 +4,10 @@ ```bash cp .env.example .env +cp config/omero/users.example.yml config/omero/users.yml +vim .env +vim config/omero/users.yml +vim config/omero/scan_dirs.yml uv run poe preflight uv run poe scan-dirs uv run poe provision @@ -11,6 +15,11 @@ uv run poe up uv run poe sync-users ``` +`config/omero/users.yml` is local-only and ignored by Git. Store real OMERO +user passwords in `.env` and reference them with `password_env` entries. Every +configured scan directory must exist locally before `scan-dirs` runs; mount SMB +shares at the paths listed in `config/omero/scan_dirs.yml`. + ## Configure scan directories Edit `config/omero/scan_dirs.yml` and keep entries project-relative (for example: `data/inbox`). @@ -30,6 +39,43 @@ uv run poe logs All `poe` tasks must be run from the project root directory. `up` and the main `remote-run*` tasks also run `preflight` automatically. +## LAN hostname setup + +The Docker stack exposes OMERO.web on the Linux host port configured by +`OMERO_WEB_PORT`. To make a LAN URL such as `habomero.local` work, configure +hostname resolution on the host or network. + +For mDNS on a Linux server: + +```bash +sudo hostnamectl set-hostname habomero +sudo apt-get update +sudo apt-get install -y avahi-daemon +sudo systemctl enable --now avahi-daemon +sudo ufw allow 4080/tcp +sudo ufw allow 5353/udp +``` + +Set the hostname printed by habomero in `.env`: + +```bash +OMERO_PUBLIC_HOSTNAME=habomero.local +``` + +Most macOS/Linux clients can then use +`http://habomero.local:${OMERO_WEB_PORT:-4080}/webclient/`. For bare +`habomero`, configure router/DHCP DNS or add a hosts-file entry on each client: + +```text +192.168.1.50 habomero +``` + +Confirm the URLs: + +```bash +uv run poe show-url +``` + ## Safe restart without deleting data Use this when recovering an existing OMERO stack after an unclean shutdown or @@ -77,13 +123,20 @@ IMPORT_WORKERS=4 uv run poe import-remote-safe-continuous-parallel-full ## Configure user access allowlist -Edit `config/omero/users.yml` and define approved user accounts. +Edit the local-only `config/omero/users.yml` and define approved user accounts. +Use `password_env` entries and set the real password values in `.env`. Then synchronize those users into OMERO: ```bash uv run poe sync-users ``` +Users are joined to `shared_group` by default when it is configured in +`config/omero/scan_dirs.yml`. Set `join_shared_group: false` for a restricted +account, and use `extra_groups` for service/import users that need access to +per-root import groups. A scan directory entry may be a mapping with `path` and +`group` to import that root into a separate OMERO group. + ## Backup ```bash diff --git a/scripts/import_scan.py b/scripts/import_scan.py index 712cf90..0d26cdb 100644 --- a/scripts/import_scan.py +++ b/scripts/import_scan.py @@ -15,6 +15,7 @@ PROJECT_ROOT = Path(__file__).resolve().parent.parent USERS_CONFIG_PATH = PROJECT_ROOT / "config/omero/users.yml" SCAN_CONFIG_PATH = PROJECT_ROOT / "config/omero/scan_dirs.yml" +ENV_PATH = PROJECT_ROOT / ".env" SCAN_ROOTS_STATE_PATH = PROJECT_ROOT / "data/state/scan_roots.yml" IMPORT_STATE_PATH = PROJECT_ROOT / "data/state/imported_files.txt" DATASET_STATE_PATH = PROJECT_ROOT / "data/state/path_datasets.yml" @@ -49,6 +50,37 @@ class ImportConfigError(ValueError): """Raised for invalid import configuration.""" +def read_env_var(name: str) -> str: + """Read a required variable from process env or local .env file.""" + + if value := os.getenv(name): + return value + + if ENV_PATH.exists(): + for line in ENV_PATH.read_text(encoding="utf-8").splitlines(): + if not line or line.startswith("#") or "=" not in line: + continue + key, raw = line.split("=", maxsplit=1) + if key.strip() == name: + return raw.strip() + + raise ImportConfigError(f"Missing required environment variable: {name}") + + +def user_password(item: dict[str, object]) -> str: + """Read an OMERO user password from a literal or configured env var.""" + + password = item.get("password") + if isinstance(password, str) and password.strip(): + return password.strip() + + password_env = item.get("password_env") + if isinstance(password_env, str) and password_env.strip(): + return read_env_var(password_env.strip()) + + raise ImportConfigError("Each user needs password or password_env") + + def positive_int(payload: dict[str, object], key: str, default: int) -> int: """Read a positive integer from config with fallback.""" @@ -97,17 +129,16 @@ def load_user_credentials() -> tuple[dict[str, str], str]: if not isinstance(item, dict): raise ImportConfigError("Each user entry must be a mapping") username = str(item.get("username", "")).strip() - password = str(item.get("password", "")).strip() - if not username or not password: - raise ImportConfigError("Each user needs username and password") - credentials[username] = password + if not username: + raise ImportConfigError("Each user needs username") + credentials[username] = user_password(item) if index == 0: first_username = username return credentials, first_username def load_import_config( # noqa: C901, PLR0912, PLR0915 -) -> tuple[str, str, str, int, int, int, int, int, int, int, int]: +) -> tuple[str, str, str, int, int, int, int, int, int, int, int, bool]: shared_group = "" root_prefix = "scan-root" import_mode = "copy" @@ -119,6 +150,7 @@ def load_import_config( # noqa: C901, PLR0912, PLR0915 scan_progress_every_paths = SCAN_PROGRESS_EVERY_PATHS import_progress_every_files = IMPORT_PROGRESS_EVERY_FILES import_workers = IMPORT_WORKERS + delete_omero_missing_files = False if SCAN_CONFIG_PATH.exists(): payload = yaml.safe_load(SCAN_CONFIG_PATH.read_text(encoding="utf-8")) or {} group = payload.get("shared_group") @@ -164,6 +196,12 @@ def load_import_config( # noqa: C901, PLR0912, PLR0915 import_progress_every_files, ) import_workers = positive_int(payload, "import_workers", import_workers) + delete_missing_value = payload.get( + "delete_omero_missing_files", + payload.get("delete_missing_files"), + ) + if isinstance(delete_missing_value, bool): + delete_omero_missing_files = delete_missing_value env_cap = os.environ.get("IMPORT_MAX_FILES_PER_RUN", "").strip() if env_cap: try: @@ -236,6 +274,7 @@ def load_import_config( # noqa: C901, PLR0912, PLR0915 scan_progress_every_paths, import_progress_every_files, import_workers, + delete_omero_missing_files, ) @@ -294,8 +333,11 @@ def load_scan_roots() -> dict[str, dict[str, str]]: if isinstance(key, str) and isinstance(value, dict): source = value.get("source") container_root = value.get("container_root") + group = value.get("group") if isinstance(source, str) and isinstance(container_root, str): result[key] = {"source": source, "container_root": container_root} + if isinstance(group, str) and group.strip(): + result[key]["group"] = group.strip() return result @@ -604,6 +646,117 @@ def load_dataset_image_names( return names +def hql_string(value: str) -> str: + """Quote a string literal for the simple HQL queries used by this script.""" + + return "'" + value.replace("'", "''") + "'" + + +def load_dataset_image_ids_by_name( + owner: str, + owner_password: str, + shared_group: str, + dataset_id: int, + image_name: str, +) -> list[int]: + """Load image IDs in a dataset matching an imported source filename.""" + + cmd = ( + "omero hql " + f'"select i.id from Image i join i.datasetLinks l ' + f'where l.parent.id = {dataset_id} and i.name = {hql_string(image_name)}"' + ) + result = run_as_user_with_retry(owner, owner_password, cmd, shared_group) + if result.returncode != 0: + return [] + + ids: list[int] = [] + for match in re.finditer(r"\[(\d+)\]", result.stdout): + ids.append(int(match.group(1))) + return ids + + +def delete_image( + owner: str, + owner_password: str, + shared_group: str, + image_id: int, +) -> bool: + """Delete a single OMERO image and wait for deletion completion.""" + + result = run_as_user_with_retry( + owner, + owner_password, + f"omero delete Image:{image_id} -w --no-wait", + shared_group, + ) + return result.returncode == 0 + + +def delete_missing_imports( # noqa: PLR0913 + owner: str, + owner_password: str, + shared_group: str, + root_key: str, + container_root: str, + root_files: set[str], + imported: set[str], + dataset_state: dict[str, int], +) -> int: + """Remove OMERO images whose source files disappeared from a scanned root.""" + + deleted = 0 + root_prefix = f"{root_key}:" + stale_tracking_keys = sorted( + key + for key in imported + if ( + key.startswith(root_prefix) + and key.removeprefix(root_prefix) not in root_files + ) + ) + if not stale_tracking_keys: + return 0 + + print(f"[cleanup] stale tracked files={len(stale_tracking_keys)} for {root_key}") + for tracking_key in stale_tracking_keys: + abs_path = tracking_key.removeprefix(root_prefix) + rel_path = rel_path_from_root(abs_path, container_root) + rel_dir = dataset_key_for_rel_path(rel_path) + dataset_id = dataset_state.get(f"{root_key}|{rel_dir}") + if dataset_id is None: + imported.remove(tracking_key) + deleted += 1 + continue + + image_ids = load_dataset_image_ids_by_name( + owner, + owner_password, + shared_group, + dataset_id, + Path(abs_path).name, + ) + if not image_ids: + imported.remove(tracking_key) + deleted += 1 + print(f"[cleanup-missing] {rel_path}: no OMERO image found") + continue + + deleted_all = True + for image_id in image_ids: + if delete_image(owner, owner_password, shared_group, image_id): + print(f"[cleanup-deleted] {rel_path} -> Image:{image_id}") + else: + print(f"[cleanup-failed] {rel_path} -> Image:{image_id}") + deleted_all = False + if deleted_all: + imported.remove(tracking_key) + deleted += 1 + if deleted: + save_string_set(IMPORT_STATE_PATH, imported) + return deleted + + def build_import_command(import_mode: str, dataset_id: int, abs_path: str) -> str: """Build OMERO import command for configured transfer mode.""" @@ -747,6 +900,7 @@ def import_root_files( # noqa: PLR0913, C901, PLR0912, PLR0915 scan_progress_every_paths: int, import_progress_every_files: int, import_workers: int, + delete_omero_missing_files: bool, ) -> tuple[int, dict[str, str], bool, dict[str, int]]: """Import files for a single root. Returns count, failures, hit_cap.""" @@ -754,6 +908,7 @@ def import_root_files( # noqa: PLR0913, C901, PLR0912, PLR0915 budget_capped = budget_remaining > 0 skipped_existing_count = 0 skipped_tracked_count = 0 + deleted_missing_count = 0 failures: dict[str, str] = {} dataset_image_cache: dict[int, set[str]] = {} root_files: list[str] | None = None @@ -783,12 +938,25 @@ def import_root_files( # noqa: PLR0913, C901, PLR0912, PLR0915 "queued": 0, "skipped_tracked": 0, "skipped_existing": 0, + "deleted_missing": 0, }, ) print( f"[root] {source_root}: candidate files={len(root_files)} " f"budget={budget_remaining if budget_capped else 'uncapped'}" ) + root_file_set = set(root_files) + if delete_omero_missing_files: + deleted_missing_count = delete_missing_imports( + owner, + owner_password, + shared_group, + root_key, + container_root, + root_file_set, + imported, + dataset_state, + ) # Build worklist serially so dataset/project bookkeeping stays consistent. work: list[tuple[str, int]] = [] @@ -804,6 +972,7 @@ def import_root_files( # noqa: PLR0913, C901, PLR0912, PLR0915 "queued": len(work), "skipped_tracked": skipped_tracked_count, "skipped_existing": skipped_existing_count, + "deleted_missing": deleted_missing_count, }, ) tracking_key = f"{root_key}:{abs_path}" @@ -872,6 +1041,7 @@ def import_root_files( # noqa: PLR0913, C901, PLR0912, PLR0915 "queued": len(work), "skipped_tracked": skipped_tracked_count, "skipped_existing": skipped_existing_count, + "deleted_missing": deleted_missing_count, }, ) continue @@ -895,6 +1065,7 @@ def import_root_files( # noqa: PLR0913, C901, PLR0912, PLR0915 "queued": len(work), "skipped_tracked": skipped_tracked_count, "skipped_existing": skipped_existing_count, + "deleted_missing": deleted_missing_count, }, ) @@ -935,6 +1106,7 @@ def import_root_files( # noqa: PLR0913, C901, PLR0912, PLR0915 "queued": len(work), "skipped_tracked": skipped_tracked_count, "skipped_existing": skipped_existing_count, + "deleted_missing": deleted_missing_count, }, ) continue @@ -956,6 +1128,7 @@ def import_root_files( # noqa: PLR0913, C901, PLR0912, PLR0915 "queued": len(work), "skipped_tracked": skipped_tracked_count, "skipped_existing": skipped_existing_count, + "deleted_missing": deleted_missing_count, }, ) @@ -975,6 +1148,7 @@ def import_files() -> None: # noqa: PLR0915 scan_progress_every_paths, import_progress_every_files, import_workers, + delete_omero_missing_files, ) = load_import_config() wait_for_db_stable(db_stable_checks, db_stable_interval) @@ -995,18 +1169,22 @@ def import_files() -> None: # noqa: PLR0915 total_queued = 0 total_skipped_tracked = 0 total_skipped_existing = 0 + total_deleted_missing = 0 interrupted = False try: for root_key, root_data in sorted(roots.items()): source = root_data["source"] container_root = root_data["container_root"] + root_group = root_data.get("group", shared_group) print(f"[root-begin] {root_key} source={source}") print(f"[root-begin] {root_key} container_root={container_root}") + if root_group: + print(f"[root-begin] {root_key} group={root_group}") try: project_id = get_or_create_project( owner, owner_password, - shared_group, + root_group, root_key, root_prefix, source, @@ -1022,7 +1200,7 @@ def import_files() -> None: # noqa: PLR0915 root_imported, root_failures, hit_cap, root_stats = import_root_files( owner, owner_password, - shared_group, + root_group, import_mode, root_key, source, @@ -1036,6 +1214,7 @@ def import_files() -> None: # noqa: PLR0915 scan_progress_every_paths, import_progress_every_files, import_workers, + delete_omero_missing_files, ) imported_count += root_imported failures.update(root_failures) @@ -1043,13 +1222,15 @@ def import_files() -> None: # noqa: PLR0915 total_queued += root_stats["queued"] total_skipped_tracked += root_stats["skipped_tracked"] total_skipped_existing += root_stats["skipped_existing"] + total_deleted_missing += root_stats["deleted_missing"] print( f"[root-end] {root_key} imported_this_root={root_imported} " f"failures_this_root={len(root_failures)} " f"candidates={root_stats['candidates']} " f"queued={root_stats['queued']} " f"skipped_tracked={root_stats['skipped_tracked']} " - f"skipped_existing={root_stats['skipped_existing']}" + f"skipped_existing={root_stats['skipped_existing']} " + f"deleted_missing={root_stats['deleted_missing']}" ) # Checkpoint state per root so restart can't replay completed root work. save_string_set(IMPORT_STATE_PATH, imported) @@ -1072,7 +1253,8 @@ def import_files() -> None: # noqa: PLR0915 f"candidates={total_candidates} queued={total_queued} " f"imported_new={imported_count} skipped_tracked={total_skipped_tracked} " f"skipped_existing={total_skipped_existing} failures={len(failures)} " - f"tracked_before={imported_before} tracked_after={imported_after}" + f"deleted_missing={total_deleted_missing} tracked_before={imported_before} " + f"tracked_after={imported_after}" ) if interrupted: return diff --git a/scripts/scan_dirs.py b/scripts/scan_dirs.py index f080bcd..f39cd3b 100644 --- a/scripts/scan_dirs.py +++ b/scripts/scan_dirs.py @@ -13,10 +13,11 @@ CONFIG_PATH = PROJECT_ROOT / "config/omero/scan_dirs.yml" STATE_PATH = PROJECT_ROOT / "data/state/scan_roots.yml" COMPOSE_OVERRIDE_PATH = PROJECT_ROOT / "data/state/scan_roots.compose.yml" +RESERVED_GROUPS = {"user", "guest", "system"} -def load_scan_directories() -> list[Path]: - """Load configured scan directories as resolved absolute paths.""" +def load_scan_directory_entries() -> list[dict[str, str]]: # noqa: C901, PLR0912 + """Load configured scan directories with optional per-root metadata.""" if not CONFIG_PATH.exists(): raise FileNotFoundError(f"Scan directory config is missing: {CONFIG_PATH}") @@ -27,12 +28,33 @@ def load_scan_directories() -> list[Path]: if not isinstance(raw_dirs, list): raise TypeError("scan_directories must be a YAML list") - resolved: list[Path] = [] + resolved: list[dict[str, str]] = [] for item in raw_dirs: - if not isinstance(item, str) or not item.strip(): - raise ValueError("scan_directories entries must be non-empty strings") + group = "" + if isinstance(item, str): + raw_path = item + elif isinstance(item, dict): + raw_path = item.get("path") + raw_group = item.get("group") + if raw_group is not None: + if not isinstance(raw_group, str) or not raw_group.strip(): + raise ValueError( + "scan_directories group entries must be non-empty strings" + ) + group = raw_group.strip() + if group in RESERVED_GROUPS: + raise ValueError( + "scan_directories group entries must be non-reserved " + "data groups (not one of: user, guest, system)" + ) + else: + raise ValueError( + "scan_directories entries must be non-empty strings or mappings" + ) + if not isinstance(raw_path, str) or not raw_path.strip(): + raise ValueError("scan_directories path entries must be non-empty strings") - entry_path = Path(item).expanduser() + entry_path = Path(raw_path).expanduser() target = ( entry_path.resolve() if entry_path.is_absolute() @@ -40,23 +62,39 @@ def load_scan_directories() -> list[Path]: ) if not allow_external and PROJECT_ROOT not in [target, *target.parents]: - raise ValueError(f"Path escapes project root: {item}") + raise ValueError(f"Path escapes project root: {raw_path}") if not target.exists() or not target.is_dir(): raise FileNotFoundError(f"Scan directory does not exist: {target}") - resolved.append(target) + row = {"path": str(target)} + if group: + row["group"] = group + resolved.append(row) # De-duplicate and collapse nested paths so one file tree is scanned once. - unique_paths = sorted(set(resolved), key=lambda p: (len(p.parts), str(p))) - collapsed: list[Path] = [] - for candidate in unique_paths: - if any(parent in [candidate, *candidate.parents] for parent in collapsed): + unique_entries = { + Path(row["path"]): row for row in sorted(resolved, key=lambda r: r["path"]) + } + sorted_entries = sorted( + unique_entries.items(), key=lambda item: (len(item[0].parts), str(item[0])) + ) + collapsed: list[dict[str, str]] = [] + collapsed_paths: list[Path] = [] + for candidate, row in sorted_entries: + if any(parent in [candidate, *candidate.parents] for parent in collapsed_paths): print(f"skip-overlap: {candidate}") continue - collapsed.append(candidate) + collapsed.append(row) + collapsed_paths.append(candidate) return collapsed +def load_scan_directories() -> list[Path]: + """Load configured scan directories as resolved absolute paths.""" + + return [Path(row["path"]) for row in load_scan_directory_entries()] + + def root_key(path: Path) -> str: """Generate a stable short key for a source root path.""" @@ -64,13 +102,21 @@ def root_key(path: Path) -> str: return f"root_{digest}" -def materialize_scan_roots(paths: list[Path]) -> dict[str, dict[str, str]]: +def materialize_scan_roots( + paths: list[Path] | list[dict[str, str]], +) -> dict[str, dict[str, str]]: """Build a root mapping and compose override for direct bind mounts.""" mapping: dict[str, dict[str, str]] = {} volumes: list[str] = [] - for source in paths: + for item in paths: + if isinstance(item, Path): + source = item + group = "" + else: + source = Path(item["path"]) + group = item.get("group", "") key = root_key(source) container_root = f"/scan/roots/{key}" @@ -78,6 +124,8 @@ def materialize_scan_roots(paths: list[Path]) -> dict[str, dict[str, str]]: "source": str(source), "container_root": container_root, } + if group: + mapping[key]["group"] = group volumes.append(f"{source}:{container_root}:ro") try: @@ -125,7 +173,7 @@ def materialize_scan_roots(paths: list[Path]) -> dict[str, dict[str, str]]: def main() -> None: """Load, validate, and materialize scan roots for container mounts.""" - directories = load_scan_directories() + directories = load_scan_directory_entries() mapping = materialize_scan_roots(directories) for key, data in sorted(mapping.items()): print(f"{key}: {data['source']}") diff --git a/scripts/show_access_url.py b/scripts/show_access_url.py index 7f41fce..c67acfb 100644 --- a/scripts/show_access_url.py +++ b/scripts/show_access_url.py @@ -44,9 +44,12 @@ def main() -> None: """Emit access URLs for OMERO.web.""" port = read_env_var("OMERO_WEB_PORT", "4080") + public_hostname = read_env_var("OMERO_PUBLIC_HOSTNAME", "").strip() host_ip = get_local_ip() print(f"OMERO.web local: http://localhost:{port}/webclient/") print(f"OMERO.web network: http://{host_ip}:{port}/webclient/") + if public_hostname: + print(f"OMERO.web hostname: http://{public_hostname}:{port}/webclient/") if __name__ == "__main__": diff --git a/scripts/sync_users.py b/scripts/sync_users.py index 272cb63..b781edb 100644 --- a/scripts/sync_users.py +++ b/scripts/sync_users.py @@ -264,21 +264,21 @@ def ensure_default_group( ) -def ensure_user(root_password: str, user: dict[str, str]) -> None: +def ensure_user(root_password: str, user: dict[str, object]) -> None: """Create user if missing and ensure group membership is present.""" - username = user["username"] - group = user["group"] + username = str(user["username"]) + group = str(user["group"]) ensure_group(root_password, group) if not user_exists(root_password, username): username_q = shlex.quote(username) - first_q = shlex.quote(user["first_name"]) - last_q = shlex.quote(user["last_name"]) + first_q = shlex.quote(str(user["first_name"])) + last_q = shlex.quote(str(user["last_name"])) group_q = shlex.quote(group) - email_q = shlex.quote(user["email"]) - institution_q = shlex.quote(user["institution"]) - password_q = shlex.quote(user.get("password", "changeme123")) + email_q = shlex.quote(str(user["email"])) + institution_q = shlex.quote(str(user["institution"])) + password_q = shlex.quote(str(user.get("password", "changeme123"))) add_cmd = ( "omero user add " f"{username_q} " @@ -298,7 +298,7 @@ def ensure_user(root_password: str, user: dict[str, str]) -> None: else: print(f"exists: {username}") - password = user.get("password", "") + password = str(user.get("password", "")) if password and not user_exists(root_password, username): passwd_commands = [ f"omero user password --user-name {username} {password}", @@ -317,9 +317,27 @@ def ensure_user(root_password: str, user: dict[str, str]) -> None: print(f"password-unchanged: {username} ({last_error})") ensure_user_group_membership(root_password, username, group) + for extra_group in user.get("extra_groups", []): + extra_group_name = str(extra_group) + ensure_group(root_password, extra_group_name) + ensure_user_group_membership(root_password, username, extra_group_name) -def load_users() -> list[dict[str, str]]: # noqa: C901 +def resolve_user_password(item: dict[str, object]) -> str | None: + """Read an optional OMERO user password from config or an env var.""" + + password = item.get("password") + if isinstance(password, str) and password.strip(): + return password.strip() + + password_env = item.get("password_env") + if isinstance(password_env, str) and password_env.strip(): + return read_env_var(password_env.strip()) + + return None + + +def load_users() -> list[dict[str, object]]: # noqa: C901, PLR0912, PLR0915 """Load and validate user allowlist entries from YAML.""" if not CONFIG_PATH.exists(): @@ -339,7 +357,7 @@ def load_users() -> list[dict[str, str]]: # noqa: C901 "institution", } - normalized: list[dict[str, str]] = [] + normalized: list[dict[str, object]] = [] for item in users: if not isinstance(item, dict): raise UserConfigError("Each user entry must be a mapping") @@ -348,7 +366,7 @@ def load_users() -> list[dict[str, str]]: # noqa: C901 if missing: raise UserConfigError(f"Missing required user fields: {', '.join(missing)}") - row: dict[str, str] = {} + row: dict[str, object] = {} for key in required: value = item[key] if not isinstance(value, str) or not value.strip(): @@ -360,13 +378,47 @@ def load_users() -> list[dict[str, str]]: # noqa: C901 "(not one of: user, guest, system)" ) - if "password" in item: - password = item["password"] - if not isinstance(password, str) or not password.strip(): + has_password = "password" in item + has_password_env = "password_env" in item + if has_password and has_password_env: + raise UserConfigError( + "User entries must define only one of 'password' or 'password_env'" + ) + if has_password or has_password_env: + try: + password = resolve_user_password(item) + except UserConfigError: + raise + if not password: + raise UserConfigError( + "User entries must define password or password_env" + ) + row["password"] = password + + extra_groups = item.get("extra_groups", []) + if extra_groups is None: + extra_groups = [] + if not isinstance(extra_groups, list): + raise UserConfigError("User field 'extra_groups' must be a YAML list") + normalized_extra_groups: list[str] = [] + for group in extra_groups: + if not isinstance(group, str) or not group.strip(): raise UserConfigError( - "User field 'password' must be a non-empty string" + "User field 'extra_groups' entries must be non-empty strings" ) - row["password"] = password.strip() + normalized_group = group.strip() + if normalized_group in RESERVED_GROUPS: + raise UserConfigError( + "User field 'extra_groups' must contain non-reserved data " + "groups (not one of: user, guest, system)" + ) + normalized_extra_groups.append(normalized_group) + row["extra_groups"] = normalized_extra_groups + + join_shared_group = item.get("join_shared_group", True) + if not isinstance(join_shared_group, bool): + raise UserConfigError("User field 'join_shared_group' must be a boolean") + row["join_shared_group"] = join_shared_group normalized.append(row) @@ -422,10 +474,16 @@ def main() -> None: ensure_group_permissions(root_password, shared_group, shared_group_permissions) for user in users: ensure_user(root_password, user) - ensure_default_group(root_password, user["username"], user["group"]) - if shared_group and user["group"] != shared_group: - ensure_user_group_membership(root_password, user["username"], shared_group) - ensure_default_group(root_password, user["username"], shared_group) + username = str(user["username"]) + primary_group = str(user["group"]) + ensure_default_group(root_password, username, primary_group) + if ( + shared_group + and primary_group != shared_group + and user.get("join_shared_group", True) + ): + ensure_user_group_membership(root_password, username, shared_group) + ensure_default_group(root_password, username, shared_group) if __name__ == "__main__": diff --git a/scripts/validate.py b/scripts/validate.py index e00eaf8..24df509 100644 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -9,7 +9,7 @@ Path("ansible/playbook.yml"), Path(".env.example"), Path("config/omero/scan_dirs.yml"), - Path("config/omero/users.yml"), + Path("config/omero/users.example.yml"), Path("data"), ] diff --git a/tests/test_scripts.py b/tests/test_scripts.py index 5489d58..8f3a5ef 100644 --- a/tests/test_scripts.py +++ b/tests/test_scripts.py @@ -6,7 +6,14 @@ import pytest -from scripts import safe_restart, scan_dirs, show_access_url, validate +from scripts import ( + import_scan, + safe_restart, + scan_dirs, + show_access_url, + sync_users, + validate, +) def test_read_env_var_prefers_environment(monkeypatch: pytest.MonkeyPatch) -> None: @@ -17,6 +24,21 @@ def test_read_env_var_prefers_environment(monkeypatch: pytest.MonkeyPatch) -> No assert value == "9999" +def test_show_access_url_prints_configured_hostname( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """Configured LAN hostnames should appear in copy/paste URLs.""" + + monkeypatch.setenv("OMERO_WEB_PORT", "4080") + monkeypatch.setenv("OMERO_PUBLIC_HOSTNAME", "habomero.local") + monkeypatch.setattr(show_access_url, "get_local_ip", lambda: "192.0.2.10") + + show_access_url.main() + + output = capsys.readouterr().out + assert "http://habomero.local:4080/webclient/" in output + + def test_scan_dirs_rejects_escape_without_external_flag( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -70,6 +92,180 @@ def test_scan_dirs_collapses_overlapping_paths( assert result == [root.resolve()] +def test_scan_dirs_materializes_per_root_group( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Mapping entries can attach an OMERO group to a scan root.""" + + project_root = tmp_path / "project" + source = project_root / "cardiac" + source.mkdir(parents=True) + config_path = project_root / "scan_dirs.yml" + state_path = project_root / "state.yml" + compose_path = project_root / "compose.yml" + config_path.write_text( + "scan_directories:\n" + " - path: cardiac\n" + " group: way_mckinsey_cardiac_fibrosis\n", + encoding="utf-8", + ) + + monkeypatch.setattr(scan_dirs, "PROJECT_ROOT", project_root) + monkeypatch.setattr(scan_dirs, "CONFIG_PATH", config_path) + monkeypatch.setattr(scan_dirs, "STATE_PATH", state_path) + monkeypatch.setattr(scan_dirs, "COMPOSE_OVERRIDE_PATH", compose_path) + + entries = scan_dirs.load_scan_directory_entries() + mapping = scan_dirs.materialize_scan_roots(entries) + + assert entries == [ + { + "path": str(source.resolve()), + "group": "way_mckinsey_cardiac_fibrosis", + } + ] + assert next(iter(mapping.values()))["group"] == "way_mckinsey_cardiac_fibrosis" + + +def test_sync_users_loads_shared_group_opt_out( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Restricted users can avoid joining the global shared import group.""" + + config_path = tmp_path / "users.yml" + config_path.write_text( + "users:\n" + " - username: way_mckinsey\n" + " first_name: Way\n" + " last_name: McKinsey\n" + " group: way_mckinsey_cardiac_fibrosis\n" + " join_shared_group: false\n" + " email: way@example.org\n" + " institution: Local Lab\n" + " password: way_mckinsey\n", + encoding="utf-8", + ) + + monkeypatch.setattr(sync_users, "CONFIG_PATH", config_path) + + users = sync_users.load_users() + + assert users[0]["join_shared_group"] is False + assert users[0]["extra_groups"] == [] + + +def test_sync_users_loads_password_from_env( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """User templates can reference password environment variables.""" + + config_path = tmp_path / "users.yml" + config_path.write_text( + "users:\n" + " - username: habomero\n" + " first_name: Habomero\n" + " last_name: Service\n" + " group: lab\n" + " email: habomero@example.org\n" + " institution: Local Lab\n" + " password_env: HABOMERO_TEST_PASSWORD\n", + encoding="utf-8", + ) + + monkeypatch.setattr(sync_users, "CONFIG_PATH", config_path) + monkeypatch.setenv("HABOMERO_TEST_PASSWORD", "from-env") + + users = sync_users.load_users() + + assert users[0]["password"] == "from-env" + + +def test_import_scan_loads_password_from_env( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Import credentials can come from password_env entries.""" + + config_path = tmp_path / "users.yml" + config_path.write_text( + "users:\n - username: habomero\n password_env: HABOMERO_IMPORT_PASSWORD\n", + encoding="utf-8", + ) + + monkeypatch.setattr(import_scan, "USERS_CONFIG_PATH", config_path) + monkeypatch.setenv("HABOMERO_IMPORT_PASSWORD", "import-password") + + credentials, first_username = import_scan.load_user_credentials() + + assert first_username == "habomero" + assert credentials == {"habomero": "import-password"} + + +def test_delete_missing_imports_removes_deleted_image_tracking( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Stale source files are deleted from OMERO and removed from state.""" + + state_path = tmp_path / "imported_files.txt" + imported = { + "root_a:/scan/roots/root_a/keep.tif", + "root_a:/scan/roots/root_a/missing.tif", + "root_b:/scan/roots/root_b/other.tif", + } + deleted_ids: list[int] = [] + + monkeypatch.setattr(import_scan, "IMPORT_STATE_PATH", state_path) + monkeypatch.setattr( + import_scan, + "load_dataset_image_ids_by_name", + lambda *args: [123], + ) + + def fake_delete_image( + owner: str, + owner_password: str, + shared_group: str, + image_id: int, + ) -> bool: + deleted_ids.append(image_id) + return True + + monkeypatch.setattr(import_scan, "delete_image", fake_delete_image) + + deleted = import_scan.delete_missing_imports( + "habomero", + "habomero", + "lab", + "root_a", + "/scan/roots/root_a", + {"/scan/roots/root_a/keep.tif"}, + imported, + {"root_a|root": 99}, + ) + + assert deleted == 1 + assert deleted_ids == [123] + assert imported == { + "root_a:/scan/roots/root_a/keep.tif", + "root_b:/scan/roots/root_b/other.tif", + } + assert state_path.read_text(encoding="utf-8").splitlines() == sorted(imported) + + +def test_import_config_loads_explicit_omero_delete_flag( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The public cleanup flag names OMERO as the deletion target.""" + + config_path = tmp_path / "scan_dirs.yml" + config_path.write_text("delete_omero_missing_files: true\n", encoding="utf-8") + + monkeypatch.setattr(import_scan, "SCAN_CONFIG_PATH", config_path) + + config = import_scan.load_import_config() + + assert config[-1] is True + + def test_safe_restart_compose_args_include_scan_roots_when_present( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: