Skip to content

CLI: config-ssh: coder config-ssh --remove is not supported, but is mentioned in .ssh/config file #1704

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.

Sign up for GitHub

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

Jump to bottom
Closed
nadzeyav opened this issue May 24, 2022 · 7 comments
Closed

CLI: config-ssh: coder config-ssh --remove is not supported, but is mentioned in .ssh/config file #1704

nadzeyav opened this issue May 24, 2022 · 7 comments
Labels
api Area: HTTP API stale This issue is like stale bread.

Comments

@nadzeyav
Copy link

OS Information

  • OS: Windows
  • Browser (if applicable):
  • Architecture:
  • coder --version:
    Coder v0.0.0-devel+9b70a9b Tue May 24 00:04:38 UTC 2022
    9b70a9b

Steps to Reproduce

  1. Run coder config-ssh
  2. Open generated file: there is in the beginning ":
    This was generated by "coder config-ssh".
    To remove this blob, run:
    coder config-ssh --remove"
  3. Run coder config-ssh --remove

Expected

Config is removed or do not mention it in the generated file

Actual

coder config-ssh --remove
unknown flag: --remove
Run 'coder config-ssh --help' for usage.

Logs

Screenshot

image

Notes
@misskniss misskniss added the api Area: HTTP API label May 24, 2022
@sharkymark
Copy link
Contributor

I noticed this, and it's annoying, since I was having ssh config problems and to manually delete these.

@kylecarbs kylecarbs changed the title Bug: CLI: config-ssh: coder config-ssh --remove is not supported, but is mentioned in .ssh/config file CLI: config-ssh: coder config-ssh --remove is not supported, but is mentioned in .ssh/config file Jun 7, 2022
@mafredri
Copy link
Member

This behavior has changed in #1900 and #2341 (part of latest release), --remove is no longer mentioned.

# You should not hand-edit this section unless you are removing it, all
# changes will be lost when running "coder config-ssh".

The comment mentions manual removal. Can this be closed or do we need to implement --remove?

@ketang
Copy link
Contributor

ketang commented Jun 16, 2022 •
edited
Loading

I think manual removal can be dicey the way this implemented. How's this? Add obvious delimiter rows and per line comments e.g.:

#~~~~~Coder~~~~~Start~~~~~Coder~~~~~...
 [directive] #+++++Coder
 [directive] #+++++Coder
 [directive] #+++++Coder
#~~~~~Coder~~~~~End~~~~~Coder~~~~~...

@ketang ketang closed this as completed Jun 16, 2022
@ketang ketang reopened this Jun 16, 2022
@mafredri
Copy link
Member

mafredri commented Jun 16, 2022 •
edited
Loading

@ketang currently the block looks like this in ~/.ssh/config:

# ------------START-CODER-----------
# This section is managed by coder. DO NOT EDIT.
#
# You should not hand-edit this section unless you are removing it, all
# changes will be lost when running "coder config-ssh".
#
# Last config-ssh options:
# :ssh-option=ForwardAgent=yes
#
Host coder.work
	ForwardAgent=yes
	HostName coder.work
	ConnectTimeout=0
	StrictHostKeyChecking=no
	UserKnownHostsFile=/dev/null
	LogLevel ERROR
	ProxyCommand "coder" --global-config "/Users/maf/Library/Application Support/coderv2" ssh --stdio work
# ------------END-CODER------------

We assume that it's safe to manage all the lines between # ------------START-CODER----------- and # ------------END-CODER------------ as long as both exist. We are already able to easily strip this out as part of updating the config, adding --remove would be trivial.

If we're concerned about safety, I think one alternative would be to tag each line with a trailing comment, e.g.

Host coder.work # //coder-managed-line
	ForwardAgent=yes # //coder-managed-line
	HostName coder.work # //coder-managed-line
	ConnectTimeout=0 # //coder-managed-line
	StrictHostKeyChecking=no # //coder-managed-line
	UserKnownHostsFile=/dev/null # //coder-managed-line
	LogLevel ERROR # //coder-managed-line
	ProxyCommand "coder" --global-config "/Users/maf/Library/Application Support/coderv2" ssh --stdio work # //coder-managed-line

But I fear the additional noise would be unwelcome and make it harder to read.

We could also indent comments based on the longest line:

Host coder.work                                                                                                # //coder-managed-line
	ForwardAgent=yes                                                                                       # //coder-managed-line
	HostName coder.work                                                                                    # //coder-managed-line
	ConnectTimeout=0                                                                                       # //coder-managed-line
	StrictHostKeyChecking=no                                                                               # //coder-managed-line
	UserKnownHostsFile=/dev/null                                                                           # //coder-managed-line
	LogLevel ERROR                                                                                         # //coder-managed-line
	ProxyCommand "coder" --global-config "/Users/maf/Library/Application Support/coderv2" ssh --stdio work # //coder-managed-line

Slightly better, but resulting in some potentially very long lines.

EDIT: Oh I just realized this was probably your proposal, haha, oops.

@ketang
Copy link
Contributor

ketang commented Jun 16, 2022

No worries. I think we can backburner this for now and solicit community feedback a little further down the line. We have multiple plausible options and currently lack the data to guide us to the best among them.

@github-actions
Copy link

This issue is becoming stale. In order to keep the tracker readable and actionable, I'm going close to this issue in 7 days if there isn't more activity.

@github-actions github-actions bot added the stale This issue is like stale bread. label Aug 16, 2022
@kylecarbs
Copy link
Member

Closing for now... this doesn't seem required in the short-term.

@kylecarbs kylecarbs closed this as not planned Won't fix, can't repro, duplicate, stale Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api Area: HTTP API stale This issue is like stale bread.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@ketang @mafredri @sharkymark @misskniss @kylecarbs @nadzeyav