-
Notifications
You must be signed in to change notification settings - Fork 89
Fix follower error handling when leader returns invalid compatibility… #1166
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
base: main
Are you sure you want to change the base?
Fix follower error handling when leader returns invalid compatibility… #1166
Conversation
|
@harishva23 I see a conflict. Can you pls fix ? |
Hi @muralibasani , the commit for #1140 has been reverted after I raised the PR. Therefore I shouldn't include that changes in my PR right? |
Looks ok now. |
|
@harishva23 some lint issues. |
Oh ok.. I will fix that whenever I am free and I will drop a comment when finished. |
|
Hi @muralibasani due to an issue with a Github config, the commits are being duplicated . I will try resolving this issue when i can so that the branch is clean and then all Github actions can be run. Sorry about that. |
3b0cfe0 to
1f3b517
Compare
|
Hi @muralibasani I have fixed the lint issues using the pre-commit tool and I have ran my tests locally with all cases passing. |
1f3b517 to
7dfdd23
Compare
|
@harishva23 I think there is another lint issue https://github.com/Aiven-Open/karapace/actions/runs/19358394691/job/55384303252?pr=1166#step:7:1834 |
About this change - What it does
This PR fixes 2 issues - error handling inside the follower and error handling for file-based basic authentication
Earlier, the follower could not parse error messages from leader if write requests were made to the follower and it failed due to some error like incompatible or invalid schema , trying to delete a non-existent schema etc. Now this is able to handle these error messages and pass it on to the user or client.
In the file-based authentication, if a non-existent user was sent in the Headers ( a username not in auth file), karapace threw an Internal Server Error instead of Unauthorized message. This did not happen when a correct username but wrong password was passed. So I have fixed this issue and it returns an Unauthorized error message for both the cases.
References: #1154
Why this way
Since Confluent client always expects a FastAPI response, I have changed the code to return proper format of error messages from follower. This leads to consistent error messages from both leader and follower so that the user can decide the next steps.
For the file-based authentication, raising a generic ValueError results in Internal Server Error being thrown and a full stacktrace appears in the logs which reveals that User is not found. Since karapace already throws a Unauthorised Error if wrong password is sent, I have removed the logic raising ValueError and instead returning None type which is then handled in the auth.py file in the authenticate function.
I have also included test cases for forwarding client as well as the basic authentication tests as well. For the follower client, I have patched the SSL context creation for the tests.