Add sandbox routing protobuf messages to ControlMessage #148
Conversation
📝 WalkthroughWalkthroughAdded sandbox routing protobuf messages and integrated them into control messaging; implemented a client-side API and plumbing to send a RouteSandbox request and receive its response; and added a standalone DNS CLI tool that sends DNS Add actions via a Redis-backed DNS service. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Caller
participant GC as GravityClient
participant Srv as Remote Gravity Server
participant Ctrl as ControlMessage Dispatcher
Note right of GC `#DDEBF7`: SendRouteSandboxRequest creates pending entry\nand sends request via gRPC
App->>GC: SendRouteSandboxRequest(sandboxID, virtualIP, timeout)
GC->>Srv: RPC: RouteSandboxRequest{sandbox_id, virtual_ip}
alt Server processes request
Srv->>Ctrl: emit ControlMessage(route_sandbox_response)
Ctrl->>GC: delivers ControlMessage(RouteSandboxResponse{ip})
GC->>GC: handleRouteSandboxResponse -> resolve pending chan
GC-->>App: returns RouteSandboxResponse{ip}
else Timeout / no response
GC-->>App: returns error (timeout)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
Comment |
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @gravity/proto/gravity.proto:
- Around line 567-570: Add a clarifying comment inside the RouteSandboxRequest
protobuf message to document why field number 2 is skipped: update the
RouteSandboxRequest definition (message RouteSandboxRequest) to include a
comment like "Field 2 reserved for alignment with RouteDeploymentRequest"
between sandbox_id (field 1) and virtual_ip (field 3) so readers know the gap is
intentional and mirrors the deprecated hostname field in RouteDeploymentRequest.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
gravity/proto/gravity.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (1)
gravity/proto/gravity.proto
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
gravity/proto/gravity.proto (3)
74-75: LGTM! Clean integration into ControlMessage.The new sandbox routing messages are properly integrated into the
ControlMessageoneof with appropriate field numbers (26, 27) within the deployment management range.
140-140: LGTM! Clear deprecation notice.The explicit deprecation comment helps prevent this field from being used in new implementations.
572-572: LGTM! Consistent with existing response pattern.The
RouteSandboxResponsemessage properly mirrors the structure ofRouteDeploymentResponse, maintaining consistency across similar operations.
| message RouteSandboxRequest { | ||
| string sandbox_id = 1; | ||
| string virtual_ip = 3; // Hadron virtual IP for the sandbox | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's check the structure of the proto file and find the relevant messages
cat -n gravity/proto/gravity.proto | sed -n '560,580p'Repository: agentuity/go-common
Length of output: 690
🏁 Script executed:
#!/bin/bash
# Now let's search for RouteDeploymentRequest to compare the structure
rg -n 'message RouteDeploymentRequest' gravity/proto/gravity.proto -A 5Repository: agentuity/go-common
Length of output: 328
🏁 Script executed:
#!/bin/bash
# Check for reserved field declarations in the proto file
rg -n 'reserved' gravity/proto/gravity.protoRepository: agentuity/go-common
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for messages with non-sequential field numbers
rg -n 'message \w+' gravity/proto/gravity.proto | head -20Repository: agentuity/go-common
Length of output: 847
🏁 Script executed:
#!/bin/bash
# Get a more detailed view to check field number patterns
rg -B 1 '= [23456789] *;' gravity/proto/gravity.proto | grep -E '(message|= [0-9])' | head -30Repository: agentuity/go-common
Length of output: 2052
🏁 Script executed:
#!/bin/bash
# Check specific messages with full structure
rg -n 'message ProtocolResponse' gravity/proto/gravity.proto -A 5Repository: agentuity/go-common
Length of output: 402
🏁 Script executed:
#!/bin/bash
# Check ConnectResponse which might have skipped fields
rg -n 'message ConnectResponse' gravity/proto/gravity.proto -A 10Repository: agentuity/go-common
Length of output: 843
🏁 Script executed:
#!/bin/bash
# Get overall structure - check if skipped fields are common
python3 << 'EOF'
import re
with open('gravity/proto/gravity.proto', 'r') as f:
content = f.read()
# Find all message definitions and their fields
messages = re.finditer(r'message\s+(\w+)\s*\{([^}]+)\}', content, re.MULTILINE)
for match in messages:
msg_name = match.group(1)
msg_body = match.group(2)
# Extract field numbers
fields = re.findall(r'=\s*(\d+)\s*;', msg_body)
if fields and len(fields) > 1:
field_nums = [int(f) for f in fields]
# Check for gaps
expected = set(range(1, max(field_nums) + 1))
actual = set(field_nums)
gaps = expected - actual
if gaps:
print(f"{msg_name}: Fields {sorted(field_nums)}, Missing: {sorted(gaps)}")
EOFRepository: agentuity/go-common
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Get more context around ConnectResponse to see if there's any comment about field 2
sed -n '113,145p' gravity/proto/gravity.protoRepository: agentuity/go-common
Length of output: 1667
🏁 Script executed:
#!/bin/bash
# Search for any comments or documentation about removed/deprecated fields
rg -n 'field 2|skip.*2|deprecated' gravity/proto/gravity.protoRepository: agentuity/go-common
Length of output: 94
Add a comment documenting the skipped field number.
The RouteSandboxRequest message intentionally skips field number 2 to mirror the structure of RouteDeploymentRequest, which has a deprecated hostname field at position 2. However, this gap lacks documentation. Add a comment to clarify the intent:
message RouteSandboxRequest {
string sandbox_id = 1;
// Field 2 reserved for alignment with RouteDeploymentRequest
string virtual_ip = 3; // Hadron virtual IP for the sandbox
}🤖 Prompt for AI Agents
In @gravity/proto/gravity.proto around lines 567-570, Add a clarifying comment
inside the RouteSandboxRequest protobuf message to document why field number 2
is skipped: update the RouteSandboxRequest definition (message
RouteSandboxRequest) to include a comment like "Field 2 reserved for alignment
with RouteDeploymentRequest" between sandbox_id (field 1) and virtual_ip (field
3) so readers know the gap is intentional and mirrors the deprecated hostname
field in RouteDeploymentRequest.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gravity/grpc_client.go (1)
38-45: Global gzip compression level affects all gRPC services in the process.The
grpcgzip.SetLevel()call sets a process-wide default compression level for all gRPC services, not just Gravity control messages. This could cause unexpected behavior in multi-service applications or when this package is imported by other code.Consider either:
- Removing this global configuration and relying on gRPC's default
- Documenting this side effect prominently in package documentation
- Moving compression configuration to per-stream options only (already done at line 463 with
grpc.UseCompressor)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/dns-cli/main.gogravity/grpc_client.go
🧰 Additional context used
🧬 Code graph analysis (2)
cmd/dns-cli/main.go (1)
dns/aether.go (4)
NewAddAction(135-146)RecordTypeCNAME(122-122)SendDNSAction(335-387)WithRedis(257-261)
gravity/grpc_client.go (1)
gravity/proto/gravity.pb.go (13)
RouteSandboxResponse(4065-4070)RouteSandboxResponse(4083-4083)RouteSandboxResponse(4098-4100)ControlMessage_RouteSandboxResponse(510-512)ControlMessage_RouteSandboxResponse(568-568)ControlMessage(244-270)ControlMessage(283-283)ControlMessage(298-300)ControlMessage_RouteSandbox(506-508)ControlMessage_RouteSandbox(566-566)RouteSandboxRequest(4013-4019)RouteSandboxRequest(4032-4032)RouteSandboxRequest(4047-4049)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (8)
cmd/dns-cli/main.go (3)
1-18: LGTM!The CLI argument validation is well-structured with clear usage messages and examples.
20-44: LGTM!The Redis client setup follows best practices with proper connection testing and resource cleanup via defer.
53-55: Code is safe—GetID()properly handles empty slices.The
GetID()method (in dns/aether.go) checksif len(r.IDs) == 0and returns an empty string, making the pre-check at line 53 redundant but harmless. The defensive checklen(result.IDs) > 0is good practice and doesn't need to change.gravity/grpc_client.go (5)
139-141: LGTM!The pending route sandbox fields mirror the existing route deployment pattern, ensuring consistency in the request/response tracking mechanism.
269-269: LGTM!Proper initialization of the
pendingRouteSandboxmap in the constructor, consistent with the existingpendingRouteDeploymentpattern.
711-712: LGTM!The
RouteSandboxResponsecase is correctly integrated into the control message processing switch, following the same pattern asRouteDeploymentResponse.
864-880: LGTM!The
handleRouteSandboxResponseimplementation correctly follows the same pattern ashandleRouteDeploymentResponsewith proper mutex handling and non-blocking channel operations.
1601-1644: LGTM!The
SendRouteSandboxRequestimplementation is correct and follows the established pattern fromSendRouteDeploymentRequestwith proper timeout handling, mutex protection, and cleanup.
Summary by CodeRabbit
New Features
Bug Fixes / Notes
✏️ Tip: You can customize this high-level summary in your review settings.