Skip to content

Commit e9fc72a

Browse files
authored
feat: CR-29912 manual cherry pick app set pr generator return 0 results if the repo does not exist (#409)
* manually added the changes Signed-off-by: reggie-k <[email protected]> * pull request functionality Signed-off-by: reggie-k <[email protected]> * pull request functionality Signed-off-by: reggie-k <[email protected]> --------- Signed-off-by: reggie-k <[email protected]>
1 parent 979b8c8 commit e9fc72a

28 files changed

+1177
-765
lines changed

applicationset/generators/pull_request.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"sigs.k8s.io/controller-runtime/pkg/client"
1212

1313
"github.com/gosimple/slug"
14+
log "github.com/sirupsen/logrus"
1415

1516
"github.com/argoproj/argo-cd/v3/applicationset/services"
1617
pullrequest "github.com/argoproj/argo-cd/v3/applicationset/services/pull_request"
@@ -49,6 +50,10 @@ func (g *PullRequestGenerator) GetRequeueAfter(appSetGenerator *argoprojiov1alph
4950
return DefaultPullRequestRequeueAfterSeconds
5051
}
5152

53+
func (g *PullRequestGenerator) GetContinueOnRepoNotFoundError(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) bool {
54+
return appSetGenerator.PullRequest.ContinueOnRepoNotFoundError
55+
}
56+
5257
func (g *PullRequestGenerator) GetTemplate(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) *argoprojiov1alpha1.ApplicationSetTemplate {
5358
return &appSetGenerator.PullRequest.Template
5459
}
@@ -69,10 +74,15 @@ func (g *PullRequestGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha
6974
}
7075

7176
pulls, err := pullrequest.ListPullRequests(ctx, svc, appSetGenerator.PullRequest.Filters)
77+
params := make([]map[string]any, 0, len(pulls))
7278
if err != nil {
79+
if pullrequest.IsRepositoryNotFoundError(err) && g.GetContinueOnRepoNotFoundError(appSetGenerator) {
80+
log.WithError(err).WithField("generator", g).
81+
Warn("Skipping params generation for this repository since it was not found.")
82+
return params, nil
83+
}
7384
return nil, fmt.Errorf("error listing repos: %w", err)
7485
}
75-
params := make([]map[string]any, 0, len(pulls))
7686

7787
// In order to follow the DNS label standard as defined in RFC 1123,
7888
// we need to limit the 'branch' to 50 to give room to append/suffix-ing it

applicationset/generators/pull_request_test.go

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ import (
1616
func TestPullRequestGithubGenerateParams(t *testing.T) {
1717
ctx := t.Context()
1818
cases := []struct {
19-
selectFunc func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error)
20-
values map[string]string
21-
expected []map[string]any
22-
expectedErr error
23-
applicationSet argoprojiov1alpha1.ApplicationSet
19+
selectFunc func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error)
20+
values map[string]string
21+
expected []map[string]any
22+
expectedErr error
23+
applicationSet argoprojiov1alpha1.ApplicationSet
24+
continueOnRepoNotFoundError bool
2425
}{
2526
{
2627
selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
@@ -171,6 +172,30 @@ func TestPullRequestGithubGenerateParams(t *testing.T) {
171172
expected: nil,
172173
expectedErr: errors.New("error listing repos: fake error"),
173174
},
175+
{
176+
selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
177+
return pullrequest.NewFakeService(
178+
ctx,
179+
nil,
180+
pullrequest.NewRepositoryNotFoundError(errors.New("repository not found")),
181+
)
182+
},
183+
expected: []map[string]any{},
184+
expectedErr: nil,
185+
continueOnRepoNotFoundError: true,
186+
},
187+
{
188+
selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
189+
return pullrequest.NewFakeService(
190+
ctx,
191+
nil,
192+
pullrequest.NewRepositoryNotFoundError(errors.New("repository not found")),
193+
)
194+
},
195+
expected: nil,
196+
expectedErr: errors.New("error listing repos: repository not found"),
197+
continueOnRepoNotFoundError: false,
198+
},
174199
{
175200
selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
176201
return pullrequest.NewFakeService(
@@ -260,7 +285,8 @@ func TestPullRequestGithubGenerateParams(t *testing.T) {
260285
}
261286
generatorConfig := argoprojiov1alpha1.ApplicationSetGenerator{
262287
PullRequest: &argoprojiov1alpha1.PullRequestGenerator{
263-
Values: c.values,
288+
Values: c.values,
289+
ContinueOnRepoNotFoundError: c.continueOnRepoNotFoundError,
264290
},
265291
}
266292

applicationset/services/pull_request/azure_devops.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import (
1010
"github.com/microsoft/azure-devops-go-api/azuredevops/git"
1111
)
1212

13-
const AZURE_DEVOPS_DEFAULT_URL = "https://dev.azure.com"
13+
const (
14+
AZURE_DEVOPS_DEFAULT_URL = "https://dev.azure.com"
15+
AZURE_DEVOPS_PROJECT_NOT_FOUND_ERROR = "The following project does not exist"
16+
)
1417

1518
type AzureDevOpsClientFactory interface {
1619
// Returns an Azure Devops Client interface.
@@ -70,13 +73,21 @@ func (a *AzureDevOpsService) List(ctx context.Context) ([]*PullRequest, error) {
7073
SearchCriteria: &git.GitPullRequestSearchCriteria{},
7174
}
7275

76+
pullRequests := []*PullRequest{}
77+
7378
azurePullRequests, err := client.GetPullRequestsByProject(ctx, args)
7479
if err != nil {
75-
return nil, fmt.Errorf("failed to get pull requests by project: %w", err)
80+
// A standard Http 404 error is not returned for Azure DevOps,
81+
// so checking the error message for a specific pattern.
82+
// NOTE: Since the repos are filtered later, only existence of the project
83+
// is relevant for AzureDevOps
84+
if strings.Contains(err.Error(), AZURE_DEVOPS_PROJECT_NOT_FOUND_ERROR) {
85+
// return a custom error indicating that the repository is not found,
86+
// but also return the empty result since the decision to continue or not in this case is made by the caller
87+
return pullRequests, NewRepositoryNotFoundError(err)
88+
}
7689
}
7790

78-
pullRequests := []*PullRequest{}
79-
8091
for _, pr := range *azurePullRequests {
8192
if pr.Repository == nil ||
8293
pr.Repository.Name == nil ||

applicationset/services/pull_request/azure_devops_test.go

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

33
import (
44
"context"
5+
"errors"
56
"testing"
67

78
"github.com/microsoft/azure-devops-go-api/azuredevops/webapi"
@@ -236,3 +237,36 @@ func TestBuildURL(t *testing.T) {
236237
})
237238
}
238239
}
240+
241+
func TestAzureDevOpsListReturnsRepositoryNotFoundError(t *testing.T) {
242+
args := git.GetPullRequestsByProjectArgs{
243+
Project: createStringPtr("nonexistent"),
244+
SearchCriteria: &git.GitPullRequestSearchCriteria{},
245+
}
246+
247+
pullRequestMock := []git.GitPullRequest{}
248+
249+
gitClientMock := azureMock.Client{}
250+
clientFactoryMock := &AzureClientFactoryMock{mock: &mock.Mock{}}
251+
clientFactoryMock.mock.On("GetClient", mock.Anything).Return(&gitClientMock, nil)
252+
253+
// Mock the GetPullRequestsByProject to return an error containing "404"
254+
gitClientMock.On("GetPullRequestsByProject", t.Context(), args).Return(&pullRequestMock,
255+
errors.New("The following project does not exist:"))
256+
257+
provider := AzureDevOpsService{
258+
clientFactory: clientFactoryMock,
259+
project: "nonexistent",
260+
repo: "nonexistent",
261+
labels: nil,
262+
}
263+
264+
prs, err := provider.List(t.Context())
265+
266+
// Should return empty pull requests list
267+
assert.Empty(t, prs)
268+
269+
// Should return RepositoryNotFoundError
270+
require.Error(t, err)
271+
assert.True(t, IsRepositoryNotFoundError(err), "Expected RepositoryNotFoundError but got: %v", err)
272+
}

applicationset/services/pull_request/bitbucket_cloud.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"net/url"
9+
"strings"
910

1011
"github.com/ktrysmt/go-bitbucket"
1112
)
@@ -108,8 +109,17 @@ func (b *BitbucketCloudService) List(_ context.Context) ([]*PullRequest, error)
108109
RepoSlug: b.repositorySlug,
109110
}
110111

112+
pullRequests := []*PullRequest{}
113+
111114
response, err := b.client.Repositories.PullRequests.Gets(opts)
112115
if err != nil {
116+
// A standard Http 404 error is not returned for Bitbucket Cloud,
117+
// so checking the error message for a specific pattern
118+
if strings.Contains(err.Error(), "404 Not Found") {
119+
// return a custom error indicating that the repository is not found,
120+
// but also return the empty result since the decision to continue or not in this case is made by the caller
121+
return pullRequests, NewRepositoryNotFoundError(err)
122+
}
113123
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", b.owner, b.repositorySlug, err)
114124
}
115125

@@ -133,7 +143,6 @@ func (b *BitbucketCloudService) List(_ context.Context) ([]*PullRequest, error)
133143
return nil, fmt.Errorf("error unmarshalling json to type '[]BitbucketCloudPullRequest': %w", err)
134144
}
135145

136-
pullRequests := []*PullRequest{}
137146
for _, pull := range pulls {
138147
pullRequests = append(pullRequests, &PullRequest{
139148
Number: pull.ID,

applicationset/services/pull_request/bitbucket_cloud_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,3 +455,29 @@ func TestListPullRequestBranchMatchCloud(t *testing.T) {
455455
})
456456
require.Error(t, err)
457457
}
458+
459+
func TestBitbucketCloudListReturnsRepositoryNotFoundError(t *testing.T) {
460+
mux := http.NewServeMux()
461+
server := httptest.NewServer(mux)
462+
defer server.Close()
463+
464+
path := "/repositories/nonexistent/nonexistent/pullrequests/"
465+
466+
mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) {
467+
// Return 404 status to simulate repository not found
468+
w.WriteHeader(http.StatusNotFound)
469+
_, _ = w.Write([]byte(`{"message": "404 Project Not Found"}`))
470+
})
471+
472+
svc, err := NewBitbucketCloudServiceNoAuth(server.URL, "nonexistent", "nonexistent")
473+
require.NoError(t, err)
474+
475+
prs, err := svc.List(t.Context())
476+
477+
// Should return empty pull requests list
478+
assert.Empty(t, prs)
479+
480+
// Should return RepositoryNotFoundError
481+
require.Error(t, err)
482+
assert.True(t, IsRepositoryNotFoundError(err), "Expected RepositoryNotFoundError but got: %v", err)
483+
}

applicationset/services/pull_request/bitbucket_server.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ func (b *BitbucketService) List(_ context.Context) ([]*PullRequest, error) {
7272
for {
7373
response, err := b.client.DefaultApi.GetPullRequestsPage(b.projectKey, b.repositorySlug, paged)
7474
if err != nil {
75+
if response != nil && response.Response != nil && response.StatusCode == http.StatusNotFound {
76+
// return a custom error indicating that the repository is not found,
77+
// but also return the empty result since the decision to continue or not in this case is made by the caller
78+
return pullRequests, NewRepositoryNotFoundError(err)
79+
}
7580
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", b.projectKey, b.repositorySlug, err)
7681
}
7782
pulls, err := bitbucketv1.GetPullRequestsResponse(response)

applicationset/services/pull_request/bitbucket_server_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,3 +510,29 @@ func TestListPullRequestBranchMatch(t *testing.T) {
510510
})
511511
require.Error(t, err)
512512
}
513+
514+
func TestBitbucketServerListReturnsRepositoryNotFoundError(t *testing.T) {
515+
mux := http.NewServeMux()
516+
server := httptest.NewServer(mux)
517+
defer server.Close()
518+
519+
path := "/rest/api/1.0/projects/nonexistent/repos/nonexistent/pull-requests?limit=100"
520+
521+
mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) {
522+
// Return 404 status to simulate repository not found
523+
w.WriteHeader(http.StatusNotFound)
524+
_, _ = w.Write([]byte(`{"message": "404 Project Not Found"}`))
525+
})
526+
527+
svc, err := NewBitbucketServiceNoAuth(t.Context(), server.URL, "nonexistent", "nonexistent", "", false, nil)
528+
require.NoError(t, err)
529+
530+
prs, err := svc.List(t.Context())
531+
532+
// Should return empty pull requests list
533+
assert.Empty(t, prs)
534+
535+
// Should return RepositoryNotFoundError
536+
require.Error(t, err)
537+
assert.True(t, IsRepositoryNotFoundError(err), "Expected RepositoryNotFoundError but got: %v", err)
538+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package pull_request
2+
3+
import "errors"
4+
5+
// RepositoryNotFoundError represents an error when a repository is not found by a pull request provider
6+
type RepositoryNotFoundError struct {
7+
causingError error
8+
}
9+
10+
func (e *RepositoryNotFoundError) Error() string {
11+
return e.causingError.Error()
12+
}
13+
14+
// NewRepositoryNotFoundError creates a new repository not found error
15+
func NewRepositoryNotFoundError(err error) error {
16+
return &RepositoryNotFoundError{causingError: err}
17+
}
18+
19+
// IsRepositoryNotFoundError checks if the given error is a repository not found error
20+
func IsRepositoryNotFoundError(err error) bool {
21+
var repoErr *RepositoryNotFoundError
22+
return errors.As(err, &repoErr)
23+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package pull_request
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestRepositoryNotFoundError(t *testing.T) {
12+
t.Run("NewRepositoryNotFoundError creates correct error type", func(t *testing.T) {
13+
originalErr := errors.New("repository does not exist")
14+
repoNotFoundErr := NewRepositoryNotFoundError(originalErr)
15+
16+
require.Error(t, repoNotFoundErr)
17+
assert.Equal(t, "repository does not exist", repoNotFoundErr.Error())
18+
})
19+
20+
t.Run("IsRepositoryNotFoundError identifies RepositoryNotFoundError", func(t *testing.T) {
21+
originalErr := errors.New("repository does not exist")
22+
repoNotFoundErr := NewRepositoryNotFoundError(originalErr)
23+
24+
assert.True(t, IsRepositoryNotFoundError(repoNotFoundErr))
25+
})
26+
27+
t.Run("IsRepositoryNotFoundError returns false for regular errors", func(t *testing.T) {
28+
regularErr := errors.New("some other error")
29+
30+
assert.False(t, IsRepositoryNotFoundError(regularErr))
31+
})
32+
33+
t.Run("IsRepositoryNotFoundError returns false for nil error", func(t *testing.T) {
34+
assert.False(t, IsRepositoryNotFoundError(nil))
35+
})
36+
37+
t.Run("IsRepositoryNotFoundError works with wrapped errors", func(t *testing.T) {
38+
originalErr := errors.New("repository does not exist")
39+
repoNotFoundErr := NewRepositoryNotFoundError(originalErr)
40+
wrappedErr := errors.New("wrapped: " + repoNotFoundErr.Error())
41+
42+
// Direct RepositoryNotFoundError should be identified
43+
assert.True(t, IsRepositoryNotFoundError(repoNotFoundErr))
44+
45+
// Wrapped string error should not be identified (this is expected behavior)
46+
assert.False(t, IsRepositoryNotFoundError(wrappedErr))
47+
})
48+
}

0 commit comments

Comments
 (0)