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(login): auto-retry login with existing credentials #251

Merged
merged 7 commits into from
Aug 30, 2021

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Aug 26, 2021

Fixes #250

This PR enhances the login flow.

To test, start the backend as follows:

$ mkdir conf
$ echo "user:d74ff0ee8da3b9806b18c877dbf29bbde50b5bd8e4dad7a3a725000feb82e8f1" > conf/cryostat-users.properties
$ # the line above creates a Basic auth user with name "user" and password "pass"
$ CRYOSTAT_IMAGE=quay.io/andrewazores/cryostat:periodic-disconnect-ws CRYOSTAT_AUTH_MANAGER=io.cryostat.net.BasicAuthManager CRYOSTAT_CORS_ORIGIN=http://localhost:9000 CRYOSTAT_DISABLE_SSL=true CRYOSTAT_DISABLE_JMX_AUTH=true sh smoketest.sh

Start the frontend as usual:

$ yarn yarn:frzinstall && yarn start:dev

You should be presented with the Basic auth login form. Enter user/pass credentials and log in. Pay attention to the graphical notifications - there should be one that the connection was accepted here. After ~10s pass, the backend will terminate the WebSocket connection. A graphical notification should be displayed here as well, and you should be returned to the graphical login page momentarily. The login page should have the username and password fields pre-filled. After a very short delay, the login page should disappear and you should be returned back to the in-app page you were on previously. ~10s more later and you should again be disconnected. This will loop indefinitely. The web client should continuously reconnect itself after the connection is lost.

Next, attempt the same procedure as above. However, some time after the first successful web client connection is established, you should forcibly kill the Cryostat backend (Ctrl-C). This should cause the web client to display a disconnection notification and the login page again. Wait some time (and observe in your browser's dev tools network tab that the client is attempting to reconnect via POST /api/v1/auth), then restart the Cryostat backend using the same command as the first time. Within a short time (Cryostat backend startup + ~5s or less), the web client should automatically reconnect to the newly restarted backend instance.

Next, retry the above two procedures, but without setting the CRYOSTAT_AUTH_MANAGER env var when starting the backend. This will default it to the NoopAuthManager, which does not require any authentication credentials. This test case should behave very similarly to the first case with Basic authentication enabled, however the delay between disconnect and successful reconnect is likely to be much shorter.

Finally, the above scenarios can also be tested in CRC to exercise the Bearer authentication login flow. This should behave identically to the Basic authentication flow, albeit with only a single form field to fill, and perhaps a slower reconnection time due to a longer request latency to validate your supplied credentials.

@andrewazores andrewazores added the feat New feature or request label Aug 26, 2021
@andrewazores andrewazores requested review from ebaron and jan-law August 26, 2021 20:56
@andrewazores andrewazores force-pushed the reconnect-ws-termination branch 2 times, most recently from 7759efb to ac17de4 Compare August 26, 2021 21:46
@jan-law
Copy link
Contributor

jan-law commented Aug 26, 2021

I started the backend first, then the frontend with the above commands. When I enter "user" and "pass", the cryostat logs show a 401 and a warning about cryostat-user.properties. Am I missing a setup step?

...
INFO: (10.0.2.100:35046): GET /api/v1/grafana_dashboard_url 200 0ms
Aug 26, 2021 9:46:23 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:35046): OPTIONS /api/v1/auth 200 0ms
Aug 26, 2021 9:46:23 PM io.cryostat.core.log.Logger warn
WARNING: User properties file "/opt/cryostat.d/conf.d/cryostat-users.properties" does not exist
Aug 26, 2021 9:46:23 PM io.cryostat.core.log.Logger warn
WARNING: Exception thrown
io.vertx.ext.web.handler.impl.HttpStatusException: Unauthorized

Aug 26, 2021 9:46:23 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:35042): POST /api/v1/auth 401 2ms

@andrewazores
Copy link
Member Author

andrewazores commented Aug 26, 2021

Oh, my mistake - you should also check out cryostatio/cryostat#664 for the backend. No need to build it since you're running an image I pushed to quay already which was built from this branch (plus a tweak to terminate connections periodically, which isn't in the branch). There is a small change to run.sh that you will need there so that the user properties file is picked up. I think that should fix it.

@andrewazores andrewazores force-pushed the reconnect-ws-termination branch from ac17de4 to 62691a2 Compare August 26, 2021 22:34
@jan-law
Copy link
Contributor

jan-law commented Aug 27, 2021

How do I resolve an accessDeniedException at /opt/cryostat.d/conf.d/credentials?

 ~/cryostat (single-conf-dir) [1]> git fetch aazores
 ~/cryostat (single-conf-dir) [1]> mvn clean package
 ~/cryostat (single-conf-dir) [1]> mkdir conf && echo "user:d74ff0ee8da3b9806b18c877dbf29bbde50b5bd8e4dad7a3a725000feb82e8f1" > conf/cryostat-users.properties 
~/cryostat (single-conf-dir) [1]> CRYOSTAT_IMAGE=quay.io/andrewazores/cryostat:periodic-disconnect-ws CRYOSTAT_AUTH_MANAGER=io.cryostat.net.BasicAuthManager CRYOSTAT_CORS_ORIGIN=http://localhost:9000 CRYOSTAT_DISABLE_SSL=true CRYOSTAT_DISABLE_JMX_AUTH=true sh smoketest.sh
...
Aug 26, 2021 9:58:52 PM io.cryostat.core.log.Logger info
INFO: Local config path set as /opt/cryostat.d/conf.d
Exception in thread "main" java.lang.RuntimeException: java.nio.file.AccessDeniedException: /opt/cryostat.d/conf.d/credentials
	at io.cryostat.configuration.ConfigurationModule.provideCredentialsManager(ConfigurationModule.java:92)
	at io.cryostat.configuration.ConfigurationModule_ProvideCredentialsManagerFactory.provideCredentialsManager(ConfigurationModule_ProvideCredentialsManagerFactory.java:51)
	at io.cryostat.configuration.ConfigurationModule_ProvideCredentialsManagerFactory.get(ConfigurationModule_ProvideCredentialsManagerFactory.java:40)
	at io.cryostat.configuration.ConfigurationModule_ProvideCredentialsManagerFactory.get(ConfigurationModule_ProvideCredentialsManagerFactory.java:12)
	at dagger.internal.DoubleCheck.get(DoubleCheck.java:47)
	at io.cryostat.DaggerCryostat_Client.credentialsManager(DaggerCryostat_Client.java:591)
	at io.cryostat.Cryostat.main(Cryostat.java:74)
Caused by: java.nio.file.AccessDeniedException: /opt/cryostat.d/conf.d/credentials
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:90)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:116)
	at java.base/sun.nio.fs.UnixFileSystemProvider.createDirectory(UnixFileSystemProvider.java:389)
	at java.base/java.nio.file.Files.createDirectory(Files.java:690)
	at io.cryostat.configuration.ConfigurationModule.provideCredentialsManager(ConfigurationModule.java:82)
	... 6 more
+ cleanup
+ podman pod kill cryostat

@andrewazores andrewazores force-pushed the reconnect-ws-termination branch from 62691a2 to 7fb7c48 Compare August 27, 2021 16:14
@jan-law
Copy link
Contributor

jan-law commented Aug 27, 2021

the above scenarios can also be tested in CRC

I have a running cluster, logged in with oc login -u kubeadmin https://api.crc.testing:6443 , then tried starting the operator and get this error after make deploy. What is causing this error?
http://pastebin.test.redhat.com/990119

Error from server (NotFound): error when creating "STDIN": namespaces "cryostat-operator-system" not found
make: *** [Makefile:115: deploy] Error 1

Also, how can I pass the above env vars to cryostat in CRC?

@andrewazores
Copy link
Member Author

I think for the first part, you wan to do something like DEPLOY_NAMESPACE=$(oc project -q) make deploy, so that the namespace deployed into is the one you're currently working in (ex. default or myproject).

For passing env vars to Cryostat running in CRC, I don't think there is an existing way to set the CORS variable at least - it wasn't really put together with that in mind. You might need to edit resource_definitions.go manually to set these values and then build the operator with those hardcoded for this test. You wouldn't set the AUTH_MANAGER though, if you're running in CRC you would want it to use platform detection and use the OpenShiftAuthManager to exercise the Bearer auth flow.

@ebaron is there any easier way to do the above?

@andrewazores
Copy link
Member Author

Actually, hold on. Even if you build the operator with the CORS variable hardcoded in, that may not work, because you'll still be behind the cluster's SSL by default, and that'll break the CORS and separate frontend setup too. Hmm.

Easiest way to test in CRC may be to just build a custom Cryostat image with this frontend PR checked out in the web-client submodule, and then have the standard operator deploy that customized image by deploying the cryostat-operator-controller-manager deployment object.

@andrewazores
Copy link
Member Author

I have just pushed quay.io/andrewazores/cryostat:reconnect-ws that you should be able to use to test this in CRC. It contains this PR for the frontend, as well as the modified backend that terminates connections every 10s.

@jan-law
Copy link
Contributor

jan-law commented Aug 27, 2021

When I go to https://cryostat-sample-default.apps-crc.testing, I don't see the notifications icon or the gear icon. Is there another way to use your reconnect-ws image?

$ crc start
$ kubectl create namespace cryostat-operator-system
$ make cert_manager
$ make deploy
$ make create_cryostat_cr
    kubectl delete -f config/samples/operator_v1beta1_cryostat.yaml
    cryostat.operator.cryostat.io "cryostat-sample" deleted
$ oc new-app --docker-image=quay.io/andrewazores/cryostat:reconnect-ws
...
--> Success
    Application is not exposed. You can expose services to the outside world by executing one or more of the commands below:
     'oc expose service/cryostat' 
    Run 'oc status' to view your app.
$ oc expose service/cryostat
route.route.openshift.io/cryostat exposed
$ oc status
...
https://cryostat-sample-default.apps-crc.testing (reencrypt) to pod port 8181 (svc/cryostat-sample)
...

image

@andrewazores
Copy link
Member Author

andrewazores commented Aug 27, 2021

I think what your setup has there is an operator-deployed Cryostat instance and then another one running separately via new-app. The cryostat-sample URL would probably go to the operator-deployed one, which is running a different backend image. The other one that you deployed by new-app may or may not be running correctly - it might be missing some other required configuration, not sure. If it is running then I would expect its URL to just be cryostat-default.apps-crc.testing or something, not cryostat-sample, since when you did new-app the resource name would probably just be cryostat (like it was for the service and therefore I think the route as well).

I would do oc edit deployment cryostat-operator-controller-manager and edit the RELATED_CORE_IMAGE (I think that's what it's called) to change the image tag that the operator deploys. After you do this, watch oc get all should show you that the operator sees the change and destroys/recreates the Cryostat container instance to replace it with the version running the reconnect-ws image. Then you can go back to that cryostat-sample-default.apps-crc.testing URL.

@jan-law
Copy link
Contributor

jan-law commented Aug 27, 2021

After editing the yaml, the cryostat-sample-default testing URL looks the same as before. Here's the output of oc get all and oc status: http://pastebin.test.redhat.com/990152

$ oc edit deployment cryostat-operator-controller-manager
env:
	- name: RELATED_IMAGE_CORE
          value: quay.io/andrewazores/cryostat:reconnect-ws

@andrewazores
Copy link
Member Author

3. pod/cryostat-6fc44fb7d5-xbd6n 0/1 CrashLoopBackOff that doesn't look good. Can you do an oc describe pod/cryostat-6fc44fb7d5-xbd6n and/or oc get -o yaml pod/cryostat-6fc44fb7d5-xbd6n to find out why it's failing to start the pod?

@ebaron
Copy link
Member

ebaron commented Aug 27, 2021

@jan-law I think maybe that operator image is too old. I know we had to add some new things to support v2 Cryostat.

@ebaron
Copy link
Member

ebaron commented Aug 27, 2021

This worked as expected for me, but is the disconnecting every 10s something temporary to illustrate this behaviour?

@andrewazores
Copy link
Member Author

andrewazores commented Aug 27, 2021 via email

Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good then!

@andrewazores andrewazores merged commit 35848ee into cryostatio:main Aug 30, 2021
@andrewazores andrewazores deleted the reconnect-ws-termination branch August 30, 2021 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI should attempt to reconnect after WebSocket connection terminations
3 participants