Skip to content

Commit

Permalink
Remove acltype normalization for datasets
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
anodos325 committed Aug 30, 2024
1 parent c4262a3 commit da357b8
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
4 changes: 0 additions & 4 deletions src/middlewared/middlewared/plugins/pool_/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,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)
Expand Down
22 changes: 22 additions & 0 deletions tests/api2/test_nfsv4_top_level_dataset.py
Original file line number Diff line number Diff line change
@@ -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'

0 comments on commit da357b8

Please sign in to comment.