Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for httpcaching of github requests #21309

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft
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
4 changes: 2 additions & 2 deletions applicationset/generators/pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,13 @@ 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)
token, err := utils.GetSecretRef(ctx, g.client, cfg.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode)
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)
}
17 changes: 14 additions & 3 deletions applicationset/generators/scm_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -282,12 +292,13 @@ func (g *SCMProviderGenerator) githubProvider(ctx context.Context, github *argop
github.Organization,
github.API,
github.AllBranches,
g.GitHubClientCache,
)
}

token, err := utils.GetSecretRef(ctx, g.client, github.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode)
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)
}
43 changes: 29 additions & 14 deletions applicationset/services/internal/github_app/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,46 @@ import (
"fmt"
"net/http"

"github.com/aburan28/httpcache"
"github.com/bradleyfalzon/ghinstallation/v2"
"github.com/google/go-github/v66/github"

"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) (*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
Expand Down
27 changes: 17 additions & 10 deletions applicationset/services/pull_request/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions applicationset/services/pull_request/github_app.go
Original file line number Diff line number Diff line change
@@ -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
}
Expand Down
23 changes: 23 additions & 0 deletions applicationset/services/pull_request/github_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package pull_request

import (
"context"
"testing"

"github.com/google/go-github/v66/github"
Expand All @@ -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
Expand Down
34 changes: 26 additions & 8 deletions applicationset/services/scm_provider/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions applicationset/services/scm_provider/github_app.go
Original file line number Diff line number Diff line change
@@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ func NewCommand() *cobra.Command {
probeBindAddr string
webhookAddr string
enableLeaderElection bool
enableGithubCache bool
githubCacheSize int
applicationSetNamespaces []string
argocdRepoServer string
policy string
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 3 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
Loading
Loading