From b6cff9deb0fbd15f608bd7164dea3398e06d73b6 Mon Sep 17 00:00:00 2001 From: Jeff Mendoza Date: Wed, 6 Mar 2024 15:23:58 -0800 Subject: [PATCH] Change cache to avoid memory use Orignally, the cache was intended to be long lived to handle incoming webhooks at any time. Currently, we are just polling, and just need the cache to handle a single "EnforceAll" run, where we hit the same paths multiple times in that run. Therefore, change the cache to be per-installation, and free it after each "EnforceAll". Signed-off-by: Jeff Mendoza --- pkg/enforce/enforce.go | 2 +- pkg/enforce/enforce_test.go | 2 +- pkg/ghclients/ghclients.go | 14 ++++++-------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/pkg/enforce/enforce.go b/pkg/enforce/enforce.go index 881e05c0..137b8f1c 100644 --- a/pkg/enforce/enforce.go +++ b/pkg/enforce/enforce.go @@ -156,6 +156,7 @@ func EnforceAll(ctx context.Context, ghc ghclients.GhClientsInterface, specificP } enforceAllResults[policyName]["totalFailed"] += results["totalFailed"] } + ghc.Free(iid) mu.Unlock() if err != nil { @@ -170,7 +171,6 @@ func EnforceAll(ctx context.Context, ghc ghclients.GhClientsInterface, specificP if err := g.Wait(); err != nil { return enforceAllResults, err } - ghc.LogCacheSize() log.Info(). Str("area", "bot"). Int("count", repoCount). diff --git a/pkg/enforce/enforce_test.go b/pkg/enforce/enforce_test.go index effe87a0..c259d371 100644 --- a/pkg/enforce/enforce_test.go +++ b/pkg/enforce/enforce_test.go @@ -89,7 +89,7 @@ func (m MockGhClients) Get(i int64) (*github.Client, error) { return github.NewClient(&http.Client{}), nil } -func (m MockGhClients) LogCacheSize() {} +func (m MockGhClients) Free(i int64) {} func TestRunPolicies(t *testing.T) { policiesGetPolicies = func() []policydef.Policy { diff --git a/pkg/ghclients/ghclients.go b/pkg/ghclients/ghclients.go index 44095875..187e8032 100644 --- a/pkg/ghclients/ghclients.go +++ b/pkg/ghclients/ghclients.go @@ -49,7 +49,7 @@ func init() { type GhClientsInterface interface { Get(i int64) (*github.Client, error) - LogCacheSize() + Free(i int64) } // GHClients stores clients per-installation for re-use throughout a process. @@ -57,7 +57,6 @@ type GHClients struct { clients map[int64]*github.Client tr http.RoundTripper key []byte - cache *memoryCache } // NewGHClients returns a new GHClients. The provided RoundTripper will be @@ -71,10 +70,13 @@ func NewGHClients(ctx context.Context, t http.RoundTripper) (*GHClients, error) clients: make(map[int64]*github.Client), tr: t, key: key, - cache: newMemoryCache(), }, nil } +func (g *GHClients) Free(i int64) { + delete(g.clients, i) +} + // Get gets the client for installation id i, If i is 0 it gets the client for // the app-level api. If a stored client is not available, it creates a new // client with auth and caching built in. @@ -85,7 +87,7 @@ func (g *GHClients) Get(i int64) (*github.Client, error) { ctr := &httpcache.Transport{ Transport: g.tr, - Cache: g.cache, + Cache: newMemoryCache(), MarkCachedResponses: true, } @@ -103,10 +105,6 @@ func (g *GHClients) Get(i int64) (*github.Client, error) { return g.clients[i], nil } -func (g *GHClients) LogCacheSize() { - g.cache.LogCacheSize() -} - func getKeyFromSecretReal(ctx context.Context, keySecretVal string) ([]byte, error) { v, err := runtimevar.OpenVariable(ctx, keySecretVal) if err != nil {