cmd/gitaly-lfs-smudge: Move creation of the HTTP client

The GitLab HTTP client is currently created closely before performing
the request to retrieve the LFS pointers' smudged contents. This has
been fine until now given that we only ever requested a single LFS
pointer across the whole lifetime of the command. As we are about to
implement the long-running filter protocol though this will change so
that we may request an unbounded number of LFS pointers in a single
session.

Move out creation of the HTTP client so that it can be reused across
calls to `smudgeContents()`. This avoids some overhead like re-reading
contents of the GitLab secret from disk.

Note that this is essentially a change in behaviour: previously, if we
failed to create the HTTP client, then we'd have just silently returned
the blob's clean contents. This easily papers over misconfigurations
though and is thus not all that great. Now we do return errors in case
we failed to create the client, which means that any such misconfigured
instance may now error. This feels like a reasonable change though:
better to complain about errors than to silently swallow then.

1 files changed, 8 insertions, 8 deletions

diff --git a/cmd/gitaly-lfs-smudge/lfs_smudge.go b/cmd/gitaly-lfs-smudge/lfs_smudge.go
index 47689c5a3..87fe38cdc 100644
--- a/cmd/gitaly-lfs-smudge/lfs_smudge.go
+++ b/cmd/gitaly-lfs-smudge/lfs_smudge.go
@@ -14,7 +14,12 @@ import (
 )
 
 func smudgeContents(ctx context.Context, cfg smudge.Config, to io.Writer, from io.Reader) (returnedErr error) {
-	output, err := handleSmudge(ctx, cfg, from)
+	client, err := gitlab.NewHTTPClient(log.ContextLogger(ctx), cfg.Gitlab, cfg.TLS, prometheus.Config{})
+	if err != nil {
+		return fmt.Errorf("creating HTTP client: %w", err)
+	}
+
+	output, err := handleSmudge(ctx, cfg, client, from)
 	if err != nil {
 		return fmt.Errorf("smudging contents: %w", err)
 	}
@@ -31,7 +36,7 @@ func smudgeContents(ctx context.Context, cfg smudge.Config, to io.Writer, from i
 	return nil
 }
 
-func handleSmudge(ctx context.Context, cfg smudge.Config, from io.Reader) (io.ReadCloser, error) {
+func handleSmudge(ctx context.Context, cfg smudge.Config, gitlabClient *gitlab.HTTPClient, from io.Reader) (io.ReadCloser, error) {
 	logger := log.ContextLogger(ctx)
 
 	ptr, contents, err := lfs.DecodeFrom(from)
@@ -42,17 +47,12 @@ func handleSmudge(ctx context.Context, cfg smudge.Config, from io.Reader) (io.Re
 
 	logger.WithField("oid", ptr.Oid).Debug("decoded LFS OID")
 
-	client, err := gitlab.NewHTTPClient(logger, cfg.Gitlab, cfg.TLS, prometheus.Config{})
-	if err != nil {
-		return io.NopCloser(contents), err
-	}
-
 	qs := url.Values{}
 	qs.Set("oid", ptr.Oid)
 	qs.Set("gl_repository", cfg.GlRepository)
 	u := url.URL{Path: "/lfs", RawQuery: qs.Encode()}
 
-	response, err := client.Get(ctx, u.String())
+	response, err := gitlabClient.Get(ctx, u.String())
 	if err != nil {
 		return io.NopCloser(contents), fmt.Errorf("error loading LFS object: %v", err)
 	}