smarthttp: Fix upload-pack using bundle-URI with missing backup

When `bundleuri.UploadPackGitConfig()` returns an empty slice, we see
the `PostUploadPackWithSidechannel` RPC fail with `FailedPrecondition`.
The git-upload-pack(1) process started here quits with the error:

    fatal: invalid command 'bundle-uri'

Looking at the source code of Git, we see when git-upload-pack(1)
receives the 'bundle-uri' command, it checks if the that capability
should be advertised. To query for this, Git checks if the config
`uploadpack.advertiseBundleURIs` is set to `true`. When Gitaly could not
build a bundle URI to pass to the git-upload-pack(1), this config was
not set, and Git did not accept this command.

For the SmartHTTP protocol Gitaly uses separate processes to handle
'GET info/refs' and 'POST upload-pack', using option `--stateless-rpc`.
Due to this, the server advertised to the client it supports
'bundle-uri' in the 'GET info/refs' response, but in the consecutive
requests we might spawn git-upload-pack(1) without this config, and Git
will not accept the 'bundle-uri' command.

Inject the config `uploadpack.advertiseBundleURIs=true` into all calls
to git-upload-pack(1), even when the bundle-URI cannot be built.

Label: bug::availability

3 files changed, 53 insertions, 11 deletions

diff --git a/internal/bundleuri/git_config.go b/internal/bundleuri/git_config.go
index 747a99bf2..5eb809480 100644
--- a/internal/bundleuri/git_config.go
+++ b/internal/bundleuri/git_config.go
@@ -11,10 +11,12 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/log"
 )
 
-// InfoRefsGitConfig return a slice of git.ConfigPairs you can inject into the
-// call to git-upload-pack(1) --advertise-refs, to advertise the use of
-// bundle-URI to the client who clones/fetches from the repository.
-func InfoRefsGitConfig(ctx context.Context) []git.ConfigPair {
+// CapabilitiesGitConfig returns a slice of git.ConfigPairs that can be injected
+// into the Git config to make it aware the bundle-URI capabilities are
+// supported.
+// This can be used when spawning git-upload-pack(1) --advertise-refs in
+// response to the GET /info/refs request.
+func CapabilitiesGitConfig(ctx context.Context) []git.ConfigPair {
 	if featureflag.BundleURI.IsDisabled(ctx) {
 		return []git.ConfigPair{}
 	}
@@ -30,8 +32,6 @@ func InfoRefsGitConfig(ctx context.Context) []git.ConfigPair {
 // UploadPackGitConfig return a slice of git.ConfigPairs you can inject into the
 // call to git-upload-pack(1) to advertise the available bundle to the client
 // who clones/fetches from the repository.
-// In case no backups could be found or something else goes wrong, an empty
-// slice is returned without error.
 func UploadPackGitConfig(
 	ctx context.Context,
 	backupLocator backup.Locator,
@@ -39,12 +39,12 @@ func UploadPackGitConfig(
 	repo storage.Repository,
 ) []git.ConfigPair {
 	if backupLocator == nil || backupSink == nil || featureflag.BundleURI.IsDisabled(ctx) {
-		return []git.ConfigPair{}
+		return CapabilitiesGitConfig(ctx)
 	}
 
 	theBackup, err := backupLocator.FindLatest(ctx, repo)
 	if err != nil {
-		return []git.ConfigPair{}
+		return CapabilitiesGitConfig(ctx)
 	}
 
 	for _, step := range theBackup.Steps {
@@ -55,7 +55,7 @@ func UploadPackGitConfig(
 
 		uri, err := backupSink.SignedURL(ctx, step.BundlePath, 10*time.Minute)
 		if err != nil {
-			return []git.ConfigPair{}
+			return CapabilitiesGitConfig(ctx)
 		}
 
 		log.AddFields(ctx, log.Fields{"bundle_uri": true})
@@ -80,5 +80,5 @@ func UploadPackGitConfig(
 		}
 	}
 
-	return []git.ConfigPair{}
+	return CapabilitiesGitConfig(ctx)
 }
diff --git a/internal/bundleuri/git_config_test.go b/internal/bundleuri/git_config_test.go
index c14c3625b..57427fe41 100644
--- a/internal/bundleuri/git_config_test.go
+++ b/internal/bundleuri/git_config_test.go
@@ -40,18 +40,36 @@ func testUploadPackGitConfig(t *testing.T, ctx context.Context) {
 	}{
 		{
 			desc: "no backup locator nor sink",
+			expectedConfig: []git.ConfigPair{
+				{
+					Key:   "uploadpack.advertiseBundleURIs",
+					Value: "true",
+				},
+			},
 		},
 		{
 			desc: "no backup locator",
 			signedURLFunc: func(context.Context, string, time.Duration) (string, error) {
 				return "", structerr.NewNotFound("not signed")
 			},
+			expectedConfig: []git.ConfigPair{
+				{
+					Key:   "uploadpack.advertiseBundleURIs",
+					Value: "true",
+				},
+			},
 		},
 		{
 			desc: "no backup sink",
 			findLatestFunc: func(context.Context, storage.Repository) (*backup.Backup, error) {
 				return nil, structerr.NewNotFound("no backup found")
 			},
+			expectedConfig: []git.ConfigPair{
+				{
+					Key:   "uploadpack.advertiseBundleURIs",
+					Value: "true",
+				},
+			},
 		},
 		{
 			desc: "no backup found",
@@ -61,6 +79,12 @@ func testUploadPackGitConfig(t *testing.T, ctx context.Context) {
 			signedURLFunc: func(context.Context, string, time.Duration) (string, error) {
 				return "", structerr.NewNotFound("not signed")
 			},
+			expectedConfig: []git.ConfigPair{
+				{
+					Key:   "uploadpack.advertiseBundleURIs",
+					Value: "true",
+				},
+			},
 		},
 		{
 			desc: "backup has no steps",
@@ -70,6 +94,12 @@ func testUploadPackGitConfig(t *testing.T, ctx context.Context) {
 			signedURLFunc: func(context.Context, string, time.Duration) (string, error) {
 				return "", structerr.NewNotFound("not signed")
 			},
+			expectedConfig: []git.ConfigPair{
+				{
+					Key:   "uploadpack.advertiseBundleURIs",
+					Value: "true",
+				},
+			},
 		},
 		{
 			desc: "backup step is incremental",
@@ -81,6 +111,12 @@ func testUploadPackGitConfig(t *testing.T, ctx context.Context) {
 			signedURLFunc: func(context.Context, string, time.Duration) (string, error) {
 				return "", structerr.NewNotFound("not signed")
 			},
+			expectedConfig: []git.ConfigPair{
+				{
+					Key:   "uploadpack.advertiseBundleURIs",
+					Value: "true",
+				},
+			},
 		},
 		{
 			desc: "sign failed",
@@ -92,6 +128,12 @@ func testUploadPackGitConfig(t *testing.T, ctx context.Context) {
 			signedURLFunc: func(context.Context, string, time.Duration) (string, error) {
 				return "", structerr.NewNotFound("not signed")
 			},
+			expectedConfig: []git.ConfigPair{
+				{
+					Key:   "uploadpack.advertiseBundleURIs",
+					Value: "true",
+				},
+			},
 		},
 		{
 			desc: "success",
diff --git a/internal/gitaly/service/smarthttp/inforefs.go b/internal/gitaly/service/smarthttp/inforefs.go
index 63be73f0f..2818741de 100644
--- a/internal/gitaly/service/smarthttp/inforefs.go
+++ b/internal/gitaly/service/smarthttp/inforefs.go
@@ -67,7 +67,7 @@ func (s *server) handleInfoRefs(ctx context.Context, service, repoPath string, r
 	if err != nil {
 		return err
 	}
-	gitConfig = append(gitConfig, bundleuri.InfoRefsGitConfig(ctx)...)
+	gitConfig = append(gitConfig, bundleuri.CapabilitiesGitConfig(ctx)...)
 
 	cmdOpts = append(cmdOpts, git.WithConfig(gitConfig...))