Bundle-uri capability: Only advertise when bundle exist

This MR is a building block towards enabling bundle-uri feature.

Here we modify the way bundle-uri capability is advertised to the client to only make it so when a bundle exist for the given repository.

For SSH, this change does not provide much benefit since the whole packfile negociation flow is performed on a single connection. In other words, the flow starts when the client starts the upload-pack service, and it ends when the client received all the objects it needed.

For SmartHTTP, however, this is different. Each request/command sent by the client (ls-refs, bundle-uri, fetch) is sent on different, stateless, RPC calls. There is no way to know what command the client is sending to the git-upload-pack process until that process is actually started, which means that the Git config injected into the process must be computed in advance for every request; this implies computing the SignedURL of the bundle if it exist. By advertising bundle-uri capability only when a bundle exists for the given repository, we make sure the client won't send a bundle-uri command if no bundle exist.

This not only reduces by 1 the number of round-trip request and the number of Git config computation, but it also make it easier to monitor the use of bundle-uri feature, since now we can be sure that when a client sends the bundle-uri command, it is because:

1. A bundle exists
2. The server advertised the capability
3. The client support the capability

Thus, we now have a way to know if a client is using bundle-uri or not.

References: #6572 (closed)

internal/bundleuri/git_config.go

@@ -2,61 +2,45 @@ package bundleuri
 
 import (
 	"context"
-	"errors"
 	"fmt"
+	"strconv"
 
 	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gitcmd"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/log"
 )
 
 // CapabilitiesGitConfig returns a slice of gitcmd.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
+// into the Git config to enable or disable bundle-uri capability.
+//
+// If the feature flag is OFF, this config disables bundle-uri regardless of the `enable` parameter.
+//
+// Explicitly disabling bundle-uri capability is used during calls to git-upload-pack(1)
+// when no bundle exists for the given repository. This prevents the client
+// from requesting a non-existing URI.
+//
+// This function is also used when spawning git-upload-pack(1) --advertise-refs in
 // response to the GET /info/refs request.
-func CapabilitiesGitConfig(ctx context.Context) []gitcmd.ConfigPair {
-	if featureflag.BundleURI.IsDisabled(ctx) {
-		return []gitcmd.ConfigPair{}
+func CapabilitiesGitConfig(ctx context.Context, enable bool) []gitcmd.ConfigPair {
+	cfg := gitcmd.ConfigPair{
+		Key:   "uploadpack.advertiseBundleURIs",
+		Value: strconv.FormatBool(enable),
 	}
 
-	return []gitcmd.ConfigPair{
-		{
-			Key:   "uploadpack.advertiseBundleURIs",
-			Value: "true",
-		},
+	if featureflag.BundleURI.IsDisabled(ctx) {
+		cfg.Value = strconv.FormatBool(false)
 	}
+	return []gitcmd.ConfigPair{cfg}
 }
 
 // UploadPackGitConfig return a slice of gitcmd.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.
-func UploadPackGitConfig(
-	ctx context.Context,
-	manager *GenerationManager,
-	repo storage.Repository,
-) ([]gitcmd.ConfigPair, error) {
-	if featureflag.BundleURI.IsDisabled(ctx) {
-		return []gitcmd.ConfigPair{}, nil
-	}
-
-	if manager == nil || manager.sink == nil {
-		return CapabilitiesGitConfig(ctx), errors.New("bundle-URI sink missing")
+func UploadPackGitConfig(ctx context.Context, signedURL string) []gitcmd.ConfigPair {
+	if featureflag.BundleURI.IsDisabled(ctx) || signedURL == "" {
+		return CapabilitiesGitConfig(ctx, false)
 	}
 
-	uri, err := manager.SignedURL(ctx, repo)
-	if err != nil {
-		return nil, err
-	}
-
-	log.AddFields(ctx, log.Fields{"bundle_uri": true})
-
-	return []gitcmd.ConfigPair{
-		{
-			Key:   "uploadpack.advertiseBundleURIs",
-			Value: "true",
-		},
+	config := []gitcmd.ConfigPair{
 		{
 			Key:   "bundle.version",
 			Value: "1",
@@ -71,7 +55,7 @@ func UploadPackGitConfig(
 		},
 		{
 			Key:   fmt.Sprintf("bundle.%s.uri", defaultBundle),
-			Value: uri,
+			Value: signedURL,
 		},
 		{
 			// Gitaly uses only one bundle URI bundle, and it's a
@@ -91,5 +75,6 @@ func UploadPackGitConfig(
 			Key:   fmt.Sprintf("bundle.%s.creationToken", defaultBundle),
 			Value: "1",
 		},
-	}, nil
+	}
+	return append(config, CapabilitiesGitConfig(ctx, true)...)
 }