Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit b672ff9

Browse files
authored
Connections for Gerrit Source for Batch Changes (#52401)
Part of https://github.com/sourcegraph/sourcegraph/issues/50867. You will probably notice that some things about the translation of Gerrit Change data to Changeset aren't filled in, and that's because the Gerrit Change API does not give you much info to go off of. This is an example of what a Gerrit API response looks like ```json { "id": "src-cli~master~I5de272baea22ef34dfbd00d6e96c45b25019697f", "project": "src-cli", "branch": "master", "hashtags": [ "testhash" ], "change_id": "I5de272baea22ef34dfbd00d6e96c45b25019697f", "subject": "testing off of a diff branch", "status": "NEW", "created": "2023-05-24 19:04:01.000000000", "updated": "2023-05-24 19:50:00.000000000", "submit_type": "MERGE_IF_NECESSARY", "mergeable": true, "insertions": 1, "deletions": 0, "total_comment_count": 0, "unresolved_comment_count": 0, "has_review_started": true, "_number": 329, "owner": { "_account_id": 1000018 }, "requirements": [] } ``` I plan to do the event/reviewer stuff in follow-up PRs ## Test plan Added additional unit tests
1 parent 0294b63 commit b672ff9

File tree

24 files changed

+373
-62
lines changed

24 files changed

+373
-62
lines changed

client/web/src/enterprise/batches/settings/AddCredentialModal.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,15 @@ const scopeRequirements: Record<ExternalServiceKind, JSX.Element> = {
5959
permissions.
6060
</span>
6161
),
62+
[ExternalServiceKind.GERRIT]: (
63+
// TODO: @varsanojidan change this after spike
64+
<span>
65+
with <Code>Organization:All accessible organizations</Code>, <Code>Code:Full</Code>,{' '}
66+
<Code>Code:Status</Code>, <Code>Pull Request Threads:Read & Write</Code>, and <Code>User Profile:Read</Code>{' '}
67+
permissions.
68+
</span>
69+
),
6270
// These are just for type completeness and serve as placeholders for a bright future.
63-
[ExternalServiceKind.GERRIT]: <span>Unsupported</span>,
6471
[ExternalServiceKind.GITOLITE]: <span>Unsupported</span>,
6572
[ExternalServiceKind.GOMODULES]: <span>Unsupported</span>,
6673
[ExternalServiceKind.PYTHONPACKAGES]: <span>Unsupported</span>,

enterprise/cmd/frontend/internal/batches/resolvers/code_host.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func (c *batchChangesCodeHostResolver) RequiresSSH() bool {
3131

3232
func (c *batchChangesCodeHostResolver) RequiresUsername() bool {
3333
switch c.codeHost.ExternalServiceType {
34-
case extsvc.TypeBitbucketCloud, extsvc.TypeAzureDevOps:
34+
case extsvc.TypeBitbucketCloud, extsvc.TypeAzureDevOps, extsvc.TypeGerrit:
3535
return true
3636
}
3737

enterprise/cmd/frontend/internal/batches/resolvers/resolver.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,6 +1256,13 @@ func (r *Resolver) generateAuthenticatorForCredential(ctx context.Context, exter
12561256
PublicKey: keypair.PublicKey,
12571257
Passphrase: keypair.Passphrase,
12581258
}
1259+
} else if externalServiceType == extsvc.TypeGerrit {
1260+
a = &extsvcauth.BasicAuthWithSSH{
1261+
BasicAuth: extsvcauth.BasicAuth{Username: *username, Password: credential},
1262+
PrivateKey: keypair.PrivateKey,
1263+
PublicKey: keypair.PublicKey,
1264+
Passphrase: keypair.Passphrase,
1265+
}
12591266
} else {
12601267
a = &extsvcauth.OAuthBearerTokenWithSSH{
12611268
OAuthBearerToken: extsvcauth.OAuthBearerToken{Token: credential},

enterprise/internal/batches/sources/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

enterprise/internal/batches/sources/gerrit.go

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"net/url"
66

7+
gerritbatches "github.com/sourcegraph/sourcegraph/enterprise/internal/batches/sources/gerrit"
78
"github.com/sourcegraph/sourcegraph/internal/errcode"
89
"github.com/sourcegraph/sourcegraph/internal/extsvc/auth"
910
"github.com/sourcegraph/sourcegraph/internal/extsvc/gerrit"
@@ -19,8 +20,6 @@ type GerritSource struct {
1920
client gerrit.Client
2021
}
2122

22-
var _ ForkableChangesetSource = GerritSource{}
23-
2423
func NewGerritSource(ctx context.Context, svc *types.ExternalService, cf *httpcli.Factory) (*GerritSource, error) {
2524
rawConfig, err := svc.Config.Decrypt(ctx)
2625
if err != nil {
@@ -42,7 +41,7 @@ func NewGerritSource(ctx context.Context, svc *types.ExternalService, cf *httpcl
4241

4342
gerritURL, err := url.Parse(c.Url)
4443
if err != nil {
45-
return nil, errors.Wrap(err, "parsing Gerrit URL")
44+
return nil, errors.Wrap(err, "parsing Gerrit CodeHostURL")
4645
}
4746

4847
client, err := gerrit.NewClient(svc.URN(), gerritURL, &gerrit.AccountCredentials{Username: c.Username, Password: c.Password}, cli)
@@ -87,21 +86,26 @@ func (s GerritSource) LoadChangeset(ctx context.Context, cs *Changeset) error {
8786
if errcode.IsNotFound(err) {
8887
return ChangesetNotFoundError{Changeset: cs}
8988
}
90-
return errors.Wrap(err, "getting pull request")
89+
return errors.Wrap(err, "getting change")
9190
}
9291

93-
return errors.Wrap(s.setChangesetMetadata(ctx, pr, cs), "setting Gerrit changeset metadata")
92+
return errors.Wrap(s.setChangesetMetadata(pr, cs), "setting Gerrit changeset metadata")
9493
}
9594

9695
// CreateChangeset will create the Changeset on the source. If it already
9796
// exists, *Changeset will be populated and the return value will be true.
98-
// noop, Gerrit creates changes through commits directly
99-
func (s GerritSource) CreateChangeset(_ context.Context, _ *Changeset) (bool, error) {
97+
func (s GerritSource) CreateChangeset(ctx context.Context, cs *Changeset) (bool, error) {
98+
// For Gerrit, the Change is created at `git push` time, so we just load it here to verify it
99+
// was created successfully.
100+
err := s.LoadChangeset(ctx, cs)
101+
if err != nil {
102+
return false, err
103+
}
100104
return true, nil
101105
}
102106

103107
// CreateDraftChangeset creates the given changeset on the code host in draft mode.
104-
// no-op, Gerrit creates changes through commits directly
108+
// Noop, Gerrit creates changes through commits directly
105109
func (s GerritSource) CreateDraftChangeset(context.Context, *Changeset) (bool, error) {
106110
return true, nil
107111
}
@@ -118,10 +122,10 @@ func (s GerritSource) UndraftChangeset(context.Context, *Changeset) error {
118122
func (s GerritSource) CloseChangeset(ctx context.Context, cs *Changeset) error {
119123
updated, err := s.client.AbandonChange(ctx, cs.ExternalID)
120124
if err != nil {
121-
return errors.Wrap(err, "abandoning pull request")
125+
return errors.Wrap(err, "abandoning change")
122126
}
123127

124-
return errors.Wrap(s.setChangesetMetadata(ctx, updated, cs), "setting Gerrit changeset metadata")
128+
return errors.Wrap(s.setChangesetMetadata(updated, cs), "setting Gerrit changeset metadata")
125129
}
126130

127131
// UpdateChangeset can update Changesets.
@@ -135,10 +139,10 @@ func (s GerritSource) UpdateChangeset(context.Context, *Changeset) error {
135139
func (s GerritSource) ReopenChangeset(ctx context.Context, cs *Changeset) error {
136140
updated, err := s.client.RestoreChange(ctx, cs.ExternalID)
137141
if err != nil {
138-
return errors.Wrap(err, "updating pull request")
142+
return errors.Wrap(err, "restoring change")
139143
}
140144

141-
return errors.Wrap(s.setChangesetMetadata(ctx, updated, cs), "setting Gerrit changeset metadata")
145+
return errors.Wrap(s.setChangesetMetadata(updated, cs), "setting Gerrit changeset metadata")
142146
}
143147

144148
// CreateComment posts a comment on the Changeset.
@@ -154,31 +158,29 @@ func (s GerritSource) CreateComment(ctx context.Context, cs *Changeset, comment
154158
// merge. If the changeset cannot be merged, because it is in an unmergeable
155159
// state, ChangesetNotMergeableError must be returned.
156160
// Gerrit changes are always single commit, so squash does not matter.
157-
func (s GerritSource) MergeChangeset(ctx context.Context, cs *Changeset, squash bool) error {
161+
func (s GerritSource) MergeChangeset(ctx context.Context, cs *Changeset, _ bool) error {
158162
updated, err := s.client.SubmitChange(ctx, cs.ExternalID)
159163
if err != nil {
160164
if errcode.IsNotFound(err) {
161-
return errors.Wrap(err, "merging pull request")
165+
return errors.Wrap(err, "submitting change")
162166
}
163167
return ChangesetNotMergeableError{ErrorMsg: err.Error()}
164168
}
165169

166-
return errors.Wrap(s.setChangesetMetadata(ctx, updated, cs), "setting Gerrit changeset metadata")
167-
}
168-
169-
// GetFork returns a repo pointing to a fork of the target repo, ensuring that the fork
170-
// exists and creating it if it doesn't. If namespace is not provided, the original namespace is used.
171-
// If name is not provided, the fork will be named with the default Sourcegraph convention:
172-
// "${original-namespace}-${original-name}"
173-
// Noop, Gerrit does not support Changes from forks.
174-
func (s GerritSource) GetFork(_ context.Context, _ *types.Repo, _, _ *string) (*types.Repo, error) {
175-
return nil, nil
170+
return errors.Wrap(s.setChangesetMetadata(updated, cs), "setting Gerrit changeset metadata")
176171
}
177172

178-
func (s GerritSource) setChangesetMetadata(_ context.Context, pr *gerrit.Change, cs *Changeset) error {
179-
if err := cs.SetMetadata(pr); err != nil {
173+
func (s GerritSource) setChangesetMetadata(change *gerrit.Change, cs *Changeset) error {
174+
apr := s.annotatePullRequest(change)
175+
if err := cs.SetMetadata(apr); err != nil {
180176
return errors.Wrap(err, "setting changeset metadata")
181177
}
182-
183178
return nil
184179
}
180+
181+
func (s GerritSource) annotatePullRequest(change *gerrit.Change) *gerritbatches.AnnotatedChange {
182+
return &gerritbatches.AnnotatedChange{
183+
Change: change,
184+
CodeHostURL: s.client.GetURL(),
185+
}
186+
}

enterprise/internal/batches/sources/gerrit/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package gerrit
2+
3+
import (
4+
"net/url"
5+
6+
"github.com/sourcegraph/sourcegraph/internal/extsvc/gerrit"
7+
)
8+
9+
// AnnotatedChange adds metadata we need that lives outside the main
10+
// Change type returned by the Gerrit API.
11+
// This type is used as the primary metadata type for Gerrit
12+
// changesets.
13+
type AnnotatedChange struct {
14+
*gerrit.Change
15+
CodeHostURL *url.URL
16+
}

enterprise/internal/batches/sources/gerrit_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package sources
22

33
import (
44
"context"
5+
"net/url"
56
"testing"
67

78
"github.com/stretchr/testify/assert"
@@ -124,6 +125,7 @@ func TestGerritSource_LoadChangeset(t *testing.T) {
124125
s, client := mockGerritSource()
125126

126127
change := mockGerritChange(&testProject)
128+
client.GetURLFunc.SetDefaultReturn(&url.URL{})
127129
client.GetChangeFunc.SetDefaultHook(func(ctx context.Context, changeID string) (*gerrit.Change, error) {
128130
assert.Equal(t, changeID, testGerritChangeID)
129131
return change, nil
@@ -134,6 +136,57 @@ func TestGerritSource_LoadChangeset(t *testing.T) {
134136
})
135137
}
136138

139+
func TestGerritSource_CreateChangeset(t *testing.T) {
140+
ctx := context.Background()
141+
142+
t.Run("error getting pull request", func(t *testing.T) {
143+
cs, _ := mockGerritChangeset()
144+
s, client := mockGerritSource()
145+
want := errors.New("error")
146+
client.GetChangeFunc.SetDefaultHook(func(ctx context.Context, changeID string) (*gerrit.Change, error) {
147+
assert.Equal(t, changeID, testGerritChangeID)
148+
return &gerrit.Change{}, want
149+
})
150+
151+
b, err := s.CreateChangeset(ctx, cs)
152+
assert.NotNil(t, err)
153+
assert.ErrorIs(t, err, want)
154+
assert.False(t, b)
155+
})
156+
157+
t.Run("pull request not found", func(t *testing.T) {
158+
cs, _ := mockGerritChangeset()
159+
s, client := mockGerritSource()
160+
client.GetChangeFunc.SetDefaultHook(func(ctx context.Context, changeID string) (*gerrit.Change, error) {
161+
assert.Equal(t, changeID, testGerritChangeID)
162+
return &gerrit.Change{}, &notFoundError{}
163+
})
164+
165+
b, err := s.CreateChangeset(ctx, cs)
166+
assert.NotNil(t, err)
167+
target := ChangesetNotFoundError{}
168+
assert.ErrorAs(t, err, &target)
169+
assert.Same(t, target.Changeset, cs)
170+
assert.False(t, b)
171+
})
172+
173+
t.Run("success", func(t *testing.T) {
174+
cs, _ := mockGerritChangeset()
175+
s, client := mockGerritSource()
176+
177+
change := mockGerritChange(&testProject)
178+
client.GetURLFunc.SetDefaultReturn(&url.URL{})
179+
client.GetChangeFunc.SetDefaultHook(func(ctx context.Context, changeID string) (*gerrit.Change, error) {
180+
assert.Equal(t, changeID, testGerritChangeID)
181+
return change, nil
182+
})
183+
184+
b, err := s.CreateChangeset(ctx, cs)
185+
assert.Nil(t, err)
186+
assert.True(t, b)
187+
})
188+
}
189+
137190
func TestGerritSource_CloseChangeset(t *testing.T) {
138191
ctx := context.Background()
139192

@@ -157,6 +210,7 @@ func TestGerritSource_CloseChangeset(t *testing.T) {
157210
s, client := mockGerritSource()
158211

159212
pr := mockGerritChange(&testProject)
213+
client.GetURLFunc.SetDefaultReturn(&url.URL{})
160214
client.AbandonChangeFunc.SetDefaultHook(func(ctx context.Context, changeID string) (*gerrit.Change, error) {
161215
assert.Equal(t, changeID, testGerritChangeID)
162216
return pr, nil
@@ -240,6 +294,7 @@ func TestGerritSource_MergeChangeset(t *testing.T) {
240294
s, client := mockGerritSource()
241295

242296
pr := mockGerritChange(&testProject)
297+
client.GetURLFunc.SetDefaultReturn(&url.URL{})
243298
client.SubmitChangeFunc.SetDefaultHook(func(ctx context.Context, changeID string) (*gerrit.Change, error) {
244299
assert.Equal(t, testGerritChangeID, changeID)
245300
return pr, nil
@@ -256,6 +311,7 @@ func TestGerritSource_MergeChangeset(t *testing.T) {
256311
s, client := mockGerritSource()
257312

258313
pr := mockGerritChange(&testProject)
314+
client.GetURLFunc.SetDefaultReturn(&url.URL{})
259315
client.SubmitChangeFunc.SetDefaultHook(func(ctx context.Context, changeID string) (*gerrit.Change, error) {
260316
assert.Equal(t, testGerritChangeID, changeID)
261317
return pr, nil

enterprise/internal/batches/sources/sources.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,8 @@ func loadExternalService(ctx context.Context, s database.ExternalServiceStore, o
291291
*schema.BitbucketServerConnection,
292292
*schema.GitLabConnection,
293293
*schema.BitbucketCloudConnection,
294-
*schema.AzureDevOpsConnection:
294+
*schema.AzureDevOpsConnection,
295+
*schema.GerritConnection:
295296
return e, nil
296297
}
297298
}
@@ -313,6 +314,8 @@ func buildChangesetSource(ctx context.Context, cf *httpcli.Factory, externalServ
313314
return NewBitbucketCloudSource(ctx, externalService, cf)
314315
case extsvc.KindAzureDevOps:
315316
return NewAzureDevOpsSource(ctx, externalService, cf)
317+
case extsvc.KindGerrit:
318+
return NewGerritSource(ctx, externalService, cf)
316319
default:
317320
return nil, errors.Errorf("unsupported external service type %q", extsvc.KindToType(externalService.Kind))
318321
}

enterprise/internal/batches/state/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)