Postgres/MySQL Source Strict Encrypt: stop enforce SSL if ssl mode disabled #19025

VitaliiMaltsev · 2022-11-07T12:24:39Z

What

Fix for #18992

How

Stop enforce ssl=true if ssl mode disabled or disable for MySQL Strict Encrypt and Postgres Strict Encrypt

🚨 User Impact 🚨

none

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

…sabled

…andatory-ssl' into 18992/vmaltsev-postgres-source-mandatory-ssl

bleonard

Jsut confirming: This is still enforcing SSL for our strict-encrypt CHECKs right?

grishick · 2022-11-07T19:35:21Z

@VitaliiMaltsev once this is published and merged, please don't forget to unpin connector version in cloud

VitaliiMaltsev · 2022-11-07T19:41:58Z

Jsut confirming: This is still enforcing SSL for our strict-encrypt CHECKs right?

All of jdbc strict encrypt connectors will still enforce SSL except postgres and mysql in case of customer choose ssl mode disabled during connection setup

ryankfu · 2022-11-07T20:32:07Z

@VitaliiMaltsev Can we add a test case that specifically tests for ssl_mode: disabled and SSH_TUNNEL succeeds check within both source-postgres-strict-encrypt and source-mysql-strict-encrypt? Currently neither of those integration tests have this scenario covered

ryankfu · 2022-11-07T20:34:49Z

airbyte-db/db-lib/src/main/java/io/airbyte/db/jdbc/JdbcUtils.java

@@ -114,7 +115,13 @@ public static Map<String, String> parseJdbcParameters(final String jdbcPropertie
   * @return true: if ssl has not been set or it has been set with true, false: in all other cases
   */
  public static boolean useSsl(final JsonNode config) {
-    return !config.has(SSL_KEY) || config.get(SSL_KEY).asBoolean();
+    if (!config.has(SSL_KEY)) {


Can we update the javadoc description to update what this method now does so it's not de-synced from what this method originally did?

Can we update the javadoc description to update what this method now does so it's not de-synced from what this method originally did?

updated javadoc

ryankfu · 2022-11-07T20:38:38Z

airbyte-db/db-lib/src/test/java/io/airbyte/db/jdbc/TestJdbcUtils.java

@@ -210,6 +210,42 @@ void testUssSslWithSslSetAndValueIntegerTrue() {
    assertTrue(sslSet);
  }

+  @Test
+  void testUseSslWithEmptySslKeyAndSslModeVerifyFull() {


These tests are good, let's also add an integration test for both source-mysql-strict-encrypt and source-postgres-strict-encrypt that cover the scenario that was failing for the customer which is when SSH tunnel was set and their ssl_mode: disabled

These tests are good, let's also add an integration test for both source-mysql-strict-encrypt and source-postgres-strict-encrypt that cover the scenario that was failing for the customer which is when SSH tunnel was set and their ssl_mode: disabled

@ryankfu i added a couple of tests for postgres and mysql in order to cover such a case that the user encountered (when SSH tunnel was set and ssl_mode: disabled)

ryankfu

Looks good overall, just want to specifically cover scenario that the user fails on. Currently neither the strict encrypt versions of these source connectors have this test case covered so this change cannot be conclusive that it resolves the user's issue

VitaliiMaltsev · 2022-11-08T15:46:25Z

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

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

Build Passed

Test summary info:

All Passed

VitaliiMaltsev · 2022-11-08T19:34:40Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3422601480

VitaliiMaltsev · 2022-11-08T20:09:49Z

/test connector=connectors/source-mysql

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

Build Passed

Test summary info:

All Passed

VitaliiMaltsev · 2022-11-08T21:46:34Z

/test connector=connectors/source-postgres

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

Build Passed

Test summary info:

All Passed

grishick · 2022-11-09T00:28:58Z

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

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

Build Passed

Test summary info:

All Passed

ryankfu · 2022-11-09T17:36:54Z

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

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

Build Passed

Test summary info:

All Passed

ryankfu

Looks good, thanks for adding these tests to the strict-encrypt variant of each source. Should be good pending integration tests for source-mysql-strict-encrypt passes

VitaliiMaltsev · 2022-11-09T20:06:50Z

/publish connector=connectors/source-mysql-strict-encrypt

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

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

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

VitaliiMaltsev · 2022-11-09T20:06:59Z

/publish connector=connectors/source-mysql

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

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

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

VitaliiMaltsev · 2022-11-09T20:07:14Z

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

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

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 ▶️

github-actions · 2022-11-09T20:07:20Z

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 version
Add changelog
Publish the new version

⚠ Sources (5)

Connector	Version	Changelog	Publish
`source-alloydb`	`1.0.17`	✅	✅
`source-alloydb-strict-encrypt`	`1.0.17`	✅	⚠ (not in seed)
`source-mysql`	`1.0.12`	✅	✅
`source-mysql-strict-encrypt`	`1.0.12`	✅	⚠ (not in seed)
`source-postgres-strict-encrypt`	`1.0.23`	✅	⚠ (not in seed)

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.

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.

VitaliiMaltsev · 2022-11-09T20:07:23Z

/publish connector=connectors/source-postgres

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

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 ▶️

…//github.com/airbytehq/airbyte into 18992/vmaltsev-postgres-source-mandatory-ssl

…sabled (#19025) * Postgres/MySQL Source Strict Encrypt: stop enforce SSL if ssl mode disabled * fixed checkstyle * updated changelog * add tests * replaced MySQL test to mysql-strict-encrypt module * fixed Connection Refused for mysql test * replaced Postgres Source strict-encrypt tests into new class * bump version * auto-bump connector version * auto-bump connector version Co-authored-by: Octavia Squidington III <[email protected]>

VitaliiMaltsev added 2 commits November 7, 2022 14:14

Postgres/MySQL Source Strict Encrypt: stop enforce SSL if ssl mode di…

f6fbee6

…sabled

fixed checkstyle

3e2e3c4

VitaliiMaltsev temporarily deployed to more-secrets November 7, 2022 13:10 Inactive

Merge branch 'master' into 18992/vmaltsev-postgres-source-mandatory-ssl

b6d36d7

VitaliiMaltsev temporarily deployed to more-secrets November 7, 2022 14:15 Inactive

VitaliiMaltsev added 2 commits November 7, 2022 16:16

updated changelog

2bcd621

Merge remote-tracking branch 'origin/18992/vmaltsev-postgres-source-m…

3fa7a64

…andatory-ssl' into 18992/vmaltsev-postgres-source-mandatory-ssl

github-actions bot added the area/documentation Improvements or additions to documentation label Nov 7, 2022

VitaliiMaltsev marked this pull request as ready for review November 7, 2022 14:17

VitaliiMaltsev requested review from tuliren and bleonard November 7, 2022 14:17

VitaliiMaltsev temporarily deployed to more-secrets November 7, 2022 14:19 Inactive

VitaliiMaltsev mentioned this pull request Nov 7, 2022

Source-postgres-strict-encrypt 1.0.22 fails to set up connection w/o SSL #18992

Closed

VitaliiMaltsev requested a review from subodh1810 November 7, 2022 16:21

bleonard reviewed Nov 7, 2022

View reviewed changes

ryankfu reviewed Nov 7, 2022

View reviewed changes

ryankfu suggested changes Nov 7, 2022

View reviewed changes

add tests

3d2d0c8

VitaliiMaltsev requested a review from a team as a code owner November 8, 2022 15:30

github-actions bot added the area/connectors Connector related issues label Nov 8, 2022

octavia-squidington-iv added connectors/source/mysql connectors/source/postgres labels Nov 8, 2022

VitaliiMaltsev temporarily deployed to more-secrets November 8, 2022 15:32 Inactive

VitaliiMaltsev requested review from ryankfu and removed request for tuliren November 8, 2022 15:39

grishick approved these changes Nov 8, 2022

View reviewed changes

fixed Connection Refused for mysql test

cb950f9

VitaliiMaltsev temporarily deployed to more-secrets November 8, 2022 20:10 Inactive

replaced Postgres Source strict-encrypt tests into new class

8d1527c

VitaliiMaltsev temporarily deployed to more-secrets November 8, 2022 21:47 Inactive

VitaliiMaltsev requested a review from ryankfu November 8, 2022 22:04

ryankfu approved these changes Nov 9, 2022

View reviewed changes

bump version

cca315c

octavia-squidington-iv added the connectors/source/postgres-strict-encrypt label Nov 9, 2022

VitaliiMaltsev temporarily deployed to more-secrets November 9, 2022 20:07 Inactive

auto-bump connector version

221e76d

octavia-squidington-iii temporarily deployed to more-secrets November 9, 2022 21:20 Inactive

octavia-squidington-iii added 2 commits November 9, 2022 21:59

auto-bump connector version

692ba6b

Merge branch '18992/vmaltsev-postgres-source-mandatory-ssl' of https:…

874dd03

…//github.com/airbytehq/airbyte into 18992/vmaltsev-postgres-source-mandatory-ssl

octavia-squidington-iii temporarily deployed to more-secrets November 9, 2022 22:01 Inactive

VitaliiMaltsev merged commit c72b75e into master Nov 9, 2022

VitaliiMaltsev deleted the 18992/vmaltsev-postgres-source-mandatory-ssl branch November 9, 2022 22:59

github-actions bot mentioned this pull request Nov 15, 2022

Bump helm chart version reference to 0.40.43 #19453

Closed

octavia-squidington-iii mentioned this pull request Nov 18, 2022

Bump Airbyte version from 0.40.18 to 0.40.19 #19579

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Postgres/MySQL Source Strict Encrypt: stop enforce SSL if ssl mode disabled #19025

Postgres/MySQL Source Strict Encrypt: stop enforce SSL if ssl mode disabled #19025

VitaliiMaltsev commented Nov 7, 2022 •

edited

Loading

bleonard left a comment

grishick commented Nov 7, 2022

VitaliiMaltsev commented Nov 7, 2022

ryankfu commented Nov 7, 2022 •

edited

Loading

ryankfu Nov 7, 2022

VitaliiMaltsev Nov 8, 2022

ryankfu Nov 7, 2022

VitaliiMaltsev Nov 8, 2022

ryankfu left a comment

VitaliiMaltsev commented Nov 8, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 8, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 8, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 8, 2022 •

edited by github-actions bot

Loading

grishick commented Nov 9, 2022 •

edited by github-actions bot

Loading

ryankfu commented Nov 9, 2022 •

edited by github-actions bot

Loading

ryankfu left a comment

VitaliiMaltsev commented Nov 9, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 9, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 9, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Nov 9, 2022 •

edited

Loading

⚠ Sources (5)

✅ Destinations (0)

Actionable Items

VitaliiMaltsev commented Nov 9, 2022 •

edited by github-actions bot

Loading

Postgres/MySQL Source Strict Encrypt: stop enforce SSL if ssl mode disabled #19025

Postgres/MySQL Source Strict Encrypt: stop enforce SSL if ssl mode disabled #19025

Conversation

VitaliiMaltsev commented Nov 7, 2022 • edited Loading

What

How

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

bleonard left a comment

Choose a reason for hiding this comment

grishick commented Nov 7, 2022

VitaliiMaltsev commented Nov 7, 2022

ryankfu commented Nov 7, 2022 • edited Loading

ryankfu Nov 7, 2022

Choose a reason for hiding this comment

VitaliiMaltsev Nov 8, 2022

Choose a reason for hiding this comment

ryankfu Nov 7, 2022

Choose a reason for hiding this comment

VitaliiMaltsev Nov 8, 2022

Choose a reason for hiding this comment

ryankfu left a comment

Choose a reason for hiding this comment

VitaliiMaltsev commented Nov 8, 2022 • edited by github-actions bot Loading

Build Passed

VitaliiMaltsev commented Nov 8, 2022 • edited by github-actions bot Loading

VitaliiMaltsev commented Nov 8, 2022 • edited by github-actions bot Loading

Build Passed

VitaliiMaltsev commented Nov 8, 2022 • edited by github-actions bot Loading

Build Passed

grishick commented Nov 9, 2022 • edited by github-actions bot Loading

Build Passed

ryankfu commented Nov 9, 2022 • edited by github-actions bot Loading

Build Passed

ryankfu left a comment

Choose a reason for hiding this comment

VitaliiMaltsev commented Nov 9, 2022 • edited by github-actions bot Loading

VitaliiMaltsev commented Nov 9, 2022 • edited by github-actions bot Loading

VitaliiMaltsev commented Nov 9, 2022 • edited by github-actions bot Loading

github-actions bot commented Nov 9, 2022 • edited Loading

Affected Connector Report

⚠ Sources (5)

✅ Destinations (0)

Actionable Items

VitaliiMaltsev commented Nov 9, 2022 • edited by github-actions bot Loading

VitaliiMaltsev commented Nov 7, 2022 •

edited

Loading

ryankfu commented Nov 7, 2022 •

edited

Loading

VitaliiMaltsev commented Nov 8, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 8, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 8, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 8, 2022 •

edited by github-actions bot

Loading

grishick commented Nov 9, 2022 •

edited by github-actions bot

Loading

ryankfu commented Nov 9, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 9, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 9, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 9, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Nov 9, 2022 •

edited

Loading

VitaliiMaltsev commented Nov 9, 2022 •

edited by github-actions bot

Loading