CLOUDP-349087: Block simultaneous TLS disabling and scaling for ReplicaSets

.evergreen-tasks.yml

@@ -502,7 +502,7 @@ tasks:
     commands:
       - func: "e2e_test"
 
-  - name: e2e_disable_tls_scale_up
+  - name: e2e_disable_tls_and_scale
     tags: [ "patch-run" ]
     commands:
       - func: "e2e_test"

.evergreen.yml

@@ -679,7 +679,7 @@ task_groups:
       - e2e_tls_rs_external_access
       - e2e_replica_set_tls_require
       - e2e_replica_set_tls_certs_secret_prefix
-      - e2e_disable_tls_scale_up
+      - e2e_disable_tls_and_scale
       - e2e_replica_set_tls_require_and_disable
       # e2e_x509_task_group
       - e2e_configure_tls_and_x509_simultaneously_st

api/v1/mdb/mongodb_validation.go

@@ -369,6 +369,25 @@ func noTopologyMigration(newObj, oldObj MongoDbSpec) v1.ValidationResult {
 	return v1.ValidationSuccess()
 }
 
+func noSimultaneousTLSDisablingAndScaling(newObj, oldObj MongoDbSpec) v1.ValidationResult {
+	if newObj.ResourceType != ReplicaSet {
+		return v1.ValidationSuccess()
+	}
+
+	// Check if TLS is being disabled (was enabled, now disabled)
+	tlsWasEnabled := oldObj.IsSecurityTLSConfigEnabled()
+	tlsWillBeDisabled := !newObj.IsSecurityTLSConfigEnabled()
+
+	// Check if members count is changing
+	membersChanging := oldObj.Members != newObj.Members
+
+	if tlsWasEnabled && tlsWillBeDisabled && membersChanging {
+		return v1.ValidationError("Cannot disable TLS and change member count simultaneously. Please apply these changes separately.")
+	}
+
+	return v1.ValidationSuccess()
+}
+
 // specWithExactlyOneSchema checks that exactly one among "Project/OpsManagerConfig/CloudManagerConfig"
 // is configured, doing the "oneOf" validation in the webhook.
 func specWithExactlyOneSchema(d DbCommonSpec) v1.ValidationResult {
@@ -437,6 +456,7 @@ func (m *MongoDB) RunValidations(old *MongoDB) []v1.ValidationResult {
 	updateValidators := []func(newObj MongoDbSpec, oldObj MongoDbSpec) v1.ValidationResult{
 		resourceTypeImmutable,
 		noTopologyMigration,
+		noSimultaneousTLSDisablingAndScaling,
 	}
 
 	var validationResults []v1.ValidationResult

api/v1/mdb/mongodb_validation_test.go

@@ -80,6 +80,88 @@ func TestMongoDB_ResourceTypeImmutable(t *testing.T) {
 	assert.Errorf(t, err, "'resourceType' cannot be changed once created")
 }
 
+func TestMongoDB_NoSimultaneousTLSDisablingAndScaling(t *testing.T) {
+	tests := []struct {
+		name                 string
+		oldTLSEnabled        bool
+		oldMembers           int
+		newTLSEnabled        bool
+		newMembers           int
+		expectError          bool
+		expectedErrorMessage string
+	}{
+		{
+			name:                 "Simultaneous TLS disabling and scaling is blocked",
+			oldTLSEnabled:        true,
+			oldMembers:           3,
+			newTLSEnabled:        false,
+			newMembers:           5,
+			expectError:          true,
+			expectedErrorMessage: "Cannot disable TLS and change member count simultaneously. Please apply these changes separately.",
+		},
+		{
+			name:          "TLS disabling without scaling is allowed",
+			oldTLSEnabled: true,
+			oldMembers:    3,
+			newTLSEnabled: false,
+			newMembers:    3,
+			expectError:   false,
+		},
+		{
+			name:          "Scaling without TLS changes is allowed",
+			oldTLSEnabled: true,
+			oldMembers:    3,
+			newTLSEnabled: true,
+			newMembers:    5,
+			expectError:   false,
+		},
+		{
+			name:          "TLS enabling with scaling is allowed",
+			oldTLSEnabled: false,
+			oldMembers:    3,
+			newTLSEnabled: true,
+			newMembers:    5,
+			expectError:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Build old ReplicaSet
+			oldBuilder := NewReplicaSetBuilder()
+			if tt.oldTLSEnabled {
+				oldBuilder = oldBuilder.SetSecurityTLSEnabled()
+			}
+			oldRs := oldBuilder.Build()
+			oldRs.Spec.CloudManagerConfig = &PrivateCloudConfig{
+				ConfigMapRef: ConfigMapRef{Name: "cloud-manager"},
+			}
+			oldRs.Spec.Members = tt.oldMembers
+
+			// Build new ReplicaSet
+			newBuilder := NewReplicaSetBuilder()
+			if tt.newTLSEnabled {
+				newBuilder = newBuilder.SetSecurityTLSEnabled()
+			}
+			newRs := newBuilder.Build()
+			newRs.Spec.CloudManagerConfig = &PrivateCloudConfig{
+				ConfigMapRef: ConfigMapRef{Name: "cloud-manager"},
+			}
+			newRs.Spec.Members = tt.newMembers
+
+			// Validate
+			_, err := newRs.ValidateUpdate(oldRs)
+
+			if tt.expectError {
+				require.Error(t, err)
+				assert.Equal(t, tt.expectedErrorMessage, err.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestSpecProjectOnlyOneValue(t *testing.T) {
 	rs := NewReplicaSetBuilder().Build()
 	rs.Spec.CloudManagerConfig = &PrivateCloudConfig{

changelog/20251024_fix_block_simultaneous_tls_disabling_and_scaling.md

@@ -0,0 +1,6 @@
+---
+kind: fix
+date: 2025-10-24
+---
+
+* **ReplicaSet**: Blocked disabling TLS and changing member count simultaneously. These operations must now be applied separately to prevent configuration inconsistencies.

controllers/om/deployment.go

@@ -100,24 +100,6 @@ func NewDeployment() Deployment {
 	return ans
 }
 
-// TLSConfigurationWillBeDisabled checks that if applying this security configuration the Deployment
-// TLS configuration will go from Enabled -> Disabled.
-func (d Deployment) TLSConfigurationWillBeDisabled(security *mdbv1.Security) bool {
-	tlsIsCurrentlyEnabled := false
-
-	// To detect that TLS is enabled, it is sufficient to check for the
-	// d["tls"]["CAFilePath"] attribute to have a value different from nil.
-	if tlsMap, ok := d["tls"]; ok {
-		if caFilePath, ok := tlsMap.(map[string]interface{})["CAFilePath"]; ok {
-			if caFilePath != nil {
-				tlsIsCurrentlyEnabled = true
-			}
-		}
-	}
-
-	return tlsIsCurrentlyEnabled && !security.IsTLSEnabled()
-}
-
 // ConfigureTLS configures the deployment's TLS settings from the security
 // specification provided by the user in the mongodb resource.
 func (d Deployment) ConfigureTLS(security *mdbv1.Security, caFilePath string) {

controllers/om/deployment_test.go

@@ -152,20 +152,6 @@ func TestConfigureSSL_Deployment(t *testing.T) {
 	assert.Equal(t, d["tls"], map[string]any{"clientCertificateMode": string(automationconfig.ClientCertificateModeOptional)})
 }
 
-func TestTLSConfigurationWillBeDisabled(t *testing.T) {
-	d := Deployment{}
-	d.ConfigureTLS(&mdbv1.Security{TLSConfig: &mdbv1.TLSConfig{Enabled: false}}, util.CAFilePathInContainer)
-
-	assert.False(t, d.TLSConfigurationWillBeDisabled(&mdbv1.Security{TLSConfig: &mdbv1.TLSConfig{Enabled: false}}))
-	assert.False(t, d.TLSConfigurationWillBeDisabled(&mdbv1.Security{TLSConfig: &mdbv1.TLSConfig{Enabled: true}}))
-
-	d = Deployment{}
-	d.ConfigureTLS(&mdbv1.Security{TLSConfig: &mdbv1.TLSConfig{Enabled: true}}, util.CAFilePathInContainer)
-
-	assert.False(t, d.TLSConfigurationWillBeDisabled(&mdbv1.Security{TLSConfig: &mdbv1.TLSConfig{Enabled: true}}))
-	assert.True(t, d.TLSConfigurationWillBeDisabled(&mdbv1.Security{TLSConfig: &mdbv1.TLSConfig{Enabled: false}}))
-}
-
 // TestMergeDeployment_BigReplicaset ensures that adding a big replica set (> 7 members) works correctly and no more than
 // 7 voting members are added
 func TestMergeDeployment_BigReplicaset(t *testing.T) {

controllers/operator/mongodbreplicaset_controller.go

@@ -578,32 +578,8 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c
 	}
 
 	caFilePath := fmt.Sprintf("%s/ca-pem", util.TLSCaMountPath)
-	// If current operation is to Disable TLS, then we should the current members of the Replica Set,
-	// this is, do not scale them up or down util TLS disabling has completed.
-	shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, membersNumberBefore, rs, log, caFilePath, tlsCertPath)
-	if err != nil && !isRecovering {
-		return workflow.Failed(err)
-	}
-
-	var updatedMembers int
-	// This lock member logic will be removed soon, we should rather block possibility to disable tls + scale
-	// Tracked in CLOUDP-349087
-	if shouldLockMembers {
-		// We should not add or remove members during this run, we'll wait for
-		// TLS to be completely disabled first.
-		// However, on first reconciliation (membersNumberBefore=0), we need to use replicasTarget
-		// because the OM deployment is initialized with TLS enabled by default.
-		log.Debugf("locking members for this reconciliation because TLS was disabled")
-		if membersNumberBefore == 0 {
-			updatedMembers = replicasTarget
-		} else {
-			updatedMembers = membersNumberBefore
-		}
-	} else {
-		updatedMembers = replicasTarget
-	}
 
-	replicaSet := replicaset.BuildFromMongoDBWithReplicas(r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, rs, updatedMembers, rs.CalculateFeatureCompatibilityVersion(), tlsCertPath)
+	replicaSet := replicaset.BuildFromMongoDBWithReplicas(r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, rs, replicasTarget, rs.CalculateFeatureCompatibilityVersion(), tlsCertPath)
 	processNames := replicaSet.GetProcessNames()
 
 	status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, deploymentOptionsRS.agentCertPath, caFilePath, internalClusterCertPath, isRecovering, log)
@@ -668,40 +644,6 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c
 	return workflow.OK()
 }
 
-// updateOmDeploymentDisableTLSConfiguration checks if TLS configuration needs
-// to be disabled. In which case it will disable it and inform to the calling
-// function.
-func updateOmDeploymentDisableTLSConfiguration(conn om.Connection, mongoDBImage string, forceEnterprise bool, membersNumberBefore int, rs *mdbv1.MongoDB, log *zap.SugaredLogger, caFilePath, tlsCertPath string) (bool, error) {
-	tlsConfigWasDisabled := false
-
-	err := conn.ReadUpdateDeployment(
-		func(d om.Deployment) error {
-			if !d.TLSConfigurationWillBeDisabled(rs.Spec.GetSecurity()) {
-				return nil
-			}
-
-			tlsConfigWasDisabled = true
-			d.ConfigureTLS(rs.Spec.GetSecurity(), caFilePath)
-
-			// configure as many agents/Pods as we currently have, no more (in case
-			// there's a scale up change at the same time).
-			replicaSet := replicaset.BuildFromMongoDBWithReplicas(mongoDBImage, forceEnterprise, rs, membersNumberBefore, rs.CalculateFeatureCompatibilityVersion(), tlsCertPath)
-
-			lastConfig, err := rs.GetLastAdditionalMongodConfigByType(mdbv1.ReplicaSetConfig)
-			if err != nil {
-				return err
-			}
-
-			d.MergeReplicaSet(replicaSet, rs.Spec.AdditionalMongodConfig.ToMap(), lastConfig.ToMap(), log)
-
-			return nil
-		},
-		log,
-	)
-
-	return tlsConfigWasDisabled, err
-}
-
 func (r *ReconcileMongoDbReplicaSet) OnDelete(ctx context.Context, obj runtime.Object, log *zap.SugaredLogger) error {
 	rs := obj.(*mdbv1.MongoDB)
 

controllers/operator/mongodbreplicaset_controller_test.go

@@ -68,7 +68,7 @@ func TestCreateReplicaSet(t *testing.T) {
 
 	connection := omConnectionFactory.GetConnection()
 	connection.(*om.MockedOmConnection).CheckDeployment(t, deployment.CreateFromReplicaSet("fake-mongoDBImage", false, rs), "auth", "ssl")
-	connection.(*om.MockedOmConnection).CheckNumberOfUpdateRequests(t, 2)
+	connection.(*om.MockedOmConnection).CheckNumberOfUpdateRequests(t, 1)
 }
 
 func TestReplicaSetRace(t *testing.T) {
@@ -385,39 +385,6 @@ func TestCreateReplicaSet_TLS(t *testing.T) {
 	assert.Equal(t, "OPTIONAL", sslConfig["clientCertificateMode"])
 }
 
-// TestUpdateDeploymentTLSConfiguration a combination of tests checking that:
-//
-// TLS Disabled -> TLS Disabled: should not lock members
-// TLS Disabled -> TLS Enabled: should not lock members
-// TLS Enabled -> TLS Enabled: should not lock members
-// TLS Enabled -> TLS Disabled: *should lock members*
-func TestUpdateDeploymentTLSConfiguration(t *testing.T) {
-	rsWithTLS := mdbv1.NewReplicaSetBuilder().SetSecurityTLSEnabled().Build()
-	rsNoTLS := mdbv1.NewReplicaSetBuilder().Build()
-	deploymentWithTLS := deployment.CreateFromReplicaSet("fake-mongoDBImage", false, rsWithTLS)
-	deploymentNoTLS := deployment.CreateFromReplicaSet("fake-mongoDBImage", false, rsNoTLS)
-
-	// TLS Disabled -> TLS Disabled
-	shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentNoTLS), "fake-mongoDBImage", false, 3, rsNoTLS, zap.S(), util.CAFilePathInContainer, "")
-	assert.NoError(t, err)
-	assert.False(t, shouldLockMembers)
-
-	// TLS Disabled -> TLS Enabled
-	shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentNoTLS), "fake-mongoDBImage", false, 3, rsWithTLS, zap.S(), util.CAFilePathInContainer, "")
-	assert.NoError(t, err)
-	assert.False(t, shouldLockMembers)
-
-	// TLS Enabled -> TLS Enabled
-	shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentWithTLS), "fake-mongoDBImage", false, 3, rsWithTLS, zap.S(), util.CAFilePathInContainer, "")
-	assert.NoError(t, err)
-	assert.False(t, shouldLockMembers)
-
-	// TLS Enabled -> TLS Disabled
-	shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentWithTLS), "fake-mongoDBImage", false, 3, rsNoTLS, zap.S(), util.CAFilePathInContainer, "")
-	assert.NoError(t, err)
-	assert.True(t, shouldLockMembers)
-}
-
 // TestCreateDeleteReplicaSet checks that no state is left in OpsManager on removal of the replicaset
 func TestCreateDeleteReplicaSet(t *testing.T) {
 	ctx := context.Background()