-
Notifications
You must be signed in to change notification settings - Fork 119
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
feat: enable setting ipType configuration option for SQL Server connector #936
Changes from all commits
8fff660
53b5936
75c06e7
11fbfbe
abc4c1e
b85aaed
43cf623
ec1b325
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 |
---|---|---|
|
@@ -17,9 +17,13 @@ | |
package com.google.cloud.sql.sqlserver; | ||
|
||
import com.google.cloud.sql.core.CoreSocketFactory; | ||
import com.google.common.annotations.VisibleForTesting; | ||
import java.io.IOException; | ||
import java.io.UnsupportedEncodingException; | ||
import java.net.InetAddress; | ||
import java.net.Socket; | ||
import java.net.URLDecoder; | ||
import java.nio.charset.StandardCharsets; | ||
import java.util.Properties; | ||
import java.util.logging.Logger; | ||
|
||
|
@@ -32,7 +36,9 @@ | |
public class SocketFactory extends javax.net.SocketFactory { | ||
|
||
private static final Logger logger = Logger.getLogger(SocketFactory.class.getName()); | ||
private Properties props = new Properties(); | ||
// props are protected, not private, so that they can be accessed from unit tests | ||
@VisibleForTesting | ||
protected Properties props = new Properties(); | ||
|
||
static { | ||
CoreSocketFactory.addArtifactId("cloud-sql-connector-jdbc-sqlserver"); | ||
|
@@ -42,8 +48,25 @@ public class SocketFactory extends javax.net.SocketFactory { | |
* Implements the {@link SocketFactory} constructor, which can be used to create authenticated | ||
* connections to a Cloud SQL instance. | ||
*/ | ||
public SocketFactory(String instanceName) { | ||
this.props.setProperty(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY, instanceName); | ||
public SocketFactory(String socketFactoryConstructorArg) | ||
throws UnsupportedEncodingException { | ||
String[] s = socketFactoryConstructorArg.split("\\?"); | ||
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. You may want to use String.indexOf('?') instead of String.split(). Customer may include more than one '?' characters and you don't really want to split on all '?', just the first one. 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 would be the use case for more than one 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 use ParseURI? 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.props.setProperty(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY, s[0]); | ||
if (s.length == 2 && s[1].length() > 0) { | ||
String[] queryParams = s[1].split("&"); | ||
hessjcg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (String param : queryParams) { | ||
String[] splitParam = param.split("="); | ||
if (splitParam.length != 2 || splitParam[0].length() == 0 || splitParam[1].length() == 0) { | ||
throw new IllegalArgumentException(String.format( | ||
"Malformed query param in socketFactoryConstructorArg : %s", param)); | ||
} | ||
this.props.setProperty(URLDecoder.decode(splitParam[0], StandardCharsets.UTF_8.name()), | ||
URLDecoder.decode(splitParam[1], StandardCharsets.UTF_8.name())); | ||
} | ||
} else if (s.length > 2) { | ||
throw new IllegalArgumentException( | ||
"Only one query string allowed in socketFactoryConstructorArg"); | ||
} | ||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
/* | ||
* Copyright 2022 Google LLC | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package com.google.cloud.sql.sqlserver; | ||
|
||
import static com.google.common.truth.Truth.assertThat; | ||
|
||
import com.google.cloud.sql.core.CoreSocketFactory; | ||
import java.io.UnsupportedEncodingException; | ||
import org.junit.Test; | ||
import org.junit.runner.RunWith; | ||
import org.junit.runners.JUnit4; | ||
|
||
@RunWith(JUnit4.class) | ||
public class JdbcSqlServerUnitTests { | ||
|
||
private static final String CONNECTION_NAME = "my-projectmy-regionmy-instance"; | ||
|
||
@Test | ||
public void checkConnectionStringNoQueryParams() | ||
throws UnsupportedEncodingException { | ||
String socketFactoryConstructorArg = CONNECTION_NAME; | ||
SocketFactory socketFactory = new SocketFactory(socketFactoryConstructorArg); | ||
assertThat(socketFactory.props.get(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY)).isEqualTo( | ||
CONNECTION_NAME); | ||
} | ||
|
||
@Test | ||
public void checkConnectionStringWithQueryParam() | ||
throws UnsupportedEncodingException { | ||
String socketFactoryConstructorArg = String.format("%s?%s=%s", CONNECTION_NAME, "ipTypes", | ||
"PRIVATE"); | ||
SocketFactory socketFactory = new SocketFactory(socketFactoryConstructorArg); | ||
assertThat(socketFactory.props.get(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY)).isEqualTo( | ||
CONNECTION_NAME); | ||
assertThat(socketFactory.props.get("ipTypes")).isEqualTo( | ||
"PRIVATE"); | ||
} | ||
|
||
hessjcg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Test | ||
public void checkConnectionStringWithEmptyQueryParam() | ||
throws UnsupportedEncodingException { | ||
String socketFactoryConstructorArg = String.format("%s?", CONNECTION_NAME); | ||
SocketFactory socketFactory = new SocketFactory(socketFactoryConstructorArg); | ||
assertThat(socketFactory.props.get(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY)).isEqualTo( | ||
CONNECTION_NAME); | ||
assertThat(socketFactory.props.get("ipTypes")).isEqualTo( | ||
null); | ||
} | ||
|
||
@Test | ||
public void checkConnectionStringWithUrlEncodedParam() | ||
throws UnsupportedEncodingException { | ||
String socketFactoryConstructorArg = String.format("%s?token=%s", CONNECTION_NAME, | ||
"abc%20def%20xyz%2F%26%3D"); | ||
SocketFactory socketFactory = new SocketFactory(socketFactoryConstructorArg); | ||
assertThat(socketFactory.props.get(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY)).isEqualTo( | ||
CONNECTION_NAME); | ||
assertThat(socketFactory.props.get("token")).isEqualTo( | ||
"abc def xyz/&="); | ||
} | ||
|
||
@Test(expected = IllegalArgumentException.class) | ||
public void checkConnectionStringWithParamMissingKey() | ||
throws UnsupportedEncodingException { | ||
String socketFactoryConstructorArg = String.format("%s?=%s", CONNECTION_NAME, "PRIVATE"); | ||
new SocketFactory(socketFactoryConstructorArg); | ||
} | ||
|
||
@Test(expected = IllegalArgumentException.class) | ||
public void checkConnectionStringWithParamMissingValue() | ||
throws UnsupportedEncodingException { | ||
String socketFactoryConstructorArg = String.format("%s?enableIamAuth=true&%s", CONNECTION_NAME, | ||
"ipTypes"); | ||
new SocketFactory(socketFactoryConstructorArg); | ||
} | ||
|
||
} |
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.
https://guava.dev/releases/19.0/api/docs/com/google/common/annotations/VisibleForTesting.html