diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-04-27 14:43:10 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-04-27 14:43:10 +0300 |
commit | 97f5de04afa6473c01b9dcd924b9d1d4c0a2f107 (patch) | |
tree | 24842d9f187e4a00f5bc8b71194eba302ab343d5 | |
parent | 67520d7b374eea77f7412600271a90193bdd2e8a (diff) |
gitaly-hooks: Fix deferred calls never running
While we do defer multiple calls in gitaly-hooks' main function, these
aren't ever: the function never returns, but executes `os.Exit()`. While
it's not much of a problem right now because ultimately we do not care
for any of the deferred function calls, it invites for future errors in
case somebody misses the fact that deferred calls never run.
Fix the issue by pulling out the logic into a separate `run()` function
which has the usual return semantics.
-rw-r--r-- | cmd/gitaly-hooks/hooks.go | 36 |
1 files changed, 23 insertions, 13 deletions
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go index 980f0cf92..a65996e24 100644 --- a/cmd/gitaly-hooks/hooks.go +++ b/cmd/gitaly-hooks/hooks.go @@ -62,19 +62,28 @@ var ( func main() { logger = gitalylog.NewHookLogger() - if len(os.Args) < 2 { - logger.Fatalf("requires hook name. args: %v", os.Args) + returnCode, err := run(os.Args) + if err != nil { + logger.Fatalf("%s", err) + } + + os.Exit(returnCode) +} + +func run(args []string) (int, error) { + if len(args) < 2 { + return 0, fmt.Errorf("requires hook name. args: %v", args) } - subCmd := os.Args[1] + subCmd := args[1] if subCmd == "check" { logrus.SetLevel(logrus.ErrorLevel) - if len(os.Args) != 3 { + if len(args) != 3 { log.Fatal(errors.New("no configuration file path provided invoke with: gitaly-hooks check <config_path>")) } - configPath := os.Args[2] + configPath := args[2] fmt.Print("Checking GitLab API access: ") info, err := check(configPath) @@ -89,7 +98,8 @@ func main() { fmt.Printf("GitLab Api version: %s\n", info.APIVersion) fmt.Printf("Redis reachable for GitLab: %t\n", info.RedisReachable) fmt.Println("OK") - os.Exit(0) + + return 0, nil } ctx, cancel := context.WithCancel(context.Background()) @@ -103,33 +113,33 @@ func main() { payload, err := git.HooksPayloadFromEnv(os.Environ()) if err != nil { - logger.Fatalf("error when getting hooks payload: %v", err) + return 0, fmt.Errorf("error when getting hooks payload: %v", err) } hookCommand, ok := hooksBySubcommand[subCmd] if !ok { - logger.Fatalf("subcommand name invalid: %q", subCmd) + return 0, fmt.Errorf("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) + return 0, nil } conn, err := dialGitaly(payload) if err != nil { - logger.Fatalf("error when connecting to gitaly: %v", err) + return 0, fmt.Errorf("error when connecting to gitaly: %v", err) } hookClient := gitalypb.NewHookServiceClient(conn) ctx = featureflag.OutgoingWithRaw(ctx, payload.FeatureFlags) - returnCode, err := hookCommand.exec(ctx, payload, hookClient, os.Args) + returnCode, err := hookCommand.exec(ctx, payload, hookClient, args) if err != nil { - logger.Fatal(err) + return 0, err } - os.Exit(returnCode) + return returnCode, nil } func noopSender(c chan error) {} |