feat: Added SSH Tunnel for Postgres #35449

AnnaHariprasad5123 · 2024-08-06T10:40:55Z

@appsmithorg/contributor-support

Hi @rohan-arthur

Fixes #30792

Video Demonstration

What's in this pr :

Added connection mode option for SSH Tunnel.
Added SSH Host, Port, SSH Username and SSH key fields.
Added Validation logic for SSH fields.
Added SSH connection logic.
Added three test cases for SSH connection.

Screenshots :

Please review this pr. Let me know if any changes required.

Summary by CodeRabbit

New Features
- Introduced SSH tunneling support for PostgreSQL connections, allowing users to configure SSH settings for enhanced security.
- Added a new "Connection method" option in the configuration, enabling selection between "Standard" and "SSH tunnel" connections, with relevant fields for SSH details.
Bug Fixes
- Improved error handling by adding specific error messages for missing SSH configuration parameters, providing clearer feedback to users.
- Updated error messages for authentication issues to include missing passwords.
Tests
- Enhanced test coverage with new unit tests for validating the configuration of PostgreSQL data sources, particularly for SSH connections.

coderabbitai · 2024-08-06T10:41:04Z

Walkthrough

The recent changes to the PostgreSQL plugin implement SSH tunneling, enabling secure connections to databases behind an SSH server. This update introduces new configuration options for SSH, expands validation for connection parameters, and enhances error handling, facilitating a seamless integration of these features for users needing secure access to production databases.

Changes

Files	Change Summary
`.../PostgresPlugin.java` `.../PostgresErrorMessages.java`	Introduced SSH tunneling support with new configuration parameters, validation logic, and error messages related to SSH settings.
`.../form.json`	Enhanced configuration schema to include SSH tunneling options, conditionally displaying SSH-related fields based on user selection.
`.../PostgresDatasourceValidationTest.java`	Added comprehensive unit tests to validate the configuration of PostgreSQL data sources, focusing on standard and SSH connection methods.
`.../PostgresPluginTest.java`	Enhanced test suite for SSH connections, validating endpoint identification and ensuring correct error handling for missing credentials.
`.../SSHConnection.java`	Updated field annotations to broaden serialization views for SSH-related fields.

Assessment against linked issues

Objective	Addressed	Explanation
Implement SSH tunneling for PostgreSQL (Feature Request #30792)	✅
Enhance security for accessing production databases (Feature Request #30792)	✅

Possibly related issues

[Bug]: MySql datasource connection error - Authentication failed ([Bug]: MySql datasource connection error - Authentication failed #35328): The changes in the PostgreSQL plugin regarding SSH tunneling and enhanced error messages may provide insights or parallels for addressing authentication issues in MySQL connections.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai anywhere in the PR title to generate the title automatically.
Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 4

Outside diff range, codebase verification and nitpick comments (6)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1)
40-41: Add Javadoc for the new constant.

Consider adding a Javadoc comment to describe the purpose of this constant for better maintainability and readability.
/**
 * Error message for missing SSH port in the datasource configuration.
 */
public static final String DS_MISSING_SSH_PORT_ERROR_MSG = "Missing SSH port.";
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json (3)
15-29: Ensure consistent naming for the connection method values.

The values for the connection method options should be consistent. Consider using lowercase or uppercase for both options.
-              "value": "STANDARD"
+              "value": "standard"
...
-              "value": "SSH"
+              "value": "ssh"
67-92: Add a section name for the SSH configuration.

For better readability and organization, consider adding a section name for the SSH configuration.
-        {
-          "sectionName": null,
+        {
+          "sectionName": "SSH Configuration",
124-143: Ensure consistent naming for the SSH-related fields.

The placeholder text for the SSH username and SSH key fields should be consistent with other fields.
-              "placeholderText": "Username",
+              "placeholderText": "SSH Username",
...
-              "placeholderText": "Password",
+              "placeholderText": "SSH Key",
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (2)
12-16: Group SSH-related imports together.

For better readability, group all SSH-related imports together.
+import com.appsmith.external.helpers.SSHUtils;
...
+import com.appsmith.external.models.ConnectionContext;
Line range hint 313-330: Improve readability of the endpoint identifier logic.

The logic for constructing the endpoint identifier can be simplified for better readability.
-            if (endpoints.size() > 0) {
-                String hostName = endpoints.get(0).getHost();
-                Long port = endpoints.get(0).getPort();
-                if (!isBlank(hostName)) {
-                    identifier = hostName + "_" + ObjectUtils.defaultIfNull(port, DEFAULT_POSTGRES_PORT);
-                }
-            }
+            if (!endpoints.isEmpty()) {
+                Endpoint endpoint = endpoints.get(0);
+                String hostName = endpoint.getHost();
+                Long port = endpoint.getPort();
+                if (!isBlank(hostName)) {
+                    identifier = hostName + "_" + ObjectUtils.defaultIfNull(port, DEFAULT_POSTGRES_PORT);
+                }
+            }

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between cbb4568 and 217ffc3aa7b0b1e9841b01016ed2f048cae7b20a.

Files selected for processing (4)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (7 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json (9 hunks)
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java (1 hunks)

Additional comments not posted (4)

app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java (4)

1723-1744: LGTM!

The test case testGetEndpointIdentifierForRateLimit_HostPresentPortAbsentSshEnabled_ReturnsCorrectString is well-structured and correctly tests the expected behavior.

1746-1773: LGTM!

The test case testGetEndpointIdentifierForRateLimit_HostPresentPortAbsentSshEnabledwithHostAndPort_ReturnsCorrectString is well-structured and correctly tests the expected behavior.

1775-1801: LGTM!

The test case testGetEndpointIdentifierForRateLimit_HostPresentPortAbsentSshEnabledwithHostAndNullPort_ReturnsCorrectString is well-structured and correctly tests the expected behavior.

1803-1828: LGTM!

The test case testGetEndpointIdentifierForRateLimit_EndpointAbsentSshEnabledwithHostAndNullPort_ReturnsCorrectString is well-structured and correctly tests the expected behavior.

app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json

...erver/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 217ffc3aa7b0b1e9841b01016ed2f048cae7b20a and 8391d3c.

Files selected for processing (4)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (7 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json (9 hunks)
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java (1 hunks)

Files skipped from review as they are similar to previous changes (3)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java

Additional comments not posted (1)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1)

40-41: LGTM! The new error message is clear and concise.

The added error message DS_MISSING_SSH_PORT_ERROR_MSG aligns well with the existing error messages.

NilanshBansal · 2024-08-11T07:36:32Z

@AnnaHariprasad5123 the server build is failing after pulling your changes. Can you check this.

Ref: https://github.com/appsmithorg/appsmith/actions/runs/10337557263/job/28614567378?pr=35589

AnnaHariprasad5123 · 2024-08-11T12:15:25Z

Hi @NilanshBansal, Could you check it now.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8391d3c and 78d673e.

Files selected for processing (4)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (8 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json (9 hunks)
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java (1 hunks)

Files skipped from review as they are similar to previous changes (3)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java

Additional comments not posted (1)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1)
40-41: Addition of SSH Port Error Message Approved.

The new error message DS_MISSING_SSH_PORT_ERROR_MSG is a necessary addition for handling SSH tunneling errors. Ensure that this message is used wherever SSH port validation is performed.

To confirm its usage, verify where this constant is referenced in the codebase.

Verification successful

SSH Port Error Message Usage Verified.

The error message DS_MISSING_SSH_PORT_ERROR_MSG is correctly referenced in PostgresPlugin.java for handling validation errors related to missing SSH ports. This confirms its intended use in the codebase.

File: PostgresPlugin.java

Line: Adds the error message to a list of invalids.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify usage of DS_MISSING_SSH_PORT_ERROR_MSG in the codebase.

# Test: Search for references to DS_MISSING_SSH_PORT_ERROR_MSG. Expect: Occurrences in validation logic.
rg --type java 'DS_MISSING_SSH_PORT_ERROR_MSG'
Length of output: 431

NilanshBansal · 2024-08-12T03:42:42Z

Server Build is still failing @AnnaHariprasad5123.

https://github.com/appsmithorg/appsmith/actions/runs/10341600007/job/28623586632?pr=35589

AnnaHariprasad5123 · 2024-08-12T03:54:37Z

HI @NilanshBansal, Sorry for the mistake. Could you check it now.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 78d673e and 2f95854.

Files selected for processing (3)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (8 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json (9 hunks)

Files skipped from review as they are similar to previous changes (3)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json

NilanshBansal · 2024-08-12T04:01:40Z

@AnnaHariprasad5123 spotless check is failing, can you run mvn spotless:apply and commit the changes. You can see the failing tests on the current PR or the cloned PR
#35589

NilanshBansal · 2024-08-12T04:07:26Z

@AnnaHariprasad5123 also, is there any specific reason to do git force push? Can you also retain all the commits and just make new commits with the suggested changes.
For all the new commits, please do not force push and just keep adding commits with the new changes, so that we can also track changes

AnnaHariprasad5123 · 2024-08-12T04:10:01Z

@AnnaHariprasad5123 also, is there any specific reason to do git force push? Can you also retain all the commits and just make new commits with the suggested changes.

Because those are small changes so I am doing git amend and force push. From now, I will do new commits for every change. I will not repeat this again.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2f95854 and bff288d.

Files selected for processing (4)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (8 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json (9 hunks)
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java (2 hunks)

Files skipped from review as they are similar to previous changes (3)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java

Additional comments not posted (3)

app/server/appsmith-plugins/postgresPlugin/src/main/resources/form.json (3)

14-29: Addition of "Connection method" is appropriate.

The new "Connection method" field allows users to select between "Standard" and "SSH tunnel," aligning with the PR's objective to enhance security via SSH tunneling.

67-91: SSH configuration fields are well-implemented.

The SSH host address and port fields are conditionally displayed, enhancing the user experience by showing relevant fields based on the connection method.

122-143: SSH authentication fields are correctly added.

The SSH username and key fields are conditionally visible, which is essential for SSH tunneling configuration.

NilanshBansal · 2024-08-12T06:44:24Z

@AnnaHariprasad5123 cypress tests are failing on the shadow PR. Can you fix these.

#35591 (comment)

AnnaHariprasad5123 · 2024-08-12T10:34:27Z

HI @NilanshBansal, Cypress tests are crashing in my system. I am unable to check it. Could you check it if possible. I didn't change any cypress tests file. I have checked with others also in my team. Cypress tests are crashing after sometime. Could you provide a suggestion or idea.

NilanshBansal · 2024-08-13T03:27:46Z

@AnnaHariprasad5123 can you share the error screenshot that you are getting on running cypress locally?
Also, you can even run one spec file locally to reduce the system load

AnnaHariprasad5123 · 2024-08-13T04:31:55Z

Hi @NilanshBansal,

Good morning. Yes, I am running individual tests only. I’m encountering issues with the following tests:

ConversionFlow_Generated_App_spec.ts (cypress/e2e/Regression/ClientSide/MobileResponsiveTests/) : This test fails because it is unable to click the "select table" option from the users database. It looks like disable state.
PropertyControl_spec.ts (cypress/e2e/Regression/ClientSide/OneClickBinding/) : I am experiencing the same issue in the first test case, and from the second test case onward, Chrome starts crashing.
EntityBottomBar_spec.ts (cypress/e2e/Regression/ClientSide/OtherUIFeatures/EntityBottomBar_spec.ts) : 3rd and 4th testcases didn't get the response from the query so they fails. When 5th testcase runs chrome starts crashing.
Generate_Crud_New_Page_spec.ts (cypress/e2e/Smoke/GenerateCRUD/Generate_Crud_New_Page_spec.ts) : This test fails because it is unable to click the "select table" option from the users database. Also not getting success response from datasource creation.

sneha122 · 2024-08-14T07:18:37Z

/build-deploy-preview skip-tests=true

AnnaHariprasad5123 · 2024-08-27T10:02:48Z

Hi @NilanshBansal, We are not getting missing ssh port and ssh host value . Is this issue same for mysql plugin? Could you check mysql plugin once.

I have checked in mysql ssh, so it is also getting as same :

NilanshBansal · 2024-08-27T10:06:00Z

Hi @NilanshBansal, SSH port and key are not getting right in postgres. Is this issue same for mysql plugin? Could you check mysql plugin once.

@AnnaHariprasad5123 I believe that is not concerned with the scope of this implementation. I will check it but it should be independent of our current implementation.

AnnaHariprasad5123 · 2024-08-27T13:10:05Z

Added Basic Sanity tests.
Added missing password validation and constant.
Removed missing port validation and missing port constant.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b684895 and 580f5fc.

Files selected for processing (3)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (24 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (2 hunks)
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresDatasourceValidationTest.java (1 hunks)

Additional comments not posted (14)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1)

49-49: LGTM!

The new constant DS_MISSING_PASSWORD_ERROR_MSG is correctly defined and enhances error reporting.

The code changes are approved.

app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresDatasourceValidationTest.java (10)

85-92: LGTM!

The test case testErrorMessageOnEmptySSHHost is correctly implemented and verifies the expected error message.

The code changes are approved.

95-102: LGTM!

The test case testErrorMessageOnBadSSHHost is correctly implemented and verifies the expected error message.

The code changes are approved.

105-111: LGTM!

The test case testErrorMessageOnEmptySSHPort is correctly implemented and verifies the expected behavior.

The code changes are approved.

113-121: LGTM!

The test case testErrorMessageOnEmptySSHUsername is correctly implemented and verifies the expected error message.

The code changes are approved.

125-131: LGTM!

The test case testErrorMessageOnEmptySSHKey is correctly implemented and verifies the expected error message.

The code changes are approved.

133-145: LGTM!

The test case testValidateDatasourceNullCredentials is correctly implemented and verifies the expected error messages.

The code changes are approved.

148-153: LGTM!

The test case testValidateDatasourceMissingDBName is correctly implemented and verifies the expected error message.

The code changes are approved.

156-161: LGTM!

The test case testValidateDatasourceNullEndpoint is correctly implemented and verifies the expected error message.

The code changes are approved.

164-169: LGTM!

The test case testValidateDatasource_NullHost is correctly implemented and verifies the expected error message.

The code changes are approved.

172-177: LGTM!

The test case testValidateDatasourceInvalidEndpoint is correctly implemented and verifies the expected error message.

The code changes are approved.

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (3)

Line range hint 237-291: LGTM!

The method executeParameterized is correctly implemented and handles SSH connections appropriately.

The code changes are approved.

Line range hint 312-329: LGTM!

The method getEndpointIdentifierForRateLimit is correctly implemented and includes SSH host in the identifier appropriately.

The code changes are approved.

726-750: LGTM!

The method validateDatasource is correctly implemented and includes checks for SSH parameters appropriately.

The code changes are approved.

NilanshBansal · 2024-08-28T05:27:32Z

@AnnaHariprasad5123 there are merge conflicts in this PR, can you please resolve them.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 580f5fc and b016af6.

Files selected for processing (3)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java (9 hunks)
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1 hunks)
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresDatasourceValidationTest.java (1 hunks)

Files skipped from review as they are similar to previous changes (2)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresDatasourceValidationTest.java

Additional comments not posted (1)

app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/exceptions/PostgresErrorMessages.java (1)

49-50: LGTM!

The new constant DS_MISSING_PASSWORD_ERROR_MSG is correctly implemented and follows the existing pattern in the file. It enhances error reporting for PostgreSQL connections.

The code changes are approved.

NilanshBansal · 2024-08-28T13:32:06Z

@AnnaHariprasad5123 newly added server test is failing on the shadow PR here.

#35591 (comment)

AnnaHariprasad5123 · 2024-08-28T16:54:28Z

Hi @NilanshBansal, check once and let me know if still having an issues.

fix testcase
fix ssh values are missing in datasources api

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b016af6 and e177c1c.

Files selected for processing (2)

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/SSHConnection.java (3 hunks)
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java (3 hunks)

Additional comments not posted (11)

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/SSHConnection.java (6)

28-28: LGTM!

The host field's annotation is correctly updated to include FromRequest.class.

The code changes are approved.

31-31: LGTM!

The port field's annotation is correctly updated to include FromRequest.class.

The code changes are approved.

34-34: LGTM!

The endpoints field's annotation is correctly updated to include FromRequest.class.

The code changes are approved.

46-46: LGTM!

The username field's annotation is correctly updated to include FromRequest.class.

The code changes are approved.

49-49: LGTM!

The authType field's annotation is correctly updated to include FromRequest.class.

The code changes are approved.

52-52: LGTM!

The privateKey field's annotation is correctly updated to include FromRequest.class.

The code changes are approved.

app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java (5)

338-338: LGTM!

The assertion change enhances the clarity and specificity of the test's intent.

The code changes are approved.

1725-1745: LGTM!

The test case is correctly implemented to validate the behavior of the getEndpointIdentifierForRateLimit method when the host is set but the port is null, with SSH enabled.

The code changes are approved.

1747-1774: LGTM!

The test case is correctly implemented to validate the behavior of the getEndpointIdentifierForRateLimit method when both the host and SSH proxy details are provided.

The code changes are approved.

1776-1802: LGTM!

The test case is correctly implemented to validate the behavior of the getEndpointIdentifierForRateLimit method when the host is set but the port is null, with SSH enabled and SSH proxy details provided.

The code changes are approved.

1804-1829: LGTM!

The test case is correctly implemented to validate the behavior of the getEndpointIdentifierForRateLimit method when no endpoints are defined but an SSH proxy is set.

The code changes are approved.

NilanshBansal · 2024-08-29T04:41:59Z

/build-deploy-preview skip-tests=true

github-actions · 2024-08-29T04:42:22Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10608827308.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 35449.
recreate: .

github-actions · 2024-08-29T04:49:18Z

Deploy-Preview-URL: https://ce-35449.dp.appsmith.com

NilanshBansal · 2024-08-29T06:10:50Z

@AnnaHariprasad5123 the following cypress tests are failing. Can you please check and fix these if caused by your recent changes.
#35591 (comment)

cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts

7350993.mp4

cypress/e2e/Regression/ServerSide/QueryPane/EmptyDataSource_spec.js

7350994.mp4

AnnaHariprasad5123 · 2024-08-29T07:44:56Z

Hi @NilanshBansal, I fixed them. Check workflows once and let me know if issues still remain.

cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts : docker.internal url is not working now.
cypress/e2e/Regression/ServerSide/QueryPane/EmptyDataSource_spec.js :

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e177c1c and d3d85c7.

Files selected for processing (2)

app/client/cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts (1 hunks)
app/client/cypress/e2e/Regression/ServerSide/QueryPane/EmptyDataSource_spec.js (1 hunks)

Additional context used

Path-based instructions (2)

app/client/cypress/e2e/Regression/ServerSide/QueryPane/EmptyDataSource_spec.js (1)

Pattern app/client/cypress/**/**.*:

app/client/cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts (1)

Pattern app/client/cypress/**/**.*:

Additional comments not posted (2)

app/client/cypress/e2e/Regression/ServerSide/QueryPane/EmptyDataSource_spec.js (1)

26-26: LGTM!

The error message now includes "Missing password for authentication," which improves clarity.

The code changes are approved.

app/client/cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts (1)

48-48: LGTM!

The error message now includes "Missing password for authentication," which simplifies the error handling logic.

The code changes are approved.

NilanshBansal · 2024-08-29T10:12:19Z

@AnnaHariprasad5123 Thanks for taking the time and contributing to Appsmith! We highly appreciate your effort in making Appsmith better ❤️
You can find more issues to contribute from this list! Looking forward to reviewing more PRs!

coderabbitai bot reviewed Aug 6, 2024

View reviewed changes

AnnaHariprasad5123 force-pushed the Issue-#30792/feat-SSH-Tunnel-for-postgres branch from 217ffc3 to 8391d3c Compare August 6, 2024 11:59

coderabbitai bot reviewed Aug 6, 2024

View reviewed changes

NilanshBansal requested a review from a team August 6, 2024 19:53

carinanfonseca added the Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. label Aug 9, 2024

NilanshBansal added the Community Contributor Meant to track issues that are assigned to external contributors label Aug 10, 2024

NilanshBansal mentioned this pull request Aug 11, 2024

chore: dummy PR to run tests on #35449 #35589

Closed

2 tasks

AnnaHariprasad5123 force-pushed the Issue-#30792/feat-SSH-Tunnel-for-postgres branch from 8391d3c to 78d673e Compare August 11, 2024 12:14

coderabbitai bot reviewed Aug 11, 2024

View reviewed changes

NilanshBansal requested review from NilanshBansal and removed request for a team August 12, 2024 03:42

AnnaHariprasad5123 force-pushed the Issue-#30792/feat-SSH-Tunnel-for-postgres branch from 78d673e to 2f95854 Compare August 12, 2024 03:54

coderabbitai bot reviewed Aug 12, 2024

View reviewed changes

feat: Added SSH Tunnel for Postgres

bff288d

AnnaHariprasad5123 force-pushed the Issue-#30792/feat-SSH-Tunnel-for-postgres branch from 2f95854 to bff288d Compare August 12, 2024 04:07

coderabbitai bot reviewed Aug 12, 2024

View reviewed changes

NilanshBansal mentioned this pull request Aug 12, 2024

chore: PR to run tests on external contribution #35449 #35591

Closed

2 tasks

NilanshBansal self-assigned this Aug 27, 2024

added-basic-sanity-tests

580f5fc

AnnaHariprasad5123 requested review from nidhi-nair and sondermanish as code owners August 27, 2024 13:09

coderabbitai bot reviewed Aug 27, 2024

View reviewed changes

NilanshBansal removed request for nidhi-nair and sondermanish August 28, 2024 05:26

resolve-merge-conflicts

b016af6

coderabbitai bot reviewed Aug 28, 2024

View reviewed changes

fix-test-case

e177c1c

coderabbitai bot reviewed Aug 28, 2024

View reviewed changes

NilanshBansal requested a review from sneha122 August 29, 2024 05:57

fix-cypress-tests

d3d85c7

AnnaHariprasad5123 requested review from ApekshaBhosale and sagar-qa007 as code owners August 29, 2024 07:43

coderabbitai bot reviewed Aug 29, 2024

View reviewed changes

NilanshBansal approved these changes Aug 29, 2024

View reviewed changes

nerbos merged commit f3374bd into appsmithorg:release Aug 29, 2024
18 of 19 checks passed

This pull request was closed.

feat: Added SSH Tunnel for Postgres #35449

feat: Added SSH Tunnel for Postgres #35449

Conversation

AnnaHariprasad5123 commented Aug 6, 2024 • edited by coderabbitai bot Loading

Summary by CodeRabbit

Summary by CodeRabbit

coderabbitai bot commented Aug 6, 2024 • edited Loading

Walkthrough

Changes

Assessment against linked issues

Possibly related issues

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Aug 11, 2024

AnnaHariprasad5123 commented Aug 11, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Aug 12, 2024

AnnaHariprasad5123 commented Aug 12, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Aug 12, 2024 • edited Loading

NilanshBansal commented Aug 12, 2024 • edited Loading

AnnaHariprasad5123 commented Aug 12, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Aug 12, 2024

AnnaHariprasad5123 commented Aug 12, 2024 • edited Loading

NilanshBansal commented Aug 13, 2024

AnnaHariprasad5123 commented Aug 13, 2024

sneha122 commented Aug 14, 2024

AnnaHariprasad5123 commented Aug 27, 2024 • edited Loading

NilanshBansal commented Aug 27, 2024

AnnaHariprasad5123 commented Aug 27, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Aug 28, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Aug 28, 2024

AnnaHariprasad5123 commented Aug 28, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Aug 29, 2024

github-actions bot commented Aug 29, 2024

github-actions bot commented Aug 29, 2024

NilanshBansal commented Aug 29, 2024

AnnaHariprasad5123 commented Aug 29, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

NilanshBansal commented Aug 29, 2024

AnnaHariprasad5123 commented Aug 6, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Aug 6, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

NilanshBansal commented Aug 12, 2024 •

edited

Loading

NilanshBansal commented Aug 12, 2024 •

edited

Loading

AnnaHariprasad5123 commented Aug 12, 2024 •

edited

Loading

AnnaHariprasad5123 commented Aug 27, 2024 •

edited

Loading