Skip to content

gitserver: suppress false clone error log message

Warren Gifford requested to merge sg/gitserver-no-false-error into master

Created by: slimsag

TL;DR

This code used to assume req.URL existed and try to clone it. In reality, it only exists in certain circumstances. The end result (request failure) was the correct behavior, and that behavior remains, but attempting the clone is needless and results in a log message that is very misleading and looks like a bug on our end.

Unless you want to understand how I've confirmed this, etc. you can stop reading here. :)

Problem

Under some circumstances (such as improperly configured SSH cloning), I have observed errors from gitserver like:

t=2019-01-16T07:25:26+0000 lvl=dbug msg="error cloning repo" repo=github.com/slimsag/private-repo err="error cloning repo: repo github.com/slimsag/private-repo ([email protected]:slimsag/private-repo.git) not cloneable: exit status 128 (output follows)\n\nHost key verification failed.\r\nfatal: Could not read from remote repository.\n\nPlease make sure you have the correct access rights\nand the repository exists.\n"
t=2019-01-16T07:25:26+0000 lvl=dbug msg="error cloning repo" repo=github.com/slimsag/private-repo err="error cloning repo: repo github.com/slimsag/private-repo () not cloneable: exit status 128 (output follows)\n\nfatal: no path specified; see 'git help pull' for valid url syntax\n"

Fundamentally, the problem here is that there is an inaccessible repository (e.g. the user has not configured authentication for that repo correctly).

However, the second log message is very strange in nature and looks like we might have a serious bug: why would we try to clone something via git without specifying the URL?

I've spent a long time debugging this and trying to track down the cause, and a user also thought this might indicate a more serious issue somewhere.

Cause investigation

Per the documentation on gitserver.Repo.URL, the URL field is optional and not always present:

// URL is the repository's Git remote URL. If the gitserver already has cloned the repository,
// this field is optional (it will use the last-used Git remote URL). If the repository is not
// cloned on the gitserver, the request will fail.
URL string

Looking at gitserver.Repo references shows this is indeed often the case.

The behavior we have today when executing git commands can be simply summarized as: "If the repository doesn't exist, we try to clone it. If we can't clone it, the git exec request fails."

But how does that actually happen in practice? During git exec requests, we clone the repo if the directory doesn't exist:

	if !repoCloned(dir) {
		cloneProgress, err := s.cloneRepo(ctx, req.Repo, req.URL, nil)
		if err != nil {
			log15.Debug("error cloning repo", "repo", req.Repo, "err", err)

And therein lies the problem: we don't have a req.URL and yet we're trying to clone using it.

Merge request reports