yuqi1129 commented on code in PR #13725: URL: https://github.com/apache/gravitino/pull/13725#discussion_r4255880276
########## plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/spark/SparkApplicationUtils.java: ########## @@ -0,0 +1,206 @@ +/* + * 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.gravitino.job.k8s.spark; + +import com.google.common.collect.ImmutableMap; +import io.fabric8.kubernetes.api.model.GenericKubernetesResource; +import io.fabric8.kubernetes.api.model.GenericKubernetesResourceBuilder; +import io.fabric8.kubernetes.client.dsl.base.ResourceDefinitionContext; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.TreeMap; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.connector.job.JobContext; +import org.apache.gravitino.job.SparkJobTemplate; +import org.apache.gravitino.job.k8s.K8sJobExecutorConfigs; +import org.apache.gravitino.job.k8s.K8sJobResourceUtils; + +/** + * Builds the SparkApplication of a Spark job, a custom resource of <a + * href="https://github.com/apache/spark-kubernetes-operator";>apache/spark-kubernetes-operator</a> + * 0.9.x. + */ +public final class SparkApplicationUtils { + + /** The SparkApplication resource definition. */ + public static final ResourceDefinitionContext SPARK_APPLICATION = + new ResourceDefinitionContext.Builder() + .withGroup("spark.apache.org") + .withVersion("v1") + .withKind("SparkApplication") + .withPlural("sparkapplications") + .withNamespaced(true) + .build(); + + /** The label the operator sets on the pods of a SparkApplication, whose value is its name. */ + public static final String LABEL_SPARK_APP_NAME = "spark.operator/spark-app-name"; + + /** The label Spark sets on its pods, whose value is the role of the pod. */ + public static final String LABEL_SPARK_ROLE = "spark-role"; + + /** The {@link #LABEL_SPARK_ROLE} value of the driver pod. */ + public static final String SPARK_ROLE_DRIVER = "driver"; + + /** The name Spark gives the driver container, unless a pod template renames it. */ + public static final String DRIVER_CONTAINER_NAME = "spark-kubernetes-driver"; + + /** The Spark configuration of the namespace a job runs in. */ + public static final String SPARK_NAMESPACE = "spark.kubernetes.namespace"; + + private static final String SPARK_MASTER = "spark.master"; + private static final String SPARK_DEPLOY_MODE = "spark.submit.deployMode"; + private static final String SPARK_IMAGE = "spark.kubernetes.container.image"; + private static final String SPARK_DRIVER_SERVICE_ACCOUNT = + "spark.kubernetes.authenticate.driver.serviceAccountName"; + private static final String SPARK_JARS = "spark.jars"; + private static final String SPARK_FILES = "spark.files"; + private static final String SPARK_ARCHIVES = "spark.archives"; + + private SparkApplicationUtils() {} + + /** + * Builds the SparkApplication of a Spark job. + * + * @param context the context of the job run + * @param template the runtime job template, whose resources are the URIs given in the template + * @param configs the job executor configurations + * @return the SparkApplication + * @throws IllegalArgumentException if the job can't run on Kubernetes + */ + public static GenericKubernetesResource buildSparkApplication( + JobContext context, SparkJobTemplate template, K8sJobExecutorConfigs configs) { + if (!template.environments().isEmpty()) { + throw new IllegalArgumentException( + String.format( + "Job template %s sets environment variables %s, which the k8s job executor doesn't" + + " support yet. Pass them as Spark configurations instead, for example" + + " spark.kubernetes.driverEnv.<name> and spark.executorEnv.<name>", + template.name(), template.environments().keySet())); + } + + checkClusterReachable(template.executable()); + template.jars().forEach(SparkApplicationUtils::checkClusterReachable); + template.files().forEach(SparkApplicationUtils::checkClusterReachable); + template.archives().forEach(SparkApplicationUtils::checkClusterReachable); + + Map<String, String> sparkConf = new TreeMap<>(configs.sparkConf()); + sparkConf.putAll(template.configs()); Review Comment: [Important] I reproduced this on head eb0b028. The same bypass also applies to executor defaults: `spark.conf.spark.jars`, `spark.conf.spark.files`, and `spark.conf.spark.archives`. With a valid executable (`https://repo/app.jar`), a probe supplying `/opt/local.jar` through each of the three keys in either `template.configs()` or executor defaults found that all six inputs were accepted. The current checks only cover the executable and the typed resource lists, so local paths in the merged Spark configuration reach the generated CR instead of being rejected before submission. Please validate each comma-separated resource in the final nonblank values of `spark.jars`, `spark.files`, and `spark.archives` after configuration overrides and list merging, reusing the existing validation and credential redaction. Tests should cover both configuration sources, mixed remote/local lists, and valid cluster-reachable URIs. This can stay scoped to `SparkApplicationUtils` and its test class; no changes to the status mapper or executor lifecycle are needed. The existing 81 module tests pass, but do not cover these configuration-based rejection paths. -- 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]
