Skip to content

feat: [#22155] Optional SSL support for Redshift plugin #22844

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

Open
harshmange44 wants to merge 2 commits into
base: release
Choose a base branch
from
Open

feat: [#22155] Optional SSL support for Redshift plugin #22844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6304825
- Optional SSL support for Redshift plugin
harshmange44 Apr 28, 2023
7bef32b
Removed hidden, ca verification in form.json
harshmange44 May 5, 2023
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
4 changes: 4 additions & 0 deletions ...s/redshiftPlugin/src/main/java/com/external/plugins/exceptions/RedshiftErrorMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,8 @@ private RedshiftErrorMessages() {
public static final String JDBC_DRIVER_LOADING_ERROR_MSG = "Error loading Redshift JDBC Driver class.";

public static final String CONNECTION_POOL_CREATION_FAILED_ERROR_MSG = "Exception occurred while creating connection pool. One or more arguments in the datasource configuration may be invalid. Please check your datasource configuration.";

public static final String SSL_CONFIGURATION_ERROR_MSG = "The Appsmith server has failed to fetch SSL configuration from datasource configuration form. ";

public static final String INVALID_SSL_OPTION_ERROR_MSG = "The Appsmith server has found an unexpected SSL option: %s.";
}
10 changes: 10 additions & 0 deletions ...ins/redshiftPlugin/src/main/java/com/external/plugins/exceptions/RedshiftPluginError.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ public enum RedshiftPluginError implements BasePluginError {
ErrorType.INTERNAL_ERROR,
"{1}",
"{2}"
),
REDSHIFT_PLUGIN_ERROR(
500,
"PE-RED-5001",
"{0}",
AppsmithErrorAction.LOG_EXTERNALLY,
"Query execution error",
ErrorType.INTERNAL_ERROR,
"{1}",
"{2}"
)
;

Expand Down
40 changes: 40 additions & 0 deletions ...mith-plugins/redshiftPlugin/src/main/java/com/external/utils/RedshiftDatasourceUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.appsmith.external.models.DBAuth;
import com.appsmith.external.models.DatasourceConfiguration;
import com.external.plugins.exceptions.RedshiftErrorMessages;
import com.external.plugins.exceptions.RedshiftPluginError;
import com.zaxxer.hikari.HikariConfig;
import com.zaxxer.hikari.HikariDataSource;
import com.zaxxer.hikari.pool.HikariPool;
Expand Down Expand Up @@ -58,6 +59,45 @@ public static HikariDataSource createConnectionPool(DatasourceConfiguration data
urlBuilder.append(authentication.getDatabaseName());
}

/*
* - Ideally, it is never expected to be null because the SSL dropdown is set to
* a initial value.
*/
if (datasourceConfiguration.getConnection() == null
|| datasourceConfiguration.getConnection().getSsl() == null
|| datasourceConfiguration.getConnection().getSsl().getAuthType() == null) {
throw new AppsmithPluginException(
RedshiftPluginError.REDSHIFT_PLUGIN_ERROR,
RedshiftErrorMessages.SSL_CONFIGURATION_ERROR_MSG);
}

/*
* - By default, the driver configures SSL in the preferred mode.
*/
SSLDetails.AuthType sslAuthType = datasourceConfiguration.getConnection().getSsl().getAuthType();
switch (sslAuthType) {
case ALLOW:
case PREFER:
case REQUIRE:
config.addDataSourceProperty("ssl", "true");
config.addDataSourceProperty("sslmode", sslAuthType.toString().toLowerCase());

break;
case DISABLE:
config.addDataSourceProperty("ssl", "false");
config.addDataSourceProperty("sslmode", sslAuthType.toString().toLowerCase());

break;
case DEFAULT:
/* do nothing - accept default driver setting */

break;
default:
throw new AppsmithPluginException(
RedshiftPluginError.REDSHIFT_PLUGIN_ERROR,
String.format(RedshiftErrorMessages.INVALID_SSL_OPTION_ERROR_MSG, sslAuthType));
}

String url = urlBuilder.toString();
config.setJdbcUrl(url);

Expand Down
14 changes: 5 additions & 9 deletions app/server/appsmith-plugins/redshiftPlugin/src/main/resources/form.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,17 @@
{
"id": 3,
"sectionName": "SSL (optional)",
"hidden": true,
"children": [
{
"label": "SSL Mode",
"configProperty": "datasourceConfiguration.connection.ssl.authType",
"controlType": "DROP_DOWN",
"initialValue": "DEFAULT",
"options": [
{
"label": "Default",
"value": "DEFAULT"
},
Copy link
Copy Markdown
Contributor

@sumitsum sumitsum May 4, 2023

Choose a reason for hiding this comment

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

Can you please remove:
(1) "hidden": true
This should make the SSL dropdown visible on the Redshift datasource config page.
(2) Options that do not apply e.g. verify-ca, verify-full

Copy link
Copy Markdown
Contributor Author

@harshmange44 harshmange44 May 9, 2023

Choose a reason for hiding this comment

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

Fixed this @sumitsum

Copy link
Copy Markdown
Contributor

@sumitsum sumitsum May 4, 2023

Choose a reason for hiding this comment

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

Can you please add a small screen recording in the description showing how this looks and works ?

Copy link
Copy Markdown
Contributor Author

@harshmange44 harshmange44 May 9, 2023

Choose a reason for hiding this comment

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

Sure, will try to do it in a day or two. Currently facing issues due to lack of enough resources. I saw some of the similar plugins having this <label, value> in form.json

{
"label": "No SSL",
"value": "NO_SSL"
Expand All @@ -102,14 +106,6 @@
{
"label": "Disable",
"value": "DISABLE"
},
{
"label": "Verify-CA",
"value": "VERIFY_CA"
},
{
"label": "Verify-Full",
"value": "VERIFY_FULL"
}
]
},
Expand Down