Skip to content

Commit 3af5e7d

Browse files
authored
Evaluator: enable per-tenant HTTP caching for GitHub API client (#953)
* Larker: enable HTTP caching for GitHub API client * Bring back #689 tuning * Initialize HTTPClient once * Only use init() for tuning the HTTPClient after it's initialized * Per-instance HTTPClient and GitHub client * Share caching *http.Client for multiple github.New() invocations * Per-tenant shared HTTP cache * Use Get() methods to prevent NPE * Introduce TestGitHubHTTPCache * TestGitHubHTTPCache: test with two separate tenants * $ go mod tidy
1 parent dc2e901 commit 3af5e7d

File tree

17 files changed

+2750
-2311
lines changed

17 files changed

+2750
-2311
lines changed

api/cirrus_ci_service.proto

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,16 @@ message FileSystem {
5858
}
5959

6060
message Github {
61+
message HTTPCache {
62+
string tenant = 1;
63+
int32 size = 2;
64+
}
65+
6166
string owner = 1;
6267
string repo = 2;
6368
string reference = 3;
6469
string token = 4;
70+
HTTPCache http_cache = 5;
6571
}
6672

6773
oneof impl {

go.mod

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ require (
6565
github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.2
6666
github.com/IGLOU-EU/go-wildcard v1.0.3
6767
github.com/aws/aws-sdk-go v1.55.8
68+
github.com/bartventer/httpcache v0.10.2
6869
github.com/bmatcuk/doublestar v1.3.4
6970
github.com/cirruslabs/chacha v0.16.3
7071
github.com/cirruslabs/cirrus-ci-annotations v0.10.0
@@ -74,7 +75,10 @@ require (
7475
github.com/go-chi/render v1.0.3
7576
github.com/golang-jwt/jwt/v5 v5.3.0
7677
github.com/google/go-github/v59 v59.0.0
77-
github.com/hashicorp/vault/api v1.22.0
78+
github.com/hashicorp/golang-lru/v2 v2.0.7
79+
github.com/hashicorp/vault/api v1.21.0
80+
github.com/jarcoal/httpmock v1.4.1
81+
github.com/jellydator/ttlcache/v3 v3.4.0
7882
github.com/klauspost/pgzip v1.2.6
7983
github.com/pkg/errors v0.9.1
8084
github.com/prometheus/procfs v0.17.0

go.sum

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ github.com/avast/retry-go/v4 v4.6.1 h1:VkOLRubHdisGrHnTu89g08aQEWEgRU7LVEop3GbIc
4747
github.com/avast/retry-go/v4 v4.6.1/go.mod h1:V6oF8njAwxJ5gRo1Q7Cxab24xs5NCWZBeaHHBklR8mA=
4848
github.com/aws/aws-sdk-go v1.55.8 h1:JRmEUbU52aJQZ2AjX4q4Wu7t4uZjOu71uyNmaWlUkJQ=
4949
github.com/aws/aws-sdk-go v1.55.8/go.mod h1:ZkViS9AqA6otK+JBBNH2++sx1sgxrPKcSzPPvQkUtXk=
50+
github.com/bartventer/httpcache v0.10.2 h1:z/PRtWPf8I7zC9gWPil6wBhnbkGqc6MOzEsIbzou0W4=
51+
github.com/bartventer/httpcache v0.10.2/go.mod h1:nY3vexlqOtDlEDHfdGM3vMM6oeoKPLV8vPmzVdYOZBI=
5052
github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
5153
github.com/beorn7/perks v0.0.0-20150223135152-b965b613227f/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
5254
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
@@ -279,18 +281,24 @@ github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKe
279281
github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
280282
github.com/hashicorp/golang-lru v1.0.2 h1:dV3g9Z/unq5DpblPpw+Oqcv4dU/1omnb4Ok8iPY6p1c=
281283
github.com/hashicorp/golang-lru v1.0.2/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
284+
github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
285+
github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
282286
github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I=
283287
github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM=
284-
github.com/hashicorp/vault/api v1.22.0 h1:+HYFquE35/B74fHoIeXlZIP2YADVboaPjaSicHEZiH0=
285-
github.com/hashicorp/vault/api v1.22.0/go.mod h1:IUZA2cDvr4Ok3+NtK2Oq/r+lJeXkeCrHRmqdyWfpmGM=
288+
github.com/hashicorp/vault/api v1.21.0 h1:Xej4LJETV/spWRdjreb2vzQhEZt4+B5yxHAObfQVDOs=
289+
github.com/hashicorp/vault/api v1.21.0/go.mod h1:IUZA2cDvr4Ok3+NtK2Oq/r+lJeXkeCrHRmqdyWfpmGM=
286290
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
287291
github.com/in-toto/in-toto-golang v0.9.0 h1:tHny7ac4KgtsfrG6ybU8gVOZux2H8jN05AXJ9EBM1XU=
288292
github.com/in-toto/in-toto-golang v0.9.0/go.mod h1:xsBVrVsHNsB61++S6Dy2vWosKhuA3lUTQd+eF9HdeMo=
289293
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
290294
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
291295
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
296+
github.com/jarcoal/httpmock v1.4.1 h1:0Ju+VCFuARfFlhVXFc2HxlcQkfB+Xq12/EotHko+x2A=
297+
github.com/jarcoal/httpmock v1.4.1/go.mod h1:ftW1xULwo+j0R0JJkJIIi7UKigZUXCLLanykgjwBXL0=
292298
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A=
293299
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
300+
github.com/jellydator/ttlcache/v3 v3.4.0 h1:YS4P125qQS0tNhtL6aeYkheEaB/m8HCqdMMP4mnWdTY=
301+
github.com/jellydator/ttlcache/v3 v3.4.0/go.mod h1:Hw9EgjymziQD3yGsQdf1FqFdpp7YjFMd4Srg5EJlgD4=
294302
github.com/jinzhu/gorm v0.0.0-20170222002820-5409931a1bb8/go.mod h1:Vla75njaFJ8clLU1W44h34PjIkijhjHIYnZxMqCdxqo=
295303
github.com/jinzhu/inflection v0.0.0-20170102125226-1c35d901db3d/go.mod h1:h+uFLlag+Qp1Va5pdKtLDYj+kHp5pxUVkryuEj+Srlc=
296304
github.com/jinzhu/now v1.1.1/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/z8=
@@ -352,6 +360,8 @@ github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6T
352360
github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
353361
github.com/mattn/go-sqlite3 v1.6.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
354362
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
363+
github.com/maxatome/go-testdeep v1.14.0 h1:rRlLv1+kI8eOI3OaBXZwb3O7xY3exRzdW5QyX48g9wI=
364+
github.com/maxatome/go-testdeep v1.14.0/go.mod h1:lPZc/HAcJMP92l7yI6TRz1aZN5URwUBUAfUNvrclaNM=
355365
github.com/miekg/pkcs11 v1.0.2/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WTFxgs=
356366
github.com/miekg/pkcs11 v1.1.1 h1:Ugu9pdy6vAYku5DEpVWVFPYnzV+bxB+iRdbuFSu7TvU=
357367
github.com/miekg/pkcs11 v1.1.1/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WTFxgs=

internal/evaluator/evaluator.go

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"net"
9+
"net/http"
10+
"strings"
11+
"time"
12+
13+
"github.com/bartventer/httpcache"
14+
_ "github.com/cirruslabs/cirrus-cli/internal/evaluator/lrucache"
815
"github.com/cirruslabs/cirrus-cli/internal/version"
916
"github.com/cirruslabs/cirrus-cli/pkg/api"
1017
"github.com/cirruslabs/cirrus-cli/pkg/larker"
@@ -14,6 +21,7 @@ import (
1421
"github.com/cirruslabs/cirrus-cli/pkg/larker/fs/memory"
1522
"github.com/cirruslabs/cirrus-cli/pkg/parser"
1623
"github.com/cirruslabs/cirrus-cli/pkg/parser/parsererror"
24+
"github.com/jellydator/ttlcache/v3"
1725
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
1826
"go.opentelemetry.io/otel/metric/noop"
1927
"google.golang.org/grpc"
@@ -23,8 +31,6 @@ import (
2331
"google.golang.org/protobuf/reflect/protodesc"
2432
"google.golang.org/protobuf/reflect/protoreflect"
2533
"google.golang.org/protobuf/types/known/structpb"
26-
"net"
27-
"strings"
2834
)
2935

3036
const pathYAML = ".cirrus.yml"
@@ -33,6 +39,9 @@ const pathStarlark = ".cirrus.star"
3339
var ErrNoFS = errors.New("no filesystem available")
3440

3541
type ConfigurationEvaluatorServiceServer struct {
42+
perTenantCachingHTTPClients *ttlcache.Cache[string, *http.Client]
43+
roundTripperForTests http.RoundTripper
44+
3645
// must be embedded to have forward compatible implementations
3746
api.UnimplementedCirrusConfigurationEvaluatorServiceServer
3847
}
@@ -54,15 +63,26 @@ func addVersion(
5463
return handler(ctx, req)
5564
}
5665

57-
func Serve(ctx context.Context, lis net.Listener) error {
66+
func Serve(ctx context.Context, lis net.Listener, opts ...Option) error {
5867
server := grpc.NewServer(
5968
grpc.UnaryInterceptor(addVersion),
6069
grpc.StatsHandler(otelgrpc.NewServerHandler(
6170
otelgrpc.WithMeterProvider(noop.NewMeterProvider()),
6271
)),
6372
)
6473

65-
api.RegisterCirrusConfigurationEvaluatorServiceServer(server, &ConfigurationEvaluatorServiceServer{})
74+
r := &ConfigurationEvaluatorServiceServer{
75+
perTenantCachingHTTPClients: ttlcache.New[string, *http.Client](
76+
ttlcache.WithTTL[string, *http.Client](24 * time.Hour),
77+
),
78+
}
79+
80+
// Apply opts
81+
for _, opt := range opts {
82+
opt(r)
83+
}
84+
85+
api.RegisterCirrusConfigurationEvaluatorServiceServer(server, r)
6686

6787
errChan := make(chan error)
6888

@@ -97,7 +117,15 @@ func (r *ConfigurationEvaluatorServiceServer) EvaluateConfig(
97117
yamlConfigs = append(yamlConfigs, request.YamlConfig)
98118
}
99119

100-
fs, err := convertFS(request.Fs)
120+
var httpClient *http.Client
121+
122+
if githubFS := request.GetFs().GetGithub(); githubFS != nil {
123+
if httpCache := githubFS.GetHttpCache(); httpCache != nil {
124+
httpClient = r.cachingHTTPClient(httpCache.GetTenant(), httpCache.GetSize())
125+
}
126+
}
127+
128+
fs, err := convertFS(request.Fs, httpClient)
101129
if err != nil {
102130
return nil, status.Errorf(codes.Internal, "failed to initialize file system: %v", err)
103131
}
@@ -109,6 +137,7 @@ func (r *ConfigurationEvaluatorServiceServer) EvaluateConfig(
109137
larker.WithFileSystem(fs),
110138
larker.WithEnvironment(request.Environment),
111139
larker.WithAffectedFiles(request.AffectedFiles),
140+
larker.WithHTTPClient(httpClient),
112141
)
113142

114143
lrkResult, err := lrk.MainOptional(ctx, request.StarlarkConfig)
@@ -207,14 +236,23 @@ func (r *ConfigurationEvaluatorServiceServer) EvaluateFunction(
207236
ctx context.Context,
208237
request *api.EvaluateFunctionRequest,
209238
) (*api.EvaluateFunctionResponse, error) {
210-
fs, err := convertFS(request.Fs)
239+
var httpClient *http.Client
240+
241+
if githubFS := request.GetFs().GetGithub(); githubFS != nil {
242+
if httpCache := githubFS.GetHttpCache(); httpCache != nil {
243+
httpClient = r.cachingHTTPClient(httpCache.GetTenant(), httpCache.GetSize())
244+
}
245+
}
246+
247+
fs, err := convertFS(request.Fs, httpClient)
211248
if err != nil {
212249
return nil, status.Errorf(codes.Internal, "failed to initialize file system: %v", err)
213250
}
214251

215252
lrk := larker.New(
216253
larker.WithFileSystem(fs),
217254
larker.WithEnvironment(request.Environment),
255+
larker.WithHTTPClient(httpClient),
218256
)
219257

220258
// Run Starlark hook
@@ -281,7 +319,7 @@ func TransformAdditionalInstances(
281319
return additionalInstances, nil
282320
}
283321

284-
func convertFS(apiFS *api.FileSystem) (fs fs.FileSystem, err error) {
322+
func convertFS(apiFS *api.FileSystem, httpClient *http.Client) (fs fs.FileSystem, err error) {
285323
fs = failing.New(ErrNoFS)
286324

287325
if apiFS == nil {
@@ -292,8 +330,40 @@ func convertFS(apiFS *api.FileSystem) (fs fs.FileSystem, err error) {
292330
case *api.FileSystem_Memory_:
293331
fs, err = memory.New(impl.Memory.FilesContents)
294332
case *api.FileSystem_Github_:
295-
fs, err = github.New(impl.Github.Owner, impl.Github.Repo, impl.Github.Reference, impl.Github.Token)
333+
fs, err = github.New(impl.Github.Owner, impl.Github.Repo, impl.Github.Reference, impl.Github.Token,
334+
httpClient)
296335
}
297336

298337
return fs, err
299338
}
339+
340+
func (r *ConfigurationEvaluatorServiceServer) cachingHTTPClient(tenant string, size int32) *http.Client {
341+
if tenant == "" || size == 0 {
342+
return nil
343+
}
344+
345+
httpClient, _ := r.perTenantCachingHTTPClients.GetOrSetFunc(tenant, func() *http.Client {
346+
dsn := fmt.Sprintf("lrucache://?size=%d", size)
347+
348+
httpClient := httpcache.NewClient(dsn, httpcache.WithUpstream(r.roundTripper()))
349+
350+
// GitHub has a 10-second timeout for API requests
351+
httpClient.Timeout = 11 * time.Second
352+
353+
return httpClient
354+
})
355+
356+
return httpClient.Value()
357+
}
358+
359+
func (r *ConfigurationEvaluatorServiceServer) roundTripper() http.RoundTripper {
360+
if r.roundTripperForTests != nil {
361+
return r.roundTripperForTests
362+
}
363+
364+
return &http.Transport{
365+
MaxIdleConns: 1024,
366+
MaxIdleConnsPerHost: 1024, // default is 2 which is too small and we mostly access the same host
367+
IdleConnTimeout: time.Minute, // let's put something big but not infinite like the default
368+
}
369+
}

internal/evaluator/evaluator_test.go

Lines changed: 83 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,22 @@
44
package evaluator_test
55

66
import (
7+
"bytes"
78
"context"
9+
"encoding/base64"
810
"encoding/json"
911
"errors"
12+
"io"
13+
"net"
14+
"net/http"
15+
"testing"
16+
"time"
17+
1018
"github.com/cirruslabs/cirrus-cli/internal/evaluator"
1119
"github.com/cirruslabs/cirrus-cli/pkg/api"
1220
"github.com/golang/protobuf/protoc-gen-go/descriptor"
21+
"github.com/google/go-github/v59/github"
22+
"github.com/jarcoal/httpmock"
1323
"github.com/stretchr/testify/assert"
1424
"github.com/stretchr/testify/require"
1525
"github.com/xeipuuv/gojsonschema"
@@ -21,12 +31,9 @@ import (
2131
"google.golang.org/protobuf/types/known/anypb"
2232
"google.golang.org/protobuf/types/known/emptypb"
2333
"google.golang.org/protobuf/types/known/structpb"
24-
"net"
25-
"testing"
26-
"time"
2734
)
2835

29-
func getClient(t *testing.T) api.CirrusConfigurationEvaluatorServiceClient {
36+
func getClient(t *testing.T, opts ...evaluator.Option) api.CirrusConfigurationEvaluatorServiceClient {
3037
ctx, cancel := context.WithCancel(context.Background())
3138

3239
lis, err := net.Listen("tcp", "localhost:0")
@@ -37,7 +44,7 @@ func getClient(t *testing.T) api.CirrusConfigurationEvaluatorServiceClient {
3744
errChan := make(chan error)
3845

3946
go func() {
40-
errChan <- evaluator.Serve(ctx, lis)
47+
errChan <- evaluator.Serve(ctx, lis, opts...)
4148
}()
4249

4350
t.Cleanup(func() {
@@ -56,8 +63,8 @@ func getClient(t *testing.T) api.CirrusConfigurationEvaluatorServiceClient {
5663
return api.NewCirrusConfigurationEvaluatorServiceClient(conn)
5764
}
5865

59-
func evaluateConfigHelper(t *testing.T, request *api.EvaluateConfigRequest) (*api.EvaluateConfigResponse, error) {
60-
return getClient(t).EvaluateConfig(context.Background(), request)
66+
func evaluateConfigHelper(t *testing.T, request *api.EvaluateConfigRequest, opts ...evaluator.Option) (*api.EvaluateConfigResponse, error) {
67+
return getClient(t, opts...).EvaluateConfig(context.Background(), request)
6168
}
6269

6370
func schemaHelper(t *testing.T, request *api.JSONSchemaRequest) (*api.JSONSchemaResponse, error) {
@@ -448,3 +455,72 @@ func TestBacktraceHook(t *testing.T) {
448455
require.Contains(t, string(response.OutputLogs), "hook\nsentinel\n")
449456
require.Contains(t, string(response.OutputLogs), "Traceback (most recent call last):\n .cirrus.star:6:10")
450457
}
458+
459+
func TestGitHubHTTPCache(t *testing.T) {
460+
mockTransport := httpmock.NewMockTransport()
461+
462+
mockTransport.RegisterResponder("GET", "https://api.github.com/repos/cirruslabs/cirrus-cli/contents/README.md?ref=main", func(req *http.Request) (*http.Response, error) {
463+
githubRepositoryContent := github.RepositoryContent{
464+
Content: github.String(base64.StdEncoding.EncodeToString([]byte("Hello, World!"))),
465+
}
466+
467+
githubRepositoryContentJSON, err := json.Marshal(&githubRepositoryContent)
468+
require.NoError(t, err)
469+
470+
return &http.Response{
471+
StatusCode: http.StatusOK,
472+
Header: http.Header{},
473+
Body: io.NopCloser(bytes.NewReader(githubRepositoryContentJSON)),
474+
}, nil
475+
})
476+
477+
starlarkConfig := `load("cirrus", "fs")
478+
def main(ctx):
479+
print(fs.read("README.md"))
480+
481+
print(fs.read("README.md"))
482+
483+
return []
484+
`
485+
486+
response, err := evaluateConfigHelper(t, &api.EvaluateConfigRequest{
487+
StarlarkConfig: starlarkConfig,
488+
Fs: &api.FileSystem{
489+
Impl: &api.FileSystem_Github_{
490+
Github: &api.FileSystem_Github{
491+
Owner: "cirruslabs",
492+
Repo: "cirrus-cli",
493+
Reference: "main",
494+
HttpCache: &api.FileSystem_Github_HTTPCache{
495+
Tenant: "tenant1",
496+
Size: 64 * 1024,
497+
},
498+
},
499+
},
500+
},
501+
}, evaluator.WithRoundTripperForTests(mockTransport))
502+
require.NoError(t, err)
503+
require.Equal(t, 1, mockTransport.GetTotalCallCount())
504+
require.Equal(t, "Hello, World!\nHello, World!\n", string(response.OutputLogs))
505+
506+
// Ensure that the other tenant does not reuse our cache
507+
response, err = evaluateConfigHelper(t, &api.EvaluateConfigRequest{
508+
StarlarkConfig: starlarkConfig,
509+
Fs: &api.FileSystem{
510+
Impl: &api.FileSystem_Github_{
511+
Github: &api.FileSystem_Github{
512+
Owner: "cirruslabs",
513+
Repo: "cirrus-cli",
514+
Reference: "main",
515+
HttpCache: &api.FileSystem_Github_HTTPCache{
516+
Tenant: "tenant2",
517+
Size: 64 * 1024,
518+
},
519+
},
520+
},
521+
},
522+
}, evaluator.WithRoundTripperForTests(mockTransport))
523+
require.NoError(t, err)
524+
require.Equal(t, 2, mockTransport.GetTotalCallCount())
525+
require.Equal(t, "Hello, World!\nHello, World!\n", string(response.OutputLogs))
526+
}

0 commit comments

Comments
 (0)