From 1122ef8cdfd258e69da0fe54c1157730076f692f Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Fri, 30 Aug 2024 06:06:11 -0600 Subject: [PATCH] Remove acltype normalization for datasets When we were doing initial development of TrueNAS SCALE (pre-angelfish) a bug was encountered when importing pools from FreeBSD in which the acltype could be set to OFF. Eventually this bug was fixed on the ZFS side, but it was still early days of testing NFSv4 acltype on SCALE and so we didn't want early-adopter migrators to have broken shares. Thus we normalized creation of new datasets to be POSIX acltype if user had selected to inherit from top-level dataset. After a few releases and user bug reports we realized we needed to have specific ZFS dataset properties set depending on acltype. For example that the aclmode should be DISCARD (discard internal ZFS acl) because changing acltype does not remove the internal ZFS ACL and it could still be evaluated in zfs_zaccess calls. This eventually necessitated raising ValidationErrors on dataset creation for invalid parameter combinations. Unfortunately, the normalization behavior introduced in angelfish testing was overlooked at this point, which could cause a situation in which pools from FreeBSD where acltype now imports as nfsv4acl on top-level datasets would get flagged as needing normalization. This led to an invalid combination of acltype POSIX and aclmode PASSTHROUGH being set on new datasets when they were created to inherit from the top-level dataset. Rather than fixing the normalization, at this point we're pretty confident about NFSv4 ACL type behavior in SCALE and can just remove the normalization logic entirely. (cherry picked from commit da357b83d441a930fcc80d2f39539d58cb7a1142) --- .../middlewared/plugins/pool_/dataset.py | 4 ---- tests/api2/test_nfsv4_top_level_dataset.py | 22 +++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 tests/api2/test_nfsv4_top_level_dataset.py diff --git a/src/middlewared/middlewared/plugins/pool_/dataset.py b/src/middlewared/middlewared/plugins/pool_/dataset.py index 0ce5120c103fd..2d2b67bc95e59 100644 --- a/src/middlewared/middlewared/plugins/pool_/dataset.py +++ b/src/middlewared/middlewared/plugins/pool_/dataset.py @@ -591,10 +591,6 @@ async def do_create(self, app, data): parent_st['acltype'] = await self.middleware.call('filesystem.path_get_acltype', parent_mp) mountpoint = os.path.join('/mnt', data['name']) - if data['type'] == 'FILESYSTEM' and data.get('acltype', 'INHERIT') == 'INHERIT' and len( - data['name'].split('/') - ) == 2: - data['acltype'] = 'POSIX' try: await self.middleware.call('filesystem.stat', mountpoint) diff --git a/tests/api2/test_nfsv4_top_level_dataset.py b/tests/api2/test_nfsv4_top_level_dataset.py new file mode 100644 index 0000000000000..5bc500872ed53 --- /dev/null +++ b/tests/api2/test_nfsv4_top_level_dataset.py @@ -0,0 +1,22 @@ +import pytest + +from middlewared.test.integration.assets.pool import dataset +from middlewared.test.integration.utils import call, pool + + +@pytest.fixture(scope='module') +def set_nfsv4_top_level(): + call('pool.dataset.update', pool, {'acltype': 'NFSV4', 'aclmode': 'PASSTHROUGH'}) + + try: + yield + finally: + call('pool.dataset.update', pool, {'acltype': 'POSIX', 'aclmode': 'DISCARD'}) + + +def test__acltype_inherit(set_nfsv4_top_level): + with dataset('v4inherit') as ds: + entry = call('pool.dataset.query', [['name', '=', ds]], {'get': True}) + + assert entry['acltype']['value'] == 'NFSV4' + assert entry['aclmode']['value'] == 'PASSTHROUGH'