diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-30 18:07:02 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-30 18:07:02 +0300 |
commit | 826cf5293fb78029f76c5e769696e3b37e681207 (patch) | |
tree | 703bc997b5fa36c42e2bd7486f000ad41b01d252 /workhorse | |
parent | 7aa22e9a103b049dd2da70045a5822c51164f7db (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'workhorse')
-rw-r--r-- | workhorse/gitaly_integration_test.go | 1 | ||||
-rw-r--r-- | workhorse/gitaly_test.go | 3 | ||||
-rw-r--r-- | workhorse/internal/api/api.go | 9 | ||||
-rw-r--r-- | workhorse/internal/git/archive.go | 6 | ||||
-rw-r--r-- | workhorse/internal/git/blob.go | 6 | ||||
-rw-r--r-- | workhorse/internal/git/diff.go | 6 | ||||
-rw-r--r-- | workhorse/internal/git/format-patch.go | 6 | ||||
-rw-r--r-- | workhorse/internal/git/info-refs.go | 7 | ||||
-rw-r--r-- | workhorse/internal/git/receive-pack.go | 7 | ||||
-rw-r--r-- | workhorse/internal/git/snapshot.go | 6 | ||||
-rw-r--r-- | workhorse/internal/git/upload-pack.go | 7 | ||||
-rw-r--r-- | workhorse/internal/gitaly/gitaly.go | 63 | ||||
-rw-r--r-- | workhorse/internal/gitaly/gitaly_test.go | 87 | ||||
-rw-r--r-- | workhorse/main_test.go | 1 |
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", |