-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
Fix cluster role cleanup on cluster deletion #5399
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.
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
The head ref may contain hidden characters: "codex/\u4FEE\u590Dapollo\u95EE\u98985392"
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package com.ctrip.framework.apollo.portal.service; | ||
|
|
||
| import com.ctrip.framework.apollo.portal.environment.Env; | ||
| import com.ctrip.framework.apollo.portal.api.AdminServiceAPI; | ||
| import com.ctrip.framework.apollo.portal.entity.bo.UserInfo; | ||
| import com.ctrip.framework.apollo.portal.spi.UserInfoHolder; | ||
| import com.ctrip.framework.apollo.portal.service.RoleInitializationService; | ||
| import com.ctrip.framework.apollo.portal.service.RolePermissionService; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
| import org.mockito.InjectMocks; | ||
| import org.mockito.Mock; | ||
|
|
||
| import static org.mockito.Mockito.*; | ||
|
|
||
| public class ClusterServiceTest extends com.ctrip.framework.apollo.portal.AbstractUnitTest { | ||
|
|
||
| @Mock | ||
| private AdminServiceAPI.ClusterAPI clusterAPI; | ||
| @Mock | ||
| private RoleInitializationService roleInitializationService; | ||
| @Mock | ||
| private RolePermissionService rolePermissionService; | ||
| @Mock | ||
| private UserInfoHolder userInfoHolder; | ||
|
|
||
| @InjectMocks | ||
| private ClusterService clusterService; | ||
|
|
||
| private String appId = "clusterApp"; | ||
| private String clusterName = "default"; | ||
| private Env env = Env.DEV; | ||
|
|
||
| @Before | ||
| public void setUp() { | ||
| UserInfo user = new UserInfo(); | ||
| user.setUserId("operator"); | ||
| when(userInfoHolder.getUser()).thenReturn(user); | ||
| } | ||
|
|
||
| @Test | ||
| public void testDeleteClusterShouldCleanupRoles() { | ||
| clusterService.deleteCluster(env, appId, clusterName); | ||
|
|
||
| verify(rolePermissionService).deleteRolePermissionsByAppIdAndCluster(appId, env.getName(), clusterName, "operator"); | ||
| verify(clusterAPI).delete(env, appId, clusterName, "operator"); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,6 +32,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.portal.repository.RoleRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.portal.repository.UserRoleRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.portal.service.RolePermissionService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.portal.util.RoleUtils; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.common.collect.Sets; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Set; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -320,6 +321,40 @@ public void testUserHasPermission() throws Exception { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Sql(scripts = "/sql/permission/RolePermissionServiceTest.deleteRolePermissionsByAppIdWithClusterRoles.sql", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Sql(scripts = "/sql/cleanup.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void testDeleteRolePermissionsByAppIdWithClusterRoles() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String appId = "clusterApp"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String operator = "test"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rolePermissionService.deleteRolePermissionsByAppId(appId, operator); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String modifyRoleName = RoleUtils.buildModifyNamespacesInClusterRoleName(appId, "DEV", "default"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String releaseRoleName = RoleUtils.buildReleaseNamespacesInClusterRoleName(appId, "DEV", "default"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertNull(roleRepository.findTopByRoleName(modifyRoleName)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertNull(roleRepository.findTopByRoleName(releaseRoleName)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+324
to
+339
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix missing import for assertNull. The test method logic is correct, but Add the missing import for +import static org.junit.Assert.assertNull;The test correctly verifies that cluster-related roles are deleted when calling 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Sql(scripts = "/sql/permission/RolePermissionServiceTest.deleteRolePermissionsByAppIdWithClusterRoles.sql", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Sql(scripts = "/sql/cleanup.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void testDeleteRolePermissionsByCluster() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String appId = "clusterApp"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String operator = "test"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rolePermissionService.deleteRolePermissionsByAppIdAndCluster(appId, "DEV", "default", operator); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String modifyRoleName = RoleUtils.buildModifyNamespacesInClusterRoleName(appId, "DEV", "default"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String releaseRoleName = RoleUtils.buildReleaseNamespacesInClusterRoleName(appId, "DEV", "default"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertNull(roleRepository.findTopByRoleName(modifyRoleName)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertNull(roleRepository.findTopByRoleName(releaseRoleName)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+341
to
+356
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix missing import for assertNull. The test method logic is correct and properly tests the cluster-specific deletion functionality, but Add the missing import for +import static org.junit.Assert.assertNull;The test correctly verifies that only the specified cluster's roles are deleted when calling 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private Role assembleRole(String roleName) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Role role = new Role(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| role.setRoleName(roleName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,11 @@ | ||||||||||||||||||||||||||||||||||
| INSERT INTO "Permission" (`Id`, `PermissionType`, `TargetId`, `DataChange_CreatedBy`, `DataChange_LastModifiedBy`) VALUES | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||
| (1500, 'ModifyNamespacesInCluster', 'clusterApp+DEV+default', 'someOperator', 'someOperator'), | ||||||||||||||||||||||||||||||||||
| (1501, 'ReleaseNamespacesInCluster', 'clusterApp+DEV+default', 'someOperator', 'someOperator'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| INSERT INTO "Role" (`Id`, `RoleName`, `DataChange_CreatedBy`, `DataChange_LastModifiedBy`) VALUES | ||||||||||||||||||||||||||||||||||
| (1500, 'ModifyNamespacesInCluster+clusterApp+DEV+default', 'someOperator', 'someOperator'), | ||||||||||||||||||||||||||||||||||
| (1501, 'ReleaseNamespacesInCluster+clusterApp+DEV+default', 'someOperator', 'someOperator'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| INSERT INTO "RolePermission" (`Id`, `RoleId`, `PermissionId`) VALUES | ||||||||||||||||||||||||||||||||||
| (1500, 1500, 1500), | ||||||||||||||||||||||||||||||||||
| (1501, 1501, 1501); | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.