Support domain expansion from sshconfig #109

4U6U57 · 2017-12-02T01:44:41Z

Problem

The ~/.ssh/config file allows users to create hostname shortcuts, which can be used in the scp, ssh, and git commands (among others). For instance, cloning the following SCP style git remote:

git clone git@github.com:user/repo

if configured with the following SSH config:

Host gh
    HostName github.com
    User git

allows the user to simply run the following and produce the same result.

git clone gh:user/repo

However, git-open does not support this, and returns https://gh/user/repo. Of course, it is possible to set the open.gh.domain git config to work around this, but this is non-ideal when the information is already readily available in the sshconfig file.

Solution

This PR adds parsing of the sshconfig file if a SCP structured remote URL is detected, and then replaces the domain with the corresponding HostName entry if a matching Host entry is found. This happens before any of the fancy string manipulations, so it shouldn't break any of that.

Testing

Tests are found in the git-open.bat file with the sshconfig prefix. Note that for the tests to work, BATS has to populate the sshconfig file with dummy data so the tests can resolve it. While this is happening, your actual sshconfig file (if it exists) is stored temporarily in ~/.ssh/config~ (note the ~ suffix). Do not terminate the unit tests prematurely or the teardown will not copy your sshconfig back. This is my first time using BATS so I'm not sure if there was a better way to do that. git-open reads from ~/.ssh/config by default, or $ssh_config if defined (as it is for the unit tests).

I believe this PR is a more generalized version of #75

TODO

paulirish · 2017-12-23T19:12:54Z

git-open

@@ -167,7 +180,9 @@ case $( uname -s ) in
 esac

 # Allow printing the url if BROWSER=echo
-if [[ $BROWSER != "echo" ]]; then
+if [[ $BROWSER == "echo" ]]; then
+  openopt=''


what's this about?

It's to fix BATS testing on WSL, which still doesn't work after d97b7f3.

Click to expand test output:

[avalera:~/git-open-clean] master* ± npm test > git-open@2.0.0 test /home/avalera/git-open-clean > npm run unit && npm run lint:package && npm run lint:readme && npm run lint:editorconfig > git-open@2.0.0 unit /home/avalera/git-open-clean > bats test/ ✓ test environment ✓ help text ✓ invalid option ✗ gh: basic (from function `assert_output' in file test/test_helper/bats-assert/src/assert.bash, line 239, in test file test/git-open.bats, line 46) `assert_output "https://github.com/user/repo"' failed Switched to a new branch 'master' Reset branch 'master' -- output differs -- expected : https://github.com/user/repo actual : Start https://github.com/user/repo -- ✗ gh: branch (from function `assert_output' in file test/test_helper/bats-assert/src/assert.bash, line 239, in test file test/git-open.bats, line 53) `assert_output "https://github.com/user/repo/tree/mybranch"' failed Switched to a new branch 'master' Switched to a new branch 'mybranch' -- output differs -- expected : https://github.com/user/repo/tree/mybranch actual : Start https://github.com/user/repo/tree/mybranch -- ...

The outputted URL's are correct, but prefixed with the word Start from the $openopt variable, causing most tests to fail (ones with --partial still pass, because the correct output is there).

paulirish · 2017-12-23T19:14:38Z

test/git-open.bats

@@ -4,6 +4,7 @@ load "test_helper/bats-support/load"
 load "test_helper/bats-assert/load"

 foldername="sandboxrepo"
+sshconfig=~/.ssh/config


perhaps instead of this we could allow a env variable to override the sshconfig path. that way your real config isn't touched during testing. (i've definitely run into problems when creating the test suite and exiting it halfway ;)

wdyt?

👍 Although ~/.ssh is one of the few apps that you generally can't override, see https://wiki.archlinux.org/index.php/XDG_Base_Directory_support and https://bugzilla.mindrot.org/show_bug.cgi?id=2050

Yeah, I don't think it's possible to get around this. If we want it to be super safe, I can have it only copy in the sshconfig if none already exists, and otherwise just skip all related tests. Would that be preferred?

Actually, I guess there isn't any reason that the sshconfig be in the "right" place when testing, same as overriding BROWSER with echo, we can just supply it to the executable on runtime. I'll change that.

paulirish · 2017-12-23T19:17:32Z

I do have a project where I have it configured with a different git user and use this technique. So I've experienced how git-open fails in that case. :)

derimagia · 2017-12-23T19:33:18Z

Was a little worried about complexity here, but this can be merged with #75

See my comment here: #75 (comment)

So for example git ls-remote --get-url origin will return the proper url. Just tested it and this also works for ssh config items (awesome!)

derimagia · 2017-12-23T19:36:06Z

Note sure if it's worth adding tests for it since we would essentially be testing git's commands at that point but I can understand if you wanted it though

4U6U57 · 2017-12-23T20:06:28Z

@derimagia What do you mean by that? Believe me, if there was any other way to resolve an SSH host alias aside from awk-ing the sshconfig, I would be all for it. However, I don't think git ls-remote does it for us...

[avalera:~/wsl-open] master+
± git remote -v
origin  gitlab:4U6U57/wsl-open (fetch)
origin  gitlab:4U6U57/wsl-open (push)

[avalera:~/wsl-open] master+
± git ls-remote --get-url origin
gitlab:4U6U57/wsl-open

derimagia · 2017-12-23T20:12:34Z

Weird, git version? What does your .ssh/config entry look like for it?

❯ cat .git/config
[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
	logallrefupdates = true
	ignorecase = true
	precomposeunicode = true
[remote "origin"]
	url = gh:paulirish/git-open
	fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
	remote = origin
	merge = refs/heads/master

❯ git config --get remote.origin.url
gh:paulirish/git-open

❯ git ls-remote --get-url origin
git@github.com:paulirish/git-open

❯ cat ~/.ssh/config

Host gh
    hostname github.com
    user git

4U6U57 · 2017-12-23T20:18:14Z

I'm on Ubuntu 16.04.3 LTS (WSL), git 2.7.4. Yeah, not sure what to make of this.

Click to expand:

[avalera:~/wsl-open] master+
± cat .git/config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[branch "master"]
[branch "issue-7"]
[branch "issue-9"]
[remote "origin"]
        url = gitlab:4U6U57/wsl-open
        fetch = +refs/heads/*:refs/remotes/origin/*

[avalera:~/wsl-open] master+
± git config --get remote.origin.url
gitlab:4U6U57/wsl-open

[avalera:~/wsl-open] master+
± git ls-remote --get-url origin
gitlab:4U6U57/wsl-open

[avalera:~/wsl-open] master+
± cat ~/.ssh/config
# sshconfig: part of the 4U6U57/dotfiles project
# Edit this file to define ssh host shortcuts

# Git hosts
Host github
    HostName github.com
    User git
Host gitlab
    HostName gitlab.com
    User git

derimagia · 2017-12-23T20:18:35Z

Nevermind, hah I had an insteadOf in my global config that was doing it, sorry for that. Although that is a good way to handle it

Does your awking handle wildcards? It looks like it does an exact match which could e different from what is actually processed by ssh.

4U6U57 · 2017-12-23T20:22:31Z

I don't believe it does wildcards as it is right now. Wanted to keep it conservative at first since I didn't want to break anything, but I'll look into fixing that.

derimagia · 2017-12-23T20:25:49Z

So the two things this doesn't support is wildcard matching (ssh config propagate so you can also set defaults and match on multiple) and you can also have multiple hosts on matches on one line (So Host gitlab.com github.com).

If we can make this simple I'm good with it, although the other way to do it is to just add insteadOf's to your ssh config after we fix #75

4U6U57 · 2017-12-26T22:11:06Z

Okay, I think I have implemented everything @derimagia mentioned. Had to switch from awk to native bash since it's pattern matching matches that of sshconfig, which imo is not as readable so I put in a bunch of comments. Tests are much nicer now that I have distinct features to test. Lmk what you think.

derimagia · 2018-01-11T14:41:33Z

You mind rebasing this? I'll take a look over the weekend

paulirish · 2018-01-12T01:37:30Z

git-open

+      # Replace all instances of %h with the Host alias
+      echo "${ssh_array[1]//%h/$domain}"
+    fi
+  done < <(grep -Pi "^\\s*Host(Name)?\\s+" "$ssh_config"))


as I mentioned, i have a few repos where i use this ssh user pattern. (so thanks!)

however.. testing this out, i ran into the -P flag which isn't supported on my machine (typical mac w/ sierra). If i use ggrep (which i installed via homebrew coreutils), then this script works great.

Perhaps we can write the regex without requiring -P ?

I was just using it for the ? operator, does grep -E work on macOS?

Perfect. Yup works great

derimagia · 2018-01-22T15:49:54Z

git-open

+# Resolves an ssh alias defined in ssh_config to it's corresponding hostname
+# echos out result, should be used within subshell $( ssh_resolve $host )
+# echos out nothing if alias could not be resolved
+ssh_resolve() {


#nitpick Add function before this to be consistent with the other ones

derimagia · 2018-01-22T15:50:21Z

Added one nitpick but think this is good to go. There's probably going to be some edge cases for this and it's more code than I'd like, but I think it's fine

derimagia · 2018-01-23T14:50:13Z

@paulirish Since you said you had this issue, any last minute comments from using it with those repos?

derimagia · 2018-02-04T17:05:43Z

@4U6U57 Do you mind squashing this to a single commit? I'll take another look through and go through it myself one last time and merge it if you can

4U6U57 · 2018-02-04T17:56:48Z

@derimagia Have at it!

See `man ssh_config` for documentation for sshconfig. Only works on SCP-like remote URL's [user@]hostalias:path/to/repo.git Parses through ~/.ssh/config, looking for a Host entry matching hostalias (bash-like pattern matching supported) and expanding it to the corresponding HostName entry, if applicable.

derimagia · 2018-02-12T04:49:38Z

git-open

@@ -56,12 +56,51 @@ if [[ -z "$giturl" ]]; then
  exit 1
 fi

+ssh_config=${ssh_config:-~/.ssh/config}


Think these need to be quoted just in case the user's home dir has a space in it

shellcheck didn't catch this one 😄. I'll make the fix.

derimagia · 2018-02-12T05:09:43Z

Nice work! The only thing I don't see tested is pattern lists: https://www.freebsd.org/cgi/man.cgi?query=ssh_config&sektion=5#PATTERNS which can be negated (See this nodejs library that tests this): https://github.com/dotnil/ssh-config/blob/7cbbaa6c46e442501853f26da824623195a0fa42/test/test.glob.js#L25-L34

Does that need to be in there?

4U6U57 · 2018-02-16T05:42:52Z

@derimagia I've done some testing on my machine and determined that sshconfig does not support pattern lists for Host keyword. Not sure what their purpose is exactly (since you can accomplish them with overloading), but I've tried both quoted and unquoted to no success.

Click to expand test details:

Contents of ~/.ssh/config

...

Host "!*.dialup.example.com,*.example.com"
    HostName testing.pattern

Host !*.dialup.example.com,*.example.com
    HostName testing.lists

[avalera:~]
$ ssh test.example.com
ssh: Could not resolve hostname test.example.com: Name or service not known

[avalera:~] 255
$ ssh test.dialup.example.com
ssh: Could not resolve hostname test.dialup.example.com: Name or service not known

As seen, ssh does not resolve the domains to testing.pattern or testing.lists.

derimagia · 2018-02-19T20:05:55Z

I see what you mean, if that's the case then this should be good - I think we should go ahead and merge it

tig · 2020-03-12T19:57:45Z

I just found this tool and found this issue is not working for me. My ~/.ssh/config looks like this:

Host          tig
HostName       github.com
User           git

For this repo:

core.repositoryformatversion=0
core.filemode=false
core.bare=false
core.logallrefupdates=true
core.symlinks=false
core.ignorecase=true
remote.origin.url=tig:tig/gui.cs.git
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
branch.master.remote=origin
branch.master.merge=refs/heads/master

git open takes me to https://tig/tig/gui.cs

Which is not what I expected based on reading this Issue and resolution. What gives?

Support domain expansion from sshconfig

paulirish reviewed Dec 23, 2017

View reviewed changes

derimagia closed this Dec 23, 2017

derimagia reopened this Dec 23, 2017

4U6U57 force-pushed the master branch 2 times, most recently from 83b8611 to 67e5bf6 Compare January 12, 2018 01:17

paulirish reviewed Jan 12, 2018

View reviewed changes

derimagia reviewed Jan 22, 2018

View reviewed changes

4U6U57 force-pushed the master branch from f10c87d to 2ac59e3 Compare February 4, 2018 17:54

4U6U57 force-pushed the master branch from 2ac59e3 to 8909a2b Compare February 4, 2018 20:14

paulirish mentioned this pull request Feb 5, 2018

Split apart unit tests #116

Open

derimagia reviewed Feb 12, 2018

View reviewed changes

Fix shellcheck

775361b

derimagia merged commit d9b9580 into paulirish:master Feb 19, 2018

derimagia mentioned this pull request Mar 28, 2018

bad substitution #121

Closed

derimagia mentioned this pull request Dec 17, 2018

Opens ssh.github.com since upgrading #135

Closed

iblea pushed a commit to iblea/git-open that referenced this pull request Jun 17, 2024

Merge pull request paulirish#109 from 4U6U57/master

8c1571e

Support domain expansion from sshconfig

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support domain expansion from sshconfig #109

Support domain expansion from sshconfig #109

4U6U57 commented Dec 2, 2017 •

edited

Loading

paulirish Dec 23, 2017

4U6U57 Dec 23, 2017 •

edited

Loading

paulirish Dec 23, 2017

derimagia Dec 23, 2017

4U6U57 Dec 24, 2017

4U6U57 Dec 24, 2017

paulirish commented Dec 23, 2017

derimagia commented Dec 23, 2017 •

edited

Loading

derimagia commented Dec 23, 2017

4U6U57 commented Dec 23, 2017

derimagia commented Dec 23, 2017 •

edited

Loading

4U6U57 commented Dec 23, 2017 •

edited

Loading

derimagia commented Dec 23, 2017

4U6U57 commented Dec 23, 2017

derimagia commented Dec 23, 2017

4U6U57 commented Dec 26, 2017 •

edited

Loading

derimagia commented Jan 11, 2018

paulirish Jan 12, 2018

4U6U57 Jan 12, 2018

paulirish Jan 12, 2018

derimagia Jan 22, 2018

derimagia commented Jan 22, 2018

derimagia commented Jan 23, 2018

derimagia commented Feb 4, 2018

4U6U57 commented Feb 4, 2018

derimagia Feb 12, 2018

4U6U57 Feb 16, 2018

derimagia commented Feb 12, 2018

4U6U57 commented Feb 16, 2018 •

edited

Loading

derimagia commented Feb 19, 2018

tig commented Mar 12, 2020

Support domain expansion from sshconfig #109

Support domain expansion from sshconfig #109

Conversation

4U6U57 commented Dec 2, 2017 • edited Loading

Problem

Solution

Testing

TODO

Choose a reason for hiding this comment

4U6U57 Dec 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulirish commented Dec 23, 2017

derimagia commented Dec 23, 2017 • edited Loading

derimagia commented Dec 23, 2017

4U6U57 commented Dec 23, 2017

derimagia commented Dec 23, 2017 • edited Loading

4U6U57 commented Dec 23, 2017 • edited Loading

derimagia commented Dec 23, 2017

4U6U57 commented Dec 23, 2017

derimagia commented Dec 23, 2017

4U6U57 commented Dec 26, 2017 • edited Loading

derimagia commented Jan 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derimagia commented Jan 22, 2018

derimagia commented Jan 23, 2018

derimagia commented Feb 4, 2018

4U6U57 commented Feb 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derimagia commented Feb 12, 2018

4U6U57 commented Feb 16, 2018 • edited Loading

derimagia commented Feb 19, 2018

tig commented Mar 12, 2020

4U6U57 commented Dec 2, 2017 •

edited

Loading

4U6U57 Dec 23, 2017 •

edited

Loading

derimagia commented Dec 23, 2017 •

edited

Loading

derimagia commented Dec 23, 2017 •

edited

Loading

4U6U57 commented Dec 23, 2017 •

edited

Loading

4U6U57 commented Dec 26, 2017 •

edited

Loading

4U6U57 commented Feb 16, 2018 •

edited

Loading