mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Classify offline Proxmox node failures accurately
Treat node-scoped HTTP 595 responses as debug-level resource unavailability instead of repeated authentication warnings. Preserve warnings and returned errors for real credential failures. Refs #1794. Contract-Neutral: corrects internal log severity without changing API, resource, or extension contracts Change-source: pulse-maintainer
This commit is contained in:
@@ -3422,3 +3422,13 @@ levels continue to route by channel, including Apprise; this compatibility path
|
||||
must not silently skip a supported destination. Monitoring broadcasts the
|
||||
escalated alert after dispatch but does not reinterpret destination identity,
|
||||
retry semantics, acknowledgement, or the critical-repeat cadence.
|
||||
|
||||
### Proxmox node unavailability is not credential evidence
|
||||
|
||||
The Proxmox client treats HTTP 595 from a node-scoped API path as a resource
|
||||
availability failure. This is the pveproxy response when a cluster member is
|
||||
offline or unreachable, so it remains available at debug level without
|
||||
emitting the repeated authentication warning used for cluster-scoped 595,
|
||||
401, and 403 responses. Returned errors retain their compatibility shape;
|
||||
`pkg/proxmox/client_request_test.go` pins both sides of the log classification
|
||||
(#1794).
|
||||
|
||||
+39
-10
@@ -21,6 +21,37 @@ import (
|
||||
|
||||
const maxResponseBodyBytes int64 = 8 << 20 // 8 MiB
|
||||
|
||||
type apiErrorLogLevel uint8
|
||||
|
||||
const (
|
||||
apiErrorLogNone apiErrorLogLevel = iota
|
||||
apiErrorLogDebug
|
||||
apiErrorLogWarn
|
||||
)
|
||||
|
||||
func isNodeScopedAPIPath(path string) bool {
|
||||
parts := strings.Split(strings.Trim(path, "/"), "/")
|
||||
for i, part := range parts {
|
||||
if part == "nodes" && i+1 < len(parts) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func classifyAPIErrorLog(status int, path string) (apiErrorLogLevel, string) {
|
||||
if status == http.StatusForbidden && strings.Contains(path, "/apt/update") {
|
||||
return apiErrorLogDebug, "Proxmox permission error (optional endpoint)"
|
||||
}
|
||||
if status == 595 && isNodeScopedAPIPath(path) {
|
||||
return apiErrorLogDebug, "Proxmox node resource unavailable"
|
||||
}
|
||||
if status == 595 || status == http.StatusUnauthorized || status == http.StatusForbidden {
|
||||
return apiErrorLogWarn, "Proxmox authentication error"
|
||||
}
|
||||
return apiErrorLogNone, ""
|
||||
}
|
||||
|
||||
func readResponseBodyLimited(r io.Reader) ([]byte, error) {
|
||||
body, err := io.ReadAll(io.LimitReader(r, maxResponseBodyBytes+1))
|
||||
if err != nil {
|
||||
@@ -559,7 +590,7 @@ func (c *Client) requestWithRetry(ctx context.Context, method, path string, data
|
||||
} else if resp.StatusCode == 595 {
|
||||
// 595 can mean authentication failed OR trying to access an offline node in a cluster
|
||||
// Check if this is a node-specific endpoint
|
||||
if strings.Contains(req.URL.Path, "/nodes/") && strings.Count(req.URL.Path, "/") > 3 {
|
||||
if isNodeScopedAPIPath(req.URL.Path) {
|
||||
// This looks like a node-specific resource request
|
||||
apiErr = fmt.Errorf("API error 595: Cannot access node resource - node may be offline or credentials may be invalid")
|
||||
} else {
|
||||
@@ -571,17 +602,14 @@ func (c *Client) requestWithRetry(ctx context.Context, method, path string, data
|
||||
apiErr = fmt.Errorf("API error %d: %s", resp.StatusCode, string(body))
|
||||
}
|
||||
|
||||
// Log auth issues for debugging (595 is Proxmox "no ticket" error)
|
||||
if resp.StatusCode == 595 || resp.StatusCode == 401 || resp.StatusCode == 403 {
|
||||
// Some endpoints are optional and may return 403 if the token is intentionally
|
||||
// scoped read-only. Avoid warning-level log spam for those.
|
||||
// Classify the failed operation by its expected impact. A node-scoped 595
|
||||
// is the normal pveproxy response when a cluster member is offline; the
|
||||
// successful cluster request already proves the credential is usable.
|
||||
if level, msg := classifyAPIErrorLog(resp.StatusCode, req.URL.Path); level != apiErrorLogNone {
|
||||
event := log.Warn()
|
||||
msg := "Proxmox authentication error"
|
||||
if resp.StatusCode == 403 && strings.Contains(req.URL.Path, "/apt/update") {
|
||||
if level == apiErrorLogDebug {
|
||||
event = log.Debug()
|
||||
msg = "Proxmox permission error (optional endpoint)"
|
||||
}
|
||||
|
||||
event.
|
||||
Str("url", req.URL.String()).
|
||||
Int("status", resp.StatusCode).
|
||||
@@ -591,7 +619,8 @@ func (c *Client) requestWithRetry(ctx context.Context, method, path string, data
|
||||
Msg(msg)
|
||||
}
|
||||
|
||||
// Wrap with appropriate error type
|
||||
// Preserve the established returned-error classification for callers;
|
||||
// this distinction changes only the emitted diagnostic severity.
|
||||
if resp.StatusCode == 401 || resp.StatusCode == 403 || resp.StatusCode == 595 {
|
||||
// Import errors package at top of file
|
||||
return nil, fmt.Errorf("authentication error: %w", apiErr)
|
||||
|
||||
@@ -70,6 +70,63 @@ func TestClientRequest_595NodeSpecific(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassifyAPIErrorLog(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
status int
|
||||
path string
|
||||
wantLevel apiErrorLogLevel
|
||||
wantMsg string
|
||||
}{
|
||||
{
|
||||
name: "offline node resource is diagnostic",
|
||||
status: 595,
|
||||
path: "/api2/json/nodes/proxmox1/lxc/217/config",
|
||||
wantLevel: apiErrorLogDebug,
|
||||
wantMsg: "Proxmox node resource unavailable",
|
||||
},
|
||||
{
|
||||
name: "cluster scoped 595 remains authentication warning",
|
||||
status: 595,
|
||||
path: "/api2/json/cluster/status",
|
||||
wantLevel: apiErrorLogWarn,
|
||||
wantMsg: "Proxmox authentication error",
|
||||
},
|
||||
{
|
||||
name: "unauthorized remains authentication warning",
|
||||
status: http.StatusUnauthorized,
|
||||
path: "/api2/json/nodes",
|
||||
wantLevel: apiErrorLogWarn,
|
||||
wantMsg: "Proxmox authentication error",
|
||||
},
|
||||
{
|
||||
name: "optional apt permission remains diagnostic",
|
||||
status: http.StatusForbidden,
|
||||
path: "/api2/json/nodes/proxmox1/apt/update",
|
||||
wantLevel: apiErrorLogDebug,
|
||||
wantMsg: "Proxmox permission error (optional endpoint)",
|
||||
},
|
||||
{
|
||||
name: "server error is not logged here",
|
||||
status: http.StatusInternalServerError,
|
||||
path: "/api2/json/nodes",
|
||||
wantLevel: apiErrorLogNone,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
gotLevel, gotMsg := classifyAPIErrorLog(tt.status, tt.path)
|
||||
if gotLevel != tt.wantLevel || gotMsg != tt.wantMsg {
|
||||
t.Fatalf("classifyAPIErrorLog(%d, %q) = (%v, %q), want (%v, %q)", tt.status, tt.path, gotLevel, gotMsg, tt.wantLevel, tt.wantMsg)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestClientRequest_595Auth(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(595)
|
||||
|
||||
Reference in New Issue
Block a user