diff --git a/applicationset/generators/pull_request.go b/applicationset/generators/pull_request.go index f0c2bfaacfcf5..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) + 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 417b682e50511..0136f809d89b8 100644 --- a/applicationset/generators/scm_provider.go +++ b/applicationset/generators/scm_provider.go @@ -7,6 +7,9 @@ import ( "strings" "time" + "github.com/aburan28/httpcache" + "github.com/aburan28/httpcache/lrucache" + "sigs.k8s.io/controller-runtime/pkg/client" log "github.com/sirupsen/logrus" @@ -35,17 +38,24 @@ type SCMConfig struct { allowedSCMProviders []string enableSCMProviders bool GitHubApps github_app_auth.Credentials + GitHubClientCache httpcache.Cache tokenRefStrictMode bool } -func NewSCMConfig(scmRootCAPath string, allowedSCMProviders []string, enableSCMProviders bool, gitHubApps github_app_auth.Credentials, tokenRefStrictMode bool) SCMConfig { - return 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, enableSCMProviders: enableSCMProviders, GitHubApps: gitHubApps, tokenRefStrictMode: tokenRefStrictMode, } + + if enableGithubCache { + scmConfig.GitHubClientCache = lrucache.NewLRUCache(githubCacheSize) + } + + return scmConfig } func NewSCMProviderGenerator(client client.Client, scmConfig SCMConfig) Generator { @@ -282,6 +292,7 @@ func (g *SCMProviderGenerator) githubProvider(ctx context.Context, github *argop github.Organization, github.API, github.AllBranches, + g.GitHubClientCache, ) } @@ -289,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) + 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 7cdfd51554bdf..5980802341dee 100644 --- a/applicationset/services/internal/github_app/client.go +++ b/applicationset/services/internal/github_app/client.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" + "github.com/aburan28/httpcache" "github.com/bradleyfalzon/ghinstallation/v2" "github.com/google/go-github/v66/github" @@ -11,24 +12,38 @@ import ( ) // Client builds a github client for the given app authentication. -func Client(g github_app_auth.Authentication, url string) (*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 + + 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, + } + } else { + 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) + } + httpClient = &http.Client{ + Transport: rt, + } + + } + if url == "" { - httpClient := http.Client{Transport: rt} - client = github.NewClient(&httpClient) + 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 eaee02ffb171d..5425ad43b6b78 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/v66/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 8879a777ad277..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) (PullRequestService, error) { - client, err := github_app.Client(g, url) +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 fb35ad20b0b99..203f2a42e1c74 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/v66/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 2e2f2a7faf3d4..0b43386ada670 100644 --- a/applicationset/services/scm_provider/github.go +++ b/applicationset/services/scm_provider/github.go @@ -6,10 +6,18 @@ import ( "net/http" "os" + "github.com/aburan28/httpcache" "github.com/google/go-github/v66/github" - "golang.org/x/oauth2" ) +type contextKey struct{} + +var cacheContextKey = contextKey{} + +func ContextWithGithubCache(ctx context.Context, cache httpcache.Cache) context.Context { + return context.WithValue(ctx, cacheContextKey, cache) +} + type GithubProvider struct { client *github.Client organization string @@ -18,19 +26,29 @@ 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(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 + if token == "" { token = os.Getenv("GITHUB_TOKEN") } + + httpClient := &http.Client{} + if cache != nil { + httpClient = &http.Client{ + Transport: &httpcache.Transport{ + Cache: cache, + }, + } + } else { + httpClient = &http.Client{} + } + if token != "" { - ts = oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: token}, - ) + client.WithAuthToken(token) } - httpClient := oauth2.NewClient(ctx, ts) - var client *github.Client + if url == "" { client = github.NewClient(httpClient) } else { diff --git a/applicationset/services/scm_provider/github_app.go b/applicationset/services/scm_provider/github_app.go index 5429ed48ee8ab..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) (*GithubProvider, error) { - client, err := github_app.Client(g, url) +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 abc4d55035f72..efa7789081824 100644 --- a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go +++ b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go @@ -54,6 +54,8 @@ func NewCommand() *cobra.Command { probeBindAddr string webhookAddr string enableLeaderElection bool + enableGithubCache bool + githubCacheSize int applicationSetNamespaces []string argocdRepoServer string policy string @@ -172,7 +174,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) + scmConfig := generators.NewSCMConfig(scmRootCAPath, allowedScmProviders, enableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), tokenRefStrictMode, enableGithubCache, githubCacheSize) tlsConfig := apiclient.TLSConfiguration{ DisableTLS: repoServerPlaintext, @@ -270,6 +272,8 @@ func NewCommand() *cobra.Command { command.Flags().StringSliceVar(&globalPreservedLabels, "preserved-labels", env.StringsFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_GLOBAL_PRESERVED_LABELS", []string{}, ","), "Sets global preserved field values for labels") 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().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 9fc8ab7300ed2..7168685bc255c 100644 --- a/go.mod +++ b/go.mod @@ -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.34.0 github.com/antonmedv/expr v1.15.1 github.com/argoproj/gitops-engine v0.7.1-0.20241216155226-54992bf42431 @@ -24,7 +25,7 @@ require ( github.com/coreos/go-oidc/v3 v3.11.0 github.com/cyphar/filepath-securejoin v0.3.6 github.com/dustin/go-humanize v1.0.1 - github.com/evanphx/json-patch v5.9.0+incompatible + github.com/evanphx/json-patch v5.9.0+incompatibgolanle github.com/expr-lang/expr v1.16.9 github.com/felixge/httpsnoop v1.0.4 github.com/fsnotify/fsnotify v1.8.0 @@ -142,6 +143,7 @@ require ( github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect github.com/googleapis/gax-go/v2 v2.12.3 // 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 @@ -216,7 +218,6 @@ require ( github.com/google/gofuzz v1.2.0 // indirect github.com/gosimple/unidecode v1.0.1 // indirect github.com/gregdel/pushover v1.2.1 // indirect - github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.23.0 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-version v1.6.0 // indirect diff --git a/go.sum b/go.sum index db76aad8b25b3..db34dbd96d4f6 100644 --- a/go.sum +++ b/go.sum @@ -70,6 +70,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= @@ -527,6 +529,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= @@ -931,6 +935,8 @@ github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= 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/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= diff --git a/server/applicationset/applicationset.go b/server/applicationset/applicationset.go index b5288c71c1509..765e36615c285 100644 --- a/server/applicationset/applicationset.go +++ b/server/applicationset/applicationset.go @@ -264,8 +264,8 @@ 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 - - scmConfig := generators.NewSCMConfig(s.ScmRootCAPath, s.AllowedScmProviders, s.EnableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), true) + // 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, 10000) getRepository := func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) { return s.db.GetRepository(ctx, url, project)