Remove SSL toggle for source-postgres in OSS by nguyenaiden · Pull Request #24859 · airbytehq/airbyte

Duy Nguyen (nguyenaiden) · 2023-04-04T19:44:11Z

What

Issue
Currently for DB sources that have a strict encrypt variant, the connection will fail even if the user has selected, for example, verify-ca and filled out all relevant information. We want to mitigate confusion by having the ssl_mode be the sole source of truth for enabling/disabling SSL connection.

Solution

Remove the ssl key from spec.json for source-postgres and rely on ssl_mode key as the source of truth. This also entails removing any reference to the ssl key in the protocol. Note: this needs to be backwards compatible with historical config file. We will need to ignore the ssl key and only honor ssl_mode.

Aside from automated tests, manual tests conducted:

Successfully synced between an SSL only DB source with the config shown below before the change
Made the code change to remove the toggle and successfully synced between the same DB source and destination

Duy Nguyen (nguyenaiden) · 2023-04-04T19:44:40Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4611700148
❌ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4611700148
🐛 https://gradle.com/s/5z7gkai36lweu

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestSpec::test_match_expected[inputs0] - AssertionError:...
FAILED test_core.py::TestSpec::test_match_expected[inputs1] - AssertionError:...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
============== 2 failed, 68 passed, 3 skipped in 97.55s (0:01:37) ==============

github-actions · 2023-04-04T19:48:04Z

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

Run integration tests
Bump connector or module version
Add changelog
Publish the new version

✅ Sources (3)

Connector	Version	Changelog	Publish
`source-alloydb`	`2.0.19`	✅	✅
`source-alloydb-strict-encrypt`	`2.0.19`	🔵 (ignored)	🔵 (ignored)
`source-postgres-strict-encrypt`	`2.0.20`	🔵 (ignored)	🔵 (ignored)

See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (0)

Connector	Version	Changelog	Publish

See "Actionable Items" below for how to resolve warnings and errors.

✅ Other Modules (0)

Actionable Items

(click to expand)

Category	Status	Actionable Item
Version	❌ mismatch	The version of the connector is different from its normal variant. Please bump the version of the connector.
	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
Changelog	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
	❌ changelog missing	There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish	⚠ not in seed	The connector is not in the seed file (e.g. `source_definitions.yaml`), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.
	❌ diff seed version	The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the `/publish` command to publish the latest version.

Duy Nguyen (nguyenaiden) · 2023-04-04T20:53:53Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4612238711
❌ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4612238711
🐛 https://gradle.com/s/b6mzkmvzjohza

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestSpec::test_match_expected[inputs0] - AssertionError:...
FAILED test_core.py::TestSpec::test_match_expected[inputs1] - AssertionError:...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
============= 2 failed, 68 passed, 3 skipped in 112.42s (0:01:52) ==============

Duy Nguyen (nguyenaiden) · 2023-04-05T18:51:33Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4621836102

Duy Nguyen (nguyenaiden) · 2023-04-05T19:33:58Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4622163800
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4622163800
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
================== 70 passed, 3 skipped in 133.63s (0:02:13) ===================

Duy Nguyen (nguyenaiden) · 2023-04-05T20:59:18Z

/test connector=connectors/source-postgres

Duy Nguyen (nguyenaiden) · 2023-04-05T20:59:27Z

/test connector=connectors/source-postgres-strict-encrypt

🕑 connectors/source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/4622796889
✅ connectors/source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/4622796889
No Python unittests run

Build Passed

Test summary info:

All Passed

Rodi Reich Zilberman (rodireich) · 2023-04-05T21:19:28Z

Does it work without PARAM_SSL?
I think it should but to verify that it's not needed on either disabled or enabled modes

Just manually verified with a successful sync without SSL.

Why exactly are we removing this again?

The toggle is logically redundant.
Setting ssl mode to disabled has the same effect of setting this toggle to false.

Rodi Reich Zilberman (rodireich) · 2023-04-05T21:21:24Z

Also in source-postgres-strict-encrypt I think we're removing this ssl toggle programmatically.
Maybe cleanup from there altogether (it's something Lake Mossman (@lmossman) is also working on - maybe already done).

Lake Mossman (lmossman) · 2023-04-05T21:23:32Z

I'm not working on this anymore, but I agree that since this field is being removed from the spec, then this line that removes it for strict encrypt specifically should be removed:

airbyte/airbyte-integrations/connectors/source-postgres-strict-encrypt/src/main/java/io/airbyte/integrations/source/postgres_strict_encrypt/PostgresSourceStrictEncrypt.java

Line 42 in 9fb381d

properties.remove(JdbcUtils.SSL_KEY);

Duy Nguyen (nguyenaiden) · 2023-04-05T22:08:35Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4623306501
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4623306501
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
=================== 70 passed, 3 skipped in 97.78s (0:01:37) ===================

Duy Nguyen (nguyenaiden) · 2023-04-05T22:08:43Z

/test connector=connectors/source-postgres-strict-encrypt

🕑 connectors/source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/4623307062
✅ connectors/source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/4623307062
No Python unittests run

Build Passed

Test summary info:

All Passed

Updated expected_spec to match spec

…" from properties

This reverts commit 14ff20d.

Rodi Reich Zilberman (rodireich) · 2023-04-11T18:40:49Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4670902444
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/4670902444
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:578: The previous and actual discovered catalogs are identical.
=================== 70 passed, 3 skipped in 96.63s (0:01:36) ===================

Rodi Reich Zilberman (rodireich) · 2023-04-11T19:25:03Z

/publish connector=connectors/source-postgres

🕑 Publishing the following connectors:
connectors/source-postgres
https://github.com/airbytehq/airbyte/actions/runs/4671246602

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

Rodi Reich Zilberman (rodireich) · 2023-04-11T19:25:41Z

/publish connector=connectors/source-postgres-strict-encrypt auto-bump-version=false

🕑 Publishing the following connectors:
connectors/source-postgres-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/4671251018

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres-strict-encrypt	✅	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

* Basic removal * Removed SSL key static variables in PostgresSource Updated expected_spec to match spec * Removed autocommit for SelectDB * Removed the line in PostgresSourceStrictEncrypt that removed "SSL_KEY" from properties * Revert "Removed autocommit for SelectDB" This reverts commit 14ff20d. * bumped Dockerfile versions and updated changelog * Reverted removal of SSL key from the JDBC Connection query * Removed extraneous keys * auto-bump connector version --------- Co-authored-by: Rodi Reich Zilberman <867491+rodireich@users.noreply.github.com> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

Octavia Squidington III (octavia-squidington-iii) added area/connectors Connector related issues connectors/source/postgres labels Apr 4, 2023

Duy Nguyen (nguyenaiden) force-pushed the duy/24680 branch from 539b9c7 to df2a642 Compare April 5, 2023 18:49

Duy Nguyen (nguyenaiden) marked this pull request as ready for review April 5, 2023 18:51

Duy Nguyen (nguyenaiden) requested a review from a team as a code owner April 5, 2023 18:51

Duy Nguyen (nguyenaiden) changed the title ~~Remove SSL toggle for sources with SSL Mode dropdown~~ [WIP] Remove SSL toggle for sources with SSL Mode dropdown Apr 5, 2023

Duy Nguyen (nguyenaiden) force-pushed the duy/24680 branch from 2466ca2 to 144f9a6 Compare April 5, 2023 19:33

Duy Nguyen (nguyenaiden) force-pushed the duy/24680 branch 2 times, most recently from 6f4271e to e6a98ee Compare April 5, 2023 20:14

Duy Nguyen (nguyenaiden) changed the title ~~[WIP] Remove SSL toggle for sources with SSL Mode dropdown~~ Remove SSL toggle for sources with SSL Mode dropdown Apr 5, 2023

Duy Nguyen (nguyenaiden) changed the title ~~Remove SSL toggle for sources with SSL Mode dropdown~~ Remove SSL toggle for source-postgres Apr 5, 2023

Rodi Reich Zilberman (rodireich) reviewed Apr 5, 2023

View reviewed changes

Duy Nguyen (nguyenaiden) force-pushed the duy/24680 branch from 2a60551 to 3a3ec86 Compare April 5, 2023 21:38

Octavia Squidington III (octavia-squidington-iii) added the connectors/source/postgres-strict-encrypt label Apr 5, 2023

nguyenaiden added 4 commits April 5, 2023 15:20

Basic removal

42b2fb3

Removed SSL key static variables in PostgresSource

e5f8d05

Updated expected_spec to match spec

Removed autocommit for SelectDB

14ff20d

Removed the line in PostgresSourceStrictEncrypt that removed "SSL_KEY…

1a12e87

…" from properties

Duy Nguyen (nguyenaiden) force-pushed the duy/24680 branch from 81d8db7 to 1a12e87 Compare April 5, 2023 22:20

nguyenaiden added 2 commits April 5, 2023 15:23

Revert "Removed autocommit for SelectDB"

e9e85e4

This reverts commit 14ff20d.

bumped Dockerfile versions and updated changelog

6526161

Octavia Squidington III (octavia-squidington-iii) added the area/documentation Improvements or additions to documentation label Apr 5, 2023

Rodi Reich Zilberman (rodireich) approved these changes Apr 6, 2023

View reviewed changes

Reverted removal of SSL key from the JDBC Connection query

919074f

Akash Kulkarni (akashkulk) changed the title ~~Remove SSL toggle for source-postgres~~ Remove SSL toggle for source-postgres in OSS Apr 7, 2023

Akash Kulkarni (akashkulk) approved these changes Apr 7, 2023

View reviewed changes

nguyenaiden and others added 2 commits April 7, 2023 13:41

Removed extraneous keys

85fe20f

Merge branch 'master' into duy/24680

5dca5ae

Merge branch 'master' into duy/24680

0bf1738

Octavia Squidington III (octavia-squidington-iii) and others added 2 commits April 11, 2023 19:59

auto-bump connector version

5919ec4

Merge branch 'master' into duy/24680

b74a7ab

Rodi Reich Zilberman (rodireich) merged commit b60f4e2 into master Apr 11, 2023

Rodi Reich Zilberman (rodireich) deleted the duy/24680 branch April 11, 2023 22:29

Conversation

Duy Nguyen (nguyenaiden) commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Solution

Uh oh!

Duy Nguyen (nguyenaiden) commented Apr 4, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

Uh oh!

github-actions Bot commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Affected Connector Report

✅ Sources (3)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

Uh oh!

Duy Nguyen (nguyenaiden) commented Apr 4, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

Uh oh!

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Duy Nguyen (nguyenaiden) commented Apr 5, 2023

Uh oh!

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Rodi Reich Zilberman (rodireich) Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

Duy Nguyen (nguyenaiden) Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

Akash Kulkarni (akashkulk) Apr 6, 2023

Choose a reason for hiding this comment

Uh oh!

Rodi Reich Zilberman (rodireich) Apr 11, 2023

Choose a reason for hiding this comment

Uh oh!

Rodi Reich Zilberman (rodireich) commented Apr 5, 2023

Uh oh!

Lake Mossman (lmossman) commented Apr 5, 2023

Uh oh!

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Rodi Reich Zilberman (rodireich) commented Apr 11, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Rodi Reich Zilberman (rodireich) commented Apr 11, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rodi Reich Zilberman (rodireich) commented Apr 11, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Duy Nguyen (nguyenaiden) commented Apr 4, 2023 •

edited

Loading

Duy Nguyen (nguyenaiden) commented Apr 4, 2023 •

edited by github-actions Bot

Loading

github-actions Bot commented Apr 4, 2023 •

edited

Loading

Duy Nguyen (nguyenaiden) commented Apr 4, 2023 •

edited by github-actions Bot

Loading

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 •

edited by github-actions Bot

Loading

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 •

edited by github-actions Bot

Loading

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 •

edited by github-actions Bot

Loading

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 •

edited by github-actions Bot

Loading

Duy Nguyen (nguyenaiden) commented Apr 5, 2023 •

edited by github-actions Bot

Loading

Rodi Reich Zilberman (rodireich) commented Apr 11, 2023 •

edited by github-actions Bot

Loading

Rodi Reich Zilberman (rodireich) commented Apr 11, 2023 •

edited by github-actions Bot

Loading

Rodi Reich Zilberman (rodireich) commented Apr 11, 2023 •

edited by github-actions Bot

Loading