diff --git a/applicationset/generators/pull_request.go b/applicationset/generators/pull_request.go index 456e83f5a6a1f..1c9826804dfc5 100644 --- a/applicationset/generators/pull_request.go +++ b/applicationset/generators/pull_request.go @@ -215,7 +215,7 @@ func (g *PullRequestGenerator) github(ctx context.Context, cfg *argoprojiov1alph if err != nil { return nil, fmt.Errorf("error getting GitHub App secret: %w", err) } - return pullrequest.NewGithubAppService(*auth, cfg.API, cfg.Owner, cfg.Repo, cfg.Labels, cfg.CachingEnabled) + return pullrequest.NewGithubAppService(*auth, cfg.API, cfg.Owner, cfg.Repo, cfg.Labels, g.SCMConfig.GitHubClientCache) } // always default to token, even if not set (public access) @@ -223,5 +223,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(ctx, token, cfg.API, cfg.Owner, cfg.Repo, cfg.Labels, g.SCMConfig.GitHubClientCache) } diff --git a/applicationset/generators/scm_provider.go b/applicationset/generators/scm_provider.go index 5f33b64475ec2..0136f809d89b8 100644 --- a/applicationset/generators/scm_provider.go +++ b/applicationset/generators/scm_provider.go @@ -7,7 +7,8 @@ import ( "strings" "time" - "github.com/gregjones/httpcache" + "github.com/aburan28/httpcache" + "github.com/aburan28/httpcache/lrucache" "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,7 +42,7 @@ type SCMConfig struct { tokenRefStrictMode bool } -func NewSCMConfig(scmRootCAPath string, allowedSCMProviders []string, enableSCMProviders bool, gitHubApps github_app_auth.Credentials, tokenRefStrictMode bool, cacheEnabled bool) SCMConfig { +func NewSCMConfig(scmRootCAPath string, allowedSCMProviders []string, enableSCMProviders bool, gitHubApps github_app_auth.Credentials, tokenRefStrictMode bool, enableGithubCache bool, githubCacheSize int) SCMConfig { scmConfig := SCMConfig{ scmRootCAPath: scmRootCAPath, allowedSCMProviders: allowedSCMProviders, @@ -50,8 +51,8 @@ func NewSCMConfig(scmRootCAPath string, allowedSCMProviders []string, enableSCMP tokenRefStrictMode: tokenRefStrictMode, } - if cacheEnabled { - scmConfig.GitHubClientCache = httpcache.NewMemoryCache() + if enableGithubCache { + scmConfig.GitHubClientCache = lrucache.NewLRUCache(githubCacheSize) } return scmConfig @@ -291,7 +292,7 @@ func (g *SCMProviderGenerator) githubProvider(ctx context.Context, github *argop github.Organization, github.API, github.AllBranches, - github.CachingEnabled, + g.GitHubClientCache, ) } @@ -299,5 +300,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, github.CachingEnabled) + return scm_provider.NewGithubProvider(ctx, github.Organization, token, github.API, github.AllBranches, g.GitHubClientCache) } diff --git a/applicationset/services/internal/github_app/client.go b/applicationset/services/internal/github_app/client.go index adb9296bde002..668f0a1506628 100644 --- a/applicationset/services/internal/github_app/client.go +++ b/applicationset/services/internal/github_app/client.go @@ -4,46 +4,46 @@ import ( "fmt" "net/http" + "github.com/aburan28/httpcache" "github.com/bradleyfalzon/ghinstallation/v2" "github.com/google/go-github/v63/github" - "github.com/gregjones/httpcache" "github.com/argoproj/argo-cd/v2/applicationset/services/github_app_auth" ) // Client builds a github client for the given app authentication. -func Client(g github_app_auth.Authentication, url string, cacheEnabled bool) (*github.Client, error) { - rt, err := ghinstallation.New(http.DefaultTransport, g.Id, g.InstallationId, []byte(g.PrivateKey)) - if err != nil { - return nil, fmt.Errorf("failed to create github app install: %w", err) - } - if url == "" { - url = g.EnterpriseBaseURL - } +func Client(g github_app_auth.Authentication, url string, cache httpcache.Cache) (*github.Client, error) { + var httpClient *http.Client + var err error var client *github.Client - // determine if the http client should use a cache - if url == "" { - httpClient := http.Client{Transport: rt} - - client = github.NewClient(&httpClient) - } else if cacheEnabled { - cache := httpcache.NewMemoryCache() - cachingHttpClient := http.Client{ - Transport: &httpcache.Transport{ - Cache: cache, - }, + if cache != nil { + tr := httpcache.NewTransport(cache) + at, err := ghinstallation.NewAppsTransport(tr, g.Id, []byte(g.PrivateKey)) + if err != nil { + return nil, fmt.Errorf("failed to create github app transport: %w", err) + } + + httpClient = &http.Client{ + Transport: at, } - client, err = github.NewClient(&cachingHttpClient).WithEnterpriseURLs(url, url) + } else { + rt, err := ghinstallation.New(http.DefaultTransport, g.Id, g.InstallationId, []byte(g.PrivateKey)) if err != nil { - return nil, fmt.Errorf("failed to create http cache client: %w", err) + return nil, fmt.Errorf("failed to create github app install: %w", err) + } + httpClient = &http.Client{ + Transport: rt, } + + } + + if url == "" { + client = github.NewClient(httpClient) } else { - rt.BaseURL = url - httpClient := http.Client{Transport: rt} - client, err = github.NewClient(&httpClient).WithEnterpriseURLs(url, url) + client, err = github.NewClient(httpClient).WithEnterpriseURLs(url, url) if err != nil { - return nil, fmt.Errorf("failed to create github enterprise client: %w", err) + return nil, fmt.Errorf("failed to create github client: %w", err) } } return client, nil diff --git a/applicationset/services/pull_request/github.go b/applicationset/services/pull_request/github.go index b63f2a9de6a8e..07c41b247c7a8 100644 --- a/applicationset/services/pull_request/github.go +++ b/applicationset/services/pull_request/github.go @@ -3,10 +3,11 @@ package pull_request import ( "context" "fmt" + "net/http" "os" + "github.com/aburan28/httpcache" "github.com/google/go-github/v63/github" - "golang.org/x/oauth2" ) type GithubService struct { @@ -18,21 +19,27 @@ 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(ctx context.Context, token, url, owner, repo string, labels []string, cache httpcache.Cache) (PullRequestService, error) { + // var ts oauth2.TokenSource // 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) + var httpClient *http.Client var client *github.Client + + if cache != nil { + httpClient = &http.Client{ + Transport: &httpcache.Transport{ + Cache: cache, + }, + } + } else { + httpClient = &http.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) diff --git a/applicationset/services/pull_request/github_app.go b/applicationset/services/pull_request/github_app.go index 6705b2b829fd0..1b2f5f65332a6 100644 --- a/applicationset/services/pull_request/github_app.go +++ b/applicationset/services/pull_request/github_app.go @@ -1,12 +1,13 @@ package pull_request import ( + "github.com/aburan28/httpcache" "github.com/argoproj/argo-cd/v2/applicationset/services/github_app_auth" "github.com/argoproj/argo-cd/v2/applicationset/services/internal/github_app" ) -func NewGithubAppService(g github_app_auth.Authentication, url, owner, repo string, labels []string, cacheEnabled bool) (PullRequestService, error) { - client, err := github_app.Client(g, url, cacheEnabled) +func NewGithubAppService(g github_app_auth.Authentication, url, owner, repo string, labels []string, cache httpcache.Cache) (PullRequestService, error) { + client, err := github_app.Client(g, url, cache) if err != nil { return nil, err } diff --git a/applicationset/services/pull_request/github_test.go b/applicationset/services/pull_request/github_test.go index d68bcd8f124de..e223c52cbf40f 100644 --- a/applicationset/services/pull_request/github_test.go +++ b/applicationset/services/pull_request/github_test.go @@ -1,6 +1,7 @@ package pull_request import ( + "context" "testing" "github.com/google/go-github/v63/github" @@ -11,6 +12,28 @@ func toPtr(s string) *string { return &s } +func TestGithubToken(t *testing.T) { + ctx := context.Background() + testCases := []struct { + name string + token string + }{ + { + name: "No token", + }, + { + name: "With token", + token: "test", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := NewGithubService(ctx, tc.token, "", "test", "test", []string{}) + require.NoError(t, err) + }) + } +} + func TestContainLabels(t *testing.T) { cases := []struct { Name string diff --git a/applicationset/services/scm_provider/github.go b/applicationset/services/scm_provider/github.go index 84a4b58b6454c..4108b8a112b09 100644 --- a/applicationset/services/scm_provider/github.go +++ b/applicationset/services/scm_provider/github.go @@ -6,8 +6,8 @@ import ( "net/http" "os" + "github.com/aburan28/httpcache" "github.com/google/go-github/v63/github" - "github.com/gregjones/httpcache" ) type contextKey struct{} @@ -26,7 +26,7 @@ type GithubProvider struct { var _ SCMProviderService = &GithubProvider{} -func NewGithubProvider(ctx context.Context, organization string, token string, url string, allBranches bool, cacheEnabled bool) (*GithubProvider, error) { +func NewGithubProvider(ctx context.Context, organization string, token string, url string, allBranches bool, cache httpcache.Cache) (*GithubProvider, error) { // Undocumented environment variable to set a default token, to be used in testing to dodge anonymous rate limits. var client *github.Client @@ -35,9 +35,7 @@ func NewGithubProvider(ctx context.Context, organization string, token string, u } httpClient := &http.Client{} - if cacheEnabled { - cache := httpcache.NewMemoryCache() - + if cache != nil { httpClient = &http.Client{ Transport: &httpcache.Transport{ Cache: cache, @@ -49,7 +47,6 @@ func NewGithubProvider(ctx context.Context, organization string, token string, u if token != "" { client.WithAuthToken(token) - } if url == "" { diff --git a/applicationset/services/scm_provider/github_app.go b/applicationset/services/scm_provider/github_app.go index 16db0ad010347..286ff22399313 100644 --- a/applicationset/services/scm_provider/github_app.go +++ b/applicationset/services/scm_provider/github_app.go @@ -1,12 +1,13 @@ package scm_provider import ( + "github.com/aburan28/httpcache" "github.com/argoproj/argo-cd/v2/applicationset/services/github_app_auth" "github.com/argoproj/argo-cd/v2/applicationset/services/internal/github_app" ) -func NewGithubAppProviderFor(g github_app_auth.Authentication, organization string, url string, allBranches bool, cacheEnabled bool) (*GithubProvider, error) { - client, err := github_app.Client(g, url, cacheEnabled) +func NewGithubAppProviderFor(g github_app_auth.Authentication, organization string, url string, allBranches bool, cache httpcache.Cache) (*GithubProvider, error) { + client, err := github_app.Client(g, url, cache) if err != nil { return nil, err } diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go index c68a88341687e..a47aa7c611074 100644 --- a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go +++ b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go @@ -54,7 +54,7 @@ func NewCommand() *cobra.Command { webhookAddr string enableLeaderElection bool enableGithubCache bool - githubCacheSize int64 + githubCacheSize int applicationSetNamespaces []string argocdRepoServer string policy string @@ -166,7 +166,7 @@ func NewCommand() *cobra.Command { argoSettingsMgr := argosettings.NewSettingsManager(ctx, k8sClient, namespace) argoCDDB := db.NewDB(namespace, argoSettingsMgr, k8sClient) - scmConfig := generators.NewSCMConfig(scmRootCAPath, allowedScmProviders, enableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), tokenRefStrictMode, enableGithubCache) + scmConfig := generators.NewSCMConfig(scmRootCAPath, allowedScmProviders, enableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), tokenRefStrictMode, enableGithubCache, githubCacheSize) tlsConfig := apiclient.TLSConfiguration{ DisableTLS: repoServerPlaintext, @@ -265,8 +265,7 @@ func NewCommand() *cobra.Command { command.Flags().IntVar(&webhookParallelism, "webhook-parallelism-limit", env.ParseNumFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_WEBHOOK_PARALLELISM_LIMIT", 50, 1, 1000), "Number of webhook requests processed concurrently") command.Flags().StringSliceVar(&metricsAplicationsetLabels, "metrics-applicationset-labels", []string{}, "List of Application labels that will be added to the argocd_applicationset_labels metric") command.Flags().BoolVar(&enableGithubCache, "enabled-github-cache", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLED_GITHUB_CACHE", false), "Enable caching for GitHub client") - command.Flags().Int64Var(&githubCacheSize, "github-cache-size", env.ParseInt64FromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_GITHUB_CACHE_SIZE", 0, 0, 0), "Max size of the cache for GitHub client in kb") - + command.Flags().IntVar(&githubCacheSize, "github-cache-size", env.ParseNumFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_GITHUB_CACHE_SIZE", 5000, 1000, 100000), "Max items to hold in the cache for github") return &command } diff --git a/go.mod b/go.mod index 0593bc5a9c8c6..ab325c55294dc 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/argoproj/argo-cd/v2 -go 1.22.0 +go 1.23.3 require ( code.gitea.io/sdk/gitea v0.19.0 @@ -8,6 +8,7 @@ require ( github.com/Masterminds/semver/v3 v3.3.1 github.com/Masterminds/sprig/v3 v3.3.0 github.com/TomOnTime/utfutil v0.0.0-20180511104225-09c41003ee1d + github.com/aburan28/httpcache v0.0.1 github.com/alicebob/miniredis/v2 v2.33.0 github.com/antonmedv/expr v1.15.1 github.com/argoproj/gitops-engine v0.7.1-0.20241107145828-847cfc9f8b20 @@ -43,7 +44,6 @@ require ( github.com/golang/protobuf v1.5.4 github.com/google/btree v1.1.3 github.com/google/go-cmp v0.6.0 - github.com/google/go-github/v35 v35.3.0 github.com/google/go-github/v63 v63.0.0 github.com/google/go-jsonnet v0.20.0 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 @@ -144,6 +144,7 @@ require ( github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.2.5 // indirect github.com/googleapis/gax-go/v2 v2.12.0 // indirect + github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect diff --git a/go.sum b/go.sum index d3bee45689dac..f5d3f36b18ff0 100644 --- a/go.sum +++ b/go.sum @@ -66,6 +66,8 @@ github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMx github.com/TomOnTime/utfutil v0.0.0-20180511104225-09c41003ee1d h1:WtAMR0fPCOfK7TPGZ8ZpLLY18HRvL7XJ3xcs0wnREgo= github.com/TomOnTime/utfutil v0.0.0-20180511104225-09c41003ee1d/go.mod h1:WML6KOYjeU8N6YyusMjj2qRvaPNUEvrQvaxuFcMRFJY= github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g= +github.com/aburan28/httpcache v0.0.1 h1:Ow0+bnxO0KYeF4CpU8E9RaASrfcfW0dw36uy/Dou7uE= +github.com/aburan28/httpcache v0.0.1/go.mod h1:9l7da/RFyyILtLd4HoDzCpk2Dl2F5AEvuHx5APrFd/M= github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia65gfNATL8TAiHDNxPzPdmEL5uirI2Uyuz6c= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= @@ -431,8 +433,6 @@ github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/go-github/v35 v35.3.0 h1:fU+WBzuukn0VssbayTT+Zo3/ESKX9JYWjbZTLOTEyho= -github.com/google/go-github/v35 v35.3.0/go.mod h1:yWB7uCcVWaUbUP74Aq3whuMySRMatyRmq5U9FTNlbio= github.com/google/go-github/v41 v41.0.0 h1:HseJrM2JFf2vfiZJ8anY2hqBjdfY1Vlj/K27ueww4gg= github.com/google/go-github/v41 v41.0.0/go.mod h1:XgmCA5H323A9rtgExdTcnDkcqp6S30AVACCBDOonIxg= github.com/google/go-github/v63 v63.0.0 h1:13xwK/wk9alSokujB9lJkuzdmQuVn2QCPeck76wR3nE= @@ -527,6 +527,8 @@ github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09 github.com/hashicorp/go.net v0.0.1/go.mod h1:hjKkEWcCURg++eb33jQU7oqQcI9XDCnUzHA0oac0k90= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= +github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= +github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= github.com/hashicorp/mdns v1.0.0/go.mod h1:tL+uN++7HEJ6SQLQ2/p+z2pH24WQKWjBPkE0mNTz8vQ= github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2pPBoIllUwCN7I= @@ -930,6 +932,8 @@ github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/tj/assert v0.0.3 h1:Df/BlaZ20mq6kuai7f5z2TvPFiwC3xaWJSDQNiIS3Rk= +github.com/tj/assert v0.0.3/go.mod h1:Ne6X72Q+TB1AteidzQncjw9PabbMp4PBMZ1k+vd1Pvk= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/ugorji/go v1.1.7 h1:/68gy2h+1mWMrwZFeD1kQialdSzAb432dtpeJ42ovdo= github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw= diff --git a/server/applicationset/applicationset.go b/server/applicationset/applicationset.go index febd734d0aa68..765e36615c285 100644 --- a/server/applicationset/applicationset.go +++ b/server/applicationset/applicationset.go @@ -265,7 +265,7 @@ func (s *Server) Create(ctx context.Context, q *applicationset.ApplicationSetCre func (s *Server) generateApplicationSetApps(ctx context.Context, logEntry *log.Entry, appset v1alpha1.ApplicationSet, namespace string) ([]v1alpha1.Application, error) { argoCDDB := s.db // TODO: this is a hack to get the github httpcache to load. - scmConfig := generators.NewSCMConfig(s.ScmRootCAPath, s.AllowedScmProviders, s.EnableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), true, true) + scmConfig := generators.NewSCMConfig(s.ScmRootCAPath, s.AllowedScmProviders, s.EnableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), true, true, 10000) getRepository := func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) { return s.db.GetRepository(ctx, url, project)