Skip to content

Commit 3b5eec1

Browse files
authored
CLOUDP-317713: Separate authentication mechanism checks (#143)
## Summary of Changes - Introduced a new `IsDeploymentAuthenticationEnabled` method to the `Mechanism` interface. - Implemented `IsDeploymentAuthenticationEnabled` for all authentication mechanism types to check only if a mechanism is enabled (present in the config), without comparing configuration objects. - Updated the removal/disable logic to use this new method, ensuring mechanisms like LDAP and OIDC can be properly disabled even when configuration objects differ or are missing. - Retained the original `IsDeploymentAuthenticationConfigured` method for cases where strict configuration matching is required. Previously, the `IsDeploymentAuthenticationConfigured` method was used for both modifying and removing authentication mechanisms. However, its implementation checked both presence and exact configuration, which caused issues when removing mechanisms like LDAP or OIDC—these have separate configuration objects that may not exist locally after removal, causing the method to return `false` and preventing proper disabling. By separating this logic: - `IsDeploymentAuthenticationEnabled` is used for removal/disabling and checks only for presence. - `IsDeploymentAuthenticationConfigured` remains for full config matching. ## Proof of Work Tests pass, added an Ldap removal test to verify that mechanism removal works. ## Checklist - [x] Have you linked a jira ticket and/or is the ticket in the title? - [x] Have you checked whether your jira ticket required DOCSP changes? - [ ] Have you checked for release_note changes?
1 parent 943128f commit 3b5eec1

File tree

6 files changed

+27
-5
lines changed

6 files changed

+27
-5
lines changed

controllers/operator/authentication/authentication.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ func removeUnsupportedDeploymentMechanisms(conn om.Connection, opts Options, log
320320
unsupportedMechanisms := mechanismsToDisable(automationConfigAuthMechanisms)
321321

322322
log.Infow("Removing unsupported deployment authentication mechanisms", "Mechanisms", unsupportedMechanisms)
323-
if err := ensureDeploymentMechanismsAreDisabled(conn, ac, unsupportedMechanisms, opts, log); err != nil {
323+
if err := ensureDeploymentMechanismsAreDisabled(conn, ac, unsupportedMechanisms, log); err != nil {
324324
return xerrors.Errorf("error ensuring deployment mechanisms are disabled: %w", err)
325325
}
326326

@@ -397,10 +397,10 @@ func ensureDeploymentMechanisms(conn om.Connection, ac *om.AutomationConfig, mec
397397

398398
// ensureDeploymentMechanismsAreDisabled configures the given AutomationConfig to allow deployments to
399399
// authenticate using the specified mechanisms
400-
func ensureDeploymentMechanismsAreDisabled(conn om.Connection, ac *om.AutomationConfig, mechanismsToDisable MechanismList, opts Options, log *zap.SugaredLogger) error {
400+
func ensureDeploymentMechanismsAreDisabled(conn om.Connection, ac *om.AutomationConfig, mechanismsToDisable MechanismList, log *zap.SugaredLogger) error {
401401
deploymentMechanismsToDisable := make([]Mechanism, 0)
402402
for _, mechanism := range mechanismsToDisable {
403-
if mechanism.IsDeploymentAuthenticationConfigured(ac, opts) {
403+
if mechanism.IsDeploymentAuthenticationEnabled(ac) {
404404
deploymentMechanismsToDisable = append(deploymentMechanismsToDisable, mechanism)
405405
}
406406
}

controllers/operator/authentication/authentication_mechanism.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type Mechanism interface {
2121
// called directly after deserializing the response from OM which should not contain the util.MergoDelete value in any field.
2222
IsAgentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool
2323
IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool
24+
IsDeploymentAuthenticationEnabled(ac *om.AutomationConfig) bool
2425
GetName() MechanismName
2526
}
2627

controllers/operator/authentication/ldap.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,11 @@ func (l *ldapAuthMechanism) IsAgentAuthenticationConfigured(ac *om.AutomationCon
116116
}
117117

118118
func (l *ldapAuthMechanism) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool {
119-
return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) && ldapObjectsEqual(ac.Ldap, opts.Ldap)
119+
return l.IsDeploymentAuthenticationEnabled(ac) && ldapObjectsEqual(ac.Ldap, opts.Ldap)
120+
}
121+
122+
func (l *ldapAuthMechanism) IsDeploymentAuthenticationEnabled(ac *om.AutomationConfig) bool {
123+
return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain))
120124
}
121125

122126
func ldapObjectsEqual(lhs *ldap.Ldap, rhs *ldap.Ldap) bool {

controllers/operator/authentication/scramsha.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ func (s *automationConfigScramSha) IsAgentAuthenticationConfigured(ac *om.Automa
8383
}
8484

8585
func (s *automationConfigScramSha) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool {
86+
return s.IsDeploymentAuthenticationEnabled(ac)
87+
}
88+
89+
func (s *automationConfigScramSha) IsDeploymentAuthenticationEnabled(ac *om.AutomationConfig) bool {
8690
return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName))
8791
}
8892

controllers/operator/authentication/x509.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ func (x *connectionX509) IsAgentAuthenticationConfigured(ac *om.AutomationConfig
131131
}
132132

133133
func (x *connectionX509) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool {
134+
return x.IsDeploymentAuthenticationEnabled(ac)
135+
}
136+
137+
func (x *connectionX509) IsDeploymentAuthenticationEnabled(ac *om.AutomationConfig) bool {
134138
return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509))
135139
}
136140

docker/mongodb-kubernetes-tests/tests/authentication/replica_set_ldap_tls.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import time
21
from typing import Dict
32

43
from kubetester import create_or_update_secret, find_fixture, wait_until
@@ -145,4 +144,14 @@ def test_remove_ldap_settings(replica_set: MongoDB):
145144
replica_set["spec"]["security"]["authentication"]["modes"] = ["SCRAM"]
146145
replica_set.update()
147146

147+
def wait_for_ac_pushed() -> bool:
148+
ac = replica_set.get_automation_config_tester()
149+
try:
150+
ac.assert_authentication_mechanism_disabled(LDAP_AUTHENTICATION_MECHANISM, check_auth_mechanism=False)
151+
return True
152+
except AssertionError:
153+
return False
154+
155+
wait_until(wait_for_ac_pushed, timeout=400)
156+
148157
replica_set.assert_reaches_phase(Phase.Running, timeout=400)

0 commit comments

Comments
 (0)