Skip to content

Commit 7ac057c

Browse files
jd-hatzenbuhlerjd-hatzenbuhler
committed
feat: add support for httpcaching of github requests
Signed-off-by: Jean-Damien HATZENBUHLER <jean-damien.hatzenbuhler@contentsquare.com>
1 parent 08390e2 commit 7ac057c

File tree

14 files changed

+272
-8
lines changed

14 files changed

+272
-8
lines changed

‎applicationset/controllers/requeue_after_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func TestRequeueAfter(t *testing.T) {
5656
},
5757
}
5858
fakeDynClient := dynfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), gvrToListKind, duckType)
59-
scmConfig := generators.NewSCMConfig("", []string{""}, true, true, nil, true)
59+
scmConfig := generators.NewSCMConfig("", []string{""}, true, true, false, 100, nil, true)
6060
terminalGenerators := map[string]generators.Generator{
6161
"List": generators.NewListGenerator(),
6262
"Clusters": generators.NewClusterGenerator(ctx, k8sClient, appClientset, "argocd"),

‎applicationset/generators/pull_request_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) {
398398
"gitea.myorg.com",
399399
"bitbucket.myorg.com",
400400
"azuredevops.myorg.com",
401-
}, true, true, nil, true))
401+
}, true, true, false, 100, nil, true))
402402

403403
applicationSetInfo := argoprojiov1alpha1.ApplicationSet{
404404
ObjectMeta: metav1.ObjectMeta{
@@ -421,7 +421,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) {
421421
}
422422

423423
func TestSCMProviderDisabled_PRGenerator(t *testing.T) {
424-
generator := NewPullRequestGenerator(nil, NewSCMConfig("", []string{}, false, true, nil, true))
424+
generator := NewPullRequestGenerator(nil, NewSCMConfig("", []string{}, false, true, false, 100, nil, true))
425425

426426
applicationSetInfo := argoprojiov1alpha1.ApplicationSet{
427427
ObjectMeta: metav1.ObjectMeta{

‎applicationset/generators/scm_provider.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,20 @@ type SCMConfig struct {
3737
allowedSCMProviders []string
3838
enableSCMProviders bool
3939
enableGitHubAPIMetrics bool
40+
enableGitHubCache bool
41+
githubCacheSize int
4042
GitHubApps github_app_auth.Credentials
4143
tokenRefStrictMode bool
4244
}
4345

44-
func NewSCMConfig(scmRootCAPath string, allowedSCMProviders []string, enableSCMProviders bool, enableGitHubAPIMetrics bool, gitHubApps github_app_auth.Credentials, tokenRefStrictMode bool) SCMConfig {
46+
func NewSCMConfig(scmRootCAPath string, allowedSCMProviders []string, enableSCMProviders bool, enableGitHubAPIMetrics bool, enableGitHubCache bool, githubCacheSize int, gitHubApps github_app_auth.Credentials, tokenRefStrictMode bool) SCMConfig {
4547
return SCMConfig{
4648
scmRootCAPath: scmRootCAPath,
4749
allowedSCMProviders: allowedSCMProviders,
4850
enableSCMProviders: enableSCMProviders,
4951
enableGitHubAPIMetrics: enableGitHubAPIMetrics,
52+
enableGitHubCache: enableGitHubCache,
53+
githubCacheSize: githubCacheSize,
5054
GitHubApps: gitHubApps,
5155
tokenRefStrictMode: tokenRefStrictMode,
5256
}
@@ -289,13 +293,22 @@ func (g *SCMProviderGenerator) githubProvider(ctx context.Context, github *argop
289293
httpClient = services.NewGitHubMetricsClient(metricsCtx)
290294
}
291295

296+
if g.enableGitHubCache {
297+
storage := services.NewLRUSStorage(g.githubCacheSize)
298+
if g.enableGitHubAPIMetrics {
299+
httpClient = services.NewGitHubCache(storage, httpClient.Transport)
300+
} else {
301+
httpClient = services.NewGitHubCache(storage, nil)
302+
}
303+
}
304+
292305
if github.AppSecretName != "" {
293306
auth, err := g.GitHubApps.GetAuthSecret(ctx, github.AppSecretName)
294307
if err != nil {
295308
return nil, fmt.Errorf("error fetching Github app secret: %w", err)
296309
}
297310

298-
if g.enableGitHubAPIMetrics {
311+
if g.enableGitHubAPIMetrics || g.enableGitHubCache {
299312
return scm_provider.NewGithubAppProviderFor(ctx, *auth, github.Organization, github.API, github.AllBranches, httpClient)
300313
}
301314
return scm_provider.NewGithubAppProviderFor(ctx, *auth, github.Organization, github.API, github.AllBranches)
@@ -306,7 +319,7 @@ func (g *SCMProviderGenerator) githubProvider(ctx context.Context, github *argop
306319
return nil, fmt.Errorf("error fetching Github token: %w", err)
307320
}
308321

309-
if g.enableGitHubAPIMetrics {
322+
if g.enableGitHubAPIMetrics || g.enableGitHubCache {
310323
return scm_provider.NewGithubProvider(github.Organization, token, github.API, github.AllBranches, httpClient)
311324
}
312325
return scm_provider.NewGithubProvider(github.Organization, token, github.API, github.AllBranches)

‎applicationset/services/github_cache.go‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package services
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"fmt"
7+
"io"
8+
"net/http"
9+
"net/url"
10+
11+
ghtransport "github.com/bored-engineer/github-conditional-http-transport"
12+
log "github.com/sirupsen/logrus"
13+
"k8s.io/utils/lru"
14+
)
15+
16+
type cachedResponse struct {
17+
Response *http.Response
18+
Body []byte
19+
}
20+
21+
type Storage struct {
22+
lruMap *lru.Cache
23+
}
24+
25+
func (s Storage) Get(_ context.Context, u *url.URL) (*http.Response, error) {
26+
body, ok := s.lruMap.Get(u.String())
27+
if !ok {
28+
return nil, nil
29+
}
30+
bodyCached, valid := body.(cachedResponse)
31+
if !valid {
32+
return nil, nil
33+
}
34+
resp := *bodyCached.Response
35+
resp.Body = io.NopCloser(bytes.NewReader(bodyCached.Body))
36+
return &resp, nil
37+
}
38+
39+
func (s Storage) Put(_ context.Context, u *url.URL, resp *http.Response) error {
40+
body, err := io.ReadAll(resp.Body)
41+
if err != nil {
42+
return fmt.Errorf("(*http.Response).Body.Read failed: %w", err)
43+
}
44+
if err := resp.Body.Close(); err != nil {
45+
return fmt.Errorf("(*http.Response).Body.Close failed: %w", err)
46+
}
47+
resp.Body = nil
48+
s.lruMap.Add(u.String(), cachedResponse{
49+
Response: resp,
50+
Body: body,
51+
})
52+
return nil
53+
}
54+
55+
func NewLRUSStorage(size int) Storage {
56+
return Storage{
57+
lruMap: lru.New(size),
58+
}
59+
}
60+
61+
type GitHubCacheTransport struct {
62+
transport http.RoundTripper
63+
}
64+
65+
func (t *GitHubCacheTransport) RoundTrip(req *http.Request) (*http.Response, error) {
66+
return t.transport.RoundTrip(req)
67+
}
68+
69+
func NewGitHubCacheTransport(storage Storage, parent http.RoundTripper) *GitHubCacheTransport {
70+
return &GitHubCacheTransport{
71+
transport: ghtransport.NewTransport(
72+
storage,
73+
parent,
74+
),
75+
}
76+
}
77+
78+
func NewGitHubCache(storage Storage, parent http.RoundTripper) *http.Client {
79+
log.Debug("Creating new GitHub in memory cache")
80+
return &http.Client{
81+
Transport: NewGitHubCacheTransport(storage, parent),
82+
}
83+
}

‎applicationset/services/github_cache_test.go‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package services
2+
3+
import (
4+
"encoding/hex"
5+
"net/http"
6+
"net/http/httptest"
7+
"strings"
8+
"testing"
9+
10+
hash "github.com/bored-engineer/github-conditional-http-transport"
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
func setupFakeGithubServer(cacheHitCounter *int) *httptest.Server {
15+
payload := []byte(`{"name": "main"}`)
16+
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
17+
// Respond on 200/304 only if Authorization header start with `Bearer ok`
18+
authorization := r.Header.Get("Authorization")
19+
if !strings.HasPrefix(authorization, "Bearer ok") {
20+
w.WriteHeader(http.StatusUnauthorized)
21+
return
22+
}
23+
// If the request contains If-None-Match header, respond with 304
24+
ifNoneMatch := r.Header.Get("If-None-Match")
25+
if ifNoneMatch != "" {
26+
*cacheHitCounter++
27+
w.WriteHeader(http.StatusNotModified)
28+
return
29+
}
30+
// Otherwise respond with 200 with Etag and payload
31+
h := hash.Hash(r.Header)
32+
h.Write(payload)
33+
w.Header().Set("Etag", hex.EncodeToString(h.Sum(nil)))
34+
w.WriteHeader(http.StatusOK)
35+
_, _ = w.Write(payload)
36+
}))
37+
}
38+
39+
func TestGitHubCache_Success(t *testing.T) {
40+
// Setup a fake HTTP server
41+
cacheHitCounter := 0
42+
ts := setupFakeGithubServer(&cacheHitCounter)
43+
defer ts.Close()
44+
45+
storage := NewLRUSStorage(100)
46+
client := &http.Client{
47+
Transport: NewGitHubCacheTransport(
48+
storage,
49+
nil,
50+
),
51+
}
52+
ctx := t.Context()
53+
54+
// First request, should hit the server
55+
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL+"/owner/repo/branches/main", http.NoBody)
56+
req.Header.Set("Accept", "application/vnd.github.v3+json")
57+
req.Header.Set("Authorization", "Bearer ok1")
58+
res, err := client.Do(req)
59+
if err != nil {
60+
t.Fatalf("unexpected error: %v", err)
61+
}
62+
assert.Equal(t, http.StatusOK, res.StatusCode, "Expected ok status")
63+
64+
// Second request, should be served from cache with 304 even if Authorization header changed
65+
req.Header.Set("Authorization", "Bearer ok2")
66+
res, err = client.Do(req)
67+
if err != nil {
68+
t.Fatalf("unexpected error: %v", err)
69+
}
70+
assert.Equal(t, http.StatusOK, res.StatusCode, "Expected ok status")
71+
assert.Equal(t, cacheHitCounter, 1, "Expected one cache hit")
72+
}
73+
74+
func TestGitHubCache_NotAuthorized(t *testing.T) {
75+
// Setup a fake HTTP server
76+
cacheHitCounter := 0
77+
ts := setupFakeGithubServer(&cacheHitCounter)
78+
defer ts.Close()
79+
80+
storage := NewLRUSStorage(100)
81+
client := &http.Client{
82+
Transport: NewGitHubCacheTransport(
83+
storage,
84+
nil,
85+
),
86+
}
87+
ctx := t.Context()
88+
89+
// First request, should hit the server
90+
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL+"/owner/repo/branches/main", http.NoBody)
91+
req.Header.Set("Accept", "application/vnd.github.v3+json")
92+
req.Header.Set("Authorization", "Bearer ok1")
93+
res, err := client.Do(req)
94+
if err != nil {
95+
t.Fatalf("unexpected error: %v", err)
96+
}
97+
assert.Equal(t, http.StatusOK, res.StatusCode, "Expected ok status")
98+
99+
// Second request, should not be served from cache if Authorization is an invalid token
100+
req.Header.Set("Authorization", "Bearer ko")
101+
res, err = client.Do(req)
102+
if err != nil {
103+
t.Fatalf("unexpected error: %v", err)
104+
}
105+
assert.Equal(t, http.StatusUnauthorized, res.StatusCode, "Expected unauthorized status")
106+
assert.Equal(t, cacheHitCounter, 0, "Expected no cache hit")
107+
}
108+
109+
func TestNewGitHubCache(t *testing.T) {
110+
// Test cases
111+
testCases := []struct {
112+
name string
113+
parent http.RoundTripper
114+
}{
115+
{
116+
name: "with parent transport",
117+
parent: http.DefaultTransport,
118+
},
119+
{
120+
name: "with nil parent transport",
121+
parent: nil,
122+
},
123+
}
124+
125+
for _, tc := range testCases {
126+
t.Run(tc.name, func(t *testing.T) {
127+
// Create client
128+
storage := NewLRUSStorage(100)
129+
client := NewGitHubCache(storage, tc.parent)
130+
131+
// Assert client is not nil
132+
assert.NotNil(t, client)
133+
134+
// Assert transport is properly configured
135+
_, ok := client.Transport.(*GitHubCacheTransport)
136+
assert.True(t, ok, "Transport should be GitHubCacheTransport")
137+
})
138+
}
139+
}

‎cmd/argocd-applicationset-controller/commands/applicationset_controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ func NewCommand() *cobra.Command {
7676
globalPreservedAnnotations []string
7777
globalPreservedLabels []string
7878
enableGitHubAPIMetrics bool
79+
enableGitHubCache bool
80+
githubCacheSize int
7981
metricsAplicationsetLabels []string
8082
enableScmProviders bool
8183
webhookParallelism int
@@ -193,7 +195,7 @@ func NewCommand() *cobra.Command {
193195
argoSettingsMgr := argosettings.NewSettingsManager(ctx, k8sClient, namespace)
194196
argoCDDB := db.NewDB(namespace, argoSettingsMgr, k8sClient)
195197

196-
scmConfig := generators.NewSCMConfig(scmRootCAPath, allowedScmProviders, enableScmProviders, enableGitHubAPIMetrics, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), tokenRefStrictMode)
198+
scmConfig := generators.NewSCMConfig(scmRootCAPath, allowedScmProviders, enableScmProviders, enableGitHubAPIMetrics, enableGitHubCache, githubCacheSize, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), tokenRefStrictMode)
197199

198200
tlsConfig := apiclient.TLSConfiguration{
199201
DisableTLS: repoServerPlaintext,
@@ -292,6 +294,8 @@ func NewCommand() *cobra.Command {
292294
command.Flags().IntVar(&webhookParallelism, "webhook-parallelism-limit", env.ParseNumFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_WEBHOOK_PARALLELISM_LIMIT", 50, 1, 1000), "Number of webhook requests processed concurrently")
293295
command.Flags().StringSliceVar(&metricsAplicationsetLabels, "metrics-applicationset-labels", []string{}, "List of Application labels that will be added to the argocd_applicationset_labels metric")
294296
command.Flags().BoolVar(&enableGitHubAPIMetrics, "enable-github-api-metrics", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_GITHUB_API_METRICS", false), "Enable GitHub API metrics for generators that use the GitHub API")
297+
command.Flags().BoolVar(&enableGitHubCache, "enable-github-cache", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_GITHUB_CACHE", false), "Enable GitHub cache for generators that use the GitHub API")
298+
command.Flags().IntVar(&githubCacheSize, "github-cache-size", env.ParseNumFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_GITHUB_CACHE_SIZE", 2000, 0, math.MaxInt), "Size of the GitHub cache")
295299
command.Flags().IntVar(&maxResourcesStatusCount, "max-resources-status-count", env.ParseNumFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_MAX_RESOURCES_STATUS_COUNT", 0, 0, math.MaxInt), "Max number of resources stored in appset status.")
296300

297301
return &command

‎cmd/argocd-server/commands/argocd_server.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ func NewCommand() *cobra.Command {
9696
allowedScmProviders []string
9797
enableScmProviders bool
9898
enableGitHubAPIMetrics bool
99+
enableGitHubCache bool
100+
githubCacheSize int
99101

100102
// argocd k8s event logging flag
101103
enableK8sEvent []string
@@ -257,6 +259,8 @@ func NewCommand() *cobra.Command {
257259
AllowedScmProviders: allowedScmProviders,
258260
EnableScmProviders: enableScmProviders,
259261
EnableGitHubAPIMetrics: enableGitHubAPIMetrics,
262+
EnableGitHubCache: enableGitHubCache,
263+
GitHubCacheSize: githubCacheSize,
260264
}
261265

262266
stats.RegisterStackDumper()
@@ -336,6 +340,8 @@ func NewCommand() *cobra.Command {
336340
command.Flags().StringSliceVar(&allowedScmProviders, "appset-allowed-scm-providers", env.StringsFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ALLOWED_SCM_PROVIDERS", []string{}, ","), "The list of allowed custom SCM provider API URLs. This restriction does not apply to SCM or PR generators which do not accept a custom API URL. (Default: Empty = all)")
337341
command.Flags().BoolVar(&enableNewGitFileGlobbing, "appset-enable-new-git-file-globbing", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING", false), "Enable new globbing in Git files generator.")
338342
command.Flags().BoolVar(&enableGitHubAPIMetrics, "appset-enable-github-api-metrics", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_GITHUB_API_METRICS", false), "Enable GitHub API metrics for generators that use the GitHub API")
343+
command.Flags().BoolVar(&enableGitHubCache, "appset-enable-github-cache", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_GITHUB_CACHE", false), "Enable GitHub cache for generators that use the GitHub API")
344+
command.Flags().IntVar(&githubCacheSize, "appset-github-cache-size", env.ParseNumFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_GITHUB_CACHE_SIZE", 2000, 0, math.MaxInt), "Size of the GitHub cache")
339345

340346
tlsConfigCustomizerSrc = tls.AddTLSFlagsToCmd(command)
341347
cacheSrc = servercache.AddCacheFlagsToCmd(command, cacheutil.Options{

‎docs/operator-manual/server-commands/argocd-applicationset-controller.md‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/operator-manual/server-commands/argocd-server.md‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ require (
1919
github.com/aws/aws-sdk-go v1.55.7
2020
github.com/bmatcuk/doublestar/v4 v4.9.1
2121
github.com/bombsimon/logrusr/v4 v4.1.0
22+
github.com/bored-engineer/github-conditional-http-transport v0.0.0-20260105072410-0d0a872debdf
2223
github.com/bradleyfalzon/ghinstallation/v2 v2.17.0
2324
github.com/casbin/casbin/v2 v2.135.0
2425
github.com/casbin/govaluate v1.10.0

0 commit comments

Comments
 (0)