Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable TLS Support (DTS - Specific) #515

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Enable TLS Support (DTS - Specific) #515

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
87175f1
Enable TLS Support (DTS - Specific)
Vipinofficial11 Oct 7, 2024
8d62fb9
Hide useSSL Field from UI and fix unit test failure
Vipinofficial11 Oct 10, 2024
b68ec93
Handle null case for SSl mode and Update connector json
Vipinofficial11 Oct 11, 2024
b31e53c
Separate Connection String Formats.
Vipinofficial11 Oct 15, 2024
7dc9a6c
Add separate utility methods to get connection string.
Vipinofficial11 Oct 15, 2024
ae5d1c9
Run tests with TestRunner.java
Vipinofficial11 Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion oracle-plugin/src/main/java/io/cdap/plugin/oracle/OracleAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static class OracleActionConfig extends DBSpecificQueryConfig {

@Override
public String getConnectionString() {
return OracleConstants.getConnectionString(this.connectionType, host, port, database);
return OracleConstants.getConnectionString(this.connectionType, host, port, database, null);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion oracle-plugin/src/main/java/io/cdap/plugin/oracle/OracleConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ protected String getConnectionString(@Nullable String database) {
return config.getConnectionString();
}
return OracleConstants.getConnectionString(config.getConnectionType(),
config.getHost(), config.getPort(), database);
config.getHost(), config.getPort(), database, config.getSSlMode());
}

@Override
Expand Down
17 changes: 14 additions & 3 deletions oracle-plugin/src/main/java/io/cdap/plugin/oracle/OracleConnectorConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ public OracleConnectorConfig(String host, int port, String user, String password

public OracleConnectorConfig(String host, int port, String user, String password, String jdbcPluginName,
String connectionArguments, String connectionType, String database) {
this(host, port, user, password, jdbcPluginName, connectionArguments, connectionType, database, null);
this(host, port, user, password, jdbcPluginName, connectionArguments, connectionType, database, null, null);
}

public OracleConnectorConfig(String host, int port, String user, String password, String jdbcPluginName,
String connectionArguments, String connectionType, String database,
String role) {
String role, Boolean useSSL) {

this.host = host;
this.port = port;
Expand All @@ -59,11 +59,12 @@ public OracleConnectorConfig(String host, int port, String user, String password
this.connectionType = connectionType;
this.database = database;
this.role = role;
this.useSSL = useSSL;
}

@Override
public String getConnectionString() {
return OracleConstants.getConnectionString(connectionType, host, getPort(), database);
return OracleConstants.getConnectionString(connectionType, host, getPort(), database, useSSL);
}

@Name(OracleConstants.CONNECTION_TYPE)
Expand All @@ -86,6 +87,11 @@ public String getConnectionString() {
@Nullable
private String transactionIsolationLevel;

@Name(OracleConstants.USE_SSL)
@Description("Turns on SSL encryption. Connection will fail if SSL is not available")
@Nullable
public Boolean useSSL;

@Override
protected int getDefaultPort() {
return 1521;
Expand All @@ -103,6 +109,11 @@ public String getDatabase() {
return database;
}

public Boolean getSSlMode() {
// return false if useSSL is null, otherwise return its value
return useSSL != null && useSSL;
}

@Override
public Properties getConnectionArgumentsProperties() {
Properties prop = super.getConnectionArgumentsProperties();
Expand Down
43 changes: 30 additions & 13 deletions oracle-plugin/src/main/java/io/cdap/plugin/oracle/OracleConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,39 +27,56 @@ private OracleConstants() {
}

public static final String PLUGIN_NAME = "Oracle";
public static final String ORACLE_CONNECTION_STRING_SID_FORMAT = "jdbc:oracle:thin:@%s:%s:%s";
public static final String ORACLE_CONNECTION_STRING_SERVICE_NAME_FORMAT = "jdbc:oracle:thin:@//%s:%s/%s";
// Updating connection strings to accept protocol (e.g., jdbc:oracle:thin:@<protocol>://<host>:<port>/<SID>)
public static final String ORACLE_CONNECTION_STRING_SID_FORMAT = "jdbc:oracle:thin:@%s:%s:%s/%s";
Copy link
Member

@itsankit-google itsankit-google Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add a constant ORACLE_CONNECTION_STRING_SID_FORMAT_WITH_PROTOCOL instead of modifying the older one and use the new one when useSSL is true otherwise use the old connection string.

This would make the change backward compatible and ensure CDAP connectors are not affected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, updated [b31e53c]

public static final String ORACLE_CONNECTION_STRING_SERVICE_NAME_FORMAT = "jdbc:oracle:thin:@%s://%s:%s/%s";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar change here ORACLE_CONNECTION_STRING_SERVICE_NAME_FORMAT_WITH_PROTOCOL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated [b31e53c]

public static final String ORACLE_CONNECTION_STRING_TNS_FORMAT = "jdbc:oracle:thin:@%s";
public static final String DEFAULT_BATCH_VALUE = "defaultBatchValue";
public static final String DEFAULT_ROW_PREFETCH = "defaultRowPrefetch";
public static final String SERVICE_CONNECTION_TYPE = "service";
public static final String CONNECTION_TYPE = "connectionType";
public static final String ROLE = "role";
public static final String NAME_DATABASE = "database";
public static final String TNS_CONNECTION_TYPE = "TNS";
public static final String TNS_CONNECTION_TYPE = "tns";
public static final String TRANSACTION_ISOLATION_LEVEL = "transactionIsolationLevel";
public static final String USE_SSL = "useSSL";

/**
* Returns the Connection String for the given ConnectionType.
* Constructs the Oracle connection string based on the provided connection type, host, port, and database.
* If SSL is enabled, the connection protocol will be "tcps" instead of "tcp".
*
* @param connectionType TNS/Service/SID
* @param host Host name of the oracle server
* @param port Port of the oracle server
* @param database Database to connect to
* @return Connection String based on the given ConnectionType
* @param useSSL Whether SSL/TLS is required(YES/NO)
* @return Connection String based on the given parameters and connection type.
*/
public static String getConnectionString(String connectionType,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot see any reason of this new method, why can't we reuse the existing one?

Copy link
Contributor Author

@Vipinofficial11 Vipinofficial11 Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was actually overloaded to reduce the impact of these changes on CDAP connectors, this method will only be used by Source and Sink plugin.

Oracle Action, Post Action and Oracle Connector will still use the old method to generate connection string. When we enable TLS for CDAP Connectors we can reuse the code and remove this method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using this method here: https://github.com/data-integrations/database-plugins/pull/515/files#diff-bd85394fe9ff353467fd45fd10ad329b508e0fbbc70145fe34de0d54f483a99bR129

It means it does impact CDAP connectors so it does not really help by overriding the method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, removed. [b68ec93]

@Nullable String host,
@Nullable int port,
String database) {
if (OracleConstants.TNS_CONNECTION_TYPE.equalsIgnoreCase(connectionType)) {
return String.format(OracleConstants.ORACLE_CONNECTION_STRING_TNS_FORMAT, database);
String database,
@Nullable Boolean useSSL) {
// Use protocol as "tcps" when SSL is requested or else use "tcp".
String connectionProtocol;
if (useSSL != null && useSSL) {
connectionProtocol = "tcps";
} else {
connectionProtocol = "tcp";
}
if (OracleConstants.SERVICE_CONNECTION_TYPE.equalsIgnoreCase(connectionType)) {
return String.format(OracleConstants.ORACLE_CONNECTION_STRING_SERVICE_NAME_FORMAT,
host, port, database);

switch (connectionType.toLowerCase()) {
case OracleConstants.TNS_CONNECTION_TYPE:
// TNS connection doesn't require protocol
return String.format(OracleConstants.ORACLE_CONNECTION_STRING_TNS_FORMAT, database);
case OracleConstants.SERVICE_CONNECTION_TYPE:
// Service connection uses protocol, host, port, and database
return String.format(OracleConstants.ORACLE_CONNECTION_STRING_SERVICE_NAME_FORMAT,
connectionProtocol, host, port, database);
default:
// Default to SID format if no matching case is found
return String.format(OracleConstants.ORACLE_CONNECTION_STRING_SID_FORMAT,
connectionProtocol, host, port, database);
}
return String.format(OracleConstants.ORACLE_CONNECTION_STRING_SID_FORMAT,
host, port, database);
}
}
2 changes: 1 addition & 1 deletion oracle-plugin/src/main/java/io/cdap/plugin/oracle/OraclePostAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static class OracleQueryActionConfig extends DBSpecificQueryActionConfig

@Override
public String getConnectionString() {
return OracleConstants.getConnectionString(this.connectionType, host, port, database);
return OracleConstants.getConnectionString(this.connectionType, host, port, database, null);
}

@Override
Expand Down
6 changes: 3 additions & 3 deletions oracle-plugin/src/main/java/io/cdap/plugin/oracle/OracleSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ public OracleSourceConfig(String host, int port, String user, String password, S
String connectionArguments, String connectionType, String database, String role,
int defaultBatchValue, int defaultRowPrefetch,
String importQuery, Integer numSplits, int fetchSize,
String boundingQuery, String splitBy) {
String boundingQuery, String splitBy, Boolean useSSL) {
this.connection = new OracleConnectorConfig(host, port, user, password, jdbcPluginName, connectionArguments,
connectionType, database, role);
connectionType, database, role, useSSL);
this.defaultBatchValue = defaultBatchValue;
this.defaultRowPrefetch = defaultRowPrefetch;
this.fetchSize = fetchSize;
Expand All @@ -132,7 +132,7 @@ public OracleSourceConfig(String host, int port, String user, String password, S
@Override
public String getConnectionString() {
return OracleConstants.getConnectionString(connection.getConnectionType(), connection.getHost(),
connection.getPort(), connection.getDatabase());
connection.getPort(), connection.getDatabase(), connection.getSSlMode());
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions oracle-plugin/src/test/java/io/cdap/plugin/oracle/OracleFailedConnectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ public class OracleFailedConnectionTest extends DBSpecificFailedConnectionTest {
public void test() throws ClassNotFoundException, IOException {

OracleConnector connector = new OracleConnector(
new OracleConnectorConfig("localhost", 1521, "username", "password", "jdbc", "", "database"));
new OracleConnectorConfig("localhost", 1521, "username", "password", "jdbc", "",
"SID", "database"));

super.test(JDBC_DRIVER_CLASS_NAME, connector, "Failed to create connection to database via connection string:" +
" jdbc:oracle:thin:@localhost:1521:database and arguments: " +
" jdbc:oracle:thin:@tcp:localhost:1521/database and arguments: " +
"{user=username, oracle.jdbc.timezoneAsRegion=false, " +
"internal_logon=normal}. Error: ConnectException: Connection " +
"refused.");
Expand Down
20 changes: 20 additions & 0 deletions oracle-plugin/widgets/Oracle-batchsink.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,26 @@
"default": "TRANSACTION_SERIALIZABLE"
}
},
{
"widget-type": "hidden",
"label": "TLS Encryption",
"name": "useSSL",
"description": "Enable TLS encryption (true/false)",
"widget-attributes": {
"layout": "inline",
"default": "false",
"options": [
{
"id": "true",
"label": "true"
},
{
"id": "false",
"label": "false"
}
]
}
},
{
"name": "connectionType",
"label": "Connection Type",
Expand Down
20 changes: 20 additions & 0 deletions oracle-plugin/widgets/Oracle-batchsource.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,26 @@
"default": "TRANSACTION_SERIALIZABLE"
}
},
{
"widget-type": "hidden",
"label": "TLS Encryption",
"name": "useSSL",
"description": "Enable TLS encryption (true/false)",
"widget-attributes": {
"layout": "inline",
"default": "false",
"options": [
{
"id": "true",
"label": "true"
},
{
"id": "false",
"label": "false"
}
]
}
},
{
"name": "connectionType",
"label": "Connection Type",
Expand Down
20 changes: 20 additions & 0 deletions oracle-plugin/widgets/Oracle-connector.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,26 @@
],
"default": "TRANSACTION_SERIALIZABLE"
}
},
{
"widget-type": "hidden",
"label": "TLS Encryption",
"name": "useSSL",
"description": "Enable TLS encryption (true/false)",
"widget-attributes": {
"layout": "inline",
"default": "false",
"options": [
{
"id": "true",
"label": "true"
},
{
"id": "false",
"label": "false"
}
]
}
}
]
},
Expand Down
Loading