chore(release): v0.1.11 - #521
Conversation
| if caFile := os.Getenv(schedulerMetricsCAFileEnv); caFile != "" { | ||
| caData, err := os.ReadFile(caFile) //nolint:gosec // operator-provided path via env var, not user input. | ||
| pool := x509.NewCertPool() | ||
| if err == nil && pool.AppendCertsFromPEM(caData) { | ||
| tlsConfig.RootCAs = pool | ||
| } else { | ||
| tlsConfig.InsecureSkipVerify = true //nolint:gosec // see schedulerMetricsCAFileEnv doc comment. | ||
| } | ||
| } else { | ||
| tlsConfig.InsecureSkipVerify = true //nolint:gosec // see schedulerMetricsCAFileEnv doc comment. | ||
| } |
There was a problem hiding this comment.
💡 Security: CA file load failure silently falls back to InsecureSkipVerify
In NewSchedulerMetricsHTTPClient, when SCHEDULER_METRICS_CA_FILE is set but os.ReadFile errors (bad path, permissions) or the PEM fails to parse, the error is swallowed (only err == nil is tested) and TLS silently drops to InsecureSkipVerify=true. An operator who explicitly configures a CA bundle to enable verification gets an unverified connection with no log line explaining why — a false sense of security and no diagnostic. Log the read/parse failure (and consider failing closed) instead of silently degrading.
Log why verification was disabled when a configured CA bundle cannot be used.:
if caFile := os.Getenv(schedulerMetricsCAFileEnv); caFile != "" {
caData, err := os.ReadFile(caFile) //nolint:gosec // operator-provided path via env var, not user input.
pool := x509.NewCertPool()
if err == nil && pool.AppendCertsFromPEM(caData) {
tlsConfig.RootCAs = pool
} else {
if err != nil {
klog.Errorf("scheduler metrics CA file %q could not be loaded, falling back to InsecureSkipVerify: %v", caFile, err)
} else {
klog.Errorf("scheduler metrics CA file %q contained no usable certificates, falling back to InsecureSkipVerify", caFile)
}
tlsConfig.InsecureSkipVerify = true //nolint:gosec // see schedulerMetricsCAFileEnv doc comment.
}
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsAdvances the release tag to v0.1.11, introducing scheduler metrics collection and MIG-partitioned GPU enhancements. Consider addressing the silent fallback to InsecureSkipVerify when the CA file fails to load in NewSchedulerMetricsHTTPClient. 💡 Security: CA file load failure silently falls back to InsecureSkipVerify📄 internal/collector/scheduler_metrics_collector.go:317-327 In NewSchedulerMetricsHTTPClient, when SCHEDULER_METRICS_CA_FILE is set but os.ReadFile errors (bad path, permissions) or the PEM fails to parse, the error is swallowed (only Log why verification was disabled when a configured CA bundle cannot be used.🤖 Prompt for agentsWas this helpful? React with 👍 / 👎 | Gitar |
| if err == nil && pool.AppendCertsFromPEM(caData) { | ||
| tlsConfig.RootCAs = pool | ||
| } else { | ||
| tlsConfig.InsecureSkipVerify = true //nolint:gosec // see schedulerMetricsCAFileEnv doc comment. |
| tlsConfig.InsecureSkipVerify = true //nolint:gosec // see schedulerMetricsCAFileEnv doc comment. | ||
| } | ||
| } else { | ||
| tlsConfig.InsecureSkipVerify = true //nolint:gosec // see schedulerMetricsCAFileEnv doc comment. |
Release v0.1.11 of zxporter, published from the internal services monorepo.
The
v0.1.11tag has already been created and points at this branch's tip — ArgoCD and other tag consumers can use it immediately. This PR is only to advance public main to the released commit for browser convenience.Summary by Gitar
SchedulerMetricsCollectorto scrape and emit scheduler placement metrics fromdz-schedulerSCHEDULER_METRICS_SCRAPE_INTERVALconfig and corresponding RBAC leases/metrics reader permissionsNVIDIA_VISIBLE_DEVICESfor MIG-partitioned GPUsPodNominatedderivation from disruption blocking logicThis will update automatically on new commits.