You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/06/17 02:13:35 UTC
[GitHub] [ozone] kerneltime opened a new pull request, #3527: HDDS-6868 Add S3Auth information to thread local
kerneltime opened a new pull request, #3527:
URL: https://github.com/apache/ozone/pull/3527
## What changes were proposed in this pull request?
For operations that are not read only we should
set the thread local S3 Auth.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6868
## How was this patch tested?
Before patch.
This was manually tested, we need to improve our secure S3 testing and will file a separate jira to improve our test cases. This is an urgent fix that needs to get in.
```
mc: <ERROR> Failed to copy `/etc/hosts`. Insufficient permissions to access this file `https://localhost:9879/bucket/key`
```
After patch the object could be uploaded to S3.
--
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 a diff in pull request #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
kerneltime commented on code in PR #3527:
URL: https://github.com/apache/ozone/pull/3527#discussion_r899904305
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java:
##########
@@ -183,6 +183,9 @@ private OMResponse processRequest(OMRequest request) throws
OzoneManagerRatisUtils.checkLeaderStatus(ozoneManager);
}
try {
+ if (request.hasS3Authentication()) {
+ ozoneManager.setS3Auth(request.getS3Authentication());
+ }
Review Comment:
Yes, makes sense. Will make the change.
--
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 a diff in pull request #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3527:
URL: https://github.com/apache/ozone/pull/3527#discussion_r899825821
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java:
##########
@@ -183,6 +183,9 @@ private OMResponse processRequest(OMRequest request) throws
OzoneManagerRatisUtils.checkLeaderStatus(ozoneManager);
}
try {
+ if (request.hasS3Authentication()) {
+ ozoneManager.setS3Auth(request.getS3Authentication());
+ }
Review Comment:
I think the same is missing for write case in `submitRequestDirectlyToOM`.
Instead of repeating it in 4 places (read/write * ratis/non-ratis), wouldn't it be simpler to wrap the main flow of `processRequest` in a single `try-finally` to set/clear auth?
--
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] errose28 merged pull request #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
errose28 merged PR #3527:
URL: https://github.com/apache/ozone/pull/3527
--
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 #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3527:
URL: https://github.com/apache/ozone/pull/3527#issuecomment-1161680446
> @kerneltime Thanks for tweaking the patch. However,`submitRequestDirectlyToOM` is still not fixed.
Not sure why it was skipped originally, fixed it.
--
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 #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3527:
URL: https://github.com/apache/ozone/pull/3527#issuecomment-1162268518
> > I believe our acceptance tests cover S3 operations reasonably well, both unsecure and secure environment. [HDDS-6868](https://issues.apache.org/jira/browse/HDDS-6868) description shows that the bug happens even with a very basic "upload a file" step. This is definitely covered by the tests. However, those tests are executed as an admin user (`testuser`). I tried `testuser2` (which I think is non-admin), but the tests still passed. Another problem might be that in docker-based environments all servers are run as the same user (`hadoop`).
> > I would like to understand the specific environmental difference that exposes this bug.
>
> I would like to understand the problem better as well, as the upload condition passes our acceptance tests and I cannot see clearly why the error occurs otherwise. To be able to describe the exact difference between our CI tests condition and the condition the error is seen would be great. Also it would help us update our CI workflow if needed.
I have root caused the issue with the smoke tests and am working on the fix for it, it will take a couple of days to iron out all the tests.
--
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 #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3527:
URL: https://github.com/apache/ozone/pull/3527#issuecomment-1160138084
@kerneltime Thanks for tweaking the patch. However,`submitRequestDirectlyToOM` is still not fixed.
--
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] neils-dev commented on pull request #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
neils-dev commented on PR #3527:
URL: https://github.com/apache/ozone/pull/3527#issuecomment-1162155002
> I believe our acceptance tests cover S3 operations reasonably well, both unsecure and secure environment. [HDDS-6868](https://issues.apache.org/jira/browse/HDDS-6868) description shows that the bug happens even with a very basic "upload a file" step. This is definitely covered by the tests. However, those tests are executed as an admin user (`testuser`). I tried `testuser2` (which I think is non-admin), but the tests still passed. Another problem might be that in docker-based environments all servers are run as the same user (`hadoop`).
>
> I would like to understand the specific environmental difference that exposes this bug.
I would like to understand the problem better as well, as the upload condition passes our acceptance tests and I cannot see clearly why the error occurs otherwise. To be able to describe the exact difference between our CI tests condition and the condition the error is seen would be great. Also it would help us update our CI workflow if needed.
--
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 #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3527:
URL: https://github.com/apache/ozone/pull/3527#issuecomment-1161803981
Thanks @kerneltime for updating the patch. `processRequest` looks better, but S3 auth-related steps can be removed from `submitRequestDirectlyToOM`.
https://github.com/apache/ozone/blob/7f334061003ea29b682afa3bf172f9c2ade6be95/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java#L275-L279
https://github.com/apache/ozone/blob/7f334061003ea29b682afa3bf172f9c2ade6be95/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java#L281-L284
https://github.com/apache/ozone/blob/7f334061003ea29b682afa3bf172f9c2ade6be95/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java#L286-L288
--
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 #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3527:
URL: https://github.com/apache/ozone/pull/3527#issuecomment-1162277640
> submitRequestDirectlyToOM
Thanks, @adoroszlai for the thorough review I missed that initially. Posting an update.
--
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 #3527: HDDS-6868. Add S3Auth information to thread local
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3527:
URL: https://github.com/apache/ozone/pull/3527#issuecomment-1158640093
I have been trying to root cause why the tests pass. I think I know why s3g as a user gets treated as root, will continue debugging further, hopefully this is just an issue with the docker environment. More details tomorrow after I try changing the test environment. Right now, it looks like UGI setup lands up sneaking in root as the short username which leads to acl checks passing.
The manual tests were run using mc as AWS cli and it used systest user in a real cluster.
We should merge this fix in and I will post a fix for our tests.
--
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