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.

Sign up for GitHub

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

Add grpc proxy backend #680

Closed
wants to merge 17 commits into from
Closed

Add grpc proxy backend #680

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
79de165
GRPCProxy
AlessandroPatti Jul 30, 2023
7af17e2
GRPCProxy Tests
AlessandroPatti Jul 30, 2023
23219e9
Feedback
AlessandroPatti Jul 30, 2023
3afd5ff
Client side metrics
AlessandroPatti Jul 30, 2023
8e000f6
Update flags help message
AlessandroPatti Jul 30, 2023
849eccc
Feedback
AlessandroPatti Aug 2, 2023
32b6218
Use bufconn for tests
AlessandroPatti Aug 2, 2023
b7144c2
Check capabilities
AlessandroPatti Aug 2, 2023
728b2d6
Extract common FetchBlob logic
AlessandroPatti Aug 2, 2023
1ec3a38
Pass size to proxies
AlessandroPatti Aug 2, 2023
1aa918d
Support compressed grpc backend
AlessandroPatti Aug 9, 2023
569e34e
Improve logging
AlessandroPatti Aug 9, 2023
6498cbe
Typo
AlessandroPatti Aug 9, 2023
d3afb83
CheckCapabilities
AlessandroPatti Aug 12, 2023
ddf4182
Update grpc/http proxy protocol validation
AlessandroPatti Aug 17, 2023
cf04071
Allow setting CA file for backend proxy
AlessandroPatti Aug 17, 2023
303a252
Basic
AlessandroPatti Aug 18, 2023
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
14 changes: 14 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,20 @@ OPTIONS:
Goroutines to process parallel uploads to backend. (default: 100)
[$BAZEL_REMOTE_NUM_UPLOADERS]

--grpc_proxy.url value The base URL to use for a grpc proxy backend, e.g.
localhost:9090 or example.com:7070.
[$BAZEL_REMOTE_GRPC_PROXY_URL]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should probably provide a way to use TLS when talking to the proxy backend and not using mTLS. It looks like this PR uses TLS when authenticating with the proxy backend via mTLS (which makes sense), otherwise TLS is not used at all when talking to the proxy backend?


--grpc_proxy.key_file value Path to a key used to authenticate with the
proxy backend using mTLS. If this flag is provided, then
grpc_proxy.cert_file must also be specified.
[$BAZEL_REMOTE_GRPC_PROXY_KEY_FILE]

--grpc_proxy.cert_file value Path to a certificate used to authenticate
with the proxy backend using mTLS. If this flag is provided, then
grpc_proxy.key_file must also be specified.
[BAZEL_REMOTE_GRPC_PROXY_CERT_FILE]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How difficult would it be to add support for basic/password authentication? I think we should consider adding this, mostly to avoid documenting that it doesn't work, but also because it's probably easier to setup a system test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The password authentication is right now used only for clients to authenticate with bazel-remote, but not for it to authenticate with the proxy backend. As far as I can tell, none of the existing proxies support this right now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bazel-remote <-> proxy backend authentiation is completely separate from the client <-> bazel-remote authentication though. If bazel-remote is the main intended use case for the grpc proxy backend, then I think we should support that. But this would be OK to land in a followup (I can help if needed).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not realise that the http proxy actualy supports basic auth, just need to pass the credentials in the proxy url. I took a stab at doing the same for the grpc proxy.

--http_proxy.url value The base URL to use for a http proxy backend.
[$BAZEL_REMOTE_HTTP_PROXY_URL]

Expand Down
4 changes: 2 additions & 2 deletions cache/azblobproxy/azblobproxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (c *azBlobCache) Put(ctx context.Context, kind cache.EntryKind, hash string
}
}

func (c *azBlobCache) Get(ctx context.Context, kind cache.EntryKind, hash string) (rc io.ReadCloser, size int64, err error) {
func (c *azBlobCache) Get(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (rc io.ReadCloser, size int64, err error) {
key := c.objectKey(hash, kind)
if c.prefix != "" {
key = c.prefix + "/" + key
Expand Down Expand Up @@ -111,7 +111,7 @@ func (c *azBlobCache) Get(ctx context.Context, kind cache.EntryKind, hash string

var errNotFound = errors.New("NOT FOUND")

func (c *azBlobCache) Contains(ctx context.Context, kind cache.EntryKind, hash string) (bool, int64) {
func (c *azBlobCache) Contains(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (bool, int64) {
key := c.objectKey(hash, kind)
if c.prefix != "" {
key = c.prefix + "/" + key
Expand Down
4 changes: 2 additions & 2 deletions cache/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ type Proxy interface {
// `hash` can be read, its logical size, and an error if something went
// wrong. The data available from `rc` is in the same format as used by
// the disk.Cache instance.
Get(ctx context.Context, kind EntryKind, hash string) (rc io.ReadCloser, size int64, err error)
Get(ctx context.Context, kind EntryKind, hash string, size int64) (io.ReadCloser, int64, error)

// Contains returns whether or not the cache item exists on the
// remote end, and the size if it exists (and -1 if the size is
// unknown).
Contains(ctx context.Context, kind EntryKind, hash string) (bool, int64)
Contains(ctx context.Context, kind EntryKind, hash string, size int64) (bool, int64)
}

// TransformActionCacheKey takes an ActionCache key and an instance name
Expand Down
4 changes: 2 additions & 2 deletions cache/disk/disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ func (c *diskCache) get(ctx context.Context, kind cache.EntryKind, hash string,
return nil, -1, nil
}

r, foundSize, err := c.proxy.Get(ctx, kind, hash)
r, foundSize, err := c.proxy.Get(ctx, kind, hash, size)
if r != nil {
defer r.Close()
}
Expand Down Expand Up @@ -731,7 +731,7 @@ func (c *diskCache) Contains(ctx context.Context, kind cache.EntryKind, hash str
}

if c.proxy != nil && size <= c.maxProxyBlobSize {
exists, foundSize = c.proxy.Contains(ctx, kind, hash)
exists, foundSize = c.proxy.Contains(ctx, kind, hash, size)
if exists && foundSize <= c.maxProxyBlobSize && !isSizeMismatch(size, foundSize) {
return true, foundSize
}
Expand Down
4 changes: 2 additions & 2 deletions cache/disk/disk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func (d proxyStub) Put(ctx context.Context, kind cache.EntryKind, hash string, l
// Not implemented.
}

func (d proxyStub) Get(ctx context.Context, kind cache.EntryKind, hash string) (io.ReadCloser, int64, error) {
func (d proxyStub) Get(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (io.ReadCloser, int64, error) {
if hash != contentsHash || kind != cache.CAS {
return nil, -1, nil
}
Expand Down Expand Up @@ -269,7 +269,7 @@ func (d proxyStub) Get(ctx context.Context, kind cache.EntryKind, hash string) (
return readme, contentsLength, nil
}

func (d proxyStub) Contains(ctx context.Context, kind cache.EntryKind, hash string) (bool, int64) {
func (d proxyStub) Contains(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (bool, int64) {
if hash != contentsHash || kind != cache.CAS {
return false, -1
}
Expand Down
2 changes: 1 addition & 1 deletion cache/disk/findmissing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (c *diskCache) containsWorker() {
}
}

ok, _ = c.proxy.Contains(req.ctx, cache.CAS, (*req.digest).Hash)
ok, _ = c.proxy.Contains(req.ctx, cache.CAS, (*req.digest).Hash, (*req.digest).SizeBytes)
if ok {
c.accessLogger.Printf("GRPC CAS HEAD %s OK", (*req.digest).Hash)
// The blob exists on the proxy, remove it from the
Expand Down
8 changes: 4 additions & 4 deletions cache/disk/findmissing_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ type testCWProxy struct {
func (p *testCWProxy) Put(ctx context.Context, kind cache.EntryKind, hash string, logicalSize int64, sizeOnDisk int64, rc io.ReadCloser) {
}

func (p *testCWProxy) Get(ctx context.Context, kind cache.EntryKind, hash string) (io.ReadCloser, int64, error) {
func (p *testCWProxy) Get(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (io.ReadCloser, int64, error) {
return nil, -1, nil
}

func (p *testCWProxy) Contains(ctx context.Context, kind cache.EntryKind, hash string) (bool, int64) {
func (p *testCWProxy) Contains(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (bool, int64) {
if kind == cache.CAS && hash == p.blob {
return true, 42
}
Expand Down Expand Up @@ -176,11 +176,11 @@ func (p *proxyAdapter) Put(ctx context.Context, kind cache.EntryKind, hash strin
}
}

func (p *proxyAdapter) Get(ctx context.Context, kind cache.EntryKind, hash string) (rc io.ReadCloser, size int64, err error) {
func (p *proxyAdapter) Get(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (rc io.ReadCloser, size int64, err error) {
return p.cache.Get(ctx, kind, hash, size, 0)
}

func (p *proxyAdapter) Contains(ctx context.Context, kind cache.EntryKind, hash string) (bool, int64) {
func (p *proxyAdapter) Contains(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (bool, int64) {
return p.cache.Contains(ctx, kind, hash, -1)
}

Expand Down
42 changes: 42 additions & 0 deletions cache/grpcproxy/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
srcs = [
"grpcproxy.go",
"readcloser.go",
],
importpath = "github.com/buchgr/bazel-remote/v2/cache/grpcproxy",
visibility = ["//visibility:public"],
deps = [
"//cache:go_default_library",
"//genproto/build/bazel/remote/asset/v1:go_default_library",
"//genproto/build/bazel/remote/execution/v2:go_default_library",
"//utils/backendproxy:go_default_library",
"@com_github_google_uuid//:go_default_library",
"@go_googleapis//google/bytestream:bytestream_go_proto",
"@org_golang_google_grpc//:go_default_library",
"@org_golang_google_grpc//codes:go_default_library",
"@org_golang_google_grpc//status:go_default_library",
"@org_golang_google_protobuf//proto:go_default_library",
],
)

go_test(
name = "go_default_test",
srcs = ["grpcproxy_test.go"],
embed = [":go_default_library"],
deps = [
"//cache:go_default_library",
"//cache/disk:go_default_library",
"//genproto/build/bazel/remote/execution/v2:go_default_library",
"//server:go_default_library",
"//utils:go_default_library",
"@com_github_google_uuid//:go_default_library",
"@go_googleapis//google/bytestream:bytestream_go_proto",
"@org_golang_google_grpc//:go_default_library",
"@org_golang_google_grpc//credentials/insecure:go_default_library",
"@org_golang_google_grpc//test/bufconn:go_default_library",
"@org_golang_google_protobuf//proto:go_default_library",
],
)
Loading