HDDS-5161. Fix the regex for key name validation

[Tejaskriya]

What changes were proposed in this pull request?

Changing the regex for KEYNAME_ILLEGAL_CHARACTER_CHECK_REGEX to allow "(" and ")"

What is the link to the Apache JIRA

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

How was this patch tested?

Added few cases in existing tests. Also tested manually using Docker after setting ozone.om.keyname.character.check.enabled to true in ozone_deafult.xml

bash-4.2$ ozone sh key put "s3v/buck1/k1(" k1
bash-4.2$ ozone sh key put "s3v/buck1/k1)" k1
bash-4.2$ ozone sh key list s3v/buck1
[ {
"volumeName" : "s3v",
"bucketName" : "buck1",
"name" : "k1(",
"dataSize" : 3,
"creationTime" : "2023-08-18T07:17:31.837Z",
"modificationTime" : "2023-08-18T07:53:49.334Z",
"replicationConfig" : {
"replicationFactor" : "THREE",
"requiredNodes" : 3,
"replicationType" : "RATIS"
},
"metadata" : { },
"file" : true
}, {
"volumeName" : "s3v",
"bucketName" : "buck1",
"name" : "k1)",
"dataSize" : 3,
"creationTime" : "2023-08-18T07:55:34.549Z",
"modificationTime" : "2023-08-18T07:55:35.397Z",
"replicationConfig" : {
"replicationFactor" : "THREE",
"requiredNodes" : 3,
"replicationType" : "RATIS"
},
"metadata" : { },
"file" : true
} ]
bash-4.2$ ozone sh key put "s3v/buck1/k1|" k1
Invalid key name: k1|
bash-4.2$ ozone sh key put "s3v/buck1/k1\\" k1
Invalid key name: k1\
bash-4.2$ ozone sh key put "s3v/buck1/k1^" k1
Invalid key name: k1^

[umamaheswararao]

@SaketaChalamchala could you also please take a look at it?

[SaketaChalamchala]

LGTM

[siddhantsangwan]

@Tejaskriya thanks for working on this. The regex logic looks good to me. It would be great to have a unit test that confirms ( and ) are accepted now.

[siddhantsangwan]

I have some thoughts on compatibility between different versions of ozone. With this change we won't allow quotation marks, so clients that are writing keys with quotes will start failing.

Annotations defined under InterfaceStability dictate how an interface can evolve. ClientProtocol has no such annotation and thus provides no stability guarantees. This means it can change any time and doesn't require a minor or major release. We also don't support rolling upgrades right now.

Considering all this, I think this change is fine and the target version for this jira can be 1.4.0. @SaketaChalamchala, @errose28 what do you think?

[errose28]

LGTM. I think we can go forward with the new key name validation. Existing keys will not be affected.

[Tejaskriya]

@Tejaskriya thanks for working on this. The regex logic looks good to me. It would be great to have a unit test that confirms ( and ) are accepted now.

Thanks for the review! I added cases in the unit test for verifying valid names now. Could you take a look it again?

[siddhantsangwan]

About the repeated test failure in CI (filesystem) (pull_request): https://issues.apache.org/jira/browse/HDDS-9041.

[siddhantsangwan]

We've had 3 different CI runs and all the tests in TestOzoneFileSystem have passed at least once if we consider all these runs. Since these failures are intermittent, I'm merging this PR to the master branch.

[siddhantsangwan]

The failures are being fixed here - #5217.