Skip to content

CLOUDP-196371: Delete users removed from the resource #1587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jul 23, 2024
Merged

CLOUDP-196371: Delete users removed from the resource #1587

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
91ac04d
Modify missleading or outdated documentation
mateigrigore Jul 5, 2024
c56b2f0
Fix indentation
mateigrigore Jul 5, 2024
74cc2c5
Removed redundant flag
mateigrigore Jul 5, 2024
232a38f
Mofify Makefile to fix make deploy
mateigrigore Jul 9, 2024
8ebccf6
Cleanup connection string secrets of removed users
mateigrigore Jul 16, 2024
5d3047a
Add removed users to automation config
mateigrigore Jul 16, 2024
0bddbc3
Add e2e tests for cleaning up users
mateigrigore Jul 16, 2024
92b4ab1
Add the connection string secret to the watcher
mateigrigore Jul 16, 2024
9accbc7
Remove Makefile changes
mateigrigore Jul 16, 2024
999f2c6
Update method name
mateigrigore Jul 16, 2024
0995b72
Fix linting
mateigrigore Jul 16, 2024
105acba
Add new e2e test to workflow
mateigrigore Jul 16, 2024
9a926d9
Add new e2e test to github workflow
mateigrigore Jul 18, 2024
259f288
Fix failing test
mateigrigore Jul 22, 2024
83decf6
Implement suggested changes
mateigrigore Jul 22, 2024
396c812
Update test to check for more complex behavior
mateigrigore Jul 23, 2024
5aeacf7
Remove lftover debuggign print statement
mateigrigore Jul 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions controllers/mongodb_cleanup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestReplicaSetReconcilerCleanupScramSecrets(t *testing.T) {
t.Run("no change same resource", func(t *testing.T) {
actual := getScramSecretsToDelete(lastApplied.Spec, lastApplied.Spec)

assert.Equal(t, []string{}, actual)
assert.Equal(t, []string(nil), actual)
})

t.Run("new user new secret", func(t *testing.T) {
Expand All @@ -45,7 +45,7 @@ func TestReplicaSetReconcilerCleanupScramSecrets(t *testing.T) {

actual := getScramSecretsToDelete(current.Spec, lastApplied.Spec)

assert.Equal(t, []string{}, actual)
assert.Equal(t, []string(nil), actual)
})

t.Run("old user new secret", func(t *testing.T) {
Expand Down Expand Up @@ -165,7 +165,7 @@ func TestReplicaSetReconcilerCleanupConnectionStringSecrets(t *testing.T) {
t.Run("no change same resource", func(t *testing.T) {
actual := getConnectionStringSecretsToDelete(lastApplied.Spec, lastApplied.Spec, "my-rs")

assert.Equal(t, []string{}, actual)
assert.Equal(t, []string(nil), actual)
})

t.Run("new user does not require existing user cleanup", func(t *testing.T) {
Expand All @@ -188,7 +188,7 @@ func TestReplicaSetReconcilerCleanupConnectionStringSecrets(t *testing.T) {

actual := getConnectionStringSecretsToDelete(current.Spec, lastApplied.Spec, "my-rs")

assert.Equal(t, []string{}, actual)
assert.Equal(t, []string(nil), actual)
})

t.Run("old user new secret", func(t *testing.T) {
Expand Down
29 changes: 26 additions & 3 deletions test/e2e/mongodbtests/mongodbtests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,10 +772,22 @@ func assertEqualOwnerReference(t *testing.T, resourceType string, resourceNamesp
assert.Equal(t, expectedOwnerReference.UID, ownerReferences[0].UID)
}

func RemoveAllUsersFromResource(ctx context.Context, mdb *mdbv1.MongoDBCommunity) func(*testing.T) {
func RemoveLastUserFromMongoDBCommunity(ctx context.Context, mdb *mdbv1.MongoDBCommunity) func(*testing.T) {
return func(t *testing.T) {
err := e2eutil.UpdateMongoDBResource(ctx, mdb, func(db *mdbv1.MongoDBCommunity) {
db.Spec.Users = []mdbv1.MongoDBUser{}
db.Spec.Users = db.Spec.Users[:len(db.Spec.Users)-1]
})

if err != nil {
t.Fatal(err)
}
}
}

func EditConnectionStringSecretNameOfLastUser(ctx context.Context, mdb *mdbv1.MongoDBCommunity, newSecretName string) func(*testing.T) {
return func(t *testing.T) {
err := e2eutil.UpdateMongoDBResource(ctx, mdb, func(db *mdbv1.MongoDBCommunity) {
db.Spec.Users[len(db.Spec.Users)-1].ConnectionStringSecretName = newSecretName
})

if err != nil {
Expand All @@ -790,7 +802,7 @@ func ConnectionStringSecretIsCleanedUp(ctx context.Context, mdb *mdbv1.MongoDBCo
newErr := e2eutil.TestClient.Get(ctx, types.NamespacedName{Name: removedConnectionString, Namespace: mdb.Namespace}, &connectionStringSecret)

fmt.Println(newErr)
assert.Error(t, newErr)
assert.EqualError(t, newErr, fmt.Sprintf("secrets \"%s\" not found", removedConnectionString))
}
}

Expand All @@ -803,3 +815,14 @@ func AuthUsersDeletedIsUpdated(ctx context.Context, mdb *mdbv1.MongoDBCommunity,
assert.Contains(t, currentAc.Auth.UsersDeleted, deletedUser)
}
}

func AddUserToMongoDBCommunity(ctx context.Context, mdb *mdbv1.MongoDBCommunity, newUser mdbv1.MongoDBUser) func(t *testing.T) {
return func(t *testing.T) {
err := e2eutil.UpdateMongoDBResource(ctx, mdb, func(db *mdbv1.MongoDBCommunity) {
db.Spec.Users = append(db.Spec.Users, newUser)
})
if err != nil {
t.Fatal(err)
}
}
}
77 changes: 49 additions & 28 deletions test/e2e/replica_set_remove_user/replica_set_remove_user_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package replica_set_remove_user
import (
"context"
"fmt"
v1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1"
mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1"
"github.com/mongodb/mongodb-kubernetes-operator/pkg/automationconfig"
e2eutil "github.com/mongodb/mongodb-kubernetes-operator/test/e2e"
"github.com/mongodb/mongodb-kubernetes-operator/test/e2e/mongodbtests"
Expand Down Expand Up @@ -36,28 +36,6 @@ func TestCleanupUsers(t *testing.T) {
t.Fatal(err)
}

lcr := automationconfig.CrdLogRotate{
// fractional values are supported
SizeThresholdMB: "0.1",
LogRotate: automationconfig.LogRotate{
TimeThresholdHrs: 1,
NumUncompressed: 10,
NumTotal: 10,
IncludeAuditLogsWithMongoDBLogs: false,
},
PercentOfDiskspace: "1",
}

systemLog := automationconfig.SystemLog{
Destination: automationconfig.File,
Path: "/tmp/mongod.log",
LogAppend: false,
}

// logRotate can only be configured if systemLog to file has been configured
mdb.Spec.AgentConfiguration.LogRotate = &lcr
mdb.Spec.AgentConfiguration.SystemLog = &systemLog

// config member options
memberOptions := []automationconfig.MemberOptions{
{
Expand All @@ -81,8 +59,46 @@ func TestCleanupUsers(t *testing.T) {
settings := map[string]interface{}{
"electionTimeoutMillis": float64(20),
}
mdb.Spec.AutomationConfigOverride = &v1.AutomationConfigOverride{
ReplicaSet: v1.OverrideReplicaSet{Settings: v1.MapWrapper{Object: settings}},
mdb.Spec.AutomationConfigOverride = &mdbv1.AutomationConfigOverride{
ReplicaSet: mdbv1.OverrideReplicaSet{Settings: mdbv1.MapWrapper{Object: settings}},
}

newUser := mdbv1.MongoDBUser{
Name: fmt.Sprintf("%s-user-2", "mdb-0"),
PasswordSecretRef: mdbv1.SecretKeyReference{
Key: fmt.Sprintf("%s-password-2", "mdb-0"),
Name: fmt.Sprintf("%s-%s-password-secret-2", "mdb-0", testCtx.ExecutionId),
},
Roles: []mdbv1.Role{
// roles on testing db for general connectivity
{
DB: "testing",
Name: "readWrite",
},
{
DB: "testing",
Name: "clusterAdmin",
},
// admin roles for reading FCV
{
DB: "admin",
Name: "readWrite",
},
{
DB: "admin",
Name: "clusterAdmin",
},
{
DB: "admin",
Name: "userAdmin",
},
},
ScramCredentialsSecretName: fmt.Sprintf("%s-my-scram-2", "mdb-0"),
}

_, err = setup.GeneratePasswordForUser(testCtx, newUser, "")
if err != nil {
t.Fatal(err)
}

tester, err := FromResource(ctx, t, mdb)
Expand All @@ -93,11 +109,16 @@ func TestCleanupUsers(t *testing.T) {
t.Run("Create MongoDB Resource", mongodbtests.CreateMongoDBResource(&mdb, testCtx))
t.Run("Basic tests", mongodbtests.BasicFunctionality(ctx, &mdb))
t.Run("Keyfile authentication is configured", tester.HasKeyfileAuth(3))
t.Run("AutomationConfig has the correct logRotateConfig", mongodbtests.AutomationConfigHasLogRotationConfig(ctx, &mdb, &lcr))
t.Run("Test Basic Connectivity", tester.ConnectivitySucceeds())
t.Run("Test SRV Connectivity", tester.ConnectivitySucceeds(WithURI(mdb.MongoSRVURI("")), WithoutTls(), WithReplicaSet(mdb.Name)))
deletedUser := mdb.Spec.Users[0]
t.Run("Delete user from MongoDB Resource", mongodbtests.RemoveAllUsersFromResource(ctx, &mdb))
t.Run("Add new user to MongoDB Resource", mongodbtests.AddUserToMongoDBCommunity(ctx, &mdb, newUser))
t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb))
editedUser := mdb.Spec.Users[1]
t.Run("Edit connection string secret name of the added user", mongodbtests.EditConnectionStringSecretNameOfLastUser(ctx, &mdb, "other-secret-name"))
t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb))
t.Run("Old connection string secret is cleaned up", mongodbtests.ConnectionStringSecretIsCleanedUp(ctx, &mdb, editedUser.GetConnectionStringSecretName(mdb.Name)))
deletedUser := mdb.Spec.Users[1]
t.Run("Remove last user from MongoDB Resource", mongodbtests.RemoveLastUserFromMongoDBCommunity(ctx, &mdb))
t.Run("MongoDB reaches Pending phase", mongodbtests.MongoDBReachesPendingPhase(ctx, &mdb))
t.Run("Removed users are added to automation config", mongodbtests.AuthUsersDeletedIsUpdated(ctx, &mdb, deletedUser))
t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb))
Expand Down