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

Enable PSK by default, correct link to IDE #3963

Merged
merged 6 commits into from
Jun 21, 2023

Conversation

niloc132
Copy link
Member

@niloc132 niloc132 commented Jun 9, 2023

Also generates a random key when an empty key is provided.

Tested from gradle and docker-compose, but we'll certainly need more testing and verification before shipping this.

props/configs/src/main/resources/dh-defaults.prop Outdated Show resolved Hide resolved
@@ -14,7 +14,7 @@ services:
# with max memory.
#
# To turn on debug logging, add: -Dlogback.configurationFile=logback-debug.xml
- START_OPTS=-Xmx4g -Ddeephaven.console.type=${DEEPHAVEN_CONSOLE_TYPE} -Ddeephaven.application.dir=${DEEPHAVEN_APPLICATION_DIR}
- START_OPTS=-Xmx4g -Ddeephaven.console.type=${DEEPHAVEN_CONSOLE_TYPE} -Ddeephaven.application.dir=${DEEPHAVEN_APPLICATION_DIR} -Dauthentication.psk=${PSK}
Copy link
Member

Choose a reason for hiding this comment

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

Env var should probably be named DEEPHAVEN_PSK for consistency

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems reasonable, my goal here was to keep the invocation short to default to something simple:

➜  core.new git:(psk-default) ✗ PSK=1234 docker-compose up
[+] Building 0.0s (0/0)                                                                                                                                                                                                                                                                                                                                                                                                                                                                 
[+] Running 1/0
 ✔ Container core-server-1  Created                                                                                                                                                                                                                                                                                                                                                                                                                                                0.1s 
Attaching to core-server-1
core-server-1  | 1 compiler directives added
core-server-1  | VM settings:
core-server-1  |     Max. Heap Size: 4.00G
core-server-1  |     Using VM: OpenJDK 64-Bit Server VM
core-server-1  | 
core-server-1  | # Starting io.deephaven.server.jetty.JettyMain
...
core-server-1  | 2023-06-15T19:18:53.880Z | main                 |  INFO | d.s.a.ApplicationInjector |  found 0 exports
core-server-1  | 2023-06-15T19:18:53.880Z | main                 |  INFO | .d.s.r.DeephavenApiServer | Initializing Authentication...
core-server-1  | 2023-06-15T19:18:53.881Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.881Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.881Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.881Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.881Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.881Z | main                 |  WARN | .PskAuthenticationHandler | ================================================================================
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  WARN | .PskAuthenticationHandler | Superuser access through pre-shared key is enabled - use 1234 to connect
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  WARN | .PskAuthenticationHandler | Connect automatically to Web UI with http://localhost:10000/?psk=1234
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  WARN | .PskAuthenticationHandler | ================================================================================
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  WARN | .PskAuthenticationHandler | 
core-server-1  | 2023-06-15T19:18:53.882Z | main                 |  INFO | .d.s.r.DeephavenApiServer | Starting server...
core-server-1  | 2023-06-15T19:18:53.884Z | main                 |  INFO | o.e.jetty.server.Server   | jetty-11.0.15; built: 2023-04-11T18:37:53.775Z; git: 5bc5e562c8d05c5862505aebe5cf83a61bdbcb96; jvm 17.0.7+7

For the "use this docker-compose.yml as a template", i would agree that something like DEEPHAVEN_PSK may be preferred. I'll take your guidance here, just offering my reasoning.

@devinrsmith
Copy link
Member

niloc132#6

@niloc132 niloc132 marked this pull request as ready for review June 16, 2023 19:14
Copy link
Member

@rcaudy rcaudy left a comment

Choose a reason for hiding this comment

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

I agree with Mike about the env var naming. No other comments at this time.

@niloc132 niloc132 merged commit 14e7e28 into deephaven:main Jun 21, 2023
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2023
@deephaven-internal
Copy link
Contributor

Labels indicate documentation is required. Issues for documentation have been opened:

How-to: https://github.com/deephaven/deephaven.io/issues/2802
Reference: https://github.com/deephaven/deephaven.io/issues/2803

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants