🐛 Postgres Source Strict Encrypt: Allow connections with sslmodes 'allow' and 'prefer' if SSH tunnel established #19551

VitaliiMaltsev · 2022-11-17T14:54:22Z

What

In strict-encrypt connector, if SSH is turned on and ssl_mode is prefer or allow check connection will fail in case of Postgres Server does not support SSL connections

this happens for the reason that we have a hard-coded

  static final Map<String, String> SSL_JDBC_PARAMETERS = ImmutableMap.of(
      "ssl", "true",
      "sslmode", "require");

and even if the user chooses ssl_mode allow or prefer during connector setup, the ssl_mode require will always be used, which will lead to a connection error

If SSH tunnel established we should enforce correct ssl mode chosen by user and the connection must be successful both for servers that support SSL and for those that do not support SSL

Postgres docs

How

Fixed bug with ssl mode enforcing

🚨 User Impact 🚨

there should not be any user impact

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.

…w' and 'prefer' if SSH tunnel established

…r-ssl-modes # Conflicts: # airbyte-integrations/connectors/source-postgres/src/test/java/io/airbyte/integrations/source/postgres/PostgresSourceStrictEncryptTest.java

github-actions · 2022-11-17T14:58:27Z

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`	`1.0.17`	✅	✅
`source-alloydb-strict-encrypt`	`1.0.17`	✅	⚠ (not in seed)
`source-postgres-strict-encrypt`	`1.0.26`	✅	⚠ (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.

✅ 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.

VitaliiMaltsev · 2022-11-18T10:14:57Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3496038798
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3496038798
Python tests coverage:

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       139      5    96%   87, 93, 235, 239-240
	 source_acceptance_test/conftest.py                     196     92    53%   35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1563    349    78%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
================= 14 passed, 5 skipped, 21 warnings in 28.47s ==================

rodireich · 2022-11-21T22:45:23Z

@VitaliiMaltsev , wasn't there another PR for this fix?
I feel like I've seen this before

VitaliiMaltsev · 2022-11-22T12:44:17Z

@VitaliiMaltsev , wasn't there another PR for this fix? I feel like I've seen this before

@rodireich probably you are talking about this one so my answer is no, this PR is fix for bug with incorrect ssl modes enforcing

grishick · 2022-11-22T16:40:11Z

test strictness level check is failing with this message:

ERROR:root:The following GA connectors must enable high test strictness level: ['source-postgres']. Please check this documentation for details: [https://docs.airbyte.com/connector-development/testing-connectors/source-acceptance-tests-reference/#st](https://docs.airbyte.com/connector-development/testing-connectors/source-acceptance-tests-reference/#strictness-level)

rodireich · 2022-11-22T19:28:19Z

@grishick Until we get our test declaration/level to the required strictness, what works for now is to merge with the above check strictness error.
The ticket tracking source-postgres testing strictness is #19060

VitaliiMaltsev · 2022-11-22T19:40:50Z

/publish connector=connectors/source-postgres

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

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

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

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

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

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

Postgres Source Strict Encrypt: Allow connections with sslmodes 'allo…

6cbe146

…w' and 'prefer' if SSH tunnel established

octavia-squidington-iv added area/connectors Connector related issues connectors/source/postgres labels Nov 17, 2022

VitaliiMaltsev temporarily deployed to more-secrets November 17, 2022 14:55 Inactive

Merge branch 'master' into vmaltsev/19211-postgres-source-allow-prefe…

dc98dad

…r-ssl-modes # Conflicts: # airbyte-integrations/connectors/source-postgres/src/test/java/io/airbyte/integrations/source/postgres/PostgresSourceStrictEncryptTest.java

VitaliiMaltsev temporarily deployed to more-secrets November 17, 2022 14:59 Inactive

updated changelog

da43759

octavia-squidington-iv added the area/documentation Improvements or additions to documentation label Nov 18, 2022

VitaliiMaltsev temporarily deployed to more-secrets November 18, 2022 09:47 Inactive

fixed tests

864a359

VitaliiMaltsev temporarily deployed to more-secrets November 18, 2022 10:16 Inactive

VitaliiMaltsev requested review from bleonard, akashkulk and rodireich November 18, 2022 13:23

VitaliiMaltsev marked this pull request as ready for review November 18, 2022 13:23

VitaliiMaltsev requested a review from a team as a code owner November 18, 2022 13:23

VitaliiMaltsev changed the title ~~Postgres Source Strict Encrypt: Allow connections with sslmodes 'allow' and 'prefer' if SSH tunnel established~~ 🐛 Postgres Source Strict Encrypt: Allow connections with sslmodes 'allow' and 'prefer' if SSH tunnel established Nov 18, 2022

VitaliiMaltsev linked an issue Nov 20, 2022 that may be closed by this pull request

Postgres Source Strict Encrypt: Allow connections with sslmode allow and prefer if SSH tunnel established #19211

Closed

VitaliiMaltsev requested a review from sashaNeshcheret November 21, 2022 11:51

rodireich approved these changes Nov 22, 2022

View reviewed changes

add test_strictness_level to acceptance-test-config.yml

56a99f7

VitaliiMaltsev temporarily deployed to more-secrets November 22, 2022 19:33 Inactive

remove test_strictness_level to due to failed SAT test

e188ce3

VitaliiMaltsev temporarily deployed to more-secrets November 22, 2022 19:38 Inactive

bump version

4fe12c8

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

VitaliiMaltsev temporarily deployed to more-secrets November 22, 2022 19:41 Inactive

auto-bump connector version

dfa2c06

octavia-squidington-iii temporarily deployed to more-secrets November 22, 2022 20:30 Inactive

klsoper mentioned this pull request Nov 23, 2022

Postgres Source Strict Encrypt: Allow connections with sslmode allow and prefer if SSH tunnel established #19211

Closed

VitaliiMaltsev merged commit 637781d into master Nov 28, 2022

VitaliiMaltsev deleted the vmaltsev/19211-postgres-source-allow-prefer-ssl-modes branch November 28, 2022 14:31

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

Bump helm chart version reference to 0.42.2 #19839

Closed

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

Bump Airbyte version from 0.40.22 to 0.40.23 #19941

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Postgres Source Strict Encrypt: Allow connections with sslmodes 'allow' and 'prefer' if SSH tunnel established #19551

🐛 Postgres Source Strict Encrypt: Allow connections with sslmodes 'allow' and 'prefer' if SSH tunnel established #19551

VitaliiMaltsev commented Nov 17, 2022 •

edited

Loading

github-actions bot commented Nov 17, 2022 •

edited

Loading

⚠ Sources (3)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

VitaliiMaltsev commented Nov 18, 2022 •

edited by github-actions bot

Loading

rodireich commented Nov 21, 2022

VitaliiMaltsev commented Nov 22, 2022

grishick commented Nov 22, 2022

rodireich commented Nov 22, 2022 •

edited

Loading

VitaliiMaltsev commented Nov 22, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 22, 2022 •

edited by github-actions bot

Loading

🐛 Postgres Source Strict Encrypt: Allow connections with sslmodes 'allow' and 'prefer' if SSH tunnel established #19551

🐛 Postgres Source Strict Encrypt: Allow connections with sslmodes 'allow' and 'prefer' if SSH tunnel established #19551

Conversation

VitaliiMaltsev commented Nov 17, 2022 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

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

Affected Connector Report

⚠ Sources (3)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

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

Build Passed

rodireich commented Nov 21, 2022

VitaliiMaltsev commented Nov 22, 2022

grishick commented Nov 22, 2022

rodireich commented Nov 22, 2022 • edited Loading

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

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

VitaliiMaltsev commented Nov 17, 2022 •

edited

Loading

github-actions bot commented Nov 17, 2022 •

edited

Loading

VitaliiMaltsev commented Nov 18, 2022 •

edited by github-actions bot

Loading

rodireich commented Nov 22, 2022 •

edited

Loading

VitaliiMaltsev commented Nov 22, 2022 •

edited by github-actions bot

Loading

VitaliiMaltsev commented Nov 22, 2022 •

edited by github-actions bot

Loading