From 759756ae80a449c1989181c70696ba59fa125922 Mon Sep 17 00:00:00 2001 From: Aaron Wang Date: Sun, 23 Jul 2023 12:11:37 +0800 Subject: [PATCH] [Improvement-14269][API] Bind task group with project (#14392) * [Improvement-14269][API] Bind task group with project * remove comment * add project permission check for task group operation * add doc --- docs/docs/en/guide/resource/task-group.md | 2 +- docs/docs/zh/guide/resource/task-group.md | 2 +- .../service/impl/TaskGroupServiceImpl.java | 79 ++++++++++++------- .../controller/TaskGroupControllerTest.java | 1 + .../api/service/TaskGroupServiceTest.java | 22 +++++- .../dao/mapper/TaskGroupMapper.java | 8 +- .../dao/mapper/TaskGroupMapper.xml | 15 +--- .../dao/mapper/TaskGroupMapperTest.java | 6 -- .../option/components/form-modal.tsx | 4 +- .../resource/task-group/option/use-table.ts | 4 +- 10 files changed, 83 insertions(+), 60 deletions(-) diff --git a/docs/docs/en/guide/resource/task-group.md b/docs/docs/en/guide/resource/task-group.md index 87e04b4cb0..66071f79e0 100644 --- a/docs/docs/en/guide/resource/task-group.md +++ b/docs/docs/en/guide/resource/task-group.md @@ -1,6 +1,6 @@ # Task Group Settings -The task group is mainly used to control the concurrency of task instances, and is designed to control the pressure of other resources (it can also control the pressure of the Hadoop cluster, the cluster will have queue control it). When creating a new task definition, you can configure the corresponding task group and configure the priority of the task running in the task group. +The task group is mainly used to control the concurrency of task instances, and is designed to control the pressure of other resources (it can also control the pressure of the Hadoop cluster, the cluster will have queue control it). When creating a new task definition, you can configure the corresponding task group and configure the priority of the task running in the task group. The user can only view the task groups belongs to authorized projects, and can create or update task groups belongs to one project only if they have write permission. ## Task Group Configuration diff --git a/docs/docs/zh/guide/resource/task-group.md b/docs/docs/zh/guide/resource/task-group.md index 1c5fb18cab..86b82c746d 100644 --- a/docs/docs/zh/guide/resource/task-group.md +++ b/docs/docs/zh/guide/resource/task-group.md @@ -1,6 +1,6 @@ # 任务组管理 -任务组主要用于控制任务实例并发，旨在控制其他资源的压力（也可以控制 Hadoop 集群压力，不过集群会有队列管控）。您可在新建任务定义时，可配置对应的任务组，并配置任务在任务组内运行的优先级。 +任务组主要用于控制任务实例并发，旨在控制其他资源的压力（也可以控制 Hadoop 集群压力，不过集群会有队列管控）。您可在新建任务定义时，可配置对应的任务组，并配置任务在任务组内运行的优先级。用户仅能查看有权限的项目对应的任务组，且仅能创建或修改具有写权限的项目对应的任务组。 ### 任务组配置 diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java index bf88ba7d5b..e654332010 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java @@ -26,8 +26,13 @@ import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.common.constants.Constants; import org.apache.dolphinscheduler.common.enums.AuthorizationType; import org.apache.dolphinscheduler.common.enums.Flag; +import org.apache.dolphinscheduler.common.enums.UserType; +import org.apache.dolphinscheduler.dao.entity.Project; +import org.apache.dolphinscheduler.dao.entity.ProjectUser; import org.apache.dolphinscheduler.dao.entity.TaskGroup; import org.apache.dolphinscheduler.dao.entity.User; +import org.apache.dolphinscheduler.dao.mapper.ProjectMapper; +import org.apache.dolphinscheduler.dao.mapper.ProjectUserMapper; import org.apache.dolphinscheduler.dao.mapper.TaskGroupMapper; import org.apache.commons.collections4.CollectionUtils; @@ -39,7 +44,6 @@ import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; @@ -62,6 +66,12 @@ public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupSe @Autowired private TaskGroupMapper taskGroupMapper; + @Autowired + private ProjectMapper projectMapper; + + @Autowired + private ProjectUserMapper projectUserMapper; + @Autowired private TaskGroupQueueService taskGroupQueueService; @@ -82,10 +92,7 @@ public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupSe public Map createTaskGroup(User loginUser, Long projectCode, String name, String description, int groupSize) { Map result = new HashMap<>(); - boolean canOperatorPermissions = canOperatorPermissions(loginUser, null, AuthorizationType.TASK_GROUP, - ApiFuncIdentificationConstant.TASK_GROUP_CREATE); - if (!canOperatorPermissions) { - putMsg(result, Status.NO_CURRENT_OPERATING_PERMISSION); + if (!hasProjectPerm(loginUser, projectCode, result, true)) { return result; } if (checkDescriptionLength(description)) { @@ -147,10 +154,8 @@ public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupSe @Override public Map updateTaskGroup(User loginUser, int id, String name, String description, int groupSize) { Map result = new HashMap<>(); - boolean canOperatorPermissions = canOperatorPermissions(loginUser, null, AuthorizationType.TASK_GROUP, - ApiFuncIdentificationConstant.TASK_GROUP_EDIT); - if (!canOperatorPermissions) { - putMsg(result, Status.NO_CURRENT_OPERATING_PERMISSION); + TaskGroup taskGroup = taskGroupMapper.selectById(id); + if (!hasProjectPerm(loginUser, taskGroup.getProjectCode(), result, true)) { return result; } if (checkDescriptionLength(description)) { @@ -178,7 +183,6 @@ public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupSe putMsg(result, Status.TASK_GROUP_NAME_EXSIT); return result; } - TaskGroup taskGroup = taskGroupMapper.selectById(id); if (taskGroup.getStatus() != Flag.YES.getCode()) { log.warn("Task group has been closed, taskGroupId:{}.", id); putMsg(result, Status.TASK_GROUP_STATUS_ERROR); @@ -252,17 +256,12 @@ public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupSe @Override public Map queryTaskGroupByProjectCode(User loginUser, int pageNo, int pageSize, Long projectCode) { Map result = new HashMap<>(); - Page page = new Page<>(pageNo, pageSize); - PageInfo emptyPageInfo = new PageInfo<>(pageNo, pageSize); - Set ids = resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.TASK_GROUP, - loginUser.getId(), log); - if (ids.isEmpty()) { - result.put(Constants.DATA_LIST, emptyPageInfo); - putMsg(result, Status.SUCCESS); + if (!hasProjectPerm(loginUser, projectCode, result, false)) { return result; } + Page page = new Page<>(pageNo, pageSize); IPage taskGroupPaging = - taskGroupMapper.queryTaskGroupPagingByProjectCode(page, new ArrayList<>(ids), projectCode); + taskGroupMapper.queryTaskGroupPagingByProjectCode(page, projectCode); return getStringObjectMap(pageNo, pageSize, result, taskGroupPaging); } @@ -311,16 +310,8 @@ public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupSe Integer status) { Map result = new HashMap<>(); Page page = new Page<>(pageNo, pageSize); - PageInfo pageInfo = new PageInfo<>(pageNo, pageSize); - Set ids = resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.TASK_GROUP, - userId, log); - if (ids.isEmpty()) { - result.put(Constants.DATA_LIST, pageInfo); - putMsg(result, Status.SUCCESS); - return result; - } IPage taskGroupPaging = - taskGroupMapper.queryTaskGroupPaging(page, new ArrayList<>(ids), name, status); + taskGroupMapper.queryTaskGroupPaging(page, name, status); return getStringObjectMap(pageNo, pageSize, result, taskGroupPaging); } @@ -439,4 +430,38 @@ public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupSe taskGroupQueueService.deleteByTaskGroupIds(taskGroupIds); taskGroupMapper.deleteBatchIds(taskGroupIds); } + + private boolean hasProjectPerm(User loginUser, long projectCode, Map result, + boolean writePermission) { + Project project = projectMapper.queryByCode(projectCode); + if (project == null) { + log.warn("Project does not exist"); + putMsg(result, Status.PROJECT_NOT_FOUND, ""); + } + + if (loginUser.getUserType() == UserType.ADMIN_USER) { + return true; + } + + if (project.getUserId().equals(loginUser.getId())) { + return true; + } + + ProjectUser projectUser = projectUserMapper.queryProjectRelation(project.getId(), loginUser.getId()); + if (projectUser == null) { + log.warn("User {} does not have operation permission for project {}", loginUser.getUserName(), + project.getCode()); + putMsg(result, Status.USER_NO_OPERATION_PROJECT_PERM, loginUser.getUserName(), project.getCode()); + return false; + } + if (writePermission && projectUser.getPerm() != Constants.DEFAULT_ADMIN_PERMISSION) { + log.warn("User {} does not have write permission for project {}", loginUser.getUserName(), + project.getCode()); + putMsg(result, Status.USER_NO_WRITE_PROJECT_PERM, loginUser.getUserName(), project.getCode()); + return false; + } + + return true; + } + } diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/TaskGroupControllerTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/TaskGroupControllerTest.java index 6bc08fe228..e53bd174e6 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/TaskGroupControllerTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/TaskGroupControllerTest.java @@ -48,6 +48,7 @@ public class TaskGroupControllerTest extends AbstractControllerTest { MultiValueMap paramsMap = new LinkedMultiValueMap<>(); paramsMap.add("pageNo", "2"); paramsMap.add("pageSize", "2"); + paramsMap.add("projectCode", "123456789"); MvcResult mvcResult = mockMvc.perform(get("/task-group/query-list-by-projectCode") .header(SESSION_ID, sessionId) .params(paramsMap)) diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/TaskGroupServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/TaskGroupServiceTest.java index 1f8f2abb4a..ae50a54037 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/TaskGroupServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/TaskGroupServiceTest.java @@ -27,8 +27,10 @@ import org.apache.dolphinscheduler.common.constants.Constants; import org.apache.dolphinscheduler.common.enums.AuthorizationType; import org.apache.dolphinscheduler.common.enums.Flag; import org.apache.dolphinscheduler.common.enums.UserType; +import org.apache.dolphinscheduler.dao.entity.Project; import org.apache.dolphinscheduler.dao.entity.TaskGroup; import org.apache.dolphinscheduler.dao.entity.User; +import org.apache.dolphinscheduler.dao.mapper.ProjectMapper; import org.apache.dolphinscheduler.dao.mapper.TaskGroupMapper; import org.apache.dolphinscheduler.dao.mapper.TaskGroupQueueMapper; import org.apache.dolphinscheduler.dao.mapper.UserMapper; @@ -80,12 +82,17 @@ public class TaskGroupServiceTest { @Mock private UserMapper userMapper; + @Mock + private ProjectMapper projectMapper; + private String taskGroupName = "TaskGroupServiceTest"; private String taskGroupDesc = "this is a task group"; private String userName = "taskGroupServiceTest"; + private String projectName = "taskGroupServiceTest"; + @Mock private ResourcePermissionCheckService resourcePermissionCheckService; @@ -102,6 +109,15 @@ public class TaskGroupServiceTest { return loginUser; } + private Project getProject() { + Project project = new Project(); + project.setCode(1L); + project.setId(1); + project.setName(projectName); + project.setUserId(1); + return project; + } + private TaskGroup getTaskGroup() { TaskGroup taskGroup = TaskGroup.builder() .name(taskGroupName) @@ -149,6 +165,7 @@ public class TaskGroupServiceTest { loginUser.getId(), ApiFuncIdentificationConstant.TASK_GROUP_CREATE, serviceLogger)).thenReturn(true); Mockito.when(taskGroupMapper.insert(taskGroup)).thenReturn(1); Mockito.when(taskGroupMapper.queryByName(loginUser.getId(), taskGroupName)).thenReturn(null); + Mockito.when(projectMapper.queryByCode(taskGroup.getProjectCode())).thenReturn(getProject()); Map result = taskGroupService.createTaskGroup(loginUser, 0L, taskGroupName, taskGroupDesc, 100); Assertions.assertNotNull(result); @@ -173,8 +190,8 @@ public class TaskGroupServiceTest { loginUser.getId(), ApiFuncIdentificationConstant.TASK_GROUP_VIEW, serviceLogger)).thenReturn(true); Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.TASK_GROUP, null, 0, serviceLogger)).thenReturn(true); - Mockito.when(taskGroupMapper.queryTaskGroupPaging(Mockito.any(Page.class), Mockito.anyList(), - Mockito.eq(null), Mockito.eq(0))).thenReturn(page); + Mockito.when(taskGroupMapper.queryTaskGroupPaging(Mockito.any(Page.class), Mockito.eq(null), Mockito.eq(0))) + .thenReturn(page); // query all Map result = taskGroupService.queryAllTaskGroup(loginUser, null, null, 1, 10); @@ -196,6 +213,7 @@ public class TaskGroupServiceTest { 0, serviceLogger)).thenReturn(true); Mockito.when(taskGroupMapper.selectById(1)).thenReturn(taskGroup); Mockito.when(taskGroupMapper.updateById(taskGroup)).thenReturn(1); + Mockito.when(projectMapper.queryByCode(taskGroup.getProjectCode())).thenReturn(getProject()); Map result = taskGroupService.updateTaskGroup(loginUser, 1, "newName", "desc", 100); logger.info(result.toString()); Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); diff --git a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.java b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.java index 7cb79c9e58..794bdbab3a 100644 --- a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.java +++ b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.java @@ -53,13 +53,12 @@ public interface TaskGroupMapper extends BaseMapper { * select task groups paging * * @param page page - * @param userId user id * @param name name * @param status status * @return result page */ - IPage queryTaskGroupPaging(IPage page, @Param("ids") List ids, - @Param("name") String name, @Param("status") Integer status); + IPage queryTaskGroupPaging(IPage page, @Param("name") String name, + @Param("status") Integer status); /** * query by task group name @@ -77,8 +76,7 @@ public interface TaskGroupMapper extends BaseMapper { int selectCountByIdStatus(@Param("id") int id, @Param("status") int status); - IPage queryTaskGroupPagingByProjectCode(Page page, @Param("ids") List ids, - @Param("projectCode") Long projectCode); + IPage queryTaskGroupPagingByProjectCode(Page page, @Param("projectCode") Long projectCode); /** * listAuthorizedResource diff --git a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml index 6e387afb60..c3f1df0b0e 100644 --- a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml +++ b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml @@ -41,12 +41,6 @@ from t_ds_task_group - - and id in - - #{i} - - and status = #{status} @@ -62,14 +56,7 @@ from t_ds_task_group - where 1=1 - - and id in - - #{i} - - - and project_code in ( #{projectCode} , 0) + where project_code in ( #{projectCode} , 0) order by update_time desc diff --git a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapperTest.java b/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapperTest.java index f45beb2a72..71c01bd95e 100644 --- a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapperTest.java +++ b/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapperTest.java @@ -20,9 +20,7 @@ package org.apache.dolphinscheduler.dao.mapper; import org.apache.dolphinscheduler.dao.BaseDaoTest; import org.apache.dolphinscheduler.dao.entity.TaskGroup; -import java.util.ArrayList; import java.util.Date; -import java.util.List; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -88,12 +86,8 @@ public class TaskGroupMapperTest extends BaseDaoTest { public void testQueryTaskGroupPaging() { TaskGroup taskGroup = insertOne(); Page page = new Page(1, 3); - List ids = new ArrayList<>(); - ids.add(1); - ids.add(2); IPage taskGroupIPage = taskGroupMapper.queryTaskGroupPaging( page, - ids, taskGroup.getName(), taskGroup.getStatus()); Assertions.assertEquals(taskGroupIPage.getTotal(), 1); diff --git a/dolphinscheduler-ui/src/views/resource/task-group/option/components/form-modal.tsx b/dolphinscheduler-ui/src/views/resource/task-group/option/components/form-modal.tsx index 2e53259090..39e87a7edb 100644 --- a/dolphinscheduler-ui/src/views/resource/task-group/option/components/form-modal.tsx +++ b/dolphinscheduler-ui/src/views/resource/task-group/option/components/form-modal.tsx @@ -28,7 +28,7 @@ import { NForm, NFormItem, NInput, NSelect, NInputNumber } from 'naive-ui' import { useForm } from '../use-form' import Modal from '@/components/modal' import { createTaskGroup, updateTaskGroup } from '@/service/modules/task-group' -import { queryAllProjectList } from '@/service/modules/projects' +import { queryProjectCreatedAndAuthorizedByUser } from '@/service/modules/projects' import { SelectMixedOption } from 'naive-ui/lib/select/src/interface' const props = { @@ -54,7 +54,7 @@ const FormModal = defineComponent({ const projectOptions: Ref> = ref([]) onMounted(() => { - queryAllProjectList().then((res: any[]) => { + queryProjectCreatedAndAuthorizedByUser().then((res: any[]) => { res.map((item) => { const option: SelectMixedOption = { label: item.name, diff --git a/dolphinscheduler-ui/src/views/resource/task-group/option/use-table.ts b/dolphinscheduler-ui/src/views/resource/task-group/option/use-table.ts index e4ffd0f44c..fc286d7d0c 100644 --- a/dolphinscheduler-ui/src/views/resource/task-group/option/use-table.ts +++ b/dolphinscheduler-ui/src/views/resource/task-group/option/use-table.ts @@ -19,7 +19,7 @@ import { h, reactive, ref } from 'vue' import { useI18n } from 'vue-i18n' import { format } from 'date-fns' import { queryTaskGroupListPaging } from '@/service/modules/task-group' -import { queryAllProjectList } from '@/service/modules/projects' +import { queryProjectCreatedAndAuthorizedByUser } from '@/service/modules/projects' import TableAction from './components/table-action' import _ from 'lodash' import { parseTime } from '@/common/common' @@ -125,7 +125,7 @@ export function useTable( const getTableData = (params: any) => { if (variables.loadingRef) return variables.loadingRef = true - Promise.all([queryTaskGroupListPaging(params), queryAllProjectList()]).then( + Promise.all([queryTaskGroupListPaging(params), queryProjectCreatedAndAuthorizedByUser()]).then( (values: any[]) => { variables.totalPage = values[0].totalPage variables.tableData = values[0].totalList.map(