Skip to content

Commit

Permalink
libgit2: fix access to nil t.stdin and improve observability
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Paulo Gomes committed Mar 30, 2022
1 parent ae0b38c commit 36fcdee
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 9 deletions.
9 changes: 7 additions & 2 deletions pkg/git/libgit2/managed/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,13 @@ func (self *httpSmartSubtransportStream) Free() {
// ensure body is fully processed and closed
// for increased likelihood of transport reuse in HTTP/1.x.
// it should not be a problem to do this more than once.
_, _ = io.Copy(io.Discard, self.resp.Body) // errors can be safely ignored
_ = self.resp.Body.Close() // errors can be safely ignored
if _, err := io.Copy(io.Discard, self.resp.Body); err != nil {
traceLog.Error(err, "[http]: cannot discard response body")
}

if err := self.resp.Body.Close(); err != nil {
traceLog.Error(err, "[http]: cannot close response body")
}
}
}
}
Expand Down
24 changes: 17 additions & 7 deletions pkg/git/libgit2/managed/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
if t.lastAction == git2go.SmartServiceActionUploadpackLs {
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)

Expand All @@ -146,7 +148,9 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
if t.lastAction == git2go.SmartServiceActionReceivepackLs {
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)

Expand All @@ -161,11 +165,11 @@ func (t *sshSmartSubtransport) Action(urlString string, action git2go.SmartServi
defer cred.Free()

var addr string
port := "22"
if u.Port() != "" {
addr = fmt.Sprintf("%s:%s", u.Hostname(), u.Port())
} else {
addr = fmt.Sprintf("%s:22", u.Hostname())
port = u.Port()
}
addr = fmt.Sprintf("%s:%s", u.Hostname(), port)

ckey, sshConfig, err := cacheKeyAndConfig(addr, cred)
if err != nil {
Expand Down Expand Up @@ -264,19 +268,21 @@ func (t *sshSmartSubtransport) Close() error {

traceLog.Info("[ssh]: sshSmartSubtransport.Close()")
t.currentStream = nil
if t.client != nil {
if t.client != nil && t.stdin != nil {
if err := t.stdin.Close(); err != nil {
returnErr = fmt.Errorf("cannot close stdin: %w", err)
}
t.client = nil
}
t.client = nil

if t.session != nil {
traceLog.Info("[ssh]: skipping session.wait")
traceLog.Info("[ssh]: session.Close()")
if err := t.session.Close(); err != nil {
returnErr = fmt.Errorf("cannot close session: %w", err)
}
}
t.session = nil

return returnErr
}
Expand All @@ -302,6 +308,10 @@ func (stream *sshSmartSubtransportStream) Free() {
}

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()
if err != nil {
return "", nil, err
Expand Down

0 comments on commit 36fcdee

Please sign in to comment.