Skip to content

chore(staticcheck): resolve SA1019 in libvirt and generated protobuf #148

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

Open
jackthepunished wants to merge 5 commits into
base: main
Choose a base branch
from
Open

chore(staticcheck): resolve SA1019 in libvirt and generated protobuf #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
64dfd86
chore(staticcheck): resolve SA1019 warnings in libvirt and generated …
jackthepunished Apr 14, 2026
016fd33
ci(benchmarks): disable gh-pages auto-push on fork PRs
jackthepunished Apr 14, 2026
22aa23b
Merge main into pr-148
poyrazK Apr 25, 2026
5d84594
test(libvirt): add unit tests for libvirtUnixDialer and injectable So…
poyrazK Apr 26, 2026
534ea70
fix(lint): address testifylint issues in dialer tests
poyrazK Apr 26, 2026
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
3 changes: 2 additions & 1 deletion .github/workflows/benchmarks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ jobs:
output-file-path: bench.txt
# Access token to push results to gh-pages branch
github-token: ${{ secrets.GITHUB_TOKEN }}
auto-push: true
# Fork PRs cannot push to upstream gh-pages with GITHUB_TOKEN.
auto-push: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
# Fail if performance drops by more than 50%
alert-threshold: '200%'
comment-on-alert: true
Expand Down
2 changes: 2 additions & 0 deletions internal/autoscaler/protos/externalgrpc.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

87 changes: 66 additions & 21 deletions internal/repositories/libvirt/adapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,38 @@ import (
"time"

"github.com/digitalocean/go-libvirt"
libvirtsocket "github.com/digitalocean/go-libvirt/socket"
"github.com/google/uuid"
"github.com/poyrazk/thecloud/internal/core/ports"
)

type libvirtUnixDialer struct {
uri string
timeout time.Duration
}

var _ libvirtsocket.Dialer = (*libvirtUnixDialer)(nil)

func (d *libvirtUnixDialer) Dial() (net.Conn, error) {
return net.DialTimeout("unix", d.uri, d.timeout)
}
Comment on lines +25 to +39
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Pinned go-libvirt dependency entries:"
rg -n --no-heading 'github.com/digitalocean/go-libvirt' go.mod go.sum

echo
echo "New libvirt APIs referenced by this patch:"
sed -n '20,140p' internal/repositories/libvirt/adapter.go | rg -n 'NewWithDialer|go-libvirt/socket'

Repository: poyrazK/thecloud

Length of output: 636

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify the exact pinned version
echo "=== Pinned go-libvirt version ==="
rg 'github.com/digitalocean/go-libvirt' go.mod | head -1

# Get the full context around lines 120-130 to see the silent Close() issue
echo ""
echo "=== Code around line 125-126 (silent failure context) ==="
sed -n '115,135p' internal/repositories/libvirt/adapter.go

# Search for all usages of libvirtUnixDialer to understand its scope
echo ""
echo "=== All references to libvirtUnixDialer ==="
rg -n 'libvirtUnixDialer' internal/repositories/libvirt/

# Check imports in the file
echo ""
echo "=== Current imports in adapter.go ==="
sed -n '1,35p' internal/repositories/libvirt/adapter.go

Repository: poyrazK/thecloud

Length of output: 2113

🌐 Web query:

go-libvirt socket.Dialer dialers.NewLocal WithSocket WithLocalTimeout API documentation

💡 Result:

The go-libvirt library (github.com/digitalocean/go-libvirt) provides a socket dialing API in the socket/dialers subpackage for connecting to libvirt daemons via UNIX sockets, TCP, TLS, SSH, etc. Key components: - socket.Dialer is the interface: type Dialer interface { Dial (net.Conn, error) } - dialers.NewLocal(opts ...LocalOption) creates a local (UNIX socket) dialer. It is the default for connecting to a locally running libvirt socket. - dialers.WithSocket(socket string) LocalOption: Specifies a custom UNIX socket path (e.g., dialers.NewLocal(dialers.WithSocket("/var/run/libvirt/virtqemud-sock")) for modular daemons). - dialers.WithLocalTimeout(timeout time.Duration) LocalOption: Sets the dial timeout for the local connection. Usage example from sources: dialer := dialers.NewLocal(dialers.WithSocket("/path/to/socket"), dialers.WithLocalTimeout(5*time.Second)) lv := libvirt.NewWithDialer(dialer) lv.Connect This is used internally by libvirt.ConnectToURI for "unix" transports, parsing ?socket= from the URI. Official documentation: pkg.go.dev/github.com/digitalocean/go-libvirt/socket/dialers (updated Feb 2026). Source code examples in github.com/digitalocean/go-libvirt/tree/main/socket/dialers and connect_uri.go.

Citations:

Handle the error from Close() at line 125; do not silently ignore it.

The probe connection must have its error handled explicitly. Currently _ = c.Close() violates the guideline prohibiting silent failures via blank identifier assignment.

If the error is intentionally ignored (e.g., best-effort cleanup before reconnection), capture it explicitly or wrap the close in a helper that documents the intent.

Optional: Replace the custom libvirtUnixDialer with the upstream local dialer.

The pinned go-libvirt version (v0.0.0-20260106181248-56be673cd6a5) includes dialers.NewLocal(...) and its options (WithSocket, WithLocalTimeout), which provide the same functionality more idiomatically:

Suggested refactoring 
-	libvirtsocket "github.com/digitalocean/go-libvirt/socket"
+	"github.com/digitalocean/go-libvirt/socket/dialers"
@@
-type libvirtUnixDialer struct {
-	uri     string
-	timeout time.Duration
-}
-
-var _ libvirtsocket.Dialer = (*libvirtUnixDialer)(nil)
-
-func (d *libvirtUnixDialer) Dial() (net.Conn, error) {
-	return net.DialTimeout("unix", d.uri, d.timeout)
-}
@@
-	l := libvirt.NewWithDialer(&libvirtUnixDialer{uri: uri, timeout: 2 * time.Second})
+	l := libvirt.NewWithDialer(dialers.NewLocal(
+		dialers.WithSocket(uri),
+		dialers.WithLocalTimeout(2*time.Second),
+	))

This removes the custom type and aligns with the upstream library's design.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/repositories/libvirt/adapter.go` around lines 25 - 39, The call to
c.Close() (the probe connection close) is silently ignoring its error; change
the code that currently does `_ = c.Close()` to explicitly handle the returned
error from Close() (e.g., capture err := c.Close() and either log it via your
logger or return/wrap it so callers can act on it) or move the close into a
small helper (e.g., closeConn(conn) that documents best-effort cleanup and logs
a non-nil error) so the intent is explicit; additionally consider replacing the
custom libvirtUnixDialer type and its Dial() implementation with the upstream
dialers.NewLocal(...) and its WithSocket/WithLocalTimeout options to remove the
custom Dialer implementation if you prefer the upstream API.


// SocketDialer is the interface for creating network connections.
// It allows dependency injection for testing.
type SocketDialer interface {
Dial() (net.Conn, error)
}

// DialerOption configures NewLibvirtAdapter.
type DialerOption func(*LibvirtAdapter)

// WithSocketDialer injects a custom socket dialer for testing.
func WithSocketDialer(d SocketDialer) DialerOption {
return func(a *LibvirtAdapter) {
a.socketDialer = d
}
}

const (
defaultPoolName = "default"
userDataFileName = "user-data"
Expand Down Expand Up @@ -66,6 +94,9 @@ type LibvirtAdapter struct {
execCommandContext func(ctx context.Context, name string, arg ...string) *exec.Cmd
lookPath func(file string) (string, error)
osOpen func(name string) (*os.File, error)

// Socket dialer for testability
socketDialer SocketDialer
}

func (a *LibvirtAdapter) recordPortMapping(name string, hPortStr string, cPort string) error {
Expand All @@ -85,35 +116,16 @@ func (a *LibvirtAdapter) recordPortMapping(name string, hPortStr string, cPort s
}

// NewLibvirtAdapter creates a LibvirtAdapter connected to the provided URI.
func NewLibvirtAdapter(logger *slog.Logger, uri string) (*LibvirtAdapter, error) {
func NewLibvirtAdapter(logger *slog.Logger, uri string, opts ...DialerOption) (*LibvirtAdapter, error) {
if uri == "" {
uri = os.Getenv("LIBVIRT_URI")
}
if uri == "" {
uri = "/var/run/libvirt/libvirt-sock"
}

// Connect to libvirt socket
c, err := net.DialTimeout("unix", uri, 2*time.Second)
if err != nil {
// Fallback to session mode if system socket fails
if !strings.Contains(uri, "session") {
sessionUri := filepath.Join(os.Getenv("HOME"), ".cache/libvirt/libvirt-sock")
if c2, err2 := net.DialTimeout("unix", sessionUri, 2*time.Second); err2 == nil {
c = c2
uri = sessionUri
} else {
return nil, fmt.Errorf("failed to dial libvirt (system and session): %w", err)
}
} else {
return nil, fmt.Errorf("failed to dial libvirt: %w", err)
}
}

//nolint:staticcheck
l := libvirt.New(c)
// Create adapter with default dialer first (needed for option application)
adapter := &LibvirtAdapter{
client: &RealLibvirtClient{conn: l},
logger: logger,
uri: uri,
portMappings: make(map[string]map[string]int),
Expand All @@ -128,6 +140,39 @@ func NewLibvirtAdapter(logger *slog.Logger, uri string) (*LibvirtAdapter, error)
osOpen: os.Open,
}

// Apply options (e.g., inject mock dialer for testing)
for _, opt := range opts {
opt(adapter)
}

// Determine dialer to use
var dialer SocketDialer
if adapter.socketDialer != nil {
dialer = adapter.socketDialer
} else {
dialer = &libvirtUnixDialer{uri: uri, timeout: 2 * time.Second}
}

// Verify connectivity
if _, err := dialer.Dial(); err != nil {
// Fallback to session mode if system socket fails
if !strings.Contains(uri, "session") {
sessionUri := filepath.Join(os.Getenv("HOME"), ".cache/libvirt/libvirt-sock")
sessionDialer := &libvirtUnixDialer{uri: sessionUri, timeout: 2 * time.Second}
if _, err2 := sessionDialer.Dial(); err2 == nil {
dialer = sessionDialer
adapter.uri = sessionUri
} else {
return nil, fmt.Errorf("failed to dial libvirt (system and session): %w", err)
}
} else {
return nil, fmt.Errorf("failed to dial libvirt: %w", err)
}
}

l := libvirt.NewWithDialer(dialer)
adapter.client = &RealLibvirtClient{conn: l}

connectCtx, connectCancel := context.WithTimeout(context.Background(), 10*time.Second)
defer connectCancel()

Expand Down
75 changes: 75 additions & 0 deletions internal/repositories/libvirt/adapter_unit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,3 +829,78 @@ func TestLibvirtAdapter_IsNotFound(t *testing.T) {
assert.False(t, a.isNotFound(err))
})
}

func TestLibvirtUnixDialer(t *testing.T) {
t.Parallel()

t.Run("ImplementsSocketDialerInterface", func(t *testing.T) {
t.Parallel()
var dialer SocketDialer = &libvirtUnixDialer{uri: "/var/run/libvirt/libvirt-sock", timeout: 2 * time.Second}
assert.NotNil(t, dialer)
})

t.Run("DialTimeoutReturnsConn", func(t *testing.T) {
t.Parallel()
// This test requires a real Unix socket to exist.
// If /var/run/libvirt/libvirt-sock doesn't exist, this will fail.
// In CI without libvirt, we expect this to fail gracefully.
dialer := &libvirtUnixDialer{uri: "/var/run/libvirt/libvirt-sock", timeout: 100 * time.Millisecond}
conn, err := dialer.Dial()
if err != nil {
// Expected if no libvirt socket exists (e.g., in CI without libvirt)
return
}
require.NotNil(t, conn)
conn.Close()
})

t.Run("DialInvalidPathReturnsError", func(t *testing.T) {
t.Parallel()
dialer := &libvirtUnixDialer{uri: "/nonexistent/path/to/socket", timeout: 100 * time.Millisecond}
conn, err := dialer.Dial()
require.Error(t, err)
assert.Nil(t, conn)
})

t.Run("DialWithVeryShortTimeout", func(t *testing.T) {
t.Parallel()
dialer := &libvirtUnixDialer{uri: "/var/run/libvirt/libvirt-sock", timeout: 1 * time.Millisecond}
conn, err := dialer.Dial()
// Should either succeed quickly or timeout/fail
if err != nil {
// Expected - socket unreachable or timeout
return
}
require.NotNil(t, conn)
conn.Close()
})
}

func TestWithSocketDialerOption(t *testing.T) {
t.Parallel()

t.Run("SetsSocketDialerField", func(t *testing.T) {
t.Parallel()
logger := slog.New(slog.NewTextHandler(io.Discard, nil))

// Create a mock dialer
mockDialer := &mockSocketDialer{err: fmt.Errorf("test error")}

// Create adapter with the option
a := &LibvirtAdapter{logger: logger}
WithSocketDialer(mockDialer)(a)

// Verify the dialer was set
assert.Equal(t, mockDialer, a.socketDialer)
})
}

// mockSocketDialer is a test double for SocketDialer.
type mockSocketDialer struct {
conn net.Conn
err error
}

func (m *mockSocketDialer) Dial() (net.Conn, error) {
return m.conn, m.err
}
Loading