From e9c02eb37da0c6e9ecff75ef63b678ee18198fc1 Mon Sep 17 00:00:00 2001 From: Logan Cary Date: Mon, 9 Sep 2024 10:37:58 -0400 Subject: [PATCH 1/4] remove rest --- tests/api2/test_999_pool_dataset_unlock.py | 191 --------------------- tests/api2/test_pool_dataset_unlock.py | 160 +++++++++++++++++ 2 files changed, 160 insertions(+), 191 deletions(-) delete mode 100644 tests/api2/test_999_pool_dataset_unlock.py create mode 100644 tests/api2/test_pool_dataset_unlock.py diff --git a/tests/api2/test_999_pool_dataset_unlock.py b/tests/api2/test_999_pool_dataset_unlock.py deleted file mode 100644 index 9c19442683081..0000000000000 --- a/tests/api2/test_999_pool_dataset_unlock.py +++ /dev/null @@ -1,191 +0,0 @@ -import os -import sys -apifolder = os.getcwd() -sys.path.append(apifolder) - -import contextlib -import urllib.parse - -import pytest - -from auto_config import pool_name -from functions import POST, DELETE, wait_on_job -from middlewared.test.integration.assets.account import user -from middlewared.test.integration.utils import ssh -from protocols import SMB -from samba import ntstatus, NTSTATUSError - - -SMB_PASSWORD = 'Abcd1234' -SMB_USER = 'smbuser999' - - -def passphrase_encryption(): - return { - 'encryption_options': { - 'generate_key': False, - 'pbkdf2iters': 100000, - 'algorithm': 'AES-128-CCM', - 'passphrase': 'passphrase', - }, - 'encryption': True, - 'inherit_encryption': False, - } - - -@contextlib.contextmanager -def dataset(name, options=None): - assert "/" not in name - - dataset = f"{pool_name}/{name}" - - result = POST("/pool/dataset/", {"name": dataset, **(options or {})}) - assert result.status_code == 200, result.text - - result = POST("/filesystem/setperm/", {'path': f"/mnt/{dataset}", "mode": "777"}) - assert result.status_code == 200, result.text - job_status = wait_on_job(result.json(), 180) - assert job_status["state"] == "SUCCESS", str(job_status["results"]) - - try: - yield dataset - finally: - result = DELETE(f"/pool/dataset/id/{urllib.parse.quote(dataset, '')}/") - assert result.status_code == 200, result.text - - -@contextlib.contextmanager -def smb_share(name, path, options=None): - results = POST("/sharing/smb/", { - "name": name, - "path": path, - "guestok": True, - **(options or {}), - }) - assert results.status_code == 200, results.text - id = results.json()["id"] - - try: - yield id - finally: - result = DELETE(f"/sharing/smb/id/{id}/") - assert result.status_code == 200, result.text - - -def lock_dataset(name): - payload = { - 'id': name, - 'lock_options': { - 'force_umount': True - } - } - results = POST('/pool/dataset/lock', payload) - assert results.status_code == 200, results.text - job_id = results.json() - job_status = wait_on_job(job_id, 120) - assert job_status['state'] == 'SUCCESS', str(job_status['results']) - - -def unlock_dataset(name, options=None): - payload = { - 'id': name, - 'unlock_options': { - 'recursive': True, - 'datasets': [ - { - 'name': name, - 'passphrase': 'passphrase' - } - ], - **(options or {}), - } - } - results = POST('/pool/dataset/unlock/', payload) - assert results.status_code == 200, results.text - job_id = results.json() - job_status = wait_on_job(job_id, 120) - assert job_status['state'] == 'SUCCESS', str(job_status['results']) - assert job_status['results']['result']['unlocked'] == [name], str(job_status['results']) - - -@contextlib.contextmanager -def smb_connection(**kwargs): - c = SMB() - c.connect(**kwargs) - - try: - yield c - finally: - c.disconnect() - - -@pytest.fixture(scope='module') -def smb_user(): - with user({ - 'username': SMB_USER, - 'full_name': 'doug', - 'group_create': True, - 'password': SMB_PASSWORD, - 'smb': True - }, get_instance=True) as u: - yield u - - -@pytest.mark.dependency(name="create_dataset") -@pytest.mark.parametrize("toggle_attachments", [True, False]) -def test_pool_dataset_unlock_smb(smb_user, toggle_attachments): - # Prepare test SMB share - with dataset("normal") as normal: - with smb_share("normal", f"/mnt/{normal}"): - # Create an encrypted SMB share, unlocking which might lead to SMB service interruption - with dataset("encrypted", passphrase_encryption()) as encrypted: - with smb_share("encrypted", f"/mnt/{encrypted}"): - ssh(f"touch /mnt/{encrypted}/secret") - results = POST("/service/start/", {"service": "cifs"}) - assert results.status_code == 200, results.text - lock_dataset(encrypted) - # Mount test SMB share - with smb_connection( - share="normal", - username=SMB_USER, - password=SMB_PASSWORD - ) as normal_connection: - # Locked share should not be mountable - with pytest.raises(NTSTATUSError) as e: - with smb_connection( - share="encrypted", - username=SMB_USER, - password=SMB_PASSWORD - ): - pass - - assert e.value.args[0] == ntstatus.NT_STATUS_BAD_NETWORK_NAME - - conn = normal_connection.show_connection() - assert conn['connected'], conn - unlock_dataset(encrypted, {"toggle_attachments": toggle_attachments}) - - conn = normal_connection.show_connection() - assert conn['connected'], conn - - if toggle_attachments: - # We should be able to mount encrypted share - with smb_connection( - share="encrypted", - username=SMB_USER, - password=SMB_PASSWORD - ) as encrypted_connection: - assert [x["name"] for x in encrypted_connection.ls("")] == ["secret"] - else: - # We should still not be able to mount encrypted share as we did not reload attachments - with pytest.raises(NTSTATUSError) as e: - with smb_connection( - share="encrypted", - username=SMB_USER, - password=SMB_PASSWORD - ): - pass - - assert e.value.args[0] == ntstatus.NT_STATUS_BAD_NETWORK_NAME - results = POST("/service/stop/", {"service": "cifs"}) - assert results.status_code == 200, results.text diff --git a/tests/api2/test_pool_dataset_unlock.py b/tests/api2/test_pool_dataset_unlock.py new file mode 100644 index 0000000000000..a1d44b26df197 --- /dev/null +++ b/tests/api2/test_pool_dataset_unlock.py @@ -0,0 +1,160 @@ +import contextlib +import urllib.parse + +import pytest + +from auto_config import pool_name +from middlewared.test.integration.assets.account import user +from middlewared.test.integration.utils import call, ssh +from protocols import SMB +from samba import ntstatus, NTSTATUSError + + +SMB_PASSWORD = 'Abcd1234' +SMB_USER = 'smbuser999' + + +def passphrase_encryption(): + return { + 'encryption_options': { + 'generate_key': False, + 'pbkdf2iters': 100000, + 'algorithm': 'AES-128-CCM', + 'passphrase': 'passphrase', + }, + 'encryption': True, + 'inherit_encryption': False, + } + + +@contextlib.contextmanager +def dataset(name, options=None): + assert '/' not in name + + dataset = f'{pool_name}/{name}' + call('pool.dataset.create', {'name': dataset, **(options or {})}) + call('filesystem.setperm', {'path': f'/mnt/{dataset}', 'mode': '777'}, job=True) + + try: + yield dataset + finally: + assert call('pool.dataset.delete', urllib.parse.quote(dataset, "")) + + +@contextlib.contextmanager +def smb_share(name, path, options=None): + id = call('sharing.smb.create', { + 'name': name, + 'path': path, + 'guestok': True, + **(options or {}), + })['id'] + try: + yield id + finally: + assert call('sharing.smb.delete', id) + + +def lock_dataset(name): + payload = { + 'force_umount': True + } + assert call('pool.dataset.lock', name, payload, job=True) + + +def unlock_dataset(name, options=None): + payload = { + 'recursive': True, + 'datasets': [ + { + 'name': name, + 'passphrase': 'passphrase' + } + ], + **(options or {}), + } + result = call('pool.dataset.unlock', name, payload, job=True) + assert result['unlocked'] == [name], str(result) + + +@contextlib.contextmanager +def smb_connection(**kwargs): + c = SMB() + c.connect(**kwargs) + + try: + yield c + finally: + c.disconnect() + + +@pytest.fixture(scope='module') +def smb_user(): + with user({ + 'username': SMB_USER, + 'full_name': 'doug', + 'group_create': True, + 'password': SMB_PASSWORD, + 'smb': True + }, get_instance=True) as u: + yield u + + +@pytest.mark.parametrize('toggle_attachments', [True, False]) +def test_pool_dataset_unlock_smb(smb_user, toggle_attachments): + with ( + # Prepare test SMB share + dataset('normal') as normal, + smb_share('normal', f'/mnt/{normal}'), + # Create an encrypted SMB share, unlocking which might lead to SMB service interruption + dataset('encrypted', passphrase_encryption()) as encrypted, + smb_share('encrypted', f'/mnt/{encrypted}') + ): + ssh(f'touch /mnt/{encrypted}/secret') + assert call('service.start', 'cifs') + lock_dataset(encrypted) + # Mount test SMB share + with smb_connection( + share='normal', + username=SMB_USER, + password=SMB_PASSWORD + ) as normal_connection: + # Locked share should not be mountable + with pytest.raises(NTSTATUSError) as e: + with smb_connection( + share='encrypted', + username=SMB_USER, + password=SMB_PASSWORD + ): + pass + + assert e.value.args[0] == ntstatus.NT_STATUS_BAD_NETWORK_NAME + + conn = normal_connection.show_connection() + assert conn['connected'], conn + unlock_dataset(encrypted, {'toggle_attachments': toggle_attachments}) + + conn = normal_connection.show_connection() + assert conn['connected'], conn + + if toggle_attachments: + # We should be able to mount encrypted share + with smb_connection( + share='encrypted', + username=SMB_USER, + password=SMB_PASSWORD + ) as encrypted_connection: + assert [x['name'] for x in encrypted_connection.ls('')] == ['secret'] + else: + # We should still not be able to mount encrypted share as we did not reload attachments + with pytest.raises(NTSTATUSError) as e: + with smb_connection( + share='encrypted', + username=SMB_USER, + password=SMB_PASSWORD + ): + pass + + assert e.value.args[0] == ntstatus.NT_STATUS_BAD_NETWORK_NAME + + assert call('service.stop', 'cifs') From 66cbc67a84fc98c2dc88eb41e185549c117e67a7 Mon Sep 17 00:00:00 2001 From: Logan Cary Date: Mon, 9 Sep 2024 12:49:15 -0400 Subject: [PATCH 2/4] no longer need urllib --- tests/api2/test_pool_dataset_unlock.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/api2/test_pool_dataset_unlock.py b/tests/api2/test_pool_dataset_unlock.py index a1d44b26df197..9843f224d170b 100644 --- a/tests/api2/test_pool_dataset_unlock.py +++ b/tests/api2/test_pool_dataset_unlock.py @@ -1,5 +1,4 @@ import contextlib -import urllib.parse import pytest @@ -38,7 +37,7 @@ def dataset(name, options=None): try: yield dataset finally: - assert call('pool.dataset.delete', urllib.parse.quote(dataset, "")) + assert call('pool.dataset.delete', dataset) @contextlib.contextmanager From c3ac9867b634b4cc5b6fbb00dbdaf041f1fb6f1d Mon Sep 17 00:00:00 2001 From: Logan Cary Date: Mon, 9 Sep 2024 14:54:35 -0400 Subject: [PATCH 3/4] service.stop returns False --- tests/api2/test_pool_dataset_unlock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api2/test_pool_dataset_unlock.py b/tests/api2/test_pool_dataset_unlock.py index 9843f224d170b..3515c54a35cd3 100644 --- a/tests/api2/test_pool_dataset_unlock.py +++ b/tests/api2/test_pool_dataset_unlock.py @@ -156,4 +156,4 @@ def test_pool_dataset_unlock_smb(smb_user, toggle_attachments): assert e.value.args[0] == ntstatus.NT_STATUS_BAD_NETWORK_NAME - assert call('service.stop', 'cifs') + assert call('service.stop', 'cifs') is False # successfully stopped From 4a949228d56b95fe0c69f6d31bd1096a90c57c4a Mon Sep 17 00:00:00 2001 From: Logan Cary Date: Tue, 10 Sep 2024 09:29:15 -0400 Subject: [PATCH 4/4] address @themylogin --- .../test/integration/assets/pool.py | 9 ++--- tests/api2/test_pool_dataset_unlock.py | 40 +++---------------- 2 files changed, 10 insertions(+), 39 deletions(-) diff --git a/src/middlewared/middlewared/test/integration/assets/pool.py b/src/middlewared/middlewared/test/integration/assets/pool.py index e07bc96e29788..1bfa811946118 100644 --- a/src/middlewared/middlewared/test/integration/assets/pool.py +++ b/src/middlewared/middlewared/test/integration/assets/pool.py @@ -67,11 +67,10 @@ def dataset(name, data=None, pool=pool, **kwargs): call("pool.dataset.create", {"name": dataset, **data}) try: - if "acl" in kwargs or "mode" in kwargs: - if "acl" in kwargs: - call("filesystem.setacl", {'path': f"/mnt/{dataset}", "dacl": kwargs['acl']}) - else: - call("filesystem.setperm", {'path': f"/mnt/{dataset}", "mode": kwargs['mode'] or "777"}) + if "acl" in kwargs: + call("filesystem.setacl", {'path': f"/mnt/{dataset}", "dacl": kwargs['acl']}) + elif "mode" in kwargs: + call("filesystem.setperm", {'path': f"/mnt/{dataset}", "mode": kwargs['mode'] or "777"}) yield dataset finally: diff --git a/tests/api2/test_pool_dataset_unlock.py b/tests/api2/test_pool_dataset_unlock.py index 3515c54a35cd3..7343a54aeaaac 100644 --- a/tests/api2/test_pool_dataset_unlock.py +++ b/tests/api2/test_pool_dataset_unlock.py @@ -2,8 +2,9 @@ import pytest -from auto_config import pool_name from middlewared.test.integration.assets.account import user +from middlewared.test.integration.assets.pool import dataset +from middlewared.test.integration.assets.smb import smb_share from middlewared.test.integration.utils import call, ssh from protocols import SMB from samba import ntstatus, NTSTATUSError @@ -25,35 +26,6 @@ def passphrase_encryption(): 'inherit_encryption': False, } - -@contextlib.contextmanager -def dataset(name, options=None): - assert '/' not in name - - dataset = f'{pool_name}/{name}' - call('pool.dataset.create', {'name': dataset, **(options or {})}) - call('filesystem.setperm', {'path': f'/mnt/{dataset}', 'mode': '777'}, job=True) - - try: - yield dataset - finally: - assert call('pool.dataset.delete', dataset) - - -@contextlib.contextmanager -def smb_share(name, path, options=None): - id = call('sharing.smb.create', { - 'name': name, - 'path': path, - 'guestok': True, - **(options or {}), - })['id'] - try: - yield id - finally: - assert call('sharing.smb.delete', id) - - def lock_dataset(name): payload = { 'force_umount': True @@ -103,11 +75,11 @@ def smb_user(): def test_pool_dataset_unlock_smb(smb_user, toggle_attachments): with ( # Prepare test SMB share - dataset('normal') as normal, - smb_share('normal', f'/mnt/{normal}'), + dataset('normal', mode='777') as normal, + smb_share(f'/mnt/{normal}', 'normal', {'guestok': True}), # Create an encrypted SMB share, unlocking which might lead to SMB service interruption - dataset('encrypted', passphrase_encryption()) as encrypted, - smb_share('encrypted', f'/mnt/{encrypted}') + dataset('encrypted', passphrase_encryption(), mode='777') as encrypted, + smb_share(f'/mnt/{encrypted}', 'encrypted', {'guestok': True}) ): ssh(f'touch /mnt/{encrypted}/secret') assert call('service.start', 'cifs')