-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add ssh tunnel support for jdbc connectors, issue #312 #5438
Changes from all commits
e0e857c
2268889
c0db5bb
dfd8660
2246ffc
cf989a7
71632a5
72372d4
b03b09d
eaff495
a4f6e4d
034d23e
74604ec
9326192
00d872b
ad2b9cf
d79e6cf
5308038
52feb8b
5a4b979
ff7238f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -24,10 +24,12 @@ | |||||
|
||||||
package io.airbyte.integrations; | ||||||
|
||||||
import com.fasterxml.jackson.databind.JsonNode; | ||||||
import io.airbyte.commons.json.Jsons; | ||||||
import io.airbyte.commons.resources.MoreResources; | ||||||
import io.airbyte.integrations.base.Integration; | ||||||
import io.airbyte.protocol.models.ConnectorSpecification; | ||||||
import java.io.IOException; | ||||||
|
||||||
public abstract class BaseConnector implements Integration { | ||||||
|
||||||
|
@@ -42,7 +44,18 @@ public abstract class BaseConnector implements Integration { | |||||
public ConnectorSpecification spec() throws Exception { | ||||||
// return a JsonSchema representation of the spec for the integration. | ||||||
final String resourceString = MoreResources.readResource("spec.json"); | ||||||
return Jsons.deserialize(resourceString, ConnectorSpecification.class); | ||||||
return Jsons.object(addToSpec(Jsons.deserialize(resourceString)), ConnectorSpecification.class); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
|
||||||
/** | ||||||
* Extension point for child classes to add things to the spec json tree before it's deserialized | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
* into a ConnectorSpecification object. | ||||||
* | ||||||
* @param root | ||||||
* @return root | ||||||
*/ | ||||||
public JsonNode addToSpec(JsonNode root) throws IOException { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it makes sense to not expose this method in |
||||||
return root; | ||||||
} | ||||||
|
||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,276 @@ | ||||||||||
/* | ||||||||||
* MIT License | ||||||||||
* | ||||||||||
* Copyright (c) 2020 Airbyte | ||||||||||
* | ||||||||||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||||||||||
* of this software and associated documentation files (the "Software"), to deal | ||||||||||
* in the Software without restriction, including without limitation the rights | ||||||||||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||||||||||
* copies of the Software, and to permit persons to whom the Software is | ||||||||||
* furnished to do so, subject to the following conditions: | ||||||||||
* | ||||||||||
* The above copyright notice and this permission notice shall be included in all | ||||||||||
* copies or substantial portions of the Software. | ||||||||||
* | ||||||||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||||||||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||||||||||
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||||||||||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||||||||||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||||||||||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||||||||||
* SOFTWARE. | ||||||||||
*/ | ||||||||||
|
||||||||||
package io.airbyte.integrations.base; | ||||||||||
|
||||||||||
import com.fasterxml.jackson.databind.JsonNode; | ||||||||||
import java.io.IOException; | ||||||||||
import java.io.StringReader; | ||||||||||
import java.net.URISyntaxException; | ||||||||||
import java.security.KeyPair; | ||||||||||
import java.security.NoSuchAlgorithmException; | ||||||||||
import java.security.interfaces.RSAPrivateKey; | ||||||||||
import java.security.interfaces.RSAPublicKey; | ||||||||||
import java.security.spec.InvalidKeySpecException; | ||||||||||
import org.apache.sshd.client.SshClient; | ||||||||||
import org.apache.sshd.client.keyverifier.AcceptAllServerKeyVerifier; | ||||||||||
import org.apache.sshd.client.session.ClientSession; | ||||||||||
import org.apache.sshd.common.util.net.SshdSocketAddress; | ||||||||||
import org.apache.sshd.server.forward.AcceptAllForwardingFilter; | ||||||||||
import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo; | ||||||||||
import org.bouncycastle.openssl.PEMKeyPair; | ||||||||||
import org.bouncycastle.openssl.PEMParser; | ||||||||||
import org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter; | ||||||||||
import org.slf4j.Logger; | ||||||||||
import org.slf4j.LoggerFactory; | ||||||||||
|
||||||||||
/** | ||||||||||
* Encapsulates the connection configuration for an ssh tunnel port forward through a proxy/bastion | ||||||||||
* host plus the remote host and remote port to forward to a specified local port. | ||||||||||
*/ | ||||||||||
public class SSHTunnel { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does it make sense to extend autocloseable so we can use a try-with-resources? |
||||||||||
|
||||||||||
private static final Logger LOGGER = LoggerFactory.getLogger(SSHTunnel.class); | ||||||||||
|
||||||||||
public static final int TIMEOUT_MILLIS = 15000; // 15 seconds | ||||||||||
private final String method; | ||||||||||
private final String host; | ||||||||||
private final String tunnelSshPort; | ||||||||||
private final String user; | ||||||||||
private final String sshkey; | ||||||||||
private final String password; | ||||||||||
private final String remoteDatabaseHost; | ||||||||||
private final String remoteDatabasePort; | ||||||||||
private final String tunnelDatabasePort; | ||||||||||
|
||||||||||
private SSHTunnel tunnelConfig = null; | ||||||||||
private SshClient sshclient = null; | ||||||||||
private ClientSession tunnelSession = null; | ||||||||||
|
||||||||||
public SSHTunnel(String method, | ||||||||||
String host, | ||||||||||
String tunnelSshPort, | ||||||||||
String user, | ||||||||||
String sshkey, | ||||||||||
String password, | ||||||||||
String remoteDatabaseHost, | ||||||||||
String remoteDatabasePort, | ||||||||||
String tunnelDatabasePort) { | ||||||||||
if (method == null) { | ||||||||||
this.method = "NO_TUNNEL"; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should us a constant for |
||||||||||
} else { | ||||||||||
this.method = method; | ||||||||||
} | ||||||||||
this.host = host; | ||||||||||
this.tunnelSshPort = tunnelSshPort; | ||||||||||
this.user = user; | ||||||||||
this.sshkey = sshkey; | ||||||||||
this.password = password; | ||||||||||
this.remoteDatabaseHost = remoteDatabaseHost; | ||||||||||
this.remoteDatabasePort = remoteDatabasePort; | ||||||||||
this.tunnelDatabasePort = tunnelDatabasePort; | ||||||||||
} | ||||||||||
|
||||||||||
public static SSHTunnel getInstance(JsonNode config) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading through the code is a bit confusing for the case where the Maybe it'd be easier to read if this was There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we instead explicitly validate which case we're in (no tunnel/ssh key/pw auth) and do null checks/validation based on that? |
||||||||||
JsonNode ourConfig = config.get("tunnel_method"); | ||||||||||
SSHTunnel sshconfig = new SSHTunnel( | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_method"), | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_host"), | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_ssh_port"), | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_username"), | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_usersshkey"), | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_userpass"), | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_db_remote_host"), | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_db_remote_port"), | ||||||||||
getConfigValueOrNull(ourConfig, "tunnel_localport")); | ||||||||||
return sshconfig; | ||||||||||
} | ||||||||||
|
||||||||||
static String getConfigValueOrNull(JsonNode config, String key) { | ||||||||||
return config != null && config.has(key) ? config.get(key).asText() : null; | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Starts an ssh session; wrap this in a try-finally and use closeTunnel() to close it. | ||||||||||
* | ||||||||||
* @throws IOException | ||||||||||
*/ | ||||||||||
public void openTunnelIfRequested() { | ||||||||||
if (shouldTunnel()) { | ||||||||||
if (tunnelSession != null || sshclient != null) { | ||||||||||
throw new RuntimeException("SSH Tunnel was requested to be opened while it was already open. This is a coding error."); | ||||||||||
} | ||||||||||
sshclient = createClient(); | ||||||||||
tunnelSession = openTunnel(sshclient); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Closes a tunnel if one was open, and otherwise doesn't do anything (safe to run). | ||||||||||
*/ | ||||||||||
public void closeTunnel() { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could actually be |
||||||||||
try { | ||||||||||
if (shouldTunnel()) { | ||||||||||
if (tunnelSession != null) { | ||||||||||
tunnelSession.close(); | ||||||||||
} | ||||||||||
if (sshclient != null) { | ||||||||||
sshclient.stop(); | ||||||||||
} | ||||||||||
tunnelSession = null; | ||||||||||
sshclient = null; | ||||||||||
} | ||||||||||
} catch (Throwable t) { | ||||||||||
throw new RuntimeException(t); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
public boolean shouldTunnel() { | ||||||||||
return method != null && !"NO_TUNNEL".equals(method); | ||||||||||
} | ||||||||||
|
||||||||||
public String getMethod() { | ||||||||||
return method; | ||||||||||
} | ||||||||||
|
||||||||||
public String getHost() { | ||||||||||
return host; | ||||||||||
} | ||||||||||
|
||||||||||
public String getTunnelSshPort() { | ||||||||||
return tunnelSshPort; | ||||||||||
} | ||||||||||
|
||||||||||
public String getUser() { | ||||||||||
return user; | ||||||||||
} | ||||||||||
|
||||||||||
private String getSSHKey() { | ||||||||||
return sshkey; | ||||||||||
} | ||||||||||
|
||||||||||
private String getPassword() { | ||||||||||
return password; | ||||||||||
} | ||||||||||
|
||||||||||
public String getRemoteDatabaseHost() { | ||||||||||
return remoteDatabaseHost; | ||||||||||
} | ||||||||||
|
||||||||||
public String getRemoteDatabasePort() { | ||||||||||
return remoteDatabasePort; | ||||||||||
} | ||||||||||
|
||||||||||
public String getTunnelDatabasePort() { | ||||||||||
return tunnelDatabasePort; | ||||||||||
} | ||||||||||
Comment on lines
+153
to
+187
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the benefit of exposing all of these getters publicly? should they at least be private? or since these variables are only used internally, can we just reference the fields directly? |
||||||||||
|
||||||||||
/** | ||||||||||
* From the RSA format private key string, use bouncycastle to deserialize the key pair, reconstruct | ||||||||||
* the keys from the key info, and return the key pair for use in authentication. | ||||||||||
* | ||||||||||
* @return | ||||||||||
* @throws IOException | ||||||||||
*/ | ||||||||||
private KeyPair getPrivateKeyPair() throws IOException { | ||||||||||
PEMParser pemParser = new PEMParser(new StringReader(getSSHKey())); | ||||||||||
PEMKeyPair keypair = (PEMKeyPair) pemParser.readObject(); | ||||||||||
JcaPEMKeyConverter converter = new JcaPEMKeyConverter(); | ||||||||||
return new KeyPair( | ||||||||||
(RSAPublicKey) converter.getPublicKey(SubjectPublicKeyInfo.getInstance(keypair.getPublicKeyInfo())), | ||||||||||
(RSAPrivateKey) converter.getPrivateKey(keypair.getPrivateKeyInfo())); | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Generates a new ssh client and returns it, with forwarding set to accept all types; use this | ||||||||||
* before opening a tunnel. | ||||||||||
* | ||||||||||
* @return | ||||||||||
*/ | ||||||||||
private SshClient createClient() { | ||||||||||
java.security.Security.addProvider( | ||||||||||
new org.bouncycastle.jce.provider.BouncyCastleProvider()); | ||||||||||
SshClient client = SshClient.setUpDefaultClient(); | ||||||||||
client.setForwardingFilter(AcceptAllForwardingFilter.INSTANCE); | ||||||||||
client.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE); | ||||||||||
return client; | ||||||||||
} | ||||||||||
|
||||||||||
private void validate() { | ||||||||||
if (getHost() == null) { | ||||||||||
throw new RuntimeException("SSH Tunnel host is null - verify configuration before starting tunnel!"); | ||||||||||
} | ||||||||||
Comment on lines
+221
to
+223
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
we tend to use Preconditions for validations like this. 1. theoretically takes fewer lines of code (though that's not always true) 2. gives you a more specific error message (in this case NPE) |
||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Starts an ssh session; wrap this in a try-finally and use closeTunnel() to close it. | ||||||||||
* | ||||||||||
* @return | ||||||||||
* @throws IOException | ||||||||||
* @throws InvalidKeySpecException | ||||||||||
* @throws NoSuchAlgorithmException | ||||||||||
* @throws URISyntaxException | ||||||||||
*/ | ||||||||||
private ClientSession openTunnel(SshClient client) { | ||||||||||
try { | ||||||||||
validate(); | ||||||||||
client.start(); | ||||||||||
ClientSession session = client.connect( | ||||||||||
getUser().trim(), | ||||||||||
getHost().trim(), | ||||||||||
Integer.parseInt( | ||||||||||
getTunnelSshPort().trim())) | ||||||||||
.verify(TIMEOUT_MILLIS) | ||||||||||
.getSession(); | ||||||||||
if (getMethod().equals("SSH_KEY_AUTH")) { | ||||||||||
session.addPublicKeyIdentity(getPrivateKeyPair()); | ||||||||||
} | ||||||||||
if (getMethod().equals("SSH_PASSWORD_AUTH")) { | ||||||||||
session.addPasswordIdentity(getPassword()); | ||||||||||
} | ||||||||||
session.auth().verify(TIMEOUT_MILLIS); | ||||||||||
SshdSocketAddress address = session.startLocalPortForwarding( | ||||||||||
new SshdSocketAddress(SshdSocketAddress.LOCALHOST_ADDRESS.getHostName(), Integer.parseInt(getTunnelDatabasePort().trim())), | ||||||||||
new SshdSocketAddress(getRemoteDatabaseHost().trim(), Integer.parseInt(getRemoteDatabasePort().trim()))); | ||||||||||
LOGGER.info("Established tunneling session. Port forwarding started on " + address.toInetSocketAddress()); | ||||||||||
return session; | ||||||||||
} catch (IOException e) { | ||||||||||
throw new RuntimeException(e); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
@Override | ||||||||||
public String toString() { | ||||||||||
return "SSHTunnel{" + | ||||||||||
"method='" + method + '\'' + | ||||||||||
", host='" + host + '\'' + | ||||||||||
", tunnelSshPort='" + tunnelSshPort + '\'' + | ||||||||||
", user='" + user + '\'' + | ||||||||||
", remoteDatabaseHost='" + remoteDatabaseHost + '\'' + | ||||||||||
", remoteDatabasePort='" + remoteDatabasePort + '\'' + | ||||||||||
", tunnelDatabasePort='" + tunnelDatabasePort + '\'' + | ||||||||||
'}'; | ||||||||||
} | ||||||||||
|
||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -11,6 +11,7 @@ application { | |||||
dependencies { | ||||||
implementation 'com.google.cloud:google-cloud-storage:1.113.16' | ||||||
implementation 'com.google.auth:google-auth-library-oauth2-http:0.25.5' | ||||||
implementation group: 'org.apache.sshd', name: 'sshd-mina', version: '2.7.0' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
implementation project(':airbyte-db') | ||||||
implementation project(':airbyte-integrations:bases:base-java') | ||||||
|
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.
nit: we generally use this format