Merge branch 'po-fix-invalid-request-cache-panic' into 'master'

Fix middleware to stop panicking from bad requests

Closes #2262

See merge request gitlab-org/gitaly!1747

4 files changed, 29 insertions, 3 deletions

diff --git a/changelogs/unreleased/po-fix-invalid-request-cache-panic.yml b/changelogs/unreleased/po-fix-invalid-request-cache-panic.yml
new file mode 100644
index 000000000..bb48399df
--- /dev/null
+++ b/changelogs/unreleased/po-fix-invalid-request-cache-panic.yml
@@ -0,0 +1,5 @@
+---
+title: Fix middleware to stop panicking from bad requests
+merge_request: 1747
+author:
+type: fixed
diff --git a/internal/middleware/cache/cache.go b/internal/middleware/cache/cache.go
index 0a3415ed5..79d8066e0 100644
--- a/internal/middleware/cache/cache.go
+++ b/internal/middleware/cache/cache.go
@@ -161,11 +161,12 @@ func invalidateCache(ci Invalidator, mInfo protoregistry.MethodInfo, handler grp
 		le.Lock()
 		defer le.Unlock()
 
-		le.LeaseEnder, err = ci.StartLease(target)
+		l, err := ci.StartLease(target)
 		if err != nil {
 			errLogger(err)
 			return
 		}
+		le.LeaseEnder = l
 	}
 
 	return wrappedHandler, peekerCallback
diff --git a/internal/service/repository/archive_test.go b/internal/service/repository/archive_test.go
index 16d9e278b..a5afbc1f1 100644
--- a/internal/service/repository/archive_test.go
+++ b/internal/service/repository/archive_test.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/streamio"
@@ -199,11 +200,21 @@ func TestGetArchiveFailure(t *testing.T) {
 			path:     []byte("../../foo"),
 			code:     codes.InvalidArgument,
 		},
+		{
+			desc:     "repo missing fields",
+			repo:     &gitalypb.Repository{StorageName: "default"},
+			prefix:   "qwert",
+			commitID: "sadf",
+			format:   gitalypb.GetArchiveRequest_TAR,
+			path:     []byte("Here is a string...."),
+			code:     codes.InvalidArgument,
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
 			ctx, cancel := testhelper.Context()
+			ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.CacheInvalidator)
 			defer cancel()
 
 			req := &gitalypb.GetArchiveRequest{
diff --git a/internal/service/repository/testhelper_test.go b/internal/service/repository/testhelper_test.go
index 8c9d73d7a..37c5e6d89 100644
--- a/internal/service/repository/testhelper_test.go
+++ b/internal/service/repository/testhelper_test.go
@@ -12,7 +12,10 @@ import (
 	"github.com/stretchr/testify/assert"
 	gitalyauth "gitlab.com/gitlab-org/gitaly/auth"
 	"gitlab.com/gitlab-org/gitaly/client"
+	dcache "gitlab.com/gitlab-org/gitaly/internal/cache"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
+	mcache "gitlab.com/gitlab-org/gitaly/internal/middleware/cache"
+	"gitlab.com/gitlab-org/gitaly/internal/praefect/protoregistry"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/server/auth"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -64,8 +67,14 @@ func newSecureRepoClient(t *testing.T, serverSocketPath string, pool *x509.CertP
 var NewSecureRepoClient = newSecureRepoClient
 
 func runRepoServer(t *testing.T) (*grpc.Server, string) {
-	streamInt := []grpc.StreamServerInterceptor{auth.StreamServerInterceptor(config.Config.Auth)}
-	unaryInt := []grpc.UnaryServerInterceptor{auth.UnaryServerInterceptor(config.Config.Auth)}
+	streamInt := []grpc.StreamServerInterceptor{
+		auth.StreamServerInterceptor(config.Config.Auth),
+		mcache.StreamInvalidator(dcache.LeaseKeyer{}, protoregistry.GitalyProtoPreregistered),
+	}
+	unaryInt := []grpc.UnaryServerInterceptor{
+		auth.UnaryServerInterceptor(config.Config.Auth),
+		mcache.UnaryInvalidator(dcache.LeaseKeyer{}, protoregistry.GitalyProtoPreregistered),
+	}
 
 	server := testhelper.NewTestGrpcServer(t, streamInt, unaryInt)
 	serverSocketPath := testhelper.GetTemporaryGitalySocketFileName()