HDDS-12911. Key deletion should not validate the name

[sreejasahithi]

What changes were proposed in this pull request?

AWS S3 PutObject API allows creating keys and directories with names containing invalid characters, but delete operation fails to remove such invalid objects from a FSO bucket. Hence to resolve this issue, key name validation should not be done during the key deletion.

For example :
Before :
aws s3api --endpoint http://localhost:9878/ put-object --bucket fsobuck --key sampledir:1/ --no-verify-ssl
this has no error and gets created successfully.
but when we try to delete it we get an error as follows,
An error occurred (500) when calling the DeleteObject operation (reached max retries: 4): Internal Server Error

What is the link to the Apache JIRA

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

How was this patch tested?

https://github.com/sreejasahithi/ozone/actions/runs/15020991860

This change was also tested locally over a docker ozone cluster.
Added a testcase in TestOMKeyDeleteRequestWithFSO.java

[sarvekshayr]

Thanks for working on this, @sreejasahithi.
I tested the validation, and it works as expected.

Currently, characters like ,, ., .., etc., are allowed during the put-object operation, and delete-object is able to handle such entries unlike the : case. We can leave these as is for now and focus only on excluding : as implemented.

cc: @nandakumar131

[sadanand48]

I looked into this and the reason for the corner case of put-object allowing a key like "sample:/" is because if the --body is not provided , it gets translated into a CreateDirectory operation for an FSO bucket

$ aws s3api --endpoint http://xx:9878 put-object --bucket buck2 --key sample:/

$ aws s3api --endpoint http://xx:9878 delete-object --bucket buck2 --key sample:/

An error occurred (500) when calling the DeleteObject operation (reached max retries: 2): Internal Server Error

The keyName check is controlled by a flag ozone.om.keyname.character.check.enabled which was introduced here and defaults to false.
But the logic in OmDeleteKeyRequest is different i.e it does not call the same validateKey() method but instead calls validateAndNormalizeKey which internally only validates if the bucket is FSO.

To not allow these chars in the first place, I feel one should tune this config to true for stricter char check or we could even change default to true.

@adoroszlai what do you think?.

[adoroszlai]

Thanks @sadanand48, @sarvekshayr, @sreejasahithi for the discussion so far, and sorry for the delayed response. Can we continue allowing : and try to fix delete operation?

1. Suddenly starting to reject certain characters due to implementation detail may break things.
2. S3 doc says : may need special care, not that it is invalid.

CC @ivandika3

[sadanand48]

continue allowing : and try to fix delete operation

Yeah we should keep the behaviour consistent b/w create and delete. Both use different methods to validate key name today causing the discrepancy. The validation for extra chars should only be done during create/rename key if the strict check flag is switched on and not do this validation on delete or any other API

[ivandika3]

+1 on not restricting the naming without understanding fully the possible regressions and instead fixing the specific case.

[SaketaChalamchala]

Agree that we should be able to delete existing keys and any new restrictions on key names should be behind the character check strictness flag.

[sreejasahithi]

Currently, validateAndNormalizeKey is used by both OMKeyDeleteRequest and OMKeyCreateRequest, which results in unnecessary key name validation during deletes operations.

Since key name validation is not necessary during delete operations, because we should be able to delete any key that was allowed to be created. To address this, I propose introducing a separate method for normalizing key paths specifically for OMKeyDeleteRequest. This new method would perform key name validation (i.e invokes isValidKeyPath) only if the configuration flag keyNameCharacterCheckEnabled is set to true, thereby preserving the existing behavior. Other operations like create or rename can continue using the existing validateAndNormalizeKey flow without any changes.

Alternatively, we could instead add a check for keyNameCharacterCheckEnabled directly inside validateAndNormalizeKey. However, this approach might affect all other operations that use this method.

CC @adoroszlai , @sadanand48 , @ivandika3 , @SaketaChalamchala

[sadanand48]

Yes @sreejasahithi we should modify the code to have a similar behaviour as the method added in HDDS-13262, basically maintain consistent behaviour for these checks.

[sadanand48]

Thanks @sreejasahithi for updating the patch. I am okay with this patch which unblocks delete of keys with special chars.

[sreejasahithi]

The newly added testcase testDeleteKeyWithColonInFSOBucket fails in CI , I am fixing it.

[sreejasahithi]

I have fixed the testcase issue.

[adoroszlai]

Thanks @sreejasahithi for updating the patch.

[sadanand48]

Thanks @sreejasahithi for the patch, @adoroszlai @SaketaChalamchala @ivandika3 @sarvekshayr for the reviews and discussions.