[SPARK-28642][SQL] Hide credentials in SHOW CREATE TABLE #25375

wangyum · 2019-08-07T10:34:26Z

What changes were proposed in this pull request?

SPARK-17783 hided Credentials in CREATE and DESC FORMATTED/EXTENDED a PERSISTENT/TEMP Table for JDBC. But SHOW CREATE TABLE exposed the credentials:

spark-sql> show create table mysql_federated_sample;
CREATE TABLE `mysql_federated_sample` (`TBL_ID` BIGINT, `CREATE_TIME` INT, `DB_ID` BIGINT, `LAST_ACCESS_TIME` INT, `OWNER` STRING, `RETENTION` INT, `SD_ID` BIGINT, `TBL_NAME` STRING, `TBL_TYPE` STRING, `VIEW_EXPANDED_TEXT` STRING, `VIEW_ORIGINAL_TEXT` STRING, `IS_REWRITE_ENABLED` BOOLEAN)
USING org.apache.spark.sql.jdbc
OPTIONS (
  `url` 'jdbc:mysql://localhost/hive?user=root&password=mypasswd',
  `driver` 'com.mysql.jdbc.Driver',
  `dbtable` 'TBLS'
)

This pr fix this issue.

How was this patch tested?

unit tests and manual tests:

spark-sql> show create table  mysql_federated_sample;
CREATE TABLE `mysql_federated_sample` (`TBL_ID` BIGINT, `CREATE_TIME` INT, `DB_ID` BIGINT, `LAST_ACCESS_TIME` INT, `OWNER` STRING, `RETENTION` INT, `SD_ID` BIGINT, `TBL_NAME` STRING, `TBL_TYPE` STRING, `VIEW_EXPANDED_TEXT` STRING, `VIEW_ORIGINAL_TEXT` STRING, `IS_REWRITE_ENABLED` BOOLEAN)
USING org.apache.spark.sql.jdbc
OPTIONS (
  `url` '*********(redacted)',
  `driver` 'com.mysql.jdbc.Driver',
  `dbtable` 'TBLS'
)

wangyum · 2019-08-07T13:24:49Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala

    builder ++= s"USING ${metadata.provider.get}\n"

-    val dataSourceOptions = metadata.storage.properties.map {
+    val dataSourceOptions = Utils.redact(metadata.storage.properties).map {


Utils.redact or CatalogUtils.maskCredentials?

Seems SQLConf.redactOptions looks right one.

SparkQA · 2019-08-07T14:14:09Z

Test build #108764 has finished for PR 25375 at commit 39aa1fd.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2019-08-08T02:08:23Z

sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala

  }

+  test("Hide credentials in show create table") {
+    val password = "testPass"


Can we explicitly set the conf in the test?

Do you mean remove password declaration?

HyukjinKwon · 2019-08-08T02:08:36Z

Seems fine to me.

SparkQA · 2019-08-08T19:38:38Z

Test build #108836 has finished for PR 25375 at commit 20bf38a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-08-08T21:20:40Z

sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala

+    val password = "testPass"
+    val tableName = "tab1"
+    withTable(tableName) {
+      val df = sql(


Shall we remove val df = since this is not required here?

dongjoon-hyun · 2019-08-08T21:34:48Z

For the others, it looks good to me, @wangyum .

dongjoon-hyun

+1, LGTM. Merged to master.
Thank you, @wangyum and @HyukjinKwon .
(The last commit is to remove an unused variable declaration and the compilation already passed).

SparkQA · 2019-08-08T23:43:32Z

Test build #108847 has finished for PR 25375 at commit e764f9e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

wangyum · 2019-08-08T23:58:04Z

Do we need update DESC to use SQLConf.redactOptions:

dongjoon-hyun · 2019-08-09T06:26:53Z

Please file a new JIRA for DESC TABLE storage properties and open a PR, @wangyum .

gatorsmile

Could you submit a cherry-pick PR to 2.4?

This is a Spark branch-2.4 backport of #25375. Original description follows below: ## What changes were proposed in this pull request? [SPARK-17783](https://issues.apache.org/jira/browse/SPARK-17783) hided Credentials in `CREATE` and `DESC FORMATTED/EXTENDED` a PERSISTENT/TEMP Table for JDBC. But `SHOW CREATE TABLE` exposed the credentials: ```sql spark-sql> show create table mysql_federated_sample; CREATE TABLE `mysql_federated_sample` (`TBL_ID` BIGINT, `CREATE_TIME` INT, `DB_ID` BIGINT, `LAST_ACCESS_TIME` INT, `OWNER` STRING, `RETENTION` INT, `SD_ID` BIGINT, `TBL_NAME` STRING, `TBL_TYPE` STRING, `VIEW_EXPANDED_TEXT` STRING, `VIEW_ORIGINAL_TEXT` STRING, `IS_REWRITE_ENABLED` BOOLEAN) USING org.apache.spark.sql.jdbc OPTIONS ( `url` 'jdbc:mysql://localhost/hive?user=root&password=mypasswd', `driver` 'com.mysql.jdbc.Driver', `dbtable` 'TBLS' ) ``` This pr fix this issue. ## How was this patch tested? unit tests and manual tests: ```sql spark-sql> show create table mysql_federated_sample; CREATE TABLE `mysql_federated_sample` (`TBL_ID` BIGINT, `CREATE_TIME` INT, `DB_ID` BIGINT, `LAST_ACCESS_TIME` INT, `OWNER` STRING, `RETENTION` INT, `SD_ID` BIGINT, `TBL_NAME` STRING, `TBL_TYPE` STRING, `VIEW_EXPANDED_TEXT` STRING, `VIEW_ORIGINAL_TEXT` STRING, `IS_REWRITE_ENABLED` BOOLEAN) USING org.apache.spark.sql.jdbc OPTIONS ( `url` '*********(redacted)', `driver` 'com.mysql.jdbc.Driver', `dbtable` 'TBLS' ) ``` Closes #25560 from wangyum/SPARK-28642-branch-2.4. Authored-by: Yuming Wang <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

gatorsmile · 2019-08-26T00:28:01Z

sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala

+
+      val show = ShowCreateTableCommand(TableIdentifier(tableName))
+      spark.sessionState.executePlan(show).executedPlan.executeCollect().foreach { r =>
+        assert(!r.toString.contains(password))


WithSQLConf should be used for testing this. For example, change the value of spark.sql.redaction.options.regex and see whether it works as expected.

Normally, we do not rely on the default value of a SQLConf.

This is a Spark branch-2.4 backport of apache#25375. Original description follows below: ## What changes were proposed in this pull request? [SPARK-17783](https://issues.apache.org/jira/browse/SPARK-17783) hided Credentials in `CREATE` and `DESC FORMATTED/EXTENDED` a PERSISTENT/TEMP Table for JDBC. But `SHOW CREATE TABLE` exposed the credentials: ```sql spark-sql> show create table mysql_federated_sample; CREATE TABLE `mysql_federated_sample` (`TBL_ID` BIGINT, `CREATE_TIME` INT, `DB_ID` BIGINT, `LAST_ACCESS_TIME` INT, `OWNER` STRING, `RETENTION` INT, `SD_ID` BIGINT, `TBL_NAME` STRING, `TBL_TYPE` STRING, `VIEW_EXPANDED_TEXT` STRING, `VIEW_ORIGINAL_TEXT` STRING, `IS_REWRITE_ENABLED` BOOLEAN) USING org.apache.spark.sql.jdbc OPTIONS ( `url` 'jdbc:mysql://localhost/hive?user=root&password=mypasswd', `driver` 'com.mysql.jdbc.Driver', `dbtable` 'TBLS' ) ``` This pr fix this issue. ## How was this patch tested? unit tests and manual tests: ```sql spark-sql> show create table mysql_federated_sample; CREATE TABLE `mysql_federated_sample` (`TBL_ID` BIGINT, `CREATE_TIME` INT, `DB_ID` BIGINT, `LAST_ACCESS_TIME` INT, `OWNER` STRING, `RETENTION` INT, `SD_ID` BIGINT, `TBL_NAME` STRING, `TBL_TYPE` STRING, `VIEW_EXPANDED_TEXT` STRING, `VIEW_ORIGINAL_TEXT` STRING, `IS_REWRITE_ENABLED` BOOLEAN) USING org.apache.spark.sql.jdbc OPTIONS ( `url` '*********(redacted)', `driver` 'com.mysql.jdbc.Driver', `dbtable` 'TBLS' ) ``` Closes apache#25560 from wangyum/SPARK-28642-branch-2.4. Authored-by: Yuming Wang <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

Hide credentials in show create table

39aa1fd

wangyum commented Aug 7, 2019

View reviewed changes

dongjoon-hyun added the SQL label Aug 7, 2019

HyukjinKwon reviewed Aug 8, 2019

View reviewed changes

Utils.redact -> SQLConf.get.redactOptions?

20bf38a

dongjoon-hyun changed the title ~~[SPARK-28642][SQL] Hide credentials in show create table~~ [SPARK-28642][SQL] Hide credentials in SHOW CREATE TABLE Aug 8, 2019

dongjoon-hyun reviewed Aug 8, 2019

View reviewed changes

Remove val df =

e764f9e

dongjoon-hyun approved these changes Aug 8, 2019

View reviewed changes

dongjoon-hyun closed this in d19a56f Aug 8, 2019

wangyum deleted the SPARK-28642 branch August 8, 2019 23:34

gatorsmile reviewed Aug 23, 2019

View reviewed changes

wangyum mentioned this pull request Aug 23, 2019

[SPARK-28642][SQL][2.4] Hide credentials in show create table #25560

Closed

gatorsmile reviewed Aug 26, 2019

View reviewed changes

[SPARK-28642][SQL] Hide credentials in SHOW CREATE TABLE #25375

[SPARK-28642][SQL] Hide credentials in SHOW CREATE TABLE #25375

Uh oh!

Conversation

wangyum commented Aug 7, 2019

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 7, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Aug 8, 2019

Uh oh!

SparkQA commented Aug 8, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Aug 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 8, 2019

Uh oh!

wangyum commented Aug 8, 2019

Uh oh!

dongjoon-hyun commented Aug 9, 2019

Uh oh!

gatorsmile left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dongjoon-hyun commented Aug 8, 2019 •

edited

Loading