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

Network Tunneling in Batch SQL Connectors #2190

Merged
merged 6 commits into from
Dec 6, 2024

Conversation

willdonnelly
Copy link
Member

@willdonnelly willdonnelly commented Dec 6, 2024

Description:

Integrates network/SSH tunneling into the batch SQL connectors, with the exception of source-bigquery-batch since it's not self-evident how to apply it there and I'm just picking some extremely low-hanging fruit here.

Notes for reviewers:

In order to avoid copy-pasting a bunch of unnecessary bits of boilerplate, I have factored out some of the config structs and startup logic that other connectors used into the network tunnel Go package and am using that here. I have not gone back and modified the other dozen connectors with SSH tunneling support to use the more concise integration in this PR, but it's something we might want to do at some point just for code health reasons.


This change is Reviewable

The connector side of integrating the network tunnel has a
bunch of boilerplate which to me doesn't seem like it needs
to be copy-pasted into each connector. This commit adds the
SSH tunnel config structs and a Start() helper method into
the network tunnel Go package so that future integrations
can be a bit more concise.

This factoring-out is backwards compatible: all of the old
structs and methods still exist, it's just that the package
now includes the config structs and a Start() method which
can be used to make the connector code cleaner.

Also the package name has been changed from `network-tunnel`
to `networkTunnel` because that's the name that every single
import of the package assigns it.
Integrating SSH tunneling into the BigQuery connection isn't
entirely obvious since we're not connecting to a specific
address.
@willdonnelly willdonnelly requested a review from a team December 6, 2024 20:06
Copy link
Member

@jgraettinger jgraettinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@willdonnelly willdonnelly merged commit bb360d7 into main Dec 6, 2024
53 checks passed
@willdonnelly willdonnelly deleted the wgd/batch-sql-network-tunnel-20241206 branch December 6, 2024 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants