cleaner tls clearing

Author: nammn

controllers/om/deployment.go

@@ -312,7 +312,9 @@ func (d Deployment) ConfigureMonitoring(log *zap.SugaredLogger, tls bool, caFile
 			if pem := p.EnsureTLSConfig()["PEMKeyFile"]; pem != nil {
 				pemKeyFile = pem.(string)
 			}
-			monitoringVersion["additionalParams"] = buildTLSAdditionalParams(caFilePath, pemKeyFile)
+			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.
@@ -324,34 +326,37 @@ func (d Deployment) ConfigureMonitoring(log *zap.SugaredLogger, tls bool, caFile
 	d.setMonitoringVersions(monitoringVersions)
 }
 
-// buildTLSAdditionalParams creates the additionalParams map for monitoring with TLS enabled.
-// This is the single source of truth for TLS params - clearTLSParamsFromMonitoringVersion
-// uses this function to determine which keys to remove.
-func buildTLSAdditionalParams(caFilePath string, pemKeyFile string) map[string]string {
-	params := map[string]string{
-		"useSslForAllConnections":      "true",
-		"sslTrustedServerCertificates": caFilePath,
-	}
+// 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["sslClientCertificate"] = pemKeyFile
+		params[tlsParamClientCert] = pemKeyFile
 	}
-	return params
+}
+
+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. It derives which keys to remove from buildTLSAdditionalParams,
-// ensuring add and remove operations always stay in sync.
-// If additionalParams becomes empty after removing TLS fields, the entire map is removed.
+// 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
 	}
 
-	// Use non-empty dummy values to get all possible TLS param keys
-	for key := range buildTLSAdditionalParams("_", "_") {
-		delete(additionalParams, key)
-	}
+	clearTLSParams(additionalParams)
 
 	if len(additionalParams) == 0 {
 		delete(monitoringVersion, "additionalParams")

controllers/operator/appdbreplicaset_controller.go

@@ -1436,8 +1436,7 @@ func addMonitoring(ac *automationconfig.AutomationConfig, log *zap.SugaredLogger
 				if !tls {
 					// 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.
-					clearTLSParamsFromAdditionalParams(monitoringVersions[i].AdditionalParams)
+					clearTLSParams(monitoringVersions[i].AdditionalParams)
 					if len(monitoringVersions[i].AdditionalParams) == 0 {
 						monitoringVersions[i].AdditionalParams = nil
 					}
@@ -1455,7 +1454,8 @@ func addMonitoring(ac *automationconfig.AutomationConfig, log *zap.SugaredLogger
 				if pem := p.Args26.Get("net.tls.certificateKeyFile"); pem != nil {
 					pemKeyFile = pem.String()
 				}
-				monitoringVersion.AdditionalParams = buildTLSAdditionalParams(appdbCAFilePath, pemKeyFile)
+				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)
@@ -1464,31 +1464,25 @@ func addMonitoring(ac *automationconfig.AutomationConfig, log *zap.SugaredLogger
 	ac.MonitoringVersions = monitoringVersions
 }
 
-// buildTLSAdditionalParams creates the additionalParams map for monitoring with TLS enabled.
-// This is the single source of truth for TLS params - clearTLSParamsFromAdditionalParams
-// uses this function to determine which keys to remove.
-func buildTLSAdditionalParams(caFilePath string, pemKeyFile string) map[string]string {
-	params := map[string]string{
-		"useSslForAllConnections":      "true",
-		"sslTrustedServerCertificates": caFilePath,
-	}
+// 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["sslClientCertificate"] = pemKeyFile
+		params[tlsParamClientCert] = pemKeyFile
 	}
-	return params
 }
 
-// clearTLSParamsFromAdditionalParams removes TLS-specific fields from the monitoring
-// version's additionalParams map. It derives which keys to remove from buildTLSAdditionalParams,
-// ensuring add and remove operations always stay in sync.
-func clearTLSParamsFromAdditionalParams(additionalParams map[string]string) {
-	if additionalParams == nil {
-		return
-	}
-	// Use non-empty dummy values to get all possible TLS param keys
-	for key := range buildTLSAdditionalParams("_", "_") {
-		delete(additionalParams, key)
-	}
+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

controllers/operator/appdbreplicaset_controller_test.go

@@ -1458,11 +1458,11 @@ func createRunningAppDB(ctx context.Context, t *testing.T, startingMembers int,
 	return reconciler
 }
 
-// TestClearTLSParamsFromAdditionalParams tests CLOUDP-351614 fix:
+// 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 TestClearTLSParamsFromAdditionalParams(t *testing.T) {
+func TestClearTLSParams(t *testing.T) {
 	tests := []struct {
 		name           string
 		input          map[string]string
@@ -1526,7 +1526,7 @@ func TestClearTLSParamsFromAdditionalParams(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			clearTLSParamsFromAdditionalParams(tt.input)
+			clearTLSParams(tt.input)
 			assert.Equal(t, tt.expectedOutput, tt.input)
 		})
 	}