Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
12 changes: 11 additions & 1 deletion applicationset/generators/pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/gosimple/slug"
log "github.com/sirupsen/logrus"

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

func (g *PullRequestGenerator) GetContinueOnRepoNotFoundError(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) bool {
return appSetGenerator.PullRequest.ContinueOnRepoNotFoundError
}

func (g *PullRequestGenerator) GetTemplate(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) *argoprojiov1alpha1.ApplicationSetTemplate {
return &appSetGenerator.PullRequest.Template
}
Expand All @@ -69,10 +74,15 @@ func (g *PullRequestGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha
}

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

// In order to follow the DNS label standard as defined in RFC 1123,
// we need to limit the 'branch' to 50 to give room to append/suffix-ing it
Expand Down
38 changes: 32 additions & 6 deletions applicationset/generators/pull_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ import (
func TestPullRequestGithubGenerateParams(t *testing.T) {
ctx := t.Context()
cases := []struct {
selectFunc func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error)
values map[string]string
expected []map[string]any
expectedErr error
applicationSet argoprojiov1alpha1.ApplicationSet
selectFunc func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error)
values map[string]string
expected []map[string]any
expectedErr error
applicationSet argoprojiov1alpha1.ApplicationSet
continueOnRepoNotFoundError bool
}{
{
selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
Expand Down Expand Up @@ -171,6 +172,30 @@ func TestPullRequestGithubGenerateParams(t *testing.T) {
expected: nil,
expectedErr: errors.New("error listing repos: fake error"),
},
{
selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
return pullrequest.NewFakeService(
ctx,
nil,
pullrequest.NewRepositoryNotFoundError(errors.New("repository not found")),
)
},
expected: []map[string]any{},
expectedErr: nil,
continueOnRepoNotFoundError: true,
},
{
selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
return pullrequest.NewFakeService(
ctx,
nil,
pullrequest.NewRepositoryNotFoundError(errors.New("repository not found")),
)
},
expected: nil,
expectedErr: errors.New("error listing repos: repository not found"),
continueOnRepoNotFoundError: false,
},
{
selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
return pullrequest.NewFakeService(
Expand Down Expand Up @@ -260,7 +285,8 @@ func TestPullRequestGithubGenerateParams(t *testing.T) {
}
generatorConfig := argoprojiov1alpha1.ApplicationSetGenerator{
PullRequest: &argoprojiov1alpha1.PullRequestGenerator{
Values: c.values,
Values: c.values,
ContinueOnRepoNotFoundError: c.continueOnRepoNotFoundError,
},
}

Expand Down
19 changes: 15 additions & 4 deletions applicationset/services/pull_request/azure_devops.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import (
"github.com/microsoft/azure-devops-go-api/azuredevops/git"
)

const AZURE_DEVOPS_DEFAULT_URL = "https://dev.azure.com"
const (
AZURE_DEVOPS_DEFAULT_URL = "https://dev.azure.com"
AZURE_DEVOPS_PROJECT_NOT_FOUND_ERROR = "The following project does not exist"
)

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

pullRequests := []*PullRequest{}

azurePullRequests, err := client.GetPullRequestsByProject(ctx, args)
if err != nil {
return nil, fmt.Errorf("failed to get pull requests by project: %w", err)
// A standard Http 404 error is not returned for Azure DevOps,
// so checking the error message for a specific pattern.
// NOTE: Since the repos are filtered later, only existence of the project
// is relevant for AzureDevOps
if strings.Contains(err.Error(), AZURE_DEVOPS_PROJECT_NOT_FOUND_ERROR) {
// return a custom error indicating that the repository is not found,
// but also return the empty result since the decision to continue or not in this case is made by the caller
return pullRequests, NewRepositoryNotFoundError(err)
}
}

pullRequests := []*PullRequest{}

for _, pr := range *azurePullRequests {
if pr.Repository == nil ||
pr.Repository.Name == nil ||
Expand Down
34 changes: 34 additions & 0 deletions applicationset/services/pull_request/azure_devops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package pull_request

import (
"context"
"errors"
"testing"

"github.com/microsoft/azure-devops-go-api/azuredevops/webapi"
Expand Down Expand Up @@ -236,3 +237,36 @@ func TestBuildURL(t *testing.T) {
})
}
}

func TestAzureDevOpsListReturnsRepositoryNotFoundError(t *testing.T) {
args := git.GetPullRequestsByProjectArgs{
Project: createStringPtr("nonexistent"),
SearchCriteria: &git.GitPullRequestSearchCriteria{},
}

pullRequestMock := []git.GitPullRequest{}

gitClientMock := azureMock.Client{}
clientFactoryMock := &AzureClientFactoryMock{mock: &mock.Mock{}}
clientFactoryMock.mock.On("GetClient", mock.Anything).Return(&gitClientMock, nil)

// Mock the GetPullRequestsByProject to return an error containing "404"
gitClientMock.On("GetPullRequestsByProject", t.Context(), args).Return(&pullRequestMock,
errors.New("The following project does not exist:"))

provider := AzureDevOpsService{
clientFactory: clientFactoryMock,
project: "nonexistent",
repo: "nonexistent",
labels: nil,
}

prs, err := provider.List(t.Context())

// Should return empty pull requests list
assert.Empty(t, prs)

// Should return RepositoryNotFoundError
require.Error(t, err)
assert.True(t, IsRepositoryNotFoundError(err), "Expected RepositoryNotFoundError but got: %v", err)
}
11 changes: 10 additions & 1 deletion applicationset/services/pull_request/bitbucket_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"net/url"
"strings"

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

pullRequests := []*PullRequest{}

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

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

pullRequests := []*PullRequest{}
for _, pull := range pulls {
pullRequests = append(pullRequests, &PullRequest{
Number: pull.ID,
Expand Down
26 changes: 26 additions & 0 deletions applicationset/services/pull_request/bitbucket_cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,3 +455,29 @@ func TestListPullRequestBranchMatchCloud(t *testing.T) {
})
require.Error(t, err)
}

func TestBitbucketCloudListReturnsRepositoryNotFoundError(t *testing.T) {
mux := http.NewServeMux()
server := httptest.NewServer(mux)
defer server.Close()

path := "/repositories/nonexistent/nonexistent/pullrequests/"

mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) {
// Return 404 status to simulate repository not found
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "404 Project Not Found"}`))
})

svc, err := NewBitbucketCloudServiceNoAuth(server.URL, "nonexistent", "nonexistent")
require.NoError(t, err)

prs, err := svc.List(t.Context())

// Should return empty pull requests list
assert.Empty(t, prs)

// Should return RepositoryNotFoundError
require.Error(t, err)
assert.True(t, IsRepositoryNotFoundError(err), "Expected RepositoryNotFoundError but got: %v", err)
}
5 changes: 5 additions & 0 deletions applicationset/services/pull_request/bitbucket_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ func (b *BitbucketService) List(_ context.Context) ([]*PullRequest, error) {
for {
response, err := b.client.DefaultApi.GetPullRequestsPage(b.projectKey, b.repositorySlug, paged)
if err != nil {
if response != nil && response.Response != nil && response.StatusCode == http.StatusNotFound {
// return a custom error indicating that the repository is not found,
// but also return the empty result since the decision to continue or not in this case is made by the caller
return pullRequests, NewRepositoryNotFoundError(err)
}
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", b.projectKey, b.repositorySlug, err)
}
pulls, err := bitbucketv1.GetPullRequestsResponse(response)
Expand Down
26 changes: 26 additions & 0 deletions applicationset/services/pull_request/bitbucket_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,3 +510,29 @@ func TestListPullRequestBranchMatch(t *testing.T) {
})
require.Error(t, err)
}

func TestBitbucketServerListReturnsRepositoryNotFoundError(t *testing.T) {
mux := http.NewServeMux()
server := httptest.NewServer(mux)
defer server.Close()

path := "/rest/api/1.0/projects/nonexistent/repos/nonexistent/pull-requests?limit=100"

mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) {
// Return 404 status to simulate repository not found
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "404 Project Not Found"}`))
})

svc, err := NewBitbucketServiceNoAuth(t.Context(), server.URL, "nonexistent", "nonexistent", "", false, nil)
require.NoError(t, err)

prs, err := svc.List(t.Context())

// Should return empty pull requests list
assert.Empty(t, prs)

// Should return RepositoryNotFoundError
require.Error(t, err)
assert.True(t, IsRepositoryNotFoundError(err), "Expected RepositoryNotFoundError but got: %v", err)
}
23 changes: 23 additions & 0 deletions applicationset/services/pull_request/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package pull_request

import "errors"

// RepositoryNotFoundError represents an error when a repository is not found by a pull request provider
type RepositoryNotFoundError struct {
causingError error
}

func (e *RepositoryNotFoundError) Error() string {
return e.causingError.Error()
}

// NewRepositoryNotFoundError creates a new repository not found error
func NewRepositoryNotFoundError(err error) error {
return &RepositoryNotFoundError{causingError: err}
}

// IsRepositoryNotFoundError checks if the given error is a repository not found error
func IsRepositoryNotFoundError(err error) bool {
var repoErr *RepositoryNotFoundError
return errors.As(err, &repoErr)
}
48 changes: 48 additions & 0 deletions applicationset/services/pull_request/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package pull_request

import (
"errors"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestRepositoryNotFoundError(t *testing.T) {
t.Run("NewRepositoryNotFoundError creates correct error type", func(t *testing.T) {
originalErr := errors.New("repository does not exist")
repoNotFoundErr := NewRepositoryNotFoundError(originalErr)

require.Error(t, repoNotFoundErr)
assert.Equal(t, "repository does not exist", repoNotFoundErr.Error())
})

t.Run("IsRepositoryNotFoundError identifies RepositoryNotFoundError", func(t *testing.T) {
originalErr := errors.New("repository does not exist")
repoNotFoundErr := NewRepositoryNotFoundError(originalErr)

assert.True(t, IsRepositoryNotFoundError(repoNotFoundErr))
})

t.Run("IsRepositoryNotFoundError returns false for regular errors", func(t *testing.T) {
regularErr := errors.New("some other error")

assert.False(t, IsRepositoryNotFoundError(regularErr))
})

t.Run("IsRepositoryNotFoundError returns false for nil error", func(t *testing.T) {
assert.False(t, IsRepositoryNotFoundError(nil))
})

t.Run("IsRepositoryNotFoundError works with wrapped errors", func(t *testing.T) {
originalErr := errors.New("repository does not exist")
repoNotFoundErr := NewRepositoryNotFoundError(originalErr)
wrappedErr := errors.New("wrapped: " + repoNotFoundErr.Error())

// Direct RepositoryNotFoundError should be identified
assert.True(t, IsRepositoryNotFoundError(repoNotFoundErr))

// Wrapped string error should not be identified (this is expected behavior)
assert.False(t, IsRepositoryNotFoundError(wrappedErr))
})
}
9 changes: 7 additions & 2 deletions applicationset/services/pull_request/gitea.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,16 @@ func (g *GiteaService) List(ctx context.Context) ([]*PullRequest, error) {
State: gitea.StateOpen,
}
g.client.SetContext(ctx)
prs, _, err := g.client.ListRepoPullRequests(g.owner, g.repo, opts)
list := []*PullRequest{}
prs, resp, err := g.client.ListRepoPullRequests(g.owner, g.repo, opts)
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
// return a custom error indicating that the repository is not found,
// but also returning the empty result since the decision to continue or not in this case is made by the caller
return list, NewRepositoryNotFoundError(err)
}
return nil, err
}
list := []*PullRequest{}
for _, pr := range prs {
list = append(list, &PullRequest{
Number: int(pr.Index),
Expand Down
Loading
Loading