fix(config): honour explicit operator VerifySSL across PVE merge

ConsolidatePVEInstances promoted VerifySSL false->true whenever a merged duplicate or standalone had it enabled, silently re-enabling a disabled Verify SSL certificate setting on every save, load and monitor reconciliation (#2140). The add and update handlers now record an explicit operator choice in VerifySSLExplicit; a merge never overrides it, while the historical promotion is kept when neither side recorded a choice so an existing secure connection is not downgraded. A merged fingerprint still pins the peer.

Regression tests cover the standalone and duplicate-cluster merges and the reported PUT-save path through normalizePVEConfigState.

Change-source: pulse-maintainer

docs/release-control/v6/internal/subsystems/agent-lifecycle.md

@@ -3409,12 +3409,15 @@ 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
+`ConsolidatePVEInstances` respects an operator's explicit `VerifySSL` choice
+when it folds a duplicate cluster or an overlapping standalone into the
+canonical instance. An explicit choice is recorded as `VerifySSLExplicit` when
+the update or add handler applies a caller-supplied `verifySSL`, and a merge
+never overrides it, so a disabled setting is not silently re-enabled on every
+save, load and monitor reconciliation. When neither side recorded an explicit
+choice the historical promotion is kept, so an existing secure connection is
+not silently downgraded. Certificate pinning is carried separately by the
+merged `Fingerprint`, so a pin still verifies the peer. Regression tests
 `TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnStandaloneMerge`,
 `TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnDuplicateClusterMerge`
 and `TestHandleUpdateNodePreservesDisabledVerifySSLThroughConsolidation` cover

internal/api/configapi/config_node_handlers.go

@@ -516,6 +516,7 @@ func (h *ConfigHandlers) handleAddNode(w http.ResponseWriter, r *http.Request) {
 			TokenValue:                   req.TokenValue,
 			Fingerprint:                  req.Fingerprint,
 			VerifySSL:                    verifySSL,
+			VerifySSLExplicit:            req.VerifySSL != nil,
 			MonitorVMs:                   monitorVMs,
 			MonitorContainers:            monitorContainers,
 			MonitorStorage:               monitorStorage,
@@ -1493,6 +1494,7 @@ func (h *ConfigHandlers) handleUpdateNode(w http.ResponseWriter, r *http.Request
 		}
 		if req.VerifySSL != nil {
 			updated.VerifySSL = *req.VerifySSL
+			updated.VerifySSLExplicit = true
 		}
 		if req.MonitorVMs != nil {
 			updated.MonitorVMs = *req.MonitorVMs

internal/api/configapi/config_setup_handlers.go

@@ -2270,9 +2270,10 @@ func (h *ConfigHandlers) handleCanonicalAutoRegister(w http.ResponseWriter, r *h
 			// captured a fingerprint; applying it unconditionally re-enabled a
 			// disabled "Verify SSL certificate" setting on every agent
 			// health-check re-registration (#2140). Heal only the legacy state
-			// where strict verification is on with no pin, which can never
-			// connect to a self-signed endpoint (#1303).
-			if instance.VerifySSL && instance.Fingerprint == "" {
+			// where strict verification is on with no pin and no recorded
+			// operator choice, which can never connect to a self-signed
+			// endpoint (#1303).
+			if !instance.VerifySSLExplicit && instance.VerifySSL && instance.Fingerprint == "" {
 				instance.VerifySSL = false
 			}
 			log.Info().Str("host", host).Str("type", "pve").Msg(canonicalAutoRegisterMatchMessage("host; updated token in-place"))

internal/config/config.go

@@ -570,6 +570,7 @@ type PVEInstance struct {
 	TokenValue                   string
 	Fingerprint                  string
 	VerifySSL                    bool
+	VerifySSLExplicit            bool `json:"verifySSLExplicit,omitempty"` // operator-chosen VerifySSL; survives automatic consolidation (#2140)
 	MonitorVMs                   bool
 	MonitorContainers            bool
 	MonitorStorage               bool

internal/config/pve_instances.go

@@ -318,13 +318,22 @@ func mergePVEInstanceData(dst *PVEInstance, src PVEInstance) {
 	if dst.Source == "" && strings.TrimSpace(src.Source) != "" {
 		dst.Source = strings.TrimSpace(src.Source)
 	}
-	// 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.
+	// TLS verification preference: an explicit operator choice wins over an
+	// inferred one. When neither side recorded a choice, keep the historical
+	// promotion of a strict source so an existing secure connection is not
+	// silently downgraded. A disabled setting that the operator saved through
+	// the API is explicit and is never re-enabled by a merge (#2140).
+	// Certificate pinning is carried separately by the Fingerprint copy above,
+	// so a merged pin still verifies the peer even when VerifySSL stays false.
+	switch {
+	case dst.VerifySSLExplicit:
+		// The canonical instance's explicit choice is authoritative.
+	case src.VerifySSLExplicit:
+		dst.VerifySSL = src.VerifySSL
+		dst.VerifySSLExplicit = true
+	case !dst.VerifySSL && src.VerifySSL:
+		dst.VerifySSL = true
+	}
 	if dst.TemperatureMonitoringEnabled == nil && src.TemperatureMonitoringEnabled != nil {
 		enabled := *src.TemperatureMonitoringEnabled
 		dst.TemperatureMonitoringEnabled = &enabled

internal/config/pve_instances_test.go

@@ -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 the canonical cluster's disabled VerifySSL to survive the standalone merge (#2140)")
+	if !instances[0].VerifySSL {
+		t.Fatalf("expected VerifySSL to be promoted")
 	}
 	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 the primary cluster's disabled VerifySSL to survive the duplicate merge (#2140)")
+	if !instances[0].VerifySSL {
+		t.Fatalf("expected VerifySSL to be promoted")
 	}
 	if len(instances[0].ClusterEndpoints) != 2 {
 		t.Fatalf("expected 2 endpoints after consolidation, got %d", len(instances[0].ClusterEndpoints))
@@ -296,10 +296,11 @@ func TestConsolidatePVEInstancesKeepsStandaloneWithContradictingFingerprint(t *t
 func TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnStandaloneMerge(t *testing.T) {
 	instances, changed := ConsolidatePVEInstances([]PVEInstance{
 		{
-			Name:        "homelab",
-			ClusterName: "cluster-A",
-			IsCluster:   true,
-			VerifySSL:   false,
+			Name:              "homelab",
+			ClusterName:       "cluster-A",
+			IsCluster:         true,
+			VerifySSL:         false,
+			VerifySSLExplicit: true,
 			ClusterEndpoints: []ClusterEndpoint{
 				{NodeName: "minipc", Host: "https://10.0.0.5:8006"},
 			},
@@ -334,10 +335,11 @@ func TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnStandaloneMerge(t *t
 func TestConsolidatePVEInstancesPreservesDisabledVerifySSLOnDuplicateClusterMerge(t *testing.T) {
 	instances, changed := ConsolidatePVEInstances([]PVEInstance{
 		{
-			Name:        "c1",
-			ClusterName: "cluster-A",
-			IsCluster:   true,
-			VerifySSL:   false,
+			Name:              "c1",
+			ClusterName:       "cluster-A",
+			IsCluster:         true,
+			VerifySSL:         false,
+			VerifySSLExplicit: true,
 			ClusterEndpoints: []ClusterEndpoint{
 				{NodeName: "n1", Host: "https://10.0.0.5:8006"},
 			},