det101 commented on code in PR #18718: URL: https://github.com/apache/dolphinscheduler/pull/18718#discussion_r4490471066
########## dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/permission/TaskResourcePermissionChecker.java: ########## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.dolphinscheduler.api.permission; + +import org.apache.dolphinscheduler.api.enums.Status; +import org.apache.dolphinscheduler.api.exceptions.ServiceException; +import org.apache.dolphinscheduler.common.enums.ProjectResourceType; +import org.apache.dolphinscheduler.common.utils.JSONUtils; +import org.apache.dolphinscheduler.dao.entity.Environment; +import org.apache.dolphinscheduler.dao.entity.K8sNamespace; +import org.apache.dolphinscheduler.dao.entity.TaskDefinition; +import org.apache.dolphinscheduler.dao.entity.Tenant; +import org.apache.dolphinscheduler.dao.repository.IEnvironmentDao; +import org.apache.dolphinscheduler.dao.repository.K8sNamespaceDao; +import org.apache.dolphinscheduler.dao.repository.ProjectResourceAuthorizationDao; +import org.apache.dolphinscheduler.dao.repository.TenantDao; +import org.apache.dolphinscheduler.dao.utils.WorkerGroupUtils; +import org.apache.dolphinscheduler.plugin.task.api.parameters.SqlParameters; + +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; + +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import lombok.RequiredArgsConstructor; + +import org.springframework.stereotype.Component; + +@Component +@RequiredArgsConstructor +public class TaskResourcePermissionChecker { + + private final TaskDatasourcePermissionChecker datasourcePermissionChecker; + private final ProjectResourceAuthorizationDao resourceAuthorizationDao; + private final IEnvironmentDao environmentDao; + private final TenantDao tenantDao; + private final K8sNamespaceDao namespaceDao; + + public void checkPermission(long projectCode, Collection<? extends TaskDefinition> tasks) { + if (tasks == null || tasks.isEmpty()) { + return; + } + datasourcePermissionChecker.checkPermission(projectCode, tasks); + Set<String> workerGroups = new HashSet<>(); + Set<Long> environmentCodes = new HashSet<>(); + Set<Integer> taskGroupIds = new HashSet<>(); + Set<Integer> alertGroupIds = new HashSet<>(); + Set<Pair<String, String>> namespaces = new HashSet<>(); + for (TaskDefinition task : tasks) { + workerGroups.add(WorkerGroupUtils.getWorkerGroupOrDefault(task.getWorkerGroup())); + if (task.getEnvironmentCode() > 0) { + environmentCodes.add(task.getEnvironmentCode()); + } + if (task.getTaskGroupId() > 0) { + taskGroupIds.add(task.getTaskGroupId()); + } + if ("SQL".equals(task.getTaskType())) { + SqlParameters parameters = JSONUtils.parseObject(task.getTaskParams(), SqlParameters.class); + if (Boolean.TRUE.equals(parameters.getSendEmail()) && parameters.getGroupId() > 0) { + alertGroupIds.add(parameters.getGroupId()); + } + } + if ("K8S".equals(task.getTaskType()) || "KUBEFLOW".equals(task.getTaskType()) + || "SPARK".equals(task.getTaskType())) { + String namespace = JSONUtils.toMap(task.getTaskParams()).get("namespace"); Review Comment: ### Saving a Spark task with `mainJar` fails before namespace authorization **Path:** `dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/permission/TaskResourcePermissionChecker.java:83-85` `checkPermission` parses the whole `taskParams` with `JSONUtils.toMap`, which targets `Map<String, String>`. With Jackson 2.13.4, numbers, booleans, nulls and empty arrays are accepted. Nested objects are rejected. This is deterministic for the default Spark form. `programType` defaults to `SCALA`, and `use-main-jar.ts` requires `mainJar` unless the type is `SQL`. `format-data.ts` writes the selection as `mainJar: { resourceName }`. Saving goes through `WorkflowDefinitionServiceImpl.createDagDefine` → `checkPermission`. `PYTHON` has the same requirement. A `SQL` Spark task without nested fields does not hit this. Payload: ```json {"mainClass":"org.apache.spark.examples.SparkPi","mainJar":{"resourceName":"spark-examples.jar"},"programType":"SCALA","driverCores":1,"localParams":[],"resourceList":[]} ``` `JSONUtils.toMap` fails at `mainJar` on every save: ```text MismatchedInputException: Cannot deserialize value of type `java.lang.String` from Object value (token `JsonToken.START_OBJECT`) (through reference chain: java.util.LinkedHashMap["mainJar"]) ``` The API returns 500 and the workflow is not saved. Namespace can be empty. This happens before the worker-group grant check. The same `toMap(taskParams)` also fails when `localParams` or `resourceList` contains an object, or when a K8S task has a non-empty `customizedLabels` array. `namespaceGrantMustMatchBothNameAndCluster` only uses `{"namespace":"{\"name\":\"jobs\",\"cluster\":\"production\"}"}`, so it does not catch this. Please read `namespace` with `JsonNode` or `JSONUtils.getNodeString`, and keep the existing `JSONUtils.toMap(namespace)` for the `{"name","cluster"}` string. Spark, K8S and Kubeflow already share that value shape (`useNamespace()`, and `SparkTask` reads `toMap(namespace).get("name")`). Add a regression whose `taskParams` includes `mainJar`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
