Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 80 additions & 5 deletions pkg/gitprovider/gitea/gitea.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,25 @@ type giteaClient interface {
opts gitea.ListPullRequestsOptions,
) ([]*gitea.PullRequest, *gitea.Response, error)

ListRepoLabels(
owner string,
repo string,
opts gitea.ListLabelsOptions,
) ([]*gitea.Label, *gitea.Response, error)

GetPullRequest(
owner string,
repo string,
number int64,
) (*gitea.PullRequest, *gitea.Response, error)

AddIssueLabels(
owner string,
repo string,
number int64,
opts gitea.IssueLabelsOption,
) ([]*gitea.Label, *gitea.Response, error)

MergePullRequest(
owner string,
repo string,
Expand Down Expand Up @@ -133,6 +146,10 @@ func (p *provider) CreatePullRequest(
if opts == nil {
opts = &gitprovider.CreatePullRequestOpts{}
}
labelIDs, err := p.resolveLabelIDs(opts.Labels)
if err != nil {
return nil, err
}
giteaPR, _, err := p.client.CreatePullRequest(
p.owner,
p.repo,
Expand All @@ -149,15 +166,73 @@ func (p *provider) CreatePullRequest(
if giteaPR == nil {
return nil, fmt.Errorf("unexpected nil pull request")
}
// TODO(krancour): Add label support. The Gitea SDK's AddIssueLabels expects
// label IDs ([]int64), but Kargo's CreatePullRequestOpts.Labels are names
// ([]string). A previous implementation attempted this but silently discarded
// the labels entirely. To fix properly: list repo labels, match by name to
// get IDs, then call AddIssueLabels.
if len(labelIDs) > 0 {
if _, _, err := p.client.AddIssueLabels(
p.owner,
p.repo,
giteaPR.Index,
gitea.IssueLabelsOption{Labels: labelIDs},
); err != nil {
return nil, fmt.Errorf(
"error adding labels to pull request %d: %w",
giteaPR.Index,
err,
)
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if we get here it means we successfully created a PR but because we failed to add labels we report an error.

We could improve the error here e.g. successfully created pr number %d but failed to add labels: %w.

Although, I think a better solution would be to not have this be a separate call at all.

It looks like CreatePullRequestOption has a Labels field: https://gitea.com/gitea/go-sdk/src/tag/gitea/v0.20.0/gitea/pull.go#L159 and since you already resolve labelIDs before the CreatePullRequest call it makes more sense to just add your labelIDs there.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The separate AddIssueLabels call is gone. Resolved label IDs are now passed directly in CreatePullRequestOption.Labels, so label application stays part of PR creation instead of becoming a second failure point after the PR already exists. The updated test asserts that path, and I also validated it end-to-end against a live OrbStack-backed Gitea instance with a real paginated label set.

pr := convertGiteaPR(*giteaPR)
return &pr, nil
}

func (p *provider) resolveLabelIDs(labelNames []string) ([]int64, error) {
if len(labelNames) == 0 {
return nil, nil
}

repoLabels, _, err := p.client.ListRepoLabels(
p.owner,
p.repo,
gitea.ListLabelsOptions{},
)
if err != nil {
return nil, fmt.Errorf("error listing repository labels: %w", err)
}
Copy link
Copy Markdown
Member

@fuskovic fuskovic Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ListRepoLabels is called with an empty gitea.ListLabelsOptions{}, which uses Gitea's default page size https://gitea.com/gitea/go-sdk/src/tag/gitea/v0.20.0/gitea/list_options.go#L19.

If a repository has more labels than the default, the function will not see them and will incorrectly report them as missing.

One solution for making sure we are listing all labels here could be incrementing opts.Page in a loop until the response is empty.

I'd like to see a test for this too.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paged label lookup now walks ListRepoLabels using Response.NextPage until exhaustion instead of assuming the first page contains every repository label. I added regression coverage for a label that only appears on a later page so this does not silently regress.


labelIDsByName := make(map[string]int64, len(repoLabels))
for _, repoLabel := range repoLabels {
if repoLabel == nil {
continue
}
labelIDsByName[repoLabel.Name] = repoLabel.ID
}
Comment on lines +191 to +197
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop is unnecessary — you can build the map directly inside the pagination loop, eliminating the intermediate repoLabels slice entirely.


labelIDs := make([]int64, 0, len(labelNames))
seen := make(map[int64]struct{}, len(labelNames))
missing := make([]string, 0)
for _, labelName := range labelNames {
labelID, ok := labelIDsByName[labelName]
if !ok {
missing = append(missing, labelName)
continue
}
if _, ok := seen[labelID]; ok {
continue
}
seen[labelID] = struct{}{}
labelIDs = append(labelIDs, labelID)
}
if len(missing) > 0 {
return nil, fmt.Errorf(
"labels not found in repository %s/%s: %s",
p.owner,
p.repo,
strings.Join(missing, ", "),
)
}

return labelIDs, nil
}

// GetPullRequest implements gitprovider.Interface.
func (p *provider) GetPullRequest(
_ context.Context,
Expand Down
143 changes: 143 additions & 0 deletions pkg/gitprovider/gitea/gitea_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type mockGiteaClient struct {
owner string
repo string
issueLabelsOptions gitea.IssueLabelsOption
repoLabelsOpts gitea.ListLabelsOptions
listOpts gitea.ListPullRequestsOptions
}

Expand Down Expand Up @@ -127,6 +128,26 @@ func (m *mockGiteaClient) GetPullRequest(
return pr, resp, args.Error(2)
}

func (m *mockGiteaClient) ListRepoLabels(
owner string,
repo string,
opts gitea.ListLabelsOptions,
) ([]*gitea.Label, *gitea.Response, error) {
args := m.Called(owner, repo, opts)
m.owner = owner
m.repo = repo
m.repoLabelsOpts = opts
labels, ok := args.Get(0).([]*gitea.Label)
if !ok {
return nil, nil, args.Error(2)
}
resp, ok := args.Get(1).(*gitea.Response)
if !ok {
return labels, nil, args.Error(2)
}
return labels, resp, args.Error(2)
}

func (m *mockGiteaClient) AddIssueLabels(
owner string,
repo string,
Expand Down Expand Up @@ -252,6 +273,128 @@ func TestCreatePullRequest(t *testing.T) {
require.True(t, pr.Open)
}

func TestCreatePullRequestWithLabels(t *testing.T) {
opts := gitprovider.CreatePullRequestOpts{
Head: "feature-branch",
Base: "main",
Title: "title",
Description: "desc",
Labels: []string{"label1", "label2"},
}

mockClient := &mockGiteaClient{
pr: &gitea.PullRequest{
Index: int64(42),
State: gitea.StateOpen,
Head: &gitea.PRBranchInfo{
Sha: "HeadSha",
},
Base: &gitea.PRBranchInfo{
Sha: "BaseSha",
},
URL: "http://localhost:8080",
MergedCommitID: ptr.To("2994fd93"),
HasMerged: false,
},
}
mockClient.
On("ListRepoLabels", testRepoOwner, testRepoName, gitea.ListLabelsOptions{}).
Return(
[]*gitea.Label{
{ID: 101, Name: "label1"},
{ID: 202, Name: "label2"},
},
&gitea.Response{},
nil,
)
mockClient.
On("CreatePullRequest", testRepoOwner, testRepoName, mock.Anything).
Return(
&gitea.PullRequest{
Index: int64(42),
State: gitea.StateOpen,
Head: &gitea.PRBranchInfo{
Sha: "HeadSha",
},
Base: &gitea.PRBranchInfo{
Sha: "BaseSha",
},
URL: "http://localhost:8080",
MergedCommitID: ptr.To("BaseSha"),
HasMerged: false,
Created: &time.Time{},
},
&gitea.Response{},
nil,
)
mockClient.
On(
"AddIssueLabels",
testRepoOwner,
testRepoName,
int64(42),
gitea.IssueLabelsOption{Labels: []int64{101, 202}},
).
Return([]*gitea.Label{}, &gitea.Response{}, nil)

g := provider{
owner: testRepoOwner,
repo: testRepoName,
client: mockClient,
}
pr, err := g.CreatePullRequest(t.Context(), &opts)

mockClient.AssertExpectations(t)

require.NoError(t, err)
require.Equal(t, testRepoOwner, mockClient.owner)
require.Equal(t, testRepoName, mockClient.repo)
require.Equal(t, opts.Head, mockClient.newPr.Head)
require.Equal(t, opts.Base, mockClient.newPr.Base)
require.Equal(t, opts.Title, mockClient.newPr.Title)
require.Equal(t, opts.Description, mockClient.newPr.Body)
require.Equal(t, []int64{101, 202}, mockClient.issueLabelsOptions.Labels)
require.Equal(t, mockClient.pr.Index, pr.Number)
require.Equal(t, mockClient.pr.Base.Sha, pr.MergeCommitSHA)
require.Equal(t, mockClient.pr.URL, pr.URL)
require.True(t, pr.Open)
}

func TestCreatePullRequestWithMissingLabels(t *testing.T) {
opts := gitprovider.CreatePullRequestOpts{
Head: "feature-branch",
Base: "main",
Title: "title",
Description: "desc",
Labels: []string{"label1", "missing"},
}

mockClient := &mockGiteaClient{}
mockClient.
On("ListRepoLabels", testRepoOwner, testRepoName, gitea.ListLabelsOptions{}).
Return(
[]*gitea.Label{
{ID: 101, Name: "label1"},
},
&gitea.Response{},
nil,
)

g := provider{
owner: testRepoOwner,
repo: testRepoName,
client: mockClient,
}
pr, err := g.CreatePullRequest(t.Context(), &opts)

mockClient.AssertExpectations(t)

require.Nil(t, pr)
require.ErrorContains(t, err, "labels not found")
mockClient.AssertNotCalled(t, "CreatePullRequest", mock.Anything, mock.Anything, mock.Anything)
mockClient.AssertNotCalled(t, "AddIssueLabels", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
}

func TestGetPullRequest(t *testing.T) {
// set up mock
mockClient := &mockGiteaClient{
Expand Down
Loading