Resolve ssh hostname aliases with ssh -G
#84
Conversation
Going through the `ssh` executable ensures that hostnames get resolved using ssh's exact mechanism and alleviates the need for manually parsing and interpreting ssh configuration files.
| t.cacheMu.RLock() | ||
| cached, cacheFound := t.cacheMap[strings.ToLower(hostname)] | ||
| t.cacheMu.RUnlock() | ||
| if cacheFound { | ||
| return cached, nil |
There was a problem hiding this comment.
As far as I can tell this cache is never actually populated in this method. I am wondering about its purpose though. We can rectify this and start populating the cache or I think it would be okay to just allow the users of this package to implement the caching since our implementation is just an in-memory map. What are your thoughts?
There was a problem hiding this comment.
Good catch. If the cache is not populated, that was an oversight on my part.
I do think that an in-memory cache would be beneficial since Translate() is typically used to translate many URLs at once: think of a git repo that has multiple SSH remotes—Translate will be called 2 times per each remote. Since most remotes will be to the same hostname, a built-in cache avoids shelling out to ssh repeatedly.
Going through the
sshexecutable ensures that hostnames get resolved using ssh's exact mechanism and alleviates the need for manually parsing and interpreting ssh configuration files.I learned this trick from @cmbrose in cli/cli#5958.
Ref. cli/cli#6455