WIP: remove cert hash annotations

Author: m1kola

controllers/operator/certs/certificates.go

@@ -31,7 +31,6 @@ type certDestination string
 
 const (
 	OperatorGeneratedCertSuffix = "-pem"
-	CertHashAnnotationKey       = "certHash"
 
 	Unused     = "unused"
 	Database   = "database"

controllers/operator/construct/database_construction.go

@@ -466,14 +466,8 @@ func buildDatabaseStatefulSetConfigurationFunction(mdb databaseStatefulSetSource
 		appLabelKey: opts.ServiceName,
 	}
 
-	annotationFunc := statefulset.WithAnnotations(defaultPodAnnotations(opts.CertificateHash))
 	podTemplateAnnotationFunc := podtemplatespec.NOOP()
 
-	annotationFunc = statefulset.Apply(
-		annotationFunc,
-		statefulset.WithAnnotations(map[string]string{util.InternalCertAnnotationKey: opts.InternalClusterHash}),
-	)
-
 	if vault.IsVaultSecretBackend() {
 		podTemplateAnnotationFunc = podtemplatespec.Apply(podTemplateAnnotationFunc, podtemplatespec.WithAnnotations(secretsToInject.DatabaseAnnotations(mdb.GetNamespace())))
 	}
@@ -530,7 +524,6 @@ func buildDatabaseStatefulSetConfigurationFunction(mdb databaseStatefulSetSource
 		statefulset.WithServiceName(opts.ServiceName),
 		statefulset.WithReplicas(opts.Replicas),
 		statefulset.WithOwnerReference(opts.OwnerReference),
-		annotationFunc,
 		volumeClaimFuncs,
 		shareProcessNs,
 		statefulset.WithPodSpecTemplate(podtemplatespec.Apply(podTemplateModifications...)),
@@ -1057,15 +1050,6 @@ func DatabaseStartupProbe() probes.Modification {
 	)
 }
 
-func defaultPodAnnotations(certHash string) map[string]string {
-	return map[string]string{
-		// This annotation is necessary to trigger a pod restart
-		// if the certificate secret is out of date. This happens if
-		// existing certificates have been replaced/rotated/renewed.
-		certs.CertHashAnnotationKey: certHash,
-	}
-}
-
 // TODO: temprorary duplication to avoid circular imports
 func NewDefaultPodSpecWrapper(podSpec mdbv1.MongoDbPodSpec) *mdbv1.PodSpecWrapper {
 	return &mdbv1.PodSpecWrapper{

controllers/operator/construct/multicluster/multicluster_replicaset.go

@@ -7,7 +7,6 @@ import (
 
 	mdbv1 "github.com/mongodb/mongodb-kubernetes/api/v1/mdb"
 	mdbmultiv1 "github.com/mongodb/mongodb-kubernetes/api/v1/mdbmulti"
-	"github.com/mongodb/mongodb-kubernetes/controllers/operator/certs"
 	"github.com/mongodb/mongodb-kubernetes/controllers/operator/construct"
 	"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/util/merge"
 	"github.com/mongodb/mongodb-kubernetes/pkg/handler"
@@ -65,20 +64,19 @@ func WithStsOverride(stsOverride *appsv1.StatefulSetSpec) func(options *construc
 	}
 }
 
-func WithAnnotations(resourceName string, certHash string) func(options *construct.DatabaseStatefulSetOptions) {
+func WithAnnotations(resourceName string) func(options *construct.DatabaseStatefulSetOptions) {
 	return func(options *construct.DatabaseStatefulSetOptions) {
-		options.Annotations = statefulSetAnnotations(resourceName, certHash)
+		options.Annotations = statefulSetAnnotations(resourceName)
 	}
 }
 
 func statefulSetName(mdbmName string, clusterNum int) string {
 	return fmt.Sprintf("%s-%d", mdbmName, clusterNum)
 }
 
-func statefulSetAnnotations(mdbmName string, certHash string) map[string]string {
+func statefulSetAnnotations(mdbmName string) map[string]string {
 	return map[string]string{
 		handler.MongoDBMultiResourceAnnotation: mdbmName,
-		certs.CertHashAnnotationKey:            certHash,
 	}
 }
 

controllers/operator/mongodbmultireplicaset_controller.go

@@ -171,6 +171,21 @@ func (r *ReconcileMongoDbMultiReplicaSet) Reconcile(ctx context.Context, request
 		return r.updateStatus(ctx, &mrs, workflow.Failed(err), log)
 	}
 
+	// If tls is enabled we need to configure the "processes" array in opsManager/Cloud Manager with the
+	// correct certFilePath, with the new tls design, this path has the certHash in it(so that cert can be rotated
+	// without pod restart).
+	certificateFileName := ""
+	internalClusterPath := ""
+	if mrs.Spec.Security.IsTLSEnabled() {
+		if hash := firstStatefulSet.Annotations[util.InternalCertAnnotationKey]; hash != "" {
+			internalClusterPath = fmt.Sprintf("%s%s", util.InternalClusterAuthMountPath, hash)
+		}
+
+		if certificateHash := firstStatefulSet.Annotations[certs.CertHashAnnotationKey]; certificateHash != "" {
+			certificateFileName = fmt.Sprintf("%s/%s", util.TLSCertMountPath, certificateHash)
+		}
+	}
+
 	// 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.
@@ -499,7 +514,7 @@ func (r *ReconcileMongoDbMultiReplicaSet) reconcileStatefulSets(ctx context.Cont
 			mconstruct.WithClusterNum(clusterNum),
 			Replicas(replicasThisReconciliation),
 			mconstruct.WithStsOverride(&stsOverride),
-			mconstruct.WithAnnotations(mrs.Name, certHash),
+			mconstruct.WithAnnotations(mrs.Name),
 			mconstruct.WithServiceName(mrs.MultiHeadlessServiceName(clusterNum)),
 			PodEnvVars(newPodVars(conn, projectConfig, mrs.Spec.LogLevel)),
 			CurrentAgentAuthMechanism(currentAgentAuthMode),
@@ -725,27 +740,6 @@ func (r *ReconcileMongoDbMultiReplicaSet) updateOmDeploymentRs(ctx context.Conte
 	}
 	log.Debugf("Existing process Ids: %+v", processIds)
 
-	certificateFileName := ""
-	internalClusterPath := ""
-
-	// If tls is enabled we need to configure the "processes" array in opsManager/Cloud Manager with the
-	// correct certFilePath, with the new tls design, this path has the certHash in it(so that cert can be rotated
-	// without pod restart), we can get the cert hash from any of the statefulset, here we pick the statefulset in the first cluster.
-	if mrs.Spec.Security.IsTLSEnabled() {
-		firstStatefulSet, err := r.firstStatefulSet(ctx, &mrs)
-		if err != nil {
-			return err
-		}
-
-		if hash := firstStatefulSet.Annotations[util.InternalCertAnnotationKey]; hash != "" {
-			internalClusterPath = fmt.Sprintf("%s%s", util.InternalClusterAuthMountPath, hash)
-		}
-
-		if certificateHash := firstStatefulSet.Annotations[certs.CertHashAnnotationKey]; certificateHash != "" {
-			certificateFileName = fmt.Sprintf("%s/%s", util.TLSCertMountPath, certificateHash)
-		}
-	}
-
 	processes, err := process.CreateMongodProcessesWithLimitMulti(r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, mrs, certificateFileName)
 	if err != nil && !isRecovering {
 		return err

controllers/operator/mongodbreplicaset_controller.go

@@ -198,11 +198,13 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco
 		}
 	}
 
+	internalClusterCertHash := enterprisepem.ReadHashFromSecret(ctx, r.SecretClient, rs.Namespace, rsCertsConfig.InternalClusterSecretName, databaseSecretPath, log)
+
 	rsConfig := construct.ReplicaSetOptions(
 		PodEnvVars(newPodVars(conn, projectConfig, rs.Spec.LogLevel)),
 		CurrentAgentAuthMechanism(currentAgentAuthMode),
 		CertificateHash(enterprisepem.ReadHashFromSecret(ctx, r.SecretClient, rs.Namespace, rsCertsConfig.CertSecretName, databaseSecretPath, log)),
-		InternalClusterHash(enterprisepem.ReadHashFromSecret(ctx, r.SecretClient, rs.Namespace, rsCertsConfig.InternalClusterSecretName, databaseSecretPath, log)),
+		InternalClusterHash(internalClusterCertHash),
 		PrometheusTLSCertHash(prometheusCertHash),
 		WithVaultConfig(vaultConfig),
 		WithLabels(rs.Labels),
@@ -233,12 +235,17 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco
 	agentCertSecretSelector := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name)
 	agentCertSecretSelector.Name += certs.OperatorGeneratedCertSuffix
 
+	internalClusterPath := ""
+	if internalClusterCertHash != "" {
+		internalClusterPath = fmt.Sprintf("%s%s", util.InternalClusterAuthMountPath, internalClusterCertHash)
+	}
+
 	// 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, agentCertSecretSelector, prometheusCertHash, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):")
+		automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, sts, log, caFilePath, internalClusterPath, 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 +261,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, agentCertSecretSelector, prometheusCertHash, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):")
+			return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, sts, log, caFilePath, internalClusterPath, 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 +422,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, agentCertSecretSelector corev1.SecretKeySelector, 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, internalClusterPath 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
@@ -444,11 +451,6 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c
 	replicaSet := replicaset.BuildFromStatefulSetWithReplicas(r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, set, rs.GetSpec(), updatedMembers, rs.CalculateFeatureCompatibilityVersion())
 	processNames := replicaSet.GetProcessNames()
 
-	internalClusterPath := ""
-	if hash := set.Annotations[util.InternalCertAnnotationKey]; hash != "" {
-		internalClusterPath = fmt.Sprintf("%s%s", util.InternalClusterAuthMountPath, hash)
-	}
-
 	status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, agentCertSecretSelector, caFilePath, internalClusterPath, isRecovering, log)
 	if !status.IsOK() && !isRecovering {
 		return status

pkg/statefulset/statefulset_util.go

@@ -19,7 +19,6 @@ import (
 	apiEquality "k8s.io/apimachinery/pkg/api/equality"
 	apiErrors "k8s.io/apimachinery/pkg/api/errors"
 
-	"github.com/mongodb/mongodb-kubernetes/controllers/operator/certs"
 	"github.com/mongodb/mongodb-kubernetes/controllers/operator/inspect"
 	"github.com/mongodb/mongodb-kubernetes/controllers/operator/workflow"
 	kubernetesClient "github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/kube/client"
@@ -121,14 +120,6 @@ func CreateOrUpdateStatefulset(ctx context.Context, getUpdateCreator kubernetesC
 		log.Debug("Created StatefulSet")
 		return statefulSetToCreate, nil
 	}
-	// preserve existing certificate hash if new one is not statefulSetToCreate
-	existingCertHash, okExisting := existingStatefulSet.Spec.Template.Annotations[certs.CertHashAnnotationKey]
-	if newCertHash, okNew := statefulSetToCreate.Spec.Template.Annotations[certs.CertHashAnnotationKey]; existingCertHash != "" && newCertHash == "" && okExisting && okNew {
-		if statefulSetToCreate.Spec.Template.Annotations == nil {
-			statefulSetToCreate.Spec.Template.Annotations = map[string]string{}
-		}
-		statefulSetToCreate.Spec.Template.Annotations[certs.CertHashAnnotationKey] = existingCertHash
-	}
 
 	// there already exists a pvc size annotation, that means we did resize at least once
 	// we need to make sure to keep the annotation.

pkg/util/constants.go

@@ -281,10 +281,6 @@ const (
 	TLSCertMountPath = PvcMmsHomeMountPath + "/tls"
 	TLSCaMountPath   = PvcMmsHomeMountPath + "/tls/ca"
 
-	// TODO: remove this from here and move it to the certs package
-	// This currently creates an import cycle
-	InternalCertAnnotationKey = "internalCertHash"
-
 	// Annotation keys used by the operator
 	LastAchievedSpec        = "mongodb.com/v1.lastSuccessfulConfiguration"
 	LastAchievedRsMemberIds = "mongodb.com/v1.lastAchievedRsMemberIds"