apache
[ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API
What is this PR for?
Currently, configuration data is fetched through both REST API and WebSocket channels. However, the WebSocket path does not perform permission checks, and the only required data from it is the WebSocket max message size.
I extracted the websocket max message size field into a dedicated REST API, to improve security and simplify configuration handling.
What type of PR is it?
Improvement
Todos
- [ ] #5060
What is the Jira issue?
- Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN/6366
How should this be tested?
- Check the configuration page (/configuration)
- Check the notebook page (/notebook/{notebook_id})
Screenshots (if appropriate)
Questions:
- Does the license files need to update? N
- Is there breaking changes for older versions? N
- Does this needs documentation? N
@Reamer Looks reasonable for zeppelin-web too. Are there specific concerns you’re considering, such as maintenance or behavior differences?
@seung-00
(If we also apply this to zeppelin-web.)
There is a failing test in zeppelin-integration(test-selenium-with-spark-module-for-spark-3-5 CI job).
After a quick note: in zeppelin-web's NoteImportCtrl, wsMaxMessageSize is called even when the user is not logged in.
The server then returns a 302 redirect to /api/login (shiro.loginUrl). When the login API responds with 405, the HTTP interceptor emits session_logout (https://github.com/apache/zeppelin/blob/f1f671394dc742ec35ede32be039850cdf8a1145/zeppelin-web/src/app/app.js#L170). The login controller listens for session_logout and programmatically clicks the login button (.nav-login-btn).
As a result, the login modal pops up immediately when the window opens, which is different from before, and the integration test fails.
It looks like we should change this to issue the API request only when the user is authenticated, or defer it until the moment it is actually needed.
@Reamer Looks reasonable for zeppelin-web too. Are there specific concerns you’re considering, such as maintenance or behavior differences?
Yes both. All frontends should use the same interfaces. This makes it easier to remove things later on.
@Reamer In this case I applied it to both, as you mentioned, to make it easier to remove unreferenced things later on. But I don’t think we need to do that for every task. Covering both sides would be a bit tricky, since the front-end codebases are quite different.
@tbonelee I fixed the issue you mentioned. If the CI failed due to the login modal, I think the CI job might need to be adjusted instead, because the wsMaxMessageSize API doesn’t require authentication. However, I found that wsMaxMessageSize was being called and cached initially (same as before), but the field only needed in specific cases (e.g., exporting/importing note). So I modified it to be called only when required, now the CI error no longer occurs.
BTW, if the user isn’t logged in initially, showing the login modal seems more natural. 🤔