This is an automated email from the ASF dual-hosted git repository.
zihaoxiang pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/dolphinscheduler.git
The following commit(s) were added to refs/heads/dev by this push:
new 759756ae80 [Improvement-14269][API] Bind task group with project
(#14392)
759756ae80 is described below
commit 759756ae80a449c1989181c70696ba59fa125922
Author: Aaron Wang <[email protected]>
AuthorDate: Sun Jul 23 12:11:37 2023 +0800
[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 +-
.../api/service/impl/TaskGroupServiceImpl.java | 79 ++++++++++++++--------
.../api/controller/TaskGroupControllerTest.java | 1 +
.../api/service/TaskGroupServiceTest.java | 22 +++++-
.../dao/mapper/TaskGroupMapper.java | 8 +--
.../dao/mapper/TaskGroupMapper.xml | 15 +---
.../dao/mapper/TaskGroupMapperTest.java | 6 --
.../task-group/option/components/form-modal.tsx | 4 +-
.../views/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 b [...]
## 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<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;
+ }
+
}
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<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))
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<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));
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<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
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 @@
</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>
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<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);
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<Array<SelectMixedOption>> = 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(
