diff --git a/cli/configssh.go b/cli/configssh.go index 9f6177a6be5a1..40a5f49406b10 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -137,7 +137,6 @@ func configSSH() *cobra.Command { sshConfigFile string sshConfigOpts sshConfigOptions usePreviousOpts bool - coderConfigFile string dryRun bool skipProxyCommand bool wireguard bool @@ -198,15 +197,7 @@ func configSSH() *cobra.Command { // Parse the previous configuration only if config-ssh // has been run previously. var lastConfig *sshConfigOptions - var ok bool - var coderConfigRaw []byte - if coderConfigFile, coderConfigRaw, ok = readDeprecatedCoderConfigFile(homedir, coderConfigFile); ok { - // Deprecated: Remove after migration period. - changes = append(changes, fmt.Sprintf("Remove old auto-generated coder config file at %s", coderConfigFile)) - // Backwards compate, restore old options. - c := sshConfigParseLastOptions(bytes.NewReader(coderConfigRaw)) - lastConfig = &c - } else if section, ok := sshConfigGetCoderSection(configRaw); ok { + if section, ok := sshConfigGetCoderSection(configRaw); ok { c := sshConfigParseLastOptions(bytes.NewReader(section)) lastConfig = &c } @@ -237,6 +228,8 @@ func configSSH() *cobra.Command { } // Selecting "no" will use the last config. sshConfigOpts = *lastConfig + } else { + changes = append(changes, "Use new SSH options") } // Only print when prompts are shown. if yes, _ := cmd.Flags().GetBool("yes"); !yes { @@ -245,14 +238,6 @@ func configSSH() *cobra.Command { } configModified := configRaw - - // Check for the presence of the coder Include - // statement is present and add if missing. - // Deprecated: Remove after migration period. - if configModified, ok = removeDeprecatedSSHIncludeStatement(configModified); ok { - changes = append(changes, fmt.Sprintf("Remove %q from %s", "Include coder", sshConfigFile)) - } - root := createConfig(cmd) buf := &bytes.Buffer{} @@ -313,17 +298,34 @@ func configSSH() *cobra.Command { _, _ = buf.Write(after) if !bytes.Equal(configModified, buf.Bytes()) { - changes = append(changes, fmt.Sprintf("Update coder config section in %s", sshConfigFile)) + changes = append(changes, fmt.Sprintf("Update the coder section in %s", sshConfigFile)) configModified = buf.Bytes() } - if len(changes) > 0 { - dryRunDisclaimer := "" - if dryRun { - dryRunDisclaimer = " (dry-run, no changes will be made)" + if len(changes) == 0 { + _, _ = fmt.Fprintf(out, "No changes to make.\n") + return nil + } + + if dryRun { + _, _ = fmt.Fprintf(out, "Dry run, the following changes would be made to your SSH configuration:\n\n * %s\n\n", strings.Join(changes, "\n * ")) + + color := isTTYOut(cmd) + diff, err := diffBytes(sshConfigFile, configRaw, configModified, color) + if err != nil { + return xerrors.Errorf("diff failed: %w", err) + } + if len(diff) > 0 { + // Write diff to stdout. + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s", diff) } + + return nil + } + + if len(changes) > 0 { _, err = cliui.Prompt(cmd, cliui.PromptOptions{ - Text: fmt.Sprintf("The following changes will be made to your SSH configuration:\n\n * %s\n\n Continue?%s", strings.Join(changes, "\n * "), dryRunDisclaimer), + Text: fmt.Sprintf("The following changes will be made to your SSH configuration:\n\n * %s\n\n Continue?", strings.Join(changes, "\n * ")), IsConfirm: true, }) if err != nil { @@ -335,47 +337,18 @@ func configSSH() *cobra.Command { } } - if dryRun { - color := isTTYOut(cmd) - diffFns := []func() ([]byte, error){ - func() ([]byte, error) { return diffBytes(sshConfigFile, configRaw, configModified, color) }, - } - if len(coderConfigRaw) > 0 { - // Deprecated: Remove after migration period. - diffFns = append(diffFns, func() ([]byte, error) { return diffBytes(coderConfigFile, coderConfigRaw, nil, color) }) - } - - for _, diffFn := range diffFns { - diff, err := diffFn() - if err != nil { - return xerrors.Errorf("diff failed: %w", err) - } - if len(diff) > 0 { - // Write diff to stdout. - _, _ = fmt.Fprintf(cmd.OutOrStdout(), "\n%s", diff) - } - } - } else { - if !bytes.Equal(configRaw, configModified) { - err = writeWithTempFileAndMove(sshConfigFile, bytes.NewReader(configModified)) - if err != nil { - return xerrors.Errorf("write ssh config failed: %w", err) - } - } - // Deprecated: Remove after migration period. - if len(coderConfigRaw) > 0 { - err = os.Remove(coderConfigFile) - if err != nil { - return xerrors.Errorf("remove coder config failed: %w", err) - } + if !bytes.Equal(configRaw, configModified) { + err = writeWithTempFileAndMove(sshConfigFile, bytes.NewReader(configModified)) + if err != nil { + return xerrors.Errorf("write ssh config failed: %w", err) } } 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\n", workspaceConfigs[0].Name) + _, _ = fmt.Fprintf(out, "For example, try running:\n\n\t$ ssh coder.%s\n", workspaceConfigs[0].Name) } else { - _, _ = fmt.Fprint(out, "You don't have any workspaces yet, try creating one with:\n\n\t$ coder create \n\n") + _, _ = fmt.Fprint(out, "You don't have any workspaces yet, try creating one with:\n\n\t$ coder create \n") } return nil }, @@ -389,10 +362,6 @@ func configSSH() *cobra.Command { cliflag.BoolVarP(cmd.Flags(), &wireguard, "wireguard", "", "CODER_CONFIG_SSH_WIREGUARD", false, "Whether to use Wireguard for SSH tunneling.") _ = cmd.Flags().MarkHidden("wireguard") - // Deprecated: Remove after migration period. - cmd.Flags().StringVar(&coderConfigFile, "test.ssh-coder-config-file", sshDefaultCoderConfigFileName, "Specifies the path to an Coder SSH config file. Useful for testing.") - _ = cmd.Flags().MarkHidden("test.ssh-coder-config-file") - cliui.AllowSkipPrompt(cmd) return cmd diff --git a/cli/configssh_old.go b/cli/configssh_old.go deleted file mode 100644 index 1bb9ed11be6b3..0000000000000 --- a/cli/configssh_old.go +++ /dev/null @@ -1,66 +0,0 @@ -package cli - -import ( - "bytes" - "os" - "path/filepath" - "regexp" - "strings" -) - -// This file contains config-ssh definitions that are deprecated, they -// will be removed after a migratory period. - -const ( - sshDefaultCoderConfigFileName = "~/.ssh/coder" - sshCoderConfigHeader = "# This file is managed by coder. DO NOT EDIT." -) - -// Regular expressions are used because SSH configs do not have -// meaningful indentation and keywords are case-insensitive. -var ( - // Find the semantically correct include statement. Since the user can - // modify their configuration as they see fit, there could be: - // - Leading indentation (space, tab) - // - Trailing indentation (space, tab) - // - Select newline after Include statement for cleaner removal - // In the following cases, we will not recognize the Include statement - // and leave as-is (i.e. they're not supported): - // - User adds another file to the Include statement - // - User adds a comment on the same line as the Include statement - sshCoderIncludedRe = regexp.MustCompile(`(?m)^[\t ]*((?i)Include) coder[\t ]*[\r]?[\n]?$`) -) - -// removeDeprecatedSSHIncludeStatement checks for the Include coder statement -// and returns modified = true if it was removed. -func removeDeprecatedSSHIncludeStatement(data []byte) (modifiedData []byte, modified bool) { - coderInclude := sshCoderIncludedRe.FindIndex(data) - if coderInclude == nil { - return data, false - } - - // Remove Include statement. - d := append([]byte{}, data[:coderInclude[0]]...) - d = append(d, data[coderInclude[1]:]...) - data = d - - return data, true -} - -// readDeprecatedCoderConfigFile reads the deprecated split config file. -func readDeprecatedCoderConfigFile(homedir, coderConfigFile string) (name string, data []byte, ok bool) { - if strings.HasPrefix(coderConfigFile, "~/") { - coderConfigFile = filepath.Join(homedir, coderConfigFile[2:]) - } - - b, err := os.ReadFile(coderConfigFile) - if err != nil { - return coderConfigFile, nil, false - } - if len(b) > 0 { - if !bytes.HasPrefix(b, []byte(sshCoderConfigHeader)) { - return coderConfigFile, nil, false - } - } - return coderConfigFile, b, true -} diff --git a/cli/configssh_test.go b/cli/configssh_test.go index beb95b58f575e..29e59a7dfc8ec 100644 --- a/cli/configssh_test.go +++ b/cli/configssh_test.go @@ -6,7 +6,6 @@ import ( "context" "fmt" "io" - "io/fs" "net" "os" "os/exec" @@ -30,15 +29,14 @@ import ( "github.com/coder/coder/pty/ptytest" ) -func sshConfigFileNames(t *testing.T) (sshConfig string, coderConfig string) { +func sshConfigFileName(t *testing.T) (sshConfig string) { t.Helper() tmpdir := t.TempDir() dotssh := filepath.Join(tmpdir, ".ssh") err := os.Mkdir(dotssh, 0o700) require.NoError(t, err) - n1 := filepath.Join(dotssh, "config") - n2 := filepath.Join(dotssh, "coder") - return n1, n2 + n := filepath.Join(dotssh, "config") + return n } func sshConfigFileCreate(t *testing.T, name string, data io.Reader) { @@ -135,7 +133,7 @@ func TestConfigSSH(t *testing.T) { } }() - sshConfigFile, _ := sshConfigFileNames(t) + sshConfigFile := sshConfigFileName(t) tcpAddr, valid := listener.Addr().(*net.TCPAddr) require.True(t, valid) @@ -197,12 +195,10 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { }, "\n") type writeConfig struct { - ssh string - coder string + ssh string } type wantConfig struct { - ssh string - coderKept bool + ssh string } type match struct { match, write string @@ -514,120 +510,6 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { "--yes", }, }, - - // Tests for deprecated split coder config. - { - name: "Do not overwrite unknown coder config", - writeConfig: writeConfig{ - ssh: strings.Join([]string{ - baseHeader, - "", - }, "\n"), - coder: strings.Join([]string{ - "We're no strangers to love", - "You know the rules and so do I (do I)", - }, "\n"), - }, - wantConfig: wantConfig{ - coderKept: true, - }, - }, - { - name: "Transfer options from coder to ssh config", - writeConfig: writeConfig{ - ssh: strings.Join([]string{ - "Include coder", - "", - }, "\n"), - coder: strings.Join([]string{ - "# This file is managed by coder. DO NOT EDIT.", - "#", - "# You should not hand-edit this file, all changes will be lost when running", - "# \"coder config-ssh\".", - "#", - "# Last config-ssh options:", - "# :ssh-option=ForwardAgent=yes", - "#", - }, "\n"), - }, - wantConfig: wantConfig{ - ssh: strings.Join([]string{ - headerStart, - "# Last config-ssh options:", - "# :ssh-option=ForwardAgent=yes", - "#", - headerEnd, - "", - }, "\n"), - }, - matches: []match{ - {match: "Use new options?", write: "no"}, - {match: "Continue?", write: "yes"}, - }, - }, - { - name: "Allow overwriting previous options from coder config", - writeConfig: writeConfig{ - ssh: strings.Join([]string{ - "Include coder", - "", - }, "\n"), - coder: strings.Join([]string{ - "# This file is managed by coder. DO NOT EDIT.", - "#", - "# You should not hand-edit this file, all changes will be lost when running", - "# \"coder config-ssh\".", - "#", - "# Last config-ssh options:", - "# :ssh-option=ForwardAgent=yes", - "#", - }, "\n"), - }, - wantConfig: wantConfig{ - ssh: strings.Join([]string{ - baseHeader, - "", - }, "\n"), - }, - matches: []match{ - {match: "Use new options?", write: "yes"}, - {match: "Continue?", write: "yes"}, - }, - }, - { - name: "Allow overwriting previous options from coder config when they differ", - writeConfig: writeConfig{ - ssh: strings.Join([]string{ - "Include coder", - "", - }, "\n"), - coder: strings.Join([]string{ - "# This file is managed by coder. DO NOT EDIT.", - "#", - "# You should not hand-edit this file, all changes will be lost when running", - "# \"coder config-ssh\".", - "#", - "# Last config-ssh options:", - "# :ssh-option=ForwardAgent=yes", - "#", - }, "\n"), - }, - wantConfig: wantConfig{ - ssh: strings.Join([]string{ - headerStart, - "# Last config-ssh options:", - "# :ssh-option=ForwardAgent=no", - "#", - headerEnd, - "", - }, "\n"), - }, - args: []string{"--ssh-option", "ForwardAgent=no"}, - matches: []match{ - {match: "Use new options?", write: "yes"}, - {match: "Continue?", write: "yes"}, - }, - }, } for _, tt := range tests { tt := tt @@ -645,18 +527,14 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { ) // Prepare ssh config files. - sshConfigName, coderConfigName := sshConfigFileNames(t) + sshConfigName := sshConfigFileName(t) if tt.writeConfig.ssh != "" { sshConfigFileCreate(t, sshConfigName, strings.NewReader(tt.writeConfig.ssh)) } - if tt.writeConfig.coder != "" { - sshConfigFileCreate(t, coderConfigName, strings.NewReader(tt.writeConfig.coder)) - } args := []string{ "config-ssh", "--ssh-config-file", sshConfigName, - "--test.ssh-coder-config-file", coderConfigName, } args = append(args, tt.args...) cmd, root := clitest.New(t, args...) @@ -685,10 +563,6 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { got := sshConfigFileRead(t, sshConfigName) assert.Equal(t, tt.wantConfig.ssh, got) } - if !tt.wantConfig.coderKept { - _, err := os.ReadFile(coderConfigName) - assert.ErrorIs(t, err, fs.ErrNotExist) - } }) } } @@ -778,7 +652,7 @@ func TestConfigSSH_Hostnames(t *testing.T) { workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - sshConfigFile, _ := sshConfigFileNames(t) + sshConfigFile := sshConfigFileName(t) cmd, root := clitest.New(t, "config-ssh", "--ssh-config-file", sshConfigFile) clitest.SetupConfig(t, client, root)