Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EPIC] Support TLS for all database sources & destinations #5628

Closed
20 tasks done
sherifnada opened this issue Aug 25, 2021 · 4 comments
Closed
20 tasks done

[EPIC] Support TLS for all database sources & destinations #5628

sherifnada opened this issue Aug 25, 2021 · 4 comments
Assignees
Labels
airbyte-cloud area/connectors Connector related issues Epic lang/java priority/high High priority type/enhancement New feature or request

Comments

@sherifnada
Copy link
Contributor

sherifnada commented Aug 25, 2021

Tell us about the problem you're trying to solve

Like described in this doc, we want to support TLS encryption when connecting to the major databases/warehouse.

Note that we do NOT need to support certificate verification as part of this issue -- just encryption of data over the wire. In other words, the focus is protecting against eavesdropping, not man-in-the-middle attacks. See the document linked for more details.

Here are the DBs we should support:

Must-haves for Airbyte Cloud

Nice-to-haves for Airbyte Cloud

Describe the solution you’d like

Go through each source/destination in the must-have list. If the connector doesn't support encryption at all then create a ticket to support TLS/SSL for it.

The acceptance criteria for each ticket is:

  • Implement encryption support in the connector if not already implemented. Where possible, support encryption by default. If encryption-by-default is a bad idea (for example, if most MySQL versions do not support encryption and would require special work from the DB administrator) then expose it as an option in the connector specification, and encrypt when the user requests it.
  • The external documentation of the connector mentions that encryption is supported
  • If encryption is exposed as an option, add in the connector spec and docs a recommendation to use it (for example, MSSQL source mentions that encryption without server certification is used for testing purposes only, which is not true, see the doc above)
  • Encrypted connections are tested as part of either a custom integration test or acceptance tests. Where possible, test it using a test container. If that's impossible and it must be tested on a real DB instance, create a DB instance in AWS ideally using terraform (but if TF is too hard just create it manually and make a ticket to encode it in TF)
  • Create a PR

Implementation hints

There is a difference when implementing this for sources & destinations because destinations might need to change normalization as well.

When implementing this for sources, it's probably as simple as setting a flag e.g: Mysql uses the --ssl=REQUIRED flag.

When implementing for destinations it might be very similar, but there will be two places to edit this: in the destination connector itself and in the normalization module. It might be easiest to ask the Python team to implement the piece around normalization, but it really shouldn't be that complicated e.g: if it's just adding a flag -- it's ideal if you can implement it yourself since you'll learn a bit about normalization, but this is not a primary goal of this ticket. The goal is to support TLS as soon as possible.

@sherifnada sherifnada added type/enhancement New feature or request area/connectors Connector related issues lang/java priority/high High priority airbyte-cloud priority/critical Critical priority! and removed priority/high High priority priority/critical Critical priority! labels Aug 25, 2021
@olivermeyer
Copy link
Contributor

Hi @sherifnada, will this cover Redshift too? I'm hopeful since the title says "all databases" 😃

@sherifnada sherifnada changed the title Support TLS for all database sources & destinations [EPIC] Support TLS for all database sources & destinations Sep 8, 2021
@sherifnada sherifnada added the Epic label Sep 8, 2021
@sherifnada
Copy link
Contributor Author

@olivermeyer yes but after OLTP databases!

@sherifnada
Copy link
Contributor Author

@alexandr-shegeda should we close the tickets which already have TLS support?

@sherifnada
Copy link
Contributor Author

@olivermeyer btw Redshift now supports encryption by default!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
airbyte-cloud area/connectors Connector related issues Epic lang/java priority/high High priority type/enhancement New feature or request
Projects
No open projects
Archived in project
Development

No branches or pull requests

3 participants