Add latest changes from gitlab-org/gitlab@master

14 files changed, 57 insertions, 158 deletions

diff --git a/workhorse/gitaly_integration_test.go b/workhorse/gitaly_integration_test.go
index a2826c3edc4..ed44aaddbc3 100644
--- a/workhorse/gitaly_integration_test.go
+++ b/workhorse/gitaly_integration_test.go
@@ -58,7 +58,6 @@ func ensureGitalyRepository(t *testing.T, apiResponse *api.Response) error {
 	ctx, namespace, err := gitaly.NewNamespaceClient(
 		context.Background(),
 		apiResponse.GitalyServer,
-		gitaly.WithFeatures(apiResponse.GitalyServer.Features),
 	)
 
 	if err != nil {
diff --git a/workhorse/gitaly_test.go b/workhorse/gitaly_test.go
index b9cf4fcc685..2d7f727003f 100644
--- a/workhorse/gitaly_test.go
+++ b/workhorse/gitaly_test.go
@@ -78,7 +78,7 @@ func TestGetInfoRefsProxiedToGitalySuccessfully(t *testing.T) {
 	for k, v := range badMetadata {
 		features[k] = v
 	}
-	apiResponse.GitalyServer.Features = features
+	apiResponse.GitalyServer.CallMetadata = features
 
 	testCases := []struct {
 		showAllRefs bool
@@ -272,7 +272,6 @@ func TestPostReceivePackProxiedToGitalySuccessfully(t *testing.T) {
 	require.Equal(t, apiResponse.Repository.RelativePath, gitalyRequest.Repository.RelativePath)
 	require.Equal(t, apiResponse.GL_ID, gitalyRequest.GlId)
 	require.Equal(t, apiResponse.GL_USERNAME, gitalyRequest.GlUsername)
-	require.Equal(t, apiResponse.RemoteIp, "1.2.3.4")
 	require.Equal(t, apiResponse.GitConfigOptions, gitalyRequest.GitConfigOptions)
 	require.Equal(t, gitProtocol, gitalyRequest.GitProtocol)
 
diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go
index ba27a3e6ec9..011890be569 100644
--- a/workhorse/internal/api/api.go
+++ b/workhorse/internal/api/api.go
@@ -129,9 +129,6 @@ type Response struct {
 	// 'git push' and 'git pull'
 	GL_REPOSITORY string
 
-	// RemoteIp holds the IP of the request issuing the action
-	RemoteIp string
-
 	// GitConfigOptions holds the custom options that we want to pass to the git command
 	GitConfigOptions []string
 	// StoreLFSPath is provided by the GitLab Rails application to mark where the tmp file should be placed.
@@ -166,9 +163,9 @@ type Response struct {
 }
 
 type GitalyServer struct {
-	Address  string            `json:"address"`
-	Token    string            `json:"token"`
-	Features map[string]string `json:"features"`
+	Address      string            `json:"address"`
+	Token        string            `json:"token"`
+	CallMetadata map[string]string `json:"call_metadata"`
 }
 
 // singleJoiningSlash is taken from reverseproxy.go:singleJoiningSlash
diff --git a/workhorse/internal/git/archive.go b/workhorse/internal/git/archive.go
index 4c7b519310f..5c6bc2e2266 100644
--- a/workhorse/internal/git/archive.go
+++ b/workhorse/internal/git/archive.go
@@ -132,11 +132,7 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string
 
 func handleArchiveWithGitaly(r *http.Request, params *archiveParams, format gitalypb.GetArchiveRequest_Format) (io.Reader, error) {
 	var request *gitalypb.GetArchiveRequest
-	ctx, c, err := gitaly.NewRepositoryClient(
-		r.Context(),
-		params.GitalyServer,
-		gitaly.WithFeatures(params.GitalyServer.Features),
-	)
+	ctx, c, err := gitaly.NewRepositoryClient(r.Context(), params.GitalyServer)
 
 	if err != nil {
 		return nil, err
diff --git a/workhorse/internal/git/blob.go b/workhorse/internal/git/blob.go
index 39bd4490e66..a6d9cd8b1da 100644
--- a/workhorse/internal/git/blob.go
+++ b/workhorse/internal/git/blob.go
@@ -27,11 +27,7 @@ func (b *blob) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 		return
 	}
 
-	ctx, blobClient, err := gitaly.NewBlobClient(
-		r.Context(),
-		params.GitalyServer,
-		gitaly.WithFeatures(params.GitalyServer.Features),
-	)
+	ctx, blobClient, err := gitaly.NewBlobClient(r.Context(), params.GitalyServer)
 
 	if err != nil {
 		helper.Fail500(w, r, fmt.Errorf("blob.GetBlob: %v", err))
diff --git a/workhorse/internal/git/diff.go b/workhorse/internal/git/diff.go
index b4878384e2b..9ae95bb6680 100644
--- a/workhorse/internal/git/diff.go
+++ b/workhorse/internal/git/diff.go
@@ -34,11 +34,7 @@ func (d *diff) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 		return
 	}
 
-	ctx, diffClient, err := gitaly.NewDiffClient(
-		r.Context(),
-		params.GitalyServer,
-		gitaly.WithFeatures(params.GitalyServer.Features),
-	)
+	ctx, diffClient, err := gitaly.NewDiffClient(r.Context(), params.GitalyServer)
 	if err != nil {
 		helper.Fail500(w, r, fmt.Errorf("diff.RawDiff: %v", err))
 		return
diff --git a/workhorse/internal/git/format-patch.go b/workhorse/internal/git/format-patch.go
index 264a4001232..d0b2e875c95 100644
--- a/workhorse/internal/git/format-patch.go
+++ b/workhorse/internal/git/format-patch.go
@@ -34,11 +34,7 @@ func (p *patch) Inject(w http.ResponseWriter, r *http.Request, sendData string)
 		return
 	}
 
-	ctx, diffClient, err := gitaly.NewDiffClient(
-		r.Context(),
-		params.GitalyServer,
-		gitaly.WithFeatures(params.GitalyServer.Features),
-	)
+	ctx, diffClient, err := gitaly.NewDiffClient(r.Context(), params.GitalyServer)
 
 	if err != nil {
 		helper.Fail500(w, r, fmt.Errorf("diff.RawPatch: %v", err))
diff --git a/workhorse/internal/git/info-refs.go b/workhorse/internal/git/info-refs.go
index 89db2954e30..b7f825839f8 100644
--- a/workhorse/internal/git/info-refs.go
+++ b/workhorse/internal/git/info-refs.go
@@ -55,12 +55,7 @@ func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response)
 }
 
 func handleGetInfoRefsWithGitaly(ctx context.Context, responseWriter *HttpResponseWriter, a *api.Response, rpc, gitProtocol, encoding string) error {
-	ctx, smarthttp, err := gitaly.NewSmartHTTPClient(
-		ctx,
-		a.GitalyServer,
-		gitaly.WithFeatures(a.GitalyServer.Features),
-		gitaly.WithLoggingMetadata(a),
-	)
+	ctx, smarthttp, err := gitaly.NewSmartHTTPClient(ctx, a.GitalyServer)
 	if err != nil {
 		return err
 	}
diff --git a/workhorse/internal/git/receive-pack.go b/workhorse/internal/git/receive-pack.go
index b312bbad621..e3af472fffa 100644
--- a/workhorse/internal/git/receive-pack.go
+++ b/workhorse/internal/git/receive-pack.go
@@ -20,12 +20,7 @@ func handleReceivePack(w *HttpResponseWriter, r *http.Request, a *api.Response)
 
 	gitProtocol := r.Header.Get("Git-Protocol")
 
-	ctx, smarthttp, err := gitaly.NewSmartHTTPClient(
-		r.Context(),
-		a.GitalyServer,
-		gitaly.WithFeatures(a.GitalyServer.Features),
-		gitaly.WithLoggingMetadata(a),
-	)
+	ctx, smarthttp, err := gitaly.NewSmartHTTPClient(r.Context(), a.GitalyServer)
 	if err != nil {
 		return fmt.Errorf("smarthttp.ReceivePack: %v", err)
 	}
diff --git a/workhorse/internal/git/snapshot.go b/workhorse/internal/git/snapshot.go
index 70832ec9211..aadb35a5189 100644
--- a/workhorse/internal/git/snapshot.go
+++ b/workhorse/internal/git/snapshot.go
@@ -41,11 +41,7 @@ func (s *snapshot) Inject(w http.ResponseWriter, r *http.Request, sendData strin
 		return
 	}
 
-	ctx, c, err := gitaly.NewRepositoryClient(
-		r.Context(),
-		params.GitalyServer,
-		gitaly.WithFeatures(params.GitalyServer.Features),
-	)
+	ctx, c, err := gitaly.NewRepositoryClient(r.Context(), params.GitalyServer)
 
 	if err != nil {
 		helper.Fail500(w, r, fmt.Errorf("SendSnapshot: gitaly.NewRepositoryClient: %v", err))
diff --git a/workhorse/internal/git/upload-pack.go b/workhorse/internal/git/upload-pack.go
index 13c0069cde2..74995fb61c8 100644
--- a/workhorse/internal/git/upload-pack.go
+++ b/workhorse/internal/git/upload-pack.go
@@ -44,12 +44,7 @@ func handleUploadPack(w *HttpResponseWriter, r *http.Request, a *api.Response) e
 }
 
 func handleUploadPackWithGitaly(ctx context.Context, a *api.Response, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) error {
-	ctx, smarthttp, err := gitaly.NewSmartHTTPClient(
-		ctx,
-		a.GitalyServer,
-		gitaly.WithFeatures(a.GitalyServer.Features),
-		gitaly.WithLoggingMetadata(a),
-	)
+	ctx, smarthttp, err := gitaly.NewSmartHTTPClient(ctx, a.GitalyServer)
 	if err != nil {
 		return fmt.Errorf("get gitaly client: %w", err)
 	}
diff --git a/workhorse/internal/gitaly/gitaly.go b/workhorse/internal/gitaly/gitaly.go
index 799159689ae..af7425be1cf 100644
--- a/workhorse/internal/gitaly/gitaly.go
+++ b/workhorse/internal/gitaly/gitaly.go
@@ -67,44 +67,23 @@ func InitializeSidechannelRegistry(logger *logrus.Logger) {
 	}
 }
 
-type MetadataFunc func(metadata.MD)
-
-func WithFeatures(features map[string]string) MetadataFunc {
-	return func(md metadata.MD) {
-		for k, v := range features {
-			if !strings.HasPrefix(k, "gitaly-feature-") {
-				continue
-			}
-			md.Append(k, v)
-		}
-	}
-}
-
-func WithLoggingMetadata(r *api.Response) MetadataFunc {
-	return func(md metadata.MD) {
-		if r.GL_ID != "" {
-			md.Append("user_id", r.GL_ID)
-		}
-		if r.GL_USERNAME != "" {
-			md.Append("username", r.GL_USERNAME)
-		}
-		if r.RemoteIp != "" {
-			md.Append("remote_ip", r.RemoteIp)
-		}
-	}
+var allowedMetadataKeys = map[string]bool{
+	"user_id":   true,
+	"username":  true,
+	"remote_ip": true,
 }
 
-func withOutgoingMetadata(ctx context.Context, addMetadataFuncs ...MetadataFunc) context.Context {
+func withOutgoingMetadata(ctx context.Context, gs api.GitalyServer) context.Context {
 	md := metadata.New(nil)
-
-	for _, f := range addMetadataFuncs {
-		f(md)
+	for k, v := range gs.CallMetadata {
+		if strings.HasPrefix(k, "gitaly-feature-") || allowedMetadataKeys[k] {
+			md.Set(k, v)
+		}
 	}
-
 	return metadata.NewOutgoingContext(ctx, md)
 }
 
-func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer, metadataFuncs ...MetadataFunc) (context.Context, *SmartHTTPClient, error) {
+func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer) (context.Context, *SmartHTTPClient, error) {
 	conn, err := getOrCreateConnection(server)
 	if err != nil {
 		return nil, nil, err
@@ -114,48 +93,44 @@ func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer, metadataFu
 		SmartHTTPServiceClient: grpcClient,
 		sidechannelRegistry:    sidechannelRegistry,
 	}
-
-	return withOutgoingMetadata(
-		ctx,
-		metadataFuncs...,
-	), smartHTTPClient, nil
+	return withOutgoingMetadata(ctx, server), smartHTTPClient, nil
 }
 
-func NewBlobClient(ctx context.Context, server api.GitalyServer, addMetadataFuncs ...MetadataFunc) (context.Context, *BlobClient, error) {
+func NewBlobClient(ctx context.Context, server api.GitalyServer) (context.Context, *BlobClient, error) {
 	conn, err := getOrCreateConnection(server)
 	if err != nil {
 		return nil, nil, err
 	}
 	grpcClient := gitalypb.NewBlobServiceClient(conn)
-	return withOutgoingMetadata(ctx, addMetadataFuncs...), &BlobClient{grpcClient}, nil
+	return withOutgoingMetadata(ctx, server), &BlobClient{grpcClient}, nil
 }
 
-func NewRepositoryClient(ctx context.Context, server api.GitalyServer, addMetadataFuncs ...MetadataFunc) (context.Context, *RepositoryClient, error) {
+func NewRepositoryClient(ctx context.Context, server api.GitalyServer) (context.Context, *RepositoryClient, error) {
 	conn, err := getOrCreateConnection(server)
 	if err != nil {
 		return nil, nil, err
 	}
 	grpcClient := gitalypb.NewRepositoryServiceClient(conn)
-	return withOutgoingMetadata(ctx, addMetadataFuncs...), &RepositoryClient{grpcClient}, nil
+	return withOutgoingMetadata(ctx, server), &RepositoryClient{grpcClient}, nil
 }
 
 // NewNamespaceClient is only used by the Gitaly integration tests at present
-func NewNamespaceClient(ctx context.Context, server api.GitalyServer, addMetadataFuncs ...MetadataFunc) (context.Context, *NamespaceClient, error) {
+func NewNamespaceClient(ctx context.Context, server api.GitalyServer) (context.Context, *NamespaceClient, error) {
 	conn, err := getOrCreateConnection(server)
 	if err != nil {
 		return nil, nil, err
 	}
 	grpcClient := gitalypb.NewNamespaceServiceClient(conn)
-	return withOutgoingMetadata(ctx, addMetadataFuncs...), &NamespaceClient{grpcClient}, nil
+	return withOutgoingMetadata(ctx, server), &NamespaceClient{grpcClient}, nil
 }
 
-func NewDiffClient(ctx context.Context, server api.GitalyServer, addMetadataFuncs ...MetadataFunc) (context.Context, *DiffClient, error) {
+func NewDiffClient(ctx context.Context, server api.GitalyServer) (context.Context, *DiffClient, error) {
 	conn, err := getOrCreateConnection(server)
 	if err != nil {
 		return nil, nil, err
 	}
 	grpcClient := gitalypb.NewDiffServiceClient(conn)
-	return withOutgoingMetadata(ctx, addMetadataFuncs...), &DiffClient{grpcClient}, nil
+	return withOutgoingMetadata(ctx, server), &DiffClient{grpcClient}, nil
 }
 
 func getOrCreateConnection(server api.GitalyServer) (*grpc.ClientConn, error) {
diff --git a/workhorse/internal/gitaly/gitaly_test.go b/workhorse/internal/gitaly/gitaly_test.go
index f81dc16149a..0ea5da20da3 100644
--- a/workhorse/internal/gitaly/gitaly_test.go
+++ b/workhorse/internal/gitaly/gitaly_test.go
@@ -21,17 +21,9 @@ func TestNewSmartHTTPClient(t *testing.T) {
 	ctx, client, err := NewSmartHTTPClient(
 		context.Background(),
 		serverFixture(),
-		WithFeatures(features()),
-		WithLoggingMetadata(&api.Response{
-			GL_USERNAME: "gl_username",
-			GL_ID:       "gl_id",
-			RemoteIp:    "1.2.3.4",
-		}),
 	)
 	require.NoError(t, err)
 	testOutgoingMetadata(t, ctx)
-	testOutgoingIDAndUsername(t, ctx)
-	testOutgoingRemoteIP(t, ctx)
 	require.NotNil(t, client.sidechannelRegistry)
 }
 
@@ -39,7 +31,6 @@ func TestNewBlobClient(t *testing.T) {
 	ctx, _, err := NewBlobClient(
 		context.Background(),
 		serverFixture(),
-		WithFeatures(features()),
 	)
 	require.NoError(t, err)
 	testOutgoingMetadata(t, ctx)
@@ -49,7 +40,6 @@ func TestNewRepositoryClient(t *testing.T) {
 	ctx, _, err := NewRepositoryClient(
 		context.Background(),
 		serverFixture(),
-		WithFeatures(features()),
 	)
 
 	require.NoError(t, err)
@@ -60,7 +50,6 @@ func TestNewNamespaceClient(t *testing.T) {
 	ctx, _, err := NewNamespaceClient(
 		context.Background(),
 		serverFixture(),
-		WithFeatures(features()),
 	)
 	require.NoError(t, err)
 	testOutgoingMetadata(t, ctx)
@@ -70,69 +59,45 @@ func TestNewDiffClient(t *testing.T) {
 	ctx, _, err := NewDiffClient(
 		context.Background(),
 		serverFixture(),
-		WithFeatures(features()),
 	)
 	require.NoError(t, err)
 	testOutgoingMetadata(t, ctx)
 }
 
 func testOutgoingMetadata(t *testing.T, ctx context.Context) {
+	t.Helper()
 	md, ok := metadata.FromOutgoingContext(ctx)
 	require.True(t, ok, "get metadata from context")
 
-	for k, v := range allowedFeatures() {
-		actual := md[k]
-		require.Len(t, actual, 1, "expect one value for %v", k)
-		require.Equal(t, v, actual[0], "value for %v", k)
-	}
-
-	for k := range badFeatureMetadata() {
-		require.Empty(t, md[k], "value for bad key %v", k)
-	}
-}
-
-func testOutgoingIDAndUsername(t *testing.T, ctx context.Context) {
-	md, ok := metadata.FromOutgoingContext(ctx)
-	require.True(t, ok, "get metadata from context")
-
-	require.Equal(t, md["user_id"], []string{"gl_id"})
-	require.Equal(t, md["username"], []string{"gl_username"})
-}
-
-func testOutgoingRemoteIP(t *testing.T, ctx context.Context) {
-	md, ok := metadata.FromOutgoingContext(ctx)
-	require.True(t, ok, "get metadata from context")
-
-	require.Equal(t, md["remote_ip"], []string{"1.2.3.4"})
-}
-
-func features() map[string]string {
-	features := make(map[string]string)
-	for k, v := range allowedFeatures() {
-		features[k] = v
-	}
-
-	for k, v := range badFeatureMetadata() {
-		features[k] = v
-	}
-
-	return features
+	require.Equal(t, metadata.MD{"username": {"janedoe"}}, md)
 }
 
 func serverFixture() api.GitalyServer {
-	return api.GitalyServer{Address: "tcp://localhost:123"}
-}
-
-func allowedFeatures() map[string]string {
-	return map[string]string{
-		"gitaly-feature-foo": "bar",
-		"gitaly-feature-qux": "baz",
+	return api.GitalyServer{
+		Address:      "tcp://localhost:123",
+		CallMetadata: map[string]string{"username": "janedoe"},
 	}
 }
 
-func badFeatureMetadata() map[string]string {
-	return map[string]string{
-		"bad-metadata-1": "bad-value-1",
-		"bad-metadata-2": "bad-value-2",
-	}
+func TestWithOutgoingMetadata(t *testing.T) {
+	ctx := withOutgoingMetadata(context.Background(), api.GitalyServer{
+		CallMetadata: map[string]string{
+			"gitaly-feature-abc":    "true",
+			"gitaly-featuregarbage": "blocked",
+			"bad-header":            "blocked",
+			"user_id":               "234",
+			"username":              "janedoe",
+			"remote_ip":             "1.2.3.4",
+		},
+	})
+
+	md, ok := metadata.FromOutgoingContext(ctx)
+	require.True(t, ok)
+
+	require.Equal(t, metadata.MD{
+		"gitaly-feature-abc": {"true"},
+		"user_id":            {"234"},
+		"username":           {"janedoe"},
+		"remote_ip":          {"1.2.3.4"},
+	}, md)
 }
diff --git a/workhorse/main_test.go b/workhorse/main_test.go
index 8dce3480e0b..5ebc26c7ac7 100644
--- a/workhorse/main_test.go
+++ b/workhorse/main_test.go
@@ -813,7 +813,6 @@ func gitOkBody(t *testing.T) *api.Response {
 	return &api.Response{
 		GL_ID:       "user-123",
 		GL_USERNAME: "username",
-		RemoteIp:    "1.2.3.4",
 		Repository: gitalypb.Repository{
 			StorageName:  "default",
 			RelativePath: "foo/bar.git",