Exposing SSL-only version of Postgres Source #6362

cgardens · 2021-09-22T01:32:06Z

What

We want to provide versions of some connectors such that they only work in a "secure" mode. In the context of database connectors this means that we want the connector to only operate with TLS encryption turned on. While for the base version of the postgres connector allows a user to send data unencrypted for users with a strong security posture and for our cloud product we want to enforce that data can never be sent across the public internet unencrypted.

How

I was originally trying to figure out if we could solve this elegantly with the build system. I don't think it's a good answer. It requires passing a jq query in the build system and then a lot of confusing work to inject the new spec into the jar of the connector. At best it is not intuitive and ends up with a lot of build cruft and will be easy to mess up.
Instead I moved forward with creating a new connector that just depends on the base postgres connnector. Unfortunately it's more code and files, but it is at least straight forward and will be more resilient to any changes we want to make in the future.
I did build a light abstraction to make basic spec modifications easy to do in this case and for the rest of our dbs.

Questions

if we like this approach i'll clean up the hacks in jdbc source test
strict is a bad name, but was easy to use while i was iterating. will change it to something else if we move forward with the approach. secure? other ideas?
are the pruned set of tests that i have chosen sufficient?

cgardens · 2021-09-22T01:32:49Z

...src/testFixtures/java/io/airbyte/integrations/source/jdbc/test/JdbcSourceAcceptanceTest.java

@@ -42,6 +42,7 @@
 import io.airbyte.commons.util.MoreIterators;


ignore this class for now. i had to commit some hacks to get this to work. will clean it up if we stick with this approach.

sherifnada

seems good!

Some related work/problems we may wanna think about (not in this PR):

How to do this in destinations, specifically normalization
encoding the publish script into gradle so we can couple connectors together

sherifnada · 2021-09-22T05:38:49Z

...src/testFixtures/java/io/airbyte/integrations/source/jdbc/test/JdbcSourceAcceptanceTest.java

-    final JsonNode jdbcConfig = source.toDatabaseConfig(config);
+    toDatabaseConfig = sourceFunctionImmutablePair.getRight();
+    // final JsonNode jdbcConfig = source.toDatabaseConfig(config);
+    final JsonNode jdbcConfig = toDatabaseConfig.apply(config);


would be ideal if we can do this all at once for all vars

sherifnada · 2021-09-22T05:39:29Z

airbyte-integrations/connectors/source-postgres-strict/acceptance-test-config.yml

+# See [Source Acceptance Tests](https://docs.airbyte.io/connector-development/testing-connectors/source-acceptance-tests-reference)
+# for more information about how to configure these tests
+connector_image: airbyte/source-postgres-strict:dev
+tests:


nice! though I think this is not bing triggered from the build system. You need to add the plugin + to do it locally, add the bash wrapper (find it in any python source)

this is actually just copied from the postgres source. is it just wrong there too?

sherifnada · 2021-09-22T05:40:49Z

...e-java/src/main/java/io/airbyte/integrations/base/spec_modification/SpecModifyingSource.java

+import io.airbyte.protocol.models.ConfiguredAirbyteCatalog;
+import io.airbyte.protocol.models.ConnectorSpecification;
+
+public abstract class SpecModifyingSource implements Source {


if all we're doing is modify the spec is a whole new superclass worth it? why not just extend the source directly and modify the spec method?

fair. I think we can reuse this class for all the db sources that this applies to so it gives us a nice convention to work from if we want to expand it later. i can just document it once. otherwise we're just going to extend a bunch of sources and have to explain it in a bunch of places.

agree with @cgardens the decoration pattern makes the feature of modifying the spec more composable

CLAassistant · 2021-09-27T17:24:53Z

All committers have signed the CLA.

cgardens · 2021-09-27T23:18:39Z

/publish connector=source-postgres-strict-encrypt

❌ source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1280375638

cgardens · 2021-09-27T23:32:54Z

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

🕑 connectors/source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1280409361
✅ connectors/source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/1280409361

cgardens requested a review from sherifnada September 22, 2021 01:32

github-actions bot added the area/connectors Connector related issues label Sep 22, 2021

cgardens commented Sep 22, 2021

View reviewed changes

cgardens temporarily deployed to more-secrets September 22, 2021 01:33 Inactive

sherifnada approved these changes Sep 22, 2021

View reviewed changes

sherifnada mentioned this pull request Sep 24, 2021

[EPIC] Disable insecure settings in cloud connectors #6408

Closed

19 tasks

This was referenced Sep 27, 2021

Create a secure-only Postgres Source #6419

Closed

Create a secure-only MySQL source #6420

Closed

Create a secure-only MSSQL Source #6421

Closed

Create a secure-only Postgres Destination #6418

Closed

github-actions bot added the normalization label Sep 27, 2021

cgardens temporarily deployed to more-secrets September 27, 2021 22:07 Inactive

cgardens changed the title ~~RFC: Exposing SSL-only version of Postgres Source~~ Exposing SSL-only version of Postgres Source Sep 27, 2021

cgardens temporarily deployed to more-secrets September 27, 2021 23:19 Inactive

cgardens added 9 commits September 27, 2021 16:19

first pass

9aaae36

checkpoint for feedback

9f352a4

hack

71b2183

fix image name in acceptance tests

d567fdc

enforce ssl for postgres normalization

b5a556c

various clean

5f4b68a

fix test

50ad203

so it goes

72069d4

go

275096c

jrhizor temporarily deployed to more-secrets September 27, 2021 23:20 Inactive

cgardens added 2 commits September 27, 2021 16:24

fix main method

36e11d7

fix the noise

aaf3eaa

cgardens force-pushed the cgardens/rfc_strict_spec branch from 555ad72 to aaf3eaa Compare September 27, 2021 23:32

github-actions bot removed the normalization label Sep 27, 2021

cgardens temporarily deployed to more-secrets September 27, 2021 23:33 Inactive

jrhizor temporarily deployed to more-secrets September 27, 2021 23:35 Inactive

add java doc comments

72ba74b

cgardens merged commit f308690 into master Sep 27, 2021

cgardens deleted the cgardens/rfc_strict_spec branch September 27, 2021 23:46

cgardens temporarily deployed to more-secrets September 27, 2021 23:46 Inactive

cgardens mentioned this pull request Sep 28, 2021

Publish SSL-only version of Postgres Destination #6496

Merged

2 tasks

jrhizor mentioned this pull request Sep 29, 2021

Bump Airbyte version from 0.29.22-alpha to 0.30.0-alpha #6532

Closed

This was referenced Sep 29, 2021

Create a secure-only Oracle Source #6422

Closed

Create a secure-only MySQL destination #6423

Closed

forking the File connector to disallow pulling files from the local filesystem #6538

Closed

This was referenced Sep 29, 2021

Bump Airbyte version from 0.29.22-alpha to 0.30.0-alpha #6539

Closed

Bump Airbyte version from 0.29.22-alpha to 0.30.0-alpha #6548

Merged

This was referenced Oct 12, 2021

Redshift Source: create secure-only version #6973

Closed

Redshift Destination: create secure-only version #6974

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exposing SSL-only version of Postgres Source #6362

Exposing SSL-only version of Postgres Source #6362

cgardens commented Sep 22, 2021 •

edited

Loading

cgardens Sep 22, 2021

sherifnada left a comment

sherifnada Sep 22, 2021

sherifnada Sep 22, 2021

cgardens Sep 27, 2021

sherifnada Sep 22, 2021

cgardens Sep 22, 2021

michel-tricot Sep 22, 2021

CLAassistant commented Sep 27, 2021 •

edited

Loading

cgardens commented Sep 27, 2021 •

edited by github-actions bot

Loading

cgardens commented Sep 27, 2021 •

edited by github-actions bot

Loading

		@@ -42,6 +42,7 @@
		import io.airbyte.commons.util.MoreIterators;

Exposing SSL-only version of Postgres Source #6362

Exposing SSL-only version of Postgres Source #6362

Conversation

cgardens commented Sep 22, 2021 • edited Loading

What

How

Recommended reading order

Questions

cgardens Sep 22, 2021

Choose a reason for hiding this comment

sherifnada left a comment

Choose a reason for hiding this comment

sherifnada Sep 22, 2021

Choose a reason for hiding this comment

sherifnada Sep 22, 2021

Choose a reason for hiding this comment

cgardens Sep 27, 2021

Choose a reason for hiding this comment

sherifnada Sep 22, 2021

Choose a reason for hiding this comment

cgardens Sep 22, 2021

Choose a reason for hiding this comment

michel-tricot Sep 22, 2021

Choose a reason for hiding this comment

CLAassistant commented Sep 27, 2021 • edited Loading

cgardens commented Sep 27, 2021 • edited by github-actions bot Loading

cgardens commented Sep 27, 2021 • edited by github-actions bot Loading

cgardens commented Sep 22, 2021 •

edited

Loading

CLAassistant commented Sep 27, 2021 •

edited

Loading

cgardens commented Sep 27, 2021 •

edited by github-actions bot

Loading

cgardens commented Sep 27, 2021 •

edited by github-actions bot

Loading