From 794c2798cfb28f848cebef2899d72ad788c85207 Mon Sep 17 00:00:00 2001 From: "Roger J. A. Duthie" <343584+rjaduthie@users.noreply.github.com> Date: Wed, 8 Apr 2026 10:35:35 +0100 Subject: [PATCH 01/12] Revise README for installation and usage instructions Updated installation instructions and added unmount section. --- README.md | 94 ++++++++++++---------------- SERVER-CONFIGURATION.md | 131 +++++++++++++++++++--------------------- 2 files changed, 100 insertions(+), 125 deletions(-) diff --git a/README.md b/README.md index b6ca895..cc06741 100644 --- a/README.md +++ b/README.md @@ -1,86 +1,70 @@ # Path Finder -A Rust implementation of the SKA path finder tool for authentication, locating & mounting data from the SKA storage system within a Slurm login host. +A Rust implementation of the SKA path finder tool for authentication, locating & mounting data from the SKA storage system within a Slurm login host. It provides a single binary and an RPM installer. -## System Requirements +Features: -For instructions on the setup & requirements for your HPC Server side environment see [pathFinder - Server Configuration](./SERVER-CONFIGURATION.md) +- OAuth2 device code flow authentication +- Data location lookup via Data Management API +- Site capabilities verification via Site Capabilities API +- Secure data mounting with proper permissions +## Installation +For instructions on the setup & requirements for your HPC Server environment, see the [server configuration doc](./SERVER-CONFIGURATION.md) ## Usage -**Note**: The tool will automatically check if the file exists locally at the local RSE `/skadata`. If the file is not found locally, it will display the sites where the file is available and prompt you to ensure the data has been staged to your local site before mounting. - ### Mount Data -The `pathFinder` command is available to run on the CLI after logging into the Slurm login node. The command needs to be run as `sudo` because it is mounting data, users in the `pathfinder` group are granted `sudo` privileges to execute the `pathFinder` executable. - -#### Usage +The `pathFinder` command is available to run on the CLI after logging into the Slurm login node. The command needs to be run as `sudo` because it is mounting data, users in the `pathfinder` group are granted `sudo` privileges to execute the `pathFinder` executable. -``` -$ sudo pathFinder --help + $ sudo pathFinder --help -A tool for finding SKA data paths for mounting purposes + A tool for finding SKA data paths for mounting purposes -Usage: pathFinder [OPTIONS] --namespace --file-name + Usage: pathFinder [OPTIONS] --namespace --file-name -Options: - --namespace Namespace of the data - --file-name Name of the data file - --no-login Do not use OAuth2 for authentication - use environment variables instead - --unmount Unmount previously mounted data instead of mounting - -h, --help Print help -``` + Options: + --namespace Namespace of the data + --file-name Name of the data file + --no-login Do not use OAuth2 for authentication - use environment variables instead + --unmount Unmount previously mounted data instead of mounting + -h, --help Print help #### OAUTH Authentication -Example using SKAIAM OAuth2 (required). +Example using SKAIAM OAuth2: -``` -$ sudo pathFinder --namespace daac --file-name simple_file.txt + $ sudo pathFinder --namespace daac --file-name simple_file.txt -Authenticating with OAuth2... -Cached tokens expired + Authenticating with OAuth2... + Cached tokens expired -ACTION REQUIRED: - Open this URL in a browser and authenticate: https://ska-iam.stfc.ac.uk/device?user_code=KNIBUH + ACTION REQUIRED: + Open this URL in a browser and authenticate: https://ska-iam.stfc.ac.uk/device?user_code=KNIBUH -Waiting for authentication (timeout: 5 minutes)... -Tokens cached for 3600 seconds -Authentication successful! -RSE Path for file 'simple_file.txt' in namespace 'daac': /daac/14/66/simple_file.txt -Mount verification successful: simple_file.txt is mounted at /home/sm2921/projects/daac/simple_file.txt -``` + Waiting for authentication (timeout: 5 minutes)... + Tokens cached for 3600 seconds + Authentication successful! + RSE Path for file 'simple_file.txt' in namespace 'daac': /daac/14/66/simple_file.txt + Mount verification successful: simple_file.txt is mounted at /home/sm2921/projects/daac/simple_file.txt -#### Token Authentication +#### Authentication using environment variables -Example with environment variables (for automation): +Example with environment variables (e.g. for automation): -``` -$ export DATA_MANAGEMENT_ACCESS_TOKEN="your_token_here" -$ export SITE_CAPABILITIES_ACCESS_TOKEN="your_token_here" -$ sudo pathFinder --namespace daac --file-name simple_file.txt -``` + export DATA_MANAGEMENT_ACCESS_TOKEN="your_token_here" + export SITE_CAPABILITIES_ACCESS_TOKEN="your_token_here" + sudo pathFinder --namespace daac --file-name simple_file.txt --no-login #### Unmounting Data -Example for unmounting a file. - -``` -$ sudo pathFinder --namespace daac --file-name simple_file.txt --unmount -Unmounted simple_file.txt from /home/sm2921/projects/daac/simple_file.txt -``` - -#### RPM Package - -The binary and RPM are built and published at [pathFinder GitHub release](https://github.com/uksrc/pathFinder/releases). - -Check your current release with : - -``` -dnf info pathFinder -``` +Example for unmounting a file: + $ sudo pathFinder --namespace daac --file-name simple_file.txt --unmount + Unmounted simple_file.txt from /home/sm2921/projects/daac/simple_file.txt +## Development +Notes on development can be found in the [development doc](DEVELOPMENT.md). diff --git a/SERVER-CONFIGURATION.md b/SERVER-CONFIGURATION.md index 85cc6b9..40c68df 100644 --- a/SERVER-CONFIGURATION.md +++ b/SERVER-CONFIGURATION.md @@ -1,4 +1,4 @@ -# pathFinder - Server Configuration # +# pathFinder - Server Configuration pathFinder is a tool for mounting SKA data on Slurm clusters without copying the data locally. @@ -8,96 +8,87 @@ Two methods are planned, interactive and a workflow managed by the Science Gatew This documentation covers the prerequisites to setup on the underlying configuration on a Slurm cluster using a Ceph file-system and the installation of the pathFinder tool. -## Pre-requisites ## +## Pre-requisites -The following requirements must be met. +The following requirements must be met. (Note these are for Rocky 9.x releases and have not been tested on RHEL 10.x or Ubuntu) - - CRB Enabled + - CRB Enabled - RHEL EPEL (Extra Packages) - BindFS - Ceph Common -## Server Side Configuration ## +## Server Side Configuration The configuration is only required on the Login node of your Slurm cluster, this assumes that all your user home directories are CephFS/NFS mount points. If you already have EPEL enabled you can skip the next 2 steps. -1. Enable CRB - -``` -crb status -crb enable -``` - -2. Install EPEL -``` -sudo dnf install epel-release -sudo dnf repolist -``` - -3. Configure your Ceph Keyring - -``` -vi /etc/ceph/ceph.client.rucio_prod_ro.keyring -``` -Add your Access key. -``` -[client.rucio_prod_ro] -key = **************************** -``` - -4. Create mountpoints, this MUST be owned by root with permissions of 550 & 700. -``` -sudo mkdir /skadata /mnt/private_mounts/skadata -sudo chmod 550 /skadata -sudo chmod 700 /mnt/private_mounts/skadata -``` - -5. Add the following entries to the **/etc/fstab** file. The `/mnt/private_mounts` is used to hide the owner & group of the mounted file-system, so all files under `/skadata` are presented as `root root` for owner and group and hides the real owner **uid/gid** which would typically be the xrootd, Webdav & Storm user uid/gid. -``` -# Ceph mount -10.4.200.9:6789,10.4.200.13:6789,10.4.200.13:6789,10.4.200.17:6789,10.4.200.25:6789,10.4.200.26:6789:/volumes/_nogroup/a8af40e8-6412-44da-ad08-3731fdf19258/4945e5c2-aab7-4416-9b75-666f2af512d7 /skadata ceph name=rucio_prod_ro,x-systemd.device-timeout=30,x-systemd.mount-timeout=30,noatime,_netdev,ro,nodev,nosuid 0 2 -# Bindfs mount -/mnt/private_mounts/skadata /skadata fuse.bindfs force-user=root,force-group=root 0 0 -``` +1. Enable CRB: + + crb status + crb enable + +2. Install EPEL: + + sudo dnf install epel-release + sudo dnf repolist + +3. Configure your Ceph Keyring: + + vi /etc/ceph/ceph.client.rucio_prod_ro.keyring + +Add your Access key: + + [client.rucio_prod_ro] + key = **************************** + +4. Create mountpoints, this MUST be owned by root with permissions of 550 & 700: + + sudo mkdir /skadata /mnt/private_mounts/skadata + sudo chmod 550 /skadata + sudo chmod 700 /mnt/private_mounts/skadata + +5. Add the following entries to the **/etc/fstab** file. The `/mnt/private_mounts` is used to hide the owner & group of the mounted file-system, so all files under `/skadata` are presented as `root root` for owner and group and hides the real owner **uid/gid** which would typically be the xrootd, Webdav & Storm user uid/gid: + + # Ceph mount + 10.4.200.9:6789,10.4.200.13:6789,10.4.200.13:6789,10.4.200.17:6789,10.4.200.25:6789,10.4.200.26:6789:/volumes/_nogroup/a8af40e8-6412-44da-ad08-3731fdf19258/4945e5c2-aab7-4416-9b75-666f2af512d7 /skadata ceph name=rucio_prod_ro,x-systemd.device-timeout=30,x-systemd.mount-timeout=30,noatime,_netdev,ro,nodev,nosuid 0 2 + # Bindfs mount + /mnt/private_mounts/skadata /skadata fuse.bindfs force-user=root,force-group=root 0 0 6. Mount the /skadata mountpoints. -Note that we use bindfs here as well so all files under `/skadata` are presented as `root root` for owner and group and hides the real owner **uid/gid** which would typically be the xrootd, Webdav & Storm user uid/gid. -``` -mount -a -systemctl daemon-reload -``` +Note that we use bindfs here as well so all files under `/skadata` are presented as `root root` for owner and group and hides the real owner **uid/gid** which would typically be the xrootd, Webdav & Storm user uid/gid: + + mount -a + systemctl daemon-reload 7. Add a sudoers file to control access to the pathfinder tool. -``` -vi /etc/sudoers.d/pathFinder -``` -Using group `pathfinder` for group access for users you want to give access to. -``` -%pathfinder ALL = NOPASSWD: /usr/bin/pathfinder, /usr/bin/pathFinder -``` - -8. Add the patfinder group. -``` -groupadd pathfinder -``` - -9. Add or update the local users to their corresponding group. -``` -usermod -a -G pathfinder sm2921 -``` + + vi /etc/sudoers.d/pathFinder + +Using group `pathfinder` for group access for users you want to give access to: + + %pathfinder ALL = NOPASSWD: /usr/bin/pathfinder, /usr/bin/pathFinder + +8. Add the patfinder group: + + groupadd pathfinder + +9. Add or update the local users to their corresponding group: + + usermod -a -G pathfinder sm2921 ## pathFinder Package installation The latest version is published at [pathFinder Release](https://github.com/uksrc/pathFinder/releases) check before installing or upgrading. -Set the version and install the pathFinder package. +Set the version and install the pathFinder package: + + VERSION=1.x.x + dnf upgrade https://github.com/uksrc/pathFinder/releases/download/v1.0.0/pathfinder-${VERSION}-1.x86_64.rpm + +## pathFinder RPM package -``` -VERSION=1.x.x -dnf upgrade https://github.com/uksrc/pathFinder/releases/download/v1.0.0/pathfinder-${VERSION}-1.x86_64.rpm -``` +The RPM package for pathFinder is published as a part of a [GitHub release](https://github.com/uksrc/pathFinder/releases). From 5b4bf17110655125d62f8b0b0002254c73710d88 Mon Sep 17 00:00:00 2001 From: "Roger J. A. Duthie" <343584+rjaduthie@users.noreply.github.com> Date: Wed, 8 Apr 2026 10:35:35 +0100 Subject: [PATCH 02/12] Fix code indents and do another pass over the docs Updated installation instructions and added unmount section. --- README.md | 56 +++++++++++++------------- SERVER-CONFIGURATION.md | 88 ++++++++++++++++++++--------------------- 2 files changed, 71 insertions(+), 73 deletions(-) diff --git a/README.md b/README.md index cc06741..4c35ddd 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,8 @@ # Path Finder -A Rust implementation of the SKA path finder tool for authentication, locating & mounting data from the SKA storage system within a Slurm login host. It provides a single binary and an RPM installer. +**pathFinder** is a tool for mounting SKA data on Slurm clusters without copying the data locally. Currently, it is provided as a single binary or an RPM installer. It is important to prepare the Slurm cluster ahead of its use (see [Installation](#installation)). + +The tool allows the Scientist to specify which files, identified from the Science Gateway, they want to mount while keeping the files secure and owned by them. Two methods are planned: interactive and a workflow managed by the Science Gateway via prepareData. Features: @@ -11,7 +13,7 @@ Features: ## Installation -For instructions on the setup & requirements for your HPC Server environment, see the [server configuration doc](./SERVER-CONFIGURATION.md) +For instructions on the setup & requirements for your HPC Server environment, see the [server configuration](./SERVER-CONFIGURATION.md) doc. ## Usage @@ -19,52 +21,52 @@ For instructions on the setup & requirements for your HPC Server environment, se The `pathFinder` command is available to run on the CLI after logging into the Slurm login node. The command needs to be run as `sudo` because it is mounting data, users in the `pathfinder` group are granted `sudo` privileges to execute the `pathFinder` executable. - $ sudo pathFinder --help + $ sudo pathFinder --help - A tool for finding SKA data paths for mounting purposes + A tool for finding SKA data paths for mounting purposes - Usage: pathFinder [OPTIONS] --namespace --file-name + Usage: pathFinder [OPTIONS] --namespace --file-name - Options: - --namespace Namespace of the data - --file-name Name of the data file - --no-login Do not use OAuth2 for authentication - use environment variables instead - --unmount Unmount previously mounted data instead of mounting - -h, --help Print help + Options: + --namespace Namespace of the data + --file-name Name of the data file + --no-login Do not use OAuth2 for authentication - use environment variables instead + --unmount Unmount previously mounted data instead of mounting + -h, --help Print help #### OAUTH Authentication Example using SKAIAM OAuth2: - $ sudo pathFinder --namespace daac --file-name simple_file.txt + $ sudo pathFinder --namespace daac --file-name simple_file.txt - Authenticating with OAuth2... - Cached tokens expired + Authenticating with OAuth2... + Cached tokens expired - ACTION REQUIRED: - Open this URL in a browser and authenticate: https://ska-iam.stfc.ac.uk/device?user_code=KNIBUH + ACTION REQUIRED: + Open this URL in a browser and authenticate: https://ska-iam.stfc.ac.uk/device?user_code=KNIBUH - Waiting for authentication (timeout: 5 minutes)... - Tokens cached for 3600 seconds - Authentication successful! - RSE Path for file 'simple_file.txt' in namespace 'daac': /daac/14/66/simple_file.txt - Mount verification successful: simple_file.txt is mounted at /home/sm2921/projects/daac/simple_file.txt + Waiting for authentication (timeout: 5 minutes)... + Tokens cached for 3600 seconds + Authentication successful! + RSE Path for file 'simple_file.txt' in namespace 'daac': /daac/14/66/simple_file.txt + Mount verification successful: simple_file.txt is mounted at /home/sm2921/projects/daac/simple_file.txt #### Authentication using environment variables Example with environment variables (e.g. for automation): - export DATA_MANAGEMENT_ACCESS_TOKEN="your_token_here" - export SITE_CAPABILITIES_ACCESS_TOKEN="your_token_here" - sudo pathFinder --namespace daac --file-name simple_file.txt --no-login + export DATA_MANAGEMENT_ACCESS_TOKEN="your_token_here" + export SITE_CAPABILITIES_ACCESS_TOKEN="your_token_here" + sudo pathFinder --namespace daac --file-name simple_file.txt --no-login #### Unmounting Data Example for unmounting a file: - $ sudo pathFinder --namespace daac --file-name simple_file.txt --unmount - Unmounted simple_file.txt from /home/sm2921/projects/daac/simple_file.txt + $ sudo pathFinder --namespace daac --file-name simple_file.txt --unmount + Unmounted simple_file.txt from /home/sm2921/projects/daac/simple_file.txt ## Development -Notes on development can be found in the [development doc](DEVELOPMENT.md). +Notes on development can be found in the [development](DEVELOPMENT.md) doc. diff --git a/SERVER-CONFIGURATION.md b/SERVER-CONFIGURATION.md index 40c68df..9799f46 100644 --- a/SERVER-CONFIGURATION.md +++ b/SERVER-CONFIGURATION.md @@ -1,25 +1,19 @@ -# pathFinder - Server Configuration +# Server Configuration -pathFinder is a tool for mounting SKA data on Slurm clusters without copying the data locally. - -It allows the Scientist to specify which files, identified from the Science Gateway, they want to mount while keeping the files secure and owned by them. - -Two methods are planned, interactive and a workflow managed by the Science Gateway via prepareData. - -This documentation covers the prerequisites to setup on the underlying configuration on a Slurm cluster using a Ceph file-system and the installation of the pathFinder tool. +This document covers the prerequisites to setup on the underlying configuration on a Slurm cluster using a Ceph file-system and the installation of the pathFinder tool. ## Pre-requisites -The following requirements must be met. +The following requirements must be met: -(Note these are for Rocky 9.x releases and have not been tested on RHEL 10.x or Ubuntu) +- CRB Enabled +- RHEL EPEL (Extra Packages) +- BindFS +- Ceph Common - - CRB Enabled - - RHEL EPEL (Extra Packages) - - BindFS - - Ceph Common +Note: These are for Rocky 9.x releases and have not been tested on RHEL 10.x or Ubuntu. -## Server Side Configuration +## Server-Side Configuration The configuration is only required on the Login node of your Slurm cluster, this assumes that all your user home directories are CephFS/NFS mount points. @@ -27,68 +21,70 @@ If you already have EPEL enabled you can skip the next 2 steps. 1. Enable CRB: - crb status - crb enable + crb status + crb enable 2. Install EPEL: - sudo dnf install epel-release - sudo dnf repolist + sudo dnf install epel-release + sudo dnf repolist 3. Configure your Ceph Keyring: - vi /etc/ceph/ceph.client.rucio_prod_ro.keyring + vi /etc/ceph/ceph.client.rucio_prod_ro.keyring -Add your Access key: + ... and add your Access key: - [client.rucio_prod_ro] - key = **************************** + [client.rucio_prod_ro] + key = **************************** 4. Create mountpoints, this MUST be owned by root with permissions of 550 & 700: - sudo mkdir /skadata /mnt/private_mounts/skadata - sudo chmod 550 /skadata - sudo chmod 700 /mnt/private_mounts/skadata + sudo mkdir /skadata /mnt/private_mounts/skadata + sudo chmod 550 /skadata + sudo chmod 700 /mnt/private_mounts/skadata + +5. Add the following entries to the **/etc/fstab** file: + + # Ceph mount + 10.4.200.9:6789,10.4.200.13:6789,10.4.200.13:6789,10.4.200.17:6789,10.4.200.25:6789,10.4.200.26:6789:/volumes/_nogroup/a8af40e8-6412-44da-ad08-3731fdf19258/4945e5c2-aab7-4416-9b75-666f2af512d7 /skadata ceph name=rucio_prod_ro,x-systemd.device-timeout=30,x-systemd.mount-timeout=30,noatime,_netdev,ro,nodev,nosuid 0 2 -5. Add the following entries to the **/etc/fstab** file. The `/mnt/private_mounts` is used to hide the owner & group of the mounted file-system, so all files under `/skadata` are presented as `root root` for owner and group and hides the real owner **uid/gid** which would typically be the xrootd, Webdav & Storm user uid/gid: + # Bindfs mount + /mnt/private_mounts/skadata /skadata fuse.bindfs force-user=root,force-group=root 0 0 - # Ceph mount - 10.4.200.9:6789,10.4.200.13:6789,10.4.200.13:6789,10.4.200.17:6789,10.4.200.25:6789,10.4.200.26:6789:/volumes/_nogroup/a8af40e8-6412-44da-ad08-3731fdf19258/4945e5c2-aab7-4416-9b75-666f2af512d7 /skadata ceph name=rucio_prod_ro,x-systemd.device-timeout=30,x-systemd.mount-timeout=30,noatime,_netdev,ro,nodev,nosuid 0 2 - # Bindfs mount - /mnt/private_mounts/skadata /skadata fuse.bindfs force-user=root,force-group=root 0 0 + **Note**: The `/mnt/private_mounts` is used to hide the owner & group of the mounted file-system, so all files under `/skadata` are presented as `root root` for owner and group and hides the real owner **uid/gid** which would typically be the xrootd, Webdav & Storm user uid/gid. -6. Mount the /skadata mountpoints. +6. Mount the /skadata mountpoints: -Note that we use bindfs here as well so all files under `/skadata` are presented as `root root` for owner and group and hides the real owner **uid/gid** which would typically be the xrootd, Webdav & Storm user uid/gid: + mount -a + systemctl daemon-reload - mount -a - systemctl daemon-reload + **Note**: We use bindfs here as well so all files under `/skadata` are presented as `root root` for owner and group and hides the real owner **uid/gid** which would typically be the xrootd, Webdav & Storm user uid/gid: -7. Add a sudoers file to control access to the pathfinder tool. +7. Add a sudoers file to control access to the pathfinder tool: - vi /etc/sudoers.d/pathFinder + vi /etc/sudoers.d/pathFinder -Using group `pathfinder` for group access for users you want to give access to: + ...allow group `pathfinder` access to the `pathFinder` executable: - %pathfinder ALL = NOPASSWD: /usr/bin/pathfinder, /usr/bin/pathFinder + %pathfinder ALL = NOPASSWD: /usr/bin/pathfinder, /usr/bin/pathFinder 8. Add the patfinder group: - groupadd pathfinder + groupadd pathfinder 9. Add or update the local users to their corresponding group: - usermod -a -G pathfinder sm2921 + usermod -a -G pathfinder sm2921 -## pathFinder Package installation +## Package installation The latest version is published at [pathFinder Release](https://github.com/uksrc/pathFinder/releases) check before installing or upgrading. Set the version and install the pathFinder package: - VERSION=1.x.x - dnf upgrade https://github.com/uksrc/pathFinder/releases/download/v1.0.0/pathfinder-${VERSION}-1.x86_64.rpm + VERSION=1.x.x dnf upgrade https://github.com/uksrc/pathFinder/releases/download/v1.0.0/pathfinder-${VERSION}-1.x86_64.rpm -## pathFinder RPM package +## RPM package -The RPM package for pathFinder is published as a part of a [GitHub release](https://github.com/uksrc/pathFinder/releases). +The RPM package for pathFinder is published as a part of the [GitHub releases](https://github.com/uksrc/pathFinder/releases). From 0df8cfa1d05451c171ca9011e78a71cc7fbfe2de Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Fri, 10 Apr 2026 10:55:43 +0100 Subject: [PATCH 03/12] Fix heading levels in README --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4c35ddd..826bb26 100644 --- a/README.md +++ b/README.md @@ -34,9 +34,9 @@ The `pathFinder` command is available to run on the CLI after logging into the S --unmount Unmount previously mounted data instead of mounting -h, --help Print help -#### OAUTH Authentication +#### Authentication with SKA-IAM flow -Example using SKAIAM OAuth2: +Example using SKA-IAM OAuth2: $ sudo pathFinder --namespace daac --file-name simple_file.txt @@ -60,7 +60,7 @@ Example with environment variables (e.g. for automation): export SITE_CAPABILITIES_ACCESS_TOKEN="your_token_here" sudo pathFinder --namespace daac --file-name simple_file.txt --no-login -#### Unmounting Data +### Unmount Data Example for unmounting a file: From 972ccbfecc5c279121b4834fc433338812f69fac Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Fri, 10 Apr 2026 11:00:28 +0100 Subject: [PATCH 04/12] Minor tweaks to README --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 826bb26..9477588 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Path Finder -**pathFinder** is a tool for mounting SKA data on Slurm clusters without copying the data locally. Currently, it is provided as a single binary or an RPM installer. It is important to prepare the Slurm cluster ahead of its use (see [Installation](#installation)). +**pathFinder** is a tool for mounting SKA data on Slurm clusters without copying the data locally. Currently, it is provided as a single binary or an RPM installer. The tool allows the Scientist to specify which files, identified from the Science Gateway, they want to mount while keeping the files secure and owned by them. Two methods are planned: interactive and a workflow managed by the Science Gateway via prepareData. @@ -13,7 +13,7 @@ Features: ## Installation -For instructions on the setup & requirements for your HPC Server environment, see the [server configuration](./SERVER-CONFIGURATION.md) doc. +For instructions on the requirements and setup for your HPC server environment, and installation of **pathFinder** itself, see the [SERVER-CONFIGURATION.md](./SERVER-CONFIGURATION.md) doc. ## Usage @@ -69,4 +69,4 @@ Example for unmounting a file: ## Development -Notes on development can be found in the [development](DEVELOPMENT.md) doc. +Notes on development can be found in the [DEVELOPMENT.md](DEVELOPMENT.md) doc. From e1332eb07907b97e3d23cbb68f851910216891f7 Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Fri, 10 Apr 2026 11:05:45 +0100 Subject: [PATCH 05/12] Fix RPM instructions --- SERVER-CONFIGURATION.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/SERVER-CONFIGURATION.md b/SERVER-CONFIGURATION.md index 9799f46..4f43431 100644 --- a/SERVER-CONFIGURATION.md +++ b/SERVER-CONFIGURATION.md @@ -77,14 +77,10 @@ If you already have EPEL enabled you can skip the next 2 steps. usermod -a -G pathfinder sm2921 -## Package installation +## RPM Package installation -The latest version is published at [pathFinder Release](https://github.com/uksrc/pathFinder/releases) check before installing or upgrading. +An RPM installation package is published on the [pathFinder release](https://github.com/uksrc/pathFinder/releases) page, check for the latest version before installing or upgrading. Set the version and install the pathFinder package: - VERSION=1.x.x dnf upgrade https://github.com/uksrc/pathFinder/releases/download/v1.0.0/pathfinder-${VERSION}-1.x86_64.rpm - -## RPM package - -The RPM package for pathFinder is published as a part of the [GitHub releases](https://github.com/uksrc/pathFinder/releases). + VERSION=1.x.y dnf upgrade https://github.com/uksrc/pathFinder/releases/download/v${VERSION}/pathfinder-${VERSION}-1.x86_64.rpm From f49808472d8ff434255eb014a3ddaac66407c5b2 Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Thu, 9 Apr 2026 12:28:17 +0100 Subject: [PATCH 06/12] chore(iss-8): Update version in Cargo toml --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 33563bf..fbb747f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1394,7 +1394,7 @@ dependencies = [ [[package]] name = "pathFinder" -version = "1.0.1" +version = "1.0.2-alpha.0" dependencies = [ "anyhow", "clap", diff --git a/Cargo.toml b/Cargo.toml index bed15e6..6bc2d76 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pathFinder" -version = "1.0.1" +version = "1.0.2-alpha.0" edition = "2021" [[bin]] From da9d432ba06c4e9dede6a04ea8a93da6dd9c5452 Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Wed, 8 Apr 2026 15:59:03 +0100 Subject: [PATCH 07/12] docs(iss-8): Fix docstring on CLI function to fix CLI help message --- src/cli.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index c62a756..8c92a80 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -17,15 +17,14 @@ use std::env; use crate::oauth2::Tokens; -/// Command-line arguments for pathFinder. +/// ** pathFinder ** /// -/// Parse these with [`clap::Parser::parse`]; the resulting struct is then -/// passed to [`check_privileges`] before any API work begins. +/// A CLI tool for mounting SKA data. #[derive(Parser, Debug)] -#[command(name = "path-finder")] -#[command(about = "A tool for finding SKA data paths for mounting purposes")] +#[command(name = "pathFinder")] +#[command(about = "A CLI tool for mounting SKA data.")] pub struct Args { - /// Namespace of the data (e.g. `"ska:ska-sdp/eb-m001-20240101-00000"`). + /// Namespace of the data (e.g. "teal"). #[arg(long)] pub namespace: String, @@ -34,12 +33,12 @@ pub struct Args { pub file_name: String, /// Skip the OAuth2 device-code flow and read tokens from - /// `DATA_MANAGEMENT_ACCESS_TOKEN` and `SITE_CAPABILITIES_ACCESS_TOKEN` + /// DATA_MANAGEMENT_ACCESS_TOKEN and SITE_CAPABILITIES_ACCESS_TOKEN /// instead. #[arg(long)] pub no_login: bool, - /// Unmount a previously mounted file instead of mounting it. + /// Unmount a file instead of mounting it. #[arg(long)] pub unmount: bool, } From 4bfd38b05f44d38331c14a34999c15da96c258a0 Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Thu, 9 Apr 2026 10:14:36 +0100 Subject: [PATCH 08/12] fix(iss-8): Add ownership check before changing ownership and permissions --- Cargo.lock | 11 +++++ Cargo.toml | 1 + src/mount.rs | 137 ++++++++++++++++++++++++++++++++++++++++++++------- 3 files changed, 132 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fbb747f..e99e486 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1408,6 +1408,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", + "users", ] [[package]] @@ -2191,6 +2192,16 @@ dependencies = [ "serde", ] +[[package]] +name = "users" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24cc0f6d6f267b73e5a2cadf007ba8f9bc39c6a6f9666f8cf25ea809a153b032" +dependencies = [ + "libc", + "log", +] + [[package]] name = "utf8_iter" version = "1.0.4" diff --git a/Cargo.toml b/Cargo.toml index 6bc2d76..79f5721 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ tokio = { version = "1.40", features = ["full"] } regex = "1.10" dirs = "5.0" libc = "0.2" +users = "0.11.0" [dev-dependencies] httpmock = "0.7" diff --git a/src/mount.rs b/src/mount.rs index 6bfb78f..9413b6a 100644 --- a/src/mount.rs +++ b/src/mount.rs @@ -7,6 +7,33 @@ use anyhow::{Context, Result}; use std::fs; use std::path::Path; use std::process::Command; +use users; + +#[cfg(unix)] +use std::os::unix::fs::MetadataExt; + +/// Returns `true` if `path` is already owned by `username` (uid and gid both match). +/// +/// Falls back to `false` if the user cannot be resolved in the password database or +/// there was any error in obtaining or comparing file to user information. +fn dir_already_owned_by(path: &Path, username: &str) -> bool { + #[cfg(unix)] + { + + let (uid, gid) = match users::get_user_by_name(username) { + Some(user) => (user.uid(), user.primary_group_id()), + None => return false, // If we can't resolve the user, we can't confirm ownership, so assume it's not owned by them. + }; + + fs::metadata(path) + .map(|m| m.uid() == uid && m.gid() == gid) + .unwrap_or(false) + } + #[cfg(not(unix))] + { + false + } +} /// Abstraction over system commands, allowing real system calls in production and mock system calls during testing. trait Runner { @@ -148,32 +175,52 @@ fn mount_operation_impl( // Set ownership and permissions let user_group = format!("{}:{}", sudo_user, sudo_user); - runner.run_command( - "chown", - &["-R", &user_group, home.join(".binds").to_str().unwrap()], - "Set ownership of .binds directory", - )?; + // Set ownership of .binds/ directory. + // We do NOT use recursive chown, as this would + // fail if the directory happens to contain read-only bindfs content from a prior run. + // Skip entirely when the directory is already correctly owned (e.g. a second invocation for a + // different file that shares the same .binds/ but has already been set up). + if !dir_already_owned_by(&bind_dir, sudo_user) { + runner.run_command( + "chown", + &[&user_group, bind_dir.to_str().unwrap()], + "Set ownership of .binds directory", + )?; + } #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - let mut perms = fs::metadata(&bind_dir)?.permissions(); - perms.set_mode(0o600); - fs::set_permissions(&bind_dir, perms)?; + let perms = fs::metadata(&bind_dir)?.permissions(); + if perms.mode() & 0o777 != 0o600 { + let mut new_perms = perms; + new_perms.set_mode(0o600); + fs::set_permissions(&bind_dir, new_perms)?; + } } - runner.run_command( - "chown", - &["-R", &user_group, projects_dir.to_str().unwrap()], - "Set ownership of projects directory", - )?; + // Set ownership of projects/ directory. + // We do NOT use recursive chown, as this would + // fail if the directory happens to contain read-only bindfs content from a prior run. + // Skip entirely when the directory is already correctly owned (e.g. a second invocation for a + // different file that shares the same .binds/ but has already been set up). + if !dir_already_owned_by(&projects_dir, sudo_user) { + runner.run_command( + "chown", + &[&user_group, projects_dir.to_str().unwrap()], + "Set ownership of projects directory", + )?; + } #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - let mut perms = fs::metadata(&projects_file)?.permissions(); - perms.set_mode(0o500); - fs::set_permissions(&projects_file, perms)?; + let perms = fs::metadata(&projects_file)?.permissions(); + if perms.mode() & 0o777 != 0o500 { + let mut new_perms = perms; + new_perms.set_mode(0o500); + fs::set_permissions(&projects_file, new_perms)?; + } } // Run bindfs @@ -469,7 +516,63 @@ mod tests { ); } - // --- unmount: path edge cases --- + // --- mount: chown safety --- + + + /// Regression test: mounting a second file in the same namespace must succeed even when + /// the `projects/` directory already exists and contains a read-only placeholder + /// file left over from the first mount. + #[test] + fn mount_second_file_in_same_namespace_succeeds() { + let tmp = TempDir::new().unwrap(); + let skadata = tmp.path().join("skadata"); + let home = tmp.path().join("home"); + + // Seed two different files under the same skadata directory / namespace. + let data_dir = skadata.join("daac/08/06"); + fs::create_dir_all(&data_dir).unwrap(); + fs::write(data_dir.join("random10MiB.bin"), b"").unwrap(); + fs::write(data_dir.join("other100MiB.bin"), b"").unwrap(); + + // First mount succeeds. + mount_operation_impl( + "/daac/08/06/random10MiB.bin", + NAMESPACE, + USER, + &skadata, + &home, + &MockRunner::new(), + ) + .unwrap(); + + // Simulate the projects_dir placeholder from the first mount being read-only + // (as it would be after a real `mount --bind`). + let first_projects_file = home + .join(USER) + .join("projects") + .join(NAMESPACE) + .join("random10MiB.bin"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = fs::metadata(&first_projects_file).unwrap().permissions(); + perms.set_mode(0o000); // no permissions — simulates a read-only bind mount + fs::set_permissions(&first_projects_file, perms).unwrap(); + } + + // Second mount with a different file in the same namespace must not error. + mount_operation_impl( + "/daac/08/06/other100MiB.bin", + NAMESPACE, + USER, + &skadata, + &home, + &MockRunner::new(), + ) + .unwrap(); + } + + #[test] fn unmount_errors_on_path_with_no_filename() { From c1c75b333bf2b26e7ad55abe606716684d643a13 Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Thu, 9 Apr 2026 15:14:16 +0100 Subject: [PATCH 09/12] feat(iss-8): Add containerised integration test --- .github/workflows/ci.yml | 28 ++++++ DEVELOPMENT.md | 63 ++++++++++--- Dockerfile.test | 28 ++++++ README.md | 3 +- src/mount.rs | 185 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 294 insertions(+), 13 deletions(-) create mode 100644 Dockerfile.test diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 537294b..e5959a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,3 +23,31 @@ jobs: - name: Run tests run: cargo test + + integration-test: + runs-on: ubuntu-latest + # rust:latest runs as root by default, which is required for chown to + # reassign ownership to the test user created during the integration tests. + container: + image: rust:latest + + steps: + - uses: actions/checkout@v6 + + - name: Install test dependencies + run: | + apt-get update + apt-get install -y --no-install-recommends passwd + rm -rf /var/lib/apt/lists/* + + - name: Cache dependencies + uses: actions/cache@v5 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-integration-${{ hashFiles('**/Cargo.lock') }} + + - name: Run integration tests + run: cargo test -- --include-ignored diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 2a82f8d..0f9e9bd 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -1,18 +1,57 @@ -# Architecture +# Development Guide +There are CI pipelines in GitHub for testing and building the executable, plus publishing an RPM on a release. -## Features +For manual testing there here are some hints: -- OAuth2 device code flow authentication -- Data location lookup via Data Management API -- Site capabilities verification via Site Capabilities API -- Secure data mounting with proper permissions +## Prerequisites +- [Rust toolchain](https://rustup.rs/) (stable) +- Docker (for running integration tests locally) -### Modules +## Building -- **main.rs** - Main path finder CLI logic -- **api_client.rs** - HTTP client for Data Management and Site Capabilities APIs -- **oauth2_auth.rs** - OAuth2 device code flow implementation with token caching -- **models.rs** - Data structures for API responses (sites, nodes, storage areas, data locations) -- **mount.rs** - Mount/unmount utility for data access \ No newline at end of file + cargo build + +For a release build: + + cargo build --release + +## Running the binary locally + + cargo run -- --namespace daac --file-name pi24_test_run_1_cleaned.fits + +## Testing + +### Unit tests + +Run the fast unit test suite (no root required, all I/O mocked): + + cargo test + +### Integration tests + +The integration tests exercise real `chown` behaviour — they create a temporary system user (`pf_testuser`), call the actual `chown` binary, and verify ownership changes on disk. They require root on Linux. + +#### Locally via Docker (recommended before pushing) + + docker build -f Dockerfile.test -t pf-test . + docker run --rm pf-test + +This builds a `rust:latest` container (runs as root) and executes `cargo test -- --include-ignored`. + +#### Directly (if already root on a Linux machine) + + cargo test -- --include-ignored + +Integration tests are skipped automatically when not running as root, so it is safe to run this command in any environment. + +#### In CI + +The `integration-test` job in `.github/workflows/ci.yml` runs `cargo test -- --include-ignored` inside a `rust:latest` container, which provides root automatically. It runs in parallel with the regular unit test job on every push and pull request. + +### Cleaning up after an interrupted integration test + +If a test run is interrupted before the `pf_testuser` system user is removed, subsequent runs will panic with a clear message. Remove the leftover user with: + + sudo userdel pf_testuser diff --git a/Dockerfile.test b/Dockerfile.test new file mode 100644 index 0000000..d1e26c6 --- /dev/null +++ b/Dockerfile.test @@ -0,0 +1,28 @@ +# Integration test image. +# +# Runs as root (the default for this base image), which is required for `chown` +# to reassign ownership to the test user created during the tests. +# +# Build and run: +# docker build -f Dockerfile.test -t pf-test . +# docker run --rm pf-test +FROM rust:latest + +# passwd provides useradd/userdel, needed by integration tests to create/remove +# the pf_testuser system account. Included by default in most distros but made +# explicit here for reproducibility. +RUN apt-get update \ + && apt-get install -y --no-install-recommends passwd \ + && rm -rf /var/lib/apt/lists/* + +WORKDIR /app + +# Fetch dependencies before copying source so this layer is cached independently. +COPY Cargo.toml Cargo.lock ./ +RUN mkdir src && echo 'fn main() {}' > src/main.rs \ + && cargo fetch \ + && rm -rf src + +COPY . . + +CMD ["cargo", "test", "--", "--include-ignored"] diff --git a/README.md b/README.md index 9477588..78b9440 100644 --- a/README.md +++ b/README.md @@ -69,4 +69,5 @@ Example for unmounting a file: ## Development -Notes on development can be found in the [DEVELOPMENT.md](DEVELOPMENT.md) doc. +Notes on how to build the executable, run the unit and integration tests, and local Docker-based testing can be found in the development can be found in the [DEVELOPMENT.md](DEVELOPMENT.md) doc. + diff --git a/src/mount.rs b/src/mount.rs index 9413b6a..09d93d7 100644 --- a/src/mount.rs +++ b/src/mount.rs @@ -664,4 +664,189 @@ mod tests { "other file in same dir should be untouched" ); } + + // --- integration tests (require root on Linux) --- + // + // These tests create a real system user to exercise actual `chown` behaviour — + // verifying ownership changes on disk rather than just the shape of command arguments. + // + // They are marked `#[ignore]` and are skipped at runtime when not running as root. + // + // Run locally: + // docker build -f Dockerfile.test -t pf-test . && docker run --rm pf-test + // + // Run in CI: the `integration-test` job in .github/workflows/ci.yml runs them automatically. + + use std::sync::Mutex; + + /// Serialises integration tests that share the `pf_testuser` system user, + /// preventing conflicts when the test harness runs tests in parallel. + static INTEGRATION_MUTEX: Mutex<()> = Mutex::new(()); + + /// RAII guard: creates the `pf_testuser` system user on construction and + /// removes it via `userdel` on drop, even if the test panics. + struct TestUser; + + impl TestUser { + const NAME: &'static str = "pf_testuser"; + + /// Creates the test user, panicking if it already exists (leftover from a prior run). + fn create() -> Self { + if users::get_user_by_name(Self::NAME).is_some() { + panic!( + "test user '{}' already exists on this system — \ + remove it first with: userdel {}", + Self::NAME, + Self::NAME, + ); + } + let status = Command::new("useradd") + .args(["--no-create-home", "--system", Self::NAME]) + .status() + .expect("failed to execute useradd — is it installed?"); + assert!(status.success(), "useradd failed with exit status: {status}"); + Self + } + } + + impl Drop for TestUser { + fn drop(&mut self) { + let _ = Command::new("userdel").arg(Self::NAME).status(); + } + } + + /// Runner that executes `chown` for real (so ownership changes are visible on disk) + /// but mocks `bindfs` and `mount` to avoid needing those tools or mount privileges. + /// + /// All `run_command` calls are recorded so tests can inspect what was (or was not) invoked. + struct CapturingRealChownRunner { + commands: std::cell::RefCell)>>, + mountpoint_calls: Cell, + } + + impl CapturingRealChownRunner { + fn new() -> Self { + Self { + commands: std::cell::RefCell::new(vec![]), + mountpoint_calls: Cell::new(0), + } + } + } + + impl Runner for CapturingRealChownRunner { + fn run_command(&self, cmd: &str, args: &[&str], description: &str) -> Result<()> { + self.commands + .borrow_mut() + .push((cmd.to_string(), args.iter().map(|s| s.to_string()).collect())); + if cmd == "chown" { + run_command(cmd, args, description) + } else { + Ok(()) // mock bindfs and mount — no elevated mount privileges needed + } + } + fn is_mountpoint(&self, _path: &Path) -> Result { + let n = self.mountpoint_calls.get(); + self.mountpoint_calls.set(n + 1); + Ok(n > 0) + } + } + + /// Verifies that after a first mount the bind and projects directories are actually + /// owned by the target user on disk (uid and gid match), not just that a `chown` + /// command was issued with the right arguments. + #[test] + #[ignore = "requires root on Linux; run via `docker run --rm pf-test` or the CI integration-test job"] + fn integration_chown_sets_correct_ownership_on_first_mount() { + if unsafe { libc::getuid() } != 0 { + eprintln!("skipped: not running as root"); + return; + } + + let _guard = INTEGRATION_MUTEX.lock().unwrap(); + let _user = TestUser::create(); + + let tmp = TempDir::new().unwrap(); + let skadata = tmp.path().join("skadata"); + seed_skadata(&skadata); + let home = tmp.path().join("home"); + + mount_operation_impl( + DATA_PATH, + NAMESPACE, + TestUser::NAME, + &skadata, + &home, + &CapturingRealChownRunner::new(), + ) + .unwrap(); + + let bind_dir = home.join(TestUser::NAME).join(".binds").join("random10MiB"); + let projects_dir = home.join(TestUser::NAME).join("projects").join(NAMESPACE); + + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let user = users::get_user_by_name(TestUser::NAME).unwrap(); + let bind_meta = fs::metadata(&bind_dir).unwrap(); + let proj_meta = fs::metadata(&projects_dir).unwrap(); + assert_eq!(bind_meta.uid(), user.uid(), "bind_dir uid mismatch"); + assert_eq!(bind_meta.gid(), user.primary_group_id(), "bind_dir gid mismatch"); + assert_eq!(proj_meta.uid(), user.uid(), "projects_dir uid mismatch"); + assert_eq!(proj_meta.gid(), user.primary_group_id(), "projects_dir gid mismatch"); + } + } + + /// Verifies that mounting a second file in the same namespace does NOT call `chown` + /// again, because `dir_already_owned_by` detects correct ownership from the real inode + /// and the skip logic fires — validated against actual disk state, not command arguments. + #[test] + #[ignore = "requires root on Linux; run via `docker run --rm pf-test` or the CI integration-test job"] + fn integration_chown_skipped_on_second_mount_when_already_owned() { + if unsafe { libc::getuid() } != 0 { + eprintln!("skipped: not running as root"); + return; + } + + let _guard = INTEGRATION_MUTEX.lock().unwrap(); + let _user = TestUser::create(); + + let tmp = TempDir::new().unwrap(); + let skadata = tmp.path().join("skadata"); + let data_dir = skadata.join("daac/08/06"); + fs::create_dir_all(&data_dir).unwrap(); + fs::write(data_dir.join("random10MiB.bin"), b"").unwrap(); + fs::write(data_dir.join("other100MiB.bin"), b"").unwrap(); + let home = tmp.path().join("home"); + + // First mount: runs real chown, setting ownership on disk. + mount_operation_impl( + "/daac/08/06/random10MiB.bin", + NAMESPACE, + TestUser::NAME, + &skadata, + &home, + &CapturingRealChownRunner::new(), + ) + .unwrap(); + + // Second mount: directories are already correctly owned; chown should not be called. + let runner2 = CapturingRealChownRunner::new(); + mount_operation_impl( + "/daac/08/06/other100MiB.bin", + NAMESPACE, + TestUser::NAME, + &skadata, + &home, + &runner2, + ) + .unwrap(); + + let cmds = runner2.commands.borrow(); + let chown_calls: Vec<_> = cmds.iter().filter(|(cmd, _)| cmd == "chown").collect(); + assert!( + chown_calls.is_empty(), + "chown should not be called on second mount when dirs are already correctly owned; \ + got: {chown_calls:?}" + ); + } } From 55943cb505f356ca16a5b16b6f54db38fd7de4c0 Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Thu, 9 Apr 2026 17:32:13 +0100 Subject: [PATCH 10/12] chore(iss-8): Improve integration test image build time --- Dockerfile.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile.test b/Dockerfile.test index d1e26c6..4e7b8bd 100644 --- a/Dockerfile.test +++ b/Dockerfile.test @@ -23,6 +23,6 @@ RUN mkdir src && echo 'fn main() {}' > src/main.rs \ && cargo fetch \ && rm -rf src -COPY . . +COPY src src CMD ["cargo", "test", "--", "--include-ignored"] From 3f1b0104c38b2a8f3326b6379c733a1362ca0716 Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Thu, 9 Apr 2026 17:33:54 +0100 Subject: [PATCH 11/12] fix(iss-8): Fix ownership of mounted file in projects directory --- src/mount.rs | 96 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 58 insertions(+), 38 deletions(-) diff --git a/src/mount.rs b/src/mount.rs index 09d93d7..c9b06ad 100644 --- a/src/mount.rs +++ b/src/mount.rs @@ -19,7 +19,6 @@ use std::os::unix::fs::MetadataExt; fn dir_already_owned_by(path: &Path, username: &str) -> bool { #[cfg(unix)] { - let (uid, gid) = match users::get_user_by_name(username) { Some(user) => (user.uid(), user.primary_group_id()), None => return false, // If we can't resolve the user, we can't confirm ownership, so assume it's not owned by them. @@ -212,6 +211,13 @@ fn mount_operation_impl( )?; } + // Set ownership of the placeholder file inside projects// to ensure it's accessible to the target user. + runner.run_command( + "chown", + &[&user_group, projects_file.to_str().unwrap()], + "Set ownership of projects placeholder file", + )?; + #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; @@ -518,7 +524,6 @@ mod tests { // --- mount: chown safety --- - /// Regression test: mounting a second file in the same namespace must succeed even when /// the `projects/` directory already exists and contains a read-only placeholder /// file left over from the first mount. @@ -572,8 +577,6 @@ mod tests { .unwrap(); } - - #[test] fn unmount_errors_on_path_with_no_filename() { let tmp = TempDir::new().unwrap(); @@ -704,7 +707,10 @@ mod tests { .args(["--no-create-home", "--system", Self::NAME]) .status() .expect("failed to execute useradd — is it installed?"); - assert!(status.success(), "useradd failed with exit status: {status}"); + assert!( + status.success(), + "useradd failed with exit status: {status}" + ); Self } } @@ -735,9 +741,10 @@ mod tests { impl Runner for CapturingRealChownRunner { fn run_command(&self, cmd: &str, args: &[&str], description: &str) -> Result<()> { - self.commands - .borrow_mut() - .push((cmd.to_string(), args.iter().map(|s| s.to_string()).collect())); + self.commands.borrow_mut().push(( + cmd.to_string(), + args.iter().map(|s| s.to_string()).collect(), + )); if cmd == "chown" { run_command(cmd, args, description) } else { @@ -790,18 +797,28 @@ mod tests { let bind_meta = fs::metadata(&bind_dir).unwrap(); let proj_meta = fs::metadata(&projects_dir).unwrap(); assert_eq!(bind_meta.uid(), user.uid(), "bind_dir uid mismatch"); - assert_eq!(bind_meta.gid(), user.primary_group_id(), "bind_dir gid mismatch"); + assert_eq!( + bind_meta.gid(), + user.primary_group_id(), + "bind_dir gid mismatch" + ); assert_eq!(proj_meta.uid(), user.uid(), "projects_dir uid mismatch"); - assert_eq!(proj_meta.gid(), user.primary_group_id(), "projects_dir gid mismatch"); + assert_eq!( + proj_meta.gid(), + user.primary_group_id(), + "projects_dir gid mismatch" + ); } } - /// Verifies that mounting a second file in the same namespace does NOT call `chown` - /// again, because `dir_already_owned_by` detects correct ownership from the real inode - /// and the skip logic fires — validated against actual disk state, not command arguments. + /// Verifies that the placeholder file created inside `projects//` is owned + /// by the target user, not by root. + /// + /// The file is created by the process running as root (via `fs::OpenOptions`), so + /// without an explicit `chown` it would be root:root — inaccessible to the target user. #[test] #[ignore = "requires root on Linux; run via `docker run --rm pf-test` or the CI integration-test job"] - fn integration_chown_skipped_on_second_mount_when_already_owned() { + fn integration_projects_placeholder_file_is_owned_by_target_user() { if unsafe { libc::getuid() } != 0 { eprintln!("skipped: not running as root"); return; @@ -812,15 +829,11 @@ mod tests { let tmp = TempDir::new().unwrap(); let skadata = tmp.path().join("skadata"); - let data_dir = skadata.join("daac/08/06"); - fs::create_dir_all(&data_dir).unwrap(); - fs::write(data_dir.join("random10MiB.bin"), b"").unwrap(); - fs::write(data_dir.join("other100MiB.bin"), b"").unwrap(); + seed_skadata(&skadata); let home = tmp.path().join("home"); - // First mount: runs real chown, setting ownership on disk. mount_operation_impl( - "/daac/08/06/random10MiB.bin", + DATA_PATH, NAMESPACE, TestUser::NAME, &skadata, @@ -829,24 +842,31 @@ mod tests { ) .unwrap(); - // Second mount: directories are already correctly owned; chown should not be called. - let runner2 = CapturingRealChownRunner::new(); - mount_operation_impl( - "/daac/08/06/other100MiB.bin", - NAMESPACE, - TestUser::NAME, - &skadata, - &home, - &runner2, - ) - .unwrap(); + let projects_file = home + .join(TestUser::NAME) + .join("projects") + .join(NAMESPACE) + .join("random10MiB.bin"); - let cmds = runner2.commands.borrow(); - let chown_calls: Vec<_> = cmds.iter().filter(|(cmd, _)| cmd == "chown").collect(); - assert!( - chown_calls.is_empty(), - "chown should not be called on second mount when dirs are already correctly owned; \ - got: {chown_calls:?}" - ); + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let user = users::get_user_by_name(TestUser::NAME).unwrap(); + let file_meta = fs::metadata(&projects_file).unwrap(); + assert_eq!( + file_meta.uid(), + user.uid(), + "projects placeholder file uid should be {}, got {}", + user.uid(), + file_meta.uid() + ); + assert_eq!( + file_meta.gid(), + user.primary_group_id(), + "projects placeholder file gid should be {}, got {}", + user.primary_group_id(), + file_meta.gid() + ); + } } } From c3d638dc94e49e1e2b308228dc98acbd9cefd5e6 Mon Sep 17 00:00:00 2001 From: Roger Duthie <343584+rjaduthie@users.noreply.github.com> Date: Thu, 9 Apr 2026 18:36:52 +0100 Subject: [PATCH 12/12] chore(iss-8): Update CHANGELOG --- CHANGELOG.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5986038..9acb24e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- Mounting two or more files no longer causes error when settting permissions. +- Fixed documentation when the `--help` flag is set. + ## v1.0.1 (2026-04-01) ### Added @@ -22,4 +29,3 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Initial Rust implementation -