-
Notifications
You must be signed in to change notification settings - Fork 24
feat: coder connect integration #482
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
Changes from 1 commit
95b5b4e
a65e550
2ecf1df
fb9a263
3a77138
2a3500e
9252fff
195151a
e7cad82
ea4b179
5e4e795
c3287eb
a2df5cc
feb1021
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,12 @@ | ||
import { isAxiosError } from "axios" | ||
import { Api } from "coder/site/src/api/api" | ||
import { getErrorMessage } from "coder/site/src/api/errors" | ||
import { User, Workspace, WorkspaceAgent } from "coder/site/src/api/typesGenerated" | ||
import { lookup } from "dns" | ||
import ipRangeCheck from "ip-range-check" | ||
import { promisify } from "util" | ||
import * as vscode from "vscode" | ||
import { makeCoderSdk, needToken } from "./api" | ||
import { fetchSSHConfig, makeCoderSdk, needToken } from "./api" | ||
import { extractAgents } from "./api-helper" | ||
import { CertificateError } from "./error" | ||
import { Storage } from "./storage" | ||
import { toRemoteAuthority, toSafeHost } from "./util" | ||
import { maybeCoderConnectAddr, toRemoteAuthority, toSafeHost } from "./util" | ||
import { OpenableTreeItem } from "./workspacesProvider" | ||
|
||
export class Commands { | ||
|
@@ -573,10 +569,10 @@ export class Commands { | |
// if the workspace is stopped, in which case we can't use Coder Connect | ||
// When called from `open`, the workspaceAgent will always be set. | ||
if (workspaceAgent) { | ||
let hostnameSuffix = "coder" | ||
let sshConfig | ||
try { | ||
// If the field was undefined, it's an older server, and always 'coder' | ||
hostnameSuffix = (await this.fetchHostnameSuffix()) ?? hostnameSuffix | ||
// Fetch (or get defaults) for the SSH config. | ||
sshConfig = await fetchSSHConfig(this.restClient, this.vscodeProposed) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The extension has an SSH config section which I assume is not going apply to Coder Connect, is that right? As well as anything the user has added to their SSH config manually. Wondering if we should update the setting description to say the variables will not apply if there is a Coder Connect session we can use. Speaking of settings, should using Coder Connect be behind a setting as well? Or a prompt? "We see Coder Connect, do you want to use that?" or similar. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Will add.
These will apply, as long as they're on the Coder Connect hostname (so, if they just specify those options when running
I don't think this is necessary, I can't imagine a scenario where the user would have Coder Connect on and not want to use it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I agree. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the tricky part is that the user might not realize it will use Connect. So, they have So there should be clear messaging what the plugin is going to do IMO, somewhere. Silently changing behavior feels bad to me. But, I am neither a VS Code nor Connect user, so take what I say with a grain of salt. 😆 Maybe the behavior change will be obvious to users. If we could make the same SSH settings apply either way, then that would be a different story. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With the updated Though if we change the approach to what I discussed below that solves this issue too ig There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is quite nice! |
||
} catch (error) { | ||
const message = getErrorMessage(error, "no response from the server") | ||
this.storage.writeToCoderOutputChannel(`Failed to open workspace: ${message}`) | ||
|
@@ -592,7 +588,7 @@ export class Commands { | |
workspaceAgent, | ||
workspaceName, | ||
workspaceOwner, | ||
hostnameSuffix, | ||
sshConfig.hostname_suffix, | ||
) | ||
if (coderConnectAddr) { | ||
remoteAuthority = `ssh-remote+${coderConnectAddr}` | ||
|
@@ -656,42 +652,6 @@ export class Commands { | |
reuseWindow: !newWindow, | ||
}) | ||
} | ||
|
||
private async fetchHostnameSuffix(): Promise<string | undefined> { | ||
try { | ||
const sshConfig = await this.restClient.getDeploymentSSHConfig() | ||
return sshConfig.hostname_suffix | ||
} catch (error) { | ||
if (!isAxiosError(error)) { | ||
throw error | ||
} | ||
switch (error.response?.status) { | ||
case 404: { | ||
// Likely a very old deployment, just use the default. | ||
break | ||
} | ||
default: | ||
throw error | ||
} | ||
} | ||
} | ||
} | ||
|
||
async function maybeCoderConnectAddr( | ||
agent: string, | ||
workspace: string, | ||
owner: string, | ||
hostnameSuffix: string, | ||
): Promise<string | undefined> { | ||
const coderConnectHostname = `${agent}.${workspace}.${owner}.${hostnameSuffix}` | ||
try { | ||
const res = await promisify(lookup)(coderConnectHostname) | ||
// Captive DNS portals may return an unrelated address, so we check it's | ||
// within the Coder Service Prefix. | ||
return res.family === 6 && ipRangeCheck(res.address, "fd60:627a:a42b::/48") ? coderConnectHostname : undefined | ||
} catch { | ||
return undefined | ||
} | ||
} | ||
|
||
async function openDevContainer( | ||
|
Uh oh!
There was an error while loading. Please reload this page.