Skip to content

Commit 32f460b

Browse files
reggie-kppapapetrou76
authored andcommitted
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 <regina.voloshin@codefresh.io> * pull request functionality Signed-off-by: reggie-k <regina.voloshin@codefresh.io> * pull request functionality Signed-off-by: reggie-k <regina.voloshin@codefresh.io> --------- Signed-off-by: reggie-k <regina.voloshin@codefresh.io>
1 parent 20f7b80 commit 32f460b

26 files changed

Lines changed: 455 additions & 15 deletions

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

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
)
@@ -117,8 +118,17 @@ func (b *BitbucketCloudService) List(_ context.Context) ([]*PullRequest, error)
117118
RepoSlug: b.repositorySlug,
118119
}
119120

121+
pullRequests := []*PullRequest{}
122+
120123
response, err := b.client.Repositories.PullRequests.Gets(opts)
121124
if err != nil {
125+
// A standard Http 404 error is not returned for Bitbucket Cloud,
126+
// so checking the error message for a specific pattern
127+
if strings.Contains(err.Error(), "404 Not Found") {
128+
// return a custom error indicating that the repository is not found,
129+
// but also return the empty result since the decision to continue or not in this case is made by the caller
130+
return pullRequests, NewRepositoryNotFoundError(err)
131+
}
122132
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", b.owner, b.repositorySlug, err)
123133
}
124134

@@ -142,7 +152,6 @@ func (b *BitbucketCloudService) List(_ context.Context) ([]*PullRequest, error)
142152
return nil, fmt.Errorf("error unmarshalling json to type '[]BitbucketCloudPullRequest': %w", err)
143153
}
144154

145-
pullRequests := []*PullRequest{}
146155
for _, pull := range pulls {
147156
pullRequests = append(pullRequests, &PullRequest{
148157
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
@@ -492,3 +492,29 @@ func TestListPullRequestBranchMatchCloud(t *testing.T) {
492492
TargetBranch: "branch-200",
493493
}, *pullRequests[0])
494494
}
495+
496+
func TestBitbucketCloudListReturnsRepositoryNotFoundError(t *testing.T) {
497+
mux := http.NewServeMux()
498+
server := httptest.NewServer(mux)
499+
defer server.Close()
500+
501+
path := "/repositories/nonexistent/nonexistent/pullrequests/"
502+
503+
mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) {
504+
// Return 404 status to simulate repository not found
505+
w.WriteHeader(http.StatusNotFound)
506+
_, _ = w.Write([]byte(`{"message": "404 Project Not Found"}`))
507+
})
508+
509+
svc, err := NewBitbucketCloudServiceNoAuth(server.URL, "nonexistent", "nonexistent")
510+
require.NoError(t, err)
511+
512+
prs, err := svc.List(t.Context())
513+
514+
// Should return empty pull requests list
515+
assert.Empty(t, prs)
516+
517+
// Should return RepositoryNotFoundError
518+
require.Error(t, err)
519+
assert.True(t, IsRepositoryNotFoundError(err), "Expected RepositoryNotFoundError but got: %v", err)
520+
}

applicationset/services/pull_request/bitbucket_server.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ func (b *BitbucketService) List(_ context.Context) ([]*PullRequest, error) {
6666
for {
6767
response, err := b.client.DefaultApi.GetPullRequestsPage(b.projectKey, b.repositorySlug, paged)
6868
if err != nil {
69+
if response != nil && response.Response != nil && response.StatusCode == http.StatusNotFound {
70+
// return a custom error indicating that the repository is not found,
71+
// but also return the empty result since the decision to continue or not in this case is made by the caller
72+
return pullRequests, NewRepositoryNotFoundError(err)
73+
}
6974
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", b.projectKey, b.repositorySlug, err)
7075
}
7176
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)