Skip to content

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

Merged
merged 34 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
79535d0
feat: Allow setting deployment wide ssh config settings
Emyrk Mar 14, 2023
f0eb123
feat: config-ssh respects deployment ssh config
Emyrk Mar 15, 2023
4beeb62
Catch early parse error
Emyrk Mar 15, 2023
08f5d3d
Add to unit test
Emyrk Mar 15, 2023
c8c5189
fix typo
Emyrk Mar 15, 2023
96ad4bc
Add unit test
Emyrk Mar 15, 2023
f9f4a8f
Fix output
Emyrk Mar 15, 2023
ebf9eb9
Make gen
Emyrk Mar 15, 2023
119695b
Simplify if/else
Emyrk Mar 15, 2023
b8f3242
Fix AutorizeAllEndpoints
Emyrk Mar 15, 2023
b082a5a
Fic swager docs
Emyrk Mar 15, 2023
4a1e3c2
Make gen
Emyrk Mar 15, 2023
01ea08f
The '.' is now configurable
Emyrk Mar 15, 2023
7074f50
CODER env prefix is automatic
Emyrk Mar 15, 2023
952c591
Renames
Emyrk Mar 15, 2023
dae091a
Make gen
Emyrk Mar 15, 2023
a1dd7d4
Fix AutorizeAllEndpoints
Emyrk Mar 15, 2023
4f42634
Rename to drop 'CLI'
Emyrk Mar 15, 2023
c218edd
Prefix requires .
Emyrk Mar 15, 2023
a752fc8
Use constant in test
Emyrk Mar 15, 2023
d328d97
Linting
Emyrk Mar 15, 2023
a4b9620
Formatting
Emyrk Mar 15, 2023
78fbda8
Allow the user to override the host prefix
Emyrk Mar 16, 2023
d28b850
Fix doc messages
Emyrk Mar 16, 2023
617d987
Make gen
Emyrk Mar 16, 2023
eb4bb7b
Fix comment
Emyrk Mar 16, 2023
123ce02
Remove "CLI" part of naming
Emyrk Mar 16, 2023
ca41cce
Update golden files
Emyrk Mar 16, 2023
efcbc29
Fix 404 logic
Emyrk Mar 16, 2023
3c1c87f
remove 1 error check
Emyrk Mar 16, 2023
4586b11
Move buildinfo into deployment.go
Emyrk Mar 16, 2023
0f2ef97
fixup! Move buildinfo into deployment.go
Emyrk Mar 16, 2023
a5aac50
make gen
Emyrk Mar 16, 2023
a3254cd
Golden files
Emyrk Mar 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 115 additions & 24 deletions cli/configssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io"
"io/fs"
"net/http"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -48,6 +49,43 @@ type sshConfigOptions struct {
sshOptions []string
}

// addOptions expects options 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.
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)
Expand Down Expand Up @@ -139,6 +177,7 @@ func configSSH() *cobra.Command {
usePreviousOpts bool
dryRun bool
skipProxyCommand bool
userHostPrefix string
)
cmd := &cobra.Command{
Annotations: workspaceCommand,
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -269,42 +316,85 @@ 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 31, 2023). Just return the error
// and remove this 404 check.
var sdkErr *codersdk.Error
if !(xerrors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound) {
return xerrors.Errorf("fetch coderd config failed: %w", err)
}
coderdConfig.HostnamePrefix = "coder."
}

if userHostPrefix != "" {
// Override with user flag.
coderdConfig.HostnamePrefix = userHostPrefix
}

// Ensure stable sorting of output.
slices.SortFunc(workspaceConfigs, func(a, b sshWorkspaceConfig) bool {
return a.Name < b.Name
})
for _, wc := range workspaceConfigs {
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",
for _, workspaceHostname := range wc.Hosts {
sshHostname := fmt.Sprintf("%s%s", coderdConfig.HostnamePrefix, workspaceHostname)
defaultOptions := []string{
"HostName " + sshHostname,
"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 !skipProxyCommand {
configOptions = append(
configOptions,
fmt.Sprintf(
"\tProxyCommand %s --global-config %s ssh --stdio %s",
escapedCoderBinary, escapedGlobalConfig, hostname,
),
)
defaultOptions = append(defaultOptions, fmt.Sprintf(
"ProxyCommand %s --global-config %s ssh --stdio %s",
escapedCoderBinary, escapedGlobalConfig, workspaceHostname,
))
}

var configOptions sshConfigOptions
// Add standard options.
err := configOptions.addOptions(defaultOptions...)
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
Copy link
Member Author

Choose a reason for hiding this comment

The 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 -o flag.


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')
}
}
Expand Down Expand Up @@ -363,7 +453,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")
}
Expand All @@ -376,6 +466,7 @@ func configSSH() *cobra.Command {
cmd.Flags().BoolVarP(&skipProxyCommand, "skip-proxy-command", "", false, "Specifies whether the ProxyCommand option should be skipped. Useful for testing.")
_ = cmd.Flags().MarkHidden("skip-proxy-command")
cliflag.BoolVarP(cmd.Flags(), &usePreviousOpts, "use-previous-options", "", "CODER_SSH_USE_PREVIOUS_OPTIONS", false, "Specifies whether or not to keep options from previous run of config-ssh.")
cmd.Flags().StringVarP(&userHostPrefix, "ssh-host-prefix", "", "", "Override the default host prefix.")
cliui.AllowSkipPrompt(cmd)

return cmd
Expand Down
78 changes: 78 additions & 0 deletions cli/configssh_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os/exec"
"path/filepath"
"runtime"
"sort"
"strings"
"testing"

Expand Down Expand Up @@ -179,3 +180,80 @@ func Test_sshConfigExecEscape(t *testing.T) {
})
}
}

func Test_sshConfigOptions_addOption(t *testing.T) {
t.Parallel()
testCases := []struct {
Name string
Start []string
Add []string
Expect []string
ExpectError bool
}{
{
Name: "Empty",
},
{
Name: "AddOne",
Add: []string{"foo bar"},
Expect: []string{
"foo bar",
},
},
{
Name: "Replace",
Start: []string{
"foo bar",
},
Add: []string{"Foo baz"},
Expect: []string{
"Foo baz",
},
},
{
Name: "AddAndReplace",
Start: []string{
"a b",
"foo bar",
"buzz bazz",
},
Add: []string{
"b c",
"A hello",
"hello world",
},
Expect: []string{
"foo bar",
"buzz bazz",
"b c",
"A hello",
"hello world",
},
},
{
Name: "Error",
Add: []string{"novalue"},
ExpectError: true,
},
}

for _, tt := range testCases {
tt := tt
t.Run(tt.Name, func(t *testing.T) {
t.Parallel()

o := sshConfigOptions{
sshOptions: tt.Start,
}
err := o.addOptions(tt.Add...)
if tt.ExpectError {
require.Error(t, err)
return
}
require.NoError(t, err)
sort.Strings(tt.Expect)
sort.Strings(o.sshOptions)
require.Equal(t, tt.Expect, o.sshOptions)
})
}
}
20 changes: 18 additions & 2 deletions cli/configssh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/coder/coder/agent"
"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/codersdk/agentsdk"
"github.com/coder/coder/provisioner/echo"
"github.com/coder/coder/provisionersdk/proto"
Expand Down Expand Up @@ -63,7 +64,18 @@ func sshConfigFileRead(t *testing.T, name string) string {
func TestConfigSSH(t *testing.T) {
t.Parallel()

client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
const hostname = "test-coder."
const expectedKey = "ConnectionAttempts"
client := coderdtest.New(t, &coderdtest.Options{
IncludeProvisionerDaemon: true,
ConfigSSH: codersdk.SSHConfigResponse{
HostnamePrefix: hostname,
SSHConfigOptions: map[string]string{
// Something we can test for
expectedKey: "3",
},
},
})
user := coderdtest.CreateFirstUser(t, client)
authToken := uuid.NewString()
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
Expand Down Expand Up @@ -181,9 +193,13 @@ func TestConfigSSH(t *testing.T) {

<-doneChan

fileContents, err := os.ReadFile(sshConfigFile)
require.NoError(t, err, "read ssh config file")
require.Contains(t, string(fileContents), expectedKey, "ssh config file contains expected key")

home := filepath.Dir(filepath.Dir(sshConfigFile))
// #nosec
sshCmd := exec.Command("ssh", "-F", sshConfigFile, "coder."+workspace.Name, "echo", "test")
sshCmd := exec.Command("ssh", "-F", sshConfigFile, hostname+workspace.Name, "echo", "test")
pty = ptytest.New(t)
// Set HOME because coder config is included from ~/.ssh/coder.
sshCmd.Env = append(sshCmd.Env, fmt.Sprintf("HOME=%s", home))
Expand Down
9 changes: 9 additions & 0 deletions cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,11 @@ flags, and YAML configuration. The precedence is as follows:
return xerrors.Errorf("parse real ip config: %w", err)
}

configSSHOptions, err := cfg.SSHConfig.ParseOptions()
if err != nil {
return xerrors.Errorf("parse ssh config options %q: %w", cfg.SSHConfig.SSHConfigOptions.String(), err)
}

options := &coderd.Options{
AccessURL: cfg.AccessURL.Value(),
AppHostname: appHostname,
Expand All @@ -696,6 +701,10 @@ flags, and YAML configuration. The precedence is as follows:
LoginRateLimit: loginRateLimit,
FilesRateLimit: filesRateLimit,
HTTPClient: httpClient,
SSHConfig: codersdk.SSHConfigResponse{
HostnamePrefix: cfg.SSHConfig.DeploymentName.String(),
SSHConfigOptions: configSSHOptions,
},
}
if tlsConfig != nil {
options.TLSCertificates = tlsConfig.Certificates
Expand Down
1 change: 1 addition & 0 deletions cli/testdata/coder_config-ssh_--help.golden
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Flags:
-h, --help help for config-ssh
--ssh-config-file string Specifies the path to an SSH config.
Consumes $CODER_SSH_CONFIG_FILE (default "~/.ssh/config")
--ssh-host-prefix string Override the default host prefix.
-o, --ssh-option stringArray Specifies additional SSH options to embed in each host stanza.
--use-previous-options Specifies whether or not to keep options from previous run of
config-ssh.
Expand Down
Loading