Skip to content

CLOUDP-334941: Connection Secret Controller #2575

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

CLOUDP-334941: Connection Secret Controller #2575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
37d80ef
feat: scaffold the ConnectionSecret controller
andrpac Aug 5, 2025
1ec093a
chore: clean up loggin
andrpac Aug 12, 2025
346d7c5
feat: implement generic controller to include deployment and federation
andrpac Aug 16, 2025
586387c
chore: remove reconciler from endpoint struct, add explicit fields
andrpac Aug 19, 2025
4b524e5
chore: include integration tests for connection secrets with Deployme…
andrpac Aug 21, 2025
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ install-credentials: set-namespace ## Install the Atlas credentials for the Oper

.PHONY: prepare-run
prepare-run: generate vet manifests run-kind install-crds install-credentials
rm bin/manager
rm -f bin/manager
$(MAKE) manager VERSION=$(NEXT_VERSION)

.PHONY: run
Expand Down
9 changes: 9 additions & 0 deletions api/condition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,15 @@ func HasConditionType(typ ConditionType, source []Condition) bool {
return false
}

func HasReadyCondition(conditions []Condition) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I believe there is a generic helper in k8s libs for this, let me check...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is:
https://pkg.go.dev/k8s.io/[email protected]/pkg/api/meta#IsStatusConditionTrue

but you woudl need a metav1.Condition to use it, which is not what you have here. This is a custom Condition type.

Take this as an informational comment. Nothing we can do here while we are using custom Conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh cool, did not know this. I am not super familiar with all the k8s libraries

for _, c := range conditions {
if c.Type == ReadyType && c.Status == corev1.ConditionTrue {
return true
}
}
return false
}

// EnsureConditionExists adds or updates the condition in the copy of a 'source' slice
func EnsureConditionExists(condition Condition, source []Condition) []Condition {
condition.LastTransitionTime = metav1.Now()
Expand Down
5 changes: 5 additions & 0 deletions api/v1/atlasdatabaseuser_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@ func (p *AtlasDatabaseUser) ProjectDualRef() *ProjectDualReference {
return &p.Spec.ProjectDualReference
}

// IsDatabaseUserReady checks if the Ready condition is available
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for future shelves: this will not play nice with autogenerated CRDs

But those use metav1.Condition so we might have a generic way instead of calling one per type like here.

Nothing actionable for this PR though

func (p *AtlasDatabaseUser) IsDatabaseUserReady() bool {
return api.HasReadyCondition(p.Status.Conditions)
}

func (p *AtlasDatabaseUser) UpdateStatus(conditions []api.Condition, options ...api.Option) {
p.Status.Conditions = conditions
p.Status.ObservedGeneration = p.ObjectMeta.Generation
Expand Down
4 changes: 4 additions & 0 deletions api/v1/atlasdeployment_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,10 @@ func (c *AtlasDeployment) GetReplicationSetID() string {
return ""
}

func (c *AtlasDeployment) IsDeploymentReady() bool {
return api.HasReadyCondition(c.Status.Conditions)
}

// +kubebuilder:object:root=true

// AtlasDeploymentList contains a list of AtlasDeployment
Expand Down
8 changes: 1 addition & 7 deletions internal/controller/atlasdatabaseuser/atlasdatabaseuser_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import (
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1/status"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/atlas"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/connectionsecret"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/customresource"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/reconciler"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/statushandler"
Expand Down Expand Up @@ -141,12 +140,7 @@ func (r *AtlasDatabaseUserReconciler) terminate(
}

// unmanage remove finalizer and release resource
func (r *AtlasDatabaseUserReconciler) unmanage(ctx *workflow.Context, projectID string, atlasDatabaseUser *akov2.AtlasDatabaseUser) (ctrl.Result, error) {
err := connectionsecret.RemoveStaleSecretsByUserName(ctx.Context, r.Client, projectID, atlasDatabaseUser.Spec.Username, *atlasDatabaseUser, r.Log)
if err != nil {
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.DatabaseUserConnectionSecretsNotDeleted, true, err)
}

func (r *AtlasDatabaseUserReconciler) unmanage(ctx *workflow.Context, atlasDatabaseUser *akov2.AtlasDatabaseUser) (ctrl.Result, error) {
if customresource.HaveFinalizer(atlasDatabaseUser, customresource.FinalizerLabel) {
err := customresource.ManageFinalizer(ctx.Context, r.Client, atlasDatabaseUser, customresource.UnsetFinalizer)
if err != nil {
Expand Down
62 changes: 8 additions & 54 deletions internal/controller/atlasdatabaseuser/databaseuser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ import (
"context"
"errors"
"fmt"
"time"

corev1 "k8s.io/api/core/v1"
ctrl "sigs.k8s.io/controller-runtime"

"github.com/mongodb/mongodb-atlas-kubernetes/v2/api"
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/connectionsecret"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/customresource"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/workflow"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/timeutil"
Expand Down Expand Up @@ -84,18 +82,15 @@ func (r *AtlasDatabaseUserReconciler) dbuLifeCycle(ctx *workflow.Context, dbUser
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.Internal, true, err)
}

expired, err := isExpired(atlasDatabaseUser)
expired, err := timeutil.IsExpired(atlasDatabaseUser.Spec.DeleteAfterDate)
if err != nil {
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.DatabaseUserInvalidSpec, false, err)
}
if expired {
err = connectionsecret.RemoveStaleSecretsByUserName(ctx.Context, r.Client, atlasProject.ID, atlasDatabaseUser.Spec.Username, *atlasDatabaseUser, r.Log)
if err != nil {
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.DatabaseUserConnectionSecretsNotDeleted, true, err)
}

ctx.SetConditionFromResult(api.DatabaseUserReadyType, workflow.Terminate(workflow.DatabaseUserExpired, errors.New("an expired user cannot be managed")))
return r.unmanage(ctx, atlasProject.ID, atlasDatabaseUser)
ctx.SetConditionFromResult(api.DatabaseUserReadyType,
workflow.Terminate(workflow.DatabaseUserExpired, errors.New("an expired user cannot be managed")),
)
return r.unmanage(ctx, atlasDatabaseUser)
}

scopesAreValid, err := r.areDeploymentScopesValid(ctx, deploymentService, atlasProject.ID, atlasDatabaseUser)
Expand All @@ -117,7 +112,7 @@ func (r *AtlasDatabaseUserReconciler) dbuLifeCycle(ctx *workflow.Context, dbUser
case dbUserExists && wasDeleted:
return r.delete(ctx, dbUserService, atlasProject.ID, atlasDatabaseUser)
default:
return r.unmanage(ctx, atlasProject.ID, atlasDatabaseUser)
return r.unmanage(ctx, atlasDatabaseUser)
}
}

Expand All @@ -139,11 +134,6 @@ func (r *AtlasDatabaseUserReconciler) create(ctx *workflow.Context, dbUserServic
}

if wasRenamed(atlasDatabaseUser) {
err = connectionsecret.RemoveStaleSecretsByUserName(ctx.Context, r.Client, projectID, atlasDatabaseUser.Status.UserName, *atlasDatabaseUser, r.Log)
if err != nil {
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.DatabaseUserConnectionSecretsNotDeleted, true, err)
}

ctx.Log.Infow("'spec.username' has changed - removing the old user from Atlas", "newUserName", atlasDatabaseUser.Spec.Username, "oldUserName", atlasDatabaseUser.Status.UserName)
if err = r.removeOldUser(ctx.Context, dbUserService, projectID, atlasDatabaseUser); err != nil {
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.Internal, true, err)
Expand Down Expand Up @@ -183,7 +173,7 @@ func (r *AtlasDatabaseUserReconciler) delete(ctx *workflow.Context, dbUserServic
if customresource.IsResourcePolicyKeepOrDefault(atlasDatabaseUser, r.ObjectDeletionProtection) {
r.Log.Info("Not removing Atlas database user from Atlas as per configuration")

return r.unmanage(ctx, projectID, atlasDatabaseUser)
return r.unmanage(ctx, atlasDatabaseUser)
}

err := dbUserService.Delete(ctx.Context, atlasDatabaseUser.Spec.DatabaseName, projectID, atlasDatabaseUser.Spec.Username)
Expand All @@ -195,7 +185,7 @@ func (r *AtlasDatabaseUserReconciler) delete(ctx *workflow.Context, dbUserServic
r.Log.Info("Database user doesn't exist or is already deleted")
}

return r.unmanage(ctx, projectID, atlasDatabaseUser)
return r.unmanage(ctx, atlasDatabaseUser)
}

func (r *AtlasDatabaseUserReconciler) readiness(ctx *workflow.Context, deploymentService deployment.AtlasDeploymentsService,
Expand All @@ -205,19 +195,6 @@ func (r *AtlasDatabaseUserReconciler) readiness(ctx *workflow.Context, deploymen
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.Internal, true, err)
}

removedOrphanSecrets, err := connectionsecret.ReapOrphanConnectionSecrets(
ctx.Context, r.Client, atlasProject.ID, atlasDatabaseUser.Namespace, allDeploymentNames)
if err != nil {
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.Internal, true, err)
}
if len(removedOrphanSecrets) > 0 {
r.Log.Debugw("Removed orphan secrets bound to an non existent deployment",
"project", atlasProject.Name, "removed", len(removedOrphanSecrets))
for _, orphan := range removedOrphanSecrets {
r.Log.Debugw("Removed orphan", "secret", orphan)
}
}

deploymentsToCheck := allDeploymentNames
if atlasDatabaseUser.Spec.Scopes != nil {
deploymentsToCheck = filterScopeDeployments(atlasDatabaseUser, allDeploymentNames)
Expand All @@ -243,12 +220,6 @@ func (r *AtlasDatabaseUserReconciler) readiness(ctx *workflow.Context, deploymen
)
}

// TODO refactor connectionsecret package to follow state machine approach
result := connectionsecret.CreateOrUpdateConnectionSecrets(ctx, r.Client, deploymentService, r.EventRecorder, atlasProject, *atlasDatabaseUser)
if !result.IsOk() {
return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.DatabaseUserConnectionSecretsNotCreated, true, errors.New(result.GetMessage()))
}

return r.ready(ctx, atlasDatabaseUser, passwordVersion)
}

Expand Down Expand Up @@ -304,23 +275,6 @@ func (r *AtlasDatabaseUserReconciler) removeOldUser(ctx context.Context, dbUserS
return err
}

func isExpired(atlasDatabaseUser *akov2.AtlasDatabaseUser) (bool, error) {
if atlasDatabaseUser.Spec.DeleteAfterDate == "" {
return false, nil
}

deleteAfter, err := timeutil.ParseISO8601(atlasDatabaseUser.Spec.DeleteAfterDate)
if err != nil {
return false, err
}

if !deleteAfter.Before(time.Now()) {
return false, nil
}

return true, nil
}

func hasChanged(databaseUserInAKO, databaseUserInAtlas *dbuser.User, currentPassVersion, passVersion string) bool {
return !dbuser.EqualSpecs(databaseUserInAKO, databaseUserInAtlas) || currentPassVersion != passVersion
}
Expand Down
44 changes: 2 additions & 42 deletions internal/controller/atlasdatabaseuser/databaseuser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
atlasmock "github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/mocks/atlas"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/mocks/translation"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/pointer"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/timeutil"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/translation/dbuser"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/translation/deployment"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/translation/project"
Expand Down Expand Up @@ -693,7 +694,6 @@ func TestDbuLifeCycle(t *testing.T) {
dService: func() deployment.AtlasDeploymentsService {
service := translation.NewAtlasDeploymentsServiceMock(t)
service.EXPECT().ListDeploymentNames(context.Background(), "").Return([]string{}, nil)
service.EXPECT().ListDeploymentConnections(context.Background(), "").Return([]deployment.Connection{}, nil)

return service
},
Expand Down Expand Up @@ -1213,7 +1213,6 @@ func TestUpdate(t *testing.T) {
dService: func() deployment.AtlasDeploymentsService {
service := translation.NewAtlasDeploymentsServiceMock(t)
service.EXPECT().ListDeploymentNames(context.Background(), "").Return([]string{}, nil)
service.EXPECT().ListDeploymentConnections(context.Background(), "").Return([]deployment.Connection{}, nil)

return service
},
Expand Down Expand Up @@ -1659,43 +1658,6 @@ func TestReadiness(t *testing.T) {
WithMessageRegexp("0 out of 1 deployments have applied database user changes"),
},
},
"failed to create connection secrets": {
wantErr: true,
dbUser: &akov2.AtlasDatabaseUser{
ObjectMeta: metav1.ObjectMeta{
Name: "user1",
Namespace: "default",
},
Spec: akov2.AtlasDatabaseUserSpec{
Username: "user1",
PasswordSecret: &common.ResourceRef{
Name: "user-pass",
},
Scopes: []akov2.ScopeSpec{
{
Name: "cluster2",
Type: akov2.DeploymentScopeType,
},
},
},
},
dService: func() deployment.AtlasDeploymentsService {
service := translation.NewAtlasDeploymentsServiceMock(t)
service.EXPECT().ListDeploymentNames(context.Background(), "").
Return([]string{"cluster1", "cluster2"}, nil)
service.EXPECT().DeploymentIsReady(context.Background(), "", "cluster2").
Return(true, nil)
service.EXPECT().ListDeploymentConnections(context.Background(), "").
Return(nil, errors.New("failed to list cluster connections"))

return service
},
expectedConditions: []api.Condition{
api.FalseCondition(api.DatabaseUserReadyType).
WithReason(string(workflow.DatabaseUserConnectionSecretsNotCreated)).
WithMessageRegexp("failed to list cluster connections"),
},
},
"resource is ready": {
dbUser: &akov2.AtlasDatabaseUser{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -1721,8 +1683,6 @@ func TestReadiness(t *testing.T) {
Return([]string{"cluster1", "cluster2"}, nil)
service.EXPECT().DeploymentIsReady(context.Background(), "", "cluster2").
Return(true, nil)
service.EXPECT().ListDeploymentConnections(context.Background(), "").
Return([]deployment.Connection{}, nil)

return service
},
Expand Down Expand Up @@ -2127,7 +2087,7 @@ func TestIsExpired(t *testing.T) {

for name, tt := range tests {
t.Run(name, func(t *testing.T) {
expired, err := isExpired(tt.dbUser)
expired, err := timeutil.IsExpired(tt.dbUser.Spec.DeleteAfterDate)
assert.Equal(t, tt.err, err)
assert.Equal(t, tt.expected, expired)
})
Expand Down
Loading
Loading