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:
Diffstat
-rw-r--r--changelogs/unreleased/zj-prereceive-feature-flag.yml5
-rw-r--r--cmd/gitaly-hooks/hooks.go8
-rw-r--r--cmd/gitaly-hooks/hooks_test.go6
-rw-r--r--internal/git/receivepack.go1
-rw-r--r--internal/gitaly/service/hook/post_receive_test.go3
-rw-r--r--internal/gitaly/service/hook/pre_receive.go86
-rw-r--r--internal/gitaly/service/hook/pre_receive_test.go152
-rwxr-xr-xinternal/gitaly/service/hook/testdata/gitlab-shell/hooks/pre-receive9
-rw-r--r--internal/metadata/featureflag/feature_flags.go4
-rw-r--r--internal/testhelper/testserver.go2
-rw-r--r--ruby/lib/gitlab/git/hook.rb1
11 files changed, 8 insertions, 269 deletions
diff --git a/changelogs/unreleased/zj-prereceive-feature-flag.yml b/changelogs/unreleased/zj-prereceive-feature-flag.yml
new file mode 100644
index 000000000..6132705c1
--- /dev/null
+++ b/changelogs/unreleased/zj-prereceive-feature-flag.yml
@@ -0,0 +1,5 @@
+---
+title: 'hooks: Remove prereceive hook Ruby implementation'
+merge_request: 2519
+author:
+type: changed
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go
index ba7414107..61ea7979b 100644
--- a/cmd/gitaly-hooks/hooks.go
+++ b/cmd/gitaly-hooks/hooks.go
@@ -125,9 +125,7 @@ func main() {
logger.Fatalf("error when getting preReceiveHookStream client for %q: %v", subCmd, err)
}
- environment := glValues()
- environment = append(environment, gitObjectDirs()...)
-
+ environment := append(glValues(), gitObjectDirs()...)
for _, key := range []string{metadata.PraefectEnvKey, metadata.TransactionEnvKey} {
if value, ok := os.LookupEnv(key); ok {
env := fmt.Sprintf("%s=%s", key, value)
@@ -135,10 +133,6 @@ 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 ceed2fe02..40d282748 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -227,7 +227,7 @@ func testHooksPrePostReceive(t *testing.T) {
hookNames := []string{"pre-receive", "post-receive"}
- featureSets, err := testhelper.NewFeatureSets([]featureflag.FeatureFlag{featureflag.GoPreReceiveHook, featureflag.GoPostReceiveHook})
+ featureSets, err := testhelper.NewFeatureSets([]featureflag.FeatureFlag{featureflag.GoPostReceiveHook})
require.NoError(t, err)
for _, hookName := range hookNames {
@@ -277,10 +277,6 @@ func testHooksPrePostReceive(t *testing.T) {
gitPushOptions...,
)
- if !featureSet.IsDisabled(featureflag.GoPreReceiveHook) {
- cmd.Env = append(cmd.Env, fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar))
- }
-
cmd.Dir = testRepoPath
require.NoError(t, cmd.Run())
diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go
index 5d9384c2e..9158a82eb 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.GoPreReceiveHookEnvVar, strconv.FormatBool(featureflag.IsEnabled(ctx, featureflag.GoPreReceiveHook))),
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_test.go b/internal/gitaly/service/hook/post_receive_test.go
index cf86e9a22..4d9f953d8 100644
--- a/internal/gitaly/service/hook/post_receive_test.go
+++ b/internal/gitaly/service/hook/post_receive_test.go
@@ -351,9 +351,6 @@ To create a merge request for okay, visit:
"GL_USERNAME=username",
"GL_PROTOCOL=protocol",
"GL_REPOSITORY=repository"}
- if useGoPreReceive {
- envVars = append(envVars, "GITALY_GO_PRERECEIVE=true")
- }
require.NoError(t, stream.Send(&gitalypb.PostReceiveHookRequest{
Repository: testRepo,
diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go
index 833d9af32..c9a05e778 100644
--- a/internal/gitaly/service/hook/pre_receive.go
+++ b/internal/gitaly/service/hook/pre_receive.go
@@ -1,11 +1,8 @@
package hook
import (
- "crypto/sha1"
"errors"
"fmt"
- "io"
- "io/ioutil"
"os/exec"
"path/filepath"
"strings"
@@ -14,7 +11,6 @@ import (
"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/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitaly/streamio"
@@ -79,10 +75,6 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
reqEnvVars := firstRequest.GetEnvironmentVariables()
repository := firstRequest.GetRepository()
- if !useGoPreReceiveHook(reqEnvVars) {
- return s.preReceiveHookRuby(firstRequest, stream)
- }
-
stdin := streamio.NewReader(func() ([]byte, error) {
req, err := stream.Recv()
return req.GetStdin(), err
@@ -122,10 +114,6 @@ func validatePreReceiveHookRequest(in *gitalypb.PreReceiveHookRequest) error {
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},
@@ -137,80 +125,6 @@ func preReceiveHookResponse(stream gitalypb.HookService_PreReceiveHookServer, co
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 {
- return nil, err
- }
-
- stdin := req.GetStdin()
- if _, err := referenceUpdatesHasher.Write(stdin); err != nil {
- return stdin, err
- }
-
- return stdin, nil
- })
-
- env, err := preReceiveEnv(firstRequest)
- if err != nil {
- return helper.ErrInternal(err)
- }
-
- primary, err := isPrimary(env)
- if err != nil {
- return helper.ErrInternalf("could not check role: %w", err)
- }
-
- var status int32
-
- // Only the primary should execute hooks.
- if primary {
- 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}) })
-
- repoPath, err := helper.GetRepoPath(firstRequest.GetRepository())
- if err != nil {
- return helper.ErrInternal(err)
- }
-
- c := exec.Command(gitlabShellHook("pre-receive"))
- c.Dir = repoPath
-
- status, err = streamCommandResponse(
- stream.Context(),
- stdin,
- stdout, stderr,
- c,
- env,
- )
- if err != nil {
- return helper.ErrInternal(err)
- }
- } else {
- // We need to read all of stdin on secondaries so that we
- // arrive at the same hash as the primary.
- _, err := io.Copy(ioutil.Discard, stdin)
- if err != nil {
- return helper.ErrInternal(err)
- }
- }
-
- if err := s.manager.VoteOnTransaction(stream.Context(), referenceUpdatesHasher.Sum(nil), env); err != nil {
- return helper.ErrInternalf("error voting on transaction: %w", err)
- }
-
- if err := stream.SendMsg(&gitalypb.PreReceiveHookResponse{
- ExitStatus: &gitalypb.ExitStatus{Value: status},
- }); err != nil {
- return helper.ErrInternal(err)
- }
-
- return nil
-}
-
func getEnvVar(key string, vars []string) string {
for _, varPair := range vars {
kv := strings.SplitN(varPair, "=", 2)
diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go
index 8f1c1c416..e29e7bc94 100644
--- a/internal/gitaly/service/hook/pre_receive_test.go
+++ b/internal/gitaly/service/hook/pre_receive_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/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"
@@ -84,148 +83,6 @@ func receivePreReceive(t *testing.T, stream gitalypb.HookService_PreReceiveHookC
return stdout, stderr, status
}
-func TestPreReceive(t *testing.T) {
- rubyDir := config.Config.Ruby.Dir
- defer func(rubyDir string) {
- config.Config.Ruby.Dir = rubyDir
- }(rubyDir)
-
- cwd, err := os.Getwd()
- require.NoError(t, err)
- config.Config.Ruby.Dir = filepath.Join(cwd, "testdata")
-
- serverSocketPath, stop := runHooksServer(t, config.Config.Hooks)
- defer stop()
-
- testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
- defer cleanupFn()
-
- client, conn := newHooksClient(t, serverSocketPath)
- defer conn.Close()
-
- ctx, cancel := testhelper.Context()
- defer cancel()
-
- testCases := []struct {
- desc string
- stdin io.Reader
- req gitalypb.PreReceiveHookRequest
- status int32
- stdout, stderr string
- }{
- {
- desc: "everything OK",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PreReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=key-123",
- "GL_PROTOCOL=protocol",
- "GL_USERNAME=username",
- "GL_REPOSITORY=repository",
- },
- },
- status: 0,
- stdout: "OK",
- stderr: "",
- },
- {
- desc: "missing stdin",
- stdin: bytes.NewBuffer(nil),
- req: gitalypb.PreReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=key-123",
- "GL_PROTOCOL=protocol",
- "GL_USERNAME=username",
- "GL_REPOSITORY=repository",
- },
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing gl_protocol",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PreReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=key-123",
- "GL_USERNAME=username",
- "GL_PROTOCOL=",
- "GL_REPOSITORY=repository",
- },
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing gl_id",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PreReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=",
- "GL_PROTOCOL=protocol",
- "GL_USERNAME=username",
- "GL_REPOSITORY=repository",
- },
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing gl_username",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PreReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=key-123",
- "GL_PROTOCOL=protocol",
- "GL_USERNAME=",
- "GL_REPOSITORY=repository",
- },
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- {
- desc: "missing gl_repository",
- stdin: bytes.NewBufferString("a\nb\nc\nd\ne\nf\ng"),
- req: gitalypb.PreReceiveHookRequest{
- Repository: testRepo,
- EnvironmentVariables: []string{
- "GL_ID=key-123",
- "GL_PROTOCOL=protocol",
- "GL_USERNAME=username",
- "GL_REPOSITORY=",
- },
- },
- status: 1,
- stdout: "",
- stderr: "FAIL",
- },
- }
-
- for _, tc := range testCases {
- t.Run(tc.desc, func(t *testing.T) {
- stream, err := client.PreReceiveHook(ctx)
- require.NoError(t, err)
- require.NoError(t, stream.Send(&tc.req))
-
- stdout, stderr, status := receivePreReceive(t, stream, tc.stdin)
-
- assert.Equal(t, tc.status, status)
- 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"
@@ -299,7 +156,6 @@ func TestPreReceiveHook_GitlabAPIAccess(t *testing.T) {
"GL_PROTOCOL=" + protocol,
"GL_USERNAME=username",
"GL_REPOSITORY=" + glRepository,
- fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar),
},
}
@@ -404,7 +260,6 @@ func TestPreReceive_APIErrors(t *testing.T) {
"GL_PROTOCOL=web",
"GL_USERNAME=username",
"GL_REPOSITORY=repository",
- fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar),
},
}))
require.NoError(t, stream.CloseSend())
@@ -465,7 +320,6 @@ exit %d
"GL_PROTOCOL=web",
"GL_USERNAME=username",
"GL_REPOSITORY=repository",
- fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar),
},
}))
@@ -501,7 +355,6 @@ func TestPreReceiveHook_Primary(t *testing.T) {
testCases := []struct {
desc string
primary bool
- useRubyHook bool
allowedHandler http.HandlerFunc
preReceiveHandler http.HandlerFunc
stdin []byte
@@ -580,7 +433,6 @@ func TestPreReceiveHook_Primary(t *testing.T) {
{
desc: "primary Ruby hook triggers transaction",
primary: true,
- useRubyHook: true,
stdin: []byte("foobar"),
allowedHandler: allowedHandler(true),
preReceiveHandler: preReceiveHandler(true),
@@ -591,7 +443,6 @@ func TestPreReceiveHook_Primary(t *testing.T) {
{
desc: "secondary Ruby hook triggers transaction",
primary: false,
- useRubyHook: true,
stdin: []byte("foobar"),
allowedHandler: allowedHandler(true),
preReceiveHandler: preReceiveHandler(true),
@@ -680,9 +531,6 @@ func TestPreReceiveHook_Primary(t *testing.T) {
transactionEnv,
transactionServerEnv,
}
- if !tc.useRubyHook {
- environment = append(environment, fmt.Sprintf("%s=true", featureflag.GoPreReceiveHookEnvVar))
- }
stream, err := client.PreReceiveHook(ctx)
require.NoError(t, err)
diff --git a/internal/gitaly/service/hook/testdata/gitlab-shell/hooks/pre-receive b/internal/gitaly/service/hook/testdata/gitlab-shell/hooks/pre-receive
deleted file mode 100755
index 12574b7b7..000000000
--- a/internal/gitaly/service/hook/testdata/gitlab-shell/hooks/pre-receive
+++ /dev/null
@@ -1,9 +0,0 @@
-#!/usr/bin/env ruby
-
-# Tests inputs to pre-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? }
-
-puts "OK"
-
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index d87009a0e..103df99b2 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}
- // GoPreReceiveHook will bypass the ruby pre-receive hook and use the go implementation
- GoPreReceiveHook = FeatureFlag{Name: "go_prereceive_hook", 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
@@ -39,7 +37,6 @@ var (
var All = []FeatureFlag{
GoFetchSourceBranch,
DistributedReads,
- GoPreReceiveHook,
GoPostReceiveHook,
ReferenceTransactions,
ReferenceTransactionsOperationService,
@@ -50,7 +47,6 @@ var All = []FeatureFlag{
}
const (
- GoPreReceiveHookEnvVar = "GITALY_GO_PRERECEIVE"
GoPostReceiveHookEnvVar = "GITALY_GO_POSTRECEIVE"
ReferenceTransactionHookEnvVar = "GITALY_REFERENCE_TRANSACTION_HOOK"
)
diff --git a/internal/testhelper/testserver.go b/internal/testhelper/testserver.go
index df7b2370f..fbf50a664 100644
--- a/internal/testhelper/testserver.go
+++ b/internal/testhelper/testserver.go
@@ -513,7 +513,7 @@ func handleAllowed(options GitlabTestServerOptions) func(w http.ResponseWriter,
return
}
w.WriteHeader(http.StatusUnauthorized)
- w.Write([]byte(`{"message":"401 Unauthorized"}`))
+ w.Write([]byte(`{"message":"401 Unauthorized\n"}`))
}
}
diff --git a/ruby/lib/gitlab/git/hook.rb b/ruby/lib/gitlab/git/hook.rb
index e16895722..c391bbf78 100644
--- a/ruby/lib/gitlab/git/hook.rb
+++ b/ruby/lib/gitlab/git/hook.rb
@@ -120,7 +120,6 @@ module Gitlab
'GIT_DIR' => repo_path,
'GITALY_REPO' => repository.gitaly_repository.to_json,
'GITALY_SOCKET' => Gitlab.config.gitaly.internal_socket,
- 'GITALY_GO_PRERECEIVE' => repository.feature_enabled?('go-prereceive-hook', on_by_default: true).to_s,
'GITALY_GO_POSTRECEIVE' => repository.feature_enabled?('go-postreceive-hook').to_s
}
end
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-16 00:39:27 +0300