CLOUDP-351614: Fix monitoring failure after disabling TLS on AppDB

When TLS is disabled on AppDB, clear stale TLS params from monitoring
config's additionalParams to prevent the monitoring agent from trying
to use certificate files that no longer exist.

Added addTLSParams/clearTLSParams functions that use shared constants
to ensure add and remove operations stay in sync.

Author: nammn

.evergreen.yml

@@ -1439,7 +1439,21 @@ buildvariants:
     tags: [ "pr_patch", "staging", "e2e_test_suite", "static" ]
     run_on:
       - ubuntu2404-large
-    <<: *base_om8_dependency
+    depends_on:
+      - name: build_om_images
+        variant: build_om80_images
+      - name: build_operator_ubi
+        variant: init_test_run
+      - name: build_init_database_image_ubi
+        variant: init_test_run
+      - name: build_test_image
+        variant: init_test_run
+      - name: build_init_appdb_images_ubi
+        variant: init_test_run
+      - name: build_init_om_images_ubi
+        variant: init_test_run
+      - name: publish_helm_chart
+        variant: init_test_run
     tasks:
       - name: e2e_static_ops_manager_kind_only_task_group
       - name: e2e_static_ops_manager_kind_6_0_only_task_group

changelog/20251216_fix_tls_monitoring.md

@@ -0,0 +1,6 @@
+---
+kind: fix
+date: 2025-12-16
+---
+
+* Fixed an issue where monitoring agents would fail after disabling TLS on a MongoDB deployment.

controllers/om/deployment.go

@@ -251,15 +251,16 @@ func (d Deployment) MergeShardedCluster(opts DeploymentShardedClusterMergeOption
 	return shardsScheduledForRemoval, nil
 }
 
-// AddMonitoringAndBackup adds monitoring and backup agents to each process
-// The automation agent will update the agents versions to the latest version automatically
+// ConfigureMonitoringAndBackup configures monitoring and backup agents for each process.
+// This is called on every reconcile to ensure the monitoring/backup config matches the desired state.
+// The automation agent will update the agents versions to the latest version automatically.
 // Note, that these two are deliberately combined as all clients (standalone, rs etc.) need both backup and monitoring
-// together
-func (d Deployment) AddMonitoringAndBackup(log *zap.SugaredLogger, tls bool, caFilepath string) {
+// together.
+func (d Deployment) ConfigureMonitoringAndBackup(log *zap.SugaredLogger, tls bool, caFilepath string) {
 	if len(d.getProcesses()) == 0 {
 		return
 	}
-	d.AddMonitoring(log, tls, caFilepath)
+	d.ConfigureMonitoring(log, tls, caFilepath)
 	d.addBackup(log)
 }
 
@@ -277,8 +278,9 @@ func (d Deployment) GetReplicaSetByName(name string) ReplicaSet {
 	return nil
 }
 
-// AddMonitoring adds monitoring agents for all processes in the deployment
-func (d Deployment) AddMonitoring(log *zap.SugaredLogger, tls bool, caFilePath string) {
+// ConfigureMonitoring configures monitoring agents for all processes in the deployment.
+// This is called on every reconcile to ensure the monitoring config matches the desired state.
+func (d Deployment) ConfigureMonitoring(log *zap.SugaredLogger, tls bool, caFilePath string) {
 	if len(d.getProcesses()) == 0 {
 		return
 	}
@@ -306,23 +308,61 @@ func (d Deployment) AddMonitoring(log *zap.SugaredLogger, tls bool, caFilePath s
 		monitoringVersion["hostname"] = p.HostName()
 
 		if tls {
-			additionalParams := map[string]string{
-				"useSslForAllConnections":      "true",
-				"sslTrustedServerCertificates": caFilePath,
+			pemKeyFile := ""
+			if pem := p.EnsureTLSConfig()["PEMKeyFile"]; pem != nil {
+				pemKeyFile = pem.(string)
 			}
-
-			pemKeyFile := p.EnsureTLSConfig()["PEMKeyFile"]
-			if pemKeyFile != nil {
-				additionalParams["sslClientCertificate"] = pemKeyFile.(string)
-			}
-
-			monitoringVersion["additionalParams"] = additionalParams
+			params := map[string]string{}
+			addTLSParams(params, caFilePath, pemKeyFile)
+			monitoringVersion["additionalParams"] = params
+		} else {
+			// Clear TLS-specific params when TLS is disabled to prevent monitoring from
+			// trying to use certificate files that no longer exist.
+			// We only clear TLS fields, preserving any other additionalParams that may be set.
+			clearTLSParamsFromMonitoringVersion(monitoringVersion)
 		}
 
 	}
 	d.setMonitoringVersions(monitoringVersions)
 }
 
+// TLS param keys for monitoring additionalParams.
+const (
+	tlsParamUseSsl      = "useSslForAllConnections"
+	tlsParamTrustedCert = "sslTrustedServerCertificates"
+	tlsParamClientCert  = "sslClientCertificate"
+)
+
+func addTLSParams(params map[string]string, caFilePath, pemKeyFile string) {
+	params[tlsParamUseSsl] = "true"
+	params[tlsParamTrustedCert] = caFilePath
+	if pemKeyFile != "" {
+		params[tlsParamClientCert] = pemKeyFile
+	}
+}
+
+func clearTLSParams(params map[string]string) {
+	delete(params, tlsParamUseSsl)
+	delete(params, tlsParamTrustedCert)
+	delete(params, tlsParamClientCert)
+}
+
+// clearTLSParamsFromMonitoringVersion removes TLS-specific fields from the monitoring
+// version's additionalParams. If additionalParams becomes empty after removing TLS fields,
+// the entire map is removed.
+func clearTLSParamsFromMonitoringVersion(monitoringVersion map[string]interface{}) {
+	additionalParams, ok := monitoringVersion["additionalParams"].(map[string]string)
+	if !ok {
+		return
+	}
+
+	clearTLSParams(additionalParams)
+
+	if len(additionalParams) == 0 {
+		delete(monitoringVersion, "additionalParams")
+	}
+}
+
 // RemoveMonitoringAndBackup removes both monitoring and backup agent configurations. This must be called when the
 // Mongodb resource is being removed, otherwise UI will show non-existing agents in the "servers" tab
 func (d Deployment) RemoveMonitoringAndBackup(names []string, log *zap.SugaredLogger) {

controllers/om/deployment/testing_utils.go

@@ -37,7 +37,7 @@ func CreateFromReplicaSet(mongoDBImage string, forceEnterprise bool, rs *mdb.Mon
 		lastConfig.ToMap(),
 		zap.S(),
 	)
-	d.AddMonitoringAndBackup(zap.S(), rs.Spec.GetSecurity().IsTLSEnabled(), util.CAFilePathInContainer)
+	d.ConfigureMonitoringAndBackup(zap.S(), rs.Spec.GetSecurity().IsTLSEnabled(), util.CAFilePathInContainer)
 	d.ConfigureTLS(rs.Spec.GetSecurity(), util.CAFilePathInContainer)
 	return d
 }

controllers/om/deployment_test.go

@@ -489,12 +489,12 @@ func TestConfiguringTlsProcessFromOpsManager(t *testing.T) {
 	}
 }
 
-func TestAddMonitoring(t *testing.T) {
+func TestConfigureMonitoring(t *testing.T) {
 	d := NewDeployment()
 
 	rs0 := buildRsByProcesses("my-rs", createReplicaSetProcessesCount(3, "my-rs"))
 	d.MergeReplicaSet(rs0, nil, nil, zap.S())
-	d.AddMonitoring(zap.S(), false, util.CAFilePathInContainer)
+	d.ConfigureMonitoring(zap.S(), false, util.CAFilePathInContainer)
 
 	expectedMonitoringVersions := []interface{}{
 		map[string]interface{}{"hostname": "my-rs-0.some.host", "name": MonitoringAgentDefaultVersion},
@@ -504,16 +504,16 @@ func TestAddMonitoring(t *testing.T) {
 	assert.Equal(t, expectedMonitoringVersions, d.getMonitoringVersions())
 
 	// adding again - nothing changes
-	d.AddMonitoring(zap.S(), false, util.CAFilePathInContainer)
+	d.ConfigureMonitoring(zap.S(), false, util.CAFilePathInContainer)
 	assert.Equal(t, expectedMonitoringVersions, d.getMonitoringVersions())
 }
 
-func TestAddMonitoringTls(t *testing.T) {
+func TestConfigureMonitoringTls(t *testing.T) {
 	d := NewDeployment()
 
 	rs0 := buildRsByProcesses("my-rs", createReplicaSetProcessesCount(3, "my-rs"))
 	d.MergeReplicaSet(rs0, nil, nil, zap.S())
-	d.AddMonitoring(zap.S(), true, util.CAFilePathInContainer)
+	d.ConfigureMonitoring(zap.S(), true, util.CAFilePathInContainer)
 
 	expectedAdditionalParams := map[string]string{
 		"useSslForAllConnections":      "true",
@@ -528,10 +528,39 @@ func TestAddMonitoringTls(t *testing.T) {
 	assert.Equal(t, expectedMonitoringVersions, d.getMonitoringVersions())
 
 	// adding again - nothing changes
-	d.AddMonitoring(zap.S(), false, util.CAFilePathInContainer)
+	d.ConfigureMonitoring(zap.S(), true, util.CAFilePathInContainer)
 	assert.Equal(t, expectedMonitoringVersions, d.getMonitoringVersions())
 }
 
+func TestConfigureMonitoringTLSDisable(t *testing.T) {
+	d := NewDeployment()
+
+	rs0 := buildRsByProcesses("my-rs", createReplicaSetProcessesCount(3, "my-rs"))
+	d.MergeReplicaSet(rs0, nil, nil, zap.S())
+	d.ConfigureMonitoring(zap.S(), true, util.CAFilePathInContainer)
+
+	// verify TLS is present in additionalParams
+	expectedAdditionalParams := map[string]string{
+		"useSslForAllConnections":      "true",
+		"sslTrustedServerCertificates": util.CAFilePathInContainer,
+	}
+	expectedMonitoringVersionsWithTls := []interface{}{
+		map[string]interface{}{"hostname": "my-rs-0.some.host", "name": MonitoringAgentDefaultVersion, "additionalParams": expectedAdditionalParams},
+		map[string]interface{}{"hostname": "my-rs-1.some.host", "name": MonitoringAgentDefaultVersion, "additionalParams": expectedAdditionalParams},
+		map[string]interface{}{"hostname": "my-rs-2.some.host", "name": MonitoringAgentDefaultVersion, "additionalParams": expectedAdditionalParams},
+	}
+	assert.Equal(t, expectedMonitoringVersionsWithTls, d.getMonitoringVersions())
+
+	// disabling TLS should clear additionalParams (CLOUDP-351614)
+	d.ConfigureMonitoring(zap.S(), false, util.CAFilePathInContainer)
+	expectedMonitoringVersionsWithoutTls := []interface{}{
+		map[string]interface{}{"hostname": "my-rs-0.some.host", "name": MonitoringAgentDefaultVersion},
+		map[string]interface{}{"hostname": "my-rs-1.some.host", "name": MonitoringAgentDefaultVersion},
+		map[string]interface{}{"hostname": "my-rs-2.some.host", "name": MonitoringAgentDefaultVersion},
+	}
+	assert.Equal(t, expectedMonitoringVersionsWithoutTls, d.getMonitoringVersions())
+}
+
 func TestAddBackup(t *testing.T) {
 	d := NewDeployment()
 

controllers/operator/appdbreplicaset_controller.go

@@ -1430,9 +1430,17 @@ func addMonitoring(ac *automationconfig.AutomationConfig, log *zap.SugaredLogger
 	monitoringVersions := ac.MonitoringVersions
 	for _, p := range ac.Processes {
 		found := false
-		for _, m := range monitoringVersions {
+		for i, m := range monitoringVersions {
 			if m.Hostname == p.HostName {
 				found = true
+				if !tls {
+					// Clear TLS-specific params when TLS is disabled to prevent monitoring from
+					// trying to use certificate files that no longer exist.
+					clearTLSParams(monitoringVersions[i].AdditionalParams)
+					if len(monitoringVersions[i].AdditionalParams) == 0 {
+						monitoringVersions[i].AdditionalParams = nil
+					}
+				}
 				break
 			}
 		}
@@ -1442,15 +1450,12 @@ func addMonitoring(ac *automationconfig.AutomationConfig, log *zap.SugaredLogger
 				Name:     om.MonitoringAgentDefaultVersion,
 			}
 			if tls {
-				additionalParams := map[string]string{
-					"useSslForAllConnections":      "true",
-					"sslTrustedServerCertificates": appdbCAFilePath,
-				}
-				pemKeyFile := p.Args26.Get("net.tls.certificateKeyFile")
-				if pemKeyFile != nil {
-					additionalParams["sslClientCertificate"] = pemKeyFile.String()
+				pemKeyFile := ""
+				if pem := p.Args26.Get("net.tls.certificateKeyFile"); pem != nil {
+					pemKeyFile = pem.String()
 				}
-				monitoringVersion.AdditionalParams = additionalParams
+				monitoringVersion.AdditionalParams = map[string]string{}
+				addTLSParams(monitoringVersion.AdditionalParams, appdbCAFilePath, pemKeyFile)
 			}
 			log.Debugw("Added monitoring agent configuration", "host", p.HostName, "tls", tls)
 			monitoringVersions = append(monitoringVersions, monitoringVersion)
@@ -1459,6 +1464,27 @@ func addMonitoring(ac *automationconfig.AutomationConfig, log *zap.SugaredLogger
 	ac.MonitoringVersions = monitoringVersions
 }
 
+// TLS param keys for monitoring additionalParams.
+const (
+	tlsParamUseSsl      = "useSslForAllConnections"
+	tlsParamTrustedCert = "sslTrustedServerCertificates"
+	tlsParamClientCert  = "sslClientCertificate"
+)
+
+func addTLSParams(params map[string]string, caFilePath, pemKeyFile string) {
+	params[tlsParamUseSsl] = "true"
+	params[tlsParamTrustedCert] = caFilePath
+	if pemKeyFile != "" {
+		params[tlsParamClientCert] = pemKeyFile
+	}
+}
+
+func clearTLSParams(params map[string]string) {
+	delete(params, tlsParamUseSsl)
+	delete(params, tlsParamTrustedCert)
+	delete(params, tlsParamClientCert)
+}
+
 // registerAppDBHostsWithProject uses the Hosts API to add each process in the AppDB to the project
 func (r *ReconcileAppDbReplicaSet) registerAppDBHostsWithProject(hostnames []string, conn om.Connection, opsManagerPassword string, log *zap.SugaredLogger) error {
 	getHostsResult, err := conn.GetHosts()

controllers/operator/appdbreplicaset_controller_test.go

@@ -1457,3 +1457,77 @@ func createRunningAppDB(ctx context.Context, t *testing.T, startingMembers int,
 	assert.Equal(t, ok, res)
 	return reconciler
 }
+
+// TestClearTLSParams tests CLOUDP-351614 fix:
+// When TLS is disabled on AppDB, TLS-specific params should be cleared from
+// the monitoring config's additionalParams to prevent the monitoring agent
+// from trying to use certificate files that no longer exist.
+func TestClearTLSParams(t *testing.T) {
+	tests := []struct {
+		name           string
+		input          map[string]string
+		expectedOutput map[string]string
+	}{
+		{
+			name:           "nil map",
+			input:          nil,
+			expectedOutput: nil,
+		},
+		{
+			name:           "empty map",
+			input:          map[string]string{},
+			expectedOutput: map[string]string{},
+		},
+		{
+			name: "only TLS params",
+			input: map[string]string{
+				"useSslForAllConnections":      "true",
+				"sslTrustedServerCertificates": "/some/path/ca.pem",
+				"sslClientCertificate":         "/some/path/cert.pem",
+			},
+			expectedOutput: map[string]string{},
+		},
+		{
+			name: "mixed params - TLS and non-TLS",
+			input: map[string]string{
+				"useSslForAllConnections":      "true",
+				"sslTrustedServerCertificates": "/some/path/ca.pem",
+				"sslClientCertificate":         "/some/path/cert.pem",
+				"someOtherParam":               "someValue",
+				"anotherParam":                 "anotherValue",
+			},
+			expectedOutput: map[string]string{
+				"someOtherParam": "someValue",
+				"anotherParam":   "anotherValue",
+			},
+		},
+		{
+			name: "only non-TLS params",
+			input: map[string]string{
+				"someOtherParam": "someValue",
+				"anotherParam":   "anotherValue",
+			},
+			expectedOutput: map[string]string{
+				"someOtherParam": "someValue",
+				"anotherParam":   "anotherValue",
+			},
+		},
+		{
+			name: "partial TLS params",
+			input: map[string]string{
+				"useSslForAllConnections": "true",
+				"someOtherParam":          "someValue",
+			},
+			expectedOutput: map[string]string{
+				"someOtherParam": "someValue",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			clearTLSParams(tt.input)
+			assert.Equal(t, tt.expectedOutput, tt.input)
+		})
+	}
+}

controllers/operator/common_controller.go

@@ -1075,7 +1075,7 @@ func ReconcileReplicaSetAC(ctx context.Context, d om.Deployment, spec mdbv1.DbCo
 	}
 
 	d.MergeReplicaSet(rs, spec.GetAdditionalMongodConfig().ToMap(), lastMongodConfig, log)
-	d.AddMonitoringAndBackup(log, spec.GetSecurity().IsTLSEnabled(), caFilePath)
+	d.ConfigureMonitoringAndBackup(log, spec.GetSecurity().IsTLSEnabled(), caFilePath)
 	d.ConfigureTLS(spec.GetSecurity(), caFilePath)
 	d.ConfigureInternalClusterAuthentication(rs.GetProcessNames(), spec.GetSecurity().GetInternalClusterAuthenticationMode(), internalClusterPath)
 

controllers/operator/mongodbshardedcluster_controller.go

@@ -2006,7 +2006,7 @@ func (r *ShardedClusterReconcileHelper) publishDeployment(ctx context.Context, c
 				return err
 			}
 
-			d.AddMonitoringAndBackup(log, sc.Spec.GetSecurity().IsTLSEnabled(), opts.caFilePath)
+			d.ConfigureMonitoringAndBackup(log, sc.Spec.GetSecurity().IsTLSEnabled(), opts.caFilePath)
 			d.ConfigureTLS(sc.Spec.GetSecurity(), opts.caFilePath)
 
 			setupInternalClusterAuth(d, sc.Name, sc.GetSecurity().GetInternalClusterAuthenticationMode(),

controllers/operator/mongodbshardedcluster_controller_test.go

@@ -1676,7 +1676,7 @@ func createDeploymentFromShardedCluster(t *testing.T, updatable v1.CustomResourc
 		Finalizing:      false,
 	})
 	assert.NoError(t, err)
-	d.AddMonitoringAndBackup(zap.S(), sh.Spec.GetSecurity().IsTLSEnabled(), util.CAFilePathInContainer)
+	d.ConfigureMonitoringAndBackup(zap.S(), sh.Spec.GetSecurity().IsTLSEnabled(), util.CAFilePathInContainer)
 	return d
 }