-
Notifications
You must be signed in to change notification settings - Fork 893
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
Fixed Auth with v2 protocol #1805
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.
Looks good, apart from my question on the test case.
The very interesting fact is that when I worked on auth only Yahoo had auth in place and they used v2 :)
bookkeeper-server/src/test/java/org/apache/bookkeeper/auth/TestAuthMixed.java
Outdated
Show resolved
Hide resolved
bookkeeper-server/src/test/java/org/apache/bookkeeper/auth/TestAuthMixed.java
Outdated
Show resolved
Hide resolved
@eolivelli I think this got broken (in v2) after the changes to remove the protobuf extension mechanism for the auth message. Core problem is the test was not running with v2. |
### Motivation BK auth framework is currently broken when using v2 protocol. ### Changes * Fixed auth when using V2 protocol * Made sure a client with authentication enabled can talk to a bookie without authentication. This is required in any case when enabling/disabling authentication on a live cluster. * Run all auth tests against both v2 and v3 protocol. This should be included in 4.7.2 to give a path to upgrade. cc/ rdhabalia Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org> This closes #1805 from merlimat/fix-v2-auth (cherry picked from commit dc2aaaa) Signed-off-by: Sijie Guo <sijie@apache.org>
### Motivation BK auth framework is currently broken when using v2 protocol. ### Changes * Fixed auth when using V2 protocol * Made sure a client with authentication enabled can talk to a bookie without authentication. This is required in any case when enabling/disabling authentication on a live cluster. * Run all auth tests against both v2 and v3 protocol. This should be included in 4.7.2 to give a path to upgrade. cc/ rdhabalia Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org> This closes #1805 from merlimat/fix-v2-auth (cherry picked from commit dc2aaaa) Signed-off-by: Sijie Guo <sijie@apache.org>
merged to master, branch-4.8, and branch 4.7 |
Motivation
BK auth framework is currently broken when using v2 protocol.
Changes
This should be included in 4.7.2 to give a path to upgrade.
cc/ @rdhabalia