HDDS-3161. Block illegal characters when creating keys.

[cku328]

What changes were proposed in this pull request?

Added illegal character check (using regular expressions) for key name when creating key.

For the definition of illegal characters, I refer to Amazon S3's object key naming guide, which specifies the characters to avoid.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-3161

How was this patch tested?

• Ran UTs.
• Using Ozone Shell to create a key with illegal characters in the name, errors can be detected, like this:
  

[elek]

Thanks @cku328 to create this patch. I am fine with following the S3 naming convention but not sure how will it work when buckets will be accessed from NFS/ozone fs.

What do you think @arp7 , can we do this restriction?

From technical point of view: Didn't check the patch very closely, but it seems to be a client side check. I think it should be checked on the server side, too (but it might be included)...

[cku328]

Thanks to @elek for the comments and @mukul1987 for the review.

I referenced the commit and review comments for this pull request(#718), implemented the key name check at OMKeyCreateRequest#preExecute, and applied it the same way to other requests (e.g.: CreateFile, RenameKey).

In the newly submitted patch, I fixed some of the requests for missing check names, and I tested the put file through OzoneFS and it worked.

[adoroszlai]

Thanks @cku328 for working on this.

My 2 cents: I would prefer this check to be optional, as a convenience for users who want to make sure key names are compatible with a wider range of tools. It could be enabled eg. by a flag in Ozone clients, or some global or volume/bucket-level config. But making it mandatory might break some use cases.

[cku328]

Thanks @adoroszlai for the review.
In the new patch, this keyname check is optional and it can be enabled by Ozone global config (default is false).

In ozone-site.xml, add the following snippet to enable checking:

<property>
  <name>ozone.om.keyname.character.check.enabled</name>
  <value>true</value>
</property>

[adoroszlai]

Thanks @cku328 for updating the patch and making this check optional. Tested ozone sh and ozone fs with both enabled and disabled config setting, works fine.

[adoroszlai]

I would like to propose making this message consistent with the one in OmUtils, so that ozone fs -put and ozone sh key put both print the same error. I think it can be most simply done by moving the message from OmUtils here and letting the OMException propagate the underlying exception's message.

Suggested change

	throw new IllegalArgumentException("key name contains " +
	"illegal characters.");
	throw new IllegalArgumentException("Invalid key name: " + keyName);

If you agree, please feel free to wait for others' reviews before making this change, to reduce the number of rounds.

[cku328]

Thanks for the suggestion, I agree with you.
I'll wait for others' reviews and suggestions before I make this change.

[adoroszlai]

Corresponding to the suggestion for HddsClientUtils:

Suggested change

	throw new OMException("Invalid key name: " + keyName,
	throw new OMException(e.getMessage(),

[cku328]

Update message of exception and resolve conflict. (latest master-branch #399)

[elek]

This patch seems to be reviewed and all the comments are addressed. Will merge it if no objections.

I resolved the conflicts and started a new build to get a green build with the latest master.

[elek]

🎵 Green build, green build, tada-tada-tada... 🎵 Merging it right now, tada-tada-tada 🎵

Thanks the patch @cku328 (and sorry for the long review time) and the review @adoroszlai