update crc-org/machine module to latest main#130
Conversation
this adds RPC logs and switches the RPC server to listen on unix socket rather than TCP socket Signed-off-by: Anjan Nath <kaludios@gmail.com>
WalkthroughThe vendored machine dependency is updated. Plugin RPC transport moves from localhost TCP to secured Unix sockets, with connection lifecycle logging and structured logging across RPC server operations. ChangesRPC transport and observability
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RegisterDriver
participant UnixSocket
participant NewRPCClientDriver
participant RPCServerDriver
RegisterDriver->>UnixSocket: create secured plugin.sock listener
NewRPCClientDriver->>UnixSocket: rpc.DialHTTP("unix", addr)
UnixSocket->>RPCServerDriver: serve HTTP RPC request
RPCServerDriver-->>NewRPCClientDriver: return RPC response
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)go.modTraceback (most recent call last): 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@vendor/github.com/crc-org/machine/libmachine/drivers/plugin/register_driver.go`:
- Around line 71-92: The RegisterDriver socket setup must avoid the shared
default crc-machine/plugin.sock when multiple plugin client/server pairs run
concurrently. Update localbinary.Executor.Start and RegisterDriver to create or
propagate a unique CRC_SOCKET_DIR (or equivalent unique socket address) per
plugin instance, and ensure the client consumes the same printed address; retain
the existing directory validation for the selected path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c875bf2-d063-44fc-9f4f-b39c928c2c34
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modvendor/github.com/crc-org/machine/libmachine/drivers/base.govendor/github.com/crc-org/machine/libmachine/drivers/plugin/register_driver.govendor/github.com/crc-org/machine/libmachine/drivers/rpc/client_driver.govendor/github.com/crc-org/machine/libmachine/drivers/rpc/server_driver.govendor/modules.txt
| socketDir := os.Getenv("CRC_SOCKET_DIR") | ||
| if socketDir == "" { | ||
| socketDir = filepath.Join(os.TempDir(), "crc-machine") | ||
| } | ||
|
|
||
| if err := os.MkdirAll(socketDir, 0700); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error creating socket directory: %s\n", err) | ||
| os.Exit(1) | ||
| } | ||
| socketDirInfo, err := os.Stat(socketDir) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error checking socket directory: %s\n", err) | ||
| os.Exit(1) | ||
| } | ||
| if !socketDirInfo.IsDir() || socketDirInfo.Mode().Perm()&0077 != 0 { | ||
| fmt.Fprintf(os.Stderr, "Socket directory must be owner-only: %s\n", socketDir) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| socketPath := filepath.Join(socketDir, "plugin.sock") | ||
| os.Remove(socketPath) | ||
| listener, err := net.Listen("unix", socketPath) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the plugin-launcher implementation before inspecting its environment handling.
fd -t f -e go . vendor/github.com/crc-org/machine/libmachine/drivers/plugin/localbinary \
-x ast-grep outline {} --items all
# Confirm whether each launched plugin receives a unique CRC_SOCKET_DIR.
rg -n -C 5 'CRC_SOCKET_DIR|func NewPlugin|func .*Address' \
vendor/github.com/crc-org/machine/libmachine/drivers/plugin/localbinary \
vendor/github.com/crc-org/machine/libmachine/drivers/rpcRepository: crc-org/machine-driver-libvirt
Length of output: 4033
Use isolated socket paths per plugin client/server pair.
localbinary.Executor.Start() only launches the driver binary and sets driver-specific env vars, while RegisterDriver always creates crc-machine/plugin.sock. Concurrent clients can start with the same default socket path and os.Remove/Listen can replace another driver’s listener before the client reads the printed address. Pass/create CRC_SOCKET_DIR or a unique address for each plugin and have the plugin print/consume that unique address.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@vendor/github.com/crc-org/machine/libmachine/drivers/plugin/register_driver.go`
around lines 71 - 92, The RegisterDriver socket setup must avoid the shared
default crc-machine/plugin.sock when multiple plugin client/server pairs run
concurrently. Update localbinary.Executor.Start and RegisterDriver to create or
propagate a unique CRC_SOCKET_DIR (or equivalent unique socket address) per
plugin instance, and ensure the client consumes the same printed address; retain
the existing directory validation for the selected path.
|
|
||
| require ( | ||
| github.com/crc-org/machine v0.0.0-20240926103419-a943b47fd48b | ||
| github.com/crc-org/machine v0.0.0-20260721135927-5bcb8a00e0f1 |
2 participants
this adds RPC logs and switches the RPC server
to listen on unix socket rather than TCP socket
Summary by CodeRabbit
Security & Reliability
Diagnostics
Maintenance