bazel l1 [fork 4/5]: multi-backend S3 map — fail-closed validation + per-request routing - #30
Open
shreyas-blacksmith wants to merge 3 commits into
Conversation
shreyas-blacksmith
force-pushed
the
shreyas/bazel-l1-piece-3-forwarding
branch
from
July 30, 2026 21:22
840d6fb to
d066700
Compare
shreyas-blacksmith
force-pushed
the
shreyas/bazel-l1-piece-4-backend-map
branch
from
July 30, 2026 21:22
25e272c to
d2c394b
Compare
shreyas-blacksmith
force-pushed
the
shreyas/bazel-l1-piece-3-forwarding
branch
from
July 31, 2026 13:55
d066700 to
1a46c90
Compare
shreyas-blacksmith
force-pushed
the
shreyas/bazel-l1-piece-4-backend-map
branch
from
July 31, 2026 13:55
d2c394b to
8e73e82
Compare
shreyas-blacksmith
marked this pull request as ready for review
July 31, 2026 14:54
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
shreyas-blacksmith
force-pushed
the
shreyas/bazel-l1-piece-4-backend-map
branch
from
July 31, 2026 17:14
8e73e82 to
f7b4f83
Compare
shreyas-blacksmith
force-pushed
the
shreyas/bazel-l1-piece-4-backend-map
branch
from
July 31, 2026 19:27
f7b4f83 to
026c58e
Compare
Adds the s3_proxy backends map: one minio client, transport, and upload queue per allowlisted backend, dispatched per request from the validated selection on the context, with the bucket applied per request (default bucket plus extra_buckets). Connection hygiene for LB-less MinIO endpoints: idle-conn caps (64 total / 32 per host by default) and periodic idle-connection recycling (conn_recycle_interval, default 5m), plus bounded upload timeouts, per-backend metrics labels, and lower per-backend upload-pool defaults in multi-backend mode. Config gains the backends map YAML surface with inheritance/merging and exactly-one-default validation; UploadReq captures the request-scoped selection at enqueue time. This commit also wires the S3-selector trust interceptors from the previous commit into main.go (their allowlist now exists) and refuses to start a backends-map node without both companion trust toggles. Co-authored-by: Cursor <cursoragent@cursor.com>
Two multi-backend sizing fixes from the L1 review rounds, landed ahead of the backends map being deployable: perBackendUploadLimits treated value-equality with the single-backend defaults as "unset", so an operator's explicit --num_uploaders 100 silently became 25 per backend in map mode. Presence is now recorded at both config sources (ctx.IsSet on the flag path, a pointer-field probe unmarshal on the YAML path) and consulted instead of values. The queue limits apply PER BACKEND and every queued upload holds an open reader FD, so the process-wide worst case is backends × max_queued_uploads. That aggregate is now asserted against half the soft RLIMIT_NOFILE at startup — a config that could exhaust NOFILE under write pressure fails the converge with the arithmetic spelled out, instead of killing the serving path later. Co-authored-by: Cursor <cursoragent@cursor.com>
assertAggregateUploadFDBudget runs inside config.Get and budgets against the soft NOFILE limit, but rlimit.Raise() only lifted it to the hard ceiling after parsing. A backend map that fits the post-raise limit could therefore fail startup against the unraised one. Raise first, then parse (bugbot, PR #30). Co-authored-by: Cursor <cursoragent@cursor.com>
shreyas-blacksmith
force-pushed
the
shreyas/bazel-l1-piece-4-backend-map
branch
from
August 1, 2026 00:27
026c58e to
f32a61a
Compare
shreyas-blacksmith
force-pushed
the
shreyas/bazel-l1-piece-3-forwarding
branch
from
August 1, 2026 00:27
f389172 to
0b2a6aa
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f32a61a. Configure here.
| merged.Endpoint = backend.Endpoint | ||
| if backend.DisableSSL != nil { | ||
| merged.DisableSSL = *backend.DisableSSL | ||
| } |
There was a problem hiding this comment.
SSL not inferred for override endpoints
Medium Severity
When a backends-map entry sets an explicit endpoint and omits disable_ssl, SSL is inherited from the top-level config (usually enabled) instead of from the selector key’s http/https scheme. The unset-endpoint path does derive disable_ssl from that scheme. Adding a private dial endpoint to an http:// selector can therefore flip the backend to TLS without any SSL-related config change, and MinIO calls fail at handshake time.
Reviewed by Cursor Bugbot for commit f32a61a. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Piece 4 of 5 peeled off
shreyas/bazel-l1-goal-state(design record: FastActions/fa#4779). Base: piece 3. Two commits (b4845c1,25e272c).Summary
s3proxy.NewMulti, configs3_proxy.backends): the L1 validates the forwarded(endpoint, bucket)selector against an operator allowlist — fail-closed, opaque string match, no URL normalization — and routes the operation to that backend, including queued async write-throughs (the selector rides each queued item). Unknown selector → typedS3_BACKEND_SELECTOR_REJECTEDmarker → client degrades to a metered miss (piece 3).num_uploaders/max_queued_uploadsequal to the single-backend default is no longer silently lowered per map entry (presence tracked for both CLI flags and YAML keys).max_queued_uploadsexceeds half of softRLIMIT_NOFILE(every queued upload holds an open reader FD) — a config that could exhaust FDs fails the converge, not the serving path.Dark until a node is deployed with a non-empty backends map; merging is harmless.
Evidence
Staging E2E 2026-07-30: both nodes in three-entry map mode; two-bucket routing proven via per-backend outcome counters; unknown-selector and wrong-bucket rejections degraded to metered misses.
Made with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled. (Staging)