gitaly-lfs-smudge: Allow passing configuration as a single envvar

The gitaly-lfs-smudge binary accepts its configuration as a set of
environment variables, which is required so that we can pass it through
Git. Using separate environment variables doesn't scale though: there is
no easily accessible documentation about what is accepted and what is
not, and furthermore every new configuration needs another environment
variable.

Refactor our code to accept a single `GITALY_LFS_SMUDGE_CONFIG` variable
that contains everything needed by gitaly-lfs-smudge to operate. The old
environment variables still exist for backwards-compatibility reasons,
but are overridden in case the new environment variable exists.

4 files changed, 121 insertions, 10 deletions

diff --git a/cmd/gitaly-lfs-smudge/config.go b/cmd/gitaly-lfs-smudge/config.go
index b22742837..521373617 100644
--- a/cmd/gitaly-lfs-smudge/config.go
+++ b/cmd/gitaly-lfs-smudge/config.go
@@ -9,22 +9,37 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/env"
 )
 
+// ConfigEnvironmentKey is the key that gitaly-lfs-smudge expects the configuration to exist at. The
+// value of this environment variable should be the JSON-encoded `Config` struct.
+const ConfigEnvironmentKey = "GITALY_LFS_SMUDGE_CONFIG"
+
 // Config is the configuration used to run gitaly-lfs-smudge. It must be injected via environment
 // variables.
 type Config struct {
 	// GlRepository is the GitLab repository identifier that is required so that we can query
 	// the corresponding Rails' repository for the respective LFS contents.
-	GlRepository string
+	GlRepository string `json:"gl_repository"`
 	// Gitlab contains configuration so that we can connect to Rails in order to retrieve LFS
 	// contents.
-	Gitlab config.Gitlab
+	Gitlab config.Gitlab `json:"gitlab"`
 	// TLS contains configuration for setting up a TLS-encrypted connection to Rails.
-	TLS config.TLS
+	TLS config.TLS `json:"tls"`
 }
 
 func configFromEnvironment(environment []string) (Config, error) {
 	var cfg Config
 
+	// If ConfigEnvironmentKey is set, then we use that instead of the separate environment
+	// variables queried for below. This has been newly introduced in v15.1, so the fallback
+	// to the old environment variables can be removed with v15.2.
+	if encodedCfg := env.ExtractValue(environment, ConfigEnvironmentKey); encodedCfg != "" {
+		if err := json.Unmarshal([]byte(encodedCfg), &cfg); err != nil {
+			return Config{}, fmt.Errorf("unable to unmarshal config: %w", err)
+		}
+
+		return cfg, nil
+	}
+
 	cfg.GlRepository = env.ExtractValue(environment, "GL_REPOSITORY")
 	if cfg.GlRepository == "" {
 		return Config{}, fmt.Errorf("error loading project: GL_REPOSITORY is not defined")
@@ -50,3 +65,14 @@ func configFromEnvironment(environment []string) (Config, error) {
 
 	return cfg, nil
 }
+
+// Environment encodes the given configuration as an environment variable that can be injected into
+// `gitaly-lfs-smudge`.
+func (c Config) Environment() (string, error) {
+	marshalled, err := json.Marshal(c)
+	if err != nil {
+		return "", fmt.Errorf("marshalling configuration: %w", err)
+	}
+
+	return fmt.Sprintf("%s=%s", ConfigEnvironmentKey, marshalled), nil
+}
diff --git a/cmd/gitaly-lfs-smudge/config_test.go b/cmd/gitaly-lfs-smudge/config_test.go
index d5371c37a..a7a855953 100644
--- a/cmd/gitaly-lfs-smudge/config_test.go
+++ b/cmd/gitaly-lfs-smudge/config_test.go
@@ -28,12 +28,21 @@ func TestConfigFromEnvironment(t *testing.T) {
 		KeyPath:  "/key/path",
 	}
 
+	fullCfg := Config{
+		GlRepository: "repo",
+		Gitlab:       gitlabCfg,
+		TLS:          tlsCfg,
+	}
+
 	marshalledGitlabCfg, err := json.Marshal(gitlabCfg)
 	require.NoError(t, err)
 
 	marshalledTLSCfg, err := json.Marshal(tlsCfg)
 	require.NoError(t, err)
 
+	marshalledFullCfg, err := json.Marshal(fullCfg)
+	require.NoError(t, err)
+
 	for _, tc := range []struct {
 		desc        string
 		environment []string
@@ -45,7 +54,7 @@ func TestConfigFromEnvironment(t *testing.T) {
 			expectedErr: "error loading project: GL_REPOSITORY is not defined",
 		},
 		{
-			desc: "successful",
+			desc: "successful via separate envvars",
 			environment: []string{
 				"GL_REPOSITORY=repo",
 				"GL_INTERNAL_CONFIG=" + string(marshalledGitlabCfg),
@@ -58,6 +67,30 @@ func TestConfigFromEnvironment(t *testing.T) {
 			},
 		},
 		{
+			desc: "successful via single envvar",
+			environment: []string{
+				"GITALY_LFS_SMUDGE_CONFIG=" + string(marshalledFullCfg),
+				"GL_REPOSITORY=garbage",
+				"GL_INTERNAL_CONFIG=garbage",
+				"GITALY_TLS=garbage",
+			},
+			expectedCfg: fullCfg,
+		},
+		{
+			desc: "single envvar overrides separate envvars",
+			environment: []string{
+				"GITALY_LFS_SMUDGE_CONFIG=" + string(marshalledFullCfg),
+			},
+			expectedCfg: fullCfg,
+		},
+		{
+			desc: "invalid full config",
+			environment: []string{
+				"GITALY_LFS_SMUDGE_CONFIG=invalid",
+			},
+			expectedErr: "unable to unmarshal config: invalid character 'i' looking for beginning of value",
+		},
+		{
 			desc: "missing GlRepository",
 			environment: []string{
 				"GL_INTERNAL_CONFIG=" + string(marshalledGitlabCfg),
@@ -111,3 +144,30 @@ func TestConfigFromEnvironment(t *testing.T) {
 		})
 	}
 }
+
+func TestConfig_Environment(t *testing.T) {
+	cfg := Config{
+		GlRepository: "repo",
+		Gitlab: config.Gitlab{
+			URL:             "https://example.com",
+			RelativeURLRoot: "gitlab",
+			HTTPSettings: config.HTTPSettings{
+				ReadTimeout: 1,
+				User:        "user",
+				Password:    "correcthorsebatterystaple",
+				CAFile:      "/ca/file",
+				CAPath:      "/ca/path",
+				SelfSigned:  true,
+			},
+			SecretFile: "/secret/path",
+		},
+		TLS: config.TLS{
+			CertPath: "/cert/path",
+			KeyPath:  "/key/path",
+		},
+	}
+
+	env, err := cfg.Environment()
+	require.NoError(t, err)
+	require.Equal(t, `GITALY_LFS_SMUDGE_CONFIG={"gl_repository":"repo","gitlab":{"url":"https://example.com","relative_url_root":"gitlab","http_settings":{"read_timeout":1,"user":"user","password":"correcthorsebatterystaple","ca_file":"/ca/file","ca_path":"/ca/path","self_signed_cert":true},"secret_file":"/secret/path"},"tls":{"cert_path":"/cert/path","key_path":"/key/path"}}`, env)
+}
diff --git a/cmd/gitaly-lfs-smudge/main_test.go b/cmd/gitaly-lfs-smudge/main_test.go
index 5590a5036..68011289b 100644
--- a/cmd/gitaly-lfs-smudge/main_test.go
+++ b/cmd/gitaly-lfs-smudge/main_test.go
@@ -26,13 +26,15 @@ func TestGitalyLFSSmudge(t *testing.T) {
 	gitlabCfg, cleanup := runTestServer(t, defaultOptions)
 	defer cleanup()
 
+	tlsCfg := config.TLS{
+		CertPath: certPath,
+		KeyPath:  keyPath,
+	}
+
 	marshalledGitlabCfg, err := json.Marshal(gitlabCfg)
 	require.NoError(t, err)
 
-	marshalledTLSCfg, err := json.Marshal(config.TLS{
-		CertPath: certPath,
-		KeyPath:  keyPath,
-	})
+	marshalledTLSCfg, err := json.Marshal(tlsCfg)
 	require.NoError(t, err)
 
 	standardEnv := func(logDir string) []string {
@@ -64,6 +66,29 @@ func TestGitalyLFSSmudge(t *testing.T) {
 			expectedLogRegexp: "Finished HTTP request",
 		},
 		{
+			desc: "success with single envvar",
+			setup: func(t *testing.T) ([]string, string) {
+				logDir := testhelper.TempDir(t)
+
+				cfg := Config{
+					GlRepository: "project-1",
+					Gitlab:       gitlabCfg,
+					TLS:          tlsCfg,
+				}
+
+				env, err := cfg.Environment()
+				require.NoError(t, err)
+
+				return []string{
+					env,
+					"GITALY_LOG_DIR=" + logDir,
+				}, filepath.Join(logDir, "gitaly_lfs_smudge.log")
+			},
+			stdin:             strings.NewReader(lfsPointer),
+			expectedStdout:    "hello world",
+			expectedLogRegexp: "Finished HTTP request",
+		},
+		{
 			desc: "missing Gitlab repository",
 			setup: func(t *testing.T) ([]string, string) {
 				logDir := testhelper.TempDir(t)
diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index a7b70842c..29718876f 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -69,8 +69,8 @@ type Cfg struct {
 
 // TLS configuration
 type TLS struct {
-	CertPath string `toml:"certificate_path,omitempty"`
-	KeyPath  string `toml:"key_path,omitempty"`
+	CertPath string `toml:"certificate_path,omitempty" json:"cert_path"`
+	KeyPath  string `toml:"key_path,omitempty" json:"key_path"`
 }
 
 // GitlabShell contains the settings required for executing `gitlab-shell`