InfoRefs: Only advertize bundle-uri when a bundle exists

In a previous MR, we updated the UploadPack RPC to only
advertize the `bundle-uri` capability when a bundle exists.

!7572

However, those changes overlooked the `InfoRefs` RPC. This
RPC is also called during a `git clone` operation. We end
up in a situation where, when a bundle DOES NOT exist, the
`InfoRefs` RPC was advertizing the `bundle-uri` capability,
but when the client would call `UploadPack` afterwards to
fetch a bundle, it would fail because the `UploadPack` would
not support the `bundle-uri` capability.

This commit updates the `InfoRefs` RPC to only advertise
the `bundle-uri` capability when a bundle exist. As such,
the behavior of the `InfoRefs` RPC is now on par with the
`UploadPack` RPC, and conflicts such as the one described
above occurs.

internal/gitaly/service/smarthttp/inforefs.go

@@ -72,7 +72,11 @@ func (s *server) handleInfoRefs(ctx context.Context, service, repoPath string, r
 	if err != nil {
 		return err
 	}
-	gitConfig = append(gitConfig, bundleuri.CapabilitiesGitConfig(ctx, true)...)
+	if s.bundleURIManager != nil {
+		gitConfig = append(gitConfig, s.bundleURIManager.UploadPackGitConfig(ctx, req.GetRepository())...)
+	} else {
+		gitConfig = append(gitConfig, bundleuri.CapabilitiesGitConfig(ctx, false)...)
+	}
 
 	cmdOpts = append(cmdOpts, gitcmd.WithConfig(gitConfig...))
 

internal/gitaly/service/smarthttp/inforefs_test.go

@@ -8,15 +8,18 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/bundleuri"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/cache"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gitcmd"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/stats"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
@@ -244,26 +247,119 @@ func testInfoRefsUploadPackGitConfigOptions(t *testing.T, ctx context.Context) {
 func TestInfoRefsUploadPack_bundleURI(t *testing.T) {
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
-	ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.BundleURI, true)
+	var ctx context.Context
 
-	cfg := testcfg.Build(t)
-	cfg.SocketPath = runSmartHTTPServer(t, cfg)
+	logger := testhelper.NewLogger(t)
 
-	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+	// Create secret key for bundle-uri sink
+	tempDir := testhelper.TempDir(t)
+	keyFile, err := os.Create(filepath.Join(tempDir, "secret.key"))
+	require.NoError(t, err)
+	_, err = keyFile.WriteString("super-secret-key")
+	require.NoError(t, err)
+	require.NoError(t, keyFile.Close())
 
-	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
+	// Create the bundle sink
+	baseURL := "https://example.com"
+	sinkURI := "file://" + testhelper.TempDir(t) + "?base_url=" + baseURL + "&no_tmp_dir=true&secret_key_path=" + keyFile.Name()
+	sink, err := bundleuri.NewSink(ctx, sinkURI)
+	require.NoError(t, err)
 
-	rpcRequest := &gitalypb.InfoRefsRequest{
-		Repository:       repo,
-		GitProtocol:      gitcmd.ProtocolV2,
-		GitConfigOptions: []string{"transfer.bundleURI=true"},
+	type testData struct {
+		bundleManager *bundleuri.GenerationManager
+	}
+
+	testCases := []struct {
+		name            string
+		setup           func(cfg config.Cfg) testData
+		generateBundle  bool
+		shouldAdvertise bool
+	}{
+		{
+			name: "when a bundle exists, bundle-uri capability is advertised",
+			setup: func(cfg config.Cfg) testData {
+				ctx = testhelper.Context(t)
+				ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.BundleURI, true)
+
+				// Create the bundle manager
+				bundleManager, err := bundleuri.NewGenerationManager(ctx, sink, logger, nil, bundleuri.NewSimpleStrategy(true))
+				require.NoError(t, err)
+
+				return testData{
+					bundleManager: bundleManager,
+				}
+			},
+			generateBundle:  true,
+			shouldAdvertise: true,
+		},
+		{
+			name: "when a bundle doe snot exist, bundle-uri capability is not advertised",
+			setup: func(cfg config.Cfg) testData {
+				ctx = testhelper.Context(t)
+				ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.BundleURI, true)
+
+				// Create the bundle manager
+				bundleManager, err := bundleuri.NewGenerationManager(ctx, sink, logger, nil, bundleuri.NewSimpleStrategy(true))
+				require.NoError(t, err)
+
+				return testData{
+					bundleManager: bundleManager,
+				}
+			},
+			generateBundle:  false,
+			shouldAdvertise: false,
+		},
+		{
+			name: "when bundle manager is nil, bundle-uri capability is not advertised",
+			setup: func(cfg config.Cfg) testData {
+				ctx = testhelper.Context(t)
+				ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.BundleURI, true)
+
+				return testData{
+					bundleManager: nil,
+				}
+			},
+			generateBundle:  false,
+			shouldAdvertise: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := testcfg.Build(t)
+
+			td := tc.setup(cfg)
+			smartHTTPServer := startSmartHTTPServerWithOptions(t, cfg, nil, []testserver.GitalyServerOpt{
+				testserver.WithBundleURIManager(td.bundleManager),
+				testserver.WithLogger(logger),
+			})
+			cfg.SocketPath = smartHTTPServer.Address()
+
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
+			gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithParents())
+
+			if tc.generateBundle {
+				repo := localrepo.NewTestRepo(t, cfg, repoProto)
+				err = td.bundleManager.Generate(ctx, repo)
+				require.NoError(t, err)
+			}
+
+			rpcRequest := &gitalypb.InfoRefsRequest{
+				Repository:       repoProto,
+				GitProtocol:      gitcmd.ProtocolV2,
+				GitConfigOptions: []string{"transfer.bundleURI=true"},
+			}
+			response, err := makeInfoRefsUploadPackRequest(t, ctx, cfg.SocketPath, cfg.Auth.Token, rpcRequest)
+			require.NoError(t, err)
+
+			bundleURICapability := []string{"bundle-uri"}
+			if tc.shouldAdvertise {
+				requireAdvertisedCapabilitiesV2(t, string(response), "git-upload-pack", bundleURICapability)
+			} else {
+				requireNotAdvertisedCapabilitiesV2(t, string(response), "git-upload-pack", bundleURICapability)
+			}
+		})
 	}
-	response, err := makeInfoRefsUploadPackRequest(t, ctx, cfg.SocketPath, cfg.Auth.Token, rpcRequest)
-	require.NoError(t, err)
-	requireAdvertisedCapabilitiesV2(t, string(response), "git-upload-pack", []string{
-		"bundle-uri",
-	})
 }
 
 func TestInfoRefsUploadPack_gitProtocol(t *testing.T) {
@@ -518,6 +614,28 @@ func requireAdvertisedCapabilitiesV2(t *testing.T, responseBody, expectedService
 	}
 }
 
+func requireNotAdvertisedCapabilitiesV2(t *testing.T, responseBody, expectedService string, expectedCapabilities []string) {
+	t.Helper()
+
+	responseLines := strings.SplitAfter(responseBody, "\n")
+	require.Greater(t, len(responseLines), 2)
+
+	// The first line contains the service announcement
+	require.Equal(t, gittest.Pktlinef(t, "# service=%s\n", expectedService), responseLines[0])
+
+	// The second line contains the protocol version
+	require.Equal(t, "0000"+gittest.Pktlinef(t, "version %d\n", 2), responseLines[1])
+
+	// The third line and following lines contain capabilities
+	advertised := false
+	for _, expectedCap := range expectedCapabilities {
+		if slices.Contains(responseLines[2:], gittest.Pktlinef(t, "%s\n", expectedCap)) {
+			advertised = true
+		}
+	}
+	require.False(t, advertised)
+}
+
 func requireAdvertisedRefs(t *testing.T, responseBody, expectedService string, expectedRefs []string) {
 	t.Helper()
 

internal/testhelper/testserver/gitaly.go

@@ -385,7 +385,10 @@ func (gsd *gitalyServerDeps) createDependencies(tb testing.TB, ctx context.Conte
 		node = nodeMgr
 	}
 
-	if gsd.bundleURISink != nil {
+	// This is to allow building a bundle generation from a Sink
+	// without having to create one beforehand.
+	// If bundleURIManager is defined though, we use that one.
+	if gsd.bundleURIManager == nil && gsd.bundleURISink != nil {
 		var strategy bundleuri.GenerationStrategy
 		if gsd.bundleURIStrategy != nil {
 			strategy = gsd.bundleURIStrategy
@@ -631,6 +634,14 @@ func WithBundleURISink(sink *bundleuri.Sink) GitalyServerOpt {
 	}
 }
 
+// WithBundleURIManager sets the *bundleuri.GenerationManager that will be used by the bundleuri.GenerationManager
+func WithBundleURIManager(manager *bundleuri.GenerationManager) GitalyServerOpt {
+	return func(deps gitalyServerDeps) gitalyServerDeps {
+		deps.bundleURIManager = manager
+		return deps
+	}
+}
+
 // WithSigningKey sets the signing key path that will be used for Gitaly
 // services.
 func WithSigningKey(signingKey string) GitalyServerOpt {