diff options
-rw-r--r-- | cmd/gitaly-hooks/hooks_test.go | 8 | ||||
-rw-r--r-- | internal/git/hooks_options.go | 24 | ||||
-rw-r--r-- | internal/git/hooks_options_test.go | 1 | ||||
-rw-r--r-- | internal/gitaly/rubyserver/rubyserver.go | 11 | ||||
-rw-r--r-- | internal/gitaly/service/hook/post_receive.go | 7 | ||||
-rw-r--r-- | internal/gitaly/service/hook/pre_receive.go | 19 | ||||
-rw-r--r-- | internal/gitaly/service/operations/update_with_hooks.go | 10 | ||||
-rw-r--r-- | internal/gitaly/service/operations/update_with_hooks_test.go | 1 | ||||
-rw-r--r-- | internal/gitlabshell/env.go | 43 | ||||
-rw-r--r-- | internal/gitlabshell/env_test.go | 77 |
10 files changed, 20 insertions, 181 deletions
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index 47c73425a..c0efd9cfa 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -21,7 +21,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook" - "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -40,15 +39,12 @@ type proxyValues struct { // envForHooks generates a set of environment variables for gitaly hooks func envForHooks(t testing.TB, gitlabShellDir string, repo *gitalypb.Repository, glHookValues glHookValues, proxyValues proxyValues, gitPushOptions ...string) []string { - env, err := gitlabshell.EnvFromConfig(config.Config) - require.NoError(t, err) - payload, err := git.NewHooksPayload(config.Config, repo, nil, nil).Env() require.NoError(t, err) - env = append(env, os.Environ()...) - env = append(env, []string{ + env := append(os.Environ(), []string{ payload, + "GITALY_BIN_DIR=" + config.Config.BinDir, fmt.Sprintf("GL_ID=%s", glHookValues.GLID), fmt.Sprintf("GL_REPOSITORY=%s", glHookValues.GLRepo), fmt.Sprintf("GL_PROTOCOL=%s", glHookValues.GLProtocol), diff --git a/internal/git/hooks_options.go b/internal/git/hooks_options.go index a091351d1..10d5df9a7 100644 --- a/internal/git/hooks_options.go +++ b/internal/git/hooks_options.go @@ -7,7 +7,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/hooks" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" + "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -47,7 +47,12 @@ func (cc *cmdCfg) configureHooks(ctx context.Context, repo *gitalypb.Repository, return err } - cc.env = append(cc.env, payload, "GITALY_BIN_DIR="+cfg.BinDir) + cc.env = append( + cc.env, + payload, + "GITALY_BIN_DIR="+cfg.BinDir, + fmt.Sprintf("%s=%s", log.GitalyLogDirEnvKey, cfg.Logging.Dir), + ) cc.globals = append(cc.globals, ValueFlag{"-c", fmt.Sprintf("core.hooksPath=%s", hooks.Path(cfg))}) cc.hooksConfigured = true @@ -73,7 +78,7 @@ func WithReceivePackHooks(ctx context.Context, cfg config.Cfg, req ReceivePackRe return err } - env, err := receivePackHookEnv(ctx, cc, cfg, req, protocol) + env, err := receivePackHookEnv(req, protocol) if err != nil { return fmt.Errorf("receive-pack hook envvars: %w", err) } @@ -83,19 +88,12 @@ func WithReceivePackHooks(ctx context.Context, cfg config.Cfg, req ReceivePackRe } } -func receivePackHookEnv(ctx context.Context, cc *cmdCfg, cfg config.Cfg, req ReceivePackRequest, protocol string) ([]string, error) { - gitlabshellEnv, err := gitlabshell.EnvFromConfig(cfg) - if err != nil { - return nil, err - } - - env := append(gitlabshellEnv, +func receivePackHookEnv(req ReceivePackRequest, protocol string) ([]string, error) { + return []string{ fmt.Sprintf("GL_ID=%s", req.GetGlId()), fmt.Sprintf("GL_USERNAME=%s", req.GetGlUsername()), fmt.Sprintf("GL_REPOSITORY=%s", req.GetGlRepository()), fmt.Sprintf("GL_PROJECT_PATH=%s", req.GetRepository().GetGlProjectPath()), fmt.Sprintf("GL_PROTOCOL=%s", protocol), - ) - - return env, nil + }, nil } diff --git a/internal/git/hooks_options_test.go b/internal/git/hooks_options_test.go index 054fb6078..4117d4c1e 100644 --- a/internal/git/hooks_options_test.go +++ b/internal/git/hooks_options_test.go @@ -69,6 +69,7 @@ func TestWithRefHook(t *testing.T) { require.EqualValues(t, []string{ "GITALY_HOOKS_PAYLOAD", "GITALY_BIN_DIR", + "GITALY_LOG_DIR", }, actualEnvVars) }) } diff --git a/internal/gitaly/rubyserver/rubyserver.go b/internal/gitaly/rubyserver/rubyserver.go index f4ef92345..b64b381bc 100644 --- a/internal/gitaly/rubyserver/rubyserver.go +++ b/internal/gitaly/rubyserver/rubyserver.go @@ -16,7 +16,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/hooks" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver/balancer" - "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/supervisor" "gitlab.com/gitlab-org/gitaly/internal/version" @@ -78,11 +77,6 @@ func (s *Server) start() error { cfg := config.Config - gitlabshellEnv, err := gitlabshell.EnvFromConfig(cfg) - if err != nil { - return err - } - env := append( os.Environ(), "GITALY_RUBY_GIT_BIN_PATH="+cfg.Git.BinPath, @@ -93,9 +87,8 @@ func (s *Server) start() error { "GITALY_GIT_HOOKS_DIR="+hooks.Path(cfg), "GITALY_SOCKET="+cfg.GitalyInternalSocketPath(), "GITALY_TOKEN="+cfg.Auth.Token, - "GITALY_RUGGED_GIT_CONFIG_SEARCH_PATH="+cfg.Ruby.RuggedGitConfigSearchPath) - env = append(env, gitlabshellEnv...) - + "GITALY_RUGGED_GIT_CONFIG_SEARCH_PATH="+cfg.Ruby.RuggedGitConfigSearchPath, + ) env = append(env, command.GitEnv...) if dsn := cfg.Logging.RubySentryDSN; dsn != "" { diff --git a/internal/gitaly/service/hook/post_receive.go b/internal/gitaly/service/hook/post_receive.go index 546e987e5..957949280 100644 --- a/internal/gitaly/service/hook/post_receive.go +++ b/internal/gitaly/service/hook/post_receive.go @@ -45,16 +45,11 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ return stream.Send(&gitalypb.PostReceiveHookResponse{Stderr: p}) }) - env, err := s.hookRequestEnv(firstRequest) - if err != nil { - return helper.ErrInternal(err) - } - if err := s.manager.PostReceiveHook( stream.Context(), firstRequest.Repository, firstRequest.GetGitPushOptions(), - env, + firstRequest.GetEnvironmentVariables(), stdin, stdout, stderr, diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go index 8efe327a7..56d68e109 100644 --- a/internal/gitaly/service/hook/pre_receive.go +++ b/internal/gitaly/service/hook/pre_receive.go @@ -7,24 +7,11 @@ import ( "sync" "gitlab.com/gitlab-org/gitaly/internal/git/hooks" - "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" ) -type hookRequest interface { - GetEnvironmentVariables() []string -} - -func (s *server) hookRequestEnv(req hookRequest) ([]string, error) { - gitlabshellEnv, err := gitlabshell.EnvFromConfig(s.cfg) - if err != nil { - return nil, err - } - return append(gitlabshellEnv, req.GetEnvironmentVariables()...), nil -} - func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer) error { firstRequest, err := stream.Recv() if err != nil { @@ -49,11 +36,7 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer return stream.Send(&gitalypb.PreReceiveHookResponse{Stderr: p}) }) - env, err := s.hookRequestEnv(firstRequest) - if err != nil { - return fmt.Errorf("getting env vars from request: %v", err) - } - env = append(env, hooks.GitPushOptions(firstRequest.GetGitPushOptions())...) + env := append(firstRequest.GetEnvironmentVariables(), hooks.GitPushOptions(firstRequest.GetGitPushOptions())...) if err := s.manager.PreReceiveHook( stream.Context(), diff --git a/internal/gitaly/service/operations/update_with_hooks.go b/internal/gitaly/service/operations/update_with_hooks.go index 7a7537a6a..134466236 100644 --- a/internal/gitaly/service/operations/update_with_hooks.go +++ b/internal/gitaly/service/operations/update_with_hooks.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" - "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -32,11 +31,6 @@ func (e updateRefError) Error() string { } func (s *Server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Repository, user *gitalypb.User, reference, newrev, oldrev string) error { - gitlabshellEnv, err := gitlabshell.EnvFromConfig(s.cfg) - if err != nil { - return err - } - transaction, praefect, err := metadata.TransactionMetadataFromContext(ctx) if err != nil { return err @@ -57,14 +51,14 @@ func (s *Server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Re return helper.ErrInternalf("updateReferenceWithHooks: got invalid new value: %w", err) } - env := append([]string{ + env := []string{ payload, "GL_PROTOCOL=web", fmt.Sprintf("GL_ID=%s", user.GetGlId()), fmt.Sprintf("GL_USERNAME=%s", user.GetGlUsername()), fmt.Sprintf("GL_REPOSITORY=%s", repo.GetGlRepository()), fmt.Sprintf("GL_PROJECT_PATH=%s", repo.GetGlProjectPath()), - }, gitlabshellEnv...) + } changes := fmt.Sprintf("%s %s %s\n", oldrev, newrev, reference) var stdout, stderr bytes.Buffer diff --git a/internal/gitaly/service/operations/update_with_hooks_test.go b/internal/gitaly/service/operations/update_with_hooks_test.go index 3b34707d5..ecd480e90 100644 --- a/internal/gitaly/service/operations/update_with_hooks_test.go +++ b/internal/gitaly/service/operations/update_with_hooks_test.go @@ -136,7 +136,6 @@ func TestUpdateReferenceWithHooks(t *testing.T) { expectedEnv := []string{ payload, - "GITALY_BIN_DIR=" + config.Config.BinDir, "GL_ID=1234", "GL_PROJECT_PATH=gitlab-org/gitlab-test", "GL_PROTOCOL=web", diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go deleted file mode 100644 index f337af58b..000000000 --- a/internal/gitlabshell/env.go +++ /dev/null @@ -1,43 +0,0 @@ -package gitlabshell - -import ( - "encoding/json" - "fmt" - - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/internal/log" -) - -type Config struct { - CustomHooksDir string `json:"custom_hooks_dir"` - GitlabURL string `json:"gitlab_url"` - HTTPSettings config.HTTPSettings `json:"http_settings"` - LogFormat string `json:"log_format"` - LogLevel string `json:"log_level"` - LogPath string `json:"log_path"` - SecretFile string `json:"secret_file"` -} - -// EnvFromConfig returns a set of environment variables from a config struct relevant to gitlab shell -func EnvFromConfig(cfg config.Cfg) ([]string, error) { - gitlabShellConfig := Config{ - CustomHooksDir: cfg.Hooks.CustomHooksDir, - GitlabURL: cfg.Gitlab.URL, - HTTPSettings: cfg.Gitlab.HTTPSettings, - LogFormat: cfg.Logging.Format, - LogLevel: cfg.Logging.Level, - LogPath: cfg.Logging.Dir, - SecretFile: cfg.Gitlab.SecretFile, - } - - gitlabShellConfigString, err := json.Marshal(&gitlabShellConfig) - if err != nil { - return nil, err - } - - return []string{ - fmt.Sprintf("%s=%s", log.GitalyLogDirEnvKey, cfg.Logging.Dir), - "GITALY_BIN_DIR=" + cfg.BinDir, - "GITALY_GITLAB_SHELL_CONFIG=" + string(gitlabShellConfigString), - }, nil -} diff --git a/internal/gitlabshell/env_test.go b/internal/gitlabshell/env_test.go deleted file mode 100644 index b4357c9f2..000000000 --- a/internal/gitlabshell/env_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package gitlabshell_test - -import ( - "encoding/json" - "io/ioutil" - "os" - "strings" - "testing" - - "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" - "gitlab.com/gitlab-org/gitaly/internal/testhelper" -) - -func TestGitHooksConfig(t *testing.T) { - defer func(cfg config.Cfg) { - config.Config = cfg - }(config.Config) - - require.NoError(t, testhelper.ConfigureRuby(&config.Config)) - - loggingDir, err := ioutil.TempDir("", t.Name()) - require.NoError(t, err) - defer func() { os.RemoveAll(loggingDir) }() - - config.Config.Logging.Dir = loggingDir - config.Config.Logging.Level = "fatal" - config.Config.Logging.Format = "my-custom-format" - config.Config.GitlabShell.Dir = "../../ruby/gitlab-shell" - config.Config.Hooks.CustomHooksDir = "/path/to/custom_hooks" - config.Config.Gitlab = config.Gitlab{ - URL: "http://gitlaburl.com", - HTTPSettings: config.HTTPSettings{ - ReadTimeout: 100, - User: "user_name", - Password: "pwpw", - CAFile: "/ca_file_path", - CAPath: "/ca_path", - SelfSigned: true, - }, - SecretFile: "secret_file_path", - } - - env, err := gitlabshell.EnvFromConfig(config.Config) - require.NoError(t, err) - - require.Contains(t, env, "GITALY_LOG_DIR="+config.Config.Logging.Dir) - require.Contains(t, env, "GITALY_BIN_DIR="+config.Config.BinDir) - - jsonShellConfig := "" - for _, envVar := range env { - if strings.HasPrefix(envVar, "GITALY_GITLAB_SHELL_CONFIG=") { - jsonShellConfig = strings.SplitN(envVar, "=", 2)[1] - break - } - } - - var configMap map[string]interface{} - - require.NoError(t, json.Unmarshal([]byte(jsonShellConfig), &configMap)) - require.Equal(t, config.Config.Logging.Level, configMap["log_level"]) - require.Equal(t, config.Config.Logging.Format, configMap["log_format"]) - require.Equal(t, config.Config.Gitlab.SecretFile, configMap["secret_file"]) - require.Equal(t, config.Config.Hooks.CustomHooksDir, configMap["custom_hooks_dir"]) - require.Equal(t, config.Config.Gitlab.URL, configMap["gitlab_url"]) - - // HTTP Settings - httpSettings, ok := configMap["http_settings"].(map[string]interface{}) - require.True(t, ok) - require.Equal(t, float64(config.Config.Gitlab.HTTPSettings.ReadTimeout), httpSettings["read_timeout"]) - require.Equal(t, config.Config.Gitlab.HTTPSettings.User, httpSettings["user"]) - require.Equal(t, config.Config.Gitlab.HTTPSettings.Password, httpSettings["password"]) - require.Equal(t, config.Config.Gitlab.HTTPSettings.CAFile, httpSettings["ca_file"]) - require.Equal(t, config.Config.Gitlab.HTTPSettings.CAPath, httpSettings["ca_path"]) - require.Equal(t, config.Config.Gitlab.HTTPSettings.SelfSigned, httpSettings["self_signed_cert"]) -} |