Skip to content

Conversation

@smengcl
Copy link
Contributor

@smengcl smengcl commented May 12, 2021

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

Note: this commit changes the username set inside OFS .Trash directory when Kerberos is enabled from principal name to mapped shorted user name.

Copy link
Contributor

@xiaoyuyao xiaoyuyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @smengcl for working on this. The change LGTM. I just have one question: how do we handle the existing Trash location with full principal name? If it is not safe to do a rename, we should document how to migration after the change.

@smengcl
Copy link
Contributor Author

smengcl commented May 21, 2021

Thanks @smengcl for working on this. The change LGTM. I just have one question: how do we handle the existing Trash location with full principal name? If it is not safe to do a rename, we should document how to migration after the change.

The existing Trash becomes inactive with the patch because the path has changed. I will document this in the markdown.

It's also a bit unfortunate that the full principal (typically has / in the name, testuser/scm@EXAMPLE.COM for example) creates two levels of user directory in .Trash. Though functionally it works just fine:

2021-05-21 18:20:00,170 [main] INFO fs.TrashPolicyDefault: Moved: 'ofs://om/vol2/buck2/README.md' to trash at: ofs://om/vol2/buck2/.Trash/testuser/scm@EXAMPLE.COM/Current/vol2/buck2/README.md

On a side note, the fundamental idea of this patch is the same as HDDS-5019. We are allowing user1/NODE1@EXAMPLE.COM and user2/NODE2@EXAMPLE.COM to be mapped to the same short user name, typically user1 in this case but configurable via hadoop.security.auth_to_local, when using the FileSystem (Trash in this case).

@xiaoyuyao
Copy link
Contributor

Thanks @smengcl for the contribution. The change LGTM, +1.

@xiaoyuyao xiaoyuyao merged commit e50fe52 into apache:master May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants