[SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector #28863

gaborgsomogyi · 2020-06-18T15:43:10Z

What changes were proposed in this pull request?

When loading DataFrames from JDBC datasource with Kerberos authentication, remote executors (yarn-client/cluster etc. modes) fail to establish a connection due to lack of Kerberos ticket or ability to generate it.

This is a real issue when trying to ingest data from kerberized data sources (SQL Server, Oracle) in enterprise environment where exposing simple authentication access is not an option due to IT policy issues.

In this PR I've added Oracle support.

What this PR contains:

Added OracleConnectionProvider
Added OracleConnectionProviderSuite

Why are the changes needed?

Missing JDBC kerberos support.

Does this PR introduce any user-facing change?

Yes, now user is able to connect to Oracle using kerberos.

How was this patch tested?

Additional + existing unit tests
Test on cluster manually. There is no automated integration test, just like MS SQL. The reason is the same. Namely I've tried to make it work with MiniKDC but it was working only with Active Directory (AD is not available in docker, please see Add support for Kerberos/Active Directory/"windows" authentication microsoft/mssql-docker#165).

SparkQA · 2020-06-18T15:49:55Z

Test build #124222 has finished for PR 28863 at commit 5cc3d0a.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

gaborgsomogyi

Couple of pointers to help reviewers.

The same applies here just like on MS SQL case. Namely I was able to make this work with active directory. Please see further info here.

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

gaborgsomogyi · 2020-06-18T15:47:12Z

pom.xml

@@ -984,6 +984,12 @@
        <version>8.2.2.jre8</version>
        <scope>test</scope>
      </dependency>
+      <dependency>
+        <groupId>com.oracle.database.jdbc</groupId>


This is the latest version from the Oracle JDBC driver which supports JDK8, JDK9, and JDK11: https://mvnrepository.com/artifact/com.oracle.database.jdbc/ojdbc8

gaborgsomogyi · 2020-06-18T15:48:39Z

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala

+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)


The implementation is based on this.

gaborgsomogyi · 2020-06-18T15:50:37Z

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala

+    result
+  }
+
+  override def setAuthenticationConfigIfNeeded(): Unit = SecurityConfigurationLock.synchronized {


Here synchronization is important to avoid race just like in other providers.

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

gaborgsomogyi · 2020-06-18T18:31:51Z

retest this please

gaborgsomogyi · 2020-06-18T18:51:41Z

cc @HeartSaVioR

SparkQA · 2020-06-18T21:24:16Z

Test build #124231 has finished for PR 28863 at commit 5cc3d0a.

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

external/docker-integration-tests/pom.xml

gatorsmile · 2020-06-20T04:43:36Z

cc @maropu @MaxGekk

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

maropu · 2020-06-20T06:33:26Z

...g/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProviderSuite.scala

+package org.apache.spark.sql.execution.datasources.jdbc.connection
+
+class OracleConnectionProviderSuite extends ConnectionProviderSuiteBase {
+  test("setAuthenticationConfigIfNeeded must set authentication if not set") {


All the XXXConnectionProviderSuite has the almost same test, so could you move it into ConnectionProviderSuiteBase?

How do you suggest to do that? Driver registration and provider instantiation lines are different in each case.
The only duplicate what I see is the test name + the testSecureConnectionProvider call.

Ah, I see. But, I felt a a bit less testing for creating a separate test file.

maropu · 2020-06-20T07:28:58Z

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala

+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"


Just a question; where does this value come? From the Oracle JDBC impl?

Yeah, I've used JD-GUI to take a look at the details.

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala

maropu · 2020-06-20T07:43:20Z

Note: I've checked that OracleIntegrationSuite passed in my local Mac env.

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

gaborgsomogyi · 2020-06-22T12:02:13Z

I'm intended to merge master into this PR and resolve conflicts when #28893 accepted.

dongjoon-hyun · 2020-06-22T19:34:44Z

Thank you, @gaborgsomogyi . I reviewed #28893 . We can merge that if the timeout issue is resolved there.

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

SparkQA · 2020-06-23T14:33:18Z

Test build #124420 has finished for PR 28863 at commit e16b4a4.

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

gaborgsomogyi · 2020-06-23T14:52:07Z

retest this please

SparkQA · 2020-06-23T14:58:37Z

Test build #124425 has finished for PR 28863 at commit e16b4a4.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-06-23T15:27:02Z

Test build #124418 has finished for PR 28863 at commit d698fea.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
public final class MapOutputCommitMessage
sealed trait LogisticRegressionSummary extends ClassificationSummary
class _ClassificationSummary(JavaWrapper):
class _TrainingSummary(JavaWrapper):
class _BinaryClassificationSummary(_ClassificationSummary):
class LogisticRegressionSummary(_ClassificationSummary):
class LogisticRegressionTrainingSummary(LogisticRegressionSummary, _TrainingSummary):
class BinaryLogisticRegressionSummary(_BinaryClassificationSummary,
case class Hour(child: Expression, timeZoneId: Option[String] = None) extends GetTimeField
case class Minute(child: Expression, timeZoneId: Option[String] = None) extends GetTimeField
case class Second(child: Expression, timeZoneId: Option[String] = None) extends GetTimeField
trait GetDateField extends UnaryExpression with ImplicitCastInputTypes with NullIntolerant
case class DayOfYear(child: Expression) extends GetDateField
case class Year(child: Expression) extends GetDateField
case class YearOfWeek(child: Expression) extends GetDateField
case class Quarter(child: Expression) extends GetDateField
case class Month(child: Expression) extends GetDateField
case class DayOfMonth(child: Expression) extends GetDateField
case class DayOfWeek(child: Expression) extends GetDateField
case class WeekDay(child: Expression) extends GetDateField
case class WeekOfYear(child: Expression) extends GetDateField
case class CoalesceBucketsInSortMergeJoin(conf: SQLConf) extends Rule[SparkPlan]
class StateStoreConf(

maropu

cc: @dongjoon-hyun

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala

dongjoon-hyun · 2020-06-27T17:20:55Z

Retest this please

SparkQA · 2020-06-27T17:27:52Z

Test build #124569 has finished for PR 28863 at commit e16b4a4.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2020-06-27T22:20:36Z

Retest this please.

SparkQA · 2020-06-27T22:24:08Z

Test build #124573 has finished for PR 28863 at commit e16b4a4.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2020-06-27T23:36:41Z

Retest this please.

SparkQA · 2020-06-27T23:42:26Z

Test build #124576 has finished for PR 28863 at commit e16b4a4.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-06-29T13:15:10Z

Test build #124640 has finished for PR 28863 at commit 461eed0.

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

gaborgsomogyi · 2020-06-29T13:43:53Z

retest this please

dongjoon-hyun · 2020-06-30T00:30:56Z

Retest this please.

dongjoon-hyun · 2020-06-30T00:50:33Z

Hi, @gaborgsomogyi . Is OracleKrbIntegrationSuite missing here?

maropu · 2020-06-30T02:17:21Z

retest this please

maropu · 2020-06-30T02:17:34Z

(Seems Jenkins sleeping now...)

gaborgsomogyi · 2020-06-30T07:51:22Z

@dongjoon-hyun Yes, I've spent almost gross 1 month to make it work w/ MiniKDC but no success. Please see my comment up. Not found any clear statement it must work so I gave up. If somebody can make it work or when active directory will be available in docker we can add it.

dongjoon-hyun · 2020-06-30T16:23:54Z

@gaborgsomogyi .

+1 for adding those important content on the PR description How was this patch tested? section. Your important Investigation and decision is worth to be a commit log. Mostly, only commit logs prove your achievement and contribution, and make people trust you.
Existing integration tests (especially OracleIntegrationSuite) sounds like misleading a little. Is that testing this code patch?

SparkQA · 2020-06-30T16:50:00Z

Test build #124667 has finished for PR 28863 at commit 461eed0.

This patch fails to generate documentation.
This patch merges cleanly.
This patch adds no public classes.

gaborgsomogyi · 2020-06-30T17:01:44Z

@dongjoon-hyun Oh gosh! OracleIntegrationSuite is a leftover from the original PR where driver upgrade was inside.

gaborgsomogyi · 2020-06-30T17:02:46Z

retest this please

SparkQA · 2020-06-30T17:12:37Z

Test build #124681 has finished for PR 28863 at commit 461eed0.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun

+1, LGTM. Thank you, @gaborgsomogyi and @maropu .
I tested locally the dependency and the new test suite.
Merged to master for Apache Spark 3.1.0.

gaborgsomogyi · 2020-06-30T17:36:12Z

@dongjoon-hyun thank you taking care and giving me suggestions like your last comment. I'm going to consider them in later contributions.

MaxGekk · 2020-06-30T20:23:23Z

Can this cause the build failure? #28959 (comment)

dongjoon-hyun · 2020-06-30T20:56:36Z

Thanks, I'm looking at this, @MaxGekk and @jiangxb1987 . It's weird because GitHub action passed.

[SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

5cc3d0a

probot-autolabeler bot added BUILD SQL labels Jun 18, 2020

gaborgsomogyi commented Jun 18, 2020

View reviewed changes

dongjoon-hyun reviewed Jun 19, 2020

View reviewed changes

external/docker-integration-tests/pom.xml Outdated Show resolved Hide resolved

maropu reviewed Jun 20, 2020

View reviewed changes

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala Outdated Show resolved Hide resolved

maropu reviewed Jun 20, 2020

View reviewed changes

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala Outdated Show resolved Hide resolved

maropu reviewed Jun 20, 2020

View reviewed changes

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala Show resolved Hide resolved

maropu reviewed Jun 20, 2020

View reviewed changes

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala Outdated Show resolved Hide resolved

gaborgsomogyi mentioned this pull request Jun 22, 2020

[SPARK-32049][SQL][TESTS] Upgrade Oracle JDBC Driver 8 #28893

Closed

Merge branch 'master' into SPARK-31336

d698fea

gaborgsomogyi commented Jun 23, 2020

View reviewed changes

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala Outdated Show resolved Hide resolved

Review fixes

e16b4a4

maropu approved these changes Jun 25, 2020

View reviewed changes

dongjoon-hyun reviewed Jun 25, 2020

View reviewed changes

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jun 25, 2020

View reviewed changes

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jun 25, 2020

View reviewed changes

...la/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala Show resolved Hide resolved

Review fix

461eed0

dongjoon-hyun approved these changes Jun 30, 2020

View reviewed changes

dongjoon-hyun closed this in 67cb7ea Jun 30, 2020

dongjoon-hyun mentioned this pull request Jun 30, 2020

[SPARK-32088][PYTHON][FOLLOWUP] Replace collect() by show() in the example for timestamp_seconds #28959

Closed


		import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions

		private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)

[SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector #28863

[SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector #28863

Conversation

gaborgsomogyi commented Jun 18, 2020 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

SparkQA commented Jun 18, 2020

gaborgsomogyi left a comment • edited

Choose a reason for hiding this comment

gaborgsomogyi Jun 18, 2020

Choose a reason for hiding this comment

gaborgsomogyi Jun 18, 2020 • edited

Choose a reason for hiding this comment

gaborgsomogyi Jun 18, 2020

Choose a reason for hiding this comment

gaborgsomogyi commented Jun 18, 2020

gaborgsomogyi commented Jun 18, 2020

SparkQA commented Jun 18, 2020

gatorsmile commented Jun 20, 2020

maropu Jun 20, 2020

Choose a reason for hiding this comment

gaborgsomogyi Jun 23, 2020 • edited

Choose a reason for hiding this comment

maropu Jun 23, 2020

Choose a reason for hiding this comment

maropu Jun 20, 2020

Choose a reason for hiding this comment

gaborgsomogyi Jun 23, 2020

Choose a reason for hiding this comment

maropu commented Jun 20, 2020

gaborgsomogyi commented Jun 22, 2020

dongjoon-hyun commented Jun 22, 2020

SparkQA commented Jun 23, 2020

gaborgsomogyi commented Jun 23, 2020

SparkQA commented Jun 23, 2020

SparkQA commented Jun 23, 2020

maropu left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented Jun 27, 2020

SparkQA commented Jun 27, 2020

dongjoon-hyun commented Jun 27, 2020

SparkQA commented Jun 27, 2020

dongjoon-hyun commented Jun 27, 2020

SparkQA commented Jun 27, 2020

SparkQA commented Jun 29, 2020

gaborgsomogyi commented Jun 29, 2020

dongjoon-hyun commented Jun 30, 2020

dongjoon-hyun commented Jun 30, 2020

maropu commented Jun 30, 2020

maropu commented Jun 30, 2020

gaborgsomogyi commented Jun 30, 2020 • edited

dongjoon-hyun commented Jun 30, 2020 • edited

SparkQA commented Jun 30, 2020

gaborgsomogyi commented Jun 30, 2020

gaborgsomogyi commented Jun 30, 2020

SparkQA commented Jun 30, 2020

dongjoon-hyun left a comment

Choose a reason for hiding this comment

gaborgsomogyi commented Jun 30, 2020

MaxGekk commented Jun 30, 2020

dongjoon-hyun commented Jun 30, 2020

gaborgsomogyi commented Jun 18, 2020 •

edited

gaborgsomogyi left a comment •

edited

gaborgsomogyi Jun 18, 2020 •

edited

gaborgsomogyi Jun 23, 2020 •

edited

gaborgsomogyi commented Jun 30, 2020 •

edited

dongjoon-hyun commented Jun 30, 2020 •

edited