DaveTeng0 commented on PR #4630: URL: https://github.com/apache/ozone/pull/4630#issuecomment-1530387240
> Thanks @DaveTeng0 for the patch. I've noticed two issues: > > * Some `RpcClient` methods call `verifyResourceName` instead of `verifyVolumeName`. These methods still reject non-strict names, since those calls were not removed. > * nit: The method `verifyVolumeName` is no longer used, can be removed. > > On second thought, most of the name checks still apply to volume or bucket names regardless of the strictness setting. E.g. name should not be `null`, too short, too long, etc. > > Removing client-side validation introduces two problems: > > 1. usability: previously these were rejected at the client with specific message, but now we only get a generic message from OM > 2. performance: validation requires round-trip to OM > > Before: > > ``` > $ ozone sh volume create /v > INVALID_VOLUME_NAME Bucket or Volume length is illegal, valid length is 3-63 characters > $ ozone sh volume create /.hidden > INVALID_VOLUME_NAME Bucket or Volume name cannot start with a period or dash > $ ozone sh volume create /toooooooooooooooooooooooooooooooooooooooooooooooooolooooooooooooooooooooooooooooooooooooooooooooong > INVALID_VOLUME_NAME Bucket or Volume length is illegal, valid length is 3-63 characters > $ ozone sh volume create 192.168.1.1 > INVALID_VOLUME_NAME Bucket or Volume name cannot be an IPv4 address or all numeric > $ ozone sh volume create 12345 > INVALID_VOLUME_NAME Bucket or Volume name cannot be an IPv4 address or all numeric > ``` > > After: > > ``` > $ ozone sh volume create /v > INVALID_VOLUME_NAME Invalid volume name: v > $ ozone sh volume create /.hidden > INVALID_VOLUME_NAME Invalid volume name: .hidden > $ ozone sh volume create /toooooooooooooooooooooooooooooooooooooooooooooooooolooooooooooooooooooooooooooooooooooooooooooooong > INVALID_VOLUME_NAME Invalid volume name: toooooooooooooooooooooooooooooooooooooooooooooooooolooooooooooooooooooooooooooooooooooooooooooooong > $ ozone sh volume create 192.168.1.1 > INVALID_VOLUME_NAME Invalid volume name: 192.168.1.1 > $ ozone sh volume create 12345 > INVALID_VOLUME_NAME Invalid volume name: 12345 > ``` > > https://github.com/apache/ozone/blob/3858cd102bbc3c328c24261d929c6549f5358a40/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java#L491-L496 > > (The same applies to bucket name validation removed in [HDDS-7586](https://issues.apache.org/jira/browse/HDDS-7586).) > > So I think we should restore client-side validation (both volume and bucket), and always perform it with `strict=false`. This catches most cases at the client with the more specific message. OM will still reject names with `_` if it's set to `strict=true`. good catch! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
