-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Adding allowHosts to all non-GA source database connectors #23455
Adding allowHosts to all non-GA source database connectors #23455
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't double check all of the connectors, but this looks good to me!
@@ -553,7 +563,10 @@ | |||
icon: fauna.svg | |||
sourceType: database | |||
releaseStage: alpha | |||
- name: File (CSV, JSON, Excel, Feather, Parquet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the name change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted.
allowedHosts: | ||
hosts: | ||
- "${host}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope! I was wrong - it's still "host":
airbyte/airbyte-config/init/src/main/resources/seed/source_specs.yaml
Lines 14068 to 14075 in 7da6a3b
host: | |
description: "The host domain of the snowflake instance (must include the\ | |
\ account, region, cloud environment, and end with snowflakecomputing.com)." | |
examples: | |
- "accountname.us-east-2.aws.snowflakecomputing.com" | |
type: "string" | |
title: "Account Name" | |
order: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing me to source_specs.yaml
. Turns out it is not host
name in all places.
2. Removing host names under allowed hosts for BigQuery, Azure Table Storage and MongoDB sources until we figure out what to configure
/test connector=connectors/source-mssql
Build PassedTest summary info:
|
/test connector=connectors/source-snowflake
Build PassedTest summary info:
|
/test connector=source-cockroachdb
Build PassedTest summary info:
|
/test connector=source-fauna
Build FailedTest summary info:
|
/test connector=source-db2
Build PassedTest summary info:
|
/test connector=source-kafka
Build PassedTest summary info:
|
/test connector=source-mongodb-v2
Build FailedTest summary info:
|
/test connector=source-mysql
Build PassedTest summary info:
|
/test connector=source-oracle
Build PassedTest summary info:
|
/test connector=source-redshift
Build FailedTest summary info:
|
/test connector=source-tidb
Build PassedTest summary info:
|
/test connector=source-firebolt |
/test connector=source-azure-table
Build FailedTest summary info:
|
/test connector=source-clickhouse
Build PassedTest summary info:
|
…github.com:airbytehq/airbyte into prateekmukhedkar/add-allowedhosts-dbsources-non-ga
/publish connector=connectors/source-snowflake,connectors/source-cockroachdb,source-db2,source-kafka,source-oracle,source-tidb,source-clickhouse
if you have connectors that successfully published but failed definition generation, follow step 4 here |
/publish connector=connectors/source-db2,connectors/source-kafka,connectors/source-oracle,connectors/source-tidb,connectors/source-clickhouse
if you have connectors that successfully published but failed definition generation, follow step 4 here |
…github.com:airbytehq/airbyte into prateekmukhedkar/add-allowedhosts-dbsources-non-ga
…github.com:airbytehq/airbyte into prateekmukhedkar/add-allowedhosts-dbsources-non-ga
/publish connector=connectors/source-oracle-strict-encrypt auto-bump-version=false
if you have connectors that successfully published but failed definition generation, follow step 4 here |
/publish connector=connectors/source-db2-strict-encrypt auto-bump-version=false
if you have connectors that successfully published but failed definition generation, follow step 4 here |
/publish connector=connectors/source-clickhouse-strict-encrypt auto-bump-version=false
if you have connectors that successfully published but failed definition generation, follow step 4 here |
/publish connector=connectors/source-fauna,connectors/source-redshift,connectors/source-azure-table
if you have connectors that successfully published but failed definition generation, follow step 4 here |
…e tests in master.
…ce tests on master.
The following DB source connectors were published:
|
…#23455) Adding allowedHosts to clickhouse, cockroachdb, db2, oracle, snowflake, tidb source Connectors. --------- Co-authored-by: prateekmukhedkar <prateekmukhedkar@users.noreply.github.com> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Issue : #22372
Adding
allowedHosts
property to all non-GA database source connectors.These are the list of sources I have added allowedHosts to:
What
As part of the network isolation epic, all connectors will need to provide which hosts the connector container will be granted access to.
This change is specific to DB Source Connectors, which are not GA. (separate PR will be sent out for GA source connectors)
Note that we are adding
${tunnel_method.tunnel_host}
for only those connectors that we support connection via SSH.How
For databases, this information is dynamic and read from the connector's config. It probably looks a lot like:
airbyte/airbyte-config/init/src/main/resources/seed/source_definitions.yaml
Lines 1370 to 1373 in e6441bf
Verification
I tried with an MSSQL source that needs to be connected via an SSH tunnel and saw these in logs.
2023-02-24 19:30:30 �[32mINFO�[m i.a.w.p.DockerProcessFactory(create):125 - Creating docker container = source-mssql-read-78-0-yquyd with resources io.airbyte.config.ResourceRequirements@40cf4b83[cpuRequest=,cpuLimit=,memoryRequest=,memoryLimit=] and allowedHosts io.airbyte.config.AllowedHosts@32230bc8[hosts=[mssql-ssh-gl.cevykyaz98rn.us-east-2.rds.amazonaws.com, 3.18.93.32, *.datadoghq.com, *.datadoghq.eu, *.sentry.io],additionalProperties={}]
Recommended reading order
source_definitions.yaml