HDDS-14177. OFS#getTrashRoot should use the internal FS username instead of current UGI

[ivandika3]

What changes were proposed in this pull request?

BasicRootedOzoneFileSystem#getTrashRoot calls OFSPath#getTrashRoot which internally use UserGroupInformation#getCurrentUser. Therefore, even if FileSystem was initialized under impersonated user (e.g. proxy user), the trash root still uses the current UserGroupInformation#getCurrentUser.

We need fix getTrashRoot so that it uses the internal BasicRootedOzoneFileSystem.userName instead.

Note that BasicOzoneFileSystem#getTrashRoot (o3fs) already correctly uses the internal userName.

What is the link to the Apache JIRA

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

How was this patch tested?

Integration test. Clean run: https://github.com/ivandika3/ozone/actions/runs/20224410422

[ivandika3]

Another place that might need to be fixed is getTempMountBucketNameOfCurrentUser in OFSPath#initOFSPath where it currently always use the UserGroupInformation#getCurrentUser. However, I'm not familiar with the context, so need @smengcl's opinion.

[jojochuang]

@ivandika3 oh yeah that can break for /tmp path manipulation.

Hmm this is awkward. We would need to force OFSPath to accept a user name as a parameter, and the OFSPath usage is everywhere. I am thinking to add another OFSPath constructor to accept the "initialization user name", and abort if it turns out the path is a /tmp and that the initialization user name is not set. It's going to be awkward and error prone still.

[ivandika3]

@jojochuang Thanks for the review. Seems creating a new constructor will create a lot of changes (+ we need to change BasicRootedOzoneClientAdapterImpl constructor to also accept username).

We might need to address this in another task after some discussions.

[jojochuang]

Ok let's address the other API usage separately.

Currently OFSPath.getTrash() is used in TrashOzoneFileSystem which does not use impersonation or proxy user.

However IMO we should get rid of OFSPath.getTrashRoot() (no argument) otherwise it's going to be brittle if it is used elsewhere. Unless we can find another fool-proof solution.

[adoroszlai]

Thanks @ivandika3 for the patch.

nit: we can reduce duplication by changing getTrashRoot() to:

  public Path getTrashRoot() {
    return getTrashRoot(""); // or even null
  }

[ivandika3]

@adoroszlai Good point, thanks. Updated.

[adoroszlai]

Thanks @ivandika3 for the patch, @jojochuang for the review.