diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/K8sNamespaceController.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/K8sNamespaceController.java index 162a8aec2076..eceaa98bb538 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/K8sNamespaceController.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/K8sNamespaceController.java @@ -36,7 +36,6 @@ import org.apache.dolphinscheduler.plugin.task.api.utils.ParameterUtils; import java.util.List; -import java.util.Map; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; @@ -107,12 +106,11 @@ public Result queryNamespaceListPaging(@Parameter(hidden = true) @RequestAttribu @ResponseStatus(HttpStatus.CREATED) @ApiException(CREATE_K8S_NAMESPACE_ERROR) @OperatorLog(auditType = AuditType.K8S_NAMESPACE_CREATE) - public Result createNamespace(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam(value = "namespace") String namespace, - @RequestParam(value = "clusterCode") Long clusterCode) { - Map result = - k8sNamespaceService.registerK8sNamespace(loginUser, namespace, clusterCode); - return returnDataList(result); + public Result createNamespace(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam(value = "namespace") String namespace, + @RequestParam(value = "clusterCode") Long clusterCode) { + K8sNamespace k8sNamespace = k8sNamespaceService.registerK8sNamespace(loginUser, namespace, clusterCode); + return Result.success(k8sNamespace); } /** @@ -153,10 +151,10 @@ public Result verifyNamespace(@Parameter(hidden = true) @RequestAttribute(value @ResponseStatus(HttpStatus.OK) @ApiException(DELETE_K8S_NAMESPACE_BY_ID_ERROR) @OperatorLog(auditType = AuditType.K8S_NAMESPACE_DELETE) - public Result delNamespaceById(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam(value = "id") int id) { - Map result = k8sNamespaceService.deleteNamespaceById(loginUser, id); - return returnDataList(result); + public Result delNamespaceById(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam(value = "id") int id) { + k8sNamespaceService.deleteNamespaceById(loginUser, id); + return Result.success(); } /** @@ -173,10 +171,10 @@ public Result delNamespaceById(@Parameter(hidden = true) @RequestAttribute(value @GetMapping(value = "/unauth-namespace") @ResponseStatus(HttpStatus.OK) @ApiException(QUERY_UNAUTHORIZED_NAMESPACE_ERROR) - public Result queryUnauthorizedNamespace(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam("userId") Integer userId) { - Map result = k8sNamespaceService.queryUnauthorizedNamespace(loginUser, userId); - return returnDataList(result); + public Result> queryUnauthorizedNamespace(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam("userId") Integer userId) { + List namespaces = k8sNamespaceService.queryUnauthorizedNamespace(loginUser, userId); + return Result.success(namespaces); } /** @@ -193,10 +191,10 @@ public Result queryUnauthorizedNamespace(@Parameter(hidden = true) @RequestAttri @GetMapping(value = "/authed-namespace") @ResponseStatus(HttpStatus.OK) @ApiException(QUERY_AUTHORIZED_NAMESPACE_ERROR) - public Result queryAuthorizedNamespace(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam("userId") Integer userId) { - Map result = k8sNamespaceService.queryAuthorizedNamespace(loginUser, userId); - return returnDataList(result); + public Result> queryAuthorizedNamespace(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam("userId") Integer userId) { + List namespaces = k8sNamespaceService.queryAuthorizedNamespace(loginUser, userId); + return Result.success(namespaces); } /** diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/K8sNamespaceService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/K8sNamespaceService.java index 032612a4ee1b..2e37e62313b2 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/K8sNamespaceService.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/K8sNamespaceService.java @@ -22,7 +22,6 @@ import org.apache.dolphinscheduler.dao.entity.User; import java.util.List; -import java.util.Map; /** * k8s namespace service impl @@ -46,9 +45,8 @@ public interface K8sNamespaceService { * @param loginUser login user * @param namespace namespace * @param clusterCode k8s not null - * @return */ - Map registerK8sNamespace(User loginUser, String namespace, Long clusterCode); + K8sNamespace registerK8sNamespace(User loginUser, String namespace, Long clusterCode); /** * verify namespace and k8s @@ -64,9 +62,8 @@ public interface K8sNamespaceService { * * @param loginUser login user * @param id namespace id - * @return */ - Map deleteNamespaceById(User loginUser, int id); + void deleteNamespaceById(User loginUser, int id); /** * query unauthorized namespace @@ -75,7 +72,7 @@ public interface K8sNamespaceService { * @param userId user id * @return the namespaces which user have not permission to see */ - Map queryUnauthorizedNamespace(User loginUser, Integer userId); + List queryUnauthorizedNamespace(User loginUser, Integer userId); /** * query unauthorized namespace @@ -84,7 +81,7 @@ public interface K8sNamespaceService { * @param userId user id * @return namespaces which the user have permission to see */ - Map queryAuthorizedNamespace(User loginUser, Integer userId); + List queryAuthorizedNamespace(User loginUser, Integer userId); /** * query namespace can use diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/K8SNamespaceServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/K8SNamespaceServiceImpl.java index 9f3309414cc2..95cfb2450490 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/K8SNamespaceServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/K8SNamespaceServiceImpl.java @@ -35,8 +35,8 @@ import org.apache.commons.lang3.StringUtils; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -105,38 +105,32 @@ public Result queryListPaging(User loginUser, String searchVal, Integer pageNo, * @param loginUser login user * @param namespace namespace * @param clusterCode k8s not null - * @return */ @Override - public Map registerK8sNamespace(User loginUser, String namespace, Long clusterCode) { - Map result = new HashMap<>(); + public K8sNamespace registerK8sNamespace(User loginUser, String namespace, Long clusterCode) { if (isNotAdmin(loginUser)) { throw new ServiceException(Status.USER_NO_OPERATION_PERM); } if (StringUtils.isEmpty(namespace)) { log.warn("Parameter namespace is empty."); - putMsg(result, Status.REQUEST_PARAMS_NOT_VALID_ERROR, Constants.NAMESPACE); - return result; + throw new ServiceException(Status.REQUEST_PARAMS_NOT_VALID_ERROR, Constants.NAMESPACE); } if (clusterCode == null) { log.warn("Parameter clusterCode is null."); - putMsg(result, Status.REQUEST_PARAMS_NOT_VALID_ERROR, Constants.CLUSTER); - return result; + throw new ServiceException(Status.REQUEST_PARAMS_NOT_VALID_ERROR, Constants.CLUSTER); } if (checkNamespaceExistInDb(namespace, clusterCode)) { log.warn("K8S namespace already exists."); - putMsg(result, Status.K8S_NAMESPACE_EXIST, namespace, clusterCode); - return result; + throw new ServiceException(Status.K8S_NAMESPACE_EXIST, namespace, clusterCode); } Cluster cluster = clusterMapper.queryByClusterCode(clusterCode); if (cluster == null) { log.error("Cluster does not exist, clusterCode:{}", clusterCode); - putMsg(result, Status.CLUSTER_NOT_EXISTS, namespace, clusterCode); - return result; + throw new ServiceException(Status.CLUSTER_NOT_EXISTS, namespace, clusterCode); } long code = CodeGenerateUtils.genCode(); @@ -157,17 +151,13 @@ public Map registerK8sNamespace(User loginUser, String namespace k8sClientService.upsertNamespaceAndResourceToK8s(k8sNamespaceObj); } catch (Exception e) { log.error("Namespace create to k8s error", e); - putMsg(result, Status.K8S_CLIENT_OPS_ERROR, e.getMessage()); - return result; + throw new ServiceException(Status.K8S_CLIENT_OPS_ERROR, e.getMessage()); } } k8sNamespaceMapper.insert(k8sNamespaceObj); log.info("K8s namespace create complete, namespace:{}.", k8sNamespaceObj.getNamespace()); - result.put(Constants.DATA_LIST, k8sNamespaceObj); - putMsg(result, Status.SUCCESS); - - return result; + return k8sNamespaceObj; } /** @@ -207,11 +197,9 @@ public Result verifyNamespaceK8s(String namespace, Long clusterCode) { * * @param loginUser login user * @param id namespace id - * @return */ @Override - public Map deleteNamespaceById(User loginUser, int id) { - Map result = new HashMap<>(); + public void deleteNamespaceById(User loginUser, int id) { if (isNotAdmin(loginUser)) { throw new ServiceException(Status.USER_NO_OPERATION_PERM); } @@ -219,14 +207,11 @@ public Map deleteNamespaceById(User loginUser, int id) { K8sNamespace k8sNamespaceObj = k8sNamespaceMapper.selectById(id); if (k8sNamespaceObj == null) { log.error("K8s namespace does not exist, namespaceId:{}.", id); - putMsg(result, Status.K8S_NAMESPACE_NOT_EXIST, id); - return result; + throw new ServiceException(Status.K8S_NAMESPACE_NOT_EXIST, id); } k8sNamespaceMapper.deleteById(id); log.info("K8s namespace delete complete, namespace:{}.", k8sNamespaceObj.getNamespace()); - putMsg(result, Status.SUCCESS); - return result; } /** @@ -247,23 +232,18 @@ private boolean checkNamespaceExistInDb(String namespace, Long clusterCode) { * @return the namespaces which user have not permission to see */ @Override - public Map queryUnauthorizedNamespace(User loginUser, Integer userId) { - Map result = new HashMap<>(); + public List queryUnauthorizedNamespace(User loginUser, Integer userId) { if (loginUser.getId() != userId && isNotAdmin(loginUser)) { throw new ServiceException(Status.USER_NO_OPERATION_PERM); } // query all namespace list, this auth does not like project List namespaceList = k8sNamespaceMapper.selectList(null); - List resultList = new ArrayList<>(); - Set namespaceSet; - if (namespaceList != null && !namespaceList.isEmpty()) { - namespaceSet = new HashSet<>(namespaceList); - List authedProjectList = k8sNamespaceMapper.queryAuthedNamespaceListByUserId(userId); - resultList = getUnauthorizedNamespaces(namespaceSet, authedProjectList); + if (namespaceList == null || namespaceList.isEmpty()) { + return Collections.emptyList(); } - result.put(Constants.DATA_LIST, resultList); - putMsg(result, Status.SUCCESS); - return result; + Set namespaceSet = new HashSet<>(namespaceList); + List authedProjectList = k8sNamespaceMapper.queryAuthedNamespaceListByUserId(userId); + return getUnauthorizedNamespaces(namespaceSet, authedProjectList); } /** @@ -274,18 +254,11 @@ public Map queryUnauthorizedNamespace(User loginUser, Integer us * @return namespaces which the user have permission to see */ @Override - public Map queryAuthorizedNamespace(User loginUser, Integer userId) { - Map result = new HashMap<>(); - + public List queryAuthorizedNamespace(User loginUser, Integer userId) { if (loginUser.getId() != userId && isNotAdmin(loginUser)) { throw new ServiceException(Status.USER_NO_OPERATION_PERM); } - - List namespaces = k8sNamespaceMapper.queryAuthedNamespaceListByUserId(userId); - result.put(Constants.DATA_LIST, namespaces); - putMsg(result, Status.SUCCESS); - - return result; + return k8sNamespaceMapper.queryAuthedNamespaceListByUserId(userId); } /** diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/K8SNamespaceServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/K8SNamespaceServiceTest.java index ee852ff7da12..371f9ba4d6c2 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/K8SNamespaceServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/K8SNamespaceServiceTest.java @@ -24,7 +24,6 @@ import org.apache.dolphinscheduler.api.service.impl.K8SNamespaceServiceImpl; import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.api.utils.Result; -import org.apache.dolphinscheduler.common.constants.Constants; import org.apache.dolphinscheduler.common.enums.UserType; import org.apache.dolphinscheduler.dao.entity.Cluster; import org.apache.dolphinscheduler.dao.entity.K8sNamespace; @@ -37,7 +36,6 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.List; -import java.util.Map; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -91,19 +89,17 @@ public void queryListPaging() { @Test public void createK8sNamespace() { // namespace is null - Map result = - k8sNamespaceService.registerK8sNamespace(getLoginUser(), null, clusterCode); - logger.info(result.toString()); - Assertions.assertEquals(Status.REQUEST_PARAMS_NOT_VALID_ERROR, result.get(Constants.STATUS)); + assertThrowsServiceException(Status.REQUEST_PARAMS_NOT_VALID_ERROR, + () -> k8sNamespaceService.registerK8sNamespace(getLoginUser(), null, clusterCode)); // k8s is null - result = k8sNamespaceService.registerK8sNamespace(getLoginUser(), namespace, null); - logger.info(result.toString()); - Assertions.assertEquals(Status.REQUEST_PARAMS_NOT_VALID_ERROR, result.get(Constants.STATUS)); + assertThrowsServiceException(Status.REQUEST_PARAMS_NOT_VALID_ERROR, + () -> k8sNamespaceService.registerK8sNamespace(getLoginUser(), namespace, null)); // correct Mockito.when(clusterMapper.queryByClusterCode(Mockito.anyLong())).thenReturn(getCluster()); - result = k8sNamespaceService.registerK8sNamespace(getLoginUser(), namespace, clusterCode); - logger.info(result.toString()); - Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + K8sNamespace created = k8sNamespaceService.registerK8sNamespace(getLoginUser(), namespace, clusterCode); + Assertions.assertNotNull(created); + Assertions.assertEquals(namespace, created.getNamespace()); + Assertions.assertEquals(clusterCode, created.getClusterCode()); } @Test @@ -137,9 +133,7 @@ public void deleteNamespaceById() { Mockito.when(k8sNamespaceMapper.deleteById(Mockito.any())).thenReturn(1); Mockito.when(k8sNamespaceMapper.selectById(1)).thenReturn(getNamespace()); - Map result = k8sNamespaceService.deleteNamespaceById(getLoginUser(), 1); - logger.info(result.toString()); - Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + Assertions.assertDoesNotThrow(() -> k8sNamespaceService.deleteNamespaceById(getLoginUser(), 1)); } @Test @@ -150,9 +144,7 @@ public void testQueryAuthorizedNamespace() { // test admin user loginUser.setUserType(UserType.ADMIN_USER); - Map result = k8sNamespaceService.queryAuthorizedNamespace(loginUser, 2); - logger.info(result.toString()); - List namespaces = (List) result.get(Constants.DATA_LIST); + List namespaces = k8sNamespaceService.queryAuthorizedNamespace(loginUser, 2); Assertions.assertTrue(CollectionUtils.isNotEmpty(namespaces)); // test non-admin user @@ -170,9 +162,7 @@ public void testQueryUnAuthorizedNamespace() { // test admin user User loginUser = new User(); loginUser.setUserType(UserType.ADMIN_USER); - Map result = k8sNamespaceService.queryUnauthorizedNamespace(loginUser, 2); - logger.info(result.toString()); - List namespaces = (List) result.get(Constants.DATA_LIST); + List namespaces = k8sNamespaceService.queryUnauthorizedNamespace(loginUser, 2); Assertions.assertTrue(CollectionUtils.isNotEmpty(namespaces)); // test non-admin user