Remove pack-objects limiting by user

The pack-objects limiting feature now allows for selecting multiple
bucket keys, with the most effective strategy being RemoteIP. Accounting
based on user identity is not effective due to the majority of requests
being anonymous. As a result, this commit removes pack-objects limiting
by user.

3 files changed, 4 insertions, 39 deletions

diff --git a/internal/gitaly/service/hook/pack_objects.go b/internal/gitaly/service/hook/pack_objects.go
index 8d1ba124e..ca5c1c024 100644
--- a/internal/gitaly/service/hook/pack_objects.go
+++ b/internal/gitaly/service/hook/pack_objects.go
@@ -81,18 +81,6 @@ func (s *server) packObjectsHook(ctx context.Context, req *gitalypb.PackObjectsH
 			)
 		}
 
-		if featureflag.PackObjectsLimitingUser.IsEnabled(ctx) && req.GetGlId() != "" {
-			return s.runPackObjectsLimited(
-				ctx,
-				w,
-				req.GetGlId(),
-				req,
-				args,
-				stdin,
-				cacheKey,
-			)
-		}
-
 		if featureflag.PackObjectsLimitingRemoteIP.IsEnabled(ctx) && req.GetRemoteIp() != "" {
 			ipAddr := net.ParseIP(req.GetRemoteIp())
 			if ipAddr == nil {
diff --git a/internal/gitaly/service/hook/pack_objects_test.go b/internal/gitaly/service/hook/pack_objects_test.go
index 4c0a5342d..bba4a0c8c 100644
--- a/internal/gitaly/service/hook/pack_objects_test.go
+++ b/internal/gitaly/service/hook/pack_objects_test.go
@@ -89,7 +89,6 @@ func TestServer_PackObjectsHook_separateContext(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.PackObjectsLimitingUser,
 		featureflag.PackObjectsLimitingRepo,
 		featureflag.PackObjectsLimitingRemoteIP,
 	).Run(t, runTestsWithRuntimeDir(
@@ -222,7 +221,6 @@ func TestServer_PackObjectsHook_usesCache(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.PackObjectsLimitingUser,
 		featureflag.PackObjectsLimitingRepo,
 		featureflag.PackObjectsLimitingRemoteIP,
 	).Run(t, runTestsWithRuntimeDir(
@@ -444,7 +442,6 @@ func TestServer_PackObjectsHookWithSidechannel(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.PackObjectsLimitingUser,
 		featureflag.PackObjectsLimitingRepo,
 		featureflag.PackObjectsLimitingRemoteIP,
 	).Run(t, runTestsWithRuntimeDir(
@@ -701,7 +698,6 @@ func TestServer_PackObjectsHookWithSidechannel_Canceled(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.PackObjectsLimitingUser,
 		featureflag.PackObjectsLimitingRepo,
 		featureflag.PackObjectsLimitingRemoteIP,
 	).Run(t, runTestsWithRuntimeDir(
@@ -781,7 +777,6 @@ func TestPackObjects_concurrencyLimit(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.PackObjectsLimitingUser,
 		featureflag.PackObjectsLimitingRepo,
 		featureflag.PackObjectsLimitingRemoteIP,
 	).Run(t, testPackObjectsConcurrency)
@@ -796,8 +791,6 @@ func testPackObjectsConcurrency(t *testing.T, ctx context.Context) {
 
 	if featureflag.PackObjectsLimitingRepo.IsEnabled(ctx) {
 		keyType = "repo"
-	} else if featureflag.PackObjectsLimitingUser.IsEnabled(ctx) {
-		keyType = "user"
 	} else if featureflag.PackObjectsLimitingRemoteIP.IsEnabled(ctx) {
 		keyType = "remote_ip"
 	}
@@ -856,7 +849,6 @@ func testPackObjectsConcurrency(t *testing.T, ctx context.Context) {
 				},
 			},
 			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx) ||
-				featureflag.PackObjectsLimitingUser.IsEnabled(ctx) ||
 				featureflag.PackObjectsLimitingRemoteIP.IsEnabled(ctx),
 		},
 		{
@@ -882,7 +874,6 @@ func testPackObjectsConcurrency(t *testing.T, ctx context.Context) {
 				},
 			},
 			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx) ||
-				featureflag.PackObjectsLimitingUser.IsEnabled(ctx) ||
 				featureflag.PackObjectsLimitingRemoteIP.IsEnabled(ctx),
 		},
 		{
@@ -907,8 +898,7 @@ func testPackObjectsConcurrency(t *testing.T, ctx context.Context) {
 					Args: args,
 				},
 			},
-			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx) ||
-				featureflag.PackObjectsLimitingUser.IsEnabled(ctx),
+			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx),
 		},
 		{
 			desc: "IPv6 loopback addresses",
@@ -932,8 +922,7 @@ func testPackObjectsConcurrency(t *testing.T, ctx context.Context) {
 					Args: args,
 				},
 			},
-			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx) ||
-				featureflag.PackObjectsLimitingUser.IsEnabled(ctx),
+			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx),
 		},
 		{
 			desc: "invalid IP addresses",
@@ -957,8 +946,7 @@ func testPackObjectsConcurrency(t *testing.T, ctx context.Context) {
 					Args: args,
 				},
 			},
-			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx) ||
-				featureflag.PackObjectsLimitingUser.IsEnabled(ctx),
+			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx),
 		},
 		{
 			desc: "empty IP addresses",
@@ -982,8 +970,7 @@ func testPackObjectsConcurrency(t *testing.T, ctx context.Context) {
 					Args: args,
 				},
 			},
-			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx) ||
-				featureflag.PackObjectsLimitingUser.IsEnabled(ctx),
+			shouldLimit: featureflag.PackObjectsLimitingRepo.IsEnabled(ctx),
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
diff --git a/internal/metadata/featureflag/ff_pack_objects_limiting_user.go b/internal/metadata/featureflag/ff_pack_objects_limiting_user.go
deleted file mode 100644
index 1e8319ad1..000000000
--- a/internal/metadata/featureflag/ff_pack_objects_limiting_user.go
+++ /dev/null
@@ -1,10 +0,0 @@
-package featureflag
-
-// PackObjectsLimitingUser will enable a concurrency limiter for pack objects
-// based off of the user
-var PackObjectsLimitingUser = NewFeatureFlag(
-	"pack_objects_limiting_user",
-	"v15.11.0",
-	"https://gitlab.com/gitlab-org/gitaly/-/issues/4413",
-	false,
-)