diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-04-27 11:55:42 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-04-27 12:13:12 +0300 |
commit | 74b9bce19aff24dcbf665ac846ef25ea9f594346 (patch) | |
tree | d846c6af3461d6ed3fa2b97b87b14a311d2f9241 | |
parent | 286a951ade1dd86ae42acd731f18b0cdef506ba0 (diff) |
gitlab: Move client for GitLab's internal API into separate package
The client for GitLab's internal API is currently part of the hooks
package. While a subset of exposed functions are indeed related to
hooks, others aren't. It's thus not really the right place to live.
Split out teh client into its own package `internal/gitlab` to improve
this.
28 files changed, 152 insertions, 113 deletions
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go index ffd61ead9..4f4814b69 100644 --- a/cmd/gitaly-hooks/hooks.go +++ b/cmd/gitaly-hooks/hooks.go @@ -16,6 +16,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/stream" @@ -169,7 +170,7 @@ func sendFunc(reqWriter io.Writer, stream grpc.ClientStream, stdin io.Reader) fu } } -func check(configPath string) (*hook.CheckInfo, error) { +func check(configPath string) (*gitlab.CheckInfo, error) { cfgFile, err := os.Open(configPath) if err != nil { return nil, fmt.Errorf("failed to open config file: %w", err) @@ -181,7 +182,7 @@ func check(configPath string) (*hook.CheckInfo, error) { return nil, err } - gitlabAPI, err := hook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) + gitlabAPI, err := gitlab.NewGitlabAPI(cfg.Gitlab, cfg.TLS) if err != nil { return nil, err } diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index a79baf910..00aa8d897 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -26,6 +26,7 @@ import ( gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" @@ -171,7 +172,7 @@ func testHooksPrePostReceive(t *testing.T, cfg config.Cfg, repo *gitalypb.Reposi t.Run(fmt.Sprintf("hookName: %s", hookName), func(t *testing.T) { customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName) - gitlabAPI, err := gitalyhook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) + gitlabAPI, err := gitlab.NewGitlabAPI(cfg.Gitlab, cfg.TLS) require.NoError(t, err) stop := runHookServiceServerWithAPI(t, cfg, gitlabAPI) @@ -350,7 +351,7 @@ func TestHooksPostReceiveFailed(t *testing.T) { cfg.Gitlab.URL = serverURL cfg.Gitlab.SecretFile = testhelper.WriteShellSecretFile(t, cfg.GitlabShell.Dir, secretToken) - gitlabAPI, err := gitalyhook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) + gitlabAPI, err := gitlab.NewGitlabAPI(cfg.Gitlab, cfg.TLS) require.NoError(t, err) customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") @@ -464,7 +465,7 @@ func TestHooksNotAllowed(t *testing.T) { customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") - gitlabAPI, err := gitalyhook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) + gitlabAPI, err := gitlab.NewGitlabAPI(cfg.Gitlab, cfg.TLS) require.NoError(t, err) stop := runHookServiceServerWithAPI(t, cfg, gitlabAPI) @@ -577,7 +578,7 @@ func TestCheckBadCreds(t *testing.T) { } func runHookServiceServer(t *testing.T, cfg config.Cfg) func() { - return runHookServiceServerWithAPI(t, cfg, gitalyhook.GitlabAPIStub) + return runHookServiceServerWithAPI(t, cfg, gitlab.GitlabAPIStub) } type featureFlagAsserter struct { @@ -615,7 +616,7 @@ func (svc featureFlagAsserter) PackObjectsHook(stream gitalypb.HookService_PackO return svc.wrapped.PackObjectsHook(stream) } -func runHookServiceServerWithAPI(t *testing.T, cfg config.Cfg, gitlabAPI gitalyhook.GitlabAPI) func() { +func runHookServiceServerWithAPI(t *testing.T, cfg config.Cfg, gitlabAPI gitlab.GitlabAPI) func() { registry := backchannel.NewRegistry() txManager := transaction.NewManager(cfg, registry) hookManager := gitalyhook.NewManager(config.NewLocator(cfg), txManager, gitlabAPI, cfg) diff --git a/cmd/gitaly-lfs-smudge/lfs_smudge.go b/cmd/gitaly-lfs-smudge/lfs_smudge.go index 01a16847d..37cfb17d2 100644 --- a/cmd/gitaly-lfs-smudge/lfs_smudge.go +++ b/cmd/gitaly-lfs-smudge/lfs_smudge.go @@ -11,7 +11,7 @@ import ( "github.com/git-lfs/git-lfs/lfs" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/labkit/tracing" @@ -78,7 +78,7 @@ func handleSmudge(to io.Writer, from io.Reader, config configProvider) (io.Reade WithField("gitaly_tls_config", tlsCfg). Debug("loaded GitLab API config") - client, err := hook.NewGitlabNetClient(glCfg, tlsCfg) + client, err := gitlab.NewGitlabNetClient(glCfg, tlsCfg) if err != nil { return contents, err } diff --git a/cmd/gitaly-lfs-smudge/lfs_smudge_test.go b/cmd/gitaly-lfs-smudge/lfs_smudge_test.go index d87c2b1fe..5ca279fed 100644 --- a/cmd/gitaly-lfs-smudge/lfs_smudge_test.go +++ b/cmd/gitaly-lfs-smudge/lfs_smudge_test.go @@ -33,8 +33,8 @@ size 177735` glRepository = "project-1" secretToken = "topsecret" testData = "hello world" - certPath = "../../internal/gitaly/hook/testdata/certs/server.crt" - keyPath = "../../internal/gitaly/hook/testdata/certs/server.key" + certPath = "../../internal/gitlab/testdata/certs/server.crt" + keyPath = "../../internal/gitlab/testdata/certs/server.key" ) var ( diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go index f1b1d218b..6f1148f3b 100644 --- a/cmd/gitaly-ssh/auth_test.go +++ b/cmd/gitaly-ssh/auth_test.go @@ -21,6 +21,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/setup" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testserver" @@ -147,7 +148,7 @@ func runServer(t *testing.T, secure bool, cfg config.Cfg, connectionType string, locator := config.NewLocator(cfg) registry := backchannel.NewRegistry() txManager := transaction.NewManager(cfg, registry) - hookManager := hook.NewManager(locator, txManager, hook.GitlabAPIStub, cfg) + hookManager := hook.NewManager(locator, txManager, gitlab.GitlabAPIStub, cfg) gitCmdFactory := git.NewExecCommandFactory(cfg) srv, err := server.New(secure, cfg, testhelper.DiscardTestEntry(t), registry) require.NoError(t, err) diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go index 6f6a8b103..1b05f05f5 100644 --- a/cmd/gitaly/main.go +++ b/cmd/gitaly/main.go @@ -24,6 +24,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/setup" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" glog "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/internal/tempdir" @@ -146,7 +147,7 @@ func run(cfg config.Cfg) error { if config.SkipHooks() { log.Warn("skipping GitLab API client creation since hooks are bypassed via GITALY_TESTING_NO_GIT_HOOKS") } else { - gitlabAPI, err := hook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) + gitlabAPI, err := gitlab.NewGitlabAPI(cfg.Gitlab, cfg.TLS) if err != nil { return fmt.Errorf("could not create GitLab API client: %w", err) } diff --git a/internal/gitaly/hook/check.go b/internal/gitaly/hook/check.go index ce793277b..dea4d612c 100644 --- a/internal/gitaly/hook/check.go +++ b/internal/gitaly/hook/check.go @@ -2,53 +2,10 @@ package hook import ( "context" - "encoding/json" - "fmt" - "io" - "io/ioutil" - "net/http" -) - -// CheckInfo represents the response of GitLabs `check` API endpoint -type CheckInfo struct { - // Version of the GitLab Rails component - Version string `json:"gitlab_version"` - // Revision of the Git object of the running GitLab - Revision string `json:"gitlab_revision"` - // APIVersion of GitLab, expected to be v4 - APIVersion string `json:"api_version"` - // RedisReachable shows if GitLab can reach Redis. This can be false - // while the check itself succeeds. Normal hook API calls will likely - // fail. - RedisReachable bool `json:"redis"` -} - -// Check performs an HTTP request to the internal/check API endpoint to verify -// the connection and tokens. It returns basic information of the installed -// GitLab -func (a *gitlabAPI) Check(ctx context.Context) (*CheckInfo, error) { - resp, err := a.client.Get(ctx, "/check") - if err != nil { - return nil, fmt.Errorf("HTTP GET to GitLab endpoint /check failed: %w", err) - } - - defer func() { - io.Copy(ioutil.Discard, resp.Body) - resp.Body.Close() - }() - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("Check HTTP request failed with status: %d", resp.StatusCode) - } - - var info CheckInfo - if err := json.NewDecoder(resp.Body).Decode(&info); err != nil { - return nil, fmt.Errorf("failed to decode response from /check endpoint: %w", err) - } - - return &info, nil -} + "gitlab.com/gitlab-org/gitaly/internal/gitlab" +) -func (m *GitLabHookManager) Check(ctx context.Context) (*CheckInfo, error) { +func (m *GitLabHookManager) Check(ctx context.Context) (*gitlab.CheckInfo, error) { return m.gitlabAPI.Check(ctx) } diff --git a/internal/gitaly/hook/manager.go b/internal/gitaly/hook/manager.go index 69c90534d..5a7590b77 100644 --- a/internal/gitaly/hook/manager.go +++ b/internal/gitaly/hook/manager.go @@ -6,6 +6,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -49,13 +50,13 @@ type Manager interface { // uses the GitLab API to authenticate and track ongoing hook calls. type GitLabHookManager struct { locator storage.Locator - gitlabAPI GitlabAPI + gitlabAPI gitlab.GitlabAPI hooksConfig config.Hooks txManager transaction.Manager } // NewManager returns a new hook manager -func NewManager(locator storage.Locator, txManager transaction.Manager, gitlabAPI GitlabAPI, cfg config.Cfg) *GitLabHookManager { +func NewManager(locator storage.Locator, txManager transaction.Manager, gitlabAPI gitlab.GitlabAPI, cfg config.Cfg) *GitLabHookManager { return &GitLabHookManager{ locator: locator, gitlabAPI: gitlabAPI, diff --git a/internal/gitaly/hook/postreceive.go b/internal/gitaly/hook/postreceive.go index 6ab89ecdd..e370c5336 100644 --- a/internal/gitaly/hook/postreceive.go +++ b/internal/gitaly/hook/postreceive.go @@ -12,6 +12,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -42,7 +43,7 @@ func getEnvVar(key string, vars []string) string { return "" } -func printMessages(messages []PostReceiveMessage, w io.Writer) error { +func printMessages(messages []gitlab.PostReceiveMessage, w io.Writer) error { for _, message := range messages { if _, err := w.Write([]byte("\n")); err != nil { return err @@ -75,7 +76,7 @@ func centerLine(b []byte) []byte { return append(bytes.Repeat([]byte(" "), linePadding), b...) } -func printAlert(m PostReceiveMessage, w io.Writer) error { +func printAlert(m gitlab.PostReceiveMessage, w io.Writer) error { if _, err := w.Write(bytes.Repeat([]byte("="), maxMessageWidth)); err != nil { return err } diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go index d4ae1d00e..1f81ca287 100644 --- a/internal/gitaly/hook/postreceive_test.go +++ b/internal/gitaly/hook/postreceive_test.go @@ -15,6 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -57,7 +58,7 @@ func TestPrintAlert(t *testing.T) { for _, tc := range testCases { var result bytes.Buffer - require.NoError(t, printAlert(PostReceiveMessage{Message: tc.message}, &result)) + require.NoError(t, printAlert(gitlab.PostReceiveMessage{Message: tc.message}, &result)) assert.Equal(t, tc.expected, result.String()) } } @@ -65,7 +66,7 @@ func TestPrintAlert(t *testing.T) { func TestPostReceive_customHook(t *testing.T) { cfg, repo, repoPath := testcfg.BuildWithRepo(t) - hookManager := NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), GitlabAPIStub, cfg) + hookManager := NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), gitlab.GitlabAPIStub, cfg) receiveHooksPayload := &git.ReceiveHooksPayload{ UserID: "1234", @@ -225,10 +226,10 @@ func TestPostReceive_customHook(t *testing.T) { } type postreceiveAPIMock struct { - postreceive func(context.Context, string, string, string, ...string) (bool, []PostReceiveMessage, error) + postreceive func(context.Context, string, string, string, ...string) (bool, []gitlab.PostReceiveMessage, error) } -func (m *postreceiveAPIMock) Allowed(ctx context.Context, params AllowedParams) (bool, string, error) { +func (m *postreceiveAPIMock) Allowed(ctx context.Context, params gitlab.AllowedParams) (bool, string, error) { return true, "", nil } @@ -236,11 +237,11 @@ func (m *postreceiveAPIMock) PreReceive(ctx context.Context, glRepository string return true, nil } -func (m *postreceiveAPIMock) Check(ctx context.Context) (*CheckInfo, error) { +func (m *postreceiveAPIMock) Check(ctx context.Context) (*gitlab.CheckInfo, error) { return nil, errors.New("unexpected call") } -func (m *postreceiveAPIMock) PostReceive(ctx context.Context, glRepository, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) { +func (m *postreceiveAPIMock) PostReceive(ctx context.Context, glRepository, glID, changes string, pushOptions ...string) (bool, []gitlab.PostReceiveMessage, error) { return m.postreceive(ctx, glRepository, glID, changes, pushOptions...) } @@ -261,7 +262,7 @@ func TestPostReceive_gitlab(t *testing.T) { env []string pushOptions []string changes string - postreceive func(*testing.T, context.Context, string, string, string, ...string) (bool, []PostReceiveMessage, error) + postreceive func(*testing.T, context.Context, string, string, string, ...string) (bool, []gitlab.PostReceiveMessage, error) expectHookCall bool expectedErr error expectedStdout string @@ -271,7 +272,7 @@ func TestPostReceive_gitlab(t *testing.T) { desc: "allowed change", env: standardEnv, changes: "changes\n", - postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) { + postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []gitlab.PostReceiveMessage, error) { require.Equal(t, repo.GlRepository, glRepo) require.Equal(t, "1234", glID) require.Equal(t, "changes\n", changes) @@ -288,7 +289,7 @@ func TestPostReceive_gitlab(t *testing.T) { "mr.create", }, changes: "changes\n", - postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) { + postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []gitlab.PostReceiveMessage, error) { require.Equal(t, []string{ "mr.merge_when_pipeline_succeeds", "mr.create", @@ -301,7 +302,7 @@ func TestPostReceive_gitlab(t *testing.T) { desc: "access denied without message", env: standardEnv, changes: "changes\n", - postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) { + postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []gitlab.PostReceiveMessage, error) { return false, nil, nil }, expectedErr: errors.New(""), @@ -310,8 +311,8 @@ func TestPostReceive_gitlab(t *testing.T) { desc: "access denied with message", env: standardEnv, changes: "changes\n", - postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) { - return false, []PostReceiveMessage{ + postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []gitlab.PostReceiveMessage, error) { + return false, []gitlab.PostReceiveMessage{ { Message: "access denied", Type: "alert", @@ -325,7 +326,7 @@ func TestPostReceive_gitlab(t *testing.T) { desc: "access check returns error", env: standardEnv, changes: "changes\n", - postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) { + postreceive: func(t *testing.T, ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []gitlab.PostReceiveMessage, error) { return false, nil, errors.New("failure") }, expectedErr: errors.New("GitLab: failure"), @@ -338,7 +339,7 @@ func TestPostReceive_gitlab(t *testing.T) { defer cleanup() gitlabAPI := postreceiveAPIMock{ - postreceive: func(ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []PostReceiveMessage, error) { + postreceive: func(ctx context.Context, glRepo, glID, changes string, pushOptions ...string) (bool, []gitlab.PostReceiveMessage, error) { return tc.postreceive(t, ctx, glRepo, glID, changes, pushOptions...) }, } diff --git a/internal/gitaly/hook/prereceive.go b/internal/gitaly/hook/prereceive.go index 41dc8aca7..32311cf01 100644 --- a/internal/gitaly/hook/prereceive.go +++ b/internal/gitaly/hook/prereceive.go @@ -12,6 +12,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -116,7 +117,7 @@ func (m *GitLabHookManager) preReceiveHook(ctx context.Context, payload git.Hook return helper.ErrInternalf("protocol not set") } - params := AllowedParams{ + params := gitlab.AllowedParams{ RepoPath: repoPath, GitObjectDirectory: repo.GitObjectDirectory, GitAlternateObjectDirectories: repo.GitAlternateObjectDirectories, diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go index ee0054093..2fe6a67f0 100644 --- a/internal/gitaly/hook/prereceive_test.go +++ b/internal/gitaly/hook/prereceive_test.go @@ -14,6 +14,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" @@ -24,7 +25,7 @@ import ( func TestPrereceive_customHooks(t *testing.T) { cfg, repo, repoPath := testcfg.BuildWithRepo(t) - hookManager := NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), GitlabAPIStub, cfg) + hookManager := NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), gitlab.GitlabAPIStub, cfg) receiveHooksPayload := &git.ReceiveHooksPayload{ UserID: "1234", @@ -186,11 +187,11 @@ func TestPrereceive_customHooks(t *testing.T) { } type prereceiveAPIMock struct { - allowed func(context.Context, AllowedParams) (bool, string, error) + allowed func(context.Context, gitlab.AllowedParams) (bool, string, error) prereceive func(context.Context, string) (bool, error) } -func (m *prereceiveAPIMock) Allowed(ctx context.Context, params AllowedParams) (bool, string, error) { +func (m *prereceiveAPIMock) Allowed(ctx context.Context, params gitlab.AllowedParams) (bool, string, error) { return m.allowed(ctx, params) } @@ -198,11 +199,11 @@ func (m *prereceiveAPIMock) PreReceive(ctx context.Context, glRepository string) return m.prereceive(ctx, glRepository) } -func (m *prereceiveAPIMock) Check(ctx context.Context) (*CheckInfo, error) { +func (m *prereceiveAPIMock) Check(ctx context.Context) (*gitlab.CheckInfo, error) { return nil, errors.New("unexpected call") } -func (m *prereceiveAPIMock) PostReceive(context.Context, string, string, string, ...string) (bool, []PostReceiveMessage, error) { +func (m *prereceiveAPIMock) PostReceive(context.Context, string, string, string, ...string) (bool, []gitlab.PostReceiveMessage, error) { return true, nil, errors.New("unexpected call") } @@ -222,7 +223,7 @@ func TestPrereceive_gitlab(t *testing.T) { desc string env []string changes string - allowed func(*testing.T, context.Context, AllowedParams) (bool, string, error) + allowed func(*testing.T, context.Context, gitlab.AllowedParams) (bool, string, error) prereceive func(*testing.T, context.Context, string) (bool, error) expectHookCall bool expectedErr error @@ -231,7 +232,7 @@ func TestPrereceive_gitlab(t *testing.T) { desc: "allowed change", env: standardEnv, changes: "changes\n", - allowed: func(t *testing.T, ctx context.Context, params AllowedParams) (bool, string, error) { + allowed: func(t *testing.T, ctx context.Context, params gitlab.AllowedParams) (bool, string, error) { require.Equal(t, repoPath, params.RepoPath) require.Equal(t, repo.GlRepository, params.GLRepository) require.Equal(t, "1234", params.GLID) @@ -249,7 +250,7 @@ func TestPrereceive_gitlab(t *testing.T) { desc: "disallowed change", env: standardEnv, changes: "changes\n", - allowed: func(t *testing.T, ctx context.Context, params AllowedParams) (bool, string, error) { + allowed: func(t *testing.T, ctx context.Context, params gitlab.AllowedParams) (bool, string, error) { return false, "you shall not pass", nil }, expectHookCall: false, @@ -259,7 +260,7 @@ func TestPrereceive_gitlab(t *testing.T) { desc: "allowed returns error", env: standardEnv, changes: "changes\n", - allowed: func(t *testing.T, ctx context.Context, params AllowedParams) (bool, string, error) { + allowed: func(t *testing.T, ctx context.Context, params gitlab.AllowedParams) (bool, string, error) { return false, "", errors.New("oops") }, expectHookCall: false, @@ -269,7 +270,7 @@ func TestPrereceive_gitlab(t *testing.T) { desc: "prereceive rejects", env: standardEnv, changes: "changes\n", - allowed: func(t *testing.T, ctx context.Context, params AllowedParams) (bool, string, error) { + allowed: func(t *testing.T, ctx context.Context, params gitlab.AllowedParams) (bool, string, error) { return true, "", nil }, prereceive: func(t *testing.T, ctx context.Context, glRepo string) (bool, error) { @@ -282,7 +283,7 @@ func TestPrereceive_gitlab(t *testing.T) { desc: "prereceive errors", env: standardEnv, changes: "changes\n", - allowed: func(t *testing.T, ctx context.Context, params AllowedParams) (bool, string, error) { + allowed: func(t *testing.T, ctx context.Context, params gitlab.AllowedParams) (bool, string, error) { return true, "", nil }, prereceive: func(t *testing.T, ctx context.Context, glRepo string) (bool, error) { @@ -299,7 +300,7 @@ func TestPrereceive_gitlab(t *testing.T) { defer cleanup() gitlabAPI := prereceiveAPIMock{ - allowed: func(ctx context.Context, params AllowedParams) (bool, string, error) { + allowed: func(ctx context.Context, params gitlab.AllowedParams) (bool, string, error) { return tc.allowed(t, ctx, params) }, prereceive: func(ctx context.Context, glRepo string) (bool, error) { diff --git a/internal/gitaly/hook/transactions_test.go b/internal/gitaly/hook/transactions_test.go index 837c1e6f7..326db0bc0 100644 --- a/internal/gitaly/hook/transactions_test.go +++ b/internal/gitaly/hook/transactions_test.go @@ -14,6 +14,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -33,7 +34,7 @@ func TestHookManager_stopCalled(t *testing.T) { } var mockTxMgr transaction.MockManager - hookManager := NewManager(config.NewLocator(cfg), &mockTxMgr, GitlabAPIStub, cfg) + hookManager := NewManager(config.NewLocator(cfg), &mockTxMgr, gitlab.GitlabAPIStub, cfg) ctx, cleanup := testhelper.Context() defer cleanup() @@ -126,7 +127,7 @@ func TestHookManager_contextCancellationCancelsVote(t *testing.T) { }, } - hookManager := NewManager(config.NewLocator(cfg), &mockTxMgr, GitlabAPIStub, cfg) + hookManager := NewManager(config.NewLocator(cfg), &mockTxMgr, gitlab.GitlabAPIStub, cfg) hooksPayload, err := git.NewHooksPayload( cfg, diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go index 63b212080..730ae3f08 100644 --- a/internal/gitaly/hook/update_test.go +++ b/internal/gitaly/hook/update_test.go @@ -12,6 +12,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -21,7 +22,7 @@ import ( func TestUpdate_customHooks(t *testing.T) { cfg, repo, repoPath := testcfg.BuildWithRepo(t) - hookManager := NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), GitlabAPIStub, cfg) + hookManager := NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), gitlab.GitlabAPIStub, cfg) receiveHooksPayload := &git.ReceiveHooksPayload{ UserID: "1234", diff --git a/internal/gitaly/server/auth_test.go b/internal/gitaly/server/auth_test.go index f5fd1b1f7..f215af87c 100644 --- a/internal/gitaly/server/auth_test.go +++ b/internal/gitaly/server/auth_test.go @@ -24,6 +24,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/setup" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -196,7 +197,7 @@ func runServer(t *testing.T, cfg config.Cfg) string { t.Cleanup(func() { conns.Close() }) locator := config.NewLocator(cfg) txManager := transaction.NewManager(cfg, registry) - hookManager := hook.NewManager(locator, txManager, hook.GitlabAPIStub, cfg) + hookManager := hook.NewManager(locator, txManager, gitlab.GitlabAPIStub, cfg) gitCmdFactory := git.NewExecCommandFactory(cfg) srv, err := New(false, cfg, testhelper.DiscardTestEntry(t), registry) diff --git a/internal/gitaly/service/dependencies.go b/internal/gitaly/service/dependencies.go index f67d66724..8c7a08ca4 100644 --- a/internal/gitaly/service/dependencies.go +++ b/internal/gitaly/service/dependencies.go @@ -9,6 +9,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/linguist" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/storage" ) @@ -23,7 +24,7 @@ type Dependencies struct { GitCmdFactory git.CommandFactory Linguist *linguist.Instance BackchannelRegistry *backchannel.Registry - GitlabAPI gitalyhook.GitlabAPI + GitlabAPI gitlab.GitlabAPI } // GetCfg returns service configuration. @@ -72,6 +73,6 @@ func (dc *Dependencies) GetBackchannelRegistry() *backchannel.Registry { } // GetGitlabAPI returns client to access GitLab API. -func (dc *Dependencies) GetGitlabAPI() gitalyhook.GitlabAPI { +func (dc *Dependencies) GetGitlabAPI() gitlab.GitlabAPI { return dc.GitlabAPI } diff --git a/internal/gitaly/service/hook/post_receive_test.go b/internal/gitaly/service/hook/post_receive_test.go index e7bad14b1..fda796537 100644 --- a/internal/gitaly/service/hook/post_receive_test.go +++ b/internal/gitaly/service/hook/post_receive_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" @@ -66,7 +66,7 @@ func TestHooksMissingStdin(t *testing.T) { }, } - api, err := gitalyhook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) + api, err := gitlab.NewGitlabAPI(cfg.Gitlab, cfg.TLS) require.NoError(t, err) testCases := []struct { @@ -217,7 +217,7 @@ To create a merge request for okay, visit: }, } - api, err := gitalyhook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) + api, err := gitlab.NewGitlabAPI(cfg.Gitlab, cfg.TLS) require.NoError(t, err) serverSocketPath := runHooksServer(t, cfg, nil, testserver.WithGitLabAPI(api)) diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go index dc9d03c40..4e8895b1d 100644 --- a/internal/gitaly/service/hook/pre_receive_test.go +++ b/internal/gitaly/service/hook/pre_receive_test.go @@ -15,7 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" @@ -132,7 +132,7 @@ func TestPreReceiveHook_GitlabAPIAccess(t *testing.T) { SecretFile: secretFilePath, } - gitlabAPI, err := gitalyhook.NewGitlabAPI(gitlabConfig, cfg.TLS) + gitlabAPI, err := gitlab.NewGitlabAPI(gitlabConfig, cfg.TLS) require.NoError(t, err) serverSocketPath := runHooksServer(t, cfg, nil, testserver.WithGitLabAPI(gitlabAPI)) @@ -248,7 +248,7 @@ func TestPreReceive_APIErrors(t *testing.T) { SecretFile: secretFilePath, } - gitlabAPI, err := gitalyhook.NewGitlabAPI(gitlabConfig, cfg.TLS) + gitlabAPI, err := gitlab.NewGitlabAPI(gitlabConfig, cfg.TLS) require.NoError(t, err) serverSocketPath := runHooksServer(t, cfg, nil, testserver.WithGitLabAPI(gitlabAPI)) @@ -321,7 +321,7 @@ exit %d SecretFile: secretFilePath, } - gitlabAPI, err := gitalyhook.NewGitlabAPI(gitlabConfig, cfg.TLS) + gitlabAPI, err := gitlab.NewGitlabAPI(gitlabConfig, cfg.TLS) require.NoError(t, err) serverSocketPath := runHooksServer(t, cfg, nil, testserver.WithGitLabAPI(gitlabAPI)) @@ -446,7 +446,7 @@ func TestPreReceiveHook_Primary(t *testing.T) { gittest.WriteCustomHook(t, testRepoPath, "pre-receive", []byte(fmt.Sprintf("#!/bin/bash\nexit %d", tc.hookExitCode))) - gitlabAPI, err := gitalyhook.NewGitlabAPI(config.Gitlab{ + gitlabAPI, err := gitlab.NewGitlabAPI(config.Gitlab{ URL: srv.URL, SecretFile: secretFilePath, }, cfg.TLS) diff --git a/internal/gitaly/service/hook/server_test.go b/internal/gitaly/service/hook/server_test.go index 07aefae34..d443a7d5a 100644 --- a/internal/gitaly/service/hook/server_test.go +++ b/internal/gitaly/service/hook/server_test.go @@ -7,8 +7,9 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/backchannel" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/streamcache" ) @@ -43,7 +44,7 @@ func TestNewServer(t *testing.T) { cfg := tc.cfg poc := NewServer( cfg, - gitalyhook.NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), gitalyhook.GitlabAPIStub, cfg), + hook.NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), gitlab.GitlabAPIStub, cfg), git.NewExecCommandFactory(cfg), ).(*server).packObjectsCache diff --git a/internal/gitaly/service/objectpool/testhelper_test.go b/internal/gitaly/service/objectpool/testhelper_test.go index a55b10926..4a87401d6 100644 --- a/internal/gitaly/service/objectpool/testhelper_test.go +++ b/internal/gitaly/service/objectpool/testhelper_test.go @@ -13,6 +13,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" hookservice "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" @@ -59,7 +60,7 @@ func runObjectPoolServer(t *testing.T, cfg config.Cfg, locator storage.Locator) require.NoError(t, err) txManager := transaction.NewManager(cfg, backchannel.NewRegistry()) - hookManager := hook.NewManager(locator, txManager, hook.GitlabAPIStub, cfg) + hookManager := hook.NewManager(locator, txManager, gitlab.GitlabAPIStub, cfg) gitCmdFactory := git.NewExecCommandFactory(cfg) gitalypb.RegisterObjectPoolServiceServer(server, NewServer(cfg, locator, gitCmdFactory)) diff --git a/internal/gitaly/service/repository/fork_test.go b/internal/gitaly/service/repository/fork_test.go index c7ea4c15b..20fc79943 100644 --- a/internal/gitaly/service/repository/fork_test.go +++ b/internal/gitaly/service/repository/fork_test.go @@ -24,6 +24,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/remote" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/ssh" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testserver" @@ -216,7 +217,7 @@ func runSecureServer(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) s locator := config.NewLocator(cfg) txManager := transaction.NewManager(cfg, registry) - hookManager := hook.NewManager(locator, txManager, hook.GitlabAPIStub, cfg) + hookManager := hook.NewManager(locator, txManager, gitlab.GitlabAPIStub, cfg) gitCmdFactory := git.NewExecCommandFactory(cfg) gitalypb.RegisterRepositoryServiceServer(server, NewServer(cfg, rubySrv, locator, txManager, gitCmdFactory)) diff --git a/internal/gitaly/hook/access.go b/internal/gitlab/access.go index adb957028..ba2f6d60f 100644 --- a/internal/gitaly/hook/access.go +++ b/internal/gitlab/access.go @@ -1,4 +1,4 @@ -package hook +package gitlab import ( "context" diff --git a/internal/gitaly/hook/access_test.go b/internal/gitlab/access_test.go index da34b4c7e..324913d42 100644 --- a/internal/gitaly/hook/access_test.go +++ b/internal/gitlab/access_test.go @@ -1,4 +1,4 @@ -package hook +package gitlab import ( "context" diff --git a/internal/gitlab/check.go b/internal/gitlab/check.go new file mode 100644 index 000000000..968e4061c --- /dev/null +++ b/internal/gitlab/check.go @@ -0,0 +1,50 @@ +package gitlab + +import ( + "context" + "encoding/json" + "fmt" + "io" + "io/ioutil" + "net/http" +) + +// CheckInfo represents the response of GitLabs `check` API endpoint +type CheckInfo struct { + // Version of the GitLab Rails component + Version string `json:"gitlab_version"` + // Revision of the Git object of the running GitLab + Revision string `json:"gitlab_revision"` + // APIVersion of GitLab, expected to be v4 + APIVersion string `json:"api_version"` + // RedisReachable shows if GitLab can reach Redis. This can be false + // while the check itself succeeds. Normal hook API calls will likely + // fail. + RedisReachable bool `json:"redis"` +} + +// Check performs an HTTP request to the internal/check API endpoint to verify +// the connection and tokens. It returns basic information of the installed +// GitLab +func (a *gitlabAPI) Check(ctx context.Context) (*CheckInfo, error) { + resp, err := a.client.Get(ctx, "/check") + if err != nil { + return nil, fmt.Errorf("HTTP GET to GitLab endpoint /check failed: %w", err) + } + + defer func() { + io.Copy(ioutil.Discard, resp.Body) + resp.Body.Close() + }() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("Check HTTP request failed with status: %d", resp.StatusCode) + } + + var info CheckInfo + if err := json.NewDecoder(resp.Body).Decode(&info); err != nil { + return nil, fmt.Errorf("failed to decode response from /check endpoint: %w", err) + } + + return &info, nil +} diff --git a/internal/gitaly/hook/testdata/certs/server.crt b/internal/gitlab/testdata/certs/server.crt index af0103e8e..af0103e8e 100644 --- a/internal/gitaly/hook/testdata/certs/server.crt +++ b/internal/gitlab/testdata/certs/server.crt diff --git a/internal/gitaly/hook/testdata/certs/server.key b/internal/gitlab/testdata/certs/server.key index f343b2be7..f343b2be7 100644 --- a/internal/gitaly/hook/testdata/certs/server.key +++ b/internal/gitlab/testdata/certs/server.key diff --git a/internal/gitlab/testhelper_test.go b/internal/gitlab/testhelper_test.go new file mode 100644 index 000000000..9684f3acb --- /dev/null +++ b/internal/gitlab/testhelper_test.go @@ -0,0 +1,15 @@ +package gitlab + +import ( + "os" + "testing" + + "gitlab.com/gitlab-org/gitaly/internal/testhelper" +) + +func TestMain(m *testing.M) { + defer testhelper.MustHaveNoChildProcess() + cleanup := testhelper.Configure() + defer cleanup() + os.Exit(m.Run()) +} diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go index 64e288acc..7def5db7e 100644 --- a/internal/testhelper/testserver/gitaly.go +++ b/internal/testhelper/testserver/gitaly.go @@ -26,6 +26,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/server" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/gitlab" praefectconfig "gitlab.com/gitlab-org/gitaly/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -264,7 +265,7 @@ type gitalyServerDeps struct { locator storage.Locator txMgr transaction.Manager hookMgr hook.Manager - gitlabAPI hook.GitlabAPI + gitlabAPI gitlab.GitlabAPI gitCmdFactory git.CommandFactory linguist *linguist.Instance backchannelReg *backchannel.Registry @@ -284,7 +285,7 @@ func (gsd *gitalyServerDeps) createDependencies(t testing.TB, cfg config.Cfg, ru } if gsd.gitlabAPI == nil { - gsd.gitlabAPI = hook.GitlabAPIStub + gsd.gitlabAPI = gitlab.GitlabAPIStub } if gsd.backchannelReg == nil { @@ -343,7 +344,7 @@ func WithLocator(locator storage.Locator) GitalyServerOpt { } // WithGitLabAPI sets hook.GitlabAPI instance that will be used for gitaly services initialisation. -func WithGitLabAPI(gitlabAPI hook.GitlabAPI) GitalyServerOpt { +func WithGitLabAPI(gitlabAPI gitlab.GitlabAPI) GitalyServerOpt { return func(deps gitalyServerDeps) gitalyServerDeps { deps.gitlabAPI = gitlabAPI return deps |