Skip to content

Commit

Permalink
CLOUDP-293113: Fixed unintended removals of custom roles (#2023)
Browse files Browse the repository at this point in the history 
Fixed unintended removals of custom roles
  • Loading branch information
@igor-karpukhin
igor-karpukhin authored Jan 9, 2025
1 parent 5d3a4db commit 298ca47
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 20 deletions.
1 change: 0 additions & 1 deletion api/v1/atlascustomrole_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ func init() {
// +kubebuilder:object:root=true
// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`
// +kubebuilder:printcolumn:name="Name",type=string,JSONPath=`.spec.role.name`
// +kubebuilder:printcolumn:name="Project ID",type=string,JSONPath=`.spec.projectIDRef.id`
// +kubebuilder:subresource:status
// +groupName:=atlas.mongodb.com
// +kubebuilder:resource:categories=atlas,shortName=acr
Expand Down
33 changes: 14 additions & 19 deletions internal/controller/atlasproject/custom_roles.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,20 @@ func getLastAppliedCustomRoles(atlasProject *akov2.AtlasProject) ([]akov2.Custom
return lastAppliedSpec.CustomRoles, nil
}

func findRolesToDelete(prevSpec, atlasRoles []customroles.CustomRole) map[string]customroles.CustomRole {
func findRolesToDelete(prevSpec, akoRoles, atlasRoles []customroles.CustomRole) map[string]customroles.CustomRole {
result := map[string]customroles.CustomRole{}
for atlasRoleIdx := range atlasRoles {
for specRoleIdx := range prevSpec {
if atlasRoles[atlasRoleIdx].Name == prevSpec[specRoleIdx].Name {
result[prevSpec[specRoleIdx].Name] = prevSpec[specRoleIdx]
continue
}
lastAppliedRolesMap := mapCustomRolesByName(prevSpec)
akoRolesMap := mapCustomRolesByName(akoRoles)
atlasRolesMap := mapCustomRolesByName(atlasRoles)

for atlasName, atlasRole := range atlasRolesMap {
_, inAKO := akoRolesMap[atlasName]
_, inLastApplied := lastAppliedRolesMap[atlasName]
if !inAKO && inLastApplied {
result[atlasName] = atlasRole
}
}

return result
}

Expand Down Expand Up @@ -94,7 +98,7 @@ func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProjec
service: customroles.NewCustomRoles(workflowCtx.SdkClient.CustomDatabaseRolesApi),
}

currentCustomRoles, err := r.service.List(r.ctx.Context, r.project.ID())
currentAtlasCustomRoles, err := r.service.List(r.ctx.Context, r.project.ID())
if err != nil {
return workflow.Terminate(workflow.ProjectCustomRolesReady, err.Error())
}
Expand All @@ -104,12 +108,12 @@ func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProjec
akoRoles[i] = customroles.NewCustomRole(&project.Spec.CustomRoles[i])
}

ops := calculateChanges(currentCustomRoles, akoRoles)
ops := calculateChanges(currentAtlasCustomRoles, akoRoles)

var deleteStatus map[string]status.CustomRole
if len(lastAppliedCustomRoles) > 0 {
deleteStatus = r.deleteCustomRoles(workflowCtx, project.ID(),
findRolesToDelete(convertToInternalRoles(lastAppliedCustomRoles), currentCustomRoles))
findRolesToDelete(convertToInternalRoles(lastAppliedCustomRoles), akoRoles, currentAtlasCustomRoles))
}
updateStatus := r.updateCustomRoles(workflowCtx, project.ID(), ops.Update)
createStatus := r.createCustomRoles(workflowCtx, project.ID(), ops.Create)
Expand Down Expand Up @@ -218,22 +222,13 @@ func mapCustomRolesByName(customRoles []customroles.CustomRole) map[string]custo
type CustomRolesOperations struct {
Create map[string]customroles.CustomRole
Update map[string]customroles.CustomRole
Delete map[string]customroles.CustomRole
}

func calculateChanges(currentCustomRoles []customroles.CustomRole, desiredCustomRoles []customroles.CustomRole) CustomRolesOperations {
currentCustomRolesByName := mapCustomRolesByName(currentCustomRoles)
desiredCustomRolesByName := mapCustomRolesByName(desiredCustomRoles)
ops := CustomRolesOperations{
Create: map[string]customroles.CustomRole{},
Update: map[string]customroles.CustomRole{},
Delete: map[string]customroles.CustomRole{},
}

for _, currentCustomRole := range currentCustomRoles {
if _, ok := desiredCustomRolesByName[currentCustomRole.Name]; !ok {
ops.Delete[currentCustomRole.Name] = currentCustomRole
}
}

for _, desiredCustomRole := range desiredCustomRoles {
Expand Down
131 changes: 131 additions & 0 deletions internal/controller/atlasproject/custom_roles_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,137 @@ func TestEnsureCustomRoles(t *testing.T) {
}(),
isOK: true,
},
{
name: "Roles in AKO and in last applied config. Delete only those that were deleted from the spec",
projectAnnotations: map[string]string{
customresource.AnnotationLastAppliedConfiguration: func() string {
d, _ := json.Marshal(&akov2.AtlasProjectSpec{
CustomRoles: []akov2.CustomRole{
{
Name: "test-role",
InheritedRoles: []akov2.Role{
{Name: "role", Database: "db"},
},
Actions: []akov2.Action{
{
Name: "action",
Resources: []akov2.Resource{
{
Database: pointer.MakePtr("db"),
Cluster: pointer.MakePtr(true),
Collection: pointer.MakePtr("test-collection"),
},
},
},
},
},
{
Name: "test-role-2",
InheritedRoles: []akov2.Role{
{Name: "role2", Database: "db2"},
},
Actions: []akov2.Action{
{
Name: "action2",
Resources: []akov2.Resource{
{Database: pointer.MakePtr("db2")},
},
},
},
},
},
})
return string(d)
}(),
},
roles: []akov2.CustomRole{
{
Name: "test-role",
InheritedRoles: []akov2.Role{
{Name: "role", Database: "db"},
},
Actions: []akov2.Action{
{
Name: "action",
Resources: []akov2.Resource{
{
Database: pointer.MakePtr("db"),
Cluster: pointer.MakePtr(true),
Collection: pointer.MakePtr("test-collection"),
},
},
},
},
},
},
roleAPI: func() *mockadmin.CustomDatabaseRolesApi {
roleAPI := mockadmin.NewCustomDatabaseRolesApi(t)
roleAPI.EXPECT().ListCustomDatabaseRoles(context.Background(), "").
Return(admin.ListCustomDatabaseRolesApiRequest{ApiService: roleAPI})
roleAPI.EXPECT().ListCustomDatabaseRolesExecute(mock.Anything).
Return(
[]admin.UserCustomDBRole{
{
RoleName: "test-role",
InheritedRoles: &[]admin.DatabaseInheritedRole{
{Role: "role", Db: "db"},
},
Actions: &[]admin.DatabasePrivilegeAction{
{
Action: "action",
Resources: &[]admin.DatabasePermittedNamespaceResource{
{
Db: "db",
Collection: "test-collection",
Cluster: true,
},
},
},
},
},
{
RoleName: "test-role-1",
InheritedRoles: &[]admin.DatabaseInheritedRole{
{Role: "role1", Db: "db1"},
},
Actions: &[]admin.DatabasePrivilegeAction{
{
Action: "action1",
Resources: &[]admin.DatabasePermittedNamespaceResource{
{Db: "db1"},
},
},
},
},
{
RoleName: "test-role-2",
InheritedRoles: &[]admin.DatabaseInheritedRole{
{Role: "role2", Db: "db2"},
},
Actions: &[]admin.DatabasePrivilegeAction{
{
Action: "action2",
Resources: &[]admin.DatabasePermittedNamespaceResource{
{Db: "db2"},
},
},
},
},
},
&http.Response{},
nil,
)
roleAPI.EXPECT().DeleteCustomDatabaseRole(context.Background(), "", "test-role-2").
Return(admin.DeleteCustomDatabaseRoleApiRequest{ApiService: roleAPI})
roleAPI.EXPECT().DeleteCustomDatabaseRoleExecute(mock.Anything).
Return(
&http.Response{},
nil,
)
return roleAPI
}(),
isOK: true,
},
{
name: "Roles not in AKO but are in Atlas (Do not Delete) and NO previous in AKO",
roleAPI: func() *mockadmin.CustomDatabaseRolesApi {
Expand Down

0 comments on commit 298ca47

Please sign in to comment.