Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZeger-Jan van de Weg <git@zjvandeweg.nl>2020-09-09 15:58:27 +0300
committerZeger-Jan van de Weg <git@zjvandeweg.nl>2020-09-30 15:07:01 +0300
commit8f5eecb4bb6f7b0d9bedcf3bded2529a0c01cf09 (patch)
tree87de9014ffec660f9d958727cfd6b4c8c3e45e97
parent538e5d075acdbb46307d293251ae68fe0dd41b42 (diff)
hooks: Remove Ruby execution path for postreceive
Currently the only hook in production running their Ruby implementation is the postreceive hook. For GitLab.com the feature flag was toggled, and performs very well. As such the feature will be expose to all GitLab installs, as the feature flag is removed in this change.
-rw-r--r--.gitignore1
-rw-r--r--cmd/gitaly-hooks/hooks.go4
-rw-r--r--cmd/gitaly-hooks/hooks_test.go141
-rw-r--r--internal/git/receivepack.go1
-rw-r--r--internal/gitaly/service/hook/post_receive.go72
-rw-r--r--internal/gitaly/service/hook/post_receive_test.go377
-rw-r--r--internal/gitaly/service/hook/pre_receive.go33
-rw-r--r--internal/gitaly/service/hook/stream_command.go34
-rwxr-xr-xinternal/gitaly/service/hook/testdata/gitlab-shell/hooks/post-receive10
-rw-r--r--internal/gitaly/service/operations/rebase_test.go18
-rw-r--r--internal/gitaly/service/operations/tags_test.go18
-rw-r--r--internal/gitaly/service/smarthttp/receive_pack_test.go24
-rw-r--r--internal/metadata/featureflag/feature_flags.go4
-rw-r--r--ruby/lib/gitlab/git/hook.rb1
14 files changed, 217 insertions, 521 deletions
diff --git a/.gitignore b/.gitignore
index 4f6c22b79..9d277070e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,7 +13,6 @@ cmd/gitaly-wrapper/gitaly-wrapper
/_support/bin
/internal/service/smarthttp/testdata
/internal/testhelper/testdata/data
-/internal/testhelper/testdata/gitaly-libexec
/*.toml
/.ruby-bundle
/ruby/vendor/bundle
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go
index 009590eee..dda50710c 100644
--- a/cmd/gitaly-hooks/hooks.go
+++ b/cmd/gitaly-hooks/hooks.go
@@ -172,10 +172,6 @@ func main() {
}
}
- if os.Getenv(featureflag.GoPostReceiveHookEnvVar) == "true" {
- environment = append(environment, fmt.Sprintf("%s=true", featureflag.GoPostReceiveHookEnvVar))
- }
-
if err := postReceiveHookStream.Send(&gitalypb.PostReceiveHookRequest{
Repository: repository,
EnvironmentVariables: environment,
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index 846ab3504..5e219e983 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -18,7 +18,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/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
"gitlab.com/gitlab-org/gitaly/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -228,82 +227,80 @@ func testHooksPrePostReceive(t *testing.T) {
hookNames := []string{"pre-receive", "post-receive"}
for _, hookName := range hookNames {
- for _, useGoPostReceive := range []bool{true, false} {
- t.Run(fmt.Sprintf("hookName: %s, using Go PostReceive: %v", hookName, useGoPostReceive), func(t *testing.T) {
- customHookOutputPath, cleanup := testhelper.WriteEnvToCustomHook(t, testRepoPath, hookName)
- defer cleanup()
+ t.Run(fmt.Sprintf("hookName: %s", hookName), func(t *testing.T) {
+ customHookOutputPath, cleanup := testhelper.WriteEnvToCustomHook(t, testRepoPath, hookName)
+ defer cleanup()
- config.Config.Gitlab.URL = serverURL
- config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
- config.Config.Gitlab.HTTPSettings.User = gitlabUser
- config.Config.Gitlab.HTTPSettings.Password = gitlabPassword
+ config.Config.Gitlab.URL = serverURL
+ config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
+ config.Config.Gitlab.HTTPSettings.User = gitlabUser
+ config.Config.Gitlab.HTTPSettings.Password = gitlabPassword
- gitlabAPI, err := gitalyhook.NewGitlabAPI(config.Config.Gitlab)
- require.NoError(t, err)
+ gitlabAPI, err := gitalyhook.NewGitlabAPI(config.Config.Gitlab)
+ require.NoError(t, err)
- socket, stop := runHookServiceServerWithAPI(t, token, gitlabAPI)
- defer stop()
+ socket, stop := runHookServiceServerWithAPI(t, token, gitlabAPI)
+ defer stop()
- var stderr, stdout bytes.Buffer
- stdin := bytes.NewBuffer([]byte(changes))
- hookPath, err := filepath.Abs(fmt.Sprintf("../../ruby/git-hooks/%s", hookName))
- require.NoError(t, err)
- cmd := exec.Command(hookPath)
- cmd.Stderr = &stderr
- cmd.Stdout = &stdout
- cmd.Stdin = stdin
- cmd.Env = append(testhelper.EnvForHooks(
- t,
- tempGitlabShellDir,
- socket,
- token,
- testRepo,
- testhelper.GlHookValues{
- GLID: glID,
- GLUsername: glUsername,
- GLRepo: glRepository,
- GLProtocol: glProtocol,
- GitObjectDir: c.GitObjectDir,
- GitAlternateObjectDirs: c.GitAlternateObjectDirs,
- },
- testhelper.ProxyValues{
- HTTPProxy: httpProxy,
- HTTPSProxy: httpsProxy,
- NoProxy: noProxy,
- },
- gitPushOptions...,
- ), fmt.Sprintf("%s=%v", featureflag.GoPostReceiveHookEnvVar, useGoPostReceive))
-
- cmd.Dir = testRepoPath
-
- require.NoError(t, cmd.Run())
- require.Empty(t, stderr.String())
- require.Empty(t, stdout.String())
+ var stderr, stdout bytes.Buffer
+ stdin := bytes.NewBuffer([]byte(changes))
+ hookPath, err := filepath.Abs(fmt.Sprintf("../../ruby/git-hooks/%s", hookName))
+ require.NoError(t, err)
+ cmd := exec.Command(hookPath)
+ cmd.Stderr = &stderr
+ cmd.Stdout = &stdout
+ cmd.Stdin = stdin
+ cmd.Env = testhelper.EnvForHooks(
+ t,
+ tempGitlabShellDir,
+ socket,
+ token,
+ testRepo,
+ testhelper.GlHookValues{
+ GLID: glID,
+ GLUsername: glUsername,
+ GLRepo: glRepository,
+ GLProtocol: glProtocol,
+ GitObjectDir: c.GitObjectDir,
+ GitAlternateObjectDirs: c.GitAlternateObjectDirs,
+ },
+ testhelper.ProxyValues{
+ HTTPProxy: httpProxy,
+ HTTPSProxy: httpsProxy,
+ NoProxy: noProxy,
+ },
+ gitPushOptions...,
+ )
- output := string(testhelper.MustReadFile(t, customHookOutputPath))
- requireContainsOnce(t, output, "GL_USERNAME="+glUsername)
- requireContainsOnce(t, output, "GL_ID="+glID)
- requireContainsOnce(t, output, "GL_REPOSITORY="+glRepository)
- requireContainsOnce(t, output, "HTTP_PROXY="+httpProxy)
- requireContainsOnce(t, output, "http_proxy="+httpProxy)
- requireContainsOnce(t, output, "HTTPS_PROXY="+httpsProxy)
- requireContainsOnce(t, output, "https_proxy="+httpsProxy)
- requireContainsOnce(t, output, "no_proxy="+noProxy)
- requireContainsOnce(t, output, "NO_PROXY="+noProxy)
-
- if hookName == "pre-receive" {
- gitObjectDirMatches := gitObjectDirRegex.FindStringSubmatch(output)
- require.Len(t, gitObjectDirMatches, 2)
- require.Equal(t, gitObjectDir, gitObjectDirMatches[1])
-
- gitAlternateObjectDirMatches := gitAlternateObjectDirRegex.FindStringSubmatch(output)
- require.Len(t, gitAlternateObjectDirMatches, 2)
- require.Equal(t, strings.Join(gitAlternateObjectDirs, ":"), gitAlternateObjectDirMatches[1])
- } else {
- require.Contains(t, output, "GL_PROTOCOL="+glProtocol)
- }
- })
- }
+ cmd.Dir = testRepoPath
+
+ require.NoError(t, cmd.Run())
+ require.Empty(t, stderr.String())
+ require.Empty(t, stdout.String())
+
+ output := string(testhelper.MustReadFile(t, customHookOutputPath))
+ requireContainsOnce(t, output, "GL_USERNAME="+glUsername)
+ requireContainsOnce(t, output, "GL_ID="+glID)
+ requireContainsOnce(t, output, "GL_REPOSITORY="+glRepository)
+ requireContainsOnce(t, output, "HTTP_PROXY="+httpProxy)
+ requireContainsOnce(t, output, "http_proxy="+httpProxy)
+ requireContainsOnce(t, output, "HTTPS_PROXY="+httpsProxy)
+ requireContainsOnce(t, output, "https_proxy="+httpsProxy)
+ requireContainsOnce(t, output, "no_proxy="+noProxy)
+ requireContainsOnce(t, output, "NO_PROXY="+noProxy)
+
+ if hookName == "pre-receive" {
+ gitObjectDirMatches := gitObjectDirRegex.FindStringSubmatch(output)
+ require.Len(t, gitObjectDirMatches, 2)
+ require.Equal(t, gitObjectDir, gitObjectDirMatches[1])
+
+ gitAlternateObjectDirMatches := gitAlternateObjectDirRegex.FindStringSubmatch(output)
+ require.Len(t, gitAlternateObjectDirMatches, 2)
+ require.Equal(t, strings.Join(gitAlternateObjectDirs, ":"), gitAlternateObjectDirMatches[1])
+ } else {
+ require.Contains(t, output, "GL_PROTOCOL="+glProtocol)
+ }
+ })
}
}
diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go
index 9158a82eb..f75afd55a 100644
--- a/internal/git/receivepack.go
+++ b/internal/git/receivepack.go
@@ -47,7 +47,6 @@ func ReceivePackHookEnv(ctx context.Context, req ReceivePackRequest) ([]string,
fmt.Sprintf("GITALY_SOCKET=" + config.GitalyInternalSocketPath()),
fmt.Sprintf("GITALY_REPO=%s", repo),
fmt.Sprintf("GITALY_TOKEN=%s", config.Config.Auth.Token),
- fmt.Sprintf("%s=%s", featureflag.GoPostReceiveHookEnvVar, strconv.FormatBool(featureflag.IsEnabled(ctx, featureflag.GoPostReceiveHook))),
fmt.Sprintf("%s=%s", featureflag.ReferenceTransactionHookEnvVar, strconv.FormatBool(featureflag.IsEnabled(ctx, featureflag.ReferenceTransactionHook))),
}, gitlabshellEnv...)
diff --git a/internal/gitaly/service/hook/post_receive.go b/internal/gitaly/service/hook/post_receive.go
index 84fb27058..2fd3ecad4 100644
--- a/internal/gitaly/service/hook/post_receive.go
+++ b/internal/gitaly/service/hook/post_receive.go
@@ -3,21 +3,13 @@ package hook
import (
"errors"
"fmt"
- "io"
- "io/ioutil"
"os/exec"
- "gitlab.com/gitlab-org/gitaly/internal/git/hooks"
"gitlab.com/gitlab-org/gitaly/internal/helper"
- "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitaly/streamio"
)
-func isGoPostReceiveHookUsed(env []string) bool {
- return getEnvVar(featureflag.GoPostReceiveHookEnvVar, env) == "true"
-}
-
func postReceiveHookResponse(stream gitalypb.HookService_PostReceiveHookServer, code int32, stderr string) error {
if err := stream.Send(&gitalypb.PostReceiveHookResponse{
ExitStatus: &gitalypb.ExitStatus{Value: code},
@@ -39,10 +31,6 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
return helper.ErrInvalidArgument(err)
}
- if !isGoPostReceiveHookUsed(firstRequest.GetEnvironmentVariables()) {
- return postReceiveHookRuby(firstRequest, stream)
- }
-
stdin := streamio.NewReader(func() ([]byte, error) {
req, err := stream.Recv()
return req.GetStdin(), err
@@ -52,7 +40,7 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
env, err := hookRequestEnv(firstRequest)
if err != nil {
- return err
+ return helper.ErrInternal(err)
}
if err := s.manager.PostReceiveHook(
@@ -75,64 +63,6 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ
return postReceiveHookResponse(stream, 0, "")
}
-func postReceiveHookRuby(firstRequest *gitalypb.PostReceiveHookRequest, stream gitalypb.HookService_PostReceiveHookServer) error {
- hookEnv, err := hookRequestEnv(firstRequest)
- if err != nil {
- return helper.ErrInternal(err)
- }
-
- hookEnv = append(hookEnv, hooks.GitPushOptions(firstRequest.GetGitPushOptions())...)
-
- primary, err := isPrimary(hookEnv)
- if err != nil {
- return helper.ErrInternalf("could not check role: %w", err)
- }
-
- stdin := streamio.NewReader(func() ([]byte, error) {
- req, err := stream.Recv()
- return req.GetStdin(), err
- })
-
- var status int32
- if primary {
- stdout := streamio.NewWriter(func(p []byte) error { return stream.Send(&gitalypb.PostReceiveHookResponse{Stdout: p}) })
- stderr := streamio.NewWriter(func(p []byte) error { return stream.Send(&gitalypb.PostReceiveHookResponse{Stderr: p}) })
-
- repoPath, err := helper.GetRepoPath(firstRequest.GetRepository())
- if err != nil {
- return helper.ErrInternal(err)
- }
-
- c := exec.Command(gitlabShellHook("post-receive"))
- c.Dir = repoPath
-
- status, err = streamCommandResponse(
- stream.Context(),
- stdin,
- stdout, stderr,
- c,
- hookEnv,
- )
-
- if err != nil {
- return helper.ErrInternal(err)
- }
- } else {
- _, err := io.Copy(ioutil.Discard, stdin)
- if err != nil {
- return helper.ErrInternal(err)
- }
- }
-
- if err := stream.SendMsg(&gitalypb.PostReceiveHookResponse{
- ExitStatus: &gitalypb.ExitStatus{Value: status},
- }); err != nil {
- return helper.ErrInternal(err)
- }
-
- return nil
-}
-
func validatePostReceiveHookRequest(in *gitalypb.PostReceiveHookRequest) error {
if in.GetRepository() == nil {
return errors.New("repository is empty")
diff --git a/internal/gitaly/service/hook/post_receive_test.go b/internal/gitaly/service/hook/post_receive_test.go
index 52b108b40..bd09c2d00 100644
--- a/internal/gitaly/service/hook/post_receive_test.go
+++ b/internal/gitaly/service/hook/post_receive_test.go
@@ -2,9 +2,7 @@ package hook
import (
"bytes"
- "fmt"
"io"
- "os"
"path/filepath"
"testing"
@@ -13,7 +11,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/helper/text"
- "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
"gitlab.com/gitlab-org/gitaly/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -54,207 +51,110 @@ func transactionEnv(t *testing.T, primary bool) string {
return env
}
-func TestPostReceive(t *testing.T) {
- rubyDir := config.Config.Ruby.Dir
- defer func(rubyDir string) {
- config.Config.Ruby.Dir = rubyDir
- }(rubyDir)
+func TestHooksMissingStdin(t *testing.T) {
+ user, password, secretToken := "user", "password", "secret token"
+ tempDir, cleanup := testhelper.CreateTemporaryGitlabShellDir(t)
+ defer cleanup()
+ testhelper.WriteShellSecretFile(t, tempDir, secretToken)
- cwd, err := os.Getwd()
- require.NoError(t, err)
- config.Config.Ruby.Dir = filepath.Join(cwd, "testdata")
+ testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+ defer cleanupFn()
- serverSocketPath, stop := runHooksServer(t, config.Config)
- defer stop()
+ c := testhelper.GitlabTestServerOptions{
+ User: user,
+ Password: password,
+ SecretToken: secretToken,
+ GLID: "key_id",
+ GLRepository: "repository",
+ Changes: "changes",
+ PostReceiveCounterDecreased: true,
+ Protocol: "protocol",
+ RepoPath: testRepoPath,
+ }
- testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
- defer cleanupFn()
+ serverURL, cleanup := testhelper.NewGitlabTestServer(t, c)
+ defer cleanup()
- client, conn := newHooksClient(t, serverSocketPath)
- defer conn.Close()
+ gitlabConfig := config.Gitlab{
+ SecretFile: filepath.Join(tempDir, ".gitlab_shell_secret"),
+ URL: serverURL,
+ HTTPSettings: config.HTTPSettings{
+ User: user,
+ Password: password,
+ },
+ }
+
+ defer func(cfg config.Cfg) {
+ config.Config = cfg
+ }(config.Config)
+
+ config.Config.Gitlab = gitlabConfig
+
+ api, err := gitalyhook.NewGitlabAPI(gitlabConfig)
+ require.NoError(t, err)
testCases := []struct {
- desc string
- stdin io.Reader
- req gitalypb.PostReceiveHookRequest
- status int32
- stdout string
- stderr string
+ desc string
+ env string
+ fail bool
}{
{
- desc: "everything OK",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{"GL_ID=key_id", "GL_USERNAME=username", "GL_PROTOCOL=protocol", "GL_REPOSITORY=repository"},
- GitPushOptions: []string{"option0", "option1"}},
- status: 0,
- stdout: "OK",
- stderr: "",
- },
- {
- desc: "missing stdin",
- stdin: bytes.NewBuffer(nil),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{"GL_ID=key_id", "GL_USERNAME=username", "GL_PROTOCOL=protocol", "GL_REPOSITORY=repository"},
- GitPushOptions: []string{"option0"},
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing gl_id",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{"GL_ID=", "GL_USERNAME=username", "GL_PROTOCOL=protocol", "GL_REPOSITORY=repository"},
- GitPushOptions: []string{"option0"},
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing gl_username",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{"GL_ID=key-123", "GL_USERNAME=", "GL_PROTOCOL=protocol", "GL_REPOSITORY=repository"},
- GitPushOptions: []string{"option0"},
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing gl_protocol",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{"GL_ID=key-123", "GL_USERNAME=username", "GL_PROTOCOL=", "GL_REPOSITORY=repository"},
- GitPushOptions: []string{"option0"},
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing gl_repository value",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{"GL_ID=key-123", "GL_USERNAME=username", "GL_PROTOCOL=protocol", "GL_REPOSITORY="},
- GitPushOptions: []string{"option0"},
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing git push option",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{"GL_ID=key-123", "GL_USERNAME=username", "GL_PROTOCOL=protocol", "GL_REPOSITORY=repository"},
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "primary fails with missing stdin because hook gets executed",
- stdin: bytes.NewBuffer(nil),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=key_id",
- "GL_USERNAME=username",
- "GL_PROTOCOL=protocol",
- "GL_REPOSITORY=repository",
- transactionEnv(t, true),
- },
- GitPushOptions: []string{"option0"},
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
+ desc: "empty stdin fails if primary",
+ env: transactionEnv(t, true),
+ fail: true,
},
{
- desc: "secondary succeeds with missing stdin because hook does not get executed",
- stdin: bytes.NewBuffer(nil),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=key_id",
- "GL_USERNAME=username",
- "GL_PROTOCOL=protocol",
- "GL_REPOSITORY=repository",
- transactionEnv(t, false),
- },
- GitPushOptions: []string{"option0"},
- },
- status: 0,
- stdout: "",
- stderr: "",
- },
- {
- desc: "Go hook correctly honors the primary flag",
- stdin: bytes.NewBuffer(nil),
- req: gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=key_id",
- "GL_USERNAME=username",
- "GL_PROTOCOL=protocol",
- "GL_REPOSITORY=repository",
- transactionEnv(t, false),
- fmt.Sprintf("%s=true", featureflag.GoPostReceiveHookEnvVar),
- },
- GitPushOptions: []string{"option0"},
- },
- status: 0,
- stdout: "",
- stderr: "",
+ desc: "empty stdin success on secondary",
+ env: transactionEnv(t, false),
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
+ serverSocketPath, stop := runHooksServerWithAPI(t, api, config.Config)
+ defer stop()
+
+ client, conn := newHooksClient(t, serverSocketPath)
+ defer conn.Close()
+
ctx, cancel := testhelper.Context()
defer cancel()
stream, err := client.PostReceiveHook(ctx)
require.NoError(t, err)
- require.NoError(t, stream.Send(&tc.req))
+ require.NoError(t, stream.Send(&gitalypb.PostReceiveHookRequest{
+ Repository: testRepo,
+ EnvironmentVariables: []string{
+ "GL_ID=key_id",
+ "GL_USERNAME=username",
+ "GL_PROTOCOL=protocol",
+ "GL_REPOSITORY=repository", tc.env},
+ }))
go func() {
writer := streamio.NewWriter(func(p []byte) error {
return stream.Send(&gitalypb.PostReceiveHookRequest{Stdin: p})
})
- _, err := io.Copy(writer, tc.stdin)
+ _, err := io.Copy(writer, bytes.NewBuffer(nil))
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
}
- _, err = stdout.Write(resp.GetStdout())
- require.NoError(t, err)
- stderr.Write(resp.GetStderr())
status = resp.GetExitStatus().GetValue()
}
- 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")
+ if tc.fail {
+ require.NotEqual(t, int32(0), status, "exit code should be non-zero")
+ } else {
+ require.Equal(t, int32(0), status, "exit code unequal")
+ }
})
}
}
@@ -297,94 +197,91 @@ To create a merge request for okay, visit:
testhelper.WriteShellSecretFile(t, tempDir, secretToken)
for _, tc := range testCases {
- for _, useGoPostReceive := range []bool{true, false} {
- t.Run(fmt.Sprintf("%s:use_go_post_receive:%v", tc.desc, useGoPostReceive), func(t *testing.T) {
- c := testhelper.GitlabTestServerOptions{
- User: user,
- Password: password,
- SecretToken: secretToken,
- GLID: "key_id",
- GLRepository: "repository",
- Changes: "changes",
- PostReceiveCounterDecreased: true,
- PostReceiveMessages: tc.basicMessages,
- PostReceiveAlerts: tc.alertMessages,
- Protocol: "protocol",
- RepoPath: testRepoPath,
- }
+ t.Run(tc.desc, func(t *testing.T) {
+ c := testhelper.GitlabTestServerOptions{
+ User: user,
+ Password: password,
+ SecretToken: secretToken,
+ GLID: "key_id",
+ GLRepository: "repository",
+ Changes: "changes",
+ PostReceiveCounterDecreased: true,
+ PostReceiveMessages: tc.basicMessages,
+ PostReceiveAlerts: tc.alertMessages,
+ Protocol: "protocol",
+ RepoPath: testRepoPath,
+ }
- serverURL, cleanup := testhelper.NewGitlabTestServer(t, c)
- defer cleanup()
+ serverURL, cleanup := testhelper.NewGitlabTestServer(t, c)
+ defer cleanup()
- gitlabConfig := config.Gitlab{
- SecretFile: filepath.Join(tempDir, ".gitlab_shell_secret"),
- URL: serverURL,
- HTTPSettings: config.HTTPSettings{
- User: user,
- Password: password,
- },
- }
+ gitlabConfig := config.Gitlab{
+ SecretFile: filepath.Join(tempDir, ".gitlab_shell_secret"),
+ URL: serverURL,
+ HTTPSettings: config.HTTPSettings{
+ User: user,
+ Password: password,
+ },
+ }
- defer func(cfg config.Cfg) {
- config.Config = cfg
- }(config.Config)
+ defer func(cfg config.Cfg) {
+ config.Config = cfg
+ }(config.Config)
- config.Config.Gitlab = gitlabConfig
+ config.Config.Gitlab = gitlabConfig
- api, err := gitalyhook.NewGitlabAPI(gitlabConfig)
- require.NoError(t, err)
+ api, err := gitalyhook.NewGitlabAPI(gitlabConfig)
+ require.NoError(t, err)
- serverSocketPath, stop := runHooksServerWithAPI(t, api, config.Config)
- defer stop()
+ serverSocketPath, stop := runHooksServerWithAPI(t, api, config.Config)
+ defer stop()
- client, conn := newHooksClient(t, serverSocketPath)
- defer conn.Close()
+ client, conn := newHooksClient(t, serverSocketPath)
+ defer conn.Close()
- ctx, cancel := testhelper.Context()
- defer cancel()
+ ctx, cancel := testhelper.Context()
+ defer cancel()
- stream, err := client.PostReceiveHook(ctx)
+ stream, err := client.PostReceiveHook(ctx)
+ require.NoError(t, err)
+
+ envVars := []string{
+ "GL_ID=key_id",
+ "GL_USERNAME=username",
+ "GL_PROTOCOL=protocol",
+ "GL_REPOSITORY=repository",
+ }
+
+ require.NoError(t, stream.Send(&gitalypb.PostReceiveHookRequest{
+ Repository: testRepo,
+ EnvironmentVariables: envVars}))
+
+ go func() {
+ writer := streamio.NewWriter(func(p []byte) error {
+ return stream.Send(&gitalypb.PostReceiveHookRequest{Stdin: p})
+ })
+ _, err := writer.Write([]byte("changes"))
require.NoError(t, err)
+ require.NoError(t, stream.CloseSend(), "close send")
+ }()
- envVars := []string{
- "GL_ID=key_id",
- "GL_USERNAME=username",
- "GL_PROTOCOL=protocol",
- "GL_REPOSITORY=repository",
- fmt.Sprintf("%s=%v", featureflag.GoPostReceiveHookEnvVar, useGoPostReceive),
+ var status int32
+ var stdout, stderr bytes.Buffer
+ for {
+ resp, err := stream.Recv()
+ if err == io.EOF {
+ break
}
- require.NoError(t, stream.Send(&gitalypb.PostReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: envVars}))
-
- go func() {
- writer := streamio.NewWriter(func(p []byte) error {
- return stream.Send(&gitalypb.PostReceiveHookRequest{Stdin: p})
- })
- _, err := writer.Write([]byte("changes"))
- 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
- }
-
- _, err = stdout.Write(resp.GetStdout())
- require.NoError(t, err)
- stderr.Write(resp.GetStderr())
- status = resp.GetExitStatus().GetValue()
- }
+ _, err = stdout.Write(resp.GetStdout())
+ require.NoError(t, err)
+ stderr.Write(resp.GetStderr())
+ status = resp.GetExitStatus().GetValue()
+ }
- assert.Equal(t, int32(0), status)
- assert.Equal(t, "", text.ChompBytes(stderr.Bytes()), "hook stderr")
- assert.Equal(t, tc.expectedStdout, text.ChompBytes(stdout.Bytes()), "hook stdout")
- })
- }
+ assert.Equal(t, int32(0), status)
+ assert.Equal(t, "", text.ChompBytes(stderr.Bytes()), "hook stderr")
+ assert.Equal(t, tc.expectedStdout, text.ChompBytes(stdout.Bytes()), "hook stdout")
+ })
}
}
diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go
index c9a05e778..112ea7d53 100644
--- a/internal/gitaly/service/hook/pre_receive.go
+++ b/internal/gitaly/service/hook/pre_receive.go
@@ -4,14 +4,10 @@ import (
"errors"
"fmt"
"os/exec"
- "path/filepath"
- "strings"
"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/helper"
- "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitaly/streamio"
)
@@ -45,24 +41,6 @@ func preReceiveEnv(req prePostRequest) ([]string, error) {
return env, nil
}
-func gitlabShellHook(hookName string) string {
- return filepath.Join(config.Config.Ruby.Dir, "gitlab-shell", "hooks", hookName)
-}
-
-func isPrimary(env []string) (bool, error) {
- tx, err := metadata.TransactionFromEnv(env)
- if err != nil {
- if errors.Is(err, metadata.ErrTransactionNotFound) {
- // If there is no transaction, then we only ever write
- // to the primary. Thus, we return true.
- return true, nil
- }
- return false, err
- }
-
- return tx.Primary, nil
-}
-
func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer) error {
firstRequest, err := stream.Recv()
if err != nil {
@@ -124,14 +102,3 @@ func preReceiveHookResponse(stream gitalypb.HookService_PreReceiveHookServer, co
return nil
}
-
-func getEnvVar(key string, vars []string) string {
- for _, varPair := range vars {
- kv := strings.SplitN(varPair, "=", 2)
- if kv[0] == key {
- return kv[1]
- }
- }
-
- return ""
-}
diff --git a/internal/gitaly/service/hook/stream_command.go b/internal/gitaly/service/hook/stream_command.go
deleted file mode 100644
index 2893dd5a8..000000000
--- a/internal/gitaly/service/hook/stream_command.go
+++ /dev/null
@@ -1,34 +0,0 @@
-package hook
-
-import (
- "context"
- "io"
- "os/exec"
-
- "gitlab.com/gitlab-org/gitaly/internal/command"
- "gitlab.com/gitlab-org/gitaly/internal/helper"
-)
-
-func streamCommandResponse(
- ctx context.Context,
- stdin io.Reader,
- stdout, stderr io.Writer,
- c *exec.Cmd,
- env []string,
-) (int32, error) {
- cmd, err := command.New(ctx, c, stdin, stdout, stderr, env...)
- if err != nil {
- return 1, helper.ErrInternal(err)
- }
-
- err = cmd.Wait()
- if err == nil {
- return 0, nil
- }
-
- if code, ok := command.ExitStatus(err); ok {
- return int32(code), nil
- }
-
- return 1, err
-}
diff --git a/internal/gitaly/service/hook/testdata/gitlab-shell/hooks/post-receive b/internal/gitaly/service/hook/testdata/gitlab-shell/hooks/post-receive
deleted file mode 100755
index 09af4c677..000000000
--- a/internal/gitaly/service/hook/testdata/gitlab-shell/hooks/post-receive
+++ /dev/null
@@ -1,10 +0,0 @@
-#!/usr/bin/env ruby
-
-# Tests inputs to post-receive
-
-abort("FAIL") if $stdin.read.empty?
-abort("FAIL") if %w[GL_ID GL_REPOSITORY GL_PROTOCOL GL_USERNAME].any? { |k| ENV[k].nil? || ENV[k].empty? }
-# git push options are not required. This is only for the sake of testing the values get through
-abort("FAIL") if %w[GIT_PUSH_OPTION_COUNT GIT_PUSH_OPTION_0].any? { |k| ENV[k].nil? || ENV[k].empty? }
-
-puts "OK"
diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go
index 2149632e0..1741968e6 100644
--- a/internal/gitaly/service/operations/rebase_test.go
+++ b/internal/gitaly/service/operations/rebase_test.go
@@ -3,7 +3,6 @@ package operations
//lint:file-ignore SA1019 due to planned removal in issue https://gitlab.com/gitlab-org/gitaly/issues/1628
import (
- "context"
"fmt"
"strings"
"testing"
@@ -13,7 +12,6 @@ import (
"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log"
"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
- "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
"google.golang.org/grpc/codes"
@@ -25,9 +23,6 @@ var (
)
func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) {
- featureSets, err := testhelper.NewFeatureSets(nil, featureflag.GoPostReceiveHook)
- require.NoError(t, err)
-
var ruby rubyserver.Server
pushOptions := []string{"ci.skip", "test=value"}
@@ -40,18 +35,9 @@ func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) {
serverSocketPath, stop := runOperationServiceServerWithRubyServer(t, &ruby)
defer stop()
- for _, featureSet := range featureSets {
- t.Run(featureSet.String(), func(t *testing.T) {
- ctx, cancel := testhelper.Context()
- defer cancel()
-
- ctx = featureSet.Disable(ctx)
- testSuccessfulUserRebaseConfirmableRequest(t, ctx, serverSocketPath, pushOptions)
- })
- }
-}
+ ctxOuter, cancel := testhelper.Context()
+ defer cancel()
-func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctxOuter context.Context, serverSocketPath string, pushOptions []string) {
client, conn := newOperationClient(t, serverSocketPath)
defer conn.Close()
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index cf1b4584c..518105134 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -61,21 +61,6 @@ func TestSuccessfulUserDeleteTagRequest(t *testing.T) {
}
func TestSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T) {
- featureSets, err := testhelper.NewFeatureSets(nil, featureflag.GoPostReceiveHook)
- require.NoError(t, err)
-
- for _, featureSet := range featureSets {
- t.Run("disabled "+featureSet.String(), func(t *testing.T) {
- ctx, cancel := testhelper.Context()
- defer cancel()
-
- ctx = featureSet.Disable(ctx)
- testSuccessfulGitHooksForUserDeleteTagRequest(t, ctx)
- })
- }
-}
-
-func testSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T, ctx context.Context) {
serverSocketPath, stop := runOperationServiceServer(t)
defer stop()
@@ -88,6 +73,9 @@ func testSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T, ctx context.Con
tagNameInput := "to-be-déleted-soon-tag"
defer exec.Command(command.GitPath(), "-C", testRepoPath, "tag", "-d", tagNameInput).Run()
+ ctx, cancel := testhelper.Context()
+ defer cancel()
+
request := &gitalypb.UserDeleteTagRequest{
Repository: testRepo,
TagName: []byte(tagNameInput),
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index 183a73444..40bbc6b8c 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -355,26 +355,9 @@ func drainPostReceivePackResponse(stream gitalypb.SmartHTTPService_PostReceivePa
}
func TestPostReceivePackToHooks(t *testing.T) {
- defer func(cfg config.Cfg) {
- config.Config = cfg
- }(config.Config)
-
- features, err := testhelper.NewFeatureSets([]featureflag.FeatureFlag{featureflag.GoPostReceiveHook})
- require.NoError(t, err)
-
- for _, feature := range features {
- t.Run("disabled "+feature.String(), func(t *testing.T) {
- ctx, cancel := testhelper.Context()
- defer cancel()
-
- ctx = feature.Disable(ctx)
-
- testPostReceivePackToHooks(t, ctx)
- })
- }
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
-func testPostReceivePackToHooks(t *testing.T, ctx context.Context) {
secretToken := "secret token"
glRepository := "some_repo"
glID := "key-123"
@@ -390,6 +373,9 @@ func testPostReceivePackToHooks(t *testing.T, ctx context.Context) {
tempGitlabShellDir, cleanup := testhelper.CreateTemporaryGitlabShellDir(t)
defer cleanup()
+ defer func(cfg config.Cfg) {
+ config.Config = cfg
+ }(config.Config)
config.Config.GitlabShell.Dir = tempGitlabShellDir
defer func(override string) {
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index 6f48f055d..812556d55 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -13,8 +13,6 @@ var (
GoFetchSourceBranch = FeatureFlag{Name: "go_fetch_source_branch", OnByDefault: false}
// DistributedReads allows praefect to redirect accessor operations to up-to-date secondaries
DistributedReads = FeatureFlag{Name: "distributed_reads", OnByDefault: true}
- // GoPostReceiveHook will bypass the ruby post-receive hook and use the go implementation
- GoPostReceiveHook = FeatureFlag{Name: "go_postreceive_hook", OnByDefault: true}
// ReferenceTransactions will handle Git reference updates via the transaction service for strong consistency
ReferenceTransactions = FeatureFlag{Name: "reference_transactions", OnByDefault: true}
// ReferenceTranasctiionsPrimaryWins will change transaction registration such that
@@ -40,7 +38,6 @@ var (
var All = []FeatureFlag{
GoFetchSourceBranch,
DistributedReads,
- GoPostReceiveHook,
ReferenceTransactions,
ReferenceTransactionsPrimaryWins,
ReferenceTransactionHook,
@@ -51,6 +48,5 @@ var All = []FeatureFlag{
}
const (
- GoPostReceiveHookEnvVar = "GITALY_GO_POSTRECEIVE"
ReferenceTransactionHookEnvVar = "GITALY_REFERENCE_TRANSACTION_HOOK"
)
diff --git a/ruby/lib/gitlab/git/hook.rb b/ruby/lib/gitlab/git/hook.rb
index 4a6ee936f..070c93ab3 100644
--- a/ruby/lib/gitlab/git/hook.rb
+++ b/ruby/lib/gitlab/git/hook.rb
@@ -133,7 +133,6 @@ module Gitlab
'GIT_DIR' => repo_path,
'GITALY_REPO' => repository.gitaly_repository.to_json,
'GITALY_SOCKET' => Gitlab.config.gitaly.internal_socket,
- 'GITALY_GO_POSTRECEIVE' => repository.feature_enabled?('go-postreceive-hook').to_s,
'GITALY_REFERENCE_TRANSACTION_HOOK' => repository.feature_enabled?('reference-transaction-hook').to_s
}
end