diff --git a/controllers/operator/authentication_test.go b/controllers/operator/authentication_test.go index 6b47167de..8a2fdadf3 100644 --- a/controllers/operator/authentication_test.go +++ b/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") } diff --git a/controllers/operator/common_controller.go b/controllers/operator/common_controller.go index bf9cf2ec0..61fa4ea1b 100644 --- a/controllers/operator/common_controller.go +++ b/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 diff --git a/controllers/operator/mongodbmultireplicaset_controller.go b/controllers/operator/mongodbmultireplicaset_controller.go index fc2ccde71..8b5274179 100644 --- a/controllers/operator/mongodbmultireplicaset_controller.go +++ b/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") } diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index c16ed89c6..a7d2943aa 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/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 } diff --git a/controllers/operator/mongodbshardedcluster_controller.go b/controllers/operator/mongodbshardedcluster_controller.go index fb98984ef..1b94cac0f 100644 --- a/controllers/operator/mongodbshardedcluster_controller.go +++ b/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 diff --git a/controllers/operator/mongodbstandalone_controller.go b/controllers/operator/mongodbstandalone_controller.go index 1078fd6b8..549f4bf06 100644 --- a/controllers/operator/mongodbstandalone_controller.go +++ b/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 }