spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.55k stars 40.54k forks source link

@ConfigurationProperties should have an option to have properties read each time from Environment #259

Open philwebb opened 10 years ago

philwebb commented 10 years ago

Originally raised on the SPR issue tracker: https://jira.springsource.org/browse/SPR-11289

@ConfigurationProperties objects should have some mechanism for allowing some or all properties to be read from a propertySource on each get invocation. It seems like instead of injected a concrete implementation into the AppContext we should inject a proxy for the Config object which can reference the Environment abstraction.

The usecase for this would be when we have implemented a custom PropertySource which is aware of property changes.

dsyer commented 10 years ago

Sounds like a wider scope than just @ConfigurationProperties to me. Doesn't the same feature potentially apply to any bean that can refresh itself on an environment change?

That is leaving aside the question of whether this is a good idea or not. Is it a good idea? In a world of stateless micro-services isn't it better to just change the config and restart the app? Or are we mainly talking about development time? What's the benefit exactly?

philwebb commented 10 years ago

I am not sure if the original author is watching this or not.

I agree, this might not be a great idea. Dynamically reloading properties can be a tricky problem, often just updating the @ConfigurationProperties object won't be enough, you need more of an event based approach so that other classes are aware of the change.

I also can't see this being a very common use-case.

Perhaps we can add some hooks to allow a roll-your-own solution if 'getting on each invocation' is really needed?

twicksell commented 10 years ago

Hey, I was not watching the issue from github, sorry about that. I'm the author of the request.

I do not believe this has anything to do with beans other than @ConfigurationProperties, or at least that isn't something I'd want. To give a little more background on what I'm doing, I've got an implementation of Netflix's Archaius (https://github.com/Netflix/archaius/wiki/Users-Guide) hidden under Spring's PropertySource. Archaius provides some great support for property change listeners, reloading properties, etc. that we use pretty heavily at Netflix, but being a Spring person I greatly prefer being able to use @Value, Environment, spel property resolution, and @ConfigurationProperties whenever possible. Hence the NetflixPropertySource implementation. Of course @Value doesn't work since its only reading the property once on startup/bean creation, and I don't think I'd want that to change honestly, but with @ConfigurationProperties would be much easier to implement delegation to the Spring Environment. I'd argue it should always delegate to the PropertySources, and if you're concerned about performance/caching let that be the concern of your PropertySource implementation.

As to whether or not reloadable configuration is a good idea in a world of micro cloud services, I'd just point out that this new cloud world isn't quite perfect yet. For us, rolling out new clusters into AWS (with sizes ranging from dozens to 100+ per app cluster) can take 45minutes just to get started and perhaps hours for caches to become warm. Because of that we often find it necessary to use feature flags based on properties to enable/disable functionality across our entire cloud infrastructure. This is something Archaius does rather nicely, and its just a shame at the moment I can't leverage @ConfigurationProperties as much as I'd like to for working with Archaius.

Any kind of hook to allow me to customize the creation of @ConfigurationProperties would be sufficient, but I do suggest considering delegation to the Spring Environment as an option, if not the default behavior.

andrewharmellaw commented 10 years ago

Hi,

We're very interested in this discussion, and the recent announcement about dynamic Spring props. We've built https://github.com/Capgemini/archaius-spring-adapter which sounds like it overlaps heavily with what @twicksell has been working on. We'd be interested in being involved in work in this area as it'd help us greatly on our current project.

dsyer commented 10 years ago

@andrewharmellaw our archaius bridge is here: https://github.com/spring-cloud/spring-cloud-netflix/tree/master/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/archaius. It would be good to get feedback, or pull requests if you like, via that project.

andrewharmellaw commented 10 years ago

Cheers @dsyer We'll take a look and see if we can contribute. We're big users of Spring (Boot especially), Camel, and various Netflix bits and pieces too. Anything which helps glue them together which is OSS is of interest to us.

rachelwilson commented 9 years ago

I was wondering if someone could point me (and other readers) in the direction of the "recent announcement about dynamic Spring props" as mentioned by andrewharmellaw above. I was interested to hear what the latest thinking is and I cant seem to find anything recent.

dsyer commented 9 years ago

Try here: http://projects.spring.io/spring-cloud/spring-cloud.html#_environment_changes (and some other bits of that section).

rachelwilson commented 9 years ago

Hmm, that seems to be documentation rather than an announcement as such. Can I assume in that case that the dynamic properties apply only to the Cloud and Boot products and there are no announced changes to the original Spring framework?

dsyer commented 9 years ago

Yes, it's a Cloud feature right now (but with really minimal dependencies, as long as you are using Spring Boot).

nmarasoiu commented 8 years ago

@dsyer Hi, I am having this issue with Spring Cloud: I added a MapPropertySource to the env, I am changing the values from the ConcurentMap from another thread, but I can't see those values reflecting in env.getProperty or in @ConfigurationProperties beans. Pls advise!

dsyer commented 8 years ago

If you really changed the environment, you would see it in getProperty(), so if you didn't see it there, then I suspect you didn't change it, or you changed a property that is overridden in another property sourec. You can see all the property sources in the /env endpoint.

You have to rebind to get @ConfigurationProperties to update, so that's a different problem. There's a ContextRefresher in spring-cloud-context that you can use if you want to trigger it manually, or you can use the /refresh endpoint.

wilkinsona commented 5 years ago

We've discussed the possibility of supporting a property that's a Supplier<?> that retrieves the value from the Environment each time get() is called. Our feeling is that this isn't a viable solution for two reasons:

  1. For a frequently used infrequently changed piece of configuration, retrieving the property from the environment each time is inefficient
  2. Retrieving a consistent view of updates to multiple properties is hard, if not impossible.

Our feeling, as it was above, is that an event-based approach would be better. The event would provide a complete new instance of a configuration properties class with the changed value or values. This addresses both of the problems described above while also working with immutable configuration properties that are currently in plan for 2.2.

The question remains if this is a good idea. We'd also be interested in hearing some of the use cases that a change in this area would enable. As things stand, there hasn't been particularly broad interest in this functionality. Given the reasonably large amount of complexity that it would introduce, it's difficult to justify implementing it at this time.

Code2Life commented 2 years ago

Hi @philwebb @dsyer @wilkinsona,

I think the original feature this issue requests have been implemented in my project: https://github.com/Code2Life/spring-boot-dynamic-config

Adding @DynamicConfig on @ConfigurationProperties simply works !

With Spring Boot native features leveraged, and with the file watcher and hot-reload mechanism implemented by only 400+ lines of code, @Value @ConfigurationProperties could always be the newest value.

This lib is widely used in 20+ core applications for more than half a year in the company I'm working for. Hope this feature becomes a built-in feature of Spring Boot. @philwebb @dsyer @wilkinsona Would you like to take a look at the source codes and put some suggestions? Thanks !

denon82 commented 2 years ago

Great job. As of a suggestion I would remove the Lombok dependency and only depend on spring codebase.

One question: this dynamic configuration only affects the given project? What about share configuration between project? That what spring-config is intended to do, while your project is intented to make the configurations dynamic to the project code base. How about security?

Do you want to incorporate your codebase to spring's codebase? Of course, if they consider it interesting.

Regards,

Flavio Oliva

On Sat, 22 Jan 2022, 13:01 Joey Yang, @.***> wrote:

Already implemented in my project: https://github.com/Code2Life/spring-boot-dynamic-config

Adding @DynamicConfig on @ConfigurationProperties https://github.com/ConfigurationProperties simply works.

This lib is widely used in the company I work for. Hope this feature becomes a built-in feature of Spring Boot. @philwebb https://github.com/philwebb @dsyer https://github.com/dsyer

— Reply to this email directly, view it on GitHub https://github.com/spring-projects/spring-boot/issues/259#issuecomment-1019249813, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAL55TDUF5U2KYPXIOADRWLUXKTBFANCNFSM4ALPH44A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

Code2Life commented 2 years ago

@denon82 Thanks !

  1. This dynamic config only affects given project because it's an enhancement of spring boot config native ways.
  2. Config sharing could be on orchestration layer, for eg. mount same ConfigMap as files in different Kubernetes workloads
  3. As for security, there are conditional beans, so the config file will not be dynamic unless the owner intends to. Moreover, with GitOps workflow, the source of config files are in other systems like Git, thus, the security responsibilities are shifted to Git permission, CI/CD pipeline security, Kubernetes RBAC, etc.
philwebb commented 2 years ago

Thanks very much for sharing the project @Code2Life. It looks like an interesting alternative to @RefreshScope for those that want it. I don't have any specific comments on the code (although I agree with @denon82 about Lombok). We're unlikely to get to this feature in Spring Boot itself for a little while so it's great that users have another option.

Code2Life commented 2 years ago

Thanks for reviewing @philwebb , @RefreshScope relies on spring cloud stack and additional config server. we've tried spring-cloud-config and other spring-cloud components in production years ago and found it's too heavyweight, then we went back to Spring Boot.

dsyer commented 2 years ago

FYI spring-cloud-config isn't needed for @RefreshScope, and also @RefreshScope is not needed for @ConfigurationProperties. Just put spring-cloud-commons on the classpath - it's very lightweight, really - the only dependency is Spring Boot.