Skip to content

Commit 04ee1d5

Browse files
gkechgkech
committed
K8SPSMDB-1610 handle user updates when no changes happen
1 parent ea61848 commit 04ee1d5

2 files changed

Lines changed: 177 additions & 48 deletions

File tree

‎pkg/controller/perconaservermongodbclustersync/psmdb_clustersync_controller_test.go‎

Lines changed: 106 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import (
66
"testing"
77
"time"
88

9-
"github.com/percona/percona-server-mongodb-operator/pkg/psmdb"
109
"github.com/percona/percona-server-mongodb-operator/pkg/psmdb/clustersync/client"
1110
"github.com/percona/percona-server-mongodb-operator/pkg/psmdb/mongo"
1211
"github.com/stretchr/testify/assert"
1312
"github.com/stretchr/testify/require"
13+
corev1 "k8s.io/api/core/v1"
1414
"k8s.io/apimachinery/pkg/api/meta"
1515
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1616
"k8s.io/apimachinery/pkg/types"
@@ -351,52 +351,139 @@ func (m *recordingMongoClient) UpdateUserRoles(_ context.Context, _, _ string, r
351351
}
352352

353353
func TestEnsureTargetMongoUser(t *testing.T) {
354-
creds := psmdb.Credentials{Username: "clustersync-cr", Password: "new-password"}
354+
const (
355+
username = "clustersync-cr"
356+
password = "new-password"
357+
)
358+
passwordHash := sha256Hash([]byte(password))
359+
360+
secretWith := func(annotations map[string]string) *corev1.Secret {
361+
return &corev1.Secret{
362+
ObjectMeta: metav1.ObjectMeta{Annotations: annotations},
363+
Data: map[string][]byte{
364+
targetUserSecretUsernameKey: []byte(username),
365+
targetUserSecretPasswordKey: []byte(password),
366+
},
367+
}
368+
}
369+
existingWithDesiredRoles := &mongo.User{DB: "admin", Roles: append([]mongo.Role(nil), syncTargetUserRoles...)}
355370

356371
tests := map[string]struct {
357-
mc *recordingMongoClient
358-
wantErr string
359-
assert func(t *testing.T, m *recordingMongoClient)
372+
mc *recordingMongoClient
373+
sec *corev1.Secret
374+
wantErr string
375+
wantMutated bool
376+
assert func(t *testing.T, m *recordingMongoClient, s *corev1.Secret)
360377
}{
361-
"missing user is created with secret password": {
362-
mc: &recordingMongoClient{getUserInfoResp: nil},
363-
assert: func(t *testing.T, m *recordingMongoClient) {
378+
"missing user is created and password hash annotation is set": {
379+
mc: &recordingMongoClient{getUserInfoResp: nil},
380+
sec: secretWith(nil),
381+
wantMutated: true,
382+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
364383
assert.Equal(t, 1, m.createUserCalls)
365-
assert.Equal(t, "new-password", m.lastCreatePass)
384+
assert.Equal(t, password, m.lastCreatePass)
366385
assert.Zero(t, m.updateUserPassCalls)
367386
assert.Zero(t, m.updateRolesCalls)
387+
assert.Equal(t, passwordHash, s.Annotations[targetUserPasswordHashAnnotation])
368388
},
369389
},
370-
"existing user has password and roles re-applied": {
371-
mc: &recordingMongoClient{getUserInfoResp: &mongo.User{DB: "admin"}},
372-
assert: func(t *testing.T, m *recordingMongoClient) {
390+
"steady state converged does no writes": {
391+
mc: &recordingMongoClient{getUserInfoResp: existingWithDesiredRoles},
392+
sec: secretWith(map[string]string{targetUserPasswordHashAnnotation: passwordHash}),
393+
wantMutated: false,
394+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
373395
assert.Zero(t, m.createUserCalls)
396+
assert.Zero(t, m.updateUserPassCalls)
397+
assert.Zero(t, m.updateRolesCalls)
398+
},
399+
},
400+
"password hash mismatch triggers UpdateUserPass and annotation refresh": {
401+
mc: &recordingMongoClient{getUserInfoResp: existingWithDesiredRoles},
402+
sec: secretWith(map[string]string{targetUserPasswordHashAnnotation: "stale"}),
403+
wantMutated: true,
404+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
374405
assert.Equal(t, 1, m.updateUserPassCalls)
375-
assert.Equal(t, "new-password", m.lastUpdatePass)
406+
assert.Equal(t, password, m.lastUpdatePass)
407+
assert.Zero(t, m.updateRolesCalls)
408+
assert.Equal(t, passwordHash, s.Annotations[targetUserPasswordHashAnnotation])
409+
},
410+
},
411+
"recreated secret with existing mongo user triggers UpdateUserPass and creates annotation map": {
412+
mc: &recordingMongoClient{getUserInfoResp: existingWithDesiredRoles},
413+
sec: secretWith(nil),
414+
wantMutated: true,
415+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
416+
assert.Equal(t, 1, m.updateUserPassCalls)
417+
assert.Equal(t, password, m.lastUpdatePass)
418+
assert.Zero(t, m.updateRolesCalls)
419+
require.NotNil(t, s.Annotations)
420+
assert.Equal(t, passwordHash, s.Annotations[targetUserPasswordHashAnnotation])
421+
},
422+
},
423+
"new cr reconcile against stale mongo user with drifted roles applies both password and roles": {
424+
mc: &recordingMongoClient{
425+
getUserInfoResp: &mongo.User{DB: "admin", Roles: []mongo.Role{{Role: "clusterMonitor", DB: "admin"}}},
426+
},
427+
sec: secretWith(nil),
428+
wantMutated: true,
429+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
430+
assert.Equal(t, 1, m.updateUserPassCalls)
431+
assert.Equal(t, password, m.lastUpdatePass)
432+
assert.Equal(t, 1, m.updateRolesCalls)
433+
assert.Equal(t, syncTargetUserRoles, m.lastUpdateRoles)
434+
assert.Equal(t, passwordHash, s.Annotations[targetUserPasswordHashAnnotation])
435+
},
436+
},
437+
"role drift triggers UpdateUserRoles without touching password": {
438+
mc: &recordingMongoClient{
439+
getUserInfoResp: &mongo.User{DB: "admin", Roles: []mongo.Role{{Role: "clusterMonitor", DB: "admin"}}},
440+
},
441+
sec: secretWith(map[string]string{targetUserPasswordHashAnnotation: passwordHash}),
442+
wantMutated: false,
443+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
444+
assert.Zero(t, m.updateUserPassCalls)
376445
assert.Equal(t, 1, m.updateRolesCalls)
377446
assert.Equal(t, syncTargetUserRoles, m.lastUpdateRoles)
378447
},
379448
},
449+
"roles in a different order are treated as equal": {
450+
mc: &recordingMongoClient{
451+
getUserInfoResp: &mongo.User{DB: "admin", Roles: []mongo.Role{
452+
{Role: "readWriteAnyDatabase", DB: "admin"},
453+
{Role: "clusterManager", DB: "admin"},
454+
{Role: "clusterMonitor", DB: "admin"},
455+
{Role: "restore", DB: "admin"},
456+
}},
457+
},
458+
sec: secretWith(map[string]string{targetUserPasswordHashAnnotation: passwordHash}),
459+
wantMutated: false,
460+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
461+
assert.Zero(t, m.updateRolesCalls)
462+
},
463+
},
380464
"GetUserInfo error short-circuits with no writes": {
381465
mc: &recordingMongoClient{getUserInfoErr: errors.New("error")},
466+
sec: secretWith(nil),
382467
wantErr: "look up target user",
383-
assert: func(t *testing.T, m *recordingMongoClient) {
468+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
384469
assert.Zero(t, m.createUserCalls)
385470
assert.Zero(t, m.updateUserPassCalls)
386471
assert.Zero(t, m.updateRolesCalls)
387472
},
388473
},
389474
"CreateUser error is wrapped": {
390475
mc: &recordingMongoClient{getUserInfoResp: nil, createUserErr: errors.New("error")},
476+
sec: secretWith(nil),
391477
wantErr: "create target user",
392478
},
393479
"UpdateUserPass error stops before UpdateUserRoles": {
394480
mc: &recordingMongoClient{
395481
getUserInfoResp: &mongo.User{DB: "admin"},
396482
updateUserPassErr: errors.New("error"),
397483
},
484+
sec: secretWith(nil),
398485
wantErr: "sync target user password",
399-
assert: func(t *testing.T, m *recordingMongoClient) {
486+
assert: func(t *testing.T, m *recordingMongoClient, s *corev1.Secret) {
400487
assert.Zero(t, m.updateRolesCalls)
401488
},
402489
},
@@ -405,21 +492,23 @@ func TestEnsureTargetMongoUser(t *testing.T) {
405492
getUserInfoResp: &mongo.User{DB: "admin"},
406493
updateRolesErr: errors.New("error"),
407494
},
495+
sec: secretWith(map[string]string{targetUserPasswordHashAnnotation: passwordHash}),
408496
wantErr: "sync target user roles",
409497
},
410498
}
411499

412500
for name, tc := range tests {
413501
t.Run(name, func(t *testing.T) {
414-
err := ensureTargetMongoUser(t.Context(), tc.mc, creds)
502+
mutated, err := ensureTargetMongoUser(t.Context(), tc.mc, tc.sec)
415503
if tc.wantErr != "" {
416504
require.Error(t, err)
417505
assert.Contains(t, err.Error(), tc.wantErr)
418506
} else {
419507
require.NoError(t, err)
508+
assert.Equal(t, tc.wantMutated, mutated)
420509
}
421510
if tc.assert != nil {
422-
tc.assert(t, tc.mc)
511+
tc.assert(t, tc.mc, tc.sec)
423512
}
424513
})
425514
}

‎pkg/controller/perconaservermongodbclustersync/target_user.go‎

Lines changed: 71 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ package perconaservermongodbclustersync
22

33
import (
44
"context"
5+
"crypto/sha256"
56
"fmt"
67

8+
"github.com/google/go-cmp/cmp"
9+
"github.com/google/go-cmp/cmp/cmpopts"
710
"github.com/pkg/errors"
811
corev1 "k8s.io/api/core/v1"
912
k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -23,6 +26,8 @@ import (
2326
const (
2427
targetUserSecretUsernameKey = "username"
2528
targetUserSecretPasswordKey = "password"
29+
30+
targetUserPasswordHashAnnotation = "psmdb.percona.com/target-user-password-hash"
2631
)
2732

2833
var syncTargetUserRoles = []mongo.Role{
@@ -32,6 +37,13 @@ var syncTargetUserRoles = []mongo.Role{
3237
{Role: "readWriteAnyDatabase", DB: "admin"},
3338
}
3439

40+
var sortRolesOpt = cmpopts.SortSlices(func(a, b mongo.Role) bool {
41+
if a.DB != b.DB {
42+
return a.DB < b.DB
43+
}
44+
return a.Role < b.Role
45+
})
46+
3547
// Scoped to the CR name so a new CR after a finalized one gets its own user.
3648
func syncTargetUsername(cr *psmdbv1.PerconaServerMongoDBClusterSync) string {
3749
return fmt.Sprintf("clustersync-%s", cr.Name)
@@ -45,7 +57,7 @@ func (r *ReconcilePerconaServerMongoDBClusterSync) ensureSyncTargetUser(
4557
target *psmdbv1.PerconaServerMongoDB,
4658
) (psmdb.Credentials, error) {
4759
log := logf.FromContext(ctx)
48-
creds, err := r.ensureTargetUserSecret(ctx, cr)
60+
sec, err := r.ensureTargetUserSecret(ctx, cr)
4961
if err != nil {
5062
return psmdb.Credentials{}, errors.Wrap(err, "ensure target user secret")
5163
}
@@ -61,35 +73,44 @@ func (r *ReconcilePerconaServerMongoDBClusterSync) ensureSyncTargetUser(
6173
}
6274
}(mc, ctx)
6375

64-
if err := ensureTargetMongoUser(ctx, mc, creds); err != nil {
76+
mutated, err := ensureTargetMongoUser(ctx, mc, sec)
77+
if err != nil {
6578
return psmdb.Credentials{}, errors.Wrap(err, "ensure target mongo user")
6679
}
67-
return creds, nil
80+
if mutated {
81+
if err := r.client.Update(ctx, sec); err != nil {
82+
return psmdb.Credentials{}, errors.Wrap(err, "update target user secret annotations")
83+
}
84+
}
85+
return psmdb.Credentials{
86+
Username: string(sec.Data[targetUserSecretUsernameKey]),
87+
Password: string(sec.Data[targetUserSecretPasswordKey]),
88+
}, nil
6889
}
6990

7091
func (r *ReconcilePerconaServerMongoDBClusterSync) ensureTargetUserSecret(
7192
ctx context.Context,
7293
cr *psmdbv1.PerconaServerMongoDBClusterSync,
73-
) (psmdb.Credentials, error) {
94+
) (*corev1.Secret, error) {
7495
nn := types.NamespacedName{Name: clustersync.TargetUserSecretName(cr), Namespace: cr.Namespace}
7596
existing := &corev1.Secret{}
7697
err := r.client.Get(ctx, nn, existing)
7798
switch {
7899
case err == nil:
79-
username := string(existing.Data[targetUserSecretUsernameKey])
80-
password := string(existing.Data[targetUserSecretPasswordKey])
81-
if username == "" || password == "" {
82-
return psmdb.Credentials{}, errors.Errorf("target user secret %s missing %s/%s keys",
83-
nn, targetUserSecretUsernameKey, targetUserSecretPasswordKey)
100+
if _, ok := existing.Data[targetUserSecretUsernameKey]; !ok {
101+
return nil, errors.Errorf("target user secret %s missing %s key", nn, targetUserSecretUsernameKey)
84102
}
85-
return psmdb.Credentials{Username: username, Password: password}, nil
103+
if _, ok := existing.Data[targetUserSecretPasswordKey]; !ok {
104+
return nil, errors.Errorf("target user secret %s missing %s key", nn, targetUserSecretPasswordKey)
105+
}
106+
return existing, nil
86107
case !k8serrors.IsNotFound(err):
87-
return psmdb.Credentials{}, errors.Wrapf(err, "get target user secret %s", nn)
108+
return nil, errors.Wrapf(err, "get target user secret %s", nn)
88109
}
89110

90111
password, err := secret.GeneratePassword()
91112
if err != nil {
92-
return psmdb.Credentials{}, errors.Wrap(err, "generate target user password")
113+
return nil, errors.Wrap(err, "generate target user password")
93114
}
94115
username := syncTargetUsername(cr)
95116

@@ -106,37 +127,56 @@ func (r *ReconcilePerconaServerMongoDBClusterSync) ensureTargetUserSecret(
106127
},
107128
}
108129
if err := controllerutil.SetControllerReference(cr, newSecret, r.scheme); err != nil {
109-
return psmdb.Credentials{}, errors.Wrap(err, "set owner reference on target user secret")
130+
return nil, errors.Wrap(err, "set owner reference on target user secret")
110131
}
111132
if err := r.client.Create(ctx, newSecret); err != nil {
112-
return psmdb.Credentials{}, errors.Wrapf(err, "create target user secret %s", nn)
133+
return nil, errors.Wrapf(err, "create target user secret %s", nn)
113134
}
114-
return psmdb.Credentials{Username: username, Password: string(password)}, nil
135+
return newSecret, nil
115136
}
116137

117-
// ensureTargetMongoUser converges the sync user on the target cluster: it
118-
// creates the user when missing, and re-applies password and roles when it
119-
// already exists. The latter heals the case where the CR (and its owned
120-
// Secret) was deleted and recreated while the Mongo user, which has no owner
121-
// ref, kept its old password.
122-
func ensureTargetMongoUser(ctx context.Context, mc mongo.Client, creds psmdb.Credentials) error {
123-
existing, err := mc.GetUserInfo(ctx, creds.Username, "admin")
138+
func ensureTargetMongoUser(ctx context.Context, mc mongo.Client, sec *corev1.Secret) (bool, error) {
139+
username := string(sec.Data[targetUserSecretUsernameKey])
140+
password := sec.Data[targetUserSecretPasswordKey]
141+
142+
existing, err := mc.GetUserInfo(ctx, username, "admin")
124143
if err != nil {
125-
return errors.Wrap(err, "look up target user")
144+
return false, errors.Wrap(err, "look up target user")
126145
}
127146
if existing == nil {
128-
if err := mc.CreateUser(ctx, "admin", creds.Username, creds.Password, syncTargetUserRoles...); err != nil {
129-
return errors.Wrapf(err, "create target user %q", creds.Username)
147+
if err := mc.CreateUser(ctx, "admin", username, string(password), syncTargetUserRoles...); err != nil {
148+
return false, errors.Wrapf(err, "create target user %q", username)
130149
}
131-
return nil
150+
setPasswordHashAnnotation(sec, password)
151+
return true, nil
132152
}
133-
if err := mc.UpdateUserPass(ctx, "admin", creds.Username, creds.Password); err != nil {
134-
return errors.Wrapf(err, "sync target user password %q", creds.Username)
153+
154+
mutated := false
155+
newHash := sha256Hash(password)
156+
if sec.Annotations[targetUserPasswordHashAnnotation] != newHash {
157+
if err := mc.UpdateUserPass(ctx, "admin", username, string(password)); err != nil {
158+
return false, errors.Wrapf(err, "sync target user password %q", username)
159+
}
160+
setPasswordHashAnnotation(sec, password)
161+
mutated = true
162+
}
163+
if !cmp.Equal(existing.Roles, syncTargetUserRoles, sortRolesOpt) {
164+
if err := mc.UpdateUserRoles(ctx, "admin", username, syncTargetUserRoles); err != nil {
165+
return false, errors.Wrapf(err, "sync target user roles %q", username)
166+
}
135167
}
136-
if err := mc.UpdateUserRoles(ctx, "admin", creds.Username, syncTargetUserRoles); err != nil {
137-
return errors.Wrapf(err, "sync target user roles %q", creds.Username)
168+
return mutated, nil
169+
}
170+
171+
func setPasswordHashAnnotation(sec *corev1.Secret, password []byte) {
172+
if sec.Annotations == nil {
173+
sec.Annotations = make(map[string]string)
138174
}
139-
return nil
175+
sec.Annotations[targetUserPasswordHashAnnotation] = sha256Hash(password)
176+
}
177+
178+
func sha256Hash(data []byte) string {
179+
return fmt.Sprintf("%x", sha256.Sum256(data))
140180
}
141181

142182
func defaultTargetMongoClient(ctx context.Context, cl client.Client, target *psmdbv1.PerconaServerMongoDB) (mongo.Client, error) {

0 commit comments

Comments
 (0)