Address review comments (#829)

# Summary

Addressing some of the review comments from the PR.

## Proof of Work

<!-- Enter your proof that it works here.-->

## Checklist

- [ ] Have you linked a jira ticket and/or is the ticket in the title?
- [ ] Have you checked whether your jira ticket required DOCSP changes?
- [ ] Have you added changelog file?
    - use `skip-changelog` label if not needed
- refer to [Changelog files and Release
Notes](https://github.com/mongodb/mongodb-kubernetes/blob/master/CONTRIBUTING.md#changelog-files-and-release-notes)
section in CONTRIBUTING.md for more details

Author: viveksinghggits

api/v1/search/mongodbsearch_types.go

@@ -333,12 +333,12 @@ func (s *MongoDBSearch) CertificateKeySecretName() bool {
 	return s.Spec.Security.TLS.CertificateKeySecret.Name != ""
 }
 
-// TLSSecretNamespacedNameForShard returns the namespaced name of the TLS source secret for a specific shard.
+// TLSSecretForShard returns the namespaced name of the TLS source secret for a specific shard.
 // This is used in per-shard TLS mode for sharded clusters.
 // Naming pattern:
 //   - With prefix: {prefix}-{shardName}-search-cert
 //   - Without prefix: {shardName}-search-cert
-func (s *MongoDBSearch) TLSSecretNamespacedNameForShard(shardName string) types.NamespacedName {
+func (s *MongoDBSearch) TLSSecretForShard(shardName string) types.NamespacedName {
 	var secretName string
 	if s.Spec.Security.TLS != nil && s.Spec.Security.TLS.CertsSecretPrefix != "" {
 		secretName = fmt.Sprintf("%s-%s-search-cert", s.Spec.Security.TLS.CertsSecretPrefix, shardName)
@@ -348,9 +348,9 @@ func (s *MongoDBSearch) TLSSecretNamespacedNameForShard(shardName string) types.
 	return types.NamespacedName{Name: secretName, Namespace: s.Namespace}
 }
 
-// TLSOperatorSecretNamespacedNameForShard returns the namespaced name of the operator-managed TLS secret
+// TLSOperatorSecretForShard returns the namespaced name of the operator-managed TLS secret
 // for a specific shard. This is the secret created by the operator containing the combined certificate and key.
-func (s *MongoDBSearch) TLSOperatorSecretNamespacedNameForShard(shardName string) types.NamespacedName {
+func (s *MongoDBSearch) TLSOperatorSecretForShard(shardName string) types.NamespacedName {
 	return types.NamespacedName{Name: fmt.Sprintf("%s-search-certificate-key", shardName), Namespace: s.Namespace}
 }
 
@@ -400,14 +400,14 @@ func (s *MongoDBSearch) GetPrometheus() *Prometheus {
 	return s.Spec.Prometheus
 }
 
-func (s *MongoDBSearch) MongotStatefulSetNamespacedNameForShard(shardName string) types.NamespacedName {
+func (s *MongoDBSearch) MongotStatefulSetForShard(shardName string) types.NamespacedName {
 	return types.NamespacedName{Name: fmt.Sprintf("%s-mongot-%s", s.Name, shardName), Namespace: s.Namespace}
 }
 
-func (s *MongoDBSearch) MongotServiceNamespacedNameForShard(shardName string) types.NamespacedName {
+func (s *MongoDBSearch) MongotServiceForShard(shardName string) types.NamespacedName {
 	return types.NamespacedName{Name: fmt.Sprintf("%s-mongot-%s-svc", s.Name, shardName), Namespace: s.Namespace}
 }
 
-func (s *MongoDBSearch) MongotConfigMapNamespacedNameForShard(shardName string) types.NamespacedName {
+func (s *MongoDBSearch) MongotConfigMapForShard(shardName string) types.NamespacedName {
 	return types.NamespacedName{Name: fmt.Sprintf("%s-mongot-%s-config", s.Name, shardName), Namespace: s.Namespace}
 }

controllers/operator/mongodbsearch_controller.go

@@ -75,10 +75,10 @@ func (r *MongoDBSearchReconciler) Reconcile(ctx context.Context, request reconci
 
 	// Watch our own TLS certificate secret for changes
 	if mdbSearch.Spec.Security.TLS != nil {
-		if shardedSource, ok := searchSource.(searchcontroller.ShardedSearchSourceDBResource); ok {
+		if shardedSource, ok := searchSource.(searchcontroller.SearchSourceShardedDeployment); ok {
 			// Sharded: watch per-shard source secrets (one per shard)
 			for _, shardName := range shardedSource.GetShardNames() {
-				shardSecretNsName := mdbSearch.TLSSecretNamespacedNameForShard(shardName)
+				shardSecretNsName := mdbSearch.TLSSecretForShard(shardName)
 				r.watch.AddWatchedResourceIfNotAdded(shardSecretNsName.Name, shardSecretNsName.Namespace, watch.Secret, mdbSearch.NamespacedName())
 			}
 		} else {

controllers/operator/mongodbshardedcluster_controller.go

@@ -3,6 +3,10 @@ package operator
 import (
 	"context"
 	"fmt"
+	"slices"
+	"sort"
+	"strings"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/go-multierror"
 	"go.uber.org/zap"
@@ -21,9 +25,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
-	"slices"
-	"sort"
-	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -2311,7 +2312,17 @@ func (r *ShardedClusterReconcileHelper) createDesiredMongosProcesses(certificate
 		hostnames, podNames := r.getMongosHostnames(memberCluster, scale.ReplicasThisReconciliation(r.GetMongosScaler(memberCluster)))
 		for i := range hostnames {
 			// Use desiredMongosConfiguration which includes search parameters applied in applySearchParametersForShards
-			process := om.NewMongosProcess(podNames[i], hostnames[i], r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, r.desiredMongosConfiguration.GetAdditionalMongodConfig(), r.sc.GetSpec(), certificateFilePath, r.sc.Annotations, r.sc.CalculateFeatureCompatibilityVersion())
+			process := om.NewMongosProcess(
+				podNames[i],
+				hostnames[i],
+				r.imageUrls[mcoConstruct.MongodbImageEnv],
+				r.forceEnterprise,
+				r.desiredMongosConfiguration.GetAdditionalMongodConfig(),
+				r.sc.GetSpec(),
+				certificateFilePath,
+				r.sc.Annotations,
+				r.sc.CalculateFeatureCompatibilityVersion(),
+			)
 			processes = append(processes, process)
 		}
 	}

controllers/searchcontroller/mongodbsearch_reconcile_helper.go

@@ -121,7 +121,7 @@ func (r *MongoDBSearchReconcileHelper) reconcile(ctx context.Context, log *zap.S
 		return workflow.Failed(err)
 	}
 
-	if shardedSource, ok := r.db.(ShardedSearchSourceDBResource); ok {
+	if shardedSource, ok := r.db.(SearchSourceShardedDeployment); ok {
 		return r.reconcileSharded(ctx, log, shardedSource, version)
 	}
 
@@ -192,7 +192,7 @@ func (r *MongoDBSearchReconcileHelper) reconcileNonSharded(ctx context.Context,
 }
 
 // reconcileSharded deploys one mongot StatefulSet, Service, and ConfigMap per shard.
-func (r *MongoDBSearchReconcileHelper) reconcileSharded(ctx context.Context, log *zap.SugaredLogger, shardedSource ShardedSearchSourceDBResource, version string) workflow.Status {
+func (r *MongoDBSearchReconcileHelper) reconcileSharded(ctx context.Context, log *zap.SugaredLogger, shardedSource SearchSourceShardedDeployment, version string) workflow.Status {
 	log.Infof("Reconciling MongoDBSearch for sharded source deployment with %d shards", shardedSource.GetShardCount())
 
 	keyfileStsModification := statefulset.NOOP()
@@ -235,7 +235,7 @@ func (r *MongoDBSearchReconcileHelper) reconcileSharded(ctx context.Context, log
 			return workflow.Failed(err)
 		}
 
-		shardMongotConfig := createShardMongotConfig(r.mdbSearch, shardedSource, shardIdx)
+		shardMongotConfig := createMongotConfigForShard(r.mdbSearch, shardedSource, shardIdx)
 		configHash, err := r.ensureShardMongotConfig(ctx, shardLog, shardName, shardMongotConfig, ingressTlsMongotModification, egressTlsMongotModification, embeddingConfigMongotModification)
 		if err != nil {
 			return workflow.Failed(err)
@@ -250,7 +250,7 @@ func (r *MongoDBSearchReconcileHelper) reconcileSharded(ctx context.Context, log
 		mutatedSts, err := r.createOrUpdateShardStatefulSet(ctx,
 			shardLog,
 			shardName,
-			CreateShardSearchStatefulSetFunc(r.mdbSearch, shardedSource, shardIdx, searchImage),
+			CreateSearchStatefulSetForShardFunc(r.mdbSearch, shardedSource, shardIdx, searchImage),
 			configHashModification,
 			keyfileStsModification,
 			ingressTlsStsModification,
@@ -304,7 +304,7 @@ func (r *MongoDBSearchReconcileHelper) validatePerShardTLSSecrets(ctx context.Co
 
 	// Per-shard mode: validate each shard's source secret exists
 	for _, shardName := range shardNames {
-		secretNsName := r.mdbSearch.TLSSecretNamespacedNameForShard(shardName)
+		secretNsName := r.mdbSearch.TLSSecretForShard(shardName)
 		tlsSecret := &corev1.Secret{}
 		err := r.client.Get(ctx, secretNsName, tlsSecret)
 		if apierrors.IsNotFound(err) {
@@ -389,11 +389,11 @@ func (r *MongoDBSearchReconcileHelper) ensureMongotConfig(ctx context.Context, l
 }
 
 func (r *MongoDBSearchReconcileHelper) ensureShardSearchService(ctx context.Context, shardName string) error {
-	svcName := r.mdbSearch.MongotServiceNamespacedNameForShard(shardName)
+	svcName := r.mdbSearch.MongotServiceForShard(shardName)
 	svc := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: svcName.Name, Namespace: svcName.Namespace}}
 	op, err := controllerutil.CreateOrUpdate(ctx, r.client, svc, func() error {
 		resourceVersion := svc.ResourceVersion
-		*svc = buildShardSearchHeadlessService(r.mdbSearch, shardName)
+		*svc = buildSearchHeadlessServiceForShard(r.mdbSearch, shardName)
 		svc.ResourceVersion = resourceVersion
 		return controllerutil.SetOwnerReference(r.mdbSearch, svc, r.client.Scheme())
 	})
@@ -414,7 +414,7 @@ func (r *MongoDBSearchReconcileHelper) ensureShardMongotConfig(ctx context.Conte
 		return "", err
 	}
 
-	cmName := r.mdbSearch.MongotConfigMapNamespacedNameForShard(shardName)
+	cmName := r.mdbSearch.MongotConfigMapForShard(shardName)
 	cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: cmName.Name, Namespace: cmName.Namespace}, Data: map[string]string{}}
 	op, err := controllerutil.CreateOrUpdate(ctx, r.client, cm, func() error {
 		resourceVersion := cm.ResourceVersion
@@ -435,7 +435,7 @@ func (r *MongoDBSearchReconcileHelper) ensureShardMongotConfig(ctx context.Conte
 }
 
 func (r *MongoDBSearchReconcileHelper) createOrUpdateShardStatefulSet(ctx context.Context, log *zap.SugaredLogger, shardName string, modifications ...statefulset.Modification) (*appsv1.StatefulSet, error) {
-	stsName := r.mdbSearch.MongotStatefulSetNamespacedNameForShard(shardName)
+	stsName := r.mdbSearch.MongotStatefulSetForShard(shardName)
 	sts := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: stsName.Name, Namespace: stsName.Namespace}}
 	op, err := controllerutil.CreateOrUpdate(ctx, r.client, sts, func() error {
 		statefulset.Apply(modifications...)(sts)
@@ -450,10 +450,10 @@ func (r *MongoDBSearchReconcileHelper) createOrUpdateShardStatefulSet(ctx contex
 	return sts, nil
 }
 
-// buildShardSearchHeadlessService builds a headless Service for a specific shard's mongot.
-func buildShardSearchHeadlessService(search *searchv1.MongoDBSearch, shardName string) corev1.Service {
-	svcName := search.MongotServiceNamespacedNameForShard(shardName)
-	stsName := search.MongotStatefulSetNamespacedNameForShard(shardName).Name
+// buildSearchHeadlessServiceForShard builds a headless Service for a specific shard's mongot.
+func buildSearchHeadlessServiceForShard(search *searchv1.MongoDBSearch, shardName string) corev1.Service {
+	svcName := search.MongotServiceForShard(shardName)
+	stsName := search.MongotStatefulSetForShard(shardName).Name
 
 	labels := map[string]string{
 		"app":   svcName.Name,
@@ -495,9 +495,9 @@ func buildShardSearchHeadlessService(search *searchv1.MongoDBSearch, shardName s
 	return serviceBuilder.Build()
 }
 
-// createShardMongotConfig creates the mongot configuration for a specific shard.
+// createMongotConfigForShard creates the mongot configuration for a specific shard.
 // Each shard's mongot connects to its own shard's mongod hosts.
-func createShardMongotConfig(search *searchv1.MongoDBSearch, shardedSource ShardedSearchSourceDBResource, shardIdx int) mongot.Modification {
+func createMongotConfigForShard(search *searchv1.MongoDBSearch, shardedSource SearchSourceShardedDeployment, shardIdx int) mongot.Modification {
 	return func(config *mongot.Config) {
 		hostAndPorts := shardedSource.HostSeedsForShard(shardIdx)
 
@@ -697,12 +697,12 @@ type perShardTLSResource struct {
 
 // TLSSecretNamespacedName returns the per-shard source secret name.
 func (p *perShardTLSResource) TLSSecretNamespacedName() types.NamespacedName {
-	return p.MongoDBSearch.TLSSecretNamespacedNameForShard(p.shardName)
+	return p.MongoDBSearch.TLSSecretForShard(p.shardName)
 }
 
 // TLSOperatorSecretNamespacedName returns the per-shard operator-managed secret name.
 func (p *perShardTLSResource) TLSOperatorSecretNamespacedName() types.NamespacedName {
-	return p.MongoDBSearch.TLSOperatorSecretNamespacedNameForShard(p.shardName)
+	return p.MongoDBSearch.TLSOperatorSecretForShard(p.shardName)
 }
 
 // ensureIngressTlsConfigForShard processes TLS configuration for a specific shard.
@@ -941,7 +941,7 @@ func mongotHostAndPort(search *searchv1.MongoDBSearch, clusterDomain string) str
 
 // shardMongotHostAndPort returns the internal service endpoint for a shard's mongot deployment
 func shardMongotHostAndPort(search *searchv1.MongoDBSearch, shardName string, clusterDomain string) string {
-	svcName := search.MongotServiceNamespacedNameForShard(shardName)
+	svcName := search.MongotServiceForShard(shardName)
 	port := search.GetEffectiveMongotPort()
 	return fmt.Sprintf("%s.%s.svc.%s:%d", svcName.Name, svcName.Namespace, clusterDomain, port)
 }

controllers/searchcontroller/mongodbsearch_reconcile_helper_test.go

@@ -684,13 +684,13 @@ func TestCreateShardMongotConfig(t *testing.T) {
 	}
 
 	config := mongot.Config{}
-	createShardMongotConfig(search, shardedSource, 0)(&config)
+	createMongotConfigForShard(search, shardedSource, 0)(&config)
 
 	assert.Equal(t, []string{"my-cluster-0-0.svc:27017", "my-cluster-0-1.svc:27017", "my-cluster-0-2.svc:27017"}, config.SyncSource.ReplicaSet.HostAndPort)
 	assert.Equal(t, search.SourceUsername(), config.SyncSource.ReplicaSet.Username)
 
 	config2 := mongot.Config{}
-	createShardMongotConfig(search, shardedSource, 1)(&config2)
+	createMongotConfigForShard(search, shardedSource, 1)(&config2)
 
 	assert.Equal(t, []string{"my-cluster-1-0.svc:27017", "my-cluster-1-1.svc:27017", "my-cluster-1-2.svc:27017"}, config2.SyncSource.ReplicaSet.HostAndPort)
 }
@@ -710,7 +710,7 @@ func TestShardedMongotConfigWithTLS(t *testing.T) {
 	}
 
 	config := mongot.Config{}
-	createShardMongotConfig(search, shardedSource, 0)(&config)
+	createMongotConfigForShard(search, shardedSource, 0)(&config)
 
 	assert.NotNil(t, config.SyncSource.ReplicaSet.TLS)
 	assert.False(t, *config.SyncSource.ReplicaSet.TLS, "ReplicaSet TLS should initially be false")
@@ -747,7 +747,7 @@ func TestShardedMongotConfigWithoutTLS(t *testing.T) {
 	}
 
 	config := mongot.Config{}
-	createShardMongotConfig(search, shardedSource, 0)(&config)
+	createMongotConfigForShard(search, shardedSource, 0)(&config)
 
 	assert.NotNil(t, config.SyncSource.ReplicaSet.TLS)
 	assert.False(t, *config.SyncSource.ReplicaSet.TLS, "ReplicaSet TLS should be false when source has no TLS")
@@ -801,7 +801,7 @@ func TestBuildShardSearchHeadlessService(t *testing.T) {
 	search := newTestMongoDBSearch("test-search", "test")
 	shardName := "my-cluster-0"
 
-	svc := buildShardSearchHeadlessService(search, shardName)
+	svc := buildSearchHeadlessServiceForShard(search, shardName)
 
 	assert.Equal(t, "test-search-mongot-my-cluster-0-svc", svc.Name)
 	assert.Equal(t, "test", svc.Namespace)
@@ -1076,106 +1076,13 @@ func TestTLSSecretNamespacedNameForShard(t *testing.T) {
 				}
 			})
 
-			secretNsName := search.TLSSecretNamespacedNameForShard(tc.shardName)
+			secretNsName := search.TLSSecretForShard(tc.shardName)
 			assert.Equal(t, tc.expectedSecretName, secretNsName.Name)
 			assert.Equal(t, tc.namespace, secretNsName.Namespace)
 		})
 	}
 }
 
-func TestTLSOperatorSecretNamespacedNameForShard(t *testing.T) {
-	testCases := []struct {
-		name               string
-		shardName          string
-		namespace          string
-		expectedSecretName string
-	}{
-		{
-			name:               "first shard",
-			shardName:          "my-cluster-0",
-			namespace:          "test-ns",
-			expectedSecretName: "my-cluster-0-search-certificate-key",
-		},
-		{
-			name:               "second shard",
-			shardName:          "my-cluster-1",
-			namespace:          "mongodb",
-			expectedSecretName: "my-cluster-1-search-certificate-key",
-		},
-		{
-			name:               "different shard naming",
-			shardName:          "shard-prod-0",
-			namespace:          "production",
-			expectedSecretName: "shard-prod-0-search-certificate-key",
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			search := newTestMongoDBSearch("test-search", tc.namespace)
-
-			secretNsName := search.TLSOperatorSecretNamespacedNameForShard(tc.shardName)
-			assert.Equal(t, tc.expectedSecretName, secretNsName.Name)
-			assert.Equal(t, tc.namespace, secretNsName.Namespace)
-		})
-	}
-}
-
-func TestPerShardTLSResourceAdapter(t *testing.T) {
-	testCases := []struct {
-		name                       string
-		secretPrefix               string
-		shardName                  string
-		namespace                  string
-		expectedSourceSecretName   string
-		expectedOperatorSecretName string
-	}{
-		{
-			name:                       "with prefix",
-			secretPrefix:               "my-prefix",
-			shardName:                  "my-cluster-0",
-			namespace:                  "test-ns",
-			expectedSourceSecretName:   "my-prefix-my-cluster-0-search-cert",
-			expectedOperatorSecretName: "my-cluster-0-search-certificate-key",
-		},
-		{
-			name:                       "without prefix",
-			secretPrefix:               "",
-			shardName:                  "shard-1",
-			namespace:                  "mongodb",
-			expectedSourceSecretName:   "shard-1-search-cert",
-			expectedOperatorSecretName: "shard-1-search-certificate-key",
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			search := newTestMongoDBSearch("test-search", tc.namespace, func(s *searchv1.MongoDBSearch) {
-				s.Spec.Security = searchv1.Security{
-					TLS: &searchv1.TLS{
-						CertsSecretPrefix: tc.secretPrefix,
-					},
-				}
-			})
-
-			adapter := &perShardTLSResource{
-				MongoDBSearch: search,
-				shardName:     tc.shardName,
-			}
-
-			// Test TLSSecretNamespacedName
-			sourceSecret := adapter.TLSSecretNamespacedName()
-			assert.Equal(t, tc.expectedSourceSecretName, sourceSecret.Name)
-			assert.Equal(t, tc.namespace, sourceSecret.Namespace)
-
-			// Test TLSOperatorSecretNamespacedName
-			operatorSecret := adapter.TLSOperatorSecretNamespacedName()
-			assert.Equal(t, tc.expectedOperatorSecretName, operatorSecret.Name)
-			assert.Equal(t, tc.namespace, operatorSecret.Namespace)
-		})
-	}
-}
-
 func TestReconcileSharded_CertificateKeySecretRefRejected(t *testing.T) {
 	search := newTestMongoDBSearch("test-search", "test-ns", func(s *searchv1.MongoDBSearch) {
 		s.Spec.Security = searchv1.Security{