Skip to content

Conversation

@kerneltime
Copy link
Contributor

What changes were proposed in this pull request?

When checking for admin privileges in the OM Upgrade path, the check is
only run against the short name representation of the UGI. The check
should be done against variations of the user name. Ideally, the check
should be against UGI Objects but the admin list created also takes in a
string representation of the user name via config.

What is the link to the Apache JIRA

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

How was this patch tested?

Local secure cluster with an upgrade finalization test.

When checking for admin privileges in the OM Upgrade path, the check is
only run against the short name representation of the UGI. The check
should be done against variations of the user name. Ideally, the check
should be against UGI Objects but the admin list created also takes in a
string representation of the user name via config.
Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @kerneltime. We will also need to do this in OzoneManagerStateMachine#preAppendTransaction, which is where the prepare request is checked for admin privileges since it will start blocking log entries there. We should probably add an admin check for OzoneManager#queryUpgradeFinalizationProgress as well.

@kerneltime
Copy link
Contributor Author

Thanks for the fix @kerneltime. We will also need to do this in OzoneManagerStateMachine#preAppendTransaction, which is where the prepare request is checked for admin privileges since it will start blocking log entries there. We should probably add an admin check for OzoneManager#queryUpgradeFinalizationProgress as well.

Fixing those in https://issues.apache.org/jira/browse/HDDS-6870
To keep cherry-picking simple and fixing this in a targetted manner keeping them separate.

@smengcl smengcl changed the title HDDS-6869 Use UGI when checking Upgrade priv HDDS-6869. Use UGI when checking Upgrade priv Jun 10, 2022
Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @kerneltime. I will follow #3503 for the next round of updates.

@errose28 errose28 merged commit b0b3f0d into apache:master Jun 13, 2022
duongkame pushed a commit to duongkame/ozone that referenced this pull request Aug 16, 2022
Change-Id: Ia49b9dc40721dc475cec41164f42a9692a80e91a
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