+
Skip to content

Conversation

phillxnet
Copy link
Member

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.

Fixes #2924

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.

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.
@phillxnet
Copy link
Member Author

Testing

Before

See the associated issue #2924 (comment) for details of the existing test failures: detailing model field validation failure.

After

On a Leap 15.6 host (x86_64) via rpm (built on host) install from this PR branch (5.0.14-2926):

Pertinent test set

cd /opt/rockstor/src/rockstor
poetry run django-admin test -v 2 -p test_nfs_export.p
...
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) ... ok
test_host_str_validator_post (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_host_str_validator_post) ... ok
test_host_str_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_host_str_validator_put) ... ok
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) ... ok
test_mod_choice_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_mod_choice_validator_put) ... ok
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) ... ok
test_sync_choice_validator_put (rockstor.storageadmin.tests.test_nfs_export.NFSExportTests.test_sync_choice_validator_put) ... ok

----------------------------------------------------------------------
Ran 18 tests in 0.919s

OK

All tests

(A prerequisite of a successful rpmbuild)

As per doc instructions: https://rockstor.com/docs/contribute/contribute.html#code-test

cd /opt/rockstor/src/rockstor
poetry run django-admin test -v 2
...
Ran 295 tests in 38.523s

OK

@phillxnet
Copy link
Member Author

phillxnet commented Nov 11, 2024

Web-UI back-end validation feedback

As per referenced issue #2924 reproducer re allowed "1.!" host_str, using the same input thus:

Attempt-invalid-NFS_Clients-Web-UI-input

Results in our Web-UI user error feedback dialog:

Houston, we've had a problem.
{'host_str': ['Invalid host string: 1.!']}

            Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/rest_framework_custom/generic_view.py", line 40, in _handle_exception
    yield
  File "/opt/rockstor/src/rockstor/storageadmin/views/nfs_exports.py", line 173, in post
    eg.clean_fields(exclude="admin_host")
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/base.py", line 1527, in clean_fields
    raise ValidationError(errors)
django.core.exceptions.ValidationError: {'host_str': ['Invalid host string: 1.!']}

Indicating our successfully surfacing this newly enabled model field validation. Also, no export was entered into the DB (no model.save() ), and no export was created on the underlying NFS config (/etc/exports).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载