Fix handling of Git object dir attributes for streaming RPCsfix/handling-object-dir-handler-for-stream-rpcs

Fixes #564

3 files changed, 14 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b21b2c79..9f67356cd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@ UNRELEASED
 
 - Wait for monitor goroutine to return during supervisor shutdown
   https://gitlab.com/gitlab-org/gitaly/merge_requests/341
+- Fix handling of Git object dir attributes for streaming RPCs
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/356
 
 v0.40.0
 - Use context cancellation instead of command.Close
diff --git a/internal/server/server.go b/internal/server/server.go
index 72c9f1904..4c82bb3e7 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -31,13 +31,15 @@ func New(rubyServer *rubyserver.Server) *grpc.Server {
 
 	server := grpc.NewServer(
 		grpc.StreamInterceptor(grpc_middleware.ChainStreamServer(
-			objectdirhandler.Stream,
 			grpc_ctxtags.StreamServerInterceptor(ctxTagOpts...),
 			grpc_prometheus.StreamServerInterceptor,
 			grpc_logrus.StreamServerInterceptor(logrusEntry),
 			sentryhandler.StreamLogHandler,
 			cancelhandler.Stream, // Should be below LogHandler
 			authStreamServerInterceptor(),
+			// Should be the last one, or above interceptors that
+			// don't replace the stream context with a derived one.
+			objectdirhandler.Stream,
 			// Panic handler should remain last so that application panics will be
 			// converted to errors and logged
 			panichandler.StreamPanicHandler,
diff --git a/internal/service/commit/testhelper_test.go b/internal/service/commit/testhelper_test.go
index 7b3eaee14..a8c01cdec 100644
--- a/internal/service/commit/testhelper_test.go
+++ b/internal/service/commit/testhelper_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/grpc-ecosystem/go-grpc-middleware/tags"
 	"gitlab.com/gitlab-org/gitaly/internal/linguist"
 	"gitlab.com/gitlab-org/gitaly/internal/middleware/objectdirhandler"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
@@ -51,7 +52,14 @@ func testMain(m *testing.M) int {
 func startTestServices(t *testing.T) *grpc.Server {
 	server := testhelper.NewTestGrpcServer(
 		t,
-		[]grpc.StreamServerInterceptor{objectdirhandler.Stream},
+		[]grpc.StreamServerInterceptor{
+			// This interceptor only exists to make sure objectdirhandler works properly.
+			// If they switched positions, a test would fail.
+			grpc_ctxtags.StreamServerInterceptor(),
+			// Should be the last one, or above interceptors that
+			// don't replace the stream context with a derived one.
+			objectdirhandler.Stream,
+		},
 		[]grpc.UnaryServerInterceptor{objectdirhandler.Unary},
 	)