mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-03 04:38:48 +00:00
fix(config): keep operator VerifySSL across PVE consolidation
ConsolidatePVEInstances promoted VerifySSL false->true when folding a duplicate cluster or overlapping standalone into the canonical instance, silently re-enabling a disabled Verify SSL certificate setting on every save, load and monitor reconciliation (#2140). The canonical instance now owns the preference; a merged fingerprint still pins the peer, so verification is not downgraded. Regression tests cover the standalone and duplicate-cluster merges and the reported PUT-save path through normalizePVEConfigState. Change-source: pulse-maintainer
This commit is contained in:
pulse-triage[bot]
parent
a211d2ea17
commit
10ad85e0cf
4 changed files with 161 additions and 7 deletions
|
|
@ -3409,6 +3409,17 @@ take the captured value. Regression tests
|
|||
`TestHandleCanonicalAutoRegister_PBSReservesDisabledVerifySSL` pin the existing
|
||||
node branch.
|
||||
|
||||
`ConsolidatePVEInstances` keeps the canonical (destination) instance's
|
||||
`VerifySSL` when it folds a duplicate cluster or an overlapping standalone into
|
||||
it. A merge no longer promotes `VerifySSL` from `false` to `true` on the
|
||||
surviving instance, so a disabled setting is not silently re-enabled on every
|
||||
save, load and monitor reconciliation. Certificate pinning is carried separately
|
||||
by the merged `Fingerprint`, so a pin still verifies the peer. Regression tests
|
||||
`TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnStandaloneMerge`,
|
||||
`TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnDuplicateClusterMerge`
|
||||
and `TestHandleUpdateNodePreservesDisabledVerifySSLThroughConsolidation` cover
|
||||
the merge and save paths.
|
||||
|
||||
### Manual update freshness
|
||||
|
||||
Server update-check freshness is owned by internal/updates and its API adapter.
|
||||
|
|
|
|||
|
|
@ -541,3 +541,60 @@ func TestHandleUpdateNode_UserOnlyEditDoesNotClearProxmoxTokenAuth(t *testing.T)
|
|||
t.Fatalf("PMG monitor mail stats should preserve false when omitted")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleUpdateNodePreservesDisabledVerifySSLThroughConsolidation covers
|
||||
// #2140 end to end through the reported save path: an operator disables
|
||||
// "Verify SSL certificate" on a cluster that also has an overlapping
|
||||
// auto-registered standalone entry, saves, and reopens Manage. The save runs
|
||||
// normalizePVEConfigState, which folds the standalone into the cluster; the
|
||||
// disabled setting must survive that consolidation.
|
||||
func TestHandleUpdateNodePreservesDisabledVerifySSLThroughConsolidation(t *testing.T) {
|
||||
tempDir := t.TempDir()
|
||||
cfg := &config.Config{
|
||||
DataPath: tempDir,
|
||||
PVEInstances: []config.PVEInstance{
|
||||
{
|
||||
Name: "homelab",
|
||||
ClusterName: "cluster-A",
|
||||
IsCluster: true,
|
||||
VerifySSL: true,
|
||||
ClusterEndpoints: []config.ClusterEndpoint{
|
||||
{NodeName: "minipc", Host: "https://10.0.0.5:8006"},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "minipc-standalone",
|
||||
Host: "10.0.0.5",
|
||||
Fingerprint: "fp-standalone",
|
||||
TokenName: "pulse@pve!token",
|
||||
TokenValue: "secret",
|
||||
VerifySSL: true,
|
||||
Source: "agent",
|
||||
},
|
||||
},
|
||||
}
|
||||
handler := newTestConfigHandlers(t, cfg)
|
||||
|
||||
body, _ := json.Marshal(map[string]any{"verifySSL": false})
|
||||
req := httptest.NewRequest(http.MethodPut, "/api/config/nodes/pve-0", bytes.NewBuffer(body))
|
||||
rec := httptest.NewRecorder()
|
||||
handler.HandleUpdateNode(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("update status = %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if len(cfg.PVEInstances) != 1 {
|
||||
t.Fatalf("instances = %d, want 1 after consolidation", len(cfg.PVEInstances))
|
||||
}
|
||||
if cfg.PVEInstances[0].VerifySSL {
|
||||
t.Fatalf("save re-enabled the disabled VerifySSL through consolidation (#2140)")
|
||||
}
|
||||
|
||||
nodes := handler.GetAllNodesForAPI(req.Context())
|
||||
if len(nodes) != 1 {
|
||||
t.Fatalf("nodes = %d, want 1", len(nodes))
|
||||
}
|
||||
if nodes[0].VerifySSL {
|
||||
t.Fatalf("GET projection VerifySSL = true, want false after disabling (#2140)")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -318,9 +318,13 @@ func mergePVEInstanceData(dst *PVEInstance, src PVEInstance) {
|
|||
if dst.Source == "" && strings.TrimSpace(src.Source) != "" {
|
||||
dst.Source = strings.TrimSpace(src.Source)
|
||||
}
|
||||
if !dst.VerifySSL && src.VerifySSL {
|
||||
dst.VerifySSL = true
|
||||
}
|
||||
// The canonical (destination) instance owns the TLS verification
|
||||
// preference. Merging a duplicate or standalone must not flip a disabled
|
||||
// "Verify SSL certificate" setting back on: doing so silently re-enabled
|
||||
// the option on every save, load and monitor reconciliation, and broke
|
||||
// endpoints whose operator had deliberately opted out (#2140). Certificate
|
||||
// pinning is carried separately by the Fingerprint copy above, so a merged
|
||||
// pin still verifies the peer even when VerifySSL stays false.
|
||||
if dst.TemperatureMonitoringEnabled == nil && src.TemperatureMonitoringEnabled != nil {
|
||||
enabled := *src.TemperatureMonitoringEnabled
|
||||
dst.TemperatureMonitoringEnabled = &enabled
|
||||
|
|
|
|||
|
|
@ -88,8 +88,8 @@ func TestConsolidatePVEInstancesRemovesStandaloneCoveredByClusterEndpoint(t *tes
|
|||
if got := instances[0].TokenValue; got != "secret" {
|
||||
t.Fatalf("TokenValue = %q, want secret", got)
|
||||
}
|
||||
if !instances[0].VerifySSL {
|
||||
t.Fatalf("expected VerifySSL to be promoted")
|
||||
if instances[0].VerifySSL {
|
||||
t.Fatalf("expected the canonical cluster's disabled VerifySSL to survive the standalone merge (#2140)")
|
||||
}
|
||||
if got := instances[0].Source; got != "agent" {
|
||||
t.Fatalf("Source = %q, want agent", got)
|
||||
|
|
@ -196,8 +196,8 @@ func TestConsolidatePVEInstancesMergesDuplicateClusterAuthIntoPrimary(t *testing
|
|||
if got := instances[0].Fingerprint; got != "fp-1" {
|
||||
t.Fatalf("Fingerprint = %q, want fp-1", got)
|
||||
}
|
||||
if !instances[0].VerifySSL {
|
||||
t.Fatalf("expected VerifySSL to be promoted")
|
||||
if instances[0].VerifySSL {
|
||||
t.Fatalf("expected the primary cluster's disabled VerifySSL to survive the duplicate merge (#2140)")
|
||||
}
|
||||
if len(instances[0].ClusterEndpoints) != 2 {
|
||||
t.Fatalf("expected 2 endpoints after consolidation, got %d", len(instances[0].ClusterEndpoints))
|
||||
|
|
@ -286,3 +286,85 @@ func TestConsolidatePVEInstancesKeepsStandaloneWithContradictingFingerprint(t *t
|
|||
t.Fatalf("expected both instances to remain, got %d", len(instances))
|
||||
}
|
||||
}
|
||||
|
||||
// TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnStandaloneMerge covers
|
||||
// #2140: a standalone/auto-registered entry that captured a fingerprint is
|
||||
// created with VerifySSL true. Folding it into a cluster whose operator
|
||||
// disabled "Verify SSL certificate" must not re-enable the setting. The
|
||||
// captured pin still moves onto the cluster endpoint, so verification is not
|
||||
// silently downgraded.
|
||||
func TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnStandaloneMerge(t *testing.T) {
|
||||
instances, changed := ConsolidatePVEInstances([]PVEInstance{
|
||||
{
|
||||
Name: "homelab",
|
||||
ClusterName: "cluster-A",
|
||||
IsCluster: true,
|
||||
VerifySSL: false,
|
||||
ClusterEndpoints: []ClusterEndpoint{
|
||||
{NodeName: "minipc", Host: "https://10.0.0.5:8006"},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "minipc-standalone",
|
||||
Host: "10.0.0.5",
|
||||
Fingerprint: "fp-standalone",
|
||||
TokenName: "pulse@pve!token",
|
||||
TokenValue: "secret",
|
||||
VerifySSL: true,
|
||||
Source: "agent",
|
||||
},
|
||||
})
|
||||
|
||||
if !changed {
|
||||
t.Fatalf("expected consolidation change")
|
||||
}
|
||||
if len(instances) != 1 {
|
||||
t.Fatalf("expected 1 instance after consolidation, got %d", len(instances))
|
||||
}
|
||||
if instances[0].VerifySSL {
|
||||
t.Fatalf("standalone merge re-enabled a disabled VerifySSL (#2140)")
|
||||
}
|
||||
if got := instances[0].ClusterEndpoints[0].Fingerprint; got != "fp-standalone" {
|
||||
t.Fatalf("ClusterEndpoint Fingerprint = %q, want fp-standalone (pin must survive)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnDuplicateClusterMerge
|
||||
// is the duplicate-cluster twin of the #2140 regression above.
|
||||
func TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnDuplicateClusterMerge(t *testing.T) {
|
||||
instances, changed := ConsolidatePVEInstances([]PVEInstance{
|
||||
{
|
||||
Name: "c1",
|
||||
ClusterName: "cluster-A",
|
||||
IsCluster: true,
|
||||
VerifySSL: false,
|
||||
ClusterEndpoints: []ClusterEndpoint{
|
||||
{NodeName: "n1", Host: "https://10.0.0.5:8006"},
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "c2",
|
||||
ClusterName: "cluster-A",
|
||||
IsCluster: true,
|
||||
VerifySSL: true,
|
||||
Fingerprint: "fp-1",
|
||||
ClusterEndpoints: []ClusterEndpoint{
|
||||
{NodeName: "n1", Host: "https://10.0.0.5:8006"},
|
||||
{NodeName: "n2", Host: "https://10.0.0.6:8006"},
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
if !changed {
|
||||
t.Fatalf("expected consolidation change")
|
||||
}
|
||||
if len(instances) != 1 {
|
||||
t.Fatalf("expected 1 instance after consolidation, got %d", len(instances))
|
||||
}
|
||||
if instances[0].VerifySSL {
|
||||
t.Fatalf("duplicate-cluster merge re-enabled a disabled VerifySSL (#2140)")
|
||||
}
|
||||
if got := instances[0].Fingerprint; got != "fp-1" {
|
||||
t.Fatalf("Fingerprint = %q, want fp-1 (pin must survive)", got)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue