Skip to content
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

Modify user SSH config with single Include statement instead of managed block #1326

Closed
Tracked by #1939
mafredri opened this issue May 6, 2022 · 2 comments · Fixed by #1900
Closed
Tracked by #1939

Modify user SSH config with single Include statement instead of managed block #1326

mafredri opened this issue May 6, 2022 · 2 comments · Fixed by #1900
Assignees
Labels
api Area: HTTP API cli Area: CLI feature Something we don't have yet
Milestone

Comments

@mafredri
Copy link
Member

mafredri commented May 6, 2022

What is your suggestion?

We could improve the user experience by only adding a single line to the users SSH config. This would achieve the same functionality that we have currently whilst keeping the users SSH config cleaner and not rely on magic syntax.

~/.ssh/config:

Include coder.d/*

We probably want to include a comment explaining the line, but it's all that's required.

~/.ssh/coder.d/workspace-name:

Host coder.workspace-name
	HostName coder.workspace-name
	...

To ensure the coder SSH config is active, we only need to check for the presence of Include coder.d/* but the user is allowed to move it anywhere in the config.

This would replace the current approach of using a block in ~/.ssh/config:

# ------------START-CODER-----------
# This was generated by "coder config-ssh".
#
# To remove this blob, run:
#
#    coder config-ssh --remove
#
# You should not hand-edit this section, unless you are deleting it.`
Host coder.workspace-name
	HostName coder.workspace-name
	...
# ------------END-CODER------------

This functionality was released in OpenSSH 7.3p1 (2016-08-01), I think it's fairly safe to assume that this version of SSH at minimum will be available, or do we know of cases where this is not true?

Why do you want this feature?

After a discussion in BE Variety we concluded that it would be a nice improvement to the user experience to make a more minimal one-time change to the user SSH config.

Generally users don't like it when programs make changes to their configuration files, this change is minimal and perhaps more acceptable. Especially if the user is informed that it's a one-time addition.

Are there any workarounds to get this functionality today?

We could continue using the existing functionality.

Are you interested in submitting a PR for this?

Yes.

@mafredri mafredri added the feature Something we don't have yet label May 6, 2022
@tjcran tjcran added this to the Community MVP milestone May 16, 2022
@tjcran
Copy link

tjcran commented May 16, 2022

Not the most important change for Community MVP milestone, but it is relatively low effort it seems. If needed, should be punted to ensure we deliver on other more important items.

@ketang
Copy link
Contributor

ketang commented May 25, 2022

I like this. This is a nice suggestion to make the experience cleaner.

@mafredri mafredri self-assigned this May 27, 2022
mafredri added a commit that referenced this issue May 30, 2022
- Magic block is replaced by Include statement
- Writes are only done on changes
- Inform user of changes via prompt
- Allow displaying changes via `--diff`
- Remove magic block if present

TODO:

- [ ] Parse previous `config-ssh` options and suggest re-using them
- [ ] Auto-update `~/.ssh/coder` on `coder create` and `coder delete`
- [ ] Tests for the new functionality

Fixes #1326
@tjcran tjcran modified the milestones: Enterprise MVP , Community MVP Jun 3, 2022
@misskniss misskniss changed the title Feat: Modify user SSH config with single Include statement instead of managed block Modify user SSH config with single Include statement instead of managed block Jun 3, 2022
mafredri added a commit that referenced this issue Jun 6, 2022
- Magic block is replaced by Include statement
- Writes are only done on changes
- Inform user of changes via prompt
- Allow displaying changes via `--diff`
- Remove magic block if present

TODO:

- [ ] Parse previous `config-ssh` options and suggest re-using them
- [ ] Auto-update `~/.ssh/coder` on `coder create` and `coder delete`
- [ ] Tests for the new functionality

Fixes #1326
mafredri added a commit that referenced this issue Jun 8, 2022
- Magic block is replaced by Include statement
- Writes are only done on changes
- Inform user of changes via prompt
- Allow displaying changes via `--diff`
- Remove magic block if present

TODO:

- [ ] Parse previous `config-ssh` options and suggest re-using them
- [ ] Auto-update `~/.ssh/coder` on `coder create` and `coder delete`
- [ ] Tests for the new functionality

Fixes #1326
mafredri added a commit that referenced this issue Jun 8, 2022
- Magic block is replaced by Include statement
- Writes are only done on changes
- Inform user of changes via prompt
- Allow displaying changes via `--diff`
- Remove magic block if present
- Safer config writing via tmp-file + rename
- Parse previous `config-ssh` options, compare to new options and ask to use new (otherwise old ones are used)
- Tests the new functionality

Fixes #1326
kylecarbs pushed a commit that referenced this issue Jun 10, 2022
- Magic block is replaced by Include statement
- Writes are only done on changes
- Inform user of changes via prompt
- Allow displaying changes via `--diff`
- Remove magic block if present
- Safer config writing via tmp-file + rename
- Parse previous `config-ssh` options, compare to new options and ask to use new (otherwise old ones are used)
- Tests the new functionality

Fixes #1326
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Area: HTTP API cli Area: CLI feature Something we don't have yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants