First pass at review feedback

lib/client/keystore.go

@@ -75,7 +75,7 @@ type LocalKeyStore interface {
 
 	// AddKnownHostKeys adds the public key to the list of known hosts for
 	// a hostname.
-	AddKnownHostKeys(hostname string, proxyHost string, keys []ssh.PublicKey) error
+	AddKnownHostKeys(hostname, proxyHost string, keys []ssh.PublicKey) error
 
 	// GetKnownHostKeys returns all public keys for a hostname.
 	GetKnownHostKeys(hostname string) ([]ssh.PublicKey, error)
@@ -513,7 +513,7 @@ func (fs *fsLocalNonSessionKeyStore) kubeCertPath(idx KeyIndex, kubename string)
 }
 
 // AddKnownHostKeys adds a new entry to `known_hosts` file.
-func (fs *fsLocalNonSessionKeyStore) AddKnownHostKeys(hostname string, proxyHost string, hostKeys []ssh.PublicKey) (retErr error) {
+func (fs *fsLocalNonSessionKeyStore) AddKnownHostKeys(hostname, proxyHost string, hostKeys []ssh.PublicKey) (retErr error) {
 	fp, err := os.OpenFile(fs.knownHostsPath(), os.O_CREATE|os.O_RDWR, 0640)
 	if err != nil {
 		return trace.ConvertSystemError(err)
@@ -576,15 +576,15 @@ func (fs *fsLocalNonSessionKeyStore) AddKnownHostKeys(hostname string, proxyHost
 // The `pattern` may be prefixed with `*.` which will match exactly one domain
 // segment, meaning `*.example.com` will match `foo.example.com` but not
 // `foo.bar.example.com`.
-func matchesWildcard(hostname string, pattern string) bool {
+func matchesWildcard(hostname, pattern string) bool {
 	// Don't allow non-wildcard patterns.
 	if !strings.HasPrefix(pattern, "*.") {
 		return false
 	}
 
 	// Don't allow empty matches.
 	pattern = pattern[2:]
-	if pattern == "" {
+	if strings.TrimSpace(pattern) == "" {
 		return false
 	}
 
@@ -615,10 +615,7 @@ func (fs *fsLocalNonSessionKeyStore) GetKnownHostKeys(hostname string) ([]ssh.Pu
 			hostMatch = (hostname == "")
 			if !hostMatch {
 				for i := range hosts {
-					if hosts[i] == hostname {
-						hostMatch = true
-						break
-					} else if matchesWildcard(hostname, hosts[i]) {
+					if hosts[i] == hostname || matchesWildcard(hostname, hosts[i]) {
 						hostMatch = true
 						break
 					}
@@ -706,7 +703,7 @@ func (noLocalKeyStore) DeleteUserCerts(idx KeyIndex, opts ...CertOption) error {
 	return errNoLocalKeyStore
 }
 func (noLocalKeyStore) DeleteKeys() error { return errNoLocalKeyStore }
-func (noLocalKeyStore) AddKnownHostKeys(hostname string, proxyHost string, keys []ssh.PublicKey) error {
+func (noLocalKeyStore) AddKnownHostKeys(hostname, proxyHost string, keys []ssh.PublicKey) error {
 	return errNoLocalKeyStore
 }
 func (noLocalKeyStore) GetKnownHostKeys(hostname string) ([]ssh.PublicKey, error) {

lib/client/known_hosts_migrate.go

@@ -43,15 +43,15 @@ func parseKnownHost(raw string) (*knownHostEntry, error) {
 	// text to preserve formatting for all passed-through entries.
 	marker, hosts, pubKey, comment, _, err := ssh.ParseKnownHosts([]byte(raw))
 	if err != nil {
-		return nil, err
+		return nil, trace.Wrap(err)
 	}
 
 	return &knownHostEntry{
-		raw,
-		marker,
-		hosts,
-		pubKey,
-		comment,
+		raw:     raw,
+		marker:  marker,
+		hosts:   hosts,
+		pubKey:  pubKey,
+		comment: comment,
 	}, nil
 }
 
@@ -126,7 +126,7 @@ func pruneOldHostKeys(output []string) []string {
 		parsed, err := parseKnownHost(line)
 		if err != nil {
 			// If the line isn't parseable, pass it through.
-			log.Debugf("Unable to parse known host on line %d, skipping", i+1)
+			log.WithError(err).Debugf("Unable to parse known host on line %d, skipping", i+1)
 			prunedOutput = append(prunedOutput, line)
 			continue
 		}

tool/tsh/config.go

@@ -44,10 +44,7 @@ Host *.{{ .clusterName }} !{{ .proxyHost }}
 
 // writeSSHConfig generates an OpenSSH config block from the `sshConfigTemplate`
 // template string.
-func writeSSHConfig(
-	sb *strings.Builder, clusterName string, knownHostsPath string,
-	proxyHost string, proxyPort string, leaf bool,
-) error {
+func writeSSHConfig(sb *strings.Builder, clusterName string, knownHostsPath string, proxyHost string, proxyPort string, leaf bool) error {
 	t, err := template.New("ssh-config").Parse(sshConfigTemplate)
 	if err != nil {
 		return trace.Wrap(err)
@@ -61,7 +58,7 @@ func writeSSHConfig(
 		"leaf":           leaf,
 	})
 	if err != nil {
-		return trace.WrapWithMessage(err, "Error generating SSH configuration from template")
+		return trace.WrapWithMessage(err, "error generating SSH configuration from template")
 	}
 
 	return nil