Skip to content
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

Recommendations for new behavior of PropertySourcesPlaceholderConfigurer #34124

Open
diegomarquezp opened this issue Dec 19, 2024 · 3 comments
Open
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on

Comments

@diegomarquezp
Copy link

diegomarquezp commented Dec 19, 2024

(Originally posted in spring-projects/spring-boot#43579)

Overview

Hello, I'm a maintainer of the Spring Framework on Google Cloud. We have recently been working on upgrading Spring Cloud to 2024.0 and Spring Boot to 3.4. So far, so good, except for our secretmanager integration.

The way we support bean injections for this integration is by specifying properties with the syntax ${sm://secret_id}, implying a request to the Secret Manager service.

Unfortunately, the colon : character is now being interpreted differently since 00e05e6, with the resulting behavior being to interpret it as "property sm with fallback value of //secret_id". We have confirmed this in our code sample.

What have we tried so far

We have tried introducing our own static PropertySourcesPlaceholderConfigurer bean (see bean declaration, see source), set up to be configured before the default PlaceholderConfigurer autoconfiguration.

Interestingly, this works perfectly for @Value annotations such as @Value("${sm://my_secret}") (see example), but not for @Configuration classes that reference a properties file (in our case, simply application.properties). I confirmed that the following code has the default PropertySourcesPlaceholderConfigurer (this.placeholdersResolver in Binder.java) when debugging my application:

https://github.com/spring-projects/spring-boot/blob/60e0de79ea9de2989ab03e9a193ae95de1cfcbf0/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java#L459-L465

More context

Alternatives considered

We are also thinking of additionally supporting the ${sm//my_secret} syntax (without the colon) as a workaround to escape the colon with a backslash, however we would like to avoid our users changing their code as much as possible.

Questions

My first question is: Why would the properties file be parsed with a different PlaceholderConfigurer than what's specified in the static bean? I believe this bean is meant to be a singleton, so I'm not sure where in the application it's decided to associate the default configurer to this configuration.

My second question is: Is there a preferred way to adapt to this new behavior?

@snicoll
Copy link
Member

snicoll commented Dec 20, 2024

We should be able to find a way to restore the previous behavior in case of perfect match if I understood the use case correctly. To get started, we'd need a small sample that demonstrates the problem. I know you've shared one but it would be nice if we didn't have to use the Google Cloud SDK to get it running.

Creating your own PropertySourcesPlaceholderConfigurer at that level is a bad idea as it's an application context-wide change that should not happen.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue in: core Issues in core modules (aop, beans, core, context, expression) labels Dec 20, 2024
@diegomarquezp
Copy link
Author

diegomarquezp commented Dec 20, 2024

Thanks for the swift response! I'll soon provide a concise reproducer.

Creating your own PropertySourcesPlaceholderConfigurer at that level is a bad idea as it's an application context-wide change that should not happen.

How can we narrow down the scope of this bean?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 20, 2024
@diegomarquezp
Copy link
Author

I have created a minimal reproducer: https://github.com/diegomarquezp/spring-placeholder-configurer-behavior-demo/tree/main

Again, thanks for looking into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on
Projects
None yet
Development

No branches or pull requests

3 participants