You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "vtutrinov (via GitHub)" <gi...@apache.org> on 2023/08/29 20:51:14 UTC
[GitHub] [ozone] vtutrinov opened a new pull request, #5234: HDDS-9196: set thread local s3auth to stripes flusher
vtutrinov opened a new pull request, #5234:
URL: https://github.com/apache/ozone/pull/5234
## What changes were proposed in this pull request?
Set S3 credentials for the thread to flush stripes (EC output stream)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9196
## How was this patch tested?
manually, the test case is described in the Jira ticket
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vtutrinov commented on a diff in pull request #5234: HDDS-9196: set thread local s3auth to stripes flusher
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on code in PR #5234:
URL: https://github.com/apache/ozone/pull/5234#discussion_r1319996429
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java:
##########
@@ -367,10 +369,19 @@ public UserGroupInformation createUGI() {
omRequest.getUserInfo().getUserName());
return userGroupInformation;
} else {
- // This will never happen, as for every OM request preExecute, we
- // should add userInfo.
- return null;
+ throw new AuthenticationException("User info is not set."
+ + " Please check client auth credentials");
+ }
+ }
+
+ public UserGroupInformation createUGISafety() throws OMException {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on pull request #5234: HDDS-9196: set thread local s3auth to stripes flusher
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #5234:
URL: https://github.com/apache/ozone/pull/5234#issuecomment-1701894740
Thanks for working on this bug. OM should raise an exception when creating UGI.
```
public UserGroupInformation createUGI() {
```
A bad client should not lead to a NPE. Can you please add this to your PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #5234: HDDS-9196. Fix ECKeyOutputStream for secure S3
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5234:
URL: https://github.com/apache/ozone/pull/5234
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ivanzlenko commented on a diff in pull request #5234: HDDS-9196: set thread local s3auth to stripes flusher
Posted by "ivanzlenko (via GitHub)" <gi...@apache.org>.
ivanzlenko commented on code in PR #5234:
URL: https://github.com/apache/ozone/pull/5234#discussion_r1319963561
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java:
##########
@@ -367,10 +369,19 @@ public UserGroupInformation createUGI() {
omRequest.getUserInfo().getUserName());
return userGroupInformation;
} else {
- // This will never happen, as for every OM request preExecute, we
- // should add userInfo.
- return null;
+ throw new AuthenticationException("User info is not set."
+ + " Please check client auth credentials");
+ }
+ }
+
+ public UserGroupInformation createUGISafety() throws OMException {
Review Comment:
Javadoc required. Not sure if createUGISafety is a good name. createUGIForApi? Or something like that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vtutrinov commented on a diff in pull request #5234: HDDS-9196: set thread local s3auth to stripes flusher
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on code in PR #5234:
URL: https://github.com/apache/ozone/pull/5234#discussion_r1319953279
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/upgrade/OMFinalizeUpgradeRequest.java:
##########
@@ -67,7 +68,12 @@ public OMClientResponse validateAndUpdateCache(
try {
if (ozoneManager.getAclsEnabled()) {
- final UserGroupInformation ugi = createUGI();
+ UserGroupInformation ugi;
+ try {
+ ugi = createUGI();
+ } catch (AuthenticationException e) {
+ throw new RuntimeException(e);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ivanzlenko commented on a diff in pull request #5234: HDDS-9196: set thread local s3auth to stripes flusher
Posted by "ivanzlenko (via GitHub)" <gi...@apache.org>.
ivanzlenko commented on code in PR #5234:
URL: https://github.com/apache/ozone/pull/5234#discussion_r1319681169
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/upgrade/OMFinalizeUpgradeRequest.java:
##########
@@ -67,7 +68,12 @@ public OMClientResponse validateAndUpdateCache(
try {
if (ozoneManager.getAclsEnabled()) {
- final UserGroupInformation ugi = createUGI();
+ UserGroupInformation ugi;
+ try {
Review Comment:
Can we wrap it in another method, so we can prevent code duplication
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/upgrade/OMFinalizeUpgradeRequest.java:
##########
@@ -67,7 +68,12 @@ public OMClientResponse validateAndUpdateCache(
try {
if (ozoneManager.getAclsEnabled()) {
- final UserGroupInformation ugi = createUGI();
+ UserGroupInformation ugi;
+ try {
+ ugi = createUGI();
+ } catch (AuthenticationException e) {
+ throw new RuntimeException(e);
Review Comment:
Do not throw RuntimeException
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #5234: HDDS-9196. Fix ECKeyOutputStream for secure S3
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5234:
URL: https://github.com/apache/ozone/pull/5234#issuecomment-1715354669
Thanks @vtutrinov for the patch, @ivanzlenko, @kerneltime for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vtutrinov commented on pull request #5234: HDDS-9196: set thread local s3auth to stripes flusher
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on PR #5234:
URL: https://github.com/apache/ozone/pull/5234#issuecomment-1713854717
@kerneltime could you review the PR again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vtutrinov commented on pull request #5234: HDDS-9196: set thread local s3auth to stripes flusher
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on PR #5234:
URL: https://github.com/apache/ozone/pull/5234#issuecomment-1711360301
> Thanks for working on this bug. OM should raise an exception when creating UGI.
>
> ```
> public UserGroupInformation createUGI() {
> ```
>
> A bad client should not lead to a NPE. Can you please add this to your PR?
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vtutrinov commented on a diff in pull request #5234: HDDS-9196: set thread local s3auth to stripes flusher
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on code in PR #5234:
URL: https://github.com/apache/ozone/pull/5234#discussion_r1319952916
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/upgrade/OMFinalizeUpgradeRequest.java:
##########
@@ -67,7 +68,12 @@ public OMClientResponse validateAndUpdateCache(
try {
if (ozoneManager.getAclsEnabled()) {
- final UserGroupInformation ugi = createUGI();
+ UserGroupInformation ugi;
+ try {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org