Skip to content

Commit c0949d7

Browse files
nammnfealebenpae
authored andcommitted
CLOUDP-303965 - remove legacy min version check (#4145)
# Summary - We did not account for `AlwaysMatchVersion` minVersionDetection for setting auth - Instead of fixing this, we can fully remove the code, as we don't support <=MDB5 anymore: https://www.mongodb.com/legal/support-policy/lifecycles ## Proof of Work - passing ci suite ## 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? - [x] Have you checked for release_note changes?
1 parent cb1452f commit c0949d7

File tree

6 files changed

+39
-173
lines changed

6 files changed

+39
-173
lines changed

RELEASE_NOTES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
* Fixes the bug when status of `MongoDBUser` was being set to `Updated` prematurely. For example, new users were not immediately usable following `MongoDBUser` creation despite the operator reporting `Updated` state.
1515
* Fixed a bug causing cluster health check issues when ordering of users and tokens differed in Kubeconfig.
1616
* Fixed a bug when deploying a Multi-Cluster sharded resource with an external access configuration could result in pods not being able to reach each others.
17+
* Fixed a bug when setting `spec.fcv = AlwaysMatchVersion` and `agentAuth` to be `SCRAM` causes the operator to set the auth value to be `SCRAM-SHA-1` instead of `SCRAM-SHA-256`.
1718

1819
# MongoDB Enterprise Kubernetes Operator 1.31.0
1920

api/v1/mdb/mongodb_types.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,6 @@ func (m *MongoDB) GetPrometheus() *mdbcv1.Prometheus {
140140
return m.Spec.Prometheus
141141
}
142142

143-
func (m *MongoDB) GetMinimumMajorVersion() uint64 {
144-
return m.Spec.MinimumMajorVersion()
145-
}
146-
147143
func (m *MongoDB) GetBackupSpec() *Backup {
148144
return m.Spec.Backup
149145
}

api/v1/mdbmulti/mongodb_multi_types.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,6 @@ func (m *MongoDBMultiCluster) GetPrometheus() *mdbc.Prometheus {
118118
return m.Spec.Prometheus
119119
}
120120

121-
func (m *MongoDBMultiCluster) GetMinimumMajorVersion() uint64 {
122-
return m.Spec.MinimumMajorVersion()
123-
}
124-
125121
func (m *MongoDBMultiCluster) IsLDAPEnabled() bool {
126122
if m.Spec.Security == nil || m.Spec.Security.Authentication == nil {
127123
return false

controllers/operator/authentication/authentication.go

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,11 @@ type AuthResource interface {
2121
GetSecurity() *mdbv1.Security
2222
IsLDAPEnabled() bool
2323
GetLDAP(password, caContents string) *ldap.Ldap
24-
GetMinimumMajorVersion() uint64
2524
}
2625

2726
// Options contains all the required values that are required to configure authentication
2827
// for a set of processes
2928
type Options struct {
30-
// MinimumMajorVersion is required in order to determine if we will be enabling SCRAM-SHA-1 or SCRAM-SHA-256
31-
MinimumMajorVersion uint64
3229
// Mechanisms is a list of strings coming from MongoDB.Spec.Security.Authentication.Modes, these strings
3330
// are mapped to the corresponding mechanisms in the Automation Config
3431
Mechanisms []string
@@ -84,7 +81,7 @@ type UserOptions struct {
8481
// Configure will configure all the specified authentication Mechanisms. We need to ensure we wait for
8582
// the agents to reach ready state after each operation as prematurely updating the automation config can cause the agents to get stuck.
8683
func Configure(conn om.Connection, opts Options, isRecovering bool, log *zap.SugaredLogger) error {
87-
log.Infow("ensuring correct deployment mechanisms", "MinimumMajorVersion", opts.MinimumMajorVersion, "ProcessNames", opts.ProcessNames, "Mechanisms", opts.Mechanisms)
84+
log.Infow("ensuring correct deployment mechanisms", "ProcessNames", opts.ProcessNames, "Mechanisms", opts.Mechanisms)
8885

8986
// In case we're recovering, we can push all changes at once, because the mechanism is triggered after 20min by default.
9087
// Otherwise, we might unnecessarily enter this waiting loop 7 times, and waste >10 min
@@ -257,7 +254,7 @@ func Disable(conn om.Connection, opts Options, deleteUsers bool, log *zap.Sugare
257254
return nil
258255
}
259256

260-
func getMechanismName(mongodbResourceMode string, ac *om.AutomationConfig, minimumMajorVersion uint64) MechanismName {
257+
func getMechanismName(mongodbResourceMode string, ac *om.AutomationConfig) MechanismName {
261258
switch mongodbResourceMode {
262259
case util.X509:
263260
return MongoDBX509
@@ -278,12 +275,7 @@ func getMechanismName(mongodbResourceMode string, ac *om.AutomationConfig, minim
278275
if ac.Auth.AutoAuthMechanism == string(MongoDBCR) && ac.Auth.IsEnabled() {
279276
return MongoDBCR
280277
}
281-
282-
if minimumMajorVersion < 4 {
283-
return MongoDBCR
284-
} else {
285-
return ScramSha256
286-
}
278+
return ScramSha256
287279
}
288280
// this should never be reached as validation of this string happens at the CR level
289281
panic(fmt.Sprintf("unknown mechanism name %s", mongodbResourceMode))
@@ -316,7 +308,7 @@ func removeUnusedAuthenticationMechanisms(conn om.Connection, opts Options, log
316308
return xerrors.Errorf("error reading automation config: %w", err)
317309
}
318310

319-
automationConfigAuthMechanismNames := getMechanismNames(ac, opts.MinimumMajorVersion, opts.Mechanisms)
311+
automationConfigAuthMechanismNames := getMechanismNames(ac, opts.Mechanisms)
320312

321313
unrequiredMechanisms := mechanismsToDisable(automationConfigAuthMechanismNames)
322314

@@ -344,7 +336,7 @@ func enableAgentAuthentication(conn om.Connection, opts Options, log *zap.Sugare
344336
}
345337

346338
// we then configure the agent authentication for that type
347-
agentAuthMechanism := getMechanismName(opts.AgentMechanism, ac, opts.MinimumMajorVersion)
339+
agentAuthMechanism := getMechanismName(opts.AgentMechanism, ac)
348340
if err := ensureAgentAuthenticationIsConfigured(conn, opts, ac, agentAuthMechanism, log); err != nil {
349341
return xerrors.Errorf("error ensuring agent authentication is configured: %w", err)
350342
}
@@ -381,7 +373,7 @@ func ensureDeploymentsMechanismsExist(conn om.Connection, opts Options, log *zap
381373

382374
// "opts.Mechanisms" is the list of mechanism names passed through from the MongoDB resource.
383375
// We need to convert this to the list of strings the automation config expects.
384-
automationConfigMechanismNames := getMechanismNames(ac, opts.MinimumMajorVersion, opts.Mechanisms)
376+
automationConfigMechanismNames := getMechanismNames(ac, opts.Mechanisms)
385377

386378
log.Debugf("Automation config authentication mechanisms: %+v", automationConfigMechanismNames)
387379
if err := ensureDeploymentMechanisms(conn, ac, automationConfigMechanismNames, opts, log); err != nil {
@@ -401,7 +393,7 @@ func removeUnrequiredDeploymentMechanisms(conn om.Connection, opts Options, log
401393

402394
// "opts.Mechanisms" is the list of mechanism names passed through from the MongoDB resource.
403395
// We need to convert this to the list of strings the automation config expects.
404-
automationConfigAuthMechanismNames := getMechanismNames(ac, opts.MinimumMajorVersion, opts.Mechanisms)
396+
automationConfigAuthMechanismNames := getMechanismNames(ac, opts.Mechanisms)
405397

406398
toDisable := mechanismsToDisable(automationConfigAuthMechanismNames)
407399
log.Infow("Removing unrequired deployment authentication mechanisms", "Mechanisms", toDisable)
@@ -420,7 +412,7 @@ func addOrRemoveAgentClientCertificate(conn om.Connection, opts Options, log *za
420412
// If x509 is not enabled but still Client Certificates are, this automation config update
421413
// will add the required configuration.
422414
return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error {
423-
if getMechanismName(opts.AgentMechanism, ac, opts.MinimumMajorVersion) == MongoDBX509 {
415+
if getMechanismName(opts.AgentMechanism, ac) == MongoDBX509 {
424416
// If TLS client authentication is managed by x509, we won't disable or enable it
425417
// in here.
426418
return nil
@@ -442,10 +434,10 @@ func addOrRemoveAgentClientCertificate(conn om.Connection, opts Options, log *za
442434
}, log)
443435
}
444436

445-
func getMechanismNames(ac *om.AutomationConfig, minimumMajorVersion uint64, mechanisms []string) []MechanismName {
437+
func getMechanismNames(ac *om.AutomationConfig, mechanisms []string) []MechanismName {
446438
automationConfigMechanismNames := make([]MechanismName, 0)
447439
for _, m := range mechanisms {
448-
automationConfigMechanismNames = append(automationConfigMechanismNames, getMechanismName(m, ac, minimumMajorVersion))
440+
automationConfigMechanismNames = append(automationConfigMechanismNames, getMechanismName(m, ac))
449441
}
450442
return automationConfigMechanismNames
451443
}

controllers/operator/authentication/configure_authentication_test.go

Lines changed: 20 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -15,41 +15,15 @@ func init() {
1515
zap.ReplaceGlobals(logger)
1616
}
1717

18-
func TestConfigureScramSha1FallbackToCr(t *testing.T) {
19-
dep := om.NewDeployment()
20-
conn := om.NewMockedOmConnection(dep)
21-
22-
opts := Options{
23-
MinimumMajorVersion: 3,
24-
AuthoritativeSet: true,
25-
ProcessNames: []string{"process-1", "process-2", "process-3"},
26-
Mechanisms: []string{"SCRAM"},
27-
AgentMechanism: "SCRAM",
28-
}
29-
30-
if err := Configure(conn, opts, false, zap.S()); err != nil {
31-
t.Fatal(err)
32-
}
33-
34-
ac, err := conn.ReadAutomationConfig()
35-
if err != nil {
36-
t.Fatal(err)
37-
}
38-
39-
assertAuthenticationEnabled(t, ac.Auth)
40-
assertAuthenticationMechanism(t, ac.Auth, "MONGODB-CR")
41-
}
42-
4318
func TestConfigureScramSha256(t *testing.T) {
4419
dep := om.NewDeployment()
4520
conn := om.NewMockedOmConnection(dep)
4621

4722
opts := Options{
48-
MinimumMajorVersion: 4,
49-
AuthoritativeSet: true,
50-
ProcessNames: []string{"process-1", "process-2", "process-3"},
51-
Mechanisms: []string{"SCRAM"},
52-
AgentMechanism: "SCRAM",
23+
AuthoritativeSet: true,
24+
ProcessNames: []string{"process-1", "process-2", "process-3"},
25+
Mechanisms: []string{"SCRAM"},
26+
AgentMechanism: "SCRAM",
5327
}
5428

5529
if err := Configure(conn, opts, false, zap.S()); err != nil {
@@ -70,12 +44,11 @@ func TestConfigureX509(t *testing.T) {
7044
conn := om.NewMockedOmConnection(dep)
7145

7246
opts := Options{
73-
MinimumMajorVersion: 4,
74-
AuthoritativeSet: true,
75-
ProcessNames: []string{"process-1", "process-2", "process-3"},
76-
Mechanisms: []string{"X509"},
77-
AgentMechanism: "X509",
78-
ClientCertificates: util.RequireClientCertificates,
47+
AuthoritativeSet: true,
48+
ProcessNames: []string{"process-1", "process-2", "process-3"},
49+
Mechanisms: []string{"X509"},
50+
AgentMechanism: "X509",
51+
ClientCertificates: util.RequireClientCertificates,
7952
UserOptions: UserOptions{
8053
AutomationSubject: validSubject("automation"),
8154
},
@@ -99,11 +72,10 @@ func TestConfigureScramSha1(t *testing.T) {
9972
conn := om.NewMockedOmConnection(dep)
10073

10174
opts := Options{
102-
MinimumMajorVersion: 4,
103-
AuthoritativeSet: true,
104-
ProcessNames: []string{"process-1", "process-2", "process-3"},
105-
Mechanisms: []string{"SCRAM-SHA-1"},
106-
AgentMechanism: "SCRAM-SHA-1",
75+
AuthoritativeSet: true,
76+
ProcessNames: []string{"process-1", "process-2", "process-3"},
77+
Mechanisms: []string{"SCRAM-SHA-1"},
78+
AgentMechanism: "SCRAM-SHA-1",
10779
}
10880

10981
if err := Configure(conn, opts, false, zap.S()); err != nil {
@@ -122,11 +94,10 @@ func TestConfigureMultipleAuthenticationMechanisms(t *testing.T) {
12294
conn := om.NewMockedOmConnection(dep)
12395

12496
opts := Options{
125-
MinimumMajorVersion: 4,
126-
AuthoritativeSet: true,
127-
ProcessNames: []string{"process-1", "process-2", "process-3"},
128-
Mechanisms: []string{"X509", "SCRAM"},
129-
AgentMechanism: "SCRAM",
97+
AuthoritativeSet: true,
98+
ProcessNames: []string{"process-1", "process-2", "process-3"},
99+
Mechanisms: []string{"X509", "SCRAM"},
100+
AgentMechanism: "SCRAM",
130101
UserOptions: UserOptions{
131102
AutomationSubject: validSubject("automation"),
132103
},
@@ -151,90 +122,6 @@ func TestConfigureMultipleAuthenticationMechanisms(t *testing.T) {
151122
assert.Contains(t, ac.Auth.DeploymentAuthMechanisms, "MONGODB-X509")
152123
}
153124

154-
func TestScramSha1MongoDBUpgrade(t *testing.T) {
155-
dep := om.NewDeployment()
156-
conn := om.NewMockedOmConnection(dep)
157-
158-
opts := Options{
159-
MinimumMajorVersion: 3,
160-
AuthoritativeSet: true,
161-
ProcessNames: []string{"process-1", "process-2", "process-3"},
162-
Mechanisms: []string{"SCRAM"},
163-
AgentMechanism: "SCRAM",
164-
}
165-
166-
if err := Configure(conn, opts, false, zap.S()); err != nil {
167-
t.Fatal(err)
168-
}
169-
170-
ac, err := conn.ReadAutomationConfig()
171-
if err != nil {
172-
t.Fatal(err)
173-
}
174-
175-
assertAuthenticationEnabled(t, ac.Auth)
176-
assertAuthenticationMechanism(t, ac.Auth, "MONGODB-CR")
177-
178-
opts = Options{
179-
MinimumMajorVersion: 4,
180-
AuthoritativeSet: true,
181-
ProcessNames: []string{"process-1", "process-2", "process-3"},
182-
Mechanisms: []string{"SCRAM"},
183-
AgentMechanism: "SCRAM",
184-
}
185-
186-
if err := Configure(conn, opts, false, zap.S()); err != nil {
187-
t.Fatal(err)
188-
}
189-
190-
ac, err = conn.ReadAutomationConfig()
191-
if err != nil {
192-
t.Fatal(err)
193-
}
194-
195-
assertAuthenticationEnabled(t, ac.Auth)
196-
assertAuthenticationMechanism(t, ac.Auth, "MONGODB-CR")
197-
}
198-
199-
func TestConfigureAndDisable(t *testing.T) {
200-
dep := om.NewDeployment()
201-
conn := om.NewMockedOmConnection(dep)
202-
203-
opts := Options{
204-
MinimumMajorVersion: 3,
205-
AuthoritativeSet: true,
206-
ProcessNames: []string{"process-1", "process-2", "process-3"},
207-
Mechanisms: []string{"SCRAM"},
208-
AgentMechanism: "SCRAM",
209-
UserOptions: UserOptions{
210-
AutomationSubject: validSubject("automation"),
211-
},
212-
}
213-
214-
if err := Configure(conn, opts, false, zap.S()); err != nil {
215-
t.Fatal(err)
216-
}
217-
218-
ac, err := conn.ReadAutomationConfig()
219-
if err != nil {
220-
t.Fatal(err)
221-
}
222-
223-
assertAuthenticationEnabled(t, ac.Auth)
224-
assertAuthenticationMechanism(t, ac.Auth, "MONGODB-CR")
225-
226-
if err := Disable(conn, opts, true, zap.S()); err != nil {
227-
t.Fatal(err)
228-
}
229-
230-
ac, err = conn.ReadAutomationConfig()
231-
if err != nil {
232-
t.Fatal(err)
233-
}
234-
235-
assertAuthenticationDisabled(t, ac.Auth)
236-
}
237-
238125
func TestDisableAuthentication(t *testing.T) {
239126
dep := om.NewDeployment()
240127
conn := om.NewMockedOmConnection(dep)
@@ -261,17 +148,12 @@ func TestGetCorrectAuthMechanismFromVersion(t *testing.T) {
261148
conn := om.NewMockedOmConnection(om.NewDeployment())
262149
ac, _ := conn.ReadAutomationConfig()
263150

264-
mechanismNames := getMechanismNames(ac, 3, []string{"X509"})
151+
mechanismNames := getMechanismNames(ac, []string{"X509"})
265152

266153
assert.Len(t, mechanismNames, 1)
267154
assert.Contains(t, mechanismNames, MechanismName("MONGODB-X509"))
268155

269-
mechanismNames = getMechanismNames(ac, 3, []string{"SCRAM", "X509"})
270-
271-
assert.Contains(t, mechanismNames, MechanismName("MONGODB-CR"))
272-
assert.Contains(t, mechanismNames, MechanismName("MONGODB-X509"))
273-
274-
mechanismNames = getMechanismNames(ac, 4, []string{"SCRAM", "X509"})
156+
mechanismNames = getMechanismNames(ac, []string{"SCRAM", "X509"})
275157

276158
assert.Contains(t, mechanismNames, MechanismName("SCRAM-SHA-256"))
277159
assert.Contains(t, mechanismNames, MechanismName("MONGODB-X509"))
@@ -280,7 +162,7 @@ func TestGetCorrectAuthMechanismFromVersion(t *testing.T) {
280162
ac.Auth.AutoAuthMechanism = "MONGODB-CR"
281163
ac.Auth.Enable()
282164

283-
mechanismNames = getMechanismNames(ac, 4, []string{"SCRAM", "X509"})
165+
mechanismNames = getMechanismNames(ac, []string{"SCRAM", "X509"})
284166

285167
assert.Contains(t, mechanismNames, MechanismName("MONGODB-CR"))
286168
assert.Contains(t, mechanismNames, MechanismName("MONGODB-X509"))

controllers/operator/common_controller.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -503,15 +503,14 @@ func (r *ReconcileCommonController) updateOmAuthentication(ctx context.Context,
503503
}
504504

505505
authOpts := authentication.Options{
506-
MinimumMajorVersion: ar.GetMinimumMajorVersion(),
507-
Mechanisms: mdbv1.ConvertAuthModesToStrings(ar.GetSecurity().Authentication.Modes),
508-
ProcessNames: processNames,
509-
AuthoritativeSet: !ar.GetSecurity().Authentication.IgnoreUnknownUsers,
510-
AgentMechanism: ar.GetSecurity().GetAgentMechanism(ac.Auth.AutoAuthMechanism),
511-
ClientCertificates: clientCerts,
512-
AutoUser: scramAgentUserName,
513-
AutoLdapGroupDN: ar.GetSecurity().Authentication.Agents.AutomationLdapGroupDN,
514-
CAFilePath: caFilepath,
506+
Mechanisms: mdbv1.ConvertAuthModesToStrings(ar.GetSecurity().Authentication.Modes),
507+
ProcessNames: processNames,
508+
AuthoritativeSet: !ar.GetSecurity().Authentication.IgnoreUnknownUsers,
509+
AgentMechanism: ar.GetSecurity().GetAgentMechanism(ac.Auth.AutoAuthMechanism),
510+
ClientCertificates: clientCerts,
511+
AutoUser: scramAgentUserName,
512+
AutoLdapGroupDN: ar.GetSecurity().Authentication.Agents.AutomationLdapGroupDN,
513+
CAFilePath: caFilepath,
515514
}
516515
var databaseSecretPath string
517516
if r.VaultClient != nil {

0 commit comments

Comments
 (0)