CLOUDP-130486 fix operator crash due to empty modes array (#1116)

* fix: log validation warning if modes array empty * fix: autoAuthMechanism defaults if modes array empty * fix: add unit test for GetScramOptions * update: make warning message more descriptive

Author: hoyhbx

api/v1/mongodbcommunity_types.go

@@ -602,23 +602,27 @@ func (m MongoDBCommunity) GetOwnerReferences() []metav1.OwnerReference {
 // GetScramOptions returns a set of Options that are used to configure scram
 // authentication.
 func (m MongoDBCommunity) GetScramOptions() scram.Options {
-
 ignoreUnknownUsers := true
 if m.Spec.Security.Authentication.IgnoreUnknownUsers != nil {
 ignoreUnknownUsers = *m.Spec.Security.Authentication.IgnoreUnknownUsers
 }
 
 authModes := m.Spec.Security.Authentication.Modes
 defaultAuthMechanism := ConvertAuthModeToAuthMechanism(defaultMode)
-autoAuthMechanism := ConvertAuthModeToAuthMechanism(authModes[0])
-
+var autoAuthMechanism string
 authMechanisms := make([]string, len(authModes))
 
-for i, authMode := range authModes {
-if authMech := ConvertAuthModeToAuthMechanism(authMode); authMech != "" {
-authMechanisms[i] = authMech
-if authMech == defaultAuthMechanism {
-autoAuthMechanism = defaultAuthMechanism
+if len(authModes) == 0 {
+autoAuthMechanism = defaultAuthMechanism
+} else {
+autoAuthMechanism = ConvertAuthModeToAuthMechanism(authModes[0])
+
+for i, authMode := range authModes {
+if authMech := ConvertAuthModeToAuthMechanism(authMode); authMech != "" {
+authMechanisms[i] = authMech
+if authMech == defaultAuthMechanism {
+autoAuthMechanism = defaultAuthMechanism
+}
 }
 }
 }

api/v1/mongodbcommunity_types_test.go

@@ -66,6 +66,17 @@ func TestMongodConfigurationWithNestedMapsAfterUnmarshalling(t *testing.T) {
 }, mc.Object)
 }
 
+func TestGetScramOptions(t *testing.T) {
+t.Run("Default AutoAuthMechanism set if modes array empty", func(t *testing.T) {
+mdb := newModesArray(nil, "empty-modes-array", "my-namespace")
+
+options := mdb.GetScramOptions()
+
+assert.EqualValues(t, defaultMode, options.AutoAuthMechanism)
+assert.EqualValues(t, []string{}, options.AutoAuthMechanisms)
+})
+}
+
 func TestGetScramCredentialsSecretName(t *testing.T) {
 testusers := []struct {
 in MongoDBUser
@@ -192,3 +203,21 @@ func newReplicaSet(members int, name, namespace string) MongoDBCommunity {
 },
 }
 }
+
+func newModesArray(modes []AuthMode, name, namespace string) MongoDBCommunity {
+return MongoDBCommunity{
+TypeMeta: metav1.TypeMeta{},
+ObjectMeta: metav1.ObjectMeta{
+Name: name,
+Namespace: namespace,
+},
+Spec: MongoDBCommunitySpec{
+Security: Security{
+Authentication: Authentication{
+Modes: modes,
+IgnoreUnknownUsers: nil,
+},
+},
+},
+}
+}

controllers/replica_set_controller.go

@@ -589,7 +589,7 @@ func (r ReplicaSetReconciler) validateSpec(mdb mdbv1.MongoDBCommunity) error {
 lastSuccessfulConfigurationSaved, ok := mdb.Annotations[lastSuccessfulConfiguration]
 if !ok {
 // First version of Spec
-return validation.ValidateInitalSpec(mdb)
+return validation.ValidateInitalSpec(mdb, r.log)
 }
 
 lastSpec := mdbv1.MongoDBCommunitySpec{}
@@ -598,7 +598,7 @@ func (r ReplicaSetReconciler) validateSpec(mdb mdbv1.MongoDBCommunity) error {
 return err
 }
 
-return validation.ValidateUpdate(mdb, lastSpec)
+return validation.ValidateUpdate(mdb, lastSpec, r.log)
 }
 
 func getCustomRolesModification(mdb mdbv1.MongoDBCommunity) (automationconfig.Modification, error) {

controllers/validation/validation.go

@@ -7,23 +7,24 @@ import (
 
 mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1"
 "github.com/mongodb/mongodb-kubernetes-operator/pkg/authentication/scram"
+"go.uber.org/zap"
 )
 
 // ValidateInitalSpec checks if the resource's initial Spec is valid.
-func ValidateInitalSpec(mdb mdbv1.MongoDBCommunity) error {
-return validateSpec(mdb)
+func ValidateInitalSpec(mdb mdbv1.MongoDBCommunity, log *zap.SugaredLogger) error {
+return validateSpec(mdb, log)
 }
 
 // ValidateUpdate validates that the new Spec, corresponding to the existing one, is still valid.
-func ValidateUpdate(mdb mdbv1.MongoDBCommunity, oldSpec mdbv1.MongoDBCommunitySpec) error {
+func ValidateUpdate(mdb mdbv1.MongoDBCommunity, oldSpec mdbv1.MongoDBCommunitySpec, log *zap.SugaredLogger) error {
 if oldSpec.Security.TLS.Enabled && !mdb.Spec.Security.TLS.Enabled {
 return errors.New("TLS can't be set to disabled after it has been enabled")
 }
-return validateSpec(mdb)
+return validateSpec(mdb, log)
 }
 
 // validateSpec validates the specs of the given resource definition.
-func validateSpec(mdb mdbv1.MongoDBCommunity) error {
+func validateSpec(mdb mdbv1.MongoDBCommunity, log *zap.SugaredLogger) error {
 if err := validateUsers(mdb); err != nil {
 return err
 }
@@ -32,7 +33,7 @@ func validateSpec(mdb mdbv1.MongoDBCommunity) error {
 return err
 }
 
-if err := validateAuthModeSpec(mdb); err != nil {
+if err := validateAuthModeSpec(mdb, log); err != nil {
 return err
 }
 
@@ -102,9 +103,14 @@ func validateArbiterSpec(mdb mdbv1.MongoDBCommunity) error {
 }
 
 // validateAuthModeSpec checks that the list of modes does not contain duplicates.
-func validateAuthModeSpec(mdb mdbv1.MongoDBCommunity) error {
+func validateAuthModeSpec(mdb mdbv1.MongoDBCommunity, log *zap.SugaredLogger) error {
 allModes := mdb.Spec.Security.Authentication.Modes
 
+// Issue warning if Modes array is empty
+if len(allModes) == 0 {
+log.Warnf("An empty Modes array has been provided. The default mode (SCRAM-SHA-256) will be used.")
+}
+
 // Check that no auth is defined more than once
 mapModes := make(map[mdbv1.AuthMode]struct{})
 for i, mode := range allModes {