NAS-129743 / 24.10 / Prevent acltype and xattr dataset changes if hos…

…ting an SMB share (#14032)

* Add check for acltype and xattr dataset changes when dataset is hosting an SMB share
Add CI test.

* Change the message from delete/recreate to disable/re-enable.
Update the CI test.

src/middlewared/middlewared/plugins/pool_/dataset.py

@@ -292,6 +292,26 @@ async def __common_validation(self, verrors, schema, data, mode, parent=None, cu
         if data['type'] == 'FILESYSTEM':
             to_check = {'acltype': None, 'aclmode': None}
 
+            if mode == 'UPDATE':
+                # Prevent users from changing acltype or xattr settings underneath an active SMB share
+                # If this dataset hosts an SMB share, then prompt the user to first delete the share,
+                # make the dataset change, the recreate the share.
+                keys = ('acltype', 'xattr')
+                if any([data.get(key) for key in keys]):
+                    ds_attachments = await self.middleware.call('pool.dataset.attachments', data['name'])
+                    if smb_attachments := [share for share in ds_attachments if share['type'] == "SMB Share"]:
+                        share_names = [smb_share['attachments'] for smb_share in smb_attachments]
+                        for key in (k for k in keys if data.get(k)):
+                            if cur_dataset and (cur_dataset[key]['value'] == data.get(key)):
+                                continue
+                            verrors.add(
+                                f'{schema}.{key}',
+                                'This dataset is hosting SMB shares. '
+                                f'Before {key} can be updated the following shares must be disabled: '
+                                f'{share_names[0]}. '
+                                'The shares may be re-enabled after the change.'
+                            )
+
             # Prevent users from setting incorrect combinations of aclmode and acltype parameters
             # The final value to be set may have one of several different possible origins
             # 1. The parameter may be provided in `data` (explicit creation or update)

tests/api2/test_427_smb_acl.py

@@ -1,5 +1,6 @@
 #!/usr/bin/env python3
 
+import errno
 import pytest
 import sys
 import os
@@ -11,11 +12,13 @@
 from auto_config import (
     pool_name,
 )
+from middlewared.service_exception import ValidationError, ValidationErrors
 from middlewared.test.integration.assets.account import user
 from middlewared.test.integration.assets.smb import smb_share
 from middlewared.test.integration.assets.pool import dataset
 from middlewared.test.integration.utils import call
 from middlewared.test.integration.utils.client import truenas_server
+from middlewared.test.integration.utils.unittest import RegexString
 from protocols import SMB
 from pytest_dependency import depends
 from time import sleep
@@ -258,3 +261,33 @@ def test_007_test_disable_autoinherit(request):
             sd = c.get_sd('foo')
             assert 'SEC_DESC_DACL_PROTECTED' in sd['control']['parsed'], str(sd)
             c.disconnect()
+
+
+def test_008_test_prevent_smb_dataset_update(request):
+    """
+    Prevent changing acltype and xattr on dataset hosting SMB shares
+    """
+    ds_name = 'prevent_changes'
+    path = f'/mnt/{pool_name}/{ds_name}'
+    with create_dataset(f'{pool_name}/{ds_name}') as ds:
+        with smb_share(path, 'SMB_SHARE_1'):
+            # Create a second share for testing purposes
+            with smb_share(path, 'SMB_SHARE_2'):
+
+                # Confirm we ignore requests that don't involve changes
+                for setting in [{"xattr": "SA"}, {"acltype": "POSIX"}]:
+                    call('pool.dataset.update', ds, setting)
+
+                # Confirm we block requests that involve changes
+                for setting in [{"xattr": "ON"}, {"acltype": "OFF"}]:
+                    attrib = list(setting.keys())[0]
+                    with pytest.raises(ValidationErrors) as ve:
+                        call('pool.dataset.update', ds, setting)
+                    assert ve.value.errors == [
+                        ValidationError(
+                            f"pool_dataset_update.{attrib}",
+                            RegexString("This dataset is hosting SMB shares. .*"),
+                            errno.EINVAL,
+                        )
+                    ]
+                    assert "SMB_SHARE_2" in str(ve.value.errors[0]), ve.value.errors[0]