search

spring-cloud / spring-cloud-consul

Spring Cloud Consul
http://cloud.spring.io/spring-cloud-consul/
Apache License 2.0
813 stars 541 forks source link

The priority of defaulContext is now higher than the application context with spring.config.import #702

Closed wagabunda closed 3 years ago

wagabunda commented 3 years ago

Describe the bug There is a defaulContext property in the Consul Config library. It sets the folder name used by all applications. Link to documentation In the Spring Boot 2.3.x, its priority was lower then the application context. But in the Spring Boot 2.4.2 / Spring Cloud 2020.0.0 its priority is higher then the application context. Now it is not possible to use it as default context. Values from them override values from the application context. This is a significant problem in distributed systems. Maybe I made a mistake, but I don't see a solution.

Sample I prepared a sample on github

kobynet commented 3 years ago

I've already issued a ticket for this in spring boot repo, see https://github.com/spring-projects/spring-boot/issues/24860

The root cause is changes in spring boot 2.4, but as @spencergibb said, changes might be needed in spring-cloud-consul in order to align.

wagabunda commented 3 years ago

I agree with you. Probably the logic used here is not compatible with new Spring Boot Code link: Collections.reverse(contexts);

kobynet commented 3 years ago

@spencergibb the fix you applied did fix the propertysource order, but unfortunately when using any active profile in addition to the default profile, the profile propertysource will still get precedence over consul property sources.

You can use the same sample mentioned in this issue, just add some profile.

So we get property sources in this order now:

  1. Profile specific property source - for example application-dev.yaml
  2. Consul profile specific property source - app-name,profile-name path
  3. Consul profile specific property source - app-name path
  4. Consul profile specific property source - common-context,profilename path
  5. Consul profile specific property source - common-context path
  6. Default profile property source - application.yaml

I can open a new issue if that's necessary

tried with both versions 3.0.1 and 3.0.2-SNAPSHOT

spencergibb commented 3 years ago

The ordering of boot property sources vs those via import is known. See https://github.com/spring-cloud/spring-cloud-config/issues/1795 (similar issue in config client). That ordering is owned by boot. I'll have a chat with them.

spencergibb commented 3 years ago

Adding --spring.config.import=consul: as an cli or system property gets you the same ordering while we sort things out. Or even an EnvironmentPostProcessor with Ordered of Ordered.HIGHEST_PRECEDENCE that simply adds spring.config.import=consul: to a MapPropertySource works.

kobynet commented 3 years ago

I can confirm adding --spring.config.import=consul: as system property does get me the right order, thanks A LOT!

krisiye commented 3 years ago

@spencergibb - Similar to the above, is there a way to control the ordering of imports within config Data API? Let's say we have spring.config.import=vault:,consul: this works like a charm. However, if we have consul behind ACL, and vault is enabled for the consul secret backend and is responsible for picking up the consul acl token, there appears to be an issue where the acl-token is not going to be available in time for the Consul Data Loader. In the leagcy bootstrap, we had a workaround that is documented here: https://gist.github.com/mp911de/17f550ffecdc9e8f22061bfdf896bbb4 but that would not apply for boot 2.4 and config Data API. Any thoughts?

Please Note: There is also an open issue about enabling consul backend for config data api (see https://github.com/spring-cloud/spring-cloud-vault/pull/580)