Merge branch 'master' into feature/mk-oidc-crd-propagation

MaciejKaras · MaciejKaras · commit 1f504cfdc515 · 2025-06-02T10:11:31.000+02:00
# Conflicts:
#	api/v1/mdb/mongodb_types.go
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -1,3 +1 @@
 * @mongodb/kubernetes-hosted
-
-helm_chart/crds/ @dan-mckean @vinilage
diff --git a/api/v1/mdb/mongodb_types.go b/api/v1/mdb/mongodb_types.go
@@ -807,14 +807,6 @@ func (s *Security) IsTLSEnabled() bool {
 	return s.CertificatesSecretsPrefix != ""
 }
 
-func (s *Security) IsOIDCEnabled() bool {
-	if s == nil || s.Authentication == nil || !s.Authentication.Enabled {
-		return false
-	}
-
-	return s.Authentication.IsOIDCEnabled()
-}
-
 // GetAgentMechanism returns the authentication mechanism that the agents will be using.
 // The agents will use X509 if it is the only mechanism specified, otherwise they will use SCRAM if specified
 // and no auth if no mechanisms exist.
@@ -1007,16 +999,28 @@ type AgentAuthentication struct {
 // IsX509Enabled determines if X509 is to be enabled at the project level
 // it does not necessarily mean that the agents are using X509 authentication
 func (a *Authentication) IsX509Enabled() bool {
+	if a == nil || !a.Enabled {
+		return false
+	}
+
 	return stringutil.Contains(a.GetModes(), util.X509)
 }
 
 // IsLDAPEnabled determines if LDAP is to be enabled at the project level
 func (a *Authentication) IsLDAPEnabled() bool {
+	if a == nil || !a.Enabled {
+		return false
+	}
+
 	return stringutil.Contains(a.GetModes(), util.LDAP)
 }
 
 // IsOIDCEnabled determines if OIDC is to be enabled at the project level
 func (a *Authentication) IsOIDCEnabled() bool {
+	if a == nil || !a.Enabled {
+		return false
+	}
+
 	return stringutil.Contains(a.GetModes(), util.OIDC)
 }
 
diff --git a/api/v1/mdb/mongodb_types_test.go b/api/v1/mdb/mongodb_types_test.go
@@ -83,10 +83,20 @@ func TestGetAuthenticationIsEnabledMethods(t *testing.T) {
 			expectedLDAP:   false,
 			expectedOIDC:   false,
 		},
+		{
+			name: "Empty authentication mode list",
+			authentication: &Authentication{
+				Enabled: true,
+			},
+			expectedX509: false,
+			expectedLDAP: false,
+			expectedOIDC: false,
+		},
 		{
 			name: "Authentication with x509 only",
 			authentication: &Authentication{
-				Modes: []AuthMode{util.X509},
+				Enabled: true,
+				Modes:   []AuthMode{util.X509},
 			},
 			expectedX509: true,
 			expectedLDAP: false,
@@ -95,7 +105,8 @@ func TestGetAuthenticationIsEnabledMethods(t *testing.T) {
 		{
 			name: "Authentication with LDAP only",
 			authentication: &Authentication{
-				Modes: []AuthMode{util.LDAP},
+				Enabled: true,
+				Modes:   []AuthMode{util.LDAP},
 			},
 			expectedX509: false,
 			expectedLDAP: true,
@@ -104,7 +115,8 @@ func TestGetAuthenticationIsEnabledMethods(t *testing.T) {
 		{
 			name: "Authentication with OIDC only",
 			authentication: &Authentication{
-				Modes: []AuthMode{util.OIDC},
+				Enabled: true,
+				Modes:   []AuthMode{util.OIDC},
 			},
 			expectedX509: false,
 			expectedLDAP: false,
@@ -113,7 +125,8 @@ func TestGetAuthenticationIsEnabledMethods(t *testing.T) {
 		{
 			name: "Authentication with multiple modes",
 			authentication: &Authentication{
-				Modes: []AuthMode{util.X509, util.LDAP, util.OIDC, util.SCRAM},
+				Enabled: true,
+				Modes:   []AuthMode{util.X509, util.LDAP, util.OIDC, util.SCRAM},
 			},
 			expectedX509: true,
 			expectedLDAP: true,
diff --git a/api/v1/mdb/mongodb_validation.go b/api/v1/mdb/mongodb_validation.go
@@ -54,7 +54,7 @@ func deploymentsMustHaveTLSInX509Env(d DbCommonSpec) v1.ValidationResult {
 	if authSpec == nil {
 		return v1.ValidationSuccess()
 	}
-	if authSpec.Enabled && authSpec.IsX509Enabled() && !d.GetSecurity().IsTLSEnabled() {
+	if authSpec.IsX509Enabled() && !d.GetSecurity().IsTLSEnabled() {
 		return v1.ValidationError("Cannot have a non-tls deployment when x509 authentication is enabled")
 	}
 	return v1.ValidationSuccess()
@@ -107,11 +107,15 @@ func scramSha1AuthValidation(d DbCommonSpec) v1.ValidationResult {
 
 func oidcAuthValidators(db DbCommonSpec) []func(DbCommonSpec) v1.ValidationResult {
 	validators := make([]func(DbCommonSpec) v1.ValidationResult, 0)
-	if !db.Security.IsOIDCEnabled() {
+	if db.Security == nil || db.Security.Authentication == nil {
 		return validators
 	}
 
 	authentication := db.Security.Authentication
+	if !authentication.IsOIDCEnabled() {
+		return validators
+	}
+
 	validators = append(validators, oidcAuthModeValidator(authentication))
 	validators = append(validators, oidcAuthRequiresEnterprise)
 
diff --git a/api/v1/om/opsmanager_types.go b/api/v1/om/opsmanager_types.go
@@ -633,7 +633,7 @@ func ensureSecurityWithSCRAM(specSecurity *mdbv1.Security) *mdbv1.Security {
 		specSecurity = &mdbv1.Security{TLSConfig: &mdbv1.TLSConfig{}}
 	}
 	// the only allowed authentication is SCRAM - it's implicit to the user and hidden from him
-	specSecurity.Authentication = &mdbv1.Authentication{Modes: []mdbv1.AuthMode{util.SCRAM}}
+	specSecurity.Authentication = &mdbv1.Authentication{Enabled: true, Modes: []mdbv1.AuthMode{util.SCRAM}}
 	return specSecurity
 }
 
diff --git a/controllers/operator/mongodbshardedcluster_controller.go b/controllers/operator/mongodbshardedcluster_controller.go
@@ -1014,7 +1014,7 @@ func (r *ShardedClusterReconcileHelper) doShardedClusterProcessing(ctx context.C
 	}
 
 	security := sc.Spec.Security
-	if security.Authentication != nil && security.Authentication.Enabled && security.Authentication.IsX509Enabled() && !sc.Spec.GetSecurity().IsTLSEnabled() {
+	if security.Authentication.IsX509Enabled() && !security.IsTLSEnabled() {
 		return workflow.Invalid("cannot have a non-tls deployment when x509 authentication is enabled")
 	}
 
diff --git a/controllers/operator/mongodbstandalone_controller.go b/controllers/operator/mongodbstandalone_controller.go
@@ -177,7 +177,7 @@ func (r *ReconcileMongoDbStandalone) Reconcile(ctx context.Context, request reco
 	// cannot have a non-tls deployment in an x509 environment
 	// TODO move to webhook validations
 	security := s.Spec.Security
-	if security.Authentication != nil && security.Authentication.Enabled && security.Authentication.IsX509Enabled() && !s.Spec.GetSecurity().IsTLSEnabled() {
+	if security.Authentication.IsX509Enabled() && !security.IsTLSEnabled() {
 		return r.updateStatus(ctx, s, workflow.Invalid("cannot have a non-tls deployment when x509 authentication is enabled"), log)
 	}
 
diff --git a/docker/mongodb-kubernetes-tests/tests/multicluster_shardedcluster/multi_cluster_sharded_disaster_recovery.py b/docker/mongodb-kubernetes-tests/tests/multicluster_shardedcluster/multi_cluster_sharded_disaster_recovery.py
@@ -11,10 +11,7 @@
     try_load,
     update_configmap,
 )
-from kubetester.kubetester import (
-    KubernetesTester,
-    ensure_ent_version,
-)
+from kubetester.kubetester import KubernetesTester, ensure_ent_version
 from kubetester.kubetester import fixture as yaml_fixture
 from kubetester.kubetester import (
     get_env_var_or_fail,
diff --git a/docker/mongodb-kubernetes-tests/tests/opsmanager/om_ops_manager_backup_sharded_cluster.py b/docker/mongodb-kubernetes-tests/tests/opsmanager/om_ops_manager_backup_sharded_cluster.py
@@ -23,9 +23,7 @@
 from tests.shardedcluster.conftest import (
     enable_multi_cluster_deployment as enable_multi_cluster_deployment_mdb,
 )
-from tests.shardedcluster.conftest import (
-    get_mongos_service_names,
-)
+from tests.shardedcluster.conftest import get_mongos_service_names
 
 HEAD_PATH = "/head/"
 S3_SECRET_NAME = "my-s3-secret"
diff --git a/docker/mongodb-kubernetes-tests/tests/opsmanager/om_ops_manager_upgrade.py b/docker/mongodb-kubernetes-tests/tests/opsmanager/om_ops_manager_upgrade.py
@@ -7,9 +7,7 @@
 from kubernetes.client.rest import ApiException
 from kubetester import MongoDB, try_load
 from kubetester.awss3client import AwsS3Client
-from kubetester.kubetester import (
-    ensure_ent_version,
-)
+from kubetester.kubetester import ensure_ent_version
 from kubetester.kubetester import fixture as yaml_fixture
 from kubetester.kubetester import (
     is_default_architecture_static,
diff --git a/docker/mongodb-kubernetes-tests/tests/shardedcluster/sharded_cluster_migration.py b/docker/mongodb-kubernetes-tests/tests/shardedcluster/sharded_cluster_migration.py
@@ -2,10 +2,7 @@
 import pytest
 from kubernetes import client
 from kubetester import MongoDB, try_load
-from kubetester.kubetester import (
-    assert_statefulset_architecture,
-    ensure_ent_version,
-)
+from kubetester.kubetester import assert_statefulset_architecture, ensure_ent_version
 from kubetester.kubetester import fixture as load_fixture
 from kubetester.kubetester import (
     get_default_architecture,
diff --git a/pkg/telemetry/collector.go b/pkg/telemetry/collector.go
@@ -24,7 +24,6 @@ import (
 	"github.com/mongodb/mongodb-kubernetes/pkg/images"
 	"github.com/mongodb/mongodb-kubernetes/pkg/util"
 	"github.com/mongodb/mongodb-kubernetes/pkg/util/architectures"
-	"github.com/mongodb/mongodb-kubernetes/pkg/util/maputil"
 	"github.com/mongodb/mongodb-kubernetes/pkg/util/versionutil"
 )
 
@@ -179,19 +178,13 @@ func collectOperatorSnapshot(ctx context.Context, memberClusterMap map[string]Co
 		OperatorVersion:      versionutil.StaticContainersOperatorVersion(),
 		OperatorType:         MEKO,
 	}
-	operatorProperties, err := maputil.StructToMap(operatorEvent)
-	if err != nil {
-		Logger.Debugf("failed converting properties to map: %s", err)
-		return nil
-	}
 
-	return []Event{
-		{
-			Timestamp:  time.Now(),
-			Source:     Operators,
-			Properties: operatorProperties,
-		},
+	event := createEvent(operatorEvent, time.Now(), Operators)
+	if event == nil {
+		return []Event{}
 	}
+
+	return []Event{*event}
 }
 
 func collectDeploymentsSnapshot(ctx context.Context, operatorClusterMgr manager.Manager, operatorUUID, mongodbImage, databaseNonStaticImage string) []Event {
@@ -234,6 +227,8 @@ func getMdbEvents(ctx context.Context, operatorClusterClient kubeclient.Client,
 				Type:                     string(item.Spec.GetResourceType()),
 				IsRunningEnterpriseImage: images.IsEnterpriseImage(imageURL),
 				ExternalDomains:          getExternalDomainProperty(item),
+				AuthenticationModes:      getAuthenticationModes(item.Spec.Security),
+				AuthenticationAgentMode:  getAuthenticationAgentMode(item.Spec.Security),
 			}
 
 			if numberOfClustersUsed > 0 {
@@ -272,6 +267,8 @@ func addMultiEvents(ctx context.Context, operatorClusterClient kubeclient.Client
 			Type:                     string(item.Spec.GetResourceType()),
 			IsRunningEnterpriseImage: images.IsEnterpriseImage(imageURL),
 			ExternalDomains:          getExternalDomainPropertyForMongoDBMulti(item),
+			AuthenticationModes:      getAuthenticationModes(item.Spec.Security),
+			AuthenticationAgentMode:  getAuthenticationAgentMode(item.Spec.Security),
 		}
 
 		if event := createEvent(properties, now, Deployments); event != nil {
@@ -319,8 +316,8 @@ func addOmEvents(ctx context.Context, operatorClusterClient kubeclient.Client, o
 	return events
 }
 
-func createEvent(properties any, now time.Time, eventType EventType) *Event {
-	convertedProperties, err := maputil.StructToMap(properties)
+func createEvent(properties FlatProperties, now time.Time, eventType EventType) *Event {
+	convertedProperties, err := properties.ConvertToFlatMap()
 	if err != nil {
 		Logger.Debugf("failed to parse %s properties: %v", eventType, err)
 		return nil
@@ -422,13 +419,12 @@ func handleEvents(ctx context.Context, atlasClient *Client, events []Event, even
 		return
 	}
 
-	err = atlasClient.SendEventWithRetry(ctx, events)
-	if err == nil {
-		if err := updateTelemetryConfigMapTimeStamp(ctx, operatorClusterClient, namespace, OperatorConfigMapTelemetryConfigMapName, eventType); err != nil {
-			Logger.Debugf("Failed saving timestamp of successful sending of data for type: %s with error: %s", eventType, err)
+	if sendErr := atlasClient.SendEventWithRetry(ctx, events); sendErr == nil {
+		if updateErr := updateTelemetryConfigMapTimeStamp(ctx, operatorClusterClient, namespace, OperatorConfigMapTelemetryConfigMapName, eventType); updateErr != nil {
+			Logger.Debugf("Failed saving timestamp of successful sending of data for type: %s with error: %s", eventType, updateErr)
 		}
 	} else {
-		Logger.Debugf("Encountered error while trying to send payload to atlas; err: %s", err)
+		Logger.Debugf("Encountered error while trying to send payload to atlas; err: %s", sendErr)
 	}
 }
 
@@ -534,3 +530,27 @@ func isExternalDomainSpecifiedInClusterSpecList(clusterSpecList mdbv1.ClusterSpe
 
 	return clusterSpecList.IsExternalDomainSpecifiedInClusterSpecList()
 }
+
+func getAuthenticationModes(security *mdbv1.Security) []string {
+	if security == nil || security.Authentication == nil {
+		return nil
+	}
+
+	if !security.Authentication.Enabled {
+		return nil
+	}
+
+	return security.Authentication.GetModes()
+}
+
+func getAuthenticationAgentMode(security *mdbv1.Security) string {
+	if security == nil || security.Authentication == nil {
+		return ""
+	}
+
+	if !security.Authentication.Enabled {
+		return ""
+	}
+
+	return security.Authentication.Agents.Mode
+}
diff --git a/pkg/telemetry/collector_test.go b/pkg/telemetry/collector_test.go
diff --git a/pkg/telemetry/types.go b/pkg/telemetry/types.go

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1 @@`
`1`	`1`	`* @mongodb/kubernetes-hosted`
`2`		`-`
`3`		`-helm_chart/crds/ @dan-mckean @vinilage`
Original file line number	Diff line number	Diff line change
`@@ -633,7 +633,7 @@ func ensureSecurityWithSCRAM(specSecurity mdbv1.Security) mdbv1.Security {`
`633`	`633`	`specSecurity = &mdbv1.Security{TLSConfig: &mdbv1.TLSConfig{}}`
`634`	`634`	`}`
`635`	`635`	`// the only allowed authentication is SCRAM - it's implicit to the user and hidden from him`
`636`		`- specSecurity.Authentication = &mdbv1.Authentication{Modes: []mdbv1.AuthMode{util.SCRAM}}`
	`636`	`+ specSecurity.Authentication = &mdbv1.Authentication{Enabled: true, Modes: []mdbv1.AuthMode{util.SCRAM}}`
`637`	`637`	`return specSecurity`
`638`	`638`	`}`
`639`	`639`
Original file line number	Diff line number	Diff line change
`@@ -1014,7 +1014,7 @@ func (r *ShardedClusterReconcileHelper) doShardedClusterProcessing(ctx context.C`
`1014`	`1014`	`}`
`1015`	`1015`
`1016`	`1016`	`security := sc.Spec.Security`
`1017`		`- if security.Authentication != nil && security.Authentication.Enabled && security.Authentication.IsX509Enabled() && !sc.Spec.GetSecurity().IsTLSEnabled() {`
	`1017`	`+ if security.Authentication.IsX509Enabled() && !security.IsTLSEnabled() {`
`1018`	`1018`	`return workflow.Invalid("cannot have a non-tls deployment when x509 authentication is enabled")`
`1019`	`1019`	`}`
`1020`	`1020`
Original file line number	Diff line number	Diff line change
`@@ -177,7 +177,7 @@ func (r *ReconcileMongoDbStandalone) Reconcile(ctx context.Context, request reco`
`177`	`177`	`// cannot have a non-tls deployment in an x509 environment`
`178`	`178`	`// TODO move to webhook validations`
`179`	`179`	`security := s.Spec.Security`
`180`		`- if security.Authentication != nil && security.Authentication.Enabled && security.Authentication.IsX509Enabled() && !s.Spec.GetSecurity().IsTLSEnabled() {`
	`180`	`+ if security.Authentication.IsX509Enabled() && !security.IsTLSEnabled() {`
`181`	`181`	`return r.updateStatus(ctx, s, workflow.Invalid("cannot have a non-tls deployment when x509 authentication is enabled"), log)`
`182`	`182`	`}`
`183`	`183`