Skip to content

Commit 802c41e

Browse files
filipcirtogfilipcirtog
committed
refactoring
1 parent 6d572dc commit 802c41e

File tree

16 files changed

+69
-76
lines changed

16 files changed

+69
-76
lines changed

‎controllers/om/automation_config.go‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,12 @@ import (
1414
"github.com/mongodb/mongodb-kubernetes/controllers/operator/ldap"
1515
"github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc"
1616
"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/kube/secret"
17+
"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/util/constants"
1718
"github.com/mongodb/mongodb-kubernetes/pkg/util"
1819
"github.com/mongodb/mongodb-kubernetes/pkg/util/generate"
1920
"github.com/mongodb/mongodb-kubernetes/pkg/util/maputil"
2021
)
2122

22-
// The constants for the authentication secret
23-
const (
24-
autoPwdSecretKey = "automation-agent-password"
25-
)
26-
2723
// AutomationConfig maintains the raw map in the Deployment field
2824
// and constructs structs to make use of go's type safety
2925
// Dev notes: actually, this object is just a wrapper for the `Deployment` object which is received from Ops Manager,
@@ -455,18 +451,13 @@ func (ac *AutomationConfig) EnsurePassword(ctx context.Context, k8sClient secret
455451

456452
data, err := secret.ReadStringData(ctx, k8sClient, secretNamespacedName)
457453
if err == nil {
458-
if val, ok := data[autoPwdSecretKey]; ok && len(val) > 0 {
454+
if val, ok := data[constants.AutomationAgentAuthSecretKey]; ok && len(val) > 0 {
459455
password = val
460456
}
461457
} else if secret.SecretNotExist(err) {
462458
if ac.Auth.AutoPwd != "" && ac.Auth.AutoPwd != util.InvalidAutomationAgentPassword {
463459
password = ac.Auth.AutoPwd
464460
}
465-
466-
err := EnsureEmptySecret(ctx, k8sClient, secretNamespacedName)
467-
if err != nil {
468-
return "", err
469-
}
470461
}
471462

472463
if password == "" {
@@ -477,31 +468,21 @@ func (ac *AutomationConfig) EnsurePassword(ctx context.Context, k8sClient secret
477468
password = generatedPassword
478469
}
479470

480-
ac.Auth.AutoPwd = password
481-
err = secret.UpdateField(ctx, k8sClient, secretNamespacedName, autoPwdSecretKey, password)
482-
if err != nil {
483-
return "", fmt.Errorf("failed to update password field in shared secret %s/%s: %w", secretNamespacedName.Namespace, secretNamespacedName.Name, err)
484-
}
485-
486-
return password, nil
487-
}
488-
489-
func EnsureEmptySecret(ctx context.Context, k8sClient secret.GetUpdateCreator, secretNamespacedName types.NamespacedName) error {
490471
dataFields := map[string]string{
491-
autoPwdSecretKey: "",
472+
constants.AutomationAgentAuthSecretKey: password,
492473
}
493474

494-
emptySecret := secret.Builder().
475+
passwordSecret := secret.Builder().
495476
SetName(secretNamespacedName.Name).
496477
SetNamespace(secretNamespacedName.Namespace).
497478
SetStringMapToData(dataFields).
498479
Build()
499480

500-
if err := secret.CreateOrUpdateIfNeeded(ctx, k8sClient, emptySecret); err != nil {
501-
return fmt.Errorf("failed to create or update empty secret %s/%s: %w", secretNamespacedName.Namespace, secretNamespacedName.Name, err)
481+
if err := secret.CreateOrUpdateIfNeeded(ctx, k8sClient, passwordSecret); err != nil {
482+
return "", fmt.Errorf("failed to update password field in shared secret %s/%s: %w", secretNamespacedName.Namespace, secretNamespacedName.Name, err)
502483
}
503-
504-
return nil
484+
ac.Auth.AutoPwd = password
485+
return password, nil
505486
}
506487

507488
func (ac *AutomationConfig) CanEnableX509ProjectAuthentication() (bool, string) {

‎controllers/operator/appdbreplicaset_controller.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1666,8 +1666,9 @@ func (r *ReconcileAppDbReplicaSet) tryConfigureMonitoringInOpsManager(ctx contex
16661666
AutoUser: util.AutomationAgentUserName,
16671667
AutoPEMKeyFilePath: agentCertPath,
16681668
CAFilePath: util.CAFilePathInContainer,
1669+
MongoDBResource: types.NamespacedName{Namespace: opsManager.Namespace, Name: opsManager.Name},
16691670
}
1670-
err = authentication.Configure(ctx, r.client, types.NamespacedName{Namespace: opsManager.Namespace, Name: opsManager.Name}, conn, opts, false, log)
1671+
err = authentication.Configure(ctx, r.client, conn, opts, false, log)
16711672
if err != nil {
16721673
log.Errorf("Could not set Automation Authentication options in Ops/Cloud Manager for the Application Database. "+
16731674
"Application Database is always configured with authentication enabled, but this will not be "+

‎controllers/operator/authentication/authentication.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ type Options struct {
6767
AutoPwd string
6868

6969
AutoLdapGroupDN string
70+
71+
MongoDBResource types.NamespacedName
7072
}
7173

7274
func Redact(o Options) Options {
@@ -86,7 +88,7 @@ type UserOptions struct {
8688

8789
// Configure will configure all the specified authentication Mechanisms. We need to ensure we wait for
8890
// the agents to reach ready state after each operation as prematurely updating the automation config can cause the agents to get stuck.
89-
func Configure(ctx context.Context, client kubernetesClient.Client, mdbNamespacedName types.NamespacedName, conn om.Connection, opts Options, isRecovering bool, log *zap.SugaredLogger) error {
91+
func Configure(ctx context.Context, client kubernetesClient.Client, conn om.Connection, opts Options, isRecovering bool, log *zap.SugaredLogger) error {
9092
log.Infow("ensuring correct deployment mechanisms", "ProcessNames", opts.ProcessNames, "Mechanisms", opts.Mechanisms)
9193

9294
// In case we're recovering, we can push all changes at once, because the mechanism is triggered after 20min by default.
@@ -117,7 +119,7 @@ func Configure(ctx context.Context, client kubernetesClient.Client, mdbNamespace
117119

118120
// once we have made sure that the deployment authentication mechanism array contains the desired auth mechanism
119121
// we can then configure the agent authentication.
120-
if err := enableAgentAuthentication(ctx, client, mdbNamespacedName, conn, opts, log); err != nil {
122+
if err := enableAgentAuthentication(ctx, client, conn, opts, log); err != nil {
121123
return xerrors.Errorf("error enabling agent authentication: %w", err)
122124
}
123125
if err := waitForReadyStateIfNeeded(); err != nil {
@@ -155,7 +157,7 @@ func Configure(ctx context.Context, client kubernetesClient.Client, mdbNamespace
155157

156158
// Disable disables all authentication mechanisms, and waits for the agents to reach goal state. It is still required to provide
157159
// automation agent username, password and keyfile contents to ensure a valid Automation Config.
158-
func Disable(ctx context.Context, client kubernetesClient.Client, mdbNamespacedName types.NamespacedName, conn om.Connection, opts Options, deleteUsers bool, log *zap.SugaredLogger) error {
160+
func Disable(ctx context.Context, client kubernetesClient.Client, conn om.Connection, opts Options, deleteUsers bool, log *zap.SugaredLogger) error {
159161
ac, err := conn.ReadAutomationConfig()
160162
if err != nil {
161163
return xerrors.Errorf("error reading automation config: %w", err)
@@ -185,7 +187,7 @@ func Disable(ctx context.Context, client kubernetesClient.Client, mdbNamespacedN
185187
if err := ac.EnsureKeyFileContents(); err != nil {
186188
return xerrors.Errorf("error ensuring keyfile contents: %w", err)
187189
}
188-
if _, err := ac.EnsurePassword(ctx, client, mdbNamespacedName); err != nil {
190+
if _, err := ac.EnsurePassword(ctx, client, opts.MongoDBResource); err != nil {
189191
return xerrors.Errorf("error ensuring agent password: %w", err)
190192
}
191193

@@ -262,7 +264,7 @@ func removeUnsupportedAgentMechanisms(conn om.Connection, opts Options, log *zap
262264

263265
// enableAgentAuthentication determines which agent authentication mechanism should be configured
264266
// and enables it in Ops Manager
265-
func enableAgentAuthentication(ctx context.Context, client kubernetesClient.Client, mdbNamespacedName types.NamespacedName, conn om.Connection, opts Options, log *zap.SugaredLogger) error {
267+
func enableAgentAuthentication(ctx context.Context, client kubernetesClient.Client, conn om.Connection, opts Options, log *zap.SugaredLogger) error {
266268
ac, err := conn.ReadAutomationConfig()
267269
if err != nil {
268270
return xerrors.Errorf("error reading automation config: %w", err)
@@ -271,7 +273,7 @@ func enableAgentAuthentication(ctx context.Context, client kubernetesClient.Clie
271273
// we then configure the agent authentication for that type
272274
mechanism := convertToMechanismOrPanic(opts.AgentMechanism, ac)
273275

274-
if err := ensureAgentAuthenticationIsConfigured(ctx, client, mdbNamespacedName, conn, opts, ac, mechanism, log); err != nil {
276+
if err := ensureAgentAuthenticationIsConfigured(ctx, client, conn, opts, ac, mechanism, log); err != nil {
275277
return xerrors.Errorf("error ensuring agent authentication is configured: %w", err)
276278
}
277279

@@ -369,14 +371,14 @@ func addOrRemoveAgentClientCertificate(conn om.Connection, opts Options, log *za
369371
}
370372

371373
// ensureAgentAuthenticationIsConfigured will configure the agent authentication settings based on the desiredAgentAuthMechanism
372-
func ensureAgentAuthenticationIsConfigured(ctx context.Context, client kubernetesClient.Client, mdbNamespacedName types.NamespacedName, conn om.Connection, opts Options, ac *om.AutomationConfig, mechanism Mechanism, log *zap.SugaredLogger) error {
374+
func ensureAgentAuthenticationIsConfigured(ctx context.Context, client kubernetesClient.Client, conn om.Connection, opts Options, ac *om.AutomationConfig, mechanism Mechanism, log *zap.SugaredLogger) error {
373375
if mechanism.IsAgentAuthenticationConfigured(ac, opts) {
374376
log.Infof("Agent authentication mechanism %s is already configured", mechanism.GetName())
375377
return nil
376378
}
377379

378380
log.Infof("Enabling %s agent authentication", mechanism.GetName())
379-
return mechanism.EnableAgentAuthentication(ctx, client, mdbNamespacedName, conn, opts, log)
381+
return mechanism.EnableAgentAuthentication(ctx, client, conn, opts, log)
380382
}
381383

382384
// ensureDeploymentMechanisms configures the given AutomationConfig to allow deployments to

‎controllers/operator/authentication/authentication_mechanism.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
"go.uber.org/zap"
99
"golang.org/x/xerrors"
10-
"k8s.io/apimachinery/pkg/types"
1110

1211
"github.com/mongodb/mongodb-kubernetes/controllers/om"
1312
kubernetesClient "github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/kube/client"
@@ -16,7 +15,7 @@ import (
1615

1716
// Mechanism is an interface that needs to be implemented for any Ops Manager authentication mechanism
1817
type Mechanism interface {
19-
EnableAgentAuthentication(ctx context.Context, client kubernetesClient.Client, mdbNamespacedName types.NamespacedName, conn om.Connection, opts Options, log *zap.SugaredLogger) error
18+
EnableAgentAuthentication(ctx context.Context, client kubernetesClient.Client, conn om.Connection, opts Options, log *zap.SugaredLogger) error
2019
DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error
2120
EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error
2221
DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error

‎controllers/operator/authentication/configure_authentication_test.go‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func init() {
2222
func TestConfigureScramSha256(t *testing.T) {
2323
ctx := context.Background()
2424
kubeClient, _ := mock.NewDefaultFakeClient()
25-
mdbNamespacedName := types.NamespacedName{Namespace: "test", Name: "test"}
25+
mongoDBResource := types.NamespacedName{Namespace: "test", Name: "test"}
2626

2727
dep := om.NewDeployment()
2828
conn := om.NewMockedOmConnection(dep)
@@ -32,9 +32,10 @@ func TestConfigureScramSha256(t *testing.T) {
3232
ProcessNames: []string{"process-1", "process-2", "process-3"},
3333
Mechanisms: []string{"SCRAM"},
3434
AgentMechanism: "SCRAM",
35+
MongoDBResource: mongoDBResource,
3536
}
3637

37-
if err := Configure(ctx, kubeClient, mdbNamespacedName, conn, opts, false, zap.S()); err != nil {
38+
if err := Configure(ctx, kubeClient, conn, opts, false, zap.S()); err != nil {
3839
t.Fatal(err)
3940
}
4041

@@ -50,7 +51,7 @@ func TestConfigureScramSha256(t *testing.T) {
5051
func TestConfigureX509(t *testing.T) {
5152
ctx := context.Background()
5253
kubeClient, _ := mock.NewDefaultFakeClient()
53-
mdbNamespacedName := types.NamespacedName{Namespace: "test", Name: "test"}
54+
mongoDBResource := types.NamespacedName{Namespace: "test", Name: "test"}
5455

5556
dep := om.NewDeployment()
5657
conn := om.NewMockedOmConnection(dep)
@@ -64,9 +65,10 @@ func TestConfigureX509(t *testing.T) {
6465
UserOptions: UserOptions{
6566
AutomationSubject: validSubject("automation"),
6667
},
68+
MongoDBResource: mongoDBResource,
6769
}
6870

69-
if err := Configure(ctx, kubeClient, mdbNamespacedName, conn, opts, false, zap.S()); err != nil {
71+
if err := Configure(ctx, kubeClient, conn, opts, false, zap.S()); err != nil {
7072
t.Fatal(err)
7173
}
7274

@@ -82,7 +84,7 @@ func TestConfigureX509(t *testing.T) {
8284
func TestConfigureScramSha1(t *testing.T) {
8385
ctx := context.Background()
8486
kubeClient, _ := mock.NewDefaultFakeClient()
85-
mdbNamespacedName := types.NamespacedName{Namespace: "test", Name: "test"}
87+
mongoDBResource := types.NamespacedName{Namespace: "test", Name: "test"}
8688

8789
dep := om.NewDeployment()
8890
conn := om.NewMockedOmConnection(dep)
@@ -92,9 +94,10 @@ func TestConfigureScramSha1(t *testing.T) {
9294
ProcessNames: []string{"process-1", "process-2", "process-3"},
9395
Mechanisms: []string{"SCRAM-SHA-1"},
9496
AgentMechanism: "SCRAM-SHA-1",
97+
MongoDBResource: mongoDBResource,
9598
}
9699

97-
if err := Configure(ctx, kubeClient, mdbNamespacedName, conn, opts, false, zap.S()); err != nil {
100+
if err := Configure(ctx, kubeClient, conn, opts, false, zap.S()); err != nil {
98101
t.Fatal(err)
99102
}
100103

@@ -108,7 +111,7 @@ func TestConfigureScramSha1(t *testing.T) {
108111
func TestConfigureMultipleAuthenticationMechanisms(t *testing.T) {
109112
ctx := context.Background()
110113
kubeClient, _ := mock.NewDefaultFakeClient()
111-
mdbNamespacedName := types.NamespacedName{Namespace: "test", Name: "test"}
114+
mongoDBResource := types.NamespacedName{Namespace: "test", Name: "test"}
112115

113116
dep := om.NewDeployment()
114117
conn := om.NewMockedOmConnection(dep)
@@ -121,9 +124,10 @@ func TestConfigureMultipleAuthenticationMechanisms(t *testing.T) {
121124
UserOptions: UserOptions{
122125
AutomationSubject: validSubject("automation"),
123126
},
127+
MongoDBResource: mongoDBResource,
124128
}
125129

126-
if err := Configure(ctx, kubeClient, mdbNamespacedName, conn, opts, false, zap.S()); err != nil {
130+
if err := Configure(ctx, kubeClient, conn, opts, false, zap.S()); err != nil {
127131
t.Fatal(err)
128132
}
129133

@@ -145,7 +149,7 @@ func TestConfigureMultipleAuthenticationMechanisms(t *testing.T) {
145149
func TestDisableAuthentication(t *testing.T) {
146150
ctx := context.Background()
147151
kubeClient, _ := mock.NewDefaultFakeClient()
148-
mdbNamespacedName := types.NamespacedName{Namespace: "test", Name: "test"}
152+
mongoDBResource := types.NamespacedName{Namespace: "test", Name: "test"}
149153

150154
dep := om.NewDeployment()
151155
conn := om.NewMockedOmConnection(dep)
@@ -156,7 +160,7 @@ func TestDisableAuthentication(t *testing.T) {
156160
return nil
157161
}, zap.S())
158162

159-
if err := Disable(ctx, kubeClient, mdbNamespacedName, conn, Options{}, true, zap.S()); err != nil {
163+
if err := Disable(ctx, kubeClient, conn, Options{MongoDBResource: mongoDBResource}, true, zap.S()); err != nil {
160164
t.Fatal(err)
161165
}
162166

@@ -237,9 +241,10 @@ func assertDeploymentMechanismsConfigured(t *testing.T, authMechanism Mechanism,
237241
func assertAgentAuthenticationDisabled(t *testing.T, authMechanism Mechanism, conn om.Connection, opts Options) {
238242
ctx := context.Background()
239243
kubeClient, _ := mock.NewDefaultFakeClient()
240-
mdbNamespacedName := types.NamespacedName{Namespace: "test", Name: "test"}
244+
mongoDBResource := types.NamespacedName{Namespace: "test", Name: "test"}
245+
opts.MongoDBResource = mongoDBResource
241246

242-
err := authMechanism.EnableAgentAuthentication(ctx, kubeClient, mdbNamespacedName, conn, opts, zap.S())
247+
err := authMechanism.EnableAgentAuthentication(ctx, kubeClient, conn, opts, zap.S())
243248
require.NoError(t, err)
244249

245250
ac, err := conn.ReadAutomationConfig()

‎controllers/operator/authentication/ldap.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55

66
"go.uber.org/zap"
7-
"k8s.io/apimachinery/pkg/types"
87

98
"github.com/mongodb/mongodb-kubernetes/controllers/om"
109
"github.com/mongodb/mongodb-kubernetes/controllers/operator/ldap"
@@ -19,7 +18,7 @@ func (l *ldapAuthMechanism) GetName() MechanismName {
1918
return LDAPPlain
2019
}
2120

22-
func (l *ldapAuthMechanism) EnableAgentAuthentication(ctx context.Context, client kubernetesClient.Client, mdbNamespacedName types.NamespacedName, conn om.Connection, opts Options, log *zap.SugaredLogger) error {
21+
func (l *ldapAuthMechanism) EnableAgentAuthentication(ctx context.Context, client kubernetesClient.Client, conn om.Connection, opts Options, log *zap.SugaredLogger) error {
2322
log.Info("Configuring LDAP authentication")
2423
err := conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error {
2524
if err := ac.EnsureKeyFileContents(); err != nil {

‎controllers/operator/authentication/ldap_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func TestLdapDeploymentMechanism(t *testing.T) {
5050
func TestLdapEnableAgentAuthentication(t *testing.T) {
5151
ctx := context.Background()
5252
kubeClient, _ := mock.NewDefaultFakeClient()
53-
mdbNamespacedName := types.NamespacedName{Namespace: "test", Name: "test"}
53+
mongoDBResource := types.NamespacedName{Namespace: "test", Name: "test"}
5454

5555
conn := om.NewMockedOmConnection(om.NewDeployment())
5656
opts := Options{
@@ -60,9 +60,10 @@ func TestLdapEnableAgentAuthentication(t *testing.T) {
6060
},
6161
AuthoritativeSet: true,
6262
AutoPwd: "LDAPPassword.",
63+
MongoDBResource: mongoDBResource,
6364
}
6465

65-
err := ldapPlainMechanism.EnableAgentAuthentication(ctx, kubeClient, mdbNamespacedName, conn, opts, zap.S())
66+
err := ldapPlainMechanism.EnableAgentAuthentication(ctx, kubeClient, conn, opts, zap.S())
6667
require.NoError(t, err)
6768

6869
ac, err := conn.ReadAutomationConfig()

‎controllers/operator/authentication/oidc.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88

99
"go.uber.org/zap"
1010
"golang.org/x/xerrors"
11-
"k8s.io/apimachinery/pkg/types"
1211

1312
mdbv1 "github.com/mongodb/mongodb-kubernetes/api/v1/mdb"
1413
"github.com/mongodb/mongodb-kubernetes/controllers/om"
@@ -23,7 +22,7 @@ func (o *oidcAuthMechanism) GetName() MechanismName {
2322
return MongoDBOIDC
2423
}
2524

26-
func (o *oidcAuthMechanism) EnableAgentAuthentication(_ context.Context, _ kubernetesClient.Client, _ types.NamespacedName, _ om.Connection, _ Options, _ *zap.SugaredLogger) error {
25+
func (o *oidcAuthMechanism) EnableAgentAuthentication(_ context.Context, _ kubernetesClient.Client, _ om.Connection, _ Options, _ *zap.SugaredLogger) error {
2726
return xerrors.Errorf("OIDC agent authentication is not supported")
2827
}
2928

‎controllers/operator/authentication/oidc_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,12 @@ func TestOIDC_EnableDeploymentAuthentication(t *testing.T) {
8282
func TestOIDC_EnableAgentAuthentication(t *testing.T) {
8383
ctx := context.Background()
8484
kubeClient, _ := mock.NewDefaultFakeClient()
85-
mdbNamespacedName := types.NamespacedName{Namespace: "test", Name: "test"}
85+
mongoDBResource := types.NamespacedName{Namespace: "test", Name: "test"}
8686

8787
conn := om.NewMockedOmConnection(om.NewDeployment())
8888
opts := Options{
89-
Mechanisms: []string{string(MongoDBOIDC)},
89+
Mechanisms: []string{string(MongoDBOIDC)},
90+
MongoDBResource: mongoDBResource,
9091
}
9192

9293
ac, err := conn.ReadAutomationConfig()
@@ -95,7 +96,7 @@ func TestOIDC_EnableAgentAuthentication(t *testing.T) {
9596
configured := mongoDBOIDCMechanism.IsAgentAuthenticationConfigured(ac, opts)
9697
assert.False(t, configured)
9798

98-
err = mongoDBOIDCMechanism.EnableAgentAuthentication(ctx, kubeClient, mdbNamespacedName, conn, opts, zap.S())
99+
err = mongoDBOIDCMechanism.EnableAgentAuthentication(ctx, kubeClient, conn, opts, zap.S())
99100
require.Error(t, err)
100101

101102
err = mongoDBOIDCMechanism.DisableAgentAuthentication(conn, zap.S())

0 commit comments

Comments
 (0)