- Notifications
You must be signed in to change notification settings - Fork928
feat: Refactor CLI config-ssh to Include configurations#1848
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| // to manage other files in this folder as well, e.g. keys, vscode | ||
| // specific config (i.e. for only listing coder files in vscode), | ||
| // etc. | ||
| sshEnabledLine = "Include coder.d/host-*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is there a reason to prefer multiple configurations over a single, larger one?
e.g.~/.ssh/coder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I did have some thoughts in mind, one was to store a hash in the config file so we can detect user modifications and prompt to overwrite. Another is for the user to be able to include specific hosts into their config (example):
~/.ssh/vscode-ssh-config:
Include coder.d/host-ws1Include coder.d/host-ws2In Code you could then set"remote.SSH.configFile": "~/.ssh/vscode-ssh-config".
| err = func() error { | ||
| exists := true | ||
| configRaw, err := os.Open(sshConfigFile) | ||
| if err != nil && !xerrors.Is(err, fs.ErrNotExist) { | ||
| return err | ||
| } else if xerrors.Is(err, fs.ErrNotExist) { | ||
| exists = false | ||
| } | ||
| defer configRaw.Close() | ||
| if exists { | ||
| s := bufio.NewScanner(configRaw) | ||
| for s.Scan() { | ||
| if strings.HasPrefix(s.Text(), sshEnabledLine) { | ||
| enabled = true | ||
| break | ||
| } | ||
| } | ||
| if s.Err() != nil { | ||
| return err | ||
| } | ||
| } | ||
| return nil | ||
| }() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
🤔 make it its own func, perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Will do 👍🏻! (Lots of cleanup left to do here.)
| # To remove this blob, run: | ||
| # | ||
| # coder config-ssh --remove | ||
| # | ||
| # You should not hand-edit this section, unless you are deleting it.` | ||
| const sshEndToken = "# ------------END-CODER------------" | ||
| const ( | ||
| // Include path is relative to `~/.ssh` and each workspace will | ||
| // have a separate file (e.g. `~/.ssh/coder.d/host-my-workspace`). | ||
| // By prefixing hosts as `host-` we give ourselves the flexibility | ||
| // to manage other files in this folder as well, e.g. keys, vscode | ||
| // specific config (i.e. for only listing coder files in vscode), | ||
| // etc. | ||
| sshEnabledLine = "Include coder.d/host-*" | ||
| // TODO(mafredri): Does this hold on Windows? | ||
| sshCoderConfigd = "~/.ssh/coder.d" | ||
| // TODO(mafredri): Write a README to the folder? | ||
| // sshCoderConfigdReadme = `Information, tricks, removal, etc.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What kind of migration path will this have for users?
Will they have to remove the existing section of their ssh-configs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Unfortunately, yes. The idea was originally that we'd get this changed early enough for that not to be a problem. But if it is, we can add a cleanup function (I'd have left it in but it wasn't implemented yet 😄.)
This PR is replaced by a proper refactor in#1900. |
WIP: This PR refactors
coder config-sshto useIncludeinstead of block management, according to the proposal in#1326.This is a quick Friday hack to prototype the experience, a lot of TODOs and cleanups remaining.
First run:
SSH config contents:
Workspace config (each as separate files for more flexibility):
Re-running the command: