Refactor common controller's updateOmAuthentication

* The method now accepts a value of type `corev1.SecretKeySelector`
  instead of an optional secret name string.
* Controllers using this method now explicitly provide a `corev1.SecretKeySelector`,
  replacing the previous implicit `""`.

Author: m1kola

controllers/operator/authentication_test.go

@@ -91,7 +91,8 @@ func TestUpdateOmAuthentication_NoAuthenticationEnabled(t *testing.T) {
 
 	kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
 	r := newReplicaSetReconciler(ctx, kubeClient, nil, "", "", false, false, omConnectionFactory.GetConnectionFunc)
-	r.updateOmAuthentication(ctx, conn, processNames, rs, "", "", "", false, zap.S())
+	agentCertSecretSelector := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name)
+	r.updateOmAuthentication(ctx, conn, processNames, rs, agentCertSecretSelector, "", "", false, zap.S())
 
 	ac, _ := conn.ReadAutomationConfig()
 
@@ -112,7 +113,8 @@ func TestUpdateOmAuthentication_EnableX509_TlsNotEnabled(t *testing.T) {
 
 	kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
 	r := newReplicaSetReconciler(ctx, kubeClient, nil, "", "", false, false, omConnectionFactory.GetConnectionFunc)
-	status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, conn, []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, "", "", "", false, zap.S())
+	agentCertSecretSelector := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name)
+	status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, conn, []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, agentCertSecretSelector, "", "", false, zap.S())
 
 	assert.True(t, status.IsOK(), "configuring both options at once should not result in a failed status")
 	assert.True(t, isMultiStageReconciliation, "configuring both tls and x509 at once should result in a multi stage reconciliation")
@@ -124,7 +126,8 @@ func TestUpdateOmAuthentication_EnableX509_WithTlsAlreadyEnabled(t *testing.T) {
 	omConnectionFactory := om.NewCachedOMConnectionFactoryWithInitializedConnection(om.NewMockedOmConnection(deployment.CreateFromReplicaSet("fake-mongoDBImage", false, rs)))
 	kubeClient := mock.NewDefaultFakeClientWithOMConnectionFactory(omConnectionFactory, rs)
 	r := newReplicaSetReconciler(ctx, kubeClient, nil, "", "", false, false, omConnectionFactory.GetConnectionFunc)
-	status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, "", "", "", false, zap.S())
+	agentCertSecretSelector := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name)
+	status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, agentCertSecretSelector, "", "", false, zap.S())
 
 	assert.True(t, status.IsOK(), "configuring x509 when tls has already been enabled should not result in a failed status")
 	assert.False(t, isMultiStageReconciliation, "if tls is already enabled, we should be able to configure x509 is a single reconciliation")
@@ -140,7 +143,8 @@ func TestUpdateOmAuthentication_AuthenticationIsNotConfigured_IfAuthIsNotSet(t *
 	kubeClient := mock.NewDefaultFakeClientWithOMConnectionFactory(omConnectionFactory, rs)
 	r := newReplicaSetReconciler(ctx, kubeClient, nil, "", "", false, false, omConnectionFactory.GetConnectionFunc)
 
-	status, _ := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, "", "", "", false, zap.S())
+	agentCertSecretSelector := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name)
+	status, _ := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, agentCertSecretSelector, "", "", false, zap.S())
 	assert.True(t, status.IsOK(), "no authentication should have been configured")
 
 	ac, _ := omConnectionFactory.GetConnection().ReadAutomationConfig()
@@ -211,7 +215,8 @@ func TestUpdateOmAuthentication_EnableX509_FromEmptyDeployment(t *testing.T) {
 	r := newReplicaSetReconciler(ctx, kubeClient, nil, "", "", false, false, omConnectionFactory.GetConnectionFunc)
 	createAgentCSRs(t, ctx, 1, r.client, certsv1.CertificateApproved)
 
-	status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, "", "", "", false, zap.S())
+	agentCertSecretSelector := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name)
+	status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, agentCertSecretSelector, "", "", false, zap.S())
 	assert.True(t, status.IsOK(), "configuring x509 and tls when there are no processes should not result in a failed status")
 	assert.False(t, isMultiStageReconciliation, "if we are enabling tls and x509 at once, this should be done in a single reconciliation")
 }

controllers/operator/common_controller.go

@@ -407,7 +407,7 @@ func getSubjectFromCertificate(cert string) (string, error) {
 // enables/disables authentication. If the authentication can't be fully configured, a boolean value indicating that
 // an additional reconciliation needs to be queued up to fully make the authentication changes is returned.
 // Note: updateOmAuthentication needs to be called before reconciling other auth related settings.
-func (r *ReconcileCommonController) updateOmAuthentication(ctx context.Context, conn om.Connection, processNames []string, ar authentication.AuthResource, agentCertSecretName string, caFilepath string, clusterFilePath string, isRecovering bool, log *zap.SugaredLogger) (status workflow.Status, multiStageReconciliation bool) {
+func (r *ReconcileCommonController) updateOmAuthentication(ctx context.Context, conn om.Connection, processNames []string, ar authentication.AuthResource, agentCertSecretSelector corev1.SecretKeySelector, caFilepath string, clusterFilePath string, isRecovering bool, log *zap.SugaredLogger) (status workflow.Status, multiStageReconciliation bool) {
 	// don't touch authentication settings if resource has not been configured with them
 	if ar.GetSecurity() == nil || ar.GetSecurity().Authentication == nil {
 		return workflow.OK(), false
@@ -480,17 +480,13 @@ func (r *ReconcileCommonController) updateOmAuthentication(ctx context.Context,
 
 	log.Debugf("Using authentication options %+v", authentication.Redact(authOpts))
 
-	agentSecretSelector := ar.GetSecurity().AgentClientCertificateSecretName(ar.GetName())
-	if agentCertSecretName != "" {
-		agentSecretSelector.Name = agentCertSecretName
-	}
 	wantToEnableAuthentication := ar.GetSecurity().Authentication.Enabled
 	if wantToEnableAuthentication && canConfigureAuthentication(ac, ar.GetSecurity().Authentication.GetModes(), log) {
 		log.Info("Configuring authentication for MongoDB resource")
 
 		if ar.GetSecurity().ShouldUseX509(ac.Auth.AutoAuthMechanism) || ar.GetSecurity().ShouldUseClientCertificates() {
 			agentSecret := &corev1.Secret{}
-			if err := r.client.Get(ctx, kube.ObjectKey(ar.GetNamespace(), agentSecretSelector.Name), agentSecret); client.IgnoreNotFound(err) != nil {
+			if err := r.client.Get(ctx, kube.ObjectKey(ar.GetNamespace(), agentCertSecretSelector.Name), agentSecret); client.IgnoreNotFound(err) != nil {
 				return workflow.Failed(err), false
 			}
 			// If the agent secret is of type TLS, we can find the certificate under the standard key,
@@ -500,10 +496,10 @@ func (r *ReconcileCommonController) updateOmAuthentication(ctx context.Context,
 			//
 			// Important: In multi cluster it is working with the TLS secret in the central cluster, hence below selector update.
 			if agentSecret.Type == corev1.SecretTypeTLS {
-				agentSecretSelector.Key = corev1.TLSCertKey
+				agentCertSecretSelector.Key = corev1.TLSCertKey
 			}
 
-			authOpts, err = r.configureAgentSubjects(ctx, ar.GetNamespace(), agentSecretSelector, authOpts, log)
+			authOpts, err = r.configureAgentSubjects(ctx, ar.GetNamespace(), agentCertSecretSelector, authOpts, log)
 			if err != nil {
 				return workflow.Failed(xerrors.Errorf("error configuring agent subjects: %w", err)), false
 			}
@@ -534,17 +530,17 @@ func (r *ReconcileCommonController) updateOmAuthentication(ctx context.Context,
 		return workflow.OK(), true
 	} else {
 		agentSecret := &corev1.Secret{}
-		if err := r.client.Get(ctx, kube.ObjectKey(ar.GetNamespace(), agentSecretSelector.Name), agentSecret); client.IgnoreNotFound(err) != nil {
+		if err := r.client.Get(ctx, kube.ObjectKey(ar.GetNamespace(), agentCertSecretSelector.Name), agentSecret); client.IgnoreNotFound(err) != nil {
 			return workflow.Failed(err), false
 		}
 
 		if agentSecret.Type == corev1.SecretTypeTLS {
-			agentSecretSelector.Name = fmt.Sprintf("%s%s", agentSecretSelector.Name, certs.OperatorGeneratedCertSuffix)
+			agentCertSecretSelector.Name = fmt.Sprintf("%s%s", agentCertSecretSelector.Name, certs.OperatorGeneratedCertSuffix)
 		}
 
 		// Should not fail if the Secret object with agent certs is not found.
 		// It will only exist on x509 client auth enabled deployments.
-		userOpts, err := r.readAgentSubjectsFromSecret(ctx, ar.GetNamespace(), agentSecretSelector, log)
+		userOpts, err := r.readAgentSubjectsFromSecret(ctx, ar.GetNamespace(), agentCertSecretSelector, log)
 		err = client.IgnoreNotFound(err)
 		if err != nil {
 			return workflow.Failed(err), true

controllers/operator/mongodbmultireplicaset_controller.go

@@ -758,9 +758,8 @@ func (r *ReconcileMongoDbMultiReplicaSet) updateOmDeploymentRs(ctx context.Conte
 
 	caFilePath := fmt.Sprintf("%s/ca-pem", util.TLSCaMountPath)
 
-	// We do not provide an agentCertSecretName on purpose because then we will default to the non pem secret on the central cluster.
-	// Below method has special code handling reading certificates from the central cluster in that case.
-	status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, rs.GetProcessNames(), &mrs, "", caFilePath, internalClusterPath, isRecovering, log)
+	agentCertSecretName := mrs.GetSecurity().AgentClientCertificateSecretName(mrs.GetName())
+	status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, rs.GetProcessNames(), &mrs, agentCertSecretName, caFilePath, internalClusterPath, isRecovering, log)
 	if !status.IsOK() && !isRecovering {
 		return xerrors.Errorf("failed to enable Authentication for MongoDB Multi Replicaset")
 	}

controllers/operator/mongodbreplicaset_controller.go

@@ -230,15 +230,15 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco
 		}
 	}
 
-	agentCertSecretName := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name).Name
-	agentCertSecretName += certs.OperatorGeneratedCertSuffix
+	agentCertSecretSelector := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name)
+	agentCertSecretSelector.Name += certs.OperatorGeneratedCertSuffix
 
 	// Recovery prevents some deadlocks that can occur during reconciliation, e.g. the setting of an incorrect automation
 	// configuration and a subsequent attempt to overwrite it later, the operator would be stuck in Pending phase.
 	// See CLOUDP-189433 and CLOUDP-229222 for more details.
 	if recovery.ShouldTriggerRecovery(rs.Status.Phase != mdbstatus.PhaseRunning, rs.Status.LastTransition) {
 		log.Warnf("Triggering Automatic Recovery. The MongoDB resource %s/%s is in %s state since %s", rs.Namespace, rs.Name, rs.Status.Phase, rs.Status.LastTransition)
-		automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, sts, log, caFilePath, agentCertSecretName, prometheusCertHash, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):")
+		automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, sts, log, caFilePath, agentCertSecretSelector, prometheusCertHash, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):")
 		deploymentError := create.DatabaseInKubernetes(ctx, r.client, *rs, sts, rsConfig, log)
 		if deploymentError != nil {
 			log.Errorf("Recovery failed because of deployment errors, %w", deploymentError)
@@ -254,7 +254,7 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco
 	}
 	status = workflow.RunInGivenOrder(publishAutomationConfigFirst(ctx, r.client, *rs, lastSpec, rsConfig, log),
 		func() workflow.Status {
-			return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, sts, log, caFilePath, agentCertSecretName, prometheusCertHash, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):")
+			return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, sts, log, caFilePath, agentCertSecretSelector, prometheusCertHash, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):")
 		},
 		func() workflow.Status {
 			workflowStatus := create.HandlePVCResize(ctx, r.client, &sts, log)
@@ -415,7 +415,7 @@ func AddReplicaSetController(ctx context.Context, mgr manager.Manager, imageUrls
 
 // updateOmDeploymentRs performs OM registration operation for the replicaset. So the changes will be finally propagated
 // to automation agents in containers
-func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, rs *mdbv1.MongoDB, set appsv1.StatefulSet, log *zap.SugaredLogger, caFilePath string, agentCertSecretName string, prometheusCertHash string, isRecovering bool) workflow.Status {
+func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, rs *mdbv1.MongoDB, set appsv1.StatefulSet, log *zap.SugaredLogger, caFilePath string, agentCertSecretSelector corev1.SecretKeySelector, prometheusCertHash string, isRecovering bool) workflow.Status {
 	log.Debug("Entering UpdateOMDeployments")
 	// Only "concrete" RS members should be observed
 	// - if scaling down, let's observe only members that will remain after scale-down operation
@@ -449,7 +449,7 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c
 		internalClusterPath = fmt.Sprintf("%s%s", util.InternalClusterAuthMountPath, hash)
 	}
 
-	status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, agentCertSecretName, caFilePath, internalClusterPath, isRecovering, log)
+	status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, agentCertSecretSelector, caFilePath, internalClusterPath, isRecovering, log)
 	if !status.IsOK() && !isRecovering {
 		return status
 	}

controllers/operator/mongodbshardedcluster_controller.go

@@ -1084,14 +1084,14 @@ func (r *ShardedClusterReconcileHelper) doShardedClusterProcessing(ctx context.C
 		return workflowStatus
 	}
 
-	agentCertSecretName := sc.GetSecurity().AgentClientCertificateSecretName(sc.Name).Name
+	agentCertSecretSelector := sc.GetSecurity().AgentClientCertificateSecretName(sc.Name)
 
 	opts = deploymentOptions{
-		podEnvVars:           podEnvVars,
-		currentAgentAuthMode: currentAgentAuthMode,
-		caFilePath:           caFilePath,
-		agentCertSecretName:  agentCertSecretName,
-		prometheusCertHash:   prometheusCertHash,
+		podEnvVars:              podEnvVars,
+		currentAgentAuthMode:    currentAgentAuthMode,
+		caFilePath:              caFilePath,
+		agentCertSecretSelector: agentCertSecretSelector,
+		prometheusCertHash:      prometheusCertHash,
 	}
 	allConfigs := r.getAllConfigs(ctx, *sc, opts, log)
 
@@ -1789,14 +1789,14 @@ func (r *ShardedClusterReconcileHelper) prepareScaleDownShardedCluster(omClient
 
 // deploymentOptions contains fields required for creating the OM deployment for the Sharded Cluster.
 type deploymentOptions struct {
-	podEnvVars           *env.PodEnvVars
-	currentAgentAuthMode string
-	caFilePath           string
-	agentCertSecretName  string
-	certTLSType          map[string]bool
-	finalizing           bool
-	processNames         []string
-	prometheusCertHash   string
+	podEnvVars              *env.PodEnvVars
+	currentAgentAuthMode    string
+	caFilePath              string
+	agentCertSecretSelector corev1.SecretKeySelector
+	certTLSType             map[string]bool
+	finalizing              bool
+	processNames            []string
+	prometheusCertHash      string
 }
 
 // updateOmDeploymentShardedCluster performs OM registration operation for the sharded cluster. So the changes will be finally propagated
@@ -1950,7 +1950,7 @@ func (r *ShardedClusterReconcileHelper) publishDeployment(ctx context.Context, c
 
 	logDiffOfProcessNames(opts.processNames, healthyProcessesToWaitForReadyState, log.With("ctx", "updateOmAuthentication"))
 
-	workflowStatus, additionalReconciliationRequired := r.commonController.updateOmAuthentication(ctx, conn, healthyProcessesToWaitForReadyState, sc, opts.agentCertSecretName, opts.caFilePath, "", isRecovering, log)
+	workflowStatus, additionalReconciliationRequired := r.commonController.updateOmAuthentication(ctx, conn, healthyProcessesToWaitForReadyState, sc, opts.agentCertSecretSelector, opts.caFilePath, "", isRecovering, log)
 	if !workflowStatus.IsOK() {
 		if !isRecovering {
 			return nil, false, workflowStatus

controllers/operator/mongodbstandalone_controller.go

@@ -319,8 +319,10 @@ func (r *ReconcileMongoDbStandalone) updateOmDeployment(ctx context.Context, con
 		return workflow.Failed(err)
 	}
 
+	agentCertSecretSelector := s.GetSecurity().AgentClientCertificateSecretName(s.Name)
+
 	// TODO standalone PR
-	status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, []string{set.Name}, s, "", "", "", isRecovering, log)
+	status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, []string{set.Name}, s, agentCertSecretSelector, "", "", isRecovering, log)
 	if !status.IsOK() {
 		return status
 	}