Inactive NFS export validation #2924

phillxnet · 2024-11-11T11:43:31Z

Via Web-UI and API it is possible to enter, for example, considered invalid NFS Clients (Web-UI reference) / host_string (model/API reference) data, resulting in, in this case:

Shares	Host String	Read only	Sync / Async	Actions
test-share	1.!	no	async

With the consequent export of:

cat /etc/exports
/export/test-share 1.!(rw,async,insecure)

We have in-place a number of validation mechanism intended to filter out, in the above example, what is considered valid host_string contents via the following model field validators:

https://github.com/rockstor/rockstor-core/blob/master/src/rockstor/storageadmin/models/validators.py

representing:

validate_nfs_host_str()
validate_nfs_modify_str()
validate_nfs_sync_choice()

With the 2nd & 3rd validators there is also model choice precedence, i.e. the model only accepts ro|rw for modify_str, and async|sync for sync_choice and so we have, in place, redundant validation for those fields. But in the example case of the nfs_host_str (host_str field) used to define the core of this issue, we have at least an inactive validation. And consequently also insufficient API test coverage for this failure.

This issue is dependant on NFS test improvements proposed in the following issue:

NFS tests in need of maintenance NFS tests in need of maintenance #2923

The text was updated successfully, but these errors were encountered:

phillxnet · 2024-11-11T11:48:43Z

This issue is a spin-off from development already enacted for:

NFS exports restored as read-only NFS exports restored as read-only #2912 NFS exports restored as read-only #2912 #2922

And as such the cause has already been identified in that work, a failure to run model.clean_fields() prior to model.save().

This issue, having now been identified, was thus spun-out to more clearly detail development endeavours.

phillxnet · 2024-11-11T15:26:00Z

Further to the recent merge of #2925:

NFS tests in need of maintenance NFS tests in need of maintenance #2923 NFS tests in need of maintenance #2923 #2925

We have our proof of inactive validation via the recent test improvements:

cd /opt/rockstor/src/rockstor
poetry run django-admin test -v 2 -p test_nfs_export.py
...
test_adv_nfs_get (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_adv_nfs_get) ... ok
test_adv_nfs_post_requests (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_adv_nfs_post_requests) ... ok
test_delete_requests (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_delete_requests) ... ok
test_get (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_get) ... /opt/rockstor/.venv/lib/python3.11/site-packages/rest_framework/pagination.py:207: UnorderedObjectListWarning: Pagination may yield inconsistent results with an unordered object_list: <class 'storageadmin.models.nfs_export_group.NFSExportGroup'> QuerySet.
  paginator = self.django_paginator_class(queryset, page_size)
ok
test_host_str_validator_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_host_str_validator_post) ... FAIL
test_host_str_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_host_str_validator_put) ... FAIL
test_invalid_get (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_invalid_get) ... ok
test_low_level_error_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_low_level_error_post) ... ok
test_low_level_error_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_low_level_error_put) ... ok
test_mod_choice_validator_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_mod_choice_validator_post) ... FAIL
test_mod_choice_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_mod_choice_validator_put) ... FAIL
test_no_nfs_client (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_no_nfs_client) ... ok
test_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_post) ... ok
test_post_requests (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_post_requests) ... ok
test_put_requests (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_put_requests) ... ok
test_share_already_exported (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_share_already_exported) ... ok
test_sync_choice_validator_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_sync_choice_validator_post) ... FAIL
test_sync_choice_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_sync_choice_validator_put) ... FAIL

======================================================================
FAIL: test_host_str_validator_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_host_str_validator_post)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_nfs_export.py", line 256, in test_host_str_validator_post
    self.assertEqual(
AssertionError: 200 != 500 : {'id': 4, 'exports': [{'id': 4, 'share': 'share2', 'share_id': '22', 'mount': '/export/share2', 'export_group': 4}], 'host_str': '1.!', 'editable': 'ro', 'syncable': 'async', 'mount_security': 'insecure', 'nohide': False, 'enabled': True, 'admin_host': None}

======================================================================
FAIL: test_host_str_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_host_str_validator_put)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_nfs_export.py", line 274, in test_host_str_validator_put
    self.assertEqual(
AssertionError: 200 != 500 : {'id': 3, 'exports': [{'id': 3, 'share': 'share-nfs', 'share_id': '21', 'mount': '/export/share-nfs', 'export_group': 3}], 'host_str': '*', 'editable': 'rw', 'syncable': 'async', 'mount_security': 'insecure', 'nohide': False, 'enabled': True, 'admin_host': None}

======================================================================
FAIL: test_mod_choice_validator_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_mod_choice_validator_post)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_nfs_export.py", line 189, in test_mod_choice_validator_post
    self.assertEqual(
AssertionError: 200 != 500 : {'id': 6, 'exports': [{'id': 6, 'share': 'share2', 'share_id': '22', 'mount': '/export/share2', 'export_group': 6}], 'host_str': '*', 'editable': 'rr', 'syncable': 'async', 'mount_security': 'insecure', 'nohide': False, 'enabled': True, 'admin_host': None}

======================================================================
FAIL: test_mod_choice_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_mod_choice_validator_put)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_nfs_export.py", line 206, in test_mod_choice_validator_put
    self.assertEqual(
AssertionError: 200 != 500 : {'id': 3, 'exports': [{'id': 3, 'share': 'share-nfs', 'share_id': '21', 'mount': '/export/share-nfs', 'export_group': 3}], 'host_str': '*', 'editable': 'rw', 'syncable': 'async', 'mount_security': 'insecure', 'nohide': False, 'enabled': True, 'admin_host': None}

======================================================================
FAIL: test_sync_choice_validator_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_sync_choice_validator_post)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_nfs_export.py", line 222, in test_sync_choice_validator_post
    self.assertEqual(
AssertionError: 200 != 500 : {'id': 10, 'exports': [{'id': 10, 'share': 'share2', 'share_id': '22', 'mount': '/export/share2', 'export_group': 10}], 'host_str': '*', 'editable': 'ro', 'syncable': 'aaaaa', 'mount_security': 'insecure', 'nohide': False, 'enabled': True, 'admin_host': None}

======================================================================
FAIL: test_sync_choice_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_sync_choice_validator_put)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_nfs_export.py", line 239, in test_sync_choice_validator_put
    self.assertEqual(
AssertionError: 200 != 500 : {'id': 3, 'exports': [{'id': 3, 'share': 'share-nfs', 'share_id': '21', 'mount': '/export/share-nfs', 'export_group': 3}], 'host_str': '*', 'editable': 'rw', 'syncable': 'async', 'mount_security': 'insecure', 'nohide': False, 'enabled': True, 'admin_host': None}

----------------------------------------------------------------------
Ran 18 tests in 0.827s

FAILED (failures=6)

Enable intended NFS model field validations during POST & PUT by calling NFSExportGroup.clean_fields() prior to model.save(). N.B. There exists a build-in `choices=` field validator that takes precedence for the fields: 'editable', and 'syncable'. So only the 'host_str' field actively exercises the custom validators. I.e. we have redundant validators for 'editable', and 'syncable'. This redundancy is not the focus of this fix: only re-enabling model field validation. ## Includes - Above fix regarding model field validation. - Incidental fix for newer Django warning surfaced by recent NFS test updates: "Pagination may yield inconsistent results with an unordered object_list ...". Adds a default ordering on `id` in NFSExportGroup model. - Remove outdated model comment regarding editable default: NFSExportGroup model - nfs_export_group.py model definition file black re-formatted.

…dation Inactive NFS export validation #2924

phillxnet · 2024-11-13T16:35:42Z

Closing as:
Fixed by #2926

phillxnet self-assigned this Nov 11, 2024

phillxnet added this to the 5.1.X-X Stable release milestone Nov 11, 2024

phillxnet mentioned this issue Nov 11, 2024

NFS tests in need of maintenance #2923 #2925

Merged

phillxnet mentioned this issue Nov 11, 2024

Inactive NFS export validation #2924 #2926

Merged

phillxnet added a commit that referenced this issue Nov 13, 2024

Merge pull request #2926 from phillxnet/2924-Inactive-NFS-export-vali…

a8e7d23

…dation Inactive NFS export validation #2924

phillxnet closed this as completed Nov 13, 2024

This was referenced Nov 13, 2024

NFS exports restored as read-only #2912 #2922

Closed

(t) Error when trying to delete NFS export via WebUI #2742

Open

[NFS] Reflect new Sync default in export options #2392

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inactive NFS export validation #2924

Inactive NFS export validation #2924

phillxnet commented Nov 11, 2024

phillxnet commented Nov 11, 2024 •

edited

Loading

phillxnet commented Nov 11, 2024

phillxnet commented Nov 13, 2024

Inactive NFS export validation #2924

Inactive NFS export validation #2924

Comments

phillxnet commented Nov 11, 2024

phillxnet commented Nov 11, 2024 • edited Loading

phillxnet commented Nov 11, 2024

phillxnet commented Nov 13, 2024

phillxnet commented Nov 11, 2024 •

edited

Loading