-
Notifications
You must be signed in to change notification settings - Fork 899
feat: Add deployment side config-ssh options #6613
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 17 commits
79535d0
f0eb123
4beeb62
08f5d3d
c8c5189
96ad4bc
f9f4a8f
ebf9eb9
119695b
b8f3242
b082a5a
4a1e3c2
01ea08f
7074f50
952c591
dae091a
a1dd7d4
4f42634
c218edd
a752fc8
d328d97
a4b9620
78fbda8
d28b850
617d987
eb4bb7b
123ce02
ca41cce
efcbc29
3c1c87f
4586b11
0f2ef97
a5aac50
a3254cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import ( | |
"fmt" | ||
"io" | ||
"io/fs" | ||
"net/http" | ||
"os" | ||
"path/filepath" | ||
"runtime" | ||
|
@@ -48,6 +49,44 @@ type sshConfigOptions struct { | |
sshOptions []string | ||
} | ||
|
||
// add expects an option in the form of "option=value" or "option value". | ||
// It will override any existing option with the same key to prevent duplicates. | ||
// Invalid options will return an error. | ||
func (o *sshConfigOptions) addOptions(options ...string) error { | ||
for _, option := range options { | ||
err := o.addOption(option) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func (o *sshConfigOptions) addOption(option string) error { | ||
key, _, err := codersdk.ParseSSHConfigOption(option) | ||
if err != nil { | ||
return err | ||
} | ||
for i, existing := range o.sshOptions { | ||
// Override existing option if they share the same key. | ||
// This is case-insensitive. Parsing each time might be a little slow, | ||
// but it is ok. | ||
// | ||
Emyrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
existingKey, _, err := codersdk.ParseSSHConfigOption(existing) | ||
if err != nil { | ||
// Don't mess with original values if there is an error. | ||
// This could have come from the user's manual edits. | ||
continue | ||
} | ||
if strings.EqualFold(existingKey, key) { | ||
o.sshOptions[i] = option | ||
return nil | ||
} | ||
} | ||
o.sshOptions = append(o.sshOptions, option) | ||
return nil | ||
} | ||
|
||
func (o sshConfigOptions) equal(other sshConfigOptions) bool { | ||
// Compare without side-effects or regard to order. | ||
opt1 := slices.Clone(o.sshOptions) | ||
|
@@ -156,12 +195,13 @@ func configSSH() *cobra.Command { | |
), | ||
Args: cobra.ExactArgs(0), | ||
RunE: func(cmd *cobra.Command, _ []string) error { | ||
ctx := cmd.Context() | ||
client, err := CreateClient(cmd) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
recvWorkspaceConfigs := sshPrepareWorkspaceConfigs(cmd.Context(), client) | ||
recvWorkspaceConfigs := sshPrepareWorkspaceConfigs(ctx, client) | ||
|
||
out := cmd.OutOrStdout() | ||
if dryRun { | ||
|
@@ -220,6 +260,13 @@ func configSSH() *cobra.Command { | |
if usePreviousOpts && lastConfig != nil { | ||
sshConfigOpts = *lastConfig | ||
} else if lastConfig != nil && !sshConfigOpts.equal(*lastConfig) { | ||
for _, v := range sshConfigOpts.sshOptions { | ||
// If the user passes an invalid option, we should catch | ||
// this early. | ||
if _, _, err := codersdk.ParseSSHConfigOption(v); err != nil { | ||
return xerrors.Errorf("invalid option from flag: %w", err) | ||
} | ||
} | ||
newOpts := sshConfigOpts.asList() | ||
newOptsMsg := "\n\n New options: none" | ||
if len(newOpts) > 0 { | ||
|
@@ -269,6 +316,26 @@ func configSSH() *cobra.Command { | |
if err != nil { | ||
return xerrors.Errorf("fetch workspace configs failed: %w", err) | ||
} | ||
|
||
coderdConfig, err := client.SSHConfiguration(ctx) | ||
if err != nil { | ||
// If the error is 404, this deployment does not support | ||
// this endpoint yet. Do not error, just assume defaults. | ||
// TODO: Remove this in 2 months (May 2023). Just return the error | ||
// and remove this 404 check. | ||
var sdkErr *codersdk.Error | ||
if !xerrors.As(err, &sdkErr) { | ||
// not an SDK error, return the original error. | ||
return xerrors.Errorf("fetch coderd config failed: %w", err) | ||
} | ||
|
||
if sdkErr.StatusCode() != http.StatusNotFound { | ||
// Not a 404, return the original error. | ||
return xerrors.Errorf("fetch coderd config failed: %w", err) | ||
} | ||
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. This is just to support new coder cli against an older coderd. Can drop it after some time. 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. Meh, I don't think we should do this. Our install script supports installing by version, so it's really easy to upgrade/downgrade. 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. If we don't care, I can drop it. I think it is mostly something developers will hit. 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. What error will users see when they do this on an old client? 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. As long as it's clear to the user that they need to upgrade to resolve it, I don't think we need to have backward compatibility. 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. Let's be sure to label this as a breaking change though 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. Can I just keep this then @kylecarbs? It makes things backwards compatible without docs needed. I included a drop date to remove the code. It's just a convince to handle any edge cases, and it's not expensive or complex. |
||
coderdConfig.HostnamePrefix = "coder." | ||
} | ||
|
||
// Ensure stable sorting of output. | ||
slices.SortFunc(workspaceConfigs, func(a, b sshWorkspaceConfig) bool { | ||
return a.Name < b.Name | ||
|
@@ -277,34 +344,59 @@ func configSSH() *cobra.Command { | |
sort.Strings(wc.Hosts) | ||
// Write agent configuration. | ||
for _, hostname := range wc.Hosts { | ||
configOptions := []string{ | ||
"Host coder." + hostname, | ||
} | ||
for _, option := range sshConfigOpts.sshOptions { | ||
configOptions = append(configOptions, "\t"+option) | ||
} | ||
configOptions = append(configOptions, | ||
"\tHostName coder."+hostname, | ||
"\tConnectTimeout=0", | ||
"\tStrictHostKeyChecking=no", | ||
sshHostname := fmt.Sprintf("%s%s", coderdConfig.HostnamePrefix, hostname) | ||
Emyrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var configOptions sshConfigOptions | ||
// Add standard options. | ||
err := configOptions.addOptions( | ||
"HostName "+sshHostname, | ||
Emyrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"ConnectTimeout=0", | ||
"StrictHostKeyChecking=no", | ||
// Without this, the "REMOTE HOST IDENTITY CHANGED" | ||
// message will appear. | ||
"\tUserKnownHostsFile=/dev/null", | ||
"UserKnownHostsFile=/dev/null", | ||
// This disables the "Warning: Permanently added 'hostname' (RSA) to the list of known hosts." | ||
// message from appearing on every SSH. This happens because we ignore the known hosts. | ||
"\tLogLevel ERROR", | ||
"LogLevel ERROR", | ||
) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if !skipProxyCommand { | ||
configOptions = append( | ||
configOptions, | ||
fmt.Sprintf( | ||
"\tProxyCommand %s --global-config %s ssh --stdio %s", | ||
escapedCoderBinary, escapedGlobalConfig, hostname, | ||
), | ||
) | ||
err := configOptions.addOptions(fmt.Sprintf( | ||
"ProxyCommand %s --global-config %s ssh --stdio %s", | ||
escapedCoderBinary, escapedGlobalConfig, hostname, | ||
)) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
|
||
// Override with deployment options | ||
for k, v := range coderdConfig.SSHConfigOptions { | ||
opt := fmt.Sprintf("%s %s", k, v) | ||
err := configOptions.addOptions(opt) | ||
if err != nil { | ||
return xerrors.Errorf("add coderd config option %q: %w", opt, err) | ||
} | ||
} | ||
// Override with flag options | ||
for _, opt := range sshConfigOpts.sshOptions { | ||
err := configOptions.addOptions(opt) | ||
if err != nil { | ||
return xerrors.Errorf("add flag config option %q: %w", opt, err) | ||
} | ||
} | ||
Comment on lines
+373
to
+387
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. Default > Deployment > Flags So the user can always override any values by using |
||
|
||
hostBlock := []string{ | ||
"Host " + sshHostname, | ||
} | ||
// Prefix with '\t' | ||
for _, v := range configOptions.sshOptions { | ||
hostBlock = append(hostBlock, "\t"+v) | ||
} | ||
|
||
_, _ = buf.WriteString(strings.Join(configOptions, "\n")) | ||
_, _ = buf.WriteString(strings.Join(hostBlock, "\n")) | ||
_ = buf.WriteByte('\n') | ||
} | ||
} | ||
|
@@ -363,7 +455,7 @@ func configSSH() *cobra.Command { | |
|
||
if len(workspaceConfigs) > 0 { | ||
_, _ = fmt.Fprintln(out, "You should now be able to ssh into your workspace.") | ||
_, _ = fmt.Fprintf(out, "For example, try running:\n\n\t$ ssh coder.%s\n", workspaceConfigs[0].Name) | ||
_, _ = fmt.Fprintf(out, "For example, try running:\n\n\t$ ssh %s%s\n", coderdConfig.HostnamePrefix, workspaceConfigs[0].Name) | ||
} else { | ||
_, _ = fmt.Fprint(out, "You don't have any workspaces yet, try creating one with:\n\n\t$ coder create <workspace>\n") | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.