Skip to content

Commit

Permalink
CLOUDP-264068: [Fix] Respect Teams deletion protection (#1722)
Browse files Browse the repository at this point in the history 
* CLOUDP-264068: Respect Teams deletion protection

* Respect also the keep flag

* Add e2e test coverage to fix

* Fix linter
  • Loading branch information
@josvazg
josvazg authored Jul 31, 2024
1 parent 818e9fb commit 94862b1
Show file tree
Hide file tree
Showing 3 changed files with 195 additions and 42 deletions.
15 changes: 10 additions & 5 deletions pkg/controller/atlasproject/teams.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/common"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/status"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/customresource"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/statushandler"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/workflow"
)
Expand Down Expand Up @@ -194,12 +195,16 @@ func (r *AtlasProjectReconciler) updateTeamState(ctx *workflow.Context, project
teamCtx.Client = atlasClient

if len(assignedProjects) == 0 {
log.Debugf("team %s has no project associated to it. removing from atlas.", team.Spec.Name)
_, err = teamCtx.Client.Teams.RemoveTeamFromOrganization(ctx.Context, orgID, team.Status.ID)
if err != nil {
return err
if customresource.IsResourcePolicyKeepOrDefault(project, r.ObjectDeletionProtection) {
log.Debug("team %s has no project associated, "+
"skipping deletion from Atlas due to ObjectDeletionProtection being set", team.Spec.Name)
} else {
log.Debugf("team %s has no project associated to it. removing from atlas.", team.Spec.Name)
_, err = teamCtx.Client.Teams.RemoveTeamFromOrganization(ctx.Context, orgID, team.Status.ID)
if err != nil {
return err
}
}

teamCtx.EnsureStatusOption(status.AtlasTeamUnsetID())
}

Expand Down
147 changes: 115 additions & 32 deletions pkg/controller/atlasproject/teams_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,14 @@ import (
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/common"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/status"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/customresource"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/workflow"
)

func TestUpdateTeamState(t *testing.T) {
t.Run("should not duplicate projects listed", func(t *testing.T) {
logger := zaptest.NewLogger(t).Sugar()
workflowCtx := &workflow.Context{
Context: context.Background(),
Log: logger,
}
testScheme := runtime.NewScheme()
akov2.AddToScheme(testScheme)
corev1.AddToScheme(testScheme)
workflowCtx := defaultTestWorkflow(logger)
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "my-secret",
Expand Down Expand Up @@ -74,38 +69,25 @@ func TestUpdateTeamState(t *testing.T) {
return &mongodbatlas.Client{}, "0987654321", nil
},
}
k8sClient := fake.NewClientBuilder().
WithScheme(testScheme).
WithObjects(secret, project, team).
Build()
k8sClient := buildFakeKubernetesClient(secret, project, team)
reconciler := &AtlasProjectReconciler{
Client: k8sClient,
Log: logger,
AtlasProvider: atlasProvider,
}
teamRef := &common.ResourceRefNamespaced{
Name: team.Name,
Namespace: "testNS",
}
// check we have exactly 1 project in status
assert.Equal(t, 1, len(team.Status.Projects))

// "reconcile" the team state and check we still have 1 project in status
err := reconciler.updateTeamState(workflowCtx, project, teamRef, false)
err := reconciler.updateTeamState(workflowCtx, project, reference(team), false)
assert.NoError(t, err)
k8sClient.Get(context.Background(), types.NamespacedName{Name: team.ObjectMeta.Name, Namespace: team.ObjectMeta.Namespace}, team)
assert.Equal(t, 1, len(team.Status.Projects))
})

t.Run("must remove a team from Atlas is a team is unassigned", func(t *testing.T) {
logger := zaptest.NewLogger(t).Sugar()
workflowCtx := &workflow.Context{
Context: context.Background(),
Log: logger,
}
testScheme := runtime.NewScheme()
akov2.AddToScheme(testScheme)
corev1.AddToScheme(testScheme)
workflowCtx := defaultTestWorkflow(logger)
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "my-secret",
Expand Down Expand Up @@ -151,23 +133,124 @@ func TestUpdateTeamState(t *testing.T) {
}, "0987654321", nil
},
}
k8sClient := fake.NewClientBuilder().
WithScheme(testScheme).
WithObjects(secret, project, team).
Build()
k8sClient := buildFakeKubernetesClient(secret, project, team)
reconciler := &AtlasProjectReconciler{
Client: k8sClient,
Log: logger,
AtlasProvider: atlasProvider,
}
teamRef := &common.ResourceRefNamespaced{
Name: team.Name,
Namespace: "testNS",
}

err := reconciler.updateTeamState(workflowCtx, project, teamRef, true)
err := reconciler.updateTeamState(workflowCtx, project, reference(team), true)
assert.NoError(t, err)
k8sClient.Get(context.Background(), types.NamespacedName{Name: team.ObjectMeta.Name, Namespace: team.ObjectMeta.Namespace}, team)
assert.Len(t, teamsMock.RemoveTeamFromOrganizationRequests, 1)
})

t.Run("must honor deletion protection flag for Teams", func(t *testing.T) {
for _, tc := range []struct {
title string
deletionProtection bool
keepFlag bool
expectRemoval bool
}{
{
title: "with deletion protection unassigned teams are not removed",
deletionProtection: true,
keepFlag: false,
expectRemoval: false,
},
{
title: "without deletion protection unassigned teams are removed",
deletionProtection: false,
keepFlag: false,
expectRemoval: true,
},
{
title: "with deletion protection & keep flag teams are not removed",
deletionProtection: false,
keepFlag: true,
expectRemoval: false,
},
{
title: "without deletion protection but keep flag teams are not removed",
deletionProtection: true,
keepFlag: true,
expectRemoval: false,
},
} {
t.Run(tc.title, func(t *testing.T) {
logger := zaptest.NewLogger(t).Sugar()
workflowCtx := defaultTestWorkflow(logger)
project := &akov2.AtlasProject{
Spec: akov2.AtlasProjectSpec{
Name: "projectName",
},
}
team := &akov2.AtlasTeam{
ObjectMeta: metav1.ObjectMeta{
Name: "testTeam",
Namespace: "testNS",
},
}
teamsMock := &atlas.TeamsClientMock{
RemoveTeamFromOrganizationFunc: func(orgID string, teamID string) (*mongodbatlas.Response, error) {
return nil, nil
},
RemoveTeamFromOrganizationRequests: map[string]struct{}{},
}
atlasProvider := &atlas.TestProvider{
ClientFunc: func(secretRef *client.ObjectKey, log *zap.SugaredLogger) (*mongodbatlas.Client, string, error) {
return &mongodbatlas.Client{
Teams: teamsMock,
}, "0987654321", nil
},
}
reconciler := &AtlasProjectReconciler{
Client: buildFakeKubernetesClient(project, team),
Log: logger,
AtlasProvider: atlasProvider,
ObjectDeletionProtection: tc.deletionProtection,
}
if tc.keepFlag {
customresource.SetAnnotation(project,
customresource.ResourcePolicyAnnotation, customresource.ResourcePolicyKeep)
}
err := reconciler.updateTeamState(workflowCtx, project, reference(team), true)
assert.NoError(t, err)
expectedRemovals := 0
if tc.expectRemoval {
expectedRemovals = 1
}
assert.Len(t, teamsMock.RemoveTeamFromOrganizationRequests, expectedRemovals)
})
}
})
}

func defaultTestWorkflow(logger *zap.SugaredLogger) *workflow.Context {
return &workflow.Context{
Context: context.Background(),
Log: logger,
}
}

func defaultTestScheme() *runtime.Scheme {
scheme := runtime.NewScheme()
akov2.AddToScheme(scheme)
corev1.AddToScheme(scheme)
return scheme
}

func buildFakeKubernetesClient(objects ...client.Object) client.WithWatch {
return fake.NewClientBuilder().
WithScheme(defaultTestScheme()).
WithObjects(objects...).
Build()
}

func reference(obj client.Object) *common.ResourceRefNamespaced {
return &common.ResourceRefNamespaced{
Name: obj.GetName(),
Namespace: obj.GetNamespace(),
}
}
75 changes: 70 additions & 5 deletions test/e2e/teams_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
package e2e_test

import (
"context"
"errors"
"fmt"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"go.mongodb.org/atlas-sdk/v20231115008/admin"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api"
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/common"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/test/helper/e2e/actions"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/test/helper/e2e/api/atlas"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/test/helper/e2e/data"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/test/helper/e2e/model"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/test/helper/e2e/utils"
Expand All @@ -31,8 +36,6 @@ var _ = Describe("Teams", Label("teams"), func() {
Expect(actions.SaveTeamsToFile(testData.Context, testData.K8SClient, testData.Resources.Namespace)).Should(Succeed())
}
By("Delete Resources", func() {
actions.DeleteTestDataTeams(testData)
actions.DeleteTestDataProject(testData)
actions.AfterEachFinalCleanup([]model.TestDataProvider{*testData})
})
})
Expand All @@ -50,7 +53,7 @@ var _ = Describe("Teams", Label("teams"), func() {
model.NewEmptyAtlasKeyType().UseDefaultFullAccess(),
40000,
[]func(*model.TestDataProvider){},
).WithProject(data.DefaultProject()),
).WithProject(data.DefaultProject()).WithObjectDeletionProtection(true),
[]akov2.Team{
{
TeamRef: common.ResourceRefNamespaced{
Expand Down Expand Up @@ -120,10 +123,33 @@ func projectTeamsFlow(userData *model.TestDataProvider, teams []akov2.Team) {
Expect(userData.K8SClient.Update(userData.Context, userData.Project)).Should(Succeed())
Eventually(func(g Gomega) bool {
return ensureTeamsStatus(g, *userData, teams, teamWasRemoved)
}).WithTimeout(10*time.Minute).WithPolling(20*time.Second).Should(BeTrue(), "Team were not removed")
}).WithTimeout(10*time.Minute).WithPolling(20*time.Second).Should(BeTrue(), "Teams were not removed")

actions.CheckProjectConditionsNotSet(userData, api.ProjectTeamsReadyType)
})

if userData.ObjectDeletionProtection {
aClient := atlas.GetClientOrFail()
By("Cleanup Atlas Teams: which should have not been removed due to deletion protection", func() {
atlasTeams, err := listAtLeastNTeams(userData.Context, aClient, len(teams))
Expect(err).Should(Succeed())
Expect(clearAtlasTeams(teams, atlasTeams, aClient, userData)).Should(Succeed())
})

By("Cleanup Atlas Project: as deletion protection will skip that", func() {
Expect(userData.K8SClient.Get(userData.Context, types.NamespacedName{
Name: userData.Project.Name,
Namespace: userData.Project.Namespace,
}, userData.Project)).Should(Succeed())
projectID := userData.Project.Status.ID
Expect(userData.K8SClient.Delete(userData.Context, userData.Project)).Should(Succeed())
_, _, err := aClient.Client.ProjectsApi.DeleteProject(userData.Context, projectID).Execute()
Expect(err).Should(Succeed())
})
} else {
actions.DeleteTestDataTeams(userData)
actions.DeleteTestDataProject(userData)
}
}

func ensureTeamsStatus(g Gomega, testData model.TestDataProvider, teams []akov2.Team, check func(res *akov2.AtlasTeam) bool) bool {
Expand All @@ -136,7 +162,6 @@ func ensureTeamsStatus(g Gomega, testData model.TestDataProvider, teams []akov2.
return false
}
}

return true
}

Expand All @@ -147,3 +172,43 @@ func teamWasCreated(team *akov2.AtlasTeam) bool {
func teamWasRemoved(team *akov2.AtlasTeam) bool {
return team.Status.ID == ""
}

func listAtLeastNTeams(ctx context.Context, aClient *atlas.Atlas, minTeams int) ([]admin.TeamResponse, error) {
results := []admin.TeamResponse{}
teamsReply, _, err := aClient.Client.TeamsApi.ListOrganizationTeams(ctx, aClient.OrgID).Execute()
if err != nil {
return results, fmt.Errorf("failed to list teams: %w", err)
}
total, ok := teamsReply.GetTotalCountOk()
if !ok {
return results, errors.New("no results")
}
if *total < minTeams {
return results, fmt.Errorf("not enough teams: expected %d but got %d", minTeams, *total)
}
return teamsReply.GetResults(), nil
}

func clearAtlasTeams(teams []akov2.Team, atlasTeams []admin.TeamResponse, aClient *atlas.Atlas, userData *model.TestDataProvider) error {
var errs error
for _, team := range teams {
foundAtlasTeam := findTeam(atlasTeams, team.TeamRef.Name)
if foundAtlasTeam == nil {
errs = errors.Join(errs, fmt.Errorf("failed to find expected Atlas team %s (was it wrongly removed?)", team.TeamRef.Name))
}
_, _, err := aClient.Client.TeamsApi.DeleteTeam(userData.Context, aClient.OrgID, foundAtlasTeam.GetId()).Execute()
if err != nil {
errs = errors.Join(errs, err)
}
}
return errs
}

func findTeam(atlasTeams []admin.TeamResponse, teamName string) *admin.TeamResponse {
for _, atlasTeam := range atlasTeams {
if teamName == atlasTeam.GetName() {
return &atlasTeam
}
}
return nil
}

0 comments on commit 94862b1

Please sign in to comment.