feat: Add all additional Cloud SQL Java Connector parameters #3286
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hessjcg
force-pushed
the
gh-3285-cloud-sql-autoconfig
branch
2 times, most recently
from
14c58f5
to
a82e856
Compare
mpeddada1
reviewed
Oct 14, 2024
...src/main/java/com/google/cloud/spring/autoconfigure/sql/DefaultCloudSqlJdbcInfoProvider.java
Show resolved
Hide resolved
| @@ -54,12 +58,49 @@ public String getJdbcUrl() { | |||
| this.properties.getDatabaseName(), | |||
| this.properties.getInstanceConnectionName()); | |||
|
|
|||
| // Build additional JDBC url parameters from the configuration. | |||
There was a problem hiding this comment.
Could we also add testing for these new properties in
There was a problem hiding this comment.
This test method should do the trick:
@Test
void testCloudSqlSpringDatasourceWithAllOptions() {
this.contextRunner
.withPropertyValues(
"spring.cloud.gcp.sql.admin-quota-project=someAdminQuotaProjectValue",
"spring.cloud.gcp.sql.admin-root-url=someAdminRootUrlValue",
"spring.cloud.gcp.sql.admin-service-path=someAdminServicePathValue",
"spring.cloud.gcp.sql.database-name=test-database",
"spring.cloud.gcp.sql.delegates=delegate1,delegate2",
"spring.cloud.gcp.sql.enable-iam-auth=true",
"spring.cloud.gcp.sql.instance-connection-name=tubular-bells:singapore:test-instance",
"spring.cloud.gcp.sql.ip-types=PRIVATE",
"spring.cloud.gcp.sql.refresh-strategy=lazy",
"spring.cloud.gcp.sql.target-principal=target-principal",
"spring.cloud.gcp.sql.universe-domain=someUniverseDomainValue",
"spring.datasource.username=foo",
"spring.datasource.password=bar")
.run(
context -> {
HikariDataSource dataSource = (HikariDataSource) context.getBean(DataSource.class);
assertThat(dataSource)
.returns("com.mysql.cj.jdbc.Driver", HikariDataSource::getDriverClassName)
.returns("foo", HikariDataSource::getUsername)
.returns("bar", HikariDataSource::getPassword)
.extracting(HikariDataSource::getJdbcUrl)
.asInstanceOf(InstanceOfAssertFactories.STRING)
.startsWith("jdbc:mysql://google/test-database?"
+ "socketFactory=com.google.cloud.sql.mysql.SocketFactory")
.satisfies(
jdbcUrl -> assertThat(jdbcUrl.substring(jdbcUrl.indexOf('&') + 1).split("&"))
.containsExactlyInAnyOrder(
"cloudSqlAdminQuotaProject=someAdminQuotaProjectValue",
"cloudSqlAdminRootUrl=someAdminRootUrlValue",
"cloudSqlAdminServicePath=someAdminServicePathValue",
"cloudSqlDelegates=delegate1%2Cdelegate2",
"cloudSqlInstance=tubular-bells:singapore:test-instance",
"cloudSqlRefreshStrategy=lazy",
"cloudSqlTargetPrincipal=target-principal",
"cloudSqlUniverseDomain=someUniverseDomainValue",
"enableIamAuth=true",
"ipTypes=PRIVATE",
"sslmode=disable"));
assertThat(getSpringDatasourceDriverClassName(context))
.matches("com.mysql.cj.jdbc.Driver");
});
}I tried to create a PR to the existing PR, but I have no clue about your formatting rules (spotless?)
@hessjcg , maybe you accept this "PR" by just copy the code 😆
There was a problem hiding this comment.
Thank you @mieseprem for the test! I have moved it code into this PR.
| .map( | ||
| entry -> | ||
| URLEncoder.encode(entry.getKey()) + "=" + URLEncoder.encode(entry.getValue())) | ||
| .collect(Collectors.joining("&")); |
There was a problem hiding this comment.
Could we add at least one test that verifies the output of this logic? This would protect against future modifications.
There was a problem hiding this comment.
@burkedavison , would a test like the one in #3286 (comment) be enough?
…pring configuration. This adds JDBC needed for lazy refresh, service account impersonation, universe domains, and alternate SQL Admin API endpoints to the JDBC configuration for a Cloud SQL JDBC connection. This is related to GoogleCloudPlatform/cloud-sql-jdbc-socket-factory#2065.
hessjcg
force-pushed
the
gh-3285-cloud-sql-autoconfig
branch
from
a82e856
to
68ebfa4
Compare
|
burkedavison
approved these changes
Oct 31, 2024
mpeddada1
reviewed
Oct 31, 2024
...st/java/com/google/cloud/spring/autoconfigure/sql/CloudSqlEnvironmentPostProcessorTests.java
Show resolved
Hide resolved
mpeddada1
approved these changes
Nov 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds JDBC properties needed for lazy refresh, service account impersonation, universe domains, and alternate
SQL Admin API endpoints to the JDBC configuration for a Cloud SQL JDBC connection.
See #3285
This is related to GoogleCloudPlatform/cloud-sql-jdbc-socket-factory#2065.