Skip to content

NC | CLI | Added a validation to check if whitespace is not trimmed from string value for a flag #8722

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 17, 2025
Merged

NC | CLI | Added a validation to check if whitespace is not trimmed from string value for a flag #8722

merged 1 commit into from
Feb 17, 2025

Conversation

@aayushchouhan09
Copy link
Member

@aayushchouhan09 aayushchouhan09 commented Jan 24, 2025 •
edited
Loading

Explain the changes

  1. Problem: Leading spaces before a flag value, when using =, caused the value to be split into an empty string ('') and the actual value, resulting in faulty flag assignments.
  2. Solution: A validation has been added to check if the flag value become empty due to incorrect formatting. It checks if argv._ has more than two elements and throws an error if invalid values are detected.

This prevents incorrect assignments and ensures flag values are valid.

Issues: Fixed #xxx / Gap #xxx

  1. Fixed: NC | CLI | Weird behavior of the CLI when adding space before access_key/secret_key flag #8319

Testing Instructions:

  1. Try creating account with noobaa-cli using below command:
    $ sudo node src/cmd/manage_nsfs account add --name aayush2 --uid=9 --gid=9 --new_buckets_path= tmp/
  • Tests added

@aayushchouhan09 aayushchouhan09 requested review from a team, jackyalbo and liranmauda and removed request for a team January 24, 2025 11:01
shirady
shirady reviewed Jan 26, 2025
View reviewed changes
nadavMiz
nadavMiz reviewed Jan 27, 2025
View reviewed changes
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jan 27, 2025
shirady
shirady reviewed Jan 27, 2025
View reviewed changes
@aayushchouhan09 aayushchouhan09 force-pushed the account-space-fix branch 2 times, most recently from 3ced00b to 0b8a0a1 Compare January 29, 2025 09:49
shirady
shirady reviewed Jan 29, 2025
View reviewed changes
shirady
shirady reviewed Jan 30, 2025
View reviewed changes
romayalon
romayalon reviewed Jan 30, 2025
View reviewed changes
@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 3, 2025
@pull-request-size pull-request-size bot added size/M and removed size/L labels Feb 3, 2025
romayalon
romayalon reviewed Feb 6, 2025
View reviewed changes
@romayalon romayalon mentioned this pull request Feb 6, 2025
Merged
2 tasks
shirady
shirady reviewed Feb 12, 2025
View reviewed changes
@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 12, 2025
shirady
shirady reviewed Feb 13, 2025
View reviewed changes
romayalon
romayalon reviewed Feb 13, 2025
View reviewed changes
shirady
shirady approved these changes Feb 16, 2025
View reviewed changes
Copy link
Contributor

@shirady shirady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aayushchouhan09
Added a validation to check if there is any extra argument in 'argv._…
b60a15f 
…' which may be due to leading space in flag values

Signed-off-by: Aayush Chouhan <[email protected]>

Updated the code after Romy's PR is merged

Signed-off-by: Aayush Chouhan <[email protected]>

Updated the logic and tests

Signed-off-by: Aayush Chouhan <[email protected]>

Updated to enum, added examples and error msg

Signed-off-by: Aayush Chouhan <[email protected]>

Update the function name and unit tests

Signed-off-by: Aayush Chouhan <[email protected]>
@aayushchouhan09 aayushchouhan09 merged commit e1e56a8 into noobaa:master Feb 17, 2025
11 checks passed
@aayushchouhan09 aayushchouhan09 deleted the account-space-fix branch February 17, 2025 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shirady shirady shirady approved these changes

@naveenpaul1 naveenpaul1 Awaiting requested review from naveenpaul1

@liranmauda liranmauda Awaiting requested review from liranmauda

@jackyalbo jackyalbo Awaiting requested review from jackyalbo jackyalbo was automatically assigned from noobaa/dev

@nadavMiz nadavMiz Awaiting requested review from nadavMiz

@romayalon romayalon Awaiting requested review from romayalon

Assignees

No one assigned

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

NC | CLI | Weird behavior of the CLI when adding space before access_key/secret_key flag

4 participants

@aayushchouhan09 @nadavMiz @romayalon @shirady