NC | NSFS | Schema validation #7544

naveenpaul1 · 2023-10-25T15:26:03Z

Explain the changes

Buckets and accounts schema validation
creation_date added to bucket management CLI

Issues: Fixed #xxx / Gap #xxx

https://issues.redhat.com/browse/MCGI-204

Testing Instructions:

Test with invalid schema JSON for bucket and accounts, Should throw an error
Try to create bucket with an invalid creation date, if the creation date missing current data use

Doc added/updated
Tests added

shirady

Hi,
Added a couple of comments and questions.

src/cmd/manage_nsfs.js

src/sdk/bucketspace_fs.js

src/cmd/manage_nsfs.js

shirady

LGTM

@naveenpaul1, please check if the validation that that we have a value (without checking something about the value) is enough to meet the requirement.

src/cmd/manage_nsfs.js

src/sdk/bucketspace_fs.js

src/cmd/manage_nsfs.js

romayalon

I think we should also use the schema validation in bucketspace_fs on create bucket :)

src/cmd/manage_nsfs.js

liranmauda

@naveenpaul1
few comments/questions:

Am I correct and the creation and read of the account/bucket are not calling any RPC?
please change the names of those Schema, to reflect that those are standalone (or any other name) schemas and not a RPC ones. Also, we already have files with those names so it can get us confused.

Signed-off-by: naveenpaul1 <napaul@redhat.com>

liranmauda

LGTM

liranmauda · 2023-10-31T07:22:01Z

src/cmd/nsfs.js

@@ -317,7 +317,7 @@ async function main(argv = minimist(process.argv.slice(2))) {
            }
        });
        if (await endpoint.is_http_allowed(nsfs_config_root)) {
-            dbg.log0('nsfs: listening on', util.inspect(`http://localhost:${http_port}`));
+            console.log('nsfs: listening on', util.inspect(`http://localhost:${http_port}`));


I wonder, why are we using console and not dbg?
by requiring the console we "hijack" the console, but the console does not have levels like 0,1 ...

@liranmauda ya totally agree with you, we need to have a discussion on this. In this PR I just reverted the previous code that I accidentally updated in my last PR.

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` definition 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>

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>

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> (cherry picked from commit 4d11e6b) Signed-off-by: Nimrod Becker <bnimrod@gmail.com>

naveenpaul1 requested review from alphaprinz, romayalon and shirady October 25, 2023 15:26

pull-request-size bot added the size/M label Oct 25, 2023

naveenpaul1 force-pushed the nsfs_validation branch from 0573001 to c4e8d79 Compare October 25, 2023 15:27

shirady reviewed Oct 26, 2023

View reviewed changes

naveenpaul1 force-pushed the nsfs_validation branch from c4e8d79 to abd5b2e Compare October 26, 2023 10:01

shirady approved these changes Oct 26, 2023

View reviewed changes

romayalon requested changes Oct 26, 2023

View reviewed changes

src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved

src/sdk/bucketspace_fs.js Show resolved Hide resolved

romayalon reviewed Oct 26, 2023

View reviewed changes

src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved

romayalon reviewed Oct 26, 2023

View reviewed changes

naveenpaul1 force-pushed the nsfs_validation branch from abd5b2e to 1acf2ef Compare October 27, 2023 07:43

pull-request-size bot added size/L and removed size/M labels Oct 27, 2023

liranmauda reviewed Oct 29, 2023

View reviewed changes

src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved

liranmauda requested changes Oct 29, 2023

View reviewed changes

naveenpaul1 force-pushed the nsfs_validation branch 4 times, most recently from add5c72 to 9144dfb Compare October 30, 2023 13:21

NC | NSFS | Schema validation

626996e

Signed-off-by: naveenpaul1 <napaul@redhat.com>

naveenpaul1 force-pushed the nsfs_validation branch from 9144dfb to 626996e Compare October 30, 2023 13:25

liranmauda approved these changes Oct 31, 2023

View reviewed changes

romayalon approved these changes Oct 31, 2023

View reviewed changes

naveenpaul1 merged commit 5a62046 into noobaa:master Oct 31, 2023
8 checks passed

naveenpaul1 deleted the nsfs_validation branch December 29, 2023 04:47

shirady mentioned this pull request Jan 7, 2024

NSFS | NC | Add Schema Validation to Bucket and Account Add and Update (NC NSFS CLI) #7702

Merged

3 tasks

shirady mentioned this pull request Jan 15, 2024

NSFS | NC | AJV bucket and account schemas should be validated on creation #7694

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NC | NSFS | Schema validation #7544

NC | NSFS | Schema validation #7544

naveenpaul1 commented Oct 25, 2023 •

edited

shirady left a comment

shirady left a comment

romayalon left a comment

liranmauda left a comment

liranmauda left a comment

liranmauda Oct 31, 2023 •

edited

naveenpaul1 Oct 31, 2023

NC | NSFS | Schema validation #7544

NC | NSFS | Schema validation #7544

Conversation

naveenpaul1 commented Oct 25, 2023 • edited

Explain the changes

Issues: Fixed #xxx / Gap #xxx

Testing Instructions:

shirady left a comment

Choose a reason for hiding this comment

shirady left a comment

Choose a reason for hiding this comment

romayalon left a comment

Choose a reason for hiding this comment

liranmauda left a comment

Choose a reason for hiding this comment

liranmauda left a comment

Choose a reason for hiding this comment

liranmauda Oct 31, 2023 • edited

Choose a reason for hiding this comment

naveenpaul1 Oct 31, 2023

Choose a reason for hiding this comment

naveenpaul1 commented Oct 25, 2023 •

edited

liranmauda Oct 31, 2023 •

edited