Make sandbox traffic routable over ingress#2735
Draft
sitole wants to merge 3 commits into
Draft
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit a33b42b. Bugbot is set up for automated code reviews on this repo. Configure here. |
❌ 4 Tests Failed:
View the full list of 7 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
The ingress_sandbox_traffic_weight variable lacks validation to ensure it remains within the 0-100 range, which can result in negative weights and deployment failures in Google Cloud backend services if the value exceeds 100. Adding a validation block and setting nullable to false is necessary to ensure the routing logic remains correct.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
tvi
approved these changes
May 19, 2026
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 744000af19
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+315
to
+316
| backend_service = google_compute_backend_service.ingress.self_link | ||
| weight = var.ingress_sandbox_traffic_weight |
There was a problem hiding this comment.
Routing session-paths to google_compute_backend_service.ingress sends a fraction of wildcard sandbox traffic through the ingress backend, but the sandbox rate-limit rules are only attached to the session policy (google_compute_security_policy_rule.sandbox-throttling-host/ip target google_compute_security_policy.default["session"]). Because the ingress backend uses a different policy (google_compute_security_policy.ingress), requests routed by this new weighted branch bypass those throttles, which weakens abuse protection whenever ingress_sandbox_traffic_weight > 0.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
this is relevant, blocking merge
djeebus
reviewed
May 19, 2026
Comment on lines
+16
to
+17
| ingress_cpu_count = var.ingress_cpu_count == null ? 1 : var.ingress_cpu_count | ||
| ingress_memory_mb = var.ingress_memory_mb == null ? 512 : var.ingress_memory_mb |
Contributor
There was a problem hiding this comment.
why the extra effort around defaults? passing nulls at the root and then setting defaults in code in the module hides what those defaults are, and complicates the code. if you set the defaults at the root, we may have to do it in two places (aws and gcp) but keeping all the defaults at the root simplifies reading the code.
Member
Author
There was a problem hiding this comment.
good point i will re do it that way
djeebus
reviewed
May 19, 2026
Comment on lines
+113
to
+117
| variable "ingress_sandbox_traffic_weight" { | ||
| type = number | ||
| description = "Weight (out of 100) of sandbox traffic routed to the ingress backend. The remainder goes to the session backend." | ||
| default = 0 | ||
| } |
Contributor
There was a problem hiding this comment.
If it's out of 100, probably makes more sense to call it a percentage instead of a weight.
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
4 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.
INGRESS_SANDBOX_TRAFFIC_WEIGHTworks are the handbrake when it's set to 0.Configuration change is not affecting already established connections.