diff options
author | John Cai <jcai@gitlab.com> | 2020-05-05 21:06:03 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2020-06-09 19:25:06 +0300 |
commit | 3d35d8cbb7aee58e1a621d946dc23a2811d28690 (patch) | |
tree | 6a3afa31130701ef5211d70b201a03fde8eea9a5 | |
parent | 5c49063cb05152e09e9fcf067b68b6de6a21d561 (diff) |
PreReceive in go
Using gitlab-shell's internal api client library, make calls to the
gitlab api and port the ruby pre-receive fully to go.
-rw-r--r-- | changelogs/unreleased/jc-go-pre-receive.yml | 5 | ||||
-rw-r--r-- | cmd/gitaly-hooks/hooks.go | 4 | ||||
-rw-r--r-- | cmd/gitaly-hooks/hooks_test.go | 12 | ||||
-rw-r--r-- | go.sum | 8 | ||||
-rw-r--r-- | internal/git/receivepack.go | 1 | ||||
-rw-r--r-- | internal/metadata/featureflag/feature_flags.go | 7 | ||||
-rw-r--r-- | internal/service/hook/access.go | 22 | ||||
-rw-r--r-- | internal/service/hook/access_test.go | 18 | ||||
-rw-r--r-- | internal/service/hook/post_receive_test.go | 4 | ||||
-rw-r--r-- | internal/service/hook/pre_receive.go | 123 | ||||
-rw-r--r-- | internal/service/hook/pre_receive_test.go | 324 | ||||
-rw-r--r-- | internal/service/hook/testhelper_test.go | 8 | ||||
-rw-r--r-- | internal/service/hook/update_test.go | 4 | ||||
-rw-r--r-- | internal/service/smarthttp/receive_pack_test.go | 8 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 4 | ||||
-rw-r--r-- | ruby/lib/gitlab/git/hook.rb | 3 |
16 files changed, 492 insertions, 63 deletions
diff --git a/changelogs/unreleased/jc-go-pre-receive.yml b/changelogs/unreleased/jc-go-pre-receive.yml new file mode 100644 index 000000000..0afd8af3b --- /dev/null +++ b/changelogs/unreleased/jc-go-pre-receive.yml @@ -0,0 +1,5 @@ +--- +title: PreReceive in go +merge_request: 2155 +author: +type: changed diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go index 9eb674475..b145fef4d 100644 --- a/cmd/gitaly-hooks/hooks.go +++ b/cmd/gitaly-hooks/hooks.go @@ -116,6 +116,10 @@ func main() { } } + if os.Getenv(featureflag.GoPreReceiveHookEnvVar) == "true" { + environment = append(environment, fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar)) + } + if err := preReceiveHookStream.Send(&gitalypb.PreReceiveHookRequest{ Repository: repository, EnvironmentVariables: environment, diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index bf13a4707..54f3690b1 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -105,13 +105,18 @@ func TestHooksPrePostReceive(t *testing.T) { testhelper.WriteShellSecretFile(t, tempGitlabShellDir, secretToken) + config.Config.Gitlab.URL = ts.URL + config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret") + config.Config.Gitlab.HTTPSettings.User = gitlabUser + config.Config.Gitlab.HTTPSettings.Password = gitlabPassword + gitObjectDirRegex := regexp.MustCompile(`(?m)^GIT_OBJECT_DIRECTORY=(.*)$`) gitAlternateObjectDirRegex := regexp.MustCompile(`(?m)^GIT_ALTERNATE_OBJECT_DIRECTORIES=(.*)$`) token := "abc123" hookNames := []string{"pre-receive", "post-receive"} - featureSets, err := testhelper.NewFeatureSets([]string{featureflag.HooksRPC}) + featureSets, err := testhelper.NewFeatureSets([]string{featureflag.HooksRPC, featureflag.GoPreReceiveHook}) require.NoError(t, err) for _, hookName := range hookNames { @@ -159,6 +164,11 @@ func TestHooksPrePostReceive(t *testing.T) { if featureSet.IsEnabled(featureflag.HooksRPC) { cmd.Env = append(cmd.Env, fmt.Sprintf("%s=true", featureflag.HooksRPCEnvVar)) } + + if featureSet.IsEnabled(featureflag.GoPreReceiveHook) { + cmd.Env = append(cmd.Env, fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar)) + } + cmd.Dir = testRepoPath require.NoError(t, cmd.Run()) @@ -244,11 +244,15 @@ github.com/opentracing/opentracing-go v1.0.2 h1:3jA2P6O1F9UOrWVpwrIo17pu01KWvNWg github.com/opentracing/opentracing-go v1.0.2/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/otiai10/copy v1.0.1 h1:gtBjD8aq4nychvRZ2CyJvFWAw0aja+VHazDdruZKGZA= github.com/otiai10/copy v1.0.1/go.mod h1:8bMCJrAqOtN/d9oyh5HR7HhLQMvcGMpGdwRDYsfOCHc= +github.com/otiai10/copy v1.0.1/go.mod h1:8bMCJrAqOtN/d9oyh5HR7HhLQMvcGMpGdwRDYsfOCHc= +github.com/otiai10/copy v1.0.1/go.mod h1:8bMCJrAqOtN/d9oyh5HR7HhLQMvcGMpGdwRDYsfOCHc= github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE= github.com/otiai10/curr v1.0.0 h1:TJIWdbX0B+kpNagQrjgq8bCMrbhiuX73M2XwgtDMoOI= github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6j4vs= github.com/otiai10/mint v1.2.3 h1:PsrRBmrxR68kyNu6YlqYHbNlItc5vOkuS6LBEsNttVA= github.com/otiai10/mint v1.2.3/go.mod h1:YnfyPNhBvnY8bW4SGQHCs/aAFhkgySlMZbrF5U0bOVw= +github.com/otiai10/mint v1.2.3/go.mod h1:YnfyPNhBvnY8bW4SGQHCs/aAFhkgySlMZbrF5U0bOVw= +github.com/otiai10/mint v1.2.3/go.mod h1:YnfyPNhBvnY8bW4SGQHCs/aAFhkgySlMZbrF5U0bOVw= github.com/otiai10/mint v1.3.0 h1:Ady6MKVezQwHBkGzLFbrsywyp09Ah7rkmfjV3Bcr5uc= github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT91xUo= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= @@ -347,8 +351,11 @@ github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs= github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0= gitlab.com/gitlab-org/gitaly v1.68.0/go.mod h1:/pCsB918Zu5wFchZ9hLYin9WkJ2yQqdVNz0zlv5HbXg= gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20200506213341-716e30c55e89 h1:gZwXV5WLPmJ3NDmH+zAZkgWLVhgKvamGAjLXU5W0GiU= +gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20200506213341-716e30c55e89 h1:gZwXV5WLPmJ3NDmH+zAZkgWLVhgKvamGAjLXU5W0GiU= +gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20200506213341-716e30c55e89/go.mod h1:oUGdKtJHWQP4/VQ/NONJZy/8X2E5ow2eGTUDwdzvGsc= gitlab.com/gitlab-org/gitlab-shell v1.9.8-0.20200506213341-716e30c55e89/go.mod h1:oUGdKtJHWQP4/VQ/NONJZy/8X2E5ow2eGTUDwdzvGsc= gitlab.com/gitlab-org/labkit v0.0.0-20190221122536-0c3fc7cdd57c/go.mod h1:rYhLgfrbEcyfinG+R3EvKu6bZSsmwQqcXzLfHWSfUKM= +gitlab.com/gitlab-org/labkit v0.0.0-20190221122536-0c3fc7cdd57c/go.mod h1:rYhLgfrbEcyfinG+R3EvKu6bZSsmwQqcXzLfHWSfUKM= gitlab.com/gitlab-org/labkit v0.0.0-20200507062444-0149780c759d h1:Q5yZi+AelheHuvq/OK6DiaBzLU1AHrm7eWh88uE8Tsk= gitlab.com/gitlab-org/labkit v0.0.0-20200507062444-0149780c759d/go.mod h1:SNfxkfUwVNECgtmluVayv0GWFgEjjBs5AzgsowPQuo0= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= @@ -408,6 +415,7 @@ golang.org/x/net v0.0.0-20190501004415-9ce7a6920f09/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190503192946-f4e77d36d62c/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859 h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190724013045-ca1201d0de80/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go index 8e57fea22..2d27d620d 100644 --- a/internal/git/receivepack.go +++ b/internal/git/receivepack.go @@ -48,6 +48,7 @@ func ReceivePackHookEnv(ctx context.Context, req ReceivePackRequest) ([]string, fmt.Sprintf("GITALY_TOKEN=%s", config.Config.Auth.Token), fmt.Sprintf("%s=%s", featureflag.HooksRPCEnvVar, strconv.FormatBool(featureflag.IsEnabled(ctx, featureflag.HooksRPC))), fmt.Sprintf("%s=%s", featureflag.GoUpdateHookEnvVar, strconv.FormatBool(featureflag.IsEnabled(ctx, featureflag.GoUpdateHook))), + fmt.Sprintf("%s=%s", featureflag.GoPreReceiveHookEnvVar, strconv.FormatBool(featureflag.IsEnabled(ctx, featureflag.GoPreReceiveHook))), }, gitlabshellEnv...) praefect, err := metadata.ExtractPraefectServer(ctx) diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go index 6d3eee60f..b302fe076 100644 --- a/internal/metadata/featureflag/feature_flags.go +++ b/internal/metadata/featureflag/feature_flags.go @@ -20,10 +20,13 @@ const ( ReferenceTransactions = "reference_transactions" // DistributedReads allows praefect to redirect accessor operations to up-to-date secondaries DistributedReads = "distributed_reads" + // GoPrereceiveHook will bypass the ruby pre-receive hook and use the go implementation + GoPreReceiveHook = "go_prereceive_hook" ) const ( // HooksRPCEnvVar is the name of the environment variable we use to pass the feature flag down into gitaly-hooks - HooksRPCEnvVar = "GITALY_HOOK_RPCS_ENABLED" - GoUpdateHookEnvVar = "GITALY_GO_UPDATE" + HooksRPCEnvVar = "GITALY_HOOK_RPCS_ENABLED" + GoUpdateHookEnvVar = "GITALY_GO_UPDATE" + GoPreReceiveHookEnvVar = "GITALY_GO_PRERECEIVE" ) diff --git a/internal/service/hook/access.go b/internal/service/hook/access.go index 1cbfdcbcd..454add6a2 100644 --- a/internal/service/hook/access.go +++ b/internal/service/hook/access.go @@ -53,7 +53,7 @@ func marshallGitObjectDirs(gitObjectDirRel string, gitAltObjectDirsRel []string) // GitlabAPI is an interface for accessing the gitlab internal API type GitlabAPI interface { // Allowed queries the gitlab internal api /allowed endpoint to determine if a ref change for a given repository and user is allowed - Allowed(repo *gitalypb.Repository, glRepository, glID, glProtocol, changes string) (bool, error) + Allowed(repo *gitalypb.Repository, glRepository, glID, glProtocol, changes string) (bool, string, error) // PreReceive queries the gitlab internal api /pre_receive to increase the reference counter PreReceive(glRepository string) (bool, error) } @@ -93,15 +93,15 @@ func NewGitlabAPI(gitlabCfg config.Gitlab) (GitlabAPI, error) { } // Allowed checks if a ref change for a given repository is allowed through the gitlab internal api /allowed endpoint -func (a *gitlabAPI) Allowed(repo *gitalypb.Repository, glRepository, glID, glProtocol, changes string) (bool, error) { +func (a *gitlabAPI) Allowed(repo *gitalypb.Repository, glRepository, glID, glProtocol, changes string) (bool, string, error) { repoPath, err := helper.GetRepoPath(repo) if err != nil { - return false, fmt.Errorf("getting the repository path: %w", err) + return false, "", fmt.Errorf("getting the repository path: %w", err) } gitObjDirVars, err := marshallGitObjectDirs(repo.GetGitObjectDirectory(), repo.GetGitAlternateObjectDirectories()) if err != nil { - return false, fmt.Errorf("when getting git object directories json encoded string: %w", err) + return false, "", fmt.Errorf("when getting git object directories json encoded string: %w", err) } req := AllowedRequest{ @@ -114,12 +114,12 @@ func (a *gitlabAPI) Allowed(repo *gitalypb.Repository, glRepository, glID, glPro } if err := req.parseAndSetGLID(glID); err != nil { - return false, fmt.Errorf("setting gl_id: %w", err) + return false, "", fmt.Errorf("setting gl_id: %w", err) } resp, err := a.client.Post("/allowed", &req) if err != nil { - return false, fmt.Errorf("http post to gitlab api /allowed endpoint: %w", err) + return false, "", err } defer func() { @@ -135,21 +135,21 @@ func (a *gitlabAPI) Allowed(repo *gitalypb.Repository, glRepository, glID, glPro mtype, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type")) if err != nil { - return false, fmt.Errorf("/allowed endpoint respond with unsupported content type: %w", err) + return false, "", fmt.Errorf("/allowed endpoint respond with unsupported content type: %w", err) } if mtype != "application/json" { - return false, fmt.Errorf("/allowed endpoint respond with unsupported content type: %s", mtype) + return false, "", fmt.Errorf("/allowed endpoint respond with unsupported content type: %s", mtype) } if err = json.NewDecoder(resp.Body).Decode(&response); err != nil { - return false, fmt.Errorf("decoding response from /allowed endpoint: %w", err) + return false, "", fmt.Errorf("decoding response from /allowed endpoint: %w", err) } default: - return false, fmt.Errorf("gitlab api is not accessible: %d", resp.StatusCode) + return false, "", fmt.Errorf("gitlab api is not accessible: %d", resp.StatusCode) } - return response.Status, nil + return response.Status, response.Message, nil } type preReceiveResponse struct { diff --git a/internal/service/hook/access_test.go b/internal/service/hook/access_test.go index 432ad2b04..3aca12c9a 100644 --- a/internal/service/hook/access_test.go +++ b/internal/service/hook/access_test.go @@ -96,7 +96,7 @@ func TestAllowedVerifyParams(t *testing.T) { } for _, tc := range testCases { - allowed, err := c.Allowed(tc.repo, tc.glRepository, tc.glID, tc.protocol, tc.changes) + allowed, _, err := c.Allowed(tc.repo, tc.glRepository, tc.glID, tc.protocol, tc.changes) require.NoError(t, err) require.Equal(t, tc.allowed, allowed) } @@ -125,6 +125,7 @@ func TestAllowedResponseHandling(t *testing.T) { allowed bool errMsg string }{ + { desc: "allowed", allowedHandler: func(w http.ResponseWriter, r *http.Request) { @@ -135,6 +136,17 @@ func TestAllowedResponseHandling(t *testing.T) { allowed: true, errMsg: "", }, + + { + desc: "not allowed", + allowedHandler: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"status": false, "message": "this change is not allowed"}`)) + }, + allowed: false, + errMsg: "this change is not allowed", + }, { desc: "bad content type in response", allowedHandler: func(w http.ResponseWriter, r *http.Request) { @@ -216,10 +228,12 @@ func TestAllowedResponseHandling(t *testing.T) { }) require.NoError(t, err) - allowed, err := c.Allowed(testRepo, "repo-1", "key-123", "http", "a\nb\nc\nd") + allowed, message, err := c.Allowed(testRepo, "repo-1", "key-123", "http", "a\nb\nc\nd") require.Equal(t, tc.allowed, allowed) if err != nil { require.Contains(t, err.Error(), tc.errMsg) + } else { + require.Equal(t, tc.errMsg, message) } }) } diff --git a/internal/service/hook/post_receive_test.go b/internal/service/hook/post_receive_test.go index 451c322f7..65dd4299c 100644 --- a/internal/service/hook/post_receive_test.go +++ b/internal/service/hook/post_receive_test.go @@ -18,7 +18,7 @@ import ( ) func TestPostReceiveInvalidArgument(t *testing.T) { - serverSocketPath, stop := runHooksServer(t) + serverSocketPath, stop := runHooksServer(t, config.Config.Hooks) defer stop() client, conn := newHooksClient(t, serverSocketPath) @@ -45,7 +45,7 @@ func TestPostReceive(t *testing.T) { require.NoError(t, err) config.Config.Ruby.Dir = filepath.Join(cwd, "testdata") - serverSocketPath, stop := runHooksServer(t) + serverSocketPath, stop := runHooksServer(t, config.Config.Hooks) defer stop() testRepo, _, cleanupFn := testhelper.NewTestRepo(t) diff --git a/internal/service/hook/pre_receive.go b/internal/service/hook/pre_receive.go index b4627ee13..8fcd6d5a7 100644 --- a/internal/service/hook/pre_receive.go +++ b/internal/service/hook/pre_receive.go @@ -1,18 +1,22 @@ package hook import ( + "bytes" "context" "crypto/sha1" "errors" "fmt" + "io/ioutil" "os/exec" "path/filepath" + "strings" "time" "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/git/alternates" "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" @@ -122,14 +126,120 @@ func (s *server) voteOnTransaction(stream gitalypb.HookService_PreReceiveHookSer func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer) error { firstRequest, err := stream.Recv() if err != nil { - return helper.ErrInternal(err) + return helper.ErrInternalf("receiving first request: %w", err) } if err := validatePreReceiveHookRequest(firstRequest); err != nil { return helper.ErrInvalidArgument(err) } + reqEnvVars := firstRequest.GetEnvironmentVariables() + repository := firstRequest.GetRepository() + + if !useGoPreReceiveHook(reqEnvVars) { + return s.preReceiveHookRuby(firstRequest, stream) + } + + stdout := streamio.NewWriter(func(p []byte) error { return stream.Send(&gitalypb.PreReceiveHookResponse{Stdout: p}) }) + stderr := streamio.NewWriter(func(p []byte) error { return stream.Send(&gitalypb.PreReceiveHookResponse{Stderr: p}) }) + + stdin := streamio.NewReader(func() ([]byte, error) { + req, err := stream.Recv() + return req.GetStdin(), err + }) + + changes, err := ioutil.ReadAll(stdin) + if err != nil { + return helper.ErrInternalf("reading stdin from request: %w", err) + } + + glID, glRepo, glProtocol := getEnvVar("GL_ID", reqEnvVars), getEnvVar("GL_REPOSITORY", reqEnvVars), getEnvVar("GL_PROTOCOL", reqEnvVars) + + allowed, message, err := s.gitlabAPI.Allowed(repository, glRepo, glID, glProtocol, string(changes)) + if err != nil { + return preReceiveHookResponse(stream, int32(1), fmt.Sprintf("GitLab: %v", err)) + } + if !allowed { + return preReceiveHookResponse(stream, int32(1), message) + } + + // custom hooks execution + repoPath, err := helper.GetRepoPath(repository) + if err != nil { + return err + } + executor, err := newCustomHooksExecutor(repoPath, s.hooksConfig.CustomHooksDir, "pre-receive") + if err != nil { + return helper.ErrInternalf("creating custom hooks executor: %v", err) + } + + _, gitObjectDirEnv, err := alternates.PathAndEnv(repository) + if err != nil { + return helper.ErrInternalf("getting git object dir from request %v", err) + } + + env := append(reqEnvVars, gitObjectDirEnv...) + + if err = executor( + stream.Context(), + nil, + env, + bytes.NewReader(changes), + stdout, + stderr, + ); err != nil { + var exitError *exec.ExitError + if errors.As(err, &exitError) { + return preReceiveHookResponse(stream, int32(exitError.ExitCode()), "") + } + + return helper.ErrInternalf("executing custom hooks: %v", err) + } + + // reference counter + ok, err := s.gitlabAPI.PreReceive(glRepo) + if err != nil { + return helper.ErrInternalf("calling pre_receive endpoint: %v", err) + } + + if !ok { + return preReceiveHookResponse(stream, 1, "") + } + + hash := sha1.Sum(changes) + if err := s.voteOnTransaction(stream, hash[:], env); err != nil { + return helper.ErrInternalf("error voting on transaction: %v", err) + } + + return preReceiveHookResponse(stream, 0, "") +} + +func validatePreReceiveHookRequest(in *gitalypb.PreReceiveHookRequest) error { + if in.GetRepository() == nil { + return errors.New("repository is empty") + } + + return nil +} + +func useGoPreReceiveHook(env []string) bool { + return getEnvVar(featureflag.GoPreReceiveHookEnvVar, env) == "true" +} + +func preReceiveHookResponse(stream gitalypb.HookService_PreReceiveHookServer, code int32, stderr string) error { + if err := stream.Send(&gitalypb.PreReceiveHookResponse{ + ExitStatus: &gitalypb.ExitStatus{Value: code}, + Stderr: []byte(stderr), + }); err != nil { + return helper.ErrInternalf("sending response: %v", err) + } + + return nil +} + +func (s *server) preReceiveHookRuby(firstRequest *gitalypb.PreReceiveHookRequest, stream gitalypb.HookService_PreReceiveHookServer) error { referenceUpdatesHasher := sha1.New() + stdin := streamio.NewReader(func() ([]byte, error) { req, err := stream.Recv() if err != nil { @@ -183,10 +293,13 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer return nil } -func validatePreReceiveHookRequest(in *gitalypb.PreReceiveHookRequest) error { - if in.GetRepository() == nil { - return errors.New("repository is empty") +func getEnvVar(key string, vars []string) string { + for _, varPair := range vars { + kv := strings.SplitN(varPair, "=", 2) + if kv[0] == key { + return kv[1] + } } - return nil + return "" } diff --git a/internal/service/hook/pre_receive_test.go b/internal/service/hook/pre_receive_test.go index e1d89dccf..e98e44227 100644 --- a/internal/service/hook/pre_receive_test.go +++ b/internal/service/hook/pre_receive_test.go @@ -2,7 +2,10 @@ package hook import ( "bytes" + "fmt" "io" + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -11,6 +14,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" @@ -18,7 +22,7 @@ import ( ) func TestPreReceiveInvalidArgument(t *testing.T) { - serverSocketPath, stop := runHooksServer(t) + serverSocketPath, stop := runHooksServer(t, config.Config.Hooks) defer stop() client, conn := newHooksClient(t, serverSocketPath) @@ -35,6 +39,39 @@ func TestPreReceiveInvalidArgument(t *testing.T) { testhelper.RequireGrpcError(t, err, codes.InvalidArgument) } +func receivePreReceive(t *testing.T, stream gitalypb.HookService_PreReceiveHookClient, stdin io.Reader) ([]byte, []byte, int32) { + go func() { + writer := streamio.NewWriter(func(p []byte) error { + return stream.Send(&gitalypb.PreReceiveHookRequest{Stdin: p}) + }) + _, err := io.Copy(writer, stdin) + require.NoError(t, err) + require.NoError(t, stream.CloseSend(), "close send") + }() + + var status int32 + var stdout, stderr bytes.Buffer + for { + resp, err := stream.Recv() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("error when receiving stream: %v", err) + } + + _, err = stdout.Write(resp.GetStdout()) + require.NoError(t, err) + _, err = stderr.Write(resp.GetStderr()) + require.NoError(t, err) + + status = resp.GetExitStatus().GetValue() + require.NoError(t, err) + } + + return stdout.Bytes(), stderr.Bytes(), status +} + func TestPreReceive(t *testing.T) { rubyDir := config.Config.Ruby.Dir defer func(rubyDir string) { @@ -45,7 +82,7 @@ func TestPreReceive(t *testing.T) { require.NoError(t, err) config.Config.Ruby.Dir = filepath.Join(cwd, "testdata") - serverSocketPath, stop := runHooksServer(t) + serverSocketPath, stop := runHooksServer(t, config.Config.Hooks) defer stop() testRepo, _, cleanupFn := testhelper.NewTestRepo(t) @@ -168,38 +205,261 @@ func TestPreReceive(t *testing.T) { require.NoError(t, err) require.NoError(t, stream.Send(&tc.req)) - go func() { - writer := streamio.NewWriter(func(p []byte) error { - return stream.Send(&gitalypb.PreReceiveHookRequest{Stdin: p}) - }) - _, err := io.Copy(writer, tc.stdin) - require.NoError(t, err) - require.NoError(t, stream.CloseSend(), "close send") - }() - - var status int32 - var stdout, stderr bytes.Buffer - for { - resp, err := stream.Recv() - if err == io.EOF { - break - } - if err != nil { - t.Errorf("error when receiving stream: %v", err) - } - - _, err = stdout.Write(resp.GetStdout()) - require.NoError(t, err) - _, err = stderr.Write(resp.GetStderr()) - require.NoError(t, err) - - status = resp.GetExitStatus().GetValue() - require.NoError(t, err) - } + stdout, stderr, status := receivePreReceive(t, stream, tc.stdin) assert.Equal(t, tc.status, status) - assert.Equal(t, tc.stderr, text.ChompBytes(stderr.Bytes()), "hook stderr") - assert.Equal(t, tc.stdout, text.ChompBytes(stdout.Bytes()), "hook stdout") + assert.Equal(t, tc.stderr, text.ChompBytes(stderr), "hook stderr") + assert.Equal(t, tc.stdout, text.ChompBytes(stdout), "hook stdout") + }) + } +} + +func TestPreReceiveHook_GitlabAPIAccess(t *testing.T) { + user, password := "user", "password" + secretToken := "secret123" + glID, glRepository := "key-123", "repository" + changes := "changes123" + protocol := "http" + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) + defer cleanupFn() + + gitObjectDirRel := "git/object/dir" + gitAlternateObjectRelDirs := []string{"alt/obj/dir/1", "alt/obj/dir/2"} + + gitObjectDirAbs := filepath.Join(testRepoPath, gitObjectDirRel) + var gitAlternateObjectAbsDirs []string + + for _, gitAltObjectRel := range gitAlternateObjectRelDirs { + gitAlternateObjectAbsDirs = append(gitAlternateObjectAbsDirs, filepath.Join(testRepoPath, gitAltObjectRel)) + } + + tmpDir, cleanup := testhelper.TempDir(t) + defer cleanup() + secretFilePath := filepath.Join(tmpDir, ".gitlab_shell_secret") + testhelper.WriteShellSecretFile(t, tmpDir, secretToken) + + testRepo.GitObjectDirectory = gitObjectDirRel + testRepo.GitAlternateObjectDirectories = gitAlternateObjectRelDirs + + server := testhelper.NewGitlabTestServer(testhelper.GitlabTestServerOptions{ + User: user, + Password: password, + SecretToken: secretToken, + GLID: glID, + GLRepository: glRepository, + Changes: changes, + PostReceiveCounterDecreased: true, + Protocol: protocol, + GitPushOptions: nil, + GitObjectDir: gitObjectDirAbs, + GitAlternateObjectDirs: gitAlternateObjectAbsDirs, + RepoPath: testRepoPath, + }) + + defer server.Close() + + gitlabConfig := config.Gitlab{ + URL: server.URL, + HTTPSettings: config.HTTPSettings{ + User: user, + Password: password, + }, + SecretFile: secretFilePath, + } + + gitlabAPI, err := NewGitlabAPI(gitlabConfig) + require.NoError(t, err) + + serverSocketPath, stop := runHooksServerWithAPI(t, gitlabAPI, config.Hooks{}) + defer stop() + + client, conn := newHooksClient(t, serverSocketPath) + defer conn.Close() + + ctx, cancel := testhelper.Context() + defer cancel() + + stdin := bytes.NewBufferString(changes) + req := gitalypb.PreReceiveHookRequest{ + Repository: testRepo, + EnvironmentVariables: []string{ + "GL_ID=" + glID, + "GL_PROTOCOL=" + protocol, + "GL_USERNAME=username", + "GL_REPOSITORY=" + glRepository, + fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar), + }, + } + + stream, err := client.PreReceiveHook(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&req)) + + stdout, stderr, status := receivePreReceive(t, stream, stdin) + + require.Equal(t, int32(0), status) + assert.Equal(t, "", text.ChompBytes(stderr), "hook stderr") + assert.Equal(t, "", text.ChompBytes(stdout), "hook stdout") +} + +func TestPreReceive_APIErrors(t *testing.T) { + testRepo, _, cleanupFn := testhelper.NewTestRepo(t) + defer cleanupFn() + + testCases := []struct { + desc string + allowedHandler http.HandlerFunc + preReceiveHandler http.HandlerFunc + expectedExitStatus int32 + expectedStderr string + }{ + { + desc: "/allowed endpoint returns 401", + allowedHandler: http.HandlerFunc( + func(res http.ResponseWriter, req *http.Request) { + res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusUnauthorized) + res.Write([]byte(`{"message":"not allowed"}`)) + }), + expectedExitStatus: 1, + expectedStderr: "GitLab: not allowed", + }, + { + desc: "/pre_receive endpoint fails to increase reference coutner", + allowedHandler: http.HandlerFunc( + func(res http.ResponseWriter, req *http.Request) { + res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusOK) + res.Write([]byte(`{"status": true}`)) + }), + preReceiveHandler: http.HandlerFunc( + func(res http.ResponseWriter, req *http.Request) { + res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusOK) + res.Write([]byte(`{"reference_counter_increased": false}`)) + }), + expectedExitStatus: 1, + }, + } + + tmpDir, cleanup := testhelper.TempDir(t) + defer cleanup() + secretFilePath := filepath.Join(tmpDir, ".gitlab_shell_secret") + testhelper.WriteShellSecretFile(t, tmpDir, "token") + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + mux := http.NewServeMux() + mux.Handle("/api/v4/internal/allowed", tc.allowedHandler) + mux.Handle("/api/v4/internal/pre_receive", tc.preReceiveHandler) + srv := httptest.NewServer(mux) + defer srv.Close() + + gitlabConfig := config.Gitlab{ + URL: srv.URL, + SecretFile: secretFilePath, + } + + gitlabAPI, err := NewGitlabAPI(gitlabConfig) + require.NoError(t, err) + + serverSocketPath, stop := runHooksServerWithAPI(t, gitlabAPI, config.Hooks{}) + defer stop() + + client, conn := newHooksClient(t, serverSocketPath) + defer conn.Close() + + ctx, cancel := testhelper.Context() + defer cancel() + + stream, err := client.PreReceiveHook(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{ + Repository: testRepo, + EnvironmentVariables: []string{ + "GL_ID=key-123", + "GL_PROTOCOL=web", + "GL_USERNAME=username", + "GL_REPOSITORY=repository", + fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar), + }, + })) + require.NoError(t, stream.CloseSend()) + + _, stderr, status := receivePreReceive(t, stream, &bytes.Buffer{}) + + require.Equal(t, tc.expectedExitStatus, status) + assert.Equal(t, tc.expectedStderr, text.ChompBytes(stderr), "hook stderr") }) } } + +func TestPreReceiveHook_CustomHookErrors(t *testing.T) { + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) + defer cleanupFn() + + mux := http.NewServeMux() + mux.Handle("/api/v4/internal/allowed", + http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusOK) + res.Write([]byte(`{"status": true}`)) + })) + mux.Handle("/api/v4/internal/pre_receive", http.HandlerFunc( + func(res http.ResponseWriter, req *http.Request) { + res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusOK) + res.Write([]byte(`{"reference_counter_increased": true}`)) + })) + srv := httptest.NewServer(mux) + defer srv.Close() + + tmpDir, cleanup := testhelper.TempDir(t) + defer cleanup() + secretFilePath := filepath.Join(tmpDir, ".gitlab_shell_secret") + testhelper.WriteShellSecretFile(t, tmpDir, "token") + + customHookReturnCode := int32(128) + customHookReturnMsg := "custom hook error" + testhelper.WriteCustomHook(testRepoPath, "pre-receive", []byte(fmt.Sprintf(`#!/bin/bash +echo '%s' 1>&2 +exit %d +`, customHookReturnMsg, customHookReturnCode))) + + gitlabConfig := config.Gitlab{ + URL: srv.URL, + SecretFile: secretFilePath, + } + + gitlabAPI, err := NewGitlabAPI(gitlabConfig) + require.NoError(t, err) + + serverSocketPath, stop := runHooksServerWithAPI(t, gitlabAPI, config.Hooks{}) + defer stop() + + client, conn := newHooksClient(t, serverSocketPath) + defer conn.Close() + + ctx, cancel := testhelper.Context() + defer cancel() + + stream, err := client.PreReceiveHook(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{ + Repository: testRepo, + EnvironmentVariables: []string{ + "GL_ID=key-123", + "GL_PROTOCOL=web", + "GL_USERNAME=username", + "GL_REPOSITORY=repository", + fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar), + }, + })) + + require.NoError(t, stream.CloseSend()) + + _, stderr, status := receivePreReceive(t, stream, &bytes.Buffer{}) + + require.Equal(t, customHookReturnCode, status) + assert.Equal(t, customHookReturnMsg, text.ChompBytes(stderr), "hook stderr") +} diff --git a/internal/service/hook/testhelper_test.go b/internal/service/hook/testhelper_test.go index 28fdf720f..a59596269 100644 --- a/internal/service/hook/testhelper_test.go +++ b/internal/service/hook/testhelper_test.go @@ -31,10 +31,14 @@ func newHooksClient(t *testing.T, serverSocketPath string) (gitalypb.HookService return gitalypb.NewHookServiceClient(conn), conn } -func runHooksServer(t *testing.T) (string, func()) { +func runHooksServer(t *testing.T, hooksCfg config.Hooks) (string, func()) { + return runHooksServerWithAPI(t, testhelper.GitlabAPIStub, hooksCfg) +} + +func runHooksServerWithAPI(t *testing.T, gitlabAPI GitlabAPI, hooksCfg config.Hooks) (string, func()) { srv := testhelper.NewServer(t, nil, nil) - gitalypb.RegisterHookServiceServer(srv.GrpcServer(), NewServer(testhelper.GitlabAPIStub, config.Config.Hooks)) + gitalypb.RegisterHookServiceServer(srv.GrpcServer(), NewServer(gitlabAPI, hooksCfg)) reflection.Register(srv.GrpcServer()) require.NoError(t, srv.Start()) diff --git a/internal/service/hook/update_test.go b/internal/service/hook/update_test.go index 1155371d3..4acf58ee3 100644 --- a/internal/service/hook/update_test.go +++ b/internal/service/hook/update_test.go @@ -17,7 +17,7 @@ import ( ) func TestUpdateInvalidArgument(t *testing.T) { - serverSocketPath, stop := runHooksServer(t) + serverSocketPath, stop := runHooksServer(t, config.Config.Hooks) defer stop() client, conn := newHooksClient(t, serverSocketPath) @@ -43,7 +43,7 @@ func TestUpdate(t *testing.T) { require.NoError(t, err) config.Config.Ruby.Dir = filepath.Join(cwd, "testdata") - serverSocketPath, stop := runHooksServer(t) + serverSocketPath, stop := runHooksServer(t, config.Config.Hooks) defer stop() testRepo, _, cleanupFn := testhelper.NewTestRepo(t) diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go index 35a83d23f..0c40b4681 100644 --- a/internal/service/smarthttp/receive_pack_test.go +++ b/internal/service/smarthttp/receive_pack_test.go @@ -474,7 +474,7 @@ func TestPostReceiveWithTransactions(t *testing.T) { gitlabUser := "gitlab_user-1234" gitlabPassword := "gitlabsecret9887" - featureSets, err := testhelper.NewFeatureSets([]string{featureflag.HooksRPC, featureflag.ReferenceTransactions}) + featureSets, err := testhelper.NewFeatureSets([]string{featureflag.HooksRPC, featureflag.ReferenceTransactions, featureflag.GoPreReceiveHook}) require.NoError(t, err) for _, features := range featureSets { @@ -506,6 +506,12 @@ func TestPostReceiveWithTransactions(t *testing.T) { Password: gitlabPassword, }, }) + + config.Config.Gitlab.URL = gitlabServer.URL + config.Config.Gitlab.HTTPSettings.User = gitlabUser + config.Config.Gitlab.HTTPSettings.Password = gitlabPassword + config.Config.Gitlab.SecretFile = filepath.Join(gitlabShellDir, ".gitlab_shell_secret") + testhelper.WriteShellSecretFile(t, gitlabShellDir, secretToken) gitalyServer := testhelper.NewServerWithAuth(t, nil, nil, config.Config.Auth.Token) diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index e2fabb33f..200b46375 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -795,8 +795,8 @@ func NewFeatureSets(goFeatures []string, rubyFeatures ...string) (FeatureSets, e type mockAPI struct { } -func (m *mockAPI) Allowed(repo *gitalypb.Repository, glRepository, glID, glProtocol, changes string) (bool, error) { - return true, nil +func (m *mockAPI) Allowed(repo *gitalypb.Repository, glRepository, glID, glProtocol, changes string) (bool, string, error) { + return true, "", nil } func (m *mockAPI) PreReceive(glRepository string) (bool, error) { diff --git a/ruby/lib/gitlab/git/hook.rb b/ruby/lib/gitlab/git/hook.rb index 037fd06ed..4cea56305 100644 --- a/ruby/lib/gitlab/git/hook.rb +++ b/ruby/lib/gitlab/git/hook.rb @@ -119,7 +119,8 @@ module Gitlab 'GITALY_REPO' => repository.gitaly_repository.to_json, 'GITALY_SOCKET' => Gitlab.config.gitaly.internal_socket, 'GITALY_HOOK_RPCS_ENABLED' => repository.feature_enabled?('call-hook-rpc').to_s, - 'GITALY_GO_UPDATE' => repository.feature_enabled?('go-update-hook').to_s + 'GITALY_GO_UPDATE' => repository.feature_enabled?('go-update-hook').to_s, + 'GITALY_GO_PRERECEIVE' => repository.feature_enabled?('go-prereceive-hook').to_s } end end |