diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/main/java/com/google/cloud/spanner/jdbc/ConnectionOptions.java b/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/main/java/com/google/cloud/spanner/jdbc/ConnectionOptions.java index f483d1250c44..bae959866d45 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/main/java/com/google/cloud/spanner/jdbc/ConnectionOptions.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/main/java/com/google/cloud/spanner/jdbc/ConnectionOptions.java @@ -225,6 +225,7 @@ public static class Builder { private Credentials credentials; private List statementExecutionInterceptors = Collections.emptyList(); + private CredentialsService credentialsService = CredentialsService.INSTANCE; private Builder() {} @@ -317,6 +318,11 @@ Builder setCredentials(Credentials credentials) { return this; } + @VisibleForTesting + void setCredentialsService(CredentialsService credentialsService) { + this.credentialsService = credentialsService; + } + /** @return the {@link ConnectionOptions} */ public ConnectionOptions build() { Preconditions.checkState(this.uri != null, "Connection URI is required"); @@ -370,10 +376,23 @@ private ConnectionOptions(Builder builder) { + matcher.group(Builder.HOST_GROUP); this.instanceId = matcher.group(Builder.INSTANCE_GROUP); this.databaseName = matcher.group(Builder.DATABASE_GROUP); - this.credentials = - builder.credentials == null - ? getCredentialsService().createCredentials(this.credentialsUrl) - : builder.credentials; + Credentials credentials = null; + if (builder.credentials == null) { + try { + credentials = builder.credentialsService.createCredentials(this.credentialsUrl); + } catch (Exception e) { + // Ignore the error if the user did not set a credentials URL. + // The error is then caused by an invalid environment configuration + // and should not be thrown here. + if (this.credentialsUrl != null) { + throw e; + } + credentials = null; + } + } else { + credentials = builder.credentials; + } + this.credentials = credentials; String numChannelsValue = parseNumChannels(builder.uri); if (numChannelsValue != null) { try { @@ -401,11 +420,6 @@ private ConnectionOptions(Builder builder) { Collections.unmodifiableList(builder.statementExecutionInterceptors); } - @VisibleForTesting - CredentialsService getCredentialsService() { - return CredentialsService.INSTANCE; - } - @VisibleForTesting static boolean parseUsePlainText(String uri) { String value = parseUriProperty(uri, USE_PLAIN_TEXT_PROPERTY_NAME); diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/main/java/com/google/cloud/spanner/jdbc/SpannerPool.java b/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/main/java/com/google/cloud/spanner/jdbc/SpannerPool.java index 771ad505c77f..0e236fb698c8 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/main/java/com/google/cloud/spanner/jdbc/SpannerPool.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/main/java/com/google/cloud/spanner/jdbc/SpannerPool.java @@ -277,8 +277,10 @@ Spanner createSpanner(SpannerPoolKey key) { builder .setClientLibToken(MoreObjects.firstNonNull(key.userAgent, JdbcDriver.getClientLibToken())) .setHost(key.host) - .setProjectId(key.projectId) - .setCredentials(key.credentials); + .setProjectId(key.projectId); + if (key.credentials != null) { + builder.setCredentials(key.credentials); + } if (key.numChannels != null) { builder.setNumChannels(key.numChannels); } diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/test/java/com/google/cloud/spanner/jdbc/ConnectionOptionsTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/test/java/com/google/cloud/spanner/jdbc/ConnectionOptionsTest.java index 1435cd1d9ca6..c191d0122476 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/test/java/com/google/cloud/spanner/jdbc/ConnectionOptionsTest.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-spanner-jdbc/src/test/java/com/google/cloud/spanner/jdbc/ConnectionOptionsTest.java @@ -20,14 +20,20 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; import com.google.auth.oauth2.GoogleCredentials; import com.google.auth.oauth2.ServiceAccountCredentials; +import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.SpannerOptions; import java.util.Arrays; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.Mockito; @RunWith(JUnit4.class) public class ConnectionOptionsTest { @@ -35,6 +41,8 @@ public class ConnectionOptionsTest { ConnectionOptionsTest.class.getResource("test-key.json").getFile(); private static final String DEFAULT_HOST = "https://spanner.googleapis.com"; + @Rule public ExpectedException expected = ExpectedException.none(); + @Test public void testBuildWithValidURIAndCredentialsFileURL() { ConnectionOptions.Builder builder = ConnectionOptions.newBuilder(); @@ -53,6 +61,65 @@ public void testBuildWithValidURIAndCredentialsFileURL() { assertThat(options.isReadOnly(), is(equalTo(ConnectionOptions.DEFAULT_READONLY))); } + @Test + public void testBuildWithValidEnvCredentials() throws Exception { + // Simulate that this is the credentials that have been set for the environment. + GoogleCredentials envCredentials = new CredentialsService().createCredentials(FILE_TEST_PATH); + + CredentialsService service = mock(CredentialsService.class); + // Call with null means that the JDBC driver should get the credentials from the environment. + Mockito.when(service.createCredentials((String) Mockito.isNull())).thenReturn(envCredentials); + ConnectionOptions.Builder builder = ConnectionOptions.newBuilder(); + builder.setCredentialsService(service); + builder.setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123"); + ConnectionOptions options = builder.build(); + assertThat( + (GoogleCredentials) options.getCredentials(), + is(equalTo(new CredentialsService().createCredentials(FILE_TEST_PATH)))); + } + + @Test + public void testBuildWithInvalidEnvCredentials() throws Exception { + CredentialsService service = mock(CredentialsService.class); + // Call with null means that the JDBC driver should get the credentials from the environment. + Mockito.when(service.createCredentials((String) Mockito.isNull())) + .thenThrow( + SpannerExceptionFactory.newSpannerException( + ErrorCode.INVALID_ARGUMENT, "file not found")); + ConnectionOptions.Builder builder = ConnectionOptions.newBuilder(); + builder.setCredentialsService(service); + builder.setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123"); + ConnectionOptions options = builder.build(); + assertThat((GoogleCredentials) options.getCredentials(), is(nullValue())); + } + + @Test + public void testBuildWithInvalidBuilderCredentials() throws Exception { + ConnectionOptions.Builder builder = ConnectionOptions.newBuilder(); + // Setting an invalid credentials URL should cause an exception when creating the + // ConnectionOptions. + builder.setCredentialsUrl("/path/to/non-existing-key.json"); + builder.setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123"); + expected.expect( + SpannerExceptionMatcher.matchCodeAndMessage( + ErrorCode.INVALID_ARGUMENT, "Invalid credentials path specified")); + builder.build(); + } + + @Test + public void testBuildWithInvalidConnectionURLCredentials() throws Exception { + ConnectionOptions.Builder builder = ConnectionOptions.newBuilder(); + builder.setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123?credentials=/path/to/non-existing-key.json"); + expected.expect( + SpannerExceptionMatcher.matchCodeAndMessage( + ErrorCode.INVALID_ARGUMENT, "Invalid credentials path specified")); + builder.build(); + } + @Test public void testBuildWithValidURIAndProperties() { ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();