-
Notifications
You must be signed in to change notification settings - Fork 496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDDS-8718. Kerberos authentication does not work for GrpcOmTransport #4843
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ivanzlenko for the patch.
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertNull; | ||
|
||
class S3SecretRequestHelperTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename to TestS3SecretRequestHelper
. Only tests that follow naming convention Test...
are run by Ozone's mvn test
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Renamed.
@ashishkumar50 @Tejaskriya please take a look |
Thanks @ivanzlenko for updating the patch. The new test is failing with:
https://github.com/ivanzlenko/ozone/actions/runs/5207802223/jobs/9395707878#step:6:2408 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ivanzlenko, Thanks for working on this. Whether getOrCreateUgi(accessId)
required even for S3SetSecret request as well?
return ugi; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add new line at the end of file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashishkumar50 yeah, it make sense. Thanks for pointing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashishkumar50 I can see new line in IDE though it's omitted in GitHub. Fixed unit tests and set s3 secret request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ivanzlenko for the patch, it works fine (tested in ozonesecure
env. TLS turned off).
@kerneltime @duongkame @neils-dev can you please take a look at this review? |
Thanks @adoroszlai ! |
What changes were proposed in this pull request?
Fixed issue where it is not possible to retrieve UGI for getSecret and revokeSecret requests while OM communicating with client using GPRC transport.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8718
How was this patch tested?
Tested with integration tests introduced in pull request for HDDS-8050