Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recursive dataset mounting support to pam_zfs_key #16857

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jkolo
Copy link

@jkolo jkolo commented Dec 12, 2024

PR Title

Add recursive dataset mounting support to pam_zfs_key

Motivation and Context

This change introduces the ability to recursively mount ZFS datasets using a new configuration option, mount_recursively. This enhancement solves the need for hierarchical or nested dataset management in environments that utilize pam_zfs_key, ensuring all eligible child datasets are mounted as part of the session functionality.
This feature improves the flexibility and usability of pam_zfs_key when managing complex dataset organizations.

Description

  • Added support for a new configuration option, mount_recursively, which ensures child datasets of a given root can also be mounted.
  • Refactored the decrypt_mount() function to integrate recursive mounting functionality, leveraging zfs_iter_filesystems_v2 for child dataset traversal.
  • Updated the dataset mounting logic to ensure properties such as canmount and mountpoint are respected during recursive mounting.
  • Modified configuration handling (via zfs_key_config_t) to include the mount_recursively option.
  • Enhanced the session logic to include recursive mounting without breaking existing behavior.
  • Added a functional test, pam_mount_recursively.ksh, to validate recursive mounting behavior and edge cases (e.g., "none", "legacy", "noauto", and "off" mountpoints).

How Has This Been Tested?

  • Added a functional test (tests/zfs-tests/tests/functional/pam/pam_mount_recursively.ksh) that:

    • Validates recursive mounting for nested datasets with the mount_recursively flag enabled.
    • Tests behavior across various child dataset configurations (e.g., different canmount and mountpoint properties).
    • Ensures correct key loading status through session management (open_session and close_session) via PAM.
    • Verifies datasets are only mounted when they satisfy canmount=on and appropriate mountpoint properties.
  • Manual testing performed on Linux configurations using pamtester to ensure real-world applicability.

  • Verified backward compatibility where mount_recursively is not enabled.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly. (Implementation details documented in code comments. Additional documentation updates suggested separately if needed.)
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

Let me know if additional details are required.

@github-actions github-actions bot added the Status: Work in Progress Not yet ready for general review label Dec 12, 2024
@jkolo jkolo force-pushed the pam_zfs_mount_recursively branch 3 times, most recently from 756fecc to 15404d0 Compare December 12, 2024 09:55
@jkolo jkolo marked this pull request as ready for review December 12, 2024 11:52
@github-actions github-actions bot added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Dec 12, 2024
@jkolo jkolo force-pushed the pam_zfs_mount_recursively branch 12 times, most recently from b685481 to cfc137e Compare December 16, 2024 01:34
@jkolo
Copy link
Author

jkolo commented Dec 16, 2024

Pull request is ready for merge. Is there anything else I should do to get it to pass review? @behlendorf sorry for calling directly. I didn't know who or where best to ask :)

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. Thanks for working on this and adding the test case.

contrib/pam_zfs_key/pam_zfs_key.c Outdated Show resolved Hide resolved
contrib/pam_zfs_key/pam_zfs_key.c Outdated Show resolved Hide resolved
@jkolo jkolo force-pushed the pam_zfs_mount_recursively branch from 8a25cee to 1050b16 Compare December 21, 2024 03:10
Introduced functionality to recursively mount datasets with a new
config option `mount_recursively`. Adjusted existing functions to
handle the recursive behavior and added tests to validate the feature.
This enhances support for managing hierarchical ZFS datasets within
a PAM context.

Signed-off-by: Jerzy Kołosowski <[email protected]>
Removed unnecessary comments from return statements that were redundant
and made the code cleaner. This does not impact functionality but
enhances readability and maintainability.

Signed-off-by: Jerzy Kołosowski <[email protected]>
@jkolo jkolo force-pushed the pam_zfs_mount_recursively branch from 1050b16 to 911d30b Compare December 21, 2024 03:10
@jkolo jkolo requested a review from behlendorf December 23, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants