[Bug] Subviews like `/recordings/create` are not accessible when visited directly

tthvo commented 3 months ago

Current Behavior

If I directly visit views by specifying their paths such as:

/topology/create-custom-target
/rules/create
/recordings/create

For example, https://cryostat-sample-myns.apps-crc.testing/recordings/create. A blank page is showed:

with the following console's error:

Expected Behavior

The sub views should render correctly!

Steps To Reproduce

Launch an OpenShift instance. For example, with OpenShift Local, crc start.

On latest cryostat-operator's commit on main:

make deploy_bundle create_cryostat_cr sample_app

Environment

CRC version: 2.26.0+233df0
OpenShift version: 4.13.9
OS: Fedora 40

Anything else?

This only happens if I visit the sub-view directly. That is: If I go /recordings -> Click Create -> /recordings/create, all is fine.

andrewazores commented 3 months ago

Seems like going to /recordings/create makes the client try to look for the other asset bundles relative to /recordings instead of relative to /, so it tries to load /recordings/runtime.abcd1234.bundle.js instead of /runtime.abcd1234.bundle.js. Cryostat doesn't recognize this path so it responds with the index.html again, which results in the MIME type and SyntaxError errors.

Probably happening since https://github.com/cryostatio/cryostat-web/pull/1175 then.

andrewazores commented 3 months ago

Now that the standard Cryostat deployment includes an auth proxy, it might be possible to solve the original https://github.com/cryostatio/cryostat/issues/1810 using an auth proxy configuration plus Route/Ingress path configuration, and back out #1175.

andrewazores commented 3 months ago

This affects 2.4.1-snapshot too so I'm pretty sure #1175 is the culprit.

tthvo commented 3 months ago

Seems like going to /recordings/create makes the client try to look for the other asset bundles relative to /recordings instead of relative to /, so it tries to load /recordings/runtime.abcd1234.bundle.js instead of /runtime.abcd1234.bundle.js. Cryostat doesn't recognize this path so it responds with the index.html again, which results in the MIME type and SyntaxError errors. This affects 2.4.1-snapshot too so I'm pretty sure https://github.com/cryostatio/cryostat-web/pull/1175 is the culprit.

Right, make sense! That explained the logged error :(

Now that the standard Cryostat deployment includes an auth proxy, it might be possible to solve the original https://github.com/cryostatio/cryostat/issues/1810 using an auth proxy configuration plus Route/Ingress path configuration, and back out https://github.com/cryostatio/cryostat-web/pull/1175.

Just curious what's the plan for this fix?

andrewazores commented 3 months ago

Currently no plan, that was just a very rough idea. It would be nice to have the ability to go directly to paths like /recordings/create, but not if it comes at the expense of becoming impossible to deploy Cryostat instances on paths other than /.

tthvo commented 3 months ago

I am thinking if no errors with with the regular single level /recordings, we can make /recordings/create route into /create-recording for the sub view, basically flattening the route paths. That should help with the issue? Might not be the best UX.

andrewazores commented 3 months ago

That's one potential way around, but I think it's pretty limiting. It makes sense right now because none of the UI paths are parameterized, so applying such a transformation looks like it works. It wouldn't work anymore if we had any paths like /topology/:nodeId (just a random example), and adopting such a scheme would block us from ever having paths like that in the future.

tthvo commented 3 months ago

ah okayy pretty bad idea then :((

tthvo commented 3 months ago

well it would break if I configure istio to route to Cryostat with a 2-level prefix /tools/cryostat... Is there a way to webpack/web-client to figure this out other than relative path .?

Not very urgent I guess but just curious from my end as I am looking into the istio stack :D

andrewazores commented 3 months ago

Deploying Cryostat onto a path other than / isn't actually implemented yet, there are just a few pieces here and there that are working toward making that possible.

If there is some other router or reverse proxy in front of Cryostat (or in front of the auth proxy) then it should be possible to do this by having the router/proxy take requests like /foo/recordings, rewrite that to /recordings, and pass that request to the Cryostat container. By reconfiguring the router/proxy to change the /foo prefix, in theory this could work with / or /tools/cryostat or whatever path prefix is desired. This still won't work for /tools/cryostat/recordings/create though, because of the relative base href . issue we see here.

Backing out #1175 and adding in the router/proxy in front sounds like it could be a full solution to allow deployment to a non-root path as well as restoring direct links to deeper paths like /recordings/create.

If the auth proxy can be configured to do this, then great - we have our general-purpose upstream solution.

If it needs to be another proxy, either in front of the auth proxy or between the auth proxy and Cryostat, then we would need to have some more discussion about it. The additional container would need to be something we can disable, because if most users are not using this feature then it doesn't make sense to force them to deploy another container and pay for its footprint if it just ends up transparently passing requests.

tthvo commented 3 months ago

For the cryostat service we have a VirtualService ( istio ) that will redirect the traffic sent to : http://my-k8-host/cryostat/ to the cryostat service and together with that also rewriting the url to / ( simply removing cryostat )

Ahh okayy! As in https://github.com/cryostatio/cryostat/issues/1810, istio can allow rewriting back the path to / so I guess it can depend on the service mesh solution that the users choose and we do document it. Though, in the end, like u said, the bug with , because of the relative base href . issue we see here. will sill persist. I would expect webpack to have a way to figure it out but no idea :D

cryostatio / cryostat-web