Browse Source

[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
Aaron Wang 1 year ago
parent
commit
759756ae80

File diff suppressed because it is too large
+ 1 - 1
docs/docs/en/guide/resource/task-group.md


+ 1 - 1
docs/docs/zh/guide/resource/task-group.md

@@ -1,6 +1,6 @@
 # 任务组管理
 
-任务组主要用于控制任务实例并发,旨在控制其他资源的压力(也可以控制 Hadoop 集群压力,不过集群会有队列管控)。您可在新建任务定义时,可配置对应的任务组,并配置任务在任务组内运行的优先级。
+任务组主要用于控制任务实例并发,旨在控制其他资源的压力(也可以控制 Hadoop 集群压力,不过集群会有队列管控)。您可在新建任务定义时,可配置对应的任务组,并配置任务在任务组内运行的优先级。用户仅能查看有权限的项目对应的任务组,且仅能创建或修改具有写权限的项目对应的任务组。
 
 ### 任务组配置
 

+ 52 - 27
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<String, Object> createTaskGroup(User loginUser, Long projectCode, String name, String description,
                                                int groupSize) {
         Map<String, Object> 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<String, Object> updateTaskGroup(User loginUser, int id, String name, String description, int groupSize) {
         Map<String, Object> 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<String, Object> queryTaskGroupByProjectCode(User loginUser, int pageNo, int pageSize, Long projectCode) {
         Map<String, Object> result = new HashMap<>();
-        Page<TaskGroup> page = new Page<>(pageNo, pageSize);
-        PageInfo<TaskGroup> emptyPageInfo = new PageInfo<>(pageNo, pageSize);
-        Set<Integer> 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<TaskGroup> page = new Page<>(pageNo, pageSize);
         IPage<TaskGroup> 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<String, Object> result = new HashMap<>();
         Page<TaskGroup> page = new Page<>(pageNo, pageSize);
-        PageInfo<TaskGroup> pageInfo = new PageInfo<>(pageNo, pageSize);
-        Set<Integer> ids = resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.TASK_GROUP,
-                userId, log);
-        if (ids.isEmpty()) {
-            result.put(Constants.DATA_LIST, pageInfo);
-            putMsg(result, Status.SUCCESS);
-            return result;
-        }
         IPage<TaskGroup> 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<String, Object> 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;
+    }
+
 }

+ 1 - 0
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/TaskGroupControllerTest.java

@@ -48,6 +48,7 @@ public class TaskGroupControllerTest extends AbstractControllerTest {
         MultiValueMap<String, String> 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))

+ 20 - 2
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<String, Object> 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<String, Object> 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<String, Object> result = taskGroupService.updateTaskGroup(loginUser, 1, "newName", "desc", 100);
         logger.info(result.toString());
         Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));

+ 3 - 5
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.java

@@ -53,13 +53,12 @@ public interface TaskGroupMapper extends BaseMapper<TaskGroup> {
      * select task groups paging
      *
      * @param page   page
-     * @param userId user id
      * @param name   name
      * @param status status
      * @return result page
      */
-    IPage<TaskGroup> queryTaskGroupPaging(IPage<TaskGroup> page, @Param("ids") List<Integer> ids,
-                                          @Param("name") String name, @Param("status") Integer status);
+    IPage<TaskGroup> queryTaskGroupPaging(IPage<TaskGroup> page, @Param("name") String name,
+                                          @Param("status") Integer status);
 
     /**
      * query by task group name
@@ -77,8 +76,7 @@ public interface TaskGroupMapper extends BaseMapper<TaskGroup> {
 
     int selectCountByIdStatus(@Param("id") int id, @Param("status") int status);
 
-    IPage<TaskGroup> queryTaskGroupPagingByProjectCode(Page<TaskGroup> page, @Param("ids") List<Integer> ids,
-                                                       @Param("projectCode") Long projectCode);
+    IPage<TaskGroup> queryTaskGroupPagingByProjectCode(Page<TaskGroup> page, @Param("projectCode") Long projectCode);
 
     /**
      * listAuthorizedResource

+ 1 - 14
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml

@@ -41,12 +41,6 @@
         </include>
         from t_ds_task_group
         <where>
-            <if test="ids != null and ids.size() > 0">
-                and id in
-                <foreach collection="ids" item="i" open="(" close=")" separator=",">
-                    #{i}
-                </foreach>
-            </if>
             <if test="status != null">
                 and status = #{status}
             </if>
@@ -62,14 +56,7 @@
         <include refid="baseSql">
         </include>
         from t_ds_task_group
-        where 1=1
-        <if test="ids != null and ids.size() > 0">
-            and id in
-            <foreach collection="ids" item="i" open="(" close=")" separator=",">
-                #{i}
-            </foreach>
-        </if>
-        and project_code in ( #{projectCode} , 0)
+        where project_code in ( #{projectCode} , 0)
         order by update_time desc
     </select>
 

+ 0 - 6
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<TaskGroup> page = new Page(1, 3);
-        List<Integer> ids = new ArrayList<>();
-        ids.add(1);
-        ids.add(2);
         IPage<TaskGroup> taskGroupIPage = taskGroupMapper.queryTaskGroupPaging(
                 page,
-                ids,
                 taskGroup.getName(), taskGroup.getStatus());
 
         Assertions.assertEquals(taskGroupIPage.getTotal(), 1);

+ 2 - 2
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<Array<SelectMixedOption>> = ref([])
 
     onMounted(() => {
-      queryAllProjectList().then((res: any[]) => {
+      queryProjectCreatedAndAuthorizedByUser().then((res: any[]) => {
         res.map((item) => {
           const option: SelectMixedOption = {
             label: item.name,

+ 2 - 2
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(