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 17 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
136 changes: 114 additions & 22 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,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.
//
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 @@ -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,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)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member

@bpmct bpmct Mar 16, 2023

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Let's be sure to label this as a breaking change though

Copy link
Member Author

Choose a reason for hiding this comment

The 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
Expand All @@ -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)
var configOptions sshConfigOptions
// Add standard options.
err := configOptions.addOptions(
"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 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
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 +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")
}
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: "test-coder",
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.CLISSH.ParseOptions()
if err != nil {
return xerrors.Errorf("parse ssh config options %q: %w", cfg.CLISSH.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.CLISSH.DeploymentName.String(),
SSHConfigOptions: configSSHOptions,
},
}
if tlsConfig != nil {
options.TLSCertificates = tlsConfig.Certificates
Expand Down
Loading