CLOUDP-130486 fix operator crash due to empty modes array

[hoyhbx]

What problem does this PR solve?

This PR closes closes #1055.

When spec.security.modes field is set to an empty array, the operator crashes. Upon inspection of the operator error log, we find that the crash happens when the operator is Validating MongoDB.Spec as it results in a index out of range [0] with length 0 runtime error.

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])

	authMechanisms := make([]string, len(authModes))

	for i, authMode := range authModes {
		if authMech := ConvertAuthModeToAuthMechanism(authMode); authMech != "" {
			authMechanisms[i] = authMech
			if authMech == defaultAuthMechanism {
				autoAuthMechanism = defaultAuthMechanism
			}
		}
	}

	return scram.Options{
		AuthoritativeSet:   !ignoreUnknownUsers,
		KeyFile:            scram.AutomationAgentKeyFilePathInContainer,
		AutoAuthMechanisms: authMechanisms,
		AgentName:          scram.AgentName,
		AutoAuthMechanism:  autoAuthMechanism,
	}
}

It seems that when setting the autoAuthMechanism the operator tries to assign the first authentication mode at the 0th index of the Modes array. Since the array is empty, the index is out of range.

What changes were made?

During validation of the authModeSpec, in addition to checking for duplicates, we can include an additional check as part of validateAuthModeSpec in controllers/validation/validation.go:

func validateAuthModeSpec(mdb mdbv1.MongoDBCommunity) error {
	allModes := mdb.Spec.Security.Authentication.Modes

	// Check that no auth is defined more than once
	mapModes := make(map[mdbv1.AuthMode]struct{})
	for i, mode := range allModes {
		if value := mdbv1.ConvertAuthModeToAuthMechanism(mode); value == "" {
			return fmt.Errorf("unexpected value (%q) defined for supported authentication modes", value)
		}
		mapModes[allModes[i]] = struct{}{}
	}
	if len(mapModes) != len(allModes) {
		return fmt.Errorf("some authentication modes are declared twice or more")
	}

	return nil
}

if len(allModes) == 0 {
	return fmt.Errorf("modes list is empty.")
}

Code changes

• Has Go code change
• Has CI related scripts change

Tests

• Unit test
• E2E test
• Manual test
• No code

This is a simple fix and we suppose no test above is needed.

Side effects

• Breaking backward compatibility
• Other side effects:

Related changes

• Need to cherry-pick to the release branch
• Need to update the documentation

All Submissions:

• Have you opened an Issue before filing this PR?
• Have you signed our CLA?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if such).

[slaskawi]

Waiting for the author to address the comments mentioned in the inline review.

[hoyhbx]

@slaskawi Hi, we were wondering what is the final desired behavior of handling this case? Falling back to default makes sense, but we were wondering if this validation function is the correct place to defaulting? Seems validateAuthModeSpec should only do validation?

[slaskawi]

The code LGTM. However it would be great to make the warning message a bit more verbose and a bit more descriptive.

The other bits are fine.

[slaskawi]

Hi, we were wondering what is the final desired behavior of handling this case? Falling back to default makes sense, but we were wondering if this validation function is the correct place to defaulting? Seems validateAuthModeSpec should only do validation?

@hoyhbx Yes, precisely!

[slaskawi]

Integrated, thanks @hoyhbx