Skip to content

Commit

Permalink
Merge branch 'master' of https://github.com/argoproj/argo-cd into bb-…
Browse files Browse the repository at this point in the history
…bearer-token
  • Loading branch information
reggie-k committed Jan 16, 2025
2 parents a08b018 + 9b17495 commit ff1740d
Show file tree
Hide file tree
Showing 44 changed files with 585 additions and 88 deletions.
8 changes: 3 additions & 5 deletions applicationset/controllers/applicationset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,11 +524,9 @@ func (r *ApplicationSetReconciler) getMinRequeueAfter(applicationSetInfo *argov1
}

func ignoreNotAllowedNamespaces(namespaces []string) predicate.Predicate {
return predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
return utils.IsNamespaceAllowed(namespaces, e.Object.GetNamespace())
},
}
return predicate.NewPredicateFuncs(func(object client.Object) bool {
return utils.IsNamespaceAllowed(namespaces, object.GetNamespace())
})
}

// ignoreWhenAnnotationApplicationSetRefreshIsRemoved returns a predicate that ignores updates to ApplicationSet resources
Expand Down
83 changes: 83 additions & 0 deletions applicationset/controllers/applicationset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6727,3 +6727,86 @@ func TestIgnoreWhenAnnotationApplicationSetRefreshIsRemoved(t *testing.T) {
})
}
}

func TestIgnoreNotAllowedNamespaces(t *testing.T) {
tests := []struct {
name string
namespaces []string
objectNS string
expected bool
}{
{
name: "Namespace allowed",
namespaces: []string{"allowed-namespace"},
objectNS: "allowed-namespace",
expected: true,
},
{
name: "Namespace not allowed",
namespaces: []string{"allowed-namespace"},
objectNS: "not-allowed-namespace",
expected: false,
},
{
name: "Empty allowed namespaces",
namespaces: []string{},
objectNS: "any-namespace",
expected: false,
},
{
name: "Multiple allowed namespaces",
namespaces: []string{"allowed-namespace-1", "allowed-namespace-2"},
objectNS: "allowed-namespace-2",
expected: true,
},
{
name: "Namespace not in multiple allowed namespaces",
namespaces: []string{"allowed-namespace-1", "allowed-namespace-2"},
objectNS: "not-allowed-namespace",
expected: false,
},
{
name: "Namespace matched by glob pattern",
namespaces: []string{"allowed-namespace-*"},
objectNS: "allowed-namespace-1",
expected: true,
},
{
name: "Namespace matched by regex pattern",
namespaces: []string{"/^allowed-namespace-[^-]+$/"},
objectNS: "allowed-namespace-1",
expected: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
predicate := ignoreNotAllowedNamespaces(tt.namespaces)
object := &v1alpha1.ApplicationSet{
ObjectMeta: metav1.ObjectMeta{
Namespace: tt.objectNS,
},
}

t.Run(tt.name+":Create", func(t *testing.T) {
result := predicate.Create(event.CreateEvent{Object: object})
assert.Equal(t, tt.expected, result)
})

t.Run(tt.name+":Update", func(t *testing.T) {
result := predicate.Update(event.UpdateEvent{ObjectNew: object})
assert.Equal(t, tt.expected, result)
})

t.Run(tt.name+":Delete", func(t *testing.T) {
result := predicate.Delete(event.DeleteEvent{Object: object})
assert.Equal(t, tt.expected, result)
})

t.Run(tt.name+":Generic", func(t *testing.T) {
result := predicate.Generic(event.GenericEvent{Object: object})
assert.Equal(t, tt.expected, result)
})
})
}
}
2 changes: 1 addition & 1 deletion applicationset/generators/pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,5 @@ func (g *PullRequestGenerator) github(ctx context.Context, cfg *argoprojiov1alph
if err != nil {
return nil, fmt.Errorf("error fetching Secret token: %w", err)
}
return pullrequest.NewGithubService(ctx, token, cfg.API, cfg.Owner, cfg.Repo, cfg.Labels)
return pullrequest.NewGithubService(token, cfg.API, cfg.Owner, cfg.Repo, cfg.Labels)
}
2 changes: 1 addition & 1 deletion applicationset/generators/scm_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,5 +289,5 @@ func (g *SCMProviderGenerator) githubProvider(ctx context.Context, github *argop
if err != nil {
return nil, fmt.Errorf("error fetching Github token: %w", err)
}
return scm_provider.NewGithubProvider(ctx, github.Organization, token, github.API, github.AllBranches)
return scm_provider.NewGithubProvider(github.Organization, token, github.API, github.AllBranches)
}
14 changes: 4 additions & 10 deletions applicationset/services/pull_request/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package pull_request
import (
"context"
"fmt"
"net/http"
"os"

"github.com/google/go-github/v66/github"
"golang.org/x/oauth2"
)

type GithubService struct {
Expand All @@ -18,21 +18,15 @@ type GithubService struct {

var _ PullRequestService = (*GithubService)(nil)

func NewGithubService(ctx context.Context, token, url, owner, repo string, labels []string) (PullRequestService, error) {
var ts oauth2.TokenSource
func NewGithubService(token, url, owner, repo string, labels []string) (PullRequestService, error) {
// Undocumented environment variable to set a default token, to be used in testing to dodge anonymous rate limits.
if token == "" {
token = os.Getenv("GITHUB_TOKEN")
}
if token != "" {
ts = oauth2.StaticTokenSource(
&oauth2.Token{AccessToken: token},
)
}
httpClient := oauth2.NewClient(ctx, ts)
httpClient := &http.Client{}
var client *github.Client
if url == "" {
client = github.NewClient(httpClient)
client = github.NewClient(httpClient).WithAuthToken(token)
} else {
var err error
client, err = github.NewClient(httpClient).WithEnterpriseURLs(url, url)
Expand Down
13 changes: 3 additions & 10 deletions applicationset/services/scm_provider/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"os"

"github.com/google/go-github/v66/github"
"golang.org/x/oauth2"
)

type GithubProvider struct {
Expand All @@ -18,21 +17,15 @@ type GithubProvider struct {

var _ SCMProviderService = &GithubProvider{}

func NewGithubProvider(ctx context.Context, organization string, token string, url string, allBranches bool) (*GithubProvider, error) {
var ts oauth2.TokenSource
func NewGithubProvider(organization string, token string, url string, allBranches bool) (*GithubProvider, error) {
// Undocumented environment variable to set a default token, to be used in testing to dodge anonymous rate limits.
if token == "" {
token = os.Getenv("GITHUB_TOKEN")
}
if token != "" {
ts = oauth2.StaticTokenSource(
&oauth2.Token{AccessToken: token},
)
}
httpClient := oauth2.NewClient(ctx, ts)
httpClient := &http.Client{}
var client *github.Client
if url == "" {
client = github.NewClient(httpClient)
client = github.NewClient(httpClient).WithAuthToken(token)
} else {
var err error
client, err = github.NewClient(httpClient).WithEnterpriseURLs(url, url)
Expand Down
6 changes: 3 additions & 3 deletions applicationset/services/scm_provider/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func TestGithubListRepos(t *testing.T) {
defer ts.Close()
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
provider, _ := NewGithubProvider(context.Background(), "argoproj", "", ts.URL, c.allBranches)
provider, _ := NewGithubProvider("argoproj", "", ts.URL, c.allBranches)
rawRepos, err := ListRepos(context.Background(), provider, c.filters, c.proto)
if c.hasError {
require.Error(t, err)
Expand Down Expand Up @@ -273,7 +273,7 @@ func TestGithubHasPath(t *testing.T) {
githubMockHandler(t)(w, r)
}))
defer ts.Close()
host, _ := NewGithubProvider(context.Background(), "argoproj", "", ts.URL, false)
host, _ := NewGithubProvider("argoproj", "", ts.URL, false)
repo := &Repository{
Organization: "argoproj",
Repository: "argo-cd",
Expand All @@ -293,7 +293,7 @@ func TestGithubGetBranches(t *testing.T) {
githubMockHandler(t)(w, r)
}))
defer ts.Close()
host, _ := NewGithubProvider(context.Background(), "argoproj", "", ts.URL, false)
host, _ := NewGithubProvider("argoproj", "", ts.URL, false)
repo := &Repository{
Organization: "argoproj",
Repository: "argo-cd",
Expand Down
45 changes: 19 additions & 26 deletions applicationset/services/scm_provider/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,37 +131,30 @@ func (g *GitlabProvider) RepoHasPath(_ context.Context, repo *Repository, path s
if err != nil {
return false, err
}
directories := []string{
path,
pathpkg.Dir(path),

options := gitlab.ListTreeOptions{
Path: gitlab.Ptr(pathpkg.Dir(path)), // search parent folder
Ref: &repo.Branch,
}
for _, directory := range directories {
options := gitlab.ListTreeOptions{
Path: &directory,
Ref: &repo.Branch,
for {
treeNode, resp, err := g.client.Repositories.ListTree(p.ID, &options)
if err != nil {
return false, err
}
for {
treeNode, resp, err := g.client.Repositories.ListTree(p.ID, &options)
if err != nil {
return false, err
}
if path == directory {
if resp.TotalItems > 0 {
return true, nil
}
}
for i := range treeNode {
if treeNode[i].Path == path {
return true, nil
}
}
if resp.NextPage == 0 {
// no future pages
break

// search for presence of the requested file in the parent folder
for i := range treeNode {
if treeNode[i].Path == path {
return true, nil
}
options.Page = resp.NextPage
}
if resp.NextPage == 0 {
// no future pages
break
}
options.Page = resp.NextPage
}

return false, nil
}

Expand Down
11 changes: 11 additions & 0 deletions applicationset/services/scm_provider/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,12 @@ func gitlabMockHandler(t *testing.T) func(http.ResponseWriter, *http.Request) {
if err != nil {
t.Fail()
}
// Recent versions of the Gitlab API (v17.7+) return 404 not only when a file doesn't exist, but also
// when a path is to a file instead of a directory. Our code should not hit this path, because
// we should only send requests for parent directories. But we leave this handler in place
// to prevent regressions.
case "/api/v4/projects/27084533/repository/tree?path=argocd/filepath.yaml&ref=master":
w.WriteHeader(http.StatusNotFound)
case "/api/v4/projects/27084533/repository/branches/foo":
w.WriteHeader(http.StatusNotFound)
default:
Expand Down Expand Up @@ -1194,6 +1200,11 @@ func TestGitlabHasPath(t *testing.T) {
path: "argocd/notathing.yaml",
exists: false,
},
{
name: "send a file path",
path: "argocd/filepath.yaml",
exists: false,
},
}

for _, c := range cases {
Expand Down
2 changes: 2 additions & 0 deletions common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ const (
EnvLogLevel = "ARGOCD_LOG_LEVEL"
// EnvLogFormatEnableFullTimestamp enables the FullTimestamp option in logs
EnvLogFormatEnableFullTimestamp = "ARGOCD_LOG_FORMAT_ENABLE_FULL_TIMESTAMP"
// EnvLogFormatTimestamp is the timestamp format used in logs
EnvLogFormatTimestamp = "ARGOCD_LOG_FORMAT_TIMESTAMP"
// EnvMaxCookieNumber max number of chunks a cookie can be broken into
EnvMaxCookieNumber = "ARGOCD_MAX_COOKIE_NUMBER"
// EnvPluginSockFilePath allows to override the pluginSockFilePath for repo server and cmp server
Expand Down
2 changes: 1 addition & 1 deletion controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ func (ctrl *ApplicationController) requestAppRefresh(appName string, compareWith
key := ctrl.toAppKey(appName)

if compareWith != nil && after != nil {
ctrl.appComparisonTypeRefreshQueue.AddAfter(fmt.Sprintf("%s/%d", key, compareWith), *after)
ctrl.appComparisonTypeRefreshQueue.AddAfter(fmt.Sprintf("%s/%d", key, *compareWith), *after)
} else {
if compareWith != nil {
ctrl.refreshRequestedAppsMutex.Lock()
Expand Down
19 changes: 19 additions & 0 deletions controller/appcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,25 @@ func TestNeedRefreshAppStatus(t *testing.T) {
assert.Equal(t, CompareWithRecent, compareWith)
})

t.Run("requesting refresh with delay gives correct compression level", func(t *testing.T) {
needRefresh, _, _ := ctrl.needRefreshAppStatus(app, 1*time.Hour, 2*time.Hour)
assert.False(t, needRefresh)

// use a one-off controller so other tests don't have a manual refresh request
ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil)

// refresh app with a non-nil delay
// use zero-second delay to test the add later logic without waiting in the test
delay := time.Duration(0)
ctrl.requestAppRefresh(app.Name, CompareWithRecent.Pointer(), &delay)

ctrl.processAppComparisonTypeQueueItem()
needRefresh, refreshType, compareWith := ctrl.needRefreshAppStatus(app, 1*time.Hour, 2*time.Hour)
assert.True(t, needRefresh)
assert.Equal(t, v1alpha1.RefreshTypeNormal, refreshType)
assert.Equal(t, CompareWithRecent, compareWith)
})

t.Run("refresh application which status is not reconciled using latest commit", func(t *testing.T) {
app := app.DeepCopy()
needRefresh, _, _ := ctrl.needRefreshAppStatus(app, 1*time.Hour, 2*time.Hour)
Expand Down
5 changes: 5 additions & 0 deletions docs/operator-manual/argocd-cmd-params-cm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ data:
# Feature state: Beta
application.namespaces: ns1, ns2, ns3

# Set the logging timestamp format. The default is "" which means "2006-01-02T15:04:05Z07:00" (RFC3339).
# See https://pkg.go.dev/time#pkg-constants for more options.
# This option is used for all components.
log.format.timestamp: ""

## Controller Properties
# Repo server RPC call timeout seconds.
controller.repo.server.timeout.seconds: "60"
Expand Down
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/argoproj/gitops-engine v0.7.1-0.20241216155226-54992bf42431
github.com/argoproj/notifications-engine v0.4.1-0.20241007194503-2fef5c9049fd
github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1
github.com/aws/aws-sdk-go v1.55.5
github.com/aws/aws-sdk-go v1.55.6
github.com/bmatcuk/doublestar/v4 v4.8.0
github.com/bombsimon/logrusr/v4 v4.1.0
github.com/bradleyfalzon/ghinstallation/v2 v2.13.0
Expand All @@ -23,7 +23,7 @@ require (
github.com/cespare/xxhash/v2 v2.3.0
github.com/chainguard-dev/git-urls v1.0.2
github.com/coreos/go-oidc/v3 v3.12.0
github.com/cyphar/filepath-securejoin v0.3.6
github.com/cyphar/filepath-securejoin v0.4.0
github.com/dustin/go-humanize v1.0.1
github.com/evanphx/json-patch v5.9.0+incompatible
github.com/expr-lang/expr v1.16.9
Expand Down Expand Up @@ -91,7 +91,7 @@ require (
golang.org/x/term v0.28.0
golang.org/x/time v0.9.0
google.golang.org/genproto/googleapis/api v0.0.0-20241104194629-dd2ea8efbc28
google.golang.org/grpc v1.69.2
google.golang.org/grpc v1.69.4
google.golang.org/protobuf v1.36.2
gopkg.in/yaml.v2 v2.4.0
gopkg.in/yaml.v3 v3.0.1
Expand Down
Loading

0 comments on commit ff1740d

Please sign in to comment.