-
Notifications
You must be signed in to change notification settings - Fork 10
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(recordings): object storage URL may be different interally vs externally #234
feat(recordings): object storage URL may be different interally vs externally #234
Conversation
redirect client download requests using external storage URL, if any
currently failing due to 'signatures do not match' after the client request passes through the proxy to the s3 service
f628b72
to
9125b75
Compare
This PR/issue depends on:
|
/build_test |
Workflow started at 1/12/2024, 3:26:46 PM. View Actions Run. |
CI build and push: All tests pass ✅ |
1 similar comment
CI build and push: All tests pass ✅ |
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 to me!
Welcome to Cryostat3! 👋
Before contributing, make sure you have:
main
branch[chore, ci, docs, feat, fix, test]
To recreate commits with GPG signature
git fetch upstream && git rebase --force --gpg-sign upstream/main
Based on #235
Related to #2
Description of the change:
Adds optional configuration
storage-ext.url
, similar to existinggrafana-dashboard-ext.url
.Motivation for the change:
Cryostat uses this URL to determine the externally-facing URL where the object storage provider (S3 instance) can be reached by requesting clients, since Cryostat is likely to use an internal URL to communicate with the storage provider, but should tell clients how to download files from the object store using an externally routable URL.
How to manually test:
./mvnw clean package ; podman image prune -f
./smoketest.bash -O
http -v --follow --auth=user:pass :8080/api/v3/activedownload/1
to attempt downloading an active recording - see examples below.localhost:8080
in a browser, logging in withuser:pass
, going to Recordings, selecting Cryostat as the target, and attempting to download theonstart
recording. Open browser devtools network tab to view and verify request paths.http://localhost:8080/storage/$somepath
, wheresomepath
is an S3 presigned URL. Thelocalhost:8080/storage
prefix is the important part, indicating that the request is being routed through the auth proxy.Current difficulties:
1. S3 instances produce the presigned request URL using various parameters like the request URL and headers, which become encoded as part of the signature of the short-lived token for the resource. This is very similar to what we have implemented directly in 2.x. This means that using an internal URL for Cryostat to talk to S3, and an external URL for clients to talk to S3, does not meet the signature validation requirements and S3 rejects the client's request.2. Configuring Cryostat to talk to S3 via the external route would likely resolve the point above since then the request parameters can be the same, however in practice this would mean that Cryostat would need to go through the authentication proxy as well (the same as an external client does). This gets pretty messy since it's an additional set of credentials and authorization that need to be set up just for this reason. This might not be too bad in a real Kubernetes deployment since Cryostat should be able to use its own serviceaccount token to talk through the proxy. Maybe we can use
--skip-auth-regex
on the storage path so that the proxy only acts a gateway in front of S3 and does not perform authz duties? This means that the S3 provider must have its own authz for anything other than presigned URLs, but since our primary deployment target scenario is to use MinIO this should be okay since that does have its own robust authz mechanism that we can configure separately.Switching from Minio to SeaweedFS seems to have resolved the above. Seaweed tolerates the internal vs external URL, once it is configured with user basic auth to enable presigned downloads at all.
Content-Disposition
header set when redirecting clients to download active recordings. The new flow for this action in Cryostat 3 is that Cryostat actually pipes the JFR data out of the target JVM and uploads it to S3, rather than sending it back out to the client directly. Then Cryostat sends the client back a redirect response telling it to go get the file itself from S3 using a presigned URL, and aContent-Disposition
header that should inform the client (browser) what filename to use when saving this file, overriding the filename as it is found on the server. With Minio this works as expected, but with Seaweed the saved file retains the filename it has on the server. This ends up looking the same as any other archived recording file, which is not so bad, but the bugged behaviour is that this does not match the filename of the accompanying.metadata.json
file produced by the web-client. Potential fix: don't generate the.metadata.json
on the client, actually supply that straight from the server and just have the client download it. The filenames may end up looking like archived recording filenames, which I think is actually OK, and at least the JFR and metadata JSON files will have matching names.With Seaweed:
With Minio:
Note how in the Minio case, there is an initial
Content-Disposition
on the first redirect, the same on the second redirect, and none in the final response. Whereas in the Seaweed case, the first two redirect responses have the same header (since these come from Cryostat), and the final response from Seaweed also contains theContent-Disposition
header but with different contents. This is what causes the bug. I have patched some of the behaviour for this header as set in the two Cryostat redirect responses, as well as tried specifying that header's value in the object metadata when Cryostat uploads it to Seaweed, but to no avail. I've burned a few hours trying to work around this but keep triggering the presigned URL signature validation failure whenever I try something else, so this might just have to do for now.