Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inactive NFS export validation #2924 #2926

Merged

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