libgit2: fix access to nil t.stdin and improve observability

All errors that were previously not handled are now logged through
traceLog, to further help during transport investigations.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
This commit is contained in:
Paulo Gomes 2022-03-30 14:32:15 +01:00
parent ae0b38cba2
commit 36fcdeeb5e
No known key found for this signature in database
GPG Key ID: 9995233870E99BEE
2 changed files with 24 additions and 9 deletions

View File

@ -292,8 +292,13 @@ func (self *httpSmartSubtransportStream) Free() {
// ensure body is fully processed and closed // ensure body is fully processed and closed
// for increased likelihood of transport reuse in HTTP/1.x. // for increased likelihood of transport reuse in HTTP/1.x.
// it should not be a problem to do this more than once. // it should not be a problem to do this more than once.
_, _ = io.Copy(io.Discard, self.resp.Body) // errors can be safely ignored if _, err := io.Copy(io.Discard, self.resp.Body); err != nil {
_ = self.resp.Body.Close() // errors can be safely ignored traceLog.Error(err, "[http]: cannot discard response body")
}
if err := self.resp.Body.Close(); err != nil {
traceLog.Error(err, "[http]: cannot close response body")
}
} }
} }
} }

View File

@ -137,7 +137,9 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
if t.lastAction == git2go.SmartServiceActionUploadpackLs { if t.lastAction == git2go.SmartServiceActionUploadpackLs {
return t.currentStream, nil return t.currentStream, nil
} }
t.Close() if err := t.Close(); err != nil {
traceLog.Error(err, "[ssh]: error cleaning up previous stream")
}
} }
cmd = fmt.Sprintf("git-upload-pack '%s'", uPath) cmd = fmt.Sprintf("git-upload-pack '%s'", uPath)
@ -146,7 +148,9 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
if t.lastAction == git2go.SmartServiceActionReceivepackLs { if t.lastAction == git2go.SmartServiceActionReceivepackLs {
return t.currentStream, nil return t.currentStream, nil
} }
t.Close() if err := t.Close(); err != nil {
traceLog.Error(err, "[ssh]: error cleaning up previous stream")
}
} }
cmd = fmt.Sprintf("git-receive-pack '%s'", uPath) cmd = fmt.Sprintf("git-receive-pack '%s'", uPath)
@ -161,11 +165,11 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
defer cred.Free() defer cred.Free()
var addr string var addr string
port := "22"
if u.Port() != "" { if u.Port() != "" {
addr = fmt.Sprintf("%s:%s", u.Hostname(), u.Port()) port = u.Port()
} else {
addr = fmt.Sprintf("%s:22", u.Hostname())
} }
addr = fmt.Sprintf("%s:%s", u.Hostname(), port)
ckey, sshConfig, err := cacheKeyAndConfig(addr, cred) ckey, sshConfig, err := cacheKeyAndConfig(addr, cred)
if err != nil { if err != nil {
@ -264,12 +268,13 @@ func (t *sshSmartSubtransport) Close() error {
traceLog.Info("[ssh]: sshSmartSubtransport.Close()") traceLog.Info("[ssh]: sshSmartSubtransport.Close()")
t.currentStream = nil t.currentStream = nil
if t.client != nil { if t.client != nil && t.stdin != nil {
if err := t.stdin.Close(); err != nil { if err := t.stdin.Close(); err != nil {
returnErr = fmt.Errorf("cannot close stdin: %w", err) returnErr = fmt.Errorf("cannot close stdin: %w", err)
} }
t.client = nil
} }
t.client = nil
if t.session != nil { if t.session != nil {
traceLog.Info("[ssh]: skipping session.wait") traceLog.Info("[ssh]: skipping session.wait")
traceLog.Info("[ssh]: session.Close()") traceLog.Info("[ssh]: session.Close()")
@ -277,6 +282,7 @@ func (t *sshSmartSubtransport) Close() error {
returnErr = fmt.Errorf("cannot close session: %w", err) returnErr = fmt.Errorf("cannot close session: %w", err)
} }
} }
t.session = nil
return returnErr return returnErr
} }
@ -302,6 +308,10 @@ func (stream *sshSmartSubtransportStream) Free() {
} }
func cacheKeyAndConfig(remoteAddress string, cred *git2go.Credential) (string, *ssh.ClientConfig, error) { func cacheKeyAndConfig(remoteAddress string, cred *git2go.Credential) (string, *ssh.ClientConfig, error) {
if cred == nil {
return "", nil, fmt.Errorf("cannot create cache key from a nil credential")
}
username, _, privatekey, passphrase, err := cred.GetSSHKey() username, _, privatekey, passphrase, err := cred.GetSSHKey()
if err != nil { if err != nil {
return "", nil, err return "", nil, err