Skip to content
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

feat(proxy): websocket proxying, CSRF handling #85

Merged
merged 13 commits into from
Jan 31, 2025

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Jan 30, 2025

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Depends on cryostatio/cryostat-web#1544
Depends on cryostatio/cryostat-web#1547

Description of the change:

Combine with #1544 cryostatio/cryostat-web#1547. This should allow the plugin to make all HTTP API requests. Previously, the lack of CSRF handling meant only GET requests would get through to the backend. Now, all requests like PATCH or POST should also get through. This fixes up GraphQL in particular, so the Archives view can be used to test that.

The plugin UI does strange de-selection of the Cryostat instance, so it can be annoying to navigate around. Separate bug to be tackled. Re-selecting the same Cryostat instance should re-draw the proper routed view anyway, so it takes a few steps to get through an action. Go to Recordings and start a new recording on some target (ex. cryostat-operator make sample_app). Once the recording appears in the active recordings table, select it and use the Stop or Delete buttons. If the UI updates accordingly then the WebSocket notifications must be coming through. Check your browser's devtools network tab and ensure that there are HTTP 101 connections going to console URLs with paths like /upstream/api/v4/notifications?ns=foo&name=mycryostat, and none going to other hosts like localhost, and that one such connection is successfully established whenever the Cryostat instance context selection is changed.

aptmac
aptmac previously approved these changes Jan 30, 2025
@andrewazores
Copy link
Member Author

@aptmac ready for re-review along with cryostatio/cryostat-web#1547

@andrewazores andrewazores merged commit fc28e8c into cryostatio:main Jan 31, 2025
7 checks passed
@andrewazores andrewazores deleted the proxy-ws branch January 31, 2025 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants