diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-02-11 14:50:55 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-02-12 13:38:08 +0300 |
commit | f759f16177f4d54419cade648336c9c7beda325d (patch) | |
tree | 4eefa5af12ba34289d641af341898c94944c983e | |
parent | 868d2caf5b79d8f0e3892e501a9560b133b07a4d (diff) |
gitaly-hooks: Honor requested hooks
The HooksPayload has grown a new bitmap of requested hooks: only if the
bit corresponding to the about-to-be-executed hook is set shall we
actually execute that logic. But gitaly-hooks doesn't yet pay attention
to this flag, which this commit now fixes.
Note that we still do not inject the requested hooks anywhere. This is
done on purpose to demonstrate that the fallback of simply treating any
hook as requested works correctly to not break zero-downtime upgrades in
case the new gitaly-hooks binary is installed already while the old
server doesn't yet know about requested hooks.
-rw-r--r-- | cmd/gitaly-hooks/hooks.go | 24 | ||||
-rw-r--r-- | internal/git/hooks_payload.go | 6 | ||||
-rw-r--r-- | internal/git/hooks_payload_test.go | 71 |
3 files changed, 95 insertions, 6 deletions
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go index d94ecba05..70ea5d83e 100644 --- a/cmd/gitaly-hooks/hooks.go +++ b/cmd/gitaly-hooks/hooks.go @@ -25,25 +25,31 @@ import ( ) type hookCommand struct { - exec func(context.Context, git.HooksPayload, gitalypb.HookServiceClient, []string) (int, error) + exec func(context.Context, git.HooksPayload, gitalypb.HookServiceClient, []string) (int, error) + hookType git.Hook } var ( hooksBySubcommand = map[string]hookCommand{ "update": hookCommand{ - exec: updateHook, + exec: updateHook, + hookType: git.UpdateHook, }, "pre-receive": hookCommand{ - exec: preReceiveHook, + exec: preReceiveHook, + hookType: git.PreReceiveHook, }, "post-receive": hookCommand{ - exec: postReceiveHook, + exec: postReceiveHook, + hookType: git.PostReceiveHook, }, "reference-transaction": hookCommand{ - exec: referenceTransactionHook, + exec: referenceTransactionHook, + hookType: git.ReferenceTransactionHook, }, "git": hookCommand{ - exec: packObjectsHook, + exec: packObjectsHook, + hookType: git.PackObjectsHook, }, } @@ -102,6 +108,12 @@ func main() { logger.Fatalf("subcommand name invalid: %q", subCmd) } + // If the hook wasn't requested, then we simply skip executing any + // logic. + if !payload.IsHookRequested(hookCommand.hookType) { + os.Exit(0) + } + conn, err := dialGitaly(payload) if err != nil { logger.Fatalf("error when connecting to gitaly: %v", err) diff --git a/internal/git/hooks_payload.go b/internal/git/hooks_payload.go index 1d41631c1..7fd9d9ade 100644 --- a/internal/git/hooks_payload.go +++ b/internal/git/hooks_payload.go @@ -206,3 +206,9 @@ func (p HooksPayload) Env() (string, error) { return fmt.Sprintf("%s=%s", EnvHooksPayload, encoded), nil } + +// IsHookRequested returns whether the HooksPayload is requesting execution of +// the given git hook. +func (p HooksPayload) IsHookRequested(hook Hook) bool { + return p.RequestedHooks&hook != 0 +} diff --git a/internal/git/hooks_payload_test.go b/internal/git/hooks_payload_test.go index cae96caf1..8df82cebf 100644 --- a/internal/git/hooks_payload_test.go +++ b/internal/git/hooks_payload_test.go @@ -145,3 +145,74 @@ func TestHooksPayload(t *testing.T) { }, payload) }) } + +func TestHooksPayload_IsHookRequested(t *testing.T) { + for _, tc := range []struct { + desc string + configured Hook + request Hook + expected bool + }{ + { + desc: "exact match", + configured: PreReceiveHook, + request: PreReceiveHook, + expected: true, + }, + { + desc: "hook matches a set", + configured: PreReceiveHook | PostReceiveHook, + request: PreReceiveHook, + expected: true, + }, + { + desc: "no match", + configured: PreReceiveHook, + request: PostReceiveHook, + expected: false, + }, + { + desc: "no match with a set", + configured: PreReceiveHook | UpdateHook, + request: PostReceiveHook, + expected: false, + }, + { + desc: "no match with nothing set", + configured: 0, + request: PostReceiveHook, + expected: false, + }, + { + desc: "pre-receive hook with AllHooks", + configured: AllHooks, + request: PreReceiveHook, + expected: true, + }, + { + desc: "post-receive hook with AllHooks", + configured: AllHooks, + request: PostReceiveHook, + expected: true, + }, + { + desc: "update hook with AllHooks", + configured: AllHooks, + request: UpdateHook, + expected: true, + }, + { + desc: "reference-transaction hook with AllHooks", + configured: AllHooks, + request: ReferenceTransactionHook, + expected: true, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + actual := HooksPayload{ + RequestedHooks: tc.configured, + }.IsHookRequested(tc.request) + require.Equal(t, tc.expected, actual) + }) + } +} |