Skip to content

Conversation

@xBis7
Copy link
Contributor

@xBis7 xBis7 commented Apr 7, 2023

Description of PR

This patch will help us solve a FairCallQueue impersonation issue we have on the Apache Ozone project.

The issue: On Ozone the FairCallQueue doesn't work when used with the Ozone S3G. The OzoneManager and the S3G are using a single permanent connection for communication, which is hiding all S3G client users under a special S3G user. For every request that comes through the S3G, getUserGroupInformation() from the Schedulable returns the special s3g user. Therefore, there is no impersonation and the FairCallQueue is ineffective.

We would like to expose the CallerContext field from the Call class and use that to carry the information needed by the IdentityProvider. On Ozone, we will set the correct information on the CallerContext before issuing the request and then provide an IdentityProvider implementation which will access the new CallerContext getter and use it to return the username to makeIdentity().

Here is a draft patch on Ozone side that utilizes these changes: apache/ozone#4116

hadoop jira: https://issues.apache.org/jira/browse/HADOOP-18691

How was this patch tested?

No new tests were added for this patch as it doesn't make any functional changes but only exposing existing data. It was tested on Ozone side.

Here is a gist with steps and information on how the FairCallQueue was tested using this patch and an OzoneIdentityProvider impl: https://gist.github.com/xBis7/b8247986e718417a4b48320eab6efeda

@hadoop-yetus

This comment was marked as outdated.

@hadoop-yetus

This comment was marked as outdated.

@hadoop-yetus

This comment was marked as outdated.

@smengcl smengcl requested a review from steveloughran April 13, 2023 16:36
@smengcl
Copy link
Contributor

smengcl commented Apr 13, 2023

Thanks @xBis7 . lgtm apart from one comment above.

I'd like to invite @steveloughran to vet the interface method addition here as well.

@xBis7
Copy link
Contributor Author

xBis7 commented Apr 13, 2023

@smengcl Thanks for looking into this PR. I've addressed your comment.

@hadoop-yetus

This comment was marked as outdated.

Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

lgtm. Pending CI

@xBis7
Copy link
Contributor Author

xBis7 commented Apr 15, 2023

@smengcl Thanks for the review!

@hadoop-yetus

This comment was marked as outdated.

@xBis7
Copy link
Contributor Author

xBis7 commented Apr 15, 2023

@steveloughran Thanks for reviewing this PR. I have made the changes you requested. Let me know how the patch looks now.

@hadoop-yetus

This comment was marked as outdated.

@hadoop-yetus

This comment was marked as outdated.

@hadoop-yetus

This comment was marked as outdated.

@smengcl
Copy link
Contributor

smengcl commented Apr 18, 2023

Thanks @xBis7 . Latest changes lgtm. Looks like all comments from @steveloughran are addressed.

I will merge after a few days or when Steve approves this.

@smengcl smengcl merged commit 9e24ed2 into apache:trunk Apr 20, 2023
@smengcl
Copy link
Contributor

smengcl commented Apr 20, 2023

Thanks @xBis7 for the PR.

@smengcl
Copy link
Contributor

smengcl commented Apr 20, 2023

Backported to branch-3.3 (3.3.9): 57ff8bd

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.

4 participants