From 767cf8b2115a6f72f825a13cce9a4ccfdab95578 Mon Sep 17 00:00:00 2001 From: cveticm <119604954+cveticm@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:17:48 +0100 Subject: [PATCH] CLOUDP-228592: Migrate Assigned Teams Api (#1817) * Initial Api change * Initial test changes - issues with project_test.go to be resolved * Progress on project unit-test fixes all project unit-tests touching teams fail. Failure seems to happen in syncAssignedTeams() * Fix dereference to pointer slice Panic in project unit tests were caused by dereferencing pointer slices. This has now been fixed. * Rebasing and lint fix * Implementing review feedback Namely, dereferencing protection with use of getter/setter functions * Implementing review feedback Additional getter implementation for dereferencing protection and changing team_reconciler/ensureTeamState() and associated functions to utilise type *admin.TeamResponse in place of *admin.Team. * Implement review change requests/ Removal of setting team links and removal of unused argument projectID in teamReconciler. * Implement review change requests --- pkg/api/v1/atlasteam_types.go | 7 +- pkg/api/v1/project_teams.go | 14 +- pkg/controller/atlasproject/project_test.go | 85 ++++------ .../atlasproject/team_reconciler.go | 108 +++++++------ .../atlasproject/team_reconciler_test.go | 147 ++++++++++++------ pkg/controller/atlasproject/teams.go | 66 ++++---- pkg/controller/atlasproject/teams_test.go | 81 +++++----- 7 files changed, 272 insertions(+), 236 deletions(-) diff --git a/pkg/api/v1/atlasteam_types.go b/pkg/api/v1/atlasteam_types.go index 099909480c..fd111fde99 100644 --- a/pkg/api/v1/atlasteam_types.go +++ b/pkg/api/v1/atlasteam_types.go @@ -17,11 +17,12 @@ limitations under the License. package v1 import ( + "go.mongodb.org/atlas-sdk/v20231115008/admin" + "github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/compat" "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api" "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/status" - "go.mongodb.org/atlas/mongodbatlas" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -87,8 +88,8 @@ func (in *AtlasTeam) UpdateStatus(conditions []api.Condition, options ...api.Opt } } -func (in *AtlasTeam) ToAtlas() (*mongodbatlas.Team, error) { - result := &mongodbatlas.Team{} +func (in *AtlasTeam) ToAtlas() (*admin.Team, error) { + result := &admin.Team{} err := compat.JSONCopy(result, in.Spec) return result, err diff --git a/pkg/api/v1/project_teams.go b/pkg/api/v1/project_teams.go index 1b58ad0f15..83b1520e50 100644 --- a/pkg/api/v1/project_teams.go +++ b/pkg/api/v1/project_teams.go @@ -1,7 +1,7 @@ package v1 import ( - "go.mongodb.org/atlas/mongodbatlas" + "go.mongodb.org/atlas-sdk/v20231115008/admin" "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/common" ) @@ -27,15 +27,17 @@ type Team struct { Roles []TeamRole `json:"roles"` } -func (in *Team) ToAtlas(teamID string) *mongodbatlas.ProjectTeam { - result := &mongodbatlas.ProjectTeam{ - TeamID: teamID, - RoleNames: make([]string, 0, len(in.Roles)), +func (in *Team) ToAtlas(teamID string) admin.TeamRole { + roleNames := make([]string, 0, len(in.Roles)) + result := admin.TeamRole{ + TeamId: &teamID, + RoleNames: &roleNames, } for _, role := range in.Roles { - result.RoleNames = append(result.RoleNames, string(role)) + roleNames = append(roleNames, string(role)) } + result.SetRoleNames(roleNames) return result } diff --git a/pkg/controller/atlasproject/project_test.go b/pkg/controller/atlasproject/project_test.go index 62ba11bc4a..6fbb5ee3f0 100644 --- a/pkg/controller/atlasproject/project_test.go +++ b/pkg/controller/atlasproject/project_test.go @@ -3,6 +3,7 @@ package atlasproject import ( "context" "errors" + "net/http" "testing" "github.com/google/go-cmp/cmp" @@ -116,13 +117,6 @@ func TestHandleProject(t *testing.T) { "should delete project": { atlasClientMocker: func() *mongodbatlas.Client { projectsMock := &atlasmocks.ProjectsClientMock{ - GetProjectTeamsAssignedFunc: func(projectID string) (*mongodbatlas.TeamsAssigned, *mongodbatlas.Response, error) { - return &mongodbatlas.TeamsAssigned{ - Links: nil, - Results: []*mongodbatlas.Result{}, - TotalCount: 0, - }, nil, nil - }, DeleteFunc: func(projectID string) (*mongodbatlas.Response, error) { return nil, nil }, @@ -147,10 +141,16 @@ func TestHandleProject(t *testing.T) { mockPeeringEndpointAPI.EXPECT(). ListPeeringConnectionsExecute(admin.ListPeeringConnectionsApiRequest{ApiService: mockPeeringEndpointAPI}). Return(&admin.PaginatedContainerPeer{}, nil, nil) + mockTeamAPI := mockadmin.NewTeamsApi(t) + mockTeamAPI.EXPECT().ListProjectTeams(context.Background(), mock.Anything). + Return(admin.ListProjectTeamsApiRequest{ApiService: mockTeamAPI}) + mockTeamAPI.EXPECT().ListProjectTeamsExecute(mock.Anything). + Return(&admin.PaginatedTeamRole{}, &http.Response{}, nil) return &admin.APIClient{ PrivateEndpointServicesApi: mockPrivateEndpointAPI, NetworkPeeringApi: mockPeeringEndpointAPI, + TeamsApi: mockTeamAPI, } }, projectServiceMocker: func() project.ProjectService { @@ -288,11 +288,7 @@ func TestHandleProject(t *testing.T) { return &mongodbatlas.EncryptionAtRest{}, nil, nil }, } - projectAPI := &atlasmocks.ProjectsClientMock{ - GetProjectTeamsAssignedFunc: func(projectID string) (*mongodbatlas.TeamsAssigned, *mongodbatlas.Response, error) { - return &mongodbatlas.TeamsAssigned{}, nil, nil - }, - } + projectAPI := &atlasmocks.ProjectsClientMock{} return &mongodbatlas.Client{ Integrations: integrations, @@ -340,6 +336,11 @@ func TestHandleProject(t *testing.T) { Return(admin.GetDataProtectionSettingsApiRequest{ApiService: backup}) backup.EXPECT().GetDataProtectionSettingsExecute(mock.AnythingOfType("admin.GetDataProtectionSettingsApiRequest")). Return(nil, nil, nil) + mockTeamAPI := mockadmin.NewTeamsApi(t) + mockTeamAPI.EXPECT().ListProjectTeams(context.Background(), mock.Anything). + Return(admin.ListProjectTeamsApiRequest{ApiService: mockTeamAPI}) + mockTeamAPI.EXPECT().ListProjectTeamsExecute(mock.Anything). + Return(nil, &http.Response{}, nil) return &admin.APIClient{ ProjectIPAccessListApi: ipAccessList, @@ -349,6 +350,7 @@ func TestHandleProject(t *testing.T) { CustomDatabaseRolesApi: customRoles, ProjectsApi: projectAPI, CloudBackupsApi: backup, + TeamsApi: mockTeamAPI, } }, projectServiceMocker: func() project.ProjectService { @@ -390,16 +392,10 @@ func TestHandleProject(t *testing.T) { return &mongodbatlas.EncryptionAtRest{}, nil, nil }, } - projectAPI := &atlasmocks.ProjectsClientMock{ - GetProjectTeamsAssignedFunc: func(projectID string) (*mongodbatlas.TeamsAssigned, *mongodbatlas.Response, error) { - return &mongodbatlas.TeamsAssigned{}, nil, nil - }, - } return &mongodbatlas.Client{ Integrations: integrations, EncryptionsAtRest: encryptionAtRest, - Projects: projectAPI, } }, atlasSDKMocker: func() *admin.APIClient { @@ -442,6 +438,11 @@ func TestHandleProject(t *testing.T) { Return(admin.GetDataProtectionSettingsApiRequest{ApiService: backup}) backup.EXPECT().GetDataProtectionSettingsExecute(mock.AnythingOfType("admin.GetDataProtectionSettingsApiRequest")). Return(nil, nil, nil) + mockTeamAPI := mockadmin.NewTeamsApi(t) + mockTeamAPI.EXPECT().ListProjectTeams(context.Background(), mock.Anything). + Return(admin.ListProjectTeamsApiRequest{ApiService: mockTeamAPI}) + mockTeamAPI.EXPECT().ListProjectTeamsExecute(mock.Anything). + Return(nil, &http.Response{}, nil) return &admin.APIClient{ ProjectIPAccessListApi: ipAccessList, @@ -451,6 +452,7 @@ func TestHandleProject(t *testing.T) { CustomDatabaseRolesApi: customRoles, ProjectsApi: projectAPI, CloudBackupsApi: backup, + TeamsApi: mockTeamAPI, } }, projectServiceMocker: func() project.ProjectService { @@ -494,16 +496,10 @@ func TestHandleProject(t *testing.T) { return &mongodbatlas.EncryptionAtRest{}, nil, nil }, } - projectAPI := &atlasmocks.ProjectsClientMock{ - GetProjectTeamsAssignedFunc: func(projectID string) (*mongodbatlas.TeamsAssigned, *mongodbatlas.Response, error) { - return &mongodbatlas.TeamsAssigned{}, nil, nil - }, - } return &mongodbatlas.Client{ Integrations: integrations, EncryptionsAtRest: encryptionAtRest, - Projects: projectAPI, } }, atlasSDKMocker: func() *admin.APIClient { @@ -546,6 +542,11 @@ func TestHandleProject(t *testing.T) { Return(admin.GetDataProtectionSettingsApiRequest{ApiService: backup}) backup.EXPECT().GetDataProtectionSettingsExecute(mock.AnythingOfType("admin.GetDataProtectionSettingsApiRequest")). Return(nil, nil, nil) + mockTeamAPI := mockadmin.NewTeamsApi(t) + mockTeamAPI.EXPECT().ListProjectTeams(context.Background(), mock.Anything). + Return(admin.ListProjectTeamsApiRequest{ApiService: mockTeamAPI}) + mockTeamAPI.EXPECT().ListProjectTeamsExecute(mock.Anything). + Return(nil, &http.Response{}, nil) return &admin.APIClient{ ProjectIPAccessListApi: ipAccessList, @@ -555,6 +556,7 @@ func TestHandleProject(t *testing.T) { CustomDatabaseRolesApi: customRoles, ProjectsApi: projectAPI, CloudBackupsApi: backup, + TeamsApi: mockTeamAPI, } }, projectServiceMocker: func() project.ProjectService { @@ -938,41 +940,13 @@ func TestDelete(t *testing.T) { "should update team status when project is deleted": { atlasClientMocker: func() *mongodbatlas.Client { projectsMock := &atlasmocks.ProjectsClientMock{ - GetProjectTeamsAssignedFunc: func(projectID string) (*mongodbatlas.TeamsAssigned, *mongodbatlas.Response, error) { - return &mongodbatlas.TeamsAssigned{ - Links: nil, - Results: []*mongodbatlas.Result{ - { - Links: nil, - RoleNames: nil, - TeamID: teamID, - }, - }, - TotalCount: 0, - }, nil, nil - }, DeleteFunc: func(projectID string) (*mongodbatlas.Response, error) { return nil, nil }, } - teamsMock := &atlasmocks.TeamsClientMock{ - ListFunc: func(orgID string) ([]mongodbatlas.Team, *mongodbatlas.Response, error) { - return []mongodbatlas.Team{ - { - ID: teamID, - Name: teamName, - Usernames: nil, - }, - }, nil, nil - }, - RemoveTeamFromProjectFunc: func(projectID string, teamID string) (*mongodbatlas.Response, error) { - return nil, nil - }, - } return &mongodbatlas.Client{ Projects: projectsMock, - Teams: teamsMock, } }, atlasSDKMocker: func() *admin.APIClient { @@ -990,10 +964,15 @@ func TestDelete(t *testing.T) { mockPeeringEndpointAPI.EXPECT(). ListPeeringConnectionsExecute(admin.ListPeeringConnectionsApiRequest{ApiService: mockPeeringEndpointAPI}). Return(&admin.PaginatedContainerPeer{}, nil, nil) - + mockTeamAPI := mockadmin.NewTeamsApi(t) + mockTeamAPI.EXPECT().ListProjectTeams(context.Background(), mock.Anything). + Return(admin.ListProjectTeamsApiRequest{ApiService: mockTeamAPI}) + mockTeamAPI.EXPECT().ListProjectTeamsExecute(mock.Anything). + Return(nil, &http.Response{}, nil) return &admin.APIClient{ PrivateEndpointServicesApi: mockPrivateEndpointAPI, NetworkPeeringApi: mockPeeringEndpointAPI, + TeamsApi: mockTeamAPI, } }, projectServiceMocker: func() project.ProjectService { diff --git a/pkg/controller/atlasproject/team_reconciler.go b/pkg/controller/atlasproject/team_reconciler.go index 199e44879e..1979bfdd93 100644 --- a/pkg/controller/atlasproject/team_reconciler.go +++ b/pkg/controller/atlasproject/team_reconciler.go @@ -7,13 +7,13 @@ import ( "net/http" "sync" + "github.com/google/go-cmp/cmp" + "go.mongodb.org/atlas-sdk/v20231115008/admin" "go.mongodb.org/atlas/mongodbatlas" "golang.org/x/sync/errgroup" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/google/go-cmp/cmp" - "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/status" @@ -58,14 +58,14 @@ func (r *AtlasProjectReconciler) teamReconcile( return result.ReconcileResult(), nil } - atlasClient, orgID, err := r.AtlasProvider.Client(teamCtx.Context, connectionSecretKey, log) + atlasClient, orgID, err := r.AtlasProvider.SdkClient(teamCtx.Context, connectionSecretKey, log) if err != nil { result := workflow.Terminate(workflow.AtlasAPIAccessNotConfigured, err.Error()) setCondition(teamCtx, api.ReadyType, result) return result.ReconcileResult(), nil } teamCtx.OrgID = orgID - teamCtx.Client = atlasClient + teamCtx.SdkClient = atlasClient teamID, result := ensureTeamState(teamCtx, team) if !result.IsOk() { @@ -109,7 +109,7 @@ func (r *AtlasProjectReconciler) teamReconcile( return workflow.OK().ReconcileResult(), nil } else { log.Infow("-> Starting AtlasTeam deletion", "spec", team.Spec) - _, err := teamCtx.Client.Teams.RemoveTeamFromOrganization(teamCtx.Context, orgID, team.Status.ID) + _, _, err := teamCtx.SdkClient.TeamsApi.DeleteTeam(teamCtx.Context, orgID, team.Status.ID).Execute() var apiError *mongodbatlas.ErrorResponse if errors.As(err, &apiError) && apiError.ErrorCode == atlas.NotInGroup { log.Infow("team does not exist", "projectID", team.Status.ID) @@ -133,30 +133,30 @@ func (r *AtlasProjectReconciler) teamReconcile( } func ensureTeamState(workflowCtx *workflow.Context, team *akov2.AtlasTeam) (string, workflow.Result) { - var atlasTeam *mongodbatlas.Team + var atlasTeamResponse *admin.TeamResponse var err error if team.Status.ID != "" { - atlasTeam, err = fetchTeamByID(workflowCtx, team.Status.ID) + atlasTeamResponse, err = fetchTeamByID(workflowCtx, team.Status.ID) if err != nil { return "", workflow.Terminate(workflow.TeamNotCreatedInAtlas, err.Error()) } - atlasTeam, err = renameTeam(workflowCtx, atlasTeam, team.Spec.Name) + atlasTeamResponse, err = renameTeam(workflowCtx, atlasTeamResponse, team.Spec.Name) if err != nil { return "", workflow.Terminate(workflow.TeamNotUpdatedInAtlas, err.Error()) } - return atlasTeam.ID, workflow.OK() + return atlasTeamResponse.GetId(), workflow.OK() } - atlasTeam, err = fetchTeamByName(workflowCtx, team.Spec.Name) + atlasTeamResponse, err = fetchTeamByName(workflowCtx, team.Spec.Name) if err != nil { return "", workflow.Terminate(workflow.TeamNotCreatedInAtlas, err.Error()) } - if atlasTeam == nil { - atlasTeam, err = team.ToAtlas() + if atlasTeamResponse == nil { + atlasTeam, err := team.ToAtlas() if err != nil { return "", workflow.Terminate(workflow.TeamInvalidSpec, err.Error()) } @@ -165,18 +165,20 @@ func ensureTeamState(workflowCtx *workflow.Context, team *akov2.AtlasTeam) (stri if err != nil { return "", workflow.Terminate(workflow.TeamNotCreatedInAtlas, err.Error()) } + + return atlasTeam.GetId(), workflow.OK() } - atlasTeam, err = renameTeam(workflowCtx, atlasTeam, team.Spec.Name) + atlasTeamResponse, err = renameTeam(workflowCtx, atlasTeamResponse, team.Spec.Name) if err != nil { return "", workflow.Terminate(workflow.TeamNotUpdatedInAtlas, err.Error()) } - return atlasTeam.ID, workflow.OK() + return atlasTeamResponse.GetId(), workflow.OK() } func ensureTeamUsersAreInSync(workflowCtx *workflow.Context, teamID string, team *akov2.AtlasTeam) workflow.Result { - atlasUsers, _, err := workflowCtx.Client.Teams.GetTeamUsersAssigned(workflowCtx.Context, workflowCtx.OrgID, teamID) + atlasUsers, _, err := workflowCtx.SdkClient.TeamsApi.ListTeamUsers(workflowCtx.Context, workflowCtx.OrgID, teamID).Execute() if err != nil { return workflow.Terminate(workflow.TeamUsersNotReady, err.Error()) } @@ -186,22 +188,22 @@ func ensureTeamUsersAreInSync(workflowCtx *workflow.Context, teamID string, team usernamesMap[string(username)] = struct{}{} } - atlasUsernamesMap := map[string]mongodbatlas.AtlasUser{} - for _, atlasUser := range atlasUsers { + atlasUsernamesMap := map[string]admin.CloudAppUser{} + for _, atlasUser := range atlasUsers.GetResults() { atlasUsernamesMap[atlasUser.Username] = atlasUser } g, taskContext := errgroup.WithContext(workflowCtx.Context) - for i := range atlasUsers { - user := atlasUsers[i] - if _, ok := usernamesMap[atlasUsers[i].Username]; !ok { - g.Go(func() error { - workflowCtx.Log.Debugf("removing user %s from team %s", user.ID, teamID) - _, err := workflowCtx.Client.Teams.RemoveUserToTeam(taskContext, workflowCtx.OrgID, teamID, user.ID) - - return err - }) + if atlasUsers.Results != nil { + for _, user := range atlasUsers.GetResults() { + if _, ok := usernamesMap[user.Username]; !ok { + g.Go(func() error { + workflowCtx.Log.Debugf("removing user %s from team %s", user.GetId(), teamID) + _, err := workflowCtx.SdkClient.TeamsApi.RemoveTeamUser(taskContext, workflowCtx.OrgID, teamID, user.GetId()).Execute() + return err + }) + } } } @@ -212,20 +214,20 @@ func ensureTeamUsersAreInSync(workflowCtx *workflow.Context, teamID string, team } g, taskContext = errgroup.WithContext(workflowCtx.Context) - toAdd := make([]string, 0, len(team.Spec.Usernames)) + toAdd := make([]admin.AddUserToTeam, 0, len(team.Spec.Usernames)) lock := sync.Mutex{} for i := range team.Spec.Usernames { username := team.Spec.Usernames[i] if _, ok := atlasUsernamesMap[string(username)]; !ok { g.Go(func() error { - user, _, err := workflowCtx.Client.AtlasUsers.GetByName(taskContext, string(username)) + user, _, err := workflowCtx.SdkClient.MongoDBCloudUsersApi.GetUserByUsername(taskContext, string(username)).Execute() if err != nil { return err } lock.Lock() - toAdd = append(toAdd, user.ID) + toAdd = append(toAdd, admin.AddUserToTeam{Id: user.GetId()}) lock.Unlock() return nil @@ -244,7 +246,7 @@ func ensureTeamUsersAreInSync(workflowCtx *workflow.Context, teamID string, team } workflowCtx.Log.Debugf("Adding users to team %s", teamID) - _, _, err = workflowCtx.Client.Teams.AddUsersToTeam(workflowCtx.Context, workflowCtx.OrgID, teamID, toAdd) + _, _, err = workflowCtx.SdkClient.TeamsApi.AddTeamUser(workflowCtx.Context, workflowCtx.OrgID, teamID, &toAdd).Execute() if err != nil { return workflow.Terminate(workflow.TeamUsersNotReady, err.Error()) } @@ -252,19 +254,19 @@ func ensureTeamUsersAreInSync(workflowCtx *workflow.Context, teamID string, team return workflow.OK() } -func fetchTeamByID(workflowCtx *workflow.Context, teamID string) (*mongodbatlas.Team, error) { +func fetchTeamByID(workflowCtx *workflow.Context, teamID string) (*admin.TeamResponse, error) { workflowCtx.Log.Debugf("fetching team %s from atlas", teamID) - atlasTeam, _, err := workflowCtx.Client.Teams.Get(workflowCtx.Context, workflowCtx.OrgID, teamID) + atlasTeamResponse, _, err := workflowCtx.SdkClient.TeamsApi.GetTeamById(workflowCtx.Context, workflowCtx.OrgID, teamID).Execute() if err != nil { return nil, err } - return atlasTeam, nil + return atlasTeamResponse, nil } -func fetchTeamByName(workflowCtx *workflow.Context, teamName string) (*mongodbatlas.Team, error) { +func fetchTeamByName(workflowCtx *workflow.Context, teamName string) (*admin.TeamResponse, error) { workflowCtx.Log.Debugf("fetching team named %s from atlas", teamName) - atlasTeam, resp, err := workflowCtx.Client.Teams.GetOneTeamByName(workflowCtx.Context, workflowCtx.OrgID, teamName) + atlasTeamResponse, resp, err := workflowCtx.SdkClient.TeamsApi.GetTeamByName(workflowCtx.Context, workflowCtx.OrgID, teamName).Execute() if err != nil { if resp.StatusCode == http.StatusNotFound { return nil, nil @@ -273,31 +275,31 @@ func fetchTeamByName(workflowCtx *workflow.Context, teamName string) (*mongodbat return nil, err } - return atlasTeam, nil + return atlasTeamResponse, nil } -func createTeam(workflowCtx *workflow.Context, atlasTeam *mongodbatlas.Team) (*mongodbatlas.Team, error) { +func createTeam(workflowCtx *workflow.Context, atlasTeam *admin.Team) (*admin.Team, error) { workflowCtx.Log.Debugf("create team named %s in atlas", atlasTeam.Name) - atlasTeam, _, err := workflowCtx.Client.Teams.Create(workflowCtx.Context, workflowCtx.OrgID, atlasTeam) + atlasTeam, _, err := workflowCtx.SdkClient.TeamsApi.CreateTeam(workflowCtx.Context, workflowCtx.OrgID, atlasTeam).Execute() if err != nil { return nil, err } - return atlasTeam, nil } -func renameTeam(workflowCtx *workflow.Context, atlasTeam *mongodbatlas.Team, newName string) (*mongodbatlas.Team, error) { - if atlasTeam.Name == newName { - return atlasTeam, nil +func renameTeam(workflowCtx *workflow.Context, atlasTeamResponse *admin.TeamResponse, newName string) (*admin.TeamResponse, error) { + if atlasTeamResponse.GetName() == newName { + return atlasTeamResponse, nil } - workflowCtx.Log.Debugf("updating name of team %s in atlas", atlasTeam.ID) - atlasTeam, _, err := workflowCtx.Client.Teams.Rename(workflowCtx.Context, workflowCtx.OrgID, atlasTeam.ID, newName) + workflowCtx.Log.Debugf("updating name of team %s in atlas", atlasTeamResponse.GetId()) + teamUpdate := admin.TeamUpdate{Name: newName} + atlasTeamResponse, _, err := workflowCtx.SdkClient.TeamsApi.RenameTeam(workflowCtx.Context, workflowCtx.OrgID, atlasTeamResponse.GetId(), &teamUpdate).Execute() if err != nil { return nil, err } - return atlasTeam, nil + return atlasTeamResponse, nil } func teamsManagedByAtlas(workflowCtx *workflow.Context) customresource.AtlasChecker { @@ -311,7 +313,7 @@ func teamsManagedByAtlas(workflowCtx *workflow.Context) customresource.AtlasChec return false, nil } - atlasTeam, _, err := workflowCtx.Client.Teams.Get(workflowCtx.Context, workflowCtx.OrgID, team.Status.ID) + atlasTeam, _, err := workflowCtx.SdkClient.TeamsApi.GetTeamById(workflowCtx.Context, workflowCtx.OrgID, team.Status.ID).Execute() if err != nil { var apiError *mongodbatlas.ErrorResponse if errors.As(err, &apiError) && (apiError.ErrorCode == atlas.NotInGroup || apiError.ErrorCode == atlas.ResourceNotFound) { @@ -321,7 +323,12 @@ func teamsManagedByAtlas(workflowCtx *workflow.Context) customresource.AtlasChec return false, err } - if team.Spec.Name != atlasTeam.Name || len(atlasTeam.Usernames) == 0 { + atlasTeamUsers, _, err := workflowCtx.SdkClient.TeamsApi.ListTeamUsers(workflowCtx.Context, workflowCtx.OrgID, atlasTeam.GetName()).Execute() + if err != nil { + return false, err + } + + if team.Spec.Name != atlasTeam.GetName() || len(atlasTeamUsers.GetResults()) == 0 { return false, err } @@ -330,6 +337,11 @@ func teamsManagedByAtlas(workflowCtx *workflow.Context) customresource.AtlasChec usernames = append(usernames, string(username)) } - return cmp.Diff(usernames, atlasTeam.Usernames) != "", nil + atlasUsernames := make([]string, 0, len(atlasTeamUsers.GetResults())) + for _, user := range atlasTeamUsers.GetResults() { + atlasUsernames = append(atlasUsernames, user.Username) + } + + return cmp.Diff(usernames, atlasUsernames) != "", nil } } diff --git a/pkg/controller/atlasproject/team_reconciler_test.go b/pkg/controller/atlasproject/team_reconciler_test.go index 5747727569..7de2fb7086 100644 --- a/pkg/controller/atlasproject/team_reconciler_test.go +++ b/pkg/controller/atlasproject/team_reconciler_test.go @@ -3,12 +3,15 @@ package atlasproject import ( "context" "errors" + "net/http" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "go.mongodb.org/atlas-sdk/v20231115008/admin" + "go.mongodb.org/atlas-sdk/v20231115008/mockadmin" "go.mongodb.org/atlas/mongodbatlas" - atlasmock "github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/mocks/atlas" akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1" "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/status" "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/atlas" @@ -18,9 +21,9 @@ import ( func TestTeamManagedByAtlas(t *testing.T) { t.Run("should return error when passing wrong resource", func(t *testing.T) { workflowCtx := &workflow.Context{ - OrgID: "orgID", - Client: &mongodbatlas.Client{}, - Context: context.Background(), + OrgID: "orgID", + SdkClient: &admin.APIClient{}, + Context: context.Background(), } checker := teamsManagedByAtlas(workflowCtx) result, err := checker(&akov2.AtlasProject{}) @@ -30,9 +33,9 @@ func TestTeamManagedByAtlas(t *testing.T) { t.Run("should return false when resource has no Atlas Team ID", func(t *testing.T) { workflowCtx := &workflow.Context{ - OrgID: "orgID", - Client: &mongodbatlas.Client{}, - Context: context.Background(), + OrgID: "orgID", + SdkClient: &admin.APIClient{}, + Context: context.Background(), } checker := teamsManagedByAtlas(workflowCtx) result, err := checker(&akov2.AtlasTeam{}) @@ -41,12 +44,15 @@ func TestTeamManagedByAtlas(t *testing.T) { }) t.Run("should return false when resource was not found in Atlas", func(t *testing.T) { - atlasClient := mongodbatlas.Client{ - Teams: &atlasmock.TeamsClientMock{ - GetFunc: func(orgID string, teamID string) (*mongodbatlas.Team, *mongodbatlas.Response, error) { - return nil, &mongodbatlas.Response{}, &mongodbatlas.ErrorResponse{ErrorCode: atlas.ResourceNotFound} - }, - }, + atlasClient := admin.APIClient{ + TeamsApi: func() *mockadmin.TeamsApi { + TeamsAPI := mockadmin.NewTeamsApi(t) + TeamsAPI.EXPECT().GetTeamById(context.Background(), "orgID", "team-id-1"). + Return(admin.GetTeamByIdApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().GetTeamByIdExecute(mock.Anything). + Return(nil, &http.Response{}, &mongodbatlas.ErrorResponse{ErrorCode: atlas.ResourceNotFound}) + return TeamsAPI + }(), } team := &akov2.AtlasTeam{ Status: status.TeamStatus{ @@ -54,9 +60,9 @@ func TestTeamManagedByAtlas(t *testing.T) { }, } workflowCtx := &workflow.Context{ - OrgID: "orgID", - Client: &atlasClient, - Context: context.Background(), + OrgID: "orgID", + SdkClient: &atlasClient, + Context: context.Background(), } checker := teamsManagedByAtlas(workflowCtx) result, err := checker(team) @@ -65,12 +71,15 @@ func TestTeamManagedByAtlas(t *testing.T) { }) t.Run("should return error when failed to fetch the team from Atlas", func(t *testing.T) { - atlasClient := mongodbatlas.Client{ - Teams: &atlasmock.TeamsClientMock{ - GetFunc: func(orgID string, teamID string) (*mongodbatlas.Team, *mongodbatlas.Response, error) { - return nil, &mongodbatlas.Response{}, errors.New("unavailable") - }, - }, + atlasClient := admin.APIClient{ + TeamsApi: func() *mockadmin.TeamsApi { + TeamsAPI := mockadmin.NewTeamsApi(t) + TeamsAPI.EXPECT().GetTeamById(context.Background(), "orgID", "team-id-1"). + Return(admin.GetTeamByIdApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().GetTeamByIdExecute(mock.Anything). + Return(nil, &http.Response{}, errors.New("unavailable")) + return TeamsAPI + }(), } team := &akov2.AtlasTeam{ Status: status.TeamStatus{ @@ -78,9 +87,9 @@ func TestTeamManagedByAtlas(t *testing.T) { }, } workflowCtx := &workflow.Context{ - OrgID: "orgID", - Client: &atlasClient, - Context: context.Background(), + OrgID: "orgID", + SdkClient: &atlasClient, + Context: context.Background(), } checker := teamsManagedByAtlas(workflowCtx) result, err := checker(team) @@ -89,16 +98,34 @@ func TestTeamManagedByAtlas(t *testing.T) { }) t.Run("should return false when resource are equal", func(t *testing.T) { - atlasClient := mongodbatlas.Client{ - Teams: &atlasmock.TeamsClientMock{ - GetFunc: func(orgID string, teamID string) (*mongodbatlas.Team, *mongodbatlas.Response, error) { - return &mongodbatlas.Team{ - ID: "team-id-1", - Name: "My Team", - Usernames: []string{"user1@mongodb.com", "user2@mongodb.com"}, - }, &mongodbatlas.Response{}, nil - }, - }, + atlasClient := admin.APIClient{ + TeamsApi: func() *mockadmin.TeamsApi { + TeamsAPI := mockadmin.NewTeamsApi(t) + TeamsAPI.EXPECT().GetTeamById(context.Background(), "orgID-1", "team-id-1"). + Return(admin.GetTeamByIdApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().GetTeamByIdExecute(mock.Anything). + Return(&admin.TeamResponse{ + Id: func(s string) *string { return &s }("team-id-1"), + Links: nil, + Name: func(s string) *string { return &s }("My Team"), + }, &http.Response{}, nil) + TeamsAPI.EXPECT().ListTeamUsers(context.Background(), "orgID-1", "My Team"). + Return(admin.ListTeamUsersApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().ListTeamUsersExecute(mock.Anything). + Return(&admin.PaginatedApiAppUser{ + Links: nil, + Results: &[]admin.CloudAppUser{ + { + Username: "user1@mongodb.com", + }, + { + Username: "user2@mongodb.com", + }, + }, + TotalCount: nil, + }, &http.Response{}, nil) + return TeamsAPI + }(), } team := &akov2.AtlasTeam{ Spec: akov2.TeamSpec{ @@ -110,9 +137,9 @@ func TestTeamManagedByAtlas(t *testing.T) { }, } workflowCtx := &workflow.Context{ - OrgID: "orgID-1", - Client: &atlasClient, - Context: context.Background(), + OrgID: "orgID-1", + SdkClient: &atlasClient, + Context: context.Background(), } checker := teamsManagedByAtlas(workflowCtx) result, err := checker(team) @@ -121,16 +148,34 @@ func TestTeamManagedByAtlas(t *testing.T) { }) t.Run("should return true when resource are different", func(t *testing.T) { - atlasClient := mongodbatlas.Client{ - Teams: &atlasmock.TeamsClientMock{ - GetFunc: func(orgID string, teamID string) (*mongodbatlas.Team, *mongodbatlas.Response, error) { - return &mongodbatlas.Team{ - ID: "team-id-1", - Name: "My Team", - Usernames: []string{"user1@mongodb.com", "user2@mongodb.com"}, - }, &mongodbatlas.Response{}, nil - }, - }, + atlasClient := admin.APIClient{ + TeamsApi: func() *mockadmin.TeamsApi { + TeamsAPI := mockadmin.NewTeamsApi(t) + TeamsAPI.EXPECT().GetTeamById(context.Background(), "orgID-1", "team-id-1"). + Return(admin.GetTeamByIdApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().GetTeamByIdExecute(mock.Anything). + Return(&admin.TeamResponse{ + Id: func(s string) *string { return &s }("team-id-1"), + Links: nil, + Name: func(s string) *string { return &s }("My Team"), + }, &http.Response{}, nil) + TeamsAPI.EXPECT().ListTeamUsers(context.Background(), "orgID-1", "My Team"). + Return(admin.ListTeamUsersApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().ListTeamUsersExecute(mock.Anything). + Return(&admin.PaginatedApiAppUser{ + Links: nil, + Results: &[]admin.CloudAppUser{ + { + Username: "user1@mongodb.com", + }, + { + Username: "user2@mongodb.com", + }, + }, + TotalCount: nil, + }, &http.Response{}, nil) + return TeamsAPI + }(), } team := &akov2.AtlasTeam{ Spec: akov2.TeamSpec{ @@ -142,9 +187,9 @@ func TestTeamManagedByAtlas(t *testing.T) { }, } workflowCtx := &workflow.Context{ - OrgID: "orgID-1", - Client: &atlasClient, - Context: context.Background(), + OrgID: "orgID-1", + SdkClient: &atlasClient, + Context: context.Background(), } checker := teamsManagedByAtlas(workflowCtx) result, err := checker(team) diff --git a/pkg/controller/atlasproject/teams.go b/pkg/controller/atlasproject/teams.go index 6dd746cccc..9c33275b02 100644 --- a/pkg/controller/atlasproject/teams.go +++ b/pkg/controller/atlasproject/teams.go @@ -1,7 +1,7 @@ package atlasproject import ( - "go.mongodb.org/atlas/mongodbatlas" + "go.mongodb.org/atlas-sdk/v20231115008/admin" "k8s.io/apimachinery/pkg/types" controllerruntime "sigs.k8s.io/controller-runtime" @@ -67,7 +67,7 @@ func (r *AtlasProjectReconciler) ensureAssignedTeams(workflowCtx *workflow.Conte func (r *AtlasProjectReconciler) syncAssignedTeams(ctx *workflow.Context, projectID string, project *akov2.AtlasProject, teamsToAssign map[string]*akov2.Team) error { ctx.Log.Debug("fetching assigned teams from atlas") - atlasAssignedTeams, _, err := ctx.Client.Projects.GetProjectTeamsAssigned(ctx.Context, projectID) + atlasAssignedTeams, _, err := ctx.SdkClient.TeamsApi.ListProjectTeams(ctx.Context, projectID).Execute() if err != nil { return err } @@ -80,54 +80,62 @@ func (r *AtlasProjectReconciler) syncAssignedTeams(ctx *workflow.Context, projec defer statushandler.Update(ctx, r.Client, r.EventRecorder, project) - toDelete := make([]*mongodbatlas.Result, 0, len(atlasAssignedTeams.Results)) - for _, atlasAssignedTeam := range atlasAssignedTeams.Results { - desiredTeam, ok := teamsToAssign[atlasAssignedTeam.TeamID] - if !ok { - toDelete = append(toDelete, atlasAssignedTeam) + toDelete := make([]*admin.TeamRole, 0, len(atlasAssignedTeams.GetResults())) - continue - } + if atlasAssignedTeams != nil && atlasAssignedTeams.Results != nil { + for _, atlasAssignedTeam := range atlasAssignedTeams.GetResults() { + if atlasAssignedTeam.GetTeamId() == "" { + continue + } - if !hasTeamRolesChanged(atlasAssignedTeam.RoleNames, desiredTeam.Roles) { - currentProjectsStatus[atlasAssignedTeam.TeamID] = status.ProjectTeamStatus{ - ID: atlasAssignedTeam.TeamID, - TeamRef: desiredTeam.TeamRef, + desiredTeam, ok := teamsToAssign[atlasAssignedTeam.GetTeamId()] + if !ok { + result := atlasAssignedTeam + toDelete = append(toDelete, &result) + + continue } - delete(teamsToAssign, atlasAssignedTeam.TeamID) - continue - } + if !hasTeamRolesChanged(atlasAssignedTeam.GetRoleNames(), desiredTeam.Roles) { + currentProjectsStatus[atlasAssignedTeam.GetTeamId()] = status.ProjectTeamStatus{ + ID: atlasAssignedTeam.GetTeamId(), + TeamRef: desiredTeam.TeamRef, + } + delete(teamsToAssign, atlasAssignedTeam.GetTeamId()) - ctx.Log.Debugf("removing team %s from project for later update", atlasAssignedTeam.TeamID) - _, err = ctx.Client.Teams.RemoveTeamFromProject(ctx.Context, projectID, atlasAssignedTeam.TeamID) - if err != nil { - ctx.Log.Warnf("failed to remove team %s from project: %s", atlasAssignedTeam.TeamID, err.Error()) + continue + } + + ctx.Log.Debugf("removing team %s from project for later update", atlasAssignedTeam.GetTeamId()) + _, err = ctx.SdkClient.TeamsApi.RemoveProjectTeam(ctx.Context, projectID, atlasAssignedTeam.GetTeamId()).Execute() + if err != nil { + ctx.Log.Warnf("failed to remove team %s from project: %s", atlasAssignedTeam.GetTeamId(), err.Error()) + } } } for _, atlasAssignedTeam := range toDelete { - ctx.Log.Debugf("removing team %s from project", atlasAssignedTeam.TeamID) - _, err = ctx.Client.Teams.RemoveTeamFromProject(ctx.Context, projectID, atlasAssignedTeam.TeamID) + ctx.Log.Debugf("removing team %s from project", atlasAssignedTeam.GetTeamId()) + _, err = ctx.SdkClient.TeamsApi.RemoveProjectTeam(ctx.Context, projectID, atlasAssignedTeam.GetTeamId()).Execute() if err != nil { - ctx.Log.Warnf("failed to remove team %s from project: %s", atlasAssignedTeam.TeamID, err.Error()) + ctx.Log.Warnf("failed to remove team %s from project: %s", atlasAssignedTeam.GetTeamId(), err.Error()) } - teamRef := getTeamRefFromProjectStatus(project, atlasAssignedTeam.TeamID) + teamRef := getTeamRefFromProjectStatus(project, atlasAssignedTeam.GetTeamId()) if teamRef == nil { - ctx.Log.Warnf("unable to find team %s status in the project", atlasAssignedTeam.TeamID) + ctx.Log.Warnf("unable to find team %s status in the project", atlasAssignedTeam.GetTeamId()) } else { if err = r.updateTeamState(ctx, project, teamRef, true); err != nil { - ctx.Log.Warnf("failed to update team %s status with removed project: %s", atlasAssignedTeam.TeamID, err.Error()) + ctx.Log.Warnf("failed to update team %s status with removed project: %s", atlasAssignedTeam.GetTeamId(), err.Error()) } } - delete(currentProjectsStatus, atlasAssignedTeam.TeamID) + delete(currentProjectsStatus, atlasAssignedTeam.GetTeamId()) } if len(teamsToAssign) > 0 { ctx.Log.Debug("assigning teams to project") - projectTeams := make([]*mongodbatlas.ProjectTeam, 0, len(teamsToAssign)) + projectTeams := make([]admin.TeamRole, 0, len(teamsToAssign)) for teamID := range teamsToAssign { assignedTeam := teamsToAssign[teamID] projectTeams = append(projectTeams, assignedTeam.ToAtlas(teamID)) @@ -141,7 +149,7 @@ func (r *AtlasProjectReconciler) syncAssignedTeams(ctx *workflow.Context, projec } } - _, _, err = ctx.Client.Projects.AddTeamsToProject(ctx.Context, projectID, projectTeams) + _, _, err = ctx.SdkClient.TeamsApi.AddAllTeamsToProject(ctx.Context, projectID, &projectTeams).Execute() if err != nil { return err } diff --git a/pkg/controller/atlasproject/teams_test.go b/pkg/controller/atlasproject/teams_test.go index 29e6a6dc0a..df337bc22d 100644 --- a/pkg/controller/atlasproject/teams_test.go +++ b/pkg/controller/atlasproject/teams_test.go @@ -2,10 +2,13 @@ package atlasproject import ( "context" + "net/http" "testing" "github.com/stretchr/testify/assert" - "go.mongodb.org/atlas/mongodbatlas" + "github.com/stretchr/testify/mock" + "go.mongodb.org/atlas-sdk/v20231115008/admin" + "go.mongodb.org/atlas-sdk/v20231115008/mockadmin" "go.uber.org/zap/zaptest" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -14,7 +17,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - atlasmocks "github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/mocks/atlas" 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" @@ -148,65 +150,52 @@ func TestSyncAssignedTeams(t *testing.T) { WithStatusSubresource(project, team1, team2, team3). Build() - atlasClient := &mongodbatlas.Client{ - Projects: &atlasmocks.ProjectsClientMock{ - GetProjectTeamsAssignedFunc: func(projectID string) (*mongodbatlas.TeamsAssigned, *mongodbatlas.Response, error) { - return &mongodbatlas.TeamsAssigned{ + atlasClient := &admin.APIClient{ + TeamsApi: func() *mockadmin.TeamsApi { + TeamsAPI := mockadmin.NewTeamsApi(t) + TeamsAPI.EXPECT().ListProjectTeams(nil, "projectID"). + Return(admin.ListProjectTeamsApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().ListProjectTeamsExecute(mock.Anything). + Return(&admin.PaginatedTeamRole{ Links: nil, - Results: []*mongodbatlas.Result{ + Results: &[]admin.TeamRole{ { Links: nil, - RoleNames: []string{"GROUP_OWNER"}, - TeamID: "teamID_1", + RoleNames: &[]string{"GROUP_OWNER"}, + TeamId: &team1.Status.ID, }, { Links: nil, - RoleNames: []string{"GROUP_OWNER"}, - TeamID: "teamID_2", + RoleNames: &[]string{"GROUP_OWNER"}, + TeamId: &team2.Status.ID, }, { Links: nil, - RoleNames: []string{"GROUP_READ_ONLY"}, - TeamID: "teamID_3", + RoleNames: &[]string{"GROUP_READ_ONLY"}, + TeamId: &team3.Status.ID, }, }, - TotalCount: 0, - }, nil, nil - }, - AddTeamsToProjectFunc: func(projectId string, teams []*mongodbatlas.ProjectTeam) (*mongodbatlas.TeamsAssigned, *mongodbatlas.Response, error) { - return &mongodbatlas.TeamsAssigned{}, nil, nil - }, - }, - Teams: &atlasmocks.TeamsClientMock{ - ListFunc: func(orgID string) ([]mongodbatlas.Team, *mongodbatlas.Response, error) { - return []mongodbatlas.Team{ - { - ID: "teamID_1", - Name: "teamName_1", - Usernames: nil, - }, - { - ID: "teamID_2", - Name: "teamName_2", - Usernames: nil, - }, - { - ID: "teamID_3", - Name: "teamName_3", - Usernames: nil, - }, - }, nil, nil - }, - RemoveTeamFromProjectFunc: func(projectID string, teamID string) (*mongodbatlas.Response, error) { - return nil, nil - }, - }, + TotalCount: nil, + }, &http.Response{}, nil) + TeamsAPI.EXPECT().RemoveProjectTeam(nil, "projectID", "teamID_2"). + Return(admin.RemoveProjectTeamApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().RemoveProjectTeamExecute(mock.Anything). + Return(nil, nil) + TeamsAPI.EXPECT().RemoveProjectTeam(nil, "projectID", "teamID_3"). + Return(admin.RemoveProjectTeamApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().RemoveProjectTeamExecute(mock.Anything). + Return(nil, nil) + TeamsAPI.EXPECT().AddAllTeamsToProject(nil, "projectID", &[]admin.TeamRole{{Links: nil, RoleNames: &[]string{"GROUP_READ_ONLY"}, TeamId: &team2.Status.ID}}). + Return(admin.AddAllTeamsToProjectApiRequest{ApiService: TeamsAPI}) + TeamsAPI.EXPECT().AddAllTeamsToProjectExecute(mock.Anything).Return(&admin.PaginatedTeamRole{}, &http.Response{}, nil) + return TeamsAPI + }(), } logger := zaptest.NewLogger(t).Sugar() ctx := &workflow.Context{ - Log: logger, - Client: atlasClient, + Log: logger, + SdkClient: atlasClient, } r := &AtlasProjectReconciler{ Client: k8sClient,