diff options
author | John Cai <jcai@gitlab.com> | 2019-12-21 01:30:16 +0300 |
---|---|---|
committer | jramsay <jcai@gitlab.com> | 2020-01-24 22:34:09 +0300 |
commit | d6eca56a475556a5039fd9442755229afc526875 (patch) | |
tree | d73e528dbc6c08774fd8a4a76104de6c68807877 | |
parent | d24d30c2af6ef0bc818da224e3bdd73f06b9e42a (diff) |
Fix gitaly-hooks check command
have the gitaly-hooks check subcommand call out to the ruby check
-rw-r--r-- | changelogs/unreleased/jc-fix-gitaly-hooks-check.yml | 5 | ||||
-rw-r--r-- | cmd/gitaly-hooks/gitaly_hooks.log | 0 | ||||
-rw-r--r-- | cmd/gitaly-hooks/hooks.go | 63 | ||||
-rw-r--r-- | cmd/gitaly-hooks/hooks_test.go | 64 | ||||
-rw-r--r-- | internal/gitlabshell/env.go | 9 | ||||
-rwxr-xr-x | ruby/gitlab-shell/bin/check | 11 |
6 files changed, 87 insertions, 65 deletions
diff --git a/changelogs/unreleased/jc-fix-gitaly-hooks-check.yml b/changelogs/unreleased/jc-fix-gitaly-hooks-check.yml new file mode 100644 index 000000000..ee0c215d2 --- /dev/null +++ b/changelogs/unreleased/jc-fix-gitaly-hooks-check.yml @@ -0,0 +1,5 @@ +--- +title: Fix gitaly-hooks check command which was broken due to an incorrect implemention +merge_request: 1761 +author: +type: fixed diff --git a/cmd/gitaly-hooks/gitaly_hooks.log b/cmd/gitaly-hooks/gitaly_hooks.log deleted file mode 100644 index e69de29bb..000000000 --- a/cmd/gitaly-hooks/gitaly_hooks.log +++ /dev/null diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go index c526c1033..9f616520b 100644 --- a/cmd/gitaly-hooks/hooks.go +++ b/cmd/gitaly-hooks/hooks.go @@ -4,19 +4,20 @@ import ( "context" "errors" "fmt" - "net/http" + "log" "os" "os/exec" "path/filepath" - "strings" + "github.com/BurntSushi/toml" "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/log" - "gopkg.in/yaml.v2" + "gitlab.com/gitlab-org/gitaly/internal/config" + "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" + gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" ) func main() { - var logger = log.NewHookLogger() + var logger = gitalylog.NewHookLogger() if len(os.Args) < 2 { logger.Fatal(errors.New("requires hook name")) @@ -27,21 +28,20 @@ func main() { if subCmd == "check" { configPath := os.Args[2] - if err := checkGitlabAccess(configPath); err != nil { - os.Stderr.WriteString(err.Error()) - os.Exit(1) + status, err := check(configPath) + if err != nil { + log.Fatal(err) } - os.Stdout.WriteString("OK") - os.Exit(0) + os.Exit(status) } - gitlabRubyDir := os.Getenv("GITALY_RUBY_DIR") - if gitlabRubyDir == "" { + gitalyRubyDir := os.Getenv("GITALY_RUBY_DIR") + if gitalyRubyDir == "" { logger.Fatal(errors.New("GITALY_RUBY_DIR not set")) } - rubyHookPath := filepath.Join(gitlabRubyDir, "gitlab-shell", "hooks", subCmd) + rubyHookPath := filepath.Join(gitalyRubyDir, "gitlab-shell", "hooks", subCmd) ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -85,36 +85,31 @@ type HTTPSettings struct { Password string `yaml:"password"` } -func checkGitlabAccess(configPath string) error { +func check(configPath string) (int, error) { cfgFile, err := os.Open(configPath) if err != nil { - return fmt.Errorf("error when opening config file: %v", err) + return 1, fmt.Errorf("error when opening config file: %v", err) } defer cfgFile.Close() - config := GitlabShellConfig{} + var c config.Cfg - if err := yaml.NewDecoder(cfgFile).Decode(&config); err != nil { - return fmt.Errorf("load toml: %v", err) + if _, err := toml.DecodeReader(cfgFile, &c); err != nil { + fmt.Println(err) + return 1, err } - req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/v4/internal/check", strings.TrimRight(config.GitlabURL, "/")), nil) - if err != nil { - return fmt.Errorf("could not create request for %s: %v", config.GitlabURL, err) - } - - req.SetBasicAuth(config.HTTPSettings.User, config.HTTPSettings.Password) - - client := &http.Client{} + cmd := exec.Command(filepath.Join(c.GitlabShell.Dir, "bin", "check")) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Env = append(os.Environ(), gitlabshell.EnvFromConfig(c)...) - resp, err := client.Do(req) - if err != nil { - return fmt.Errorf("error with request for %s: %v", config.GitlabURL, err) - } - - if resp.StatusCode != 200 { - return fmt.Errorf("FAILED. code: %d", resp.StatusCode) + if err = cmd.Run(); err != nil { + if status, ok := command.ExitStatus(err); ok { + return status, nil + } + return 1, err } - return nil + return 0, nil } diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index c7dbae128..c9689580f 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -2,7 +2,6 @@ package main import ( "bytes" - "encoding/base64" "encoding/json" "fmt" "io/ioutil" @@ -14,7 +13,6 @@ import ( "path" "path/filepath" "strconv" - "strings" "testing" "github.com/stretchr/testify/require" @@ -190,7 +188,19 @@ func TestCheckOK(t *testing.T) { os.RemoveAll(tempDir) }() - configPath := writeTemporaryConfigFile(t, tempDir, GitlabShellConfig{GitlabURL: ts.URL, HTTPSettings: HTTPSettings{User: user, Password: password}}) + gitlabShellDir := filepath.Join(tempDir, "gitlab-shell") + binDir := filepath.Join(gitlabShellDir, "bin") + require.NoError(t, os.MkdirAll(gitlabShellDir, 0755)) + require.NoError(t, os.MkdirAll(binDir, 0755)) + cwd, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Symlink(filepath.Join(cwd, "../../ruby/gitlab-shell/bin/check"), filepath.Join(binDir, "check"))) + + writeShellSecretFile(t, gitlabShellDir, "the secret") + writeTemporaryConfigFile(t, gitlabShellDir, GitlabShellConfig{GitlabURL: ts.URL, HTTPSettings: HTTPSettings{User: user, Password: password}}) + + configPath, cleanup := writeTemporaryGitalyConfigFile(t, tempDir) + defer cleanup() cmd := exec.Command(fmt.Sprintf("%s/gitaly-hooks", config.Config.BinDir), "check", configPath) @@ -200,7 +210,8 @@ func TestCheckOK(t *testing.T) { require.NoError(t, cmd.Run()) require.Empty(t, stderr.String()) - require.Equal(t, "OK", stdout.String()) + expectedCheckOutput := "Check GitLab API access: OK\nRedis available via internal API: OK\n" + require.Equal(t, expectedCheckOutput, stdout.String()) } func TestCheckBadCreds(t *testing.T) { @@ -215,7 +226,19 @@ func TestCheckBadCreds(t *testing.T) { os.RemoveAll(tempDir) }() - configPath := writeTemporaryConfigFile(t, tempDir, GitlabShellConfig{GitlabURL: ts.URL, HTTPSettings: HTTPSettings{User: user + "wrong", Password: password}}) + gitlabShellDir := filepath.Join(tempDir, "gitlab-shell") + binDir := filepath.Join(gitlabShellDir, "bin") + require.NoError(t, os.MkdirAll(gitlabShellDir, 0755)) + require.NoError(t, os.MkdirAll(binDir, 0755)) + cwd, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Symlink(filepath.Join(cwd, "../../ruby/gitlab-shell/bin/check"), filepath.Join(binDir, "check"))) + + writeTemporaryConfigFile(t, gitlabShellDir, GitlabShellConfig{GitlabURL: ts.URL, HTTPSettings: HTTPSettings{User: user + "wrong", Password: password}}) + writeShellSecretFile(t, gitlabShellDir, "the secret") + + configPath, cleanup := writeTemporaryGitalyConfigFile(t, tempDir) + defer cleanup() cmd := exec.Command(fmt.Sprintf("%s/gitaly-hooks", config.Config.BinDir), "check", configPath) @@ -224,8 +247,8 @@ func TestCheckBadCreds(t *testing.T) { cmd.Stdout = &stdout require.Error(t, cmd.Run()) - require.Equal(t, "FAILED. code: 401", stderr.String()) - require.Empty(t, stdout.String()) + require.Equal(t, "Check GitLab API access: ", stdout.String()) + require.Equal(t, "FAILED. code: 401\n", stderr.String()) } func handleAllowed(t *testing.T, secretToken string, key int, glRepository, changes string) func(w http.ResponseWriter, r *http.Request) { @@ -280,21 +303,13 @@ func handlePostReceive(t *testing.T, secretToken string, key int, glRepository, func handleCheck(t *testing.T, user, password string) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { - auth := strings.SplitN(r.Header.Get("Authorization"), " ", 2) - - if len(auth) != 2 || auth[0] != "Basic" { + u, p, ok := r.BasicAuth() + if !ok || u != user || p != password { http.Error(w, "authorization failed", http.StatusUnauthorized) return } - payload, _ := base64.StdEncoding.DecodeString(auth[1]) - pair := strings.SplitN(string(payload), ":", 2) - - if pair[0] != user || pair[1] != password { - w.WriteHeader(http.StatusUnauthorized) - return - } - + w.Write([]byte(`{"redis": true}`)) w.WriteHeader(http.StatusOK) } } @@ -332,6 +347,19 @@ func writeTemporaryConfigFile(t *testing.T, dir string, config GitlabShellConfig return path } +func writeTemporaryGitalyConfigFile(t *testing.T, tempDir string) (string, func()) { + path := filepath.Join(tempDir, "config.toml") + contents := fmt.Sprintf(` +[gitlab-shell] + dir = "%s/gitlab-shell" +`, tempDir) + require.NoError(t, ioutil.WriteFile(path, []byte(contents), 0644)) + + return path, func() { + os.RemoveAll(path) + } +} + func env(t *testing.T, glRepo, gitlabShellDir string, key int) []string { rubyDir, err := filepath.Abs("../../ruby") require.NoError(t, err) diff --git a/internal/gitlabshell/env.go b/internal/gitlabshell/env.go index 70e391ece..79bae0646 100644 --- a/internal/gitlabshell/env.go +++ b/internal/gitlabshell/env.go @@ -6,13 +6,18 @@ import "gitlab.com/gitlab-org/gitaly/internal/config" func Env() []string { cfg := config.Config + return EnvFromConfig(cfg) +} + +// EnvFromConfig returns a set of environment variables from a config struct relevant to gitlab shell +func EnvFromConfig(cfg config.Cfg) []string { return []string{ "GITALY_GITLAB_SHELL_DIR=" + cfg.GitlabShell.Dir, "GITALY_LOG_DIR=" + cfg.Logging.Dir, "GITALY_LOG_FORMAT=" + cfg.Logging.Format, "GITALY_LOG_LEVEL=" + cfg.Logging.Level, "GITLAB_SHELL_DIR=" + cfg.GitlabShell.Dir, //GITLAB_SHELL_DIR has been deprecated - "GITALY_BIN_DIR=" + config.Config.BinDir, - "GITALY_RUBY_DIR=" + config.Config.Ruby.Dir, + "GITALY_BIN_DIR=" + cfg.BinDir, + "GITALY_RUBY_DIR=" + cfg.Ruby.Dir, } } diff --git a/ruby/gitlab-shell/bin/check b/ruby/gitlab-shell/bin/check index d2224a641..c8c0ae982 100755 --- a/ruby/gitlab-shell/bin/check +++ b/ruby/gitlab-shell/bin/check @@ -29,14 +29,3 @@ rescue GitlabNet::ApiUnreachableError abort "FAILED: Failed to connect to internal API" end -config = GitlabConfig.new - -abort("ERROR: missing option in config.yml") unless config.auth_file - -print "\nAccess to #{config.auth_file}: " -if system(File.dirname(__FILE__) + '/gitlab-keys', 'check-permissions') - print 'OK' -else - abort "FAILED" -end -puts "\n" |