Skip to content

5.2.0: Autoconfiguration of AlloyDB datasource is too aggressive #2847

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

Closed
Ironlink opened this issue May 4, 2024 · 1 comment · Fixed by #2848
Closed

5.2.0: Autoconfiguration of AlloyDB datasource is too aggressive #2847

Ironlink opened this issue May 4, 2024 · 1 comment · Fixed by #2848

Comments

@Ironlink
Copy link

Ironlink commented May 4, 2024

I have a Spring Boot application which has direct dependencies on spring-cloud-gcp-starter-sql-postgresql and postgres-socket-factory.

Describe the bug
When I upgrade spring-cloud-gcp-dependencies from 5.1.2 to 5.2.0, the Spring context for my unit/integration tests fails to start with the following stack trace:

Caused by: java.lang.IllegalArgumentException: 'alloydbInstanceName' must have format: projects/<PROJECT>/locations/<REGION>/clusters/<CLUSTER>/instances/<INSTANCE>
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:145) ~[guava-33.1.0-jre.jar:?]
	at com.google.cloud.alloydb.ConnectionConfig.validateProperties(ConnectionConfig.java:112) ~[alloydb-jdbc-connector-1.1.1.jar:1.1.1]
	at com.google.cloud.alloydb.ConnectionConfig.fromConnectionProperties(ConnectionConfig.java:48) ~[alloydb-jdbc-connector-1.1.1.jar:1.1.1]
	at com.google.cloud.alloydb.SocketFactory.<init>(SocketFactory.java:29) ~[alloydb-jdbc-connector-1.1.1.jar:1.1.1]
	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) ~[?:?]
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502) ~[?:?]
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486) ~[?:?]
	at org.postgresql.util.ObjectFactory.instantiate(ObjectFactory.java:66) ~[postgresql-42.6.2.jar:42.6.2]
	at org.postgresql.core.SocketFactoryFactory.getSocketFactory(SocketFactoryFactory.java:39) ~[postgresql-42.6.2.jar:42.6.2]

There is no mention of AlloyDB in my application, I am not using that product.

For autoconfiguration to be a positive experience, I think the heuristics in AlloyDbEnvironmentPostProcessor need to improve. It is fair to fail on a missing or malformed instance name when spring.cloud.gcp.alloydb.enabled is explicitly set to true, but when it is left blank doing so comes across as a burden. Perhaps the default value for spring.cloud.gcp.alloydb.enabled should not be true unless some other condition is met, such as an instance name being available?

I see there is a check for whether com.google.cloud.alloydb.SocketFactory is on the classpath, but spring-cloud-gcp-autoconfigure has a dependency on alloydb-jdbc-connector with scope compile, so this will always be true unless users start excluding artifacts. I also see that spring-cloud-gcp-starter-alloydb has a dependency on alloydb-jdbc-connector, perhaps the dependency from spring-cloud-gcp-autoconfigure to alloydb-jdbc-connector should have scope provided? Users of the starter module would still receive the AlloyDB SocketFactory transitively, since the starter module has its own dependency with compile scope.

@meltsufin
Copy link
Member

@Ironlink Thank you reporting the issue!
The dependency should be optional in spring-cloud-gcp-autoconfigure. I'm surprised that our own tests did not catch this.
cc: @ttosta-google

meltsufin added a commit that referenced this issue May 4, 2024
Making `alloydb-jdbc-connector` optional dependency in autoconfig module.

Fixes: #2847.
meltsufin added a commit that referenced this issue May 6, 2024
Making `alloydb-jdbc-connector` optional dependency in autoconfig module.

Fixes: #2847.
@meltsufin meltsufin self-assigned this May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants