Skip to content

Commit

Permalink
[KYUUBI #5786] Disable spark script transformation
Browse files Browse the repository at this point in the history
# 🔍 Description
## Issue References 🔗

This pull request fixes #5786.

## Describe Your Solution 🔧

Add spark check rule.

## Types of changes 🔖

- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests
org.apache.kyuubi.plugin.spark.authz.rule.AuthzUnsupportedOperationsCheckSuite.test("disable script transformation")

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [ ] Pull request title is okay.
- [ ] No license issues.
- [ ] Milestone correctly set?
- [ ] Test coverage is ok
- [ ] Assignees are selected.
- [ ] Minimum number of approvals
- [ ] No changes are requested

**Be nice. Be informative.**

Closes #5788 from zml1206/KYUUBI-5786.

Closes #5786

06c0098 [zml1206] fix
e2c3fee [zml1206] fix
37744f4 [zml1206] move to spark extentions
deb09fb [zml1206] add configuration
cfea484 [zml1206] Disable spark script transformation in Authz

Authored-by: zml1206 <[email protected]>
Signed-off-by: wforget <[email protected]>
  • Loading branch information
zml1206 authored and wForget committed Dec 5, 2023
1 parent 25eb53d commit 762ccd8
Show file tree
Hide file tree
Showing 15 changed files with 167 additions and 8 deletions.
1 change: 1 addition & 0 deletions docs/extensions/engines/spark/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,5 @@ Kyuubi provides some configs to make these feature easy to use.
| spark.sql.finalWriteStageExecutorMemory | fallback spark.executor.memory | Specify the executor on heap memory request for final write stage. It would be passed to the RDD resource profile. | 1.8.0 |
| spark.sql.finalWriteStageExecutorMemoryOverhead | fallback spark.executor.memoryOverhead | Specify the executor memory overhead request for final write stage. It would be passed to the RDD resource profile. | 1.8.0 |
| spark.sql.finalWriteStageExecutorOffHeapMemory | NONE | Specify the executor off heap memory request for final write stage. It would be passed to the RDD resource profile. | 1.8.0 |
| spark.sql.execution.scriptTransformation.enabled | true | When false, script transformation is not allowed. | 1.9.0 |

Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package org.apache.kyuubi.sql
import org.apache.spark.sql.SparkSessionExtensions

import org.apache.kyuubi.sql.sqlclassification.KyuubiSqlClassification
import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, MaxScanStrategy}
import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, KyuubiUnsupportedOperationsCheck, MaxScanStrategy}

// scalastyle:off line.size.limit
/**
Expand All @@ -39,6 +39,7 @@ class KyuubiSparkSQLExtension extends (SparkSessionExtensions => Unit) {
extensions.injectPostHocResolutionRule(DropIgnoreNonexistent)

// watchdog extension
extensions.injectCheckRule(_ => new KyuubiUnsupportedOperationsCheck)
extensions.injectOptimizerRule(ForcedMaxOutputRowsRule)
extensions.injectPlannerStrategy(MaxScanStrategy)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package org.apache.kyuubi.sql

import org.apache.spark.sql.SparkSessionExtensions

import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, MaxScanStrategy}
import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, KyuubiUnsupportedOperationsCheck, MaxScanStrategy}

// scalastyle:off line.size.limit
/**
Expand All @@ -37,6 +37,7 @@ class KyuubiSparkSQLExtension extends (SparkSessionExtensions => Unit) {
extensions.injectPostHocResolutionRule(DropIgnoreNonexistent)

// watchdog extension
extensions.injectCheckRule(_ => new KyuubiUnsupportedOperationsCheck)
extensions.injectOptimizerRule(ForcedMaxOutputRowsRule)
extensions.injectPlannerStrategy(MaxScanStrategy)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package org.apache.kyuubi.sql

import org.apache.spark.sql.{FinalStageResourceManager, InjectCustomResourceProfile, SparkSessionExtensions}

import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, MaxScanStrategy}
import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, KyuubiUnsupportedOperationsCheck, MaxScanStrategy}

// scalastyle:off line.size.limit
/**
Expand All @@ -37,6 +37,7 @@ class KyuubiSparkSQLExtension extends (SparkSessionExtensions => Unit) {
extensions.injectPostHocResolutionRule(DropIgnoreNonexistent)

// watchdog extension
extensions.injectCheckRule(_ => new KyuubiUnsupportedOperationsCheck)
extensions.injectOptimizerRule(ForcedMaxOutputRowsRule)
extensions.injectPlannerStrategy(MaxScanStrategy)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,4 +273,11 @@ object KyuubiSQLConf {
.version("1.8.0")
.stringConf
.createOptional

val SCRIPT_TRANSFORMATION_ENABLED =
buildConf("spark.sql.execution.scriptTransformation.enabled")
.doc("When false, script transformation is not allowed.")
.version("1.9.0")
.booleanConf
.createWithDefault(true)
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package org.apache.kyuubi.sql

import org.apache.spark.sql.{FinalStageResourceManager, InjectCustomResourceProfile, SparkSessionExtensions}

import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, MaxScanStrategy}
import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, KyuubiUnsupportedOperationsCheck, MaxScanStrategy}

// scalastyle:off line.size.limit
/**
Expand All @@ -37,6 +37,7 @@ class KyuubiSparkSQLExtension extends (SparkSessionExtensions => Unit) {
extensions.injectPostHocResolutionRule(DropIgnoreNonexistent)

// watchdog extension
extensions.injectCheckRule(_ => new KyuubiUnsupportedOperationsCheck)
extensions.injectOptimizerRule(ForcedMaxOutputRowsRule)
extensions.injectPlannerStrategy(MaxScanStrategy)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.kyuubi.sql.watchdog

import org.apache.spark.sql.catalyst.SQLConfHelper
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, ScriptTransformation}

import org.apache.kyuubi.sql.{KyuubiSQLConf, KyuubiSQLExtensionException}

class KyuubiUnsupportedOperationsCheck extends (LogicalPlan => Unit) with SQLConfHelper {
override def apply(plan: LogicalPlan): Unit =
conf.getConf(KyuubiSQLConf.SCRIPT_TRANSFORMATION_ENABLED) match {
case false => plan foreach {
case _: ScriptTransformation =>
throw new KyuubiSQLExtensionException("Script transformation is not allowed")
case _ =>
}
case true =>
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import scala.collection.JavaConverters._
import org.apache.commons.io.FileUtils
import org.apache.spark.sql.catalyst.plans.logical.{GlobalLimit, LogicalPlan}

import org.apache.kyuubi.sql.KyuubiSQLConf
import org.apache.kyuubi.sql.{KyuubiSQLConf, KyuubiSQLExtensionException}
import org.apache.kyuubi.sql.watchdog.{MaxFileSizeExceedException, MaxPartitionExceedException}

trait WatchDogSuiteBase extends KyuubiSparkSQLExtensionTest {
Expand Down Expand Up @@ -598,4 +598,13 @@ trait WatchDogSuiteBase extends KyuubiSparkSQLExtensionTest {
}
}
}

test("disable script transformation") {
withSQLConf(KyuubiSQLConf.SCRIPT_TRANSFORMATION_ENABLED.key -> "false") {
val e = intercept[KyuubiSQLExtensionException] {
sql("SELECT TRANSFORM('') USING 'ls /'")
}
assert(e.getMessage == "Script transformation is not allowed")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -289,4 +289,11 @@ object KyuubiSQLConf {
.version("1.9.0")
.intConf
.createWithDefault(2000)

val SCRIPT_TRANSFORMATION_ENABLED =
buildConf("spark.sql.execution.scriptTransformation.enabled")
.doc("When false, script transformation is not allowed.")
.version("1.9.0")
.booleanConf
.createWithDefault(true)
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package org.apache.kyuubi.sql

import org.apache.spark.sql.{FinalStageResourceManager, InjectCustomResourceProfile, SparkSessionExtensions}

import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, MaxScanStrategy}
import org.apache.kyuubi.sql.watchdog.{ForcedMaxOutputRowsRule, KyuubiUnsupportedOperationsCheck, MaxScanStrategy}

// scalastyle:off line.size.limit
/**
Expand All @@ -37,6 +37,7 @@ class KyuubiSparkSQLExtension extends (SparkSessionExtensions => Unit) {
extensions.injectPostHocResolutionRule(DropIgnoreNonexistent)

// watchdog extension
extensions.injectCheckRule(_ => new KyuubiUnsupportedOperationsCheck)
extensions.injectOptimizerRule(ForcedMaxOutputRowsRule)
extensions.injectPlannerStrategy(MaxScanStrategy)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.kyuubi.sql.watchdog

import org.apache.spark.sql.catalyst.SQLConfHelper
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, ScriptTransformation}

import org.apache.kyuubi.sql.{KyuubiSQLConf, KyuubiSQLExtensionException}

class KyuubiUnsupportedOperationsCheck extends (LogicalPlan => Unit) with SQLConfHelper {
override def apply(plan: LogicalPlan): Unit =
conf.getConf(KyuubiSQLConf.SCRIPT_TRANSFORMATION_ENABLED) match {
case false => plan foreach {
case _: ScriptTransformation =>
throw new KyuubiSQLExtensionException("Script transformation is not allowed")
case _ =>
}
case true =>
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import scala.collection.JavaConverters._
import org.apache.commons.io.FileUtils
import org.apache.spark.sql.catalyst.plans.logical.{GlobalLimit, LogicalPlan}

import org.apache.kyuubi.sql.KyuubiSQLConf
import org.apache.kyuubi.sql.{KyuubiSQLConf, KyuubiSQLExtensionException}
import org.apache.kyuubi.sql.watchdog.{MaxFileSizeExceedException, MaxPartitionExceedException}

trait WatchDogSuiteBase extends KyuubiSparkSQLExtensionTest {
Expand Down Expand Up @@ -598,4 +598,13 @@ trait WatchDogSuiteBase extends KyuubiSparkSQLExtensionTest {
}
}
}

test("disable script transformation") {
withSQLConf(KyuubiSQLConf.SCRIPT_TRANSFORMATION_ENABLED.key -> "false") {
val e = intercept[KyuubiSQLExtensionException] {
sql("SELECT TRANSFORM('') USING 'ls /'")
}
assert(e.getMessage == "Script transformation is not allowed")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -273,4 +273,11 @@ object KyuubiSQLConf {
.version("1.8.0")
.stringConf
.createOptional

val SCRIPT_TRANSFORMATION_ENABLED =
buildConf("spark.sql.execution.scriptTransformation.enabled")
.doc("When false, script transformation is not allowed.")
.version("1.9.0")
.booleanConf
.createWithDefault(true)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.kyuubi.sql.watchdog

import org.apache.spark.sql.catalyst.SQLConfHelper
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, ScriptTransformation}

import org.apache.kyuubi.sql.{KyuubiSQLConf, KyuubiSQLExtensionException}

class KyuubiUnsupportedOperationsCheck extends (LogicalPlan => Unit) with SQLConfHelper {
override def apply(plan: LogicalPlan): Unit =
conf.getConf(KyuubiSQLConf.SCRIPT_TRANSFORMATION_ENABLED) match {
case false => plan foreach {
case _: ScriptTransformation =>
throw new KyuubiSQLExtensionException("Script transformation is not allowed")
case _ =>
}
case true =>
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import scala.collection.JavaConverters._
import org.apache.commons.io.FileUtils
import org.apache.spark.sql.catalyst.plans.logical.{GlobalLimit, LogicalPlan}

import org.apache.kyuubi.sql.KyuubiSQLConf
import org.apache.kyuubi.sql.{KyuubiSQLConf, KyuubiSQLExtensionException}
import org.apache.kyuubi.sql.watchdog.{MaxFileSizeExceedException, MaxPartitionExceedException}

trait WatchDogSuiteBase extends KyuubiSparkSQLExtensionTest {
Expand Down Expand Up @@ -598,4 +598,13 @@ trait WatchDogSuiteBase extends KyuubiSparkSQLExtensionTest {
}
}
}

test("disable script transformation") {
withSQLConf(KyuubiSQLConf.SCRIPT_TRANSFORMATION_ENABLED.key -> "false") {
val e = intercept[KyuubiSQLExtensionException] {
sql("SELECT TRANSFORM('') USING 'ls /'")
}
assert(e.getMessage == "Script transformation is not allowed")
}
}
}

0 comments on commit 762ccd8

Please sign in to comment.