-
Notifications
You must be signed in to change notification settings - Fork 80
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
NSFS | NC | Add Schema Validation to Bucket and Account Add and Update (NC NSFS CLI) #7702
Conversation
Manual Testing Instructions:Schema validation for buckets and accountsFirst, create the mkdir -p /tmp/nsfs_root1
chmod 777 /tmp/nsfs_root1 This will be the argument for:
In each case the commands work, hence we added the code change (to sabotage the created object and make sure that it creates an error, in each case we chose one of the properties that is required to be deleted. Account add
Account update
Bucket add
Bucket update
Remove bucket policy and FS backend:First, Create bucket Bucket policy:
FS backend
|
@shirady please add automatic tests |
144b264
to
f9a310e
Compare
a0cf8d5
to
1223079
Compare
8e53ebc
to
c97c3cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, as mentioned on #7694, bucketspace_fs missing validations will be added in the next PR
c97c3cb
to
2940536
Compare
3129b48
to
374db01
Compare
d326558
to
ae7a132
Compare
1. Move the NC NSFS schema validation that we had in file `src/sdk/bucketspace_fs.js` (from PR noobaa#7544) to a separate file (`src/manage_nsfs/nsfs_schema_utils.js`) to reuse it. 2. Edit the values of `s3_policy` and `fs_backend` to be undefined (and not empty `''`, as it passed as an argument). 3. Validate the data using schema and return an error in case it doesn't match (details would be passed according to the original message), using a mapping `ManageCLIError.RPC_ERROR_TO_MANAGE`. 4. Update the unit tests for the cases of `s3_policy` and `fs_backend` deletions (separate the argument that we pass as `''` to the actual property that would be `undefined`). 5. Change the `uid` and `gid` extract in function `fetch_account_data` to be `undefined` when the argument user was passed (and not `false`). 6. Use `schema_utils.strictify` with option `additionalProperties: false` in the `nsfs_account_schema` and `nsfs_bucket_schema`. 7. Add the `versioning` enum to the bucket schema. 8. Add tests for the schema validation of nsfs_account_schema and nsfs_bucket_schema. Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
ae7a132
to
4d11e6b
Compare
Explain the changes
src/sdk/bucketspace_fs.js
(from PR NC | NSFS | Schema validation #7544) to a separate file (src/manage_nsfs/nsfs_schema_utils.js
) to reuse it.s3_policy
andfs_backend
to be undefined (and not empty''
, as it passed as an argument).ManageCLIError.RPC_ERROR_TO_MANAGE
.s3_policy
andfs_backend
deletions (separate the argument that we pass as''
to the actual property that would beundefined
).uid
andgid
extract in functionfetch_account_data
to beundefined
when the argument user was passed (and notfalse
).schema_utils.strictify
with optionadditionalProperties: false
in thensfs_account_schema
andnsfs_bucket_schema
.versioning
enum to the bucket schema.Issues: Fixed #7694
Error: reference "bucket_api" resolves to more than one schema
in the CI in jobrun-unit-tests
.Testing Instructions:
sudo node ./node_modules/.bin/_mocha src/test/unit_tests/test_nc_nsfs_cli.js
sudo node ./node_modules/.bin/_mocha src/test/unit_tests/test_bucketspace_fs.js
npx jest test_nc_nsfs_schema_validation.test.js
(no need for sudo permission, can see results live also with jest extension in VSCode).