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
+145 −22
Open

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

Show file tree
Hide file tree
Changes from 1 commit
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
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.

16 changes: 14 additions & 2 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,22 @@ 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

@coderabbitai coderabbitai Bot Apr 14, 2026

Copy link
Copy Markdown

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.


const (
defaultPoolName = "default"
userDataFileName = "user-data"
Expand Down Expand Up @@ -110,8 +122,8 @@ func NewLibvirtAdapter(logger *slog.Logger, uri string) (*LibvirtAdapter, error)
}
}

//nolint:staticcheck
l := libvirt.New(c)
_ = c.Close()

@coderabbitai coderabbitai Bot Apr 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Don't silently drop the probe socket Close() failure.

Line 125 discards the only signal that the preflight socket cleanup failed. At least log it so init-time socket issues do not disappear during adapter setup.

🛠️ Suggested fix 
-	_ = c.Close()
+	if err := c.Close(); err != nil {
+		logger.Warn("failed to close preflight libvirt socket", "uri", uri, "error", err)
+	}

As per coding guidelines, "Do not use silent failures - avoid blank identifier assignment like _ = someFunc()".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_ = c.Close()
if err := c.Close(); err != nil {
logger.Warn("failed to close preflight libvirt socket", "uri", uri, "error", err)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/repositories/libvirt/adapter.go` at line 125, The probe socket
Close() call currently discards any error with “_ = c.Close()”; change this to
capture the returned error from c.Close() (e.g., err := c.Close()) and handle it
instead of ignoring it — at minimum call the adapter's logger (or standard log)
to record the failure (e.g., logger.Errorf / log.Printf with context like "probe
socket close failed") and, if appropriate for the surrounding function (e.g., in
the adapter setup/cleanup code where c is closed), propagate the error or wrap
it before returning so init-time socket issues are not lost.

l := libvirt.NewWithDialer(&libvirtUnixDialer{uri: uri, timeout: 2 * time.Second})

Copilot AI Apr 14, 2026

Copy link

Choose a reason for hiding this comment

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

NewLibvirtAdapter now opens a UNIX socket connection (c) only to close it immediately before creating the libvirt client via NewWithDialer. This creates an extra connection attempt at startup and duplicates the timeout value in multiple places. Consider removing the eager DialTimeout/Close() (or moving the system/session fallback selection into the dialer) and centralizing the timeout into a constant to avoid divergence.

Copilot uses AI. Check for mistakes.
adapter := &LibvirtAdapter{
client: &RealLibvirtClient{conn: l},
logger: logger,
Expand Down
Loading