-
Notifications
You must be signed in to change notification settings - Fork 16
Handle Gateway links #289
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
Handle Gateway links #289
Conversation
The connect entrypoint is used both for Gateway links (jetbrains-gateway://) and internally in our own code at the end of the wizard and for recent connections, but this will be two separate sets of parameters (internally we have the CLI set up and pass around the hostname while the links will only have the workspace ID or name). So break out the code into a separate class that we can call internally which will let us dedicate the connect entrypoint to handle the Gateway links. There we will set up the CLI and gather the required parameters before calling the now-broken-out code.
So it can be reused in the link flow.
When you connect via a Gateway link it might not be obvious if the deployment URL is wrong.
Some exceptions have no message, like null pointer exceptions. Showing the class name seems more helpful than "no details", since it can save you a trip to the logs.
ab69920
to
11190fd
Compare
Qodana Community for JVM53 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2023.2.1
with:
upload-result: true Contact Qodana teamContact us at qodana-support@jetbrains.com
|
11190fd
to
809f5b2
Compare
src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/coder/gateway/sdk/CoderRestClientService.kt
Outdated
Show resolved
Hide resolved
Rather than asking the user to confirm. This only happens if we explicitly want to use an existing token anyway, and "existing" is defined in the help text as either the token on disk or one the user already copied, so the extra confirmation to use the token on disk seems unnecessary.
Also add a few checks against the status.
b379680
to
7735e47
Compare
7735e47
to
64fe78f
Compare
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.
👍
src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt
Outdated
Show resolved
Hide resolved
is SSLHandshakeException -> | ||
throw Exception(CoderGatewayBundle.message( | ||
"gateway.connector.view.workspaces.connect.ssl-error", | ||
url.host, | ||
e.message ?: CoderGatewayBundle.message("gateway.connector.view.workspaces.connect.no-reason") | ||
)) | ||
else -> throw e |
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.
This may cause some headaches if the required certs are not available in the JRE keystore. I think it should be fine if it's trusted by the system though.
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.
From my testing (although I might have missed something), adding to the system trust store was not enough, it had to be in the JRE key store that was bundled with Gateway.
The help text has a link to the docs that explains how to add the cert, but I do think it would be nice as a future enhancement to give the option to view and accept the cert. Not sure how difficult that would be.
The deployment host is probably not exactly where the IDEs would be stored if self-serving. Probably at most they would share a root domain. For now remove it, we can add it back or something like it back if we figure out some usage patterns, although making it configurable is probably the better bet anyway.
related to coder/coder#9679 |
I created a new class to share some common connection code between the step-based wizard flow and the link-based flow. I think we can share more things as we go on but I did not try too hard to combine more code until the shape of the link-based flow solidifies some more. So there is some duplication like the authentication loop, logging in the CLI, etc, that cannot be easily shared without some refactoring.
It might even end up being that we just always launch into the wizard flow but if some parameters are already set we automatically skip through certain steps. Not sure.
This is a first pass, right now you have to specify most of the parameters yourself (aside from deployment URL and token which, if missing, will trigger prompts to the user).
There are a few todos that I want to take in subsequent PRs to make the experience better, like making sure the workspace is on (and turning it on if needed) and giving the ability to select the IDE you want if one was not specified (or perhaps defaulting to the last used one).
The flow can be triggered with a URL like this:
For example, this should work if you run
gateway.sh <url>
. But this build of the plugin will need to be already installed since otherwise it will automatically install the currently published version which does not support the link yet.Or, if running Gateway from the
runIDE
task (which will build and provide the plugin automatically), pass the URL via--args
.Or, something like
xdg-open <url>
(on Linux) might work too.Or, adding it as an external link to a template.
More details in the commit messages, and they can be reviewed separately.
Closes #149.