Commit 772df282 authored by Igor Drozdov's avatar Igor Drozdov

Improve Git HTTPS message given when Gitaly unavailable

Changelog: changed
parent df77281e
......@@ -11,6 +11,9 @@ module Repositories
rescue_from Gitlab::GitAccess::NotFoundError, with: :render_404_with_exception
rescue_from Gitlab::GitAccessProject::CreationError, with: :render_422_with_exception
rescue_from Gitlab::GitAccess::TimeoutError, with: :render_503_with_exception
rescue_from GRPC::Unavailable do |e|
render_503_with_exception(e, message: 'The git server, Gitaly, is not available at this time. Please contact your administrator.')
end
# GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
# GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
......@@ -71,8 +74,8 @@ module Repositories
render plain: exception.message, status: :unprocessable_entity
end
def render_503_with_exception(exception)
render plain: exception.message, status: :service_unavailable
def render_503_with_exception(exception, message: nil)
render plain: message || exception.message, status: :service_unavailable
end
def update_fetch_statistics
......
......@@ -7,12 +7,33 @@ RSpec.describe Repositories::GitHttpController do
let_it_be(:personal_snippet) { create(:personal_snippet, :public, :repository) }
let_it_be(:project_snippet) { create(:project_snippet, :public, :repository, project: project) }
shared_examples 'handles unavailable Gitaly' do
let(:params) { super().merge(service: 'git-upload-pack') }
before do
request.headers.merge! auth_env(user.username, user.password, nil)
end
context 'when Gitaly is unavailable' do
it 'responds with a 503 message' do
expect(Gitlab::GitalyClient).to receive(:call).and_raise(GRPC::Unavailable)
get :info_refs, params: params
expect(response).to have_gitlab_http_status(:service_unavailable)
expect(response.body).to eq('The git server, Gitaly, is not available at this time. Please contact your administrator.')
end
end
end
context 'when repository container is a project' do
it_behaves_like Repositories::GitHttpController do
let(:container) { project }
let(:user) { project.owner }
let(:access_checker_class) { Gitlab::GitAccess }
it_behaves_like 'handles unavailable Gitaly'
describe 'POST #git_upload_pack' do
before do
allow(controller).to receive(:verify_workhorse_api!).and_return(true)
......@@ -84,6 +105,8 @@ RSpec.describe Repositories::GitHttpController do
let(:container) { personal_snippet }
let(:user) { personal_snippet.author }
let(:access_checker_class) { Gitlab::GitAccessSnippet }
it_behaves_like 'handles unavailable Gitaly'
end
end
......@@ -92,6 +115,8 @@ RSpec.describe Repositories::GitHttpController do
let(:container) { project_snippet }
let(:user) { project_snippet.author }
let(:access_checker_class) { Gitlab::GitAccessSnippet }
it_behaves_like 'handles unavailable Gitaly'
end
end
end
......@@ -8,8 +8,8 @@ import (
"net/http"
"github.com/golang/gddo/httputil"
"gitlab.com/gitlab-org/labkit/log"
grpccodes "google.golang.org/grpc/codes"
grpcstatus "google.golang.org/grpc/status"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly"
......@@ -26,6 +26,7 @@ func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response)
defer responseWriter.Log(r, 0)
rpc := getService(r)
if !(rpc == "git-upload-pack" || rpc == "git-receive-pack") {
// The 'dumb' Git HTTP protocol is not supported
http.Error(responseWriter, "Not Found", 404)
......@@ -41,19 +42,26 @@ func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response)
encoding := httputil.NegotiateContentEncoding(r, offers)
if err := handleGetInfoRefsWithGitaly(r.Context(), responseWriter, a, rpc, gitProtocol, encoding); err != nil {
helper.Fail500(responseWriter, r, fmt.Errorf("handleGetInfoRefs: %v", err))
status := grpcstatus.Convert(err)
err = fmt.Errorf("handleGetInfoRefs: %v", err)
if status != nil && status.Code() == grpccodes.Unavailable {
helper.CaptureAndFail(responseWriter, r, err, "The git server, Gitaly, is not available at this time. Please contact your administrator.", http.StatusServiceUnavailable)
} else {
helper.Fail500(responseWriter, r, err)
}
}
}
func handleGetInfoRefsWithGitaly(ctx context.Context, responseWriter *HttpResponseWriter, a *api.Response, rpc, gitProtocol, encoding string) error {
ctx, smarthttp, err := gitaly.NewSmartHTTPClient(ctx, a.GitalyServer)
if err != nil {
return fmt.Errorf("GetInfoRefsHandler: %v", err)
return err
}
infoRefsResponseReader, err := smarthttp.InfoRefsResponseReader(ctx, &a.Repository, rpc, gitConfigOptions(a), gitProtocol)
if err != nil {
return fmt.Errorf("GetInfoRefsHandler: %v", err)
return err
}
var w io.Writer
......@@ -69,7 +77,7 @@ func handleGetInfoRefsWithGitaly(ctx context.Context, responseWriter *HttpRespon
}
if _, err = io.Copy(w, infoRefsResponseReader); err != nil {
log.WithError(err).Error("GetInfoRefsHandler: error copying gitaly response")
return err
}
return nil
......
package git
import (
"net/http/httptest"
"testing"
"github.com/stretchr/testify/require"
grpccodes "google.golang.org/grpc/codes"
grpcstatus "google.golang.org/grpc/status"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly"
)
type smartHTTPServiceServerWithInfoRefs struct {
gitalypb.UnimplementedSmartHTTPServiceServer
InfoRefsUploadPackFunc func(*gitalypb.InfoRefsRequest, gitalypb.SmartHTTPService_InfoRefsUploadPackServer) error
}
func (srv *smartHTTPServiceServerWithInfoRefs) InfoRefsUploadPack(r *gitalypb.InfoRefsRequest, s gitalypb.SmartHTTPService_InfoRefsUploadPackServer) error {
return srv.InfoRefsUploadPackFunc(r, s)
}
func TestGetInfoRefsHandler(t *testing.T) {
addr := startSmartHTTPServer(t, &smartHTTPServiceServerWithInfoRefs{
InfoRefsUploadPackFunc: func(r *gitalypb.InfoRefsRequest, s gitalypb.SmartHTTPService_InfoRefsUploadPackServer) error {
return grpcstatus.Error(grpccodes.Unavailable, "error")
},
})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/?service=git-upload-pack", nil)
a := &api.Response{GitalyServer: gitaly.Server{Address: addr}}
handleGetInfoRefs(NewHttpResponseWriter(w), r, a)
require.Equal(t, 503, w.Code)
msg := "The git server, Gitaly, is not available at this time. Please contact your administrator.\n"
require.Equal(t, msg, w.Body.String())
}
......@@ -45,14 +45,13 @@ func TestUploadPackTimesOut(t *testing.T) {
uploadPackTimeout = time.Millisecond
defer func() { uploadPackTimeout = originalUploadPackTimeout }()
addr, cleanUp := startSmartHTTPServer(t, &smartHTTPServiceServer{
addr := startSmartHTTPServer(t, &smartHTTPServiceServer{
PostUploadPackFunc: func(stream gitalypb.SmartHTTPService_PostUploadPackServer) error {
_, err := stream.Recv() // trigger a read on the client request body
require.NoError(t, err)
return nil
},
})
defer cleanUp()
body := &fakeReader{n: 0, err: nil}
......@@ -64,7 +63,9 @@ func TestUploadPackTimesOut(t *testing.T) {
require.EqualError(t, err, "smarthttp.UploadPack: busyReader: context deadline exceeded")
}
func startSmartHTTPServer(t testing.TB, s gitalypb.SmartHTTPServiceServer) (string, func()) {
func startSmartHTTPServer(t testing.TB, s gitalypb.SmartHTTPServiceServer) string {
t.Helper()
tmp, err := ioutil.TempDir("", "")
require.NoError(t, err)
......@@ -78,8 +79,10 @@ func startSmartHTTPServer(t testing.TB, s gitalypb.SmartHTTPServiceServer) (stri
require.NoError(t, srv.Serve(ln))
}()
return fmt.Sprintf("%s://%s", ln.Addr().Network(), ln.Addr().String()), func() {
t.Cleanup(func() {
srv.GracefulStop()
require.NoError(t, os.RemoveAll(tmp), "error removing temp dir %q", tmp)
}
})
return fmt.Sprintf("%s://%s", ln.Addr().Network(), ln.Addr().String())
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment