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

Fixed order of the BootstrapConfigFileApplicationListener so that it always runs after the ConfigDataEnvironmentPostProcessor #1213

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

mzeijen
Copy link
Contributor

@mzeijen mzeijen commented Mar 8, 2023

This PR changes the order of the BootstrapConfigFileApplicationListener to make sure that the BootstrapConfigFileApplicationListener always is executed after the ConfigDataEnvironmentPostProcessor.

The reason
The BootstrapConfigFileApplicationListener relies on the Environment.activeProfiles to have been set correctly, like for instance with the SpringApplication.additionalProfiles. The environment post processor responsible for setting the Environment.activeProfiles correctly (e.g. by adding the SpringApplication.additionalProfiles) is the org.springframework.boot.context.config.ConfigDataEnvironmentPostProcessor. If it runs before the BootstrapConfigFileApplicationListener then everything works correctly. However both the BootstrapConfigFileApplicationListener and the ConfigDataEnvironmentPostProcessor have the exact same order so there is no guarantee in which order they run, meaning that it can also happen that BootstrapConfigFileApplicationListener is executed before the ConfigDataEnvironmentPostProcessor, and in that case the Environment.activeProfiles are not set correctly.

What happens if the two listeners are executed in the wrong order
If the listeners are executed in the wrong order, the Environment.activeProfiles will not have all profiles when the org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener is executed. If there are application-<profile>.yml files for those missing profiles, it then does not create the applicationConfig property sources for those configuration files.

For example, I start up the application and provide the production profile as an additionalProfile (for instance via SpringApplicationBuilder.additionalProfile()), and I have a application.yml and application-production.yml on the classpath. If I then look at the property sources, I expect to see the following property source ordering (I left out the unrelated property sources):

  • Config resource 'class path resource [application-production.yml]' via location 'optional:classpath:/'
  • Config resource 'class path resource [application.yml]' via location 'optional:classpath:/'
  • applicationConfig: [classpath:/application-production.yml]
  • applicationConfig: [classpath:/application.yml]

However what I sometimes get is:

  • applicationConfig: [classpath:/application.yml]
  • Config resource 'class path resource [application-production.yml]' via location 'optional:classpath:/'
  • Config resource 'class path resource [application.yml]' via location 'optional:classpath:/'

So you can see that the application.yml now has the highest order, and the applicationConfig: [classpath:/application-production.yml] property source is missing. This means the wrong property values will be returned for the properties defined in application-production.yml.

Side question:
I wonder why the BootstrapConfigFileApplicationListener is even needed to create these property sources, as what I can see is that Spring Boot itself also creates the property sources for these files. So now there are always two property sources for every configuration file.

Previously `BootstrapConfigFileApplicationListener` had the same order
as `ConfigDataEnvironmentPostProcessor`. This made it indeterminable
which one would run first. However, the
`BootstrapConfigFileApplicationListener` relies on the
`ConfigDataEnvironmentPostProcessor` to make sure the
`Environment.activeProfiles` are correctly set, so it must always run
after the `ConfigDataEnvironmentPostProcessor`.
…tProcessor.DEFAULT_ORDER` so that any accidental overflow results in an exception
@mzeijen
Copy link
Contributor Author

mzeijen commented Jul 12, 2023

@spencergibb Hi, I was wondering when this PR is going to be discussed with the team. It has been op for quite some months now.

Kind regards,
Maurice

@spencergibb
Copy link
Member

I know @ryanjbaxter has been looking at similar issues. I'll have him look at it.

@spencergibb spencergibb requested a review from ryanjbaxter July 14, 2023 02:11
@ryanjbaxter
Copy link
Contributor

Have you retested this with Spring Cloud 2022.0.3? The logic around activating profiles has changed.

@mzeijen
Copy link
Contributor Author

mzeijen commented Jul 17, 2023

@ryanjbaxter Thanks for getting back to me. I wasn't aware of those changes. I will take a look.

@mzeijen
Copy link
Contributor Author

mzeijen commented Jul 19, 2023

@ryanjbaxter I looked at it again, and I still think this issue can occur, because the BootstrapConfigFileApplicationListener.Loader uses the active profiles from the environment. And those active profiles need to be initialised first via the ConfigDataEnvironmentPostProcessor. But both EnvironmentPostProcessor have the same order, so it can still happen that that BootstrapConfigFileApplicationListener.postProcessEnvironment is executed before the ConfigDataEnvironmentPostProcessor.postProcessEnvironment method.

With the debugger I created the following stracktraces to show where the active profiles are added first via the ConfigDataEnvironmentPostProcessor, and then read by the ConfigDataEnvironmentPostProcessor.Loader.

# The ConfigDataEnvironmentPostProcessor is the first to set the active profiles in the environment
Breakpoint reached at org.springframework.core.env.AbstractEnvironment.setActiveProfiles(AbstractEnvironment.java:306)
	at ...
	at org.springframework.boot.context.config.ConfigDataEnvironmentPostProcessor.postProcessEnvironment(ConfigDataEnvironmentPostProcessor.java:96)
	
# The BootstrapConfigFileApplicationListener then reads the active profiles from the environment
Breakpoint reached at org.springframework.core.env.AbstractEnvironment.getActiveProfiles(AbstractEnvironment.java:262)
	at org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener$Loader.getOtherActiveProfiles(BootstrapConfigFileApplicationListener.java:379)
	at org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener$Loader.initializeProfiles(BootstrapConfigFileApplicationListener.java:358)
	at org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener$Loader.loadWithFilteredProperties(BootstrapConfigFileApplicationListener.java:332)
	at org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener$Loader$$Lambda$663/0x00000008012ee6e8.accept(Unknown Source:-1)
	at org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener$FilteredPropertySource.apply(BootstrapConfigFileApplicationListener.java:997)
	at org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener$Loader.load(BootstrapConfigFileApplicationListener.java:323)
	at org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener.addPropertySources(BootstrapConfigFileApplicationListener.java:217)
	at org.springframework.cloud.bootstrap.BootstrapConfigFileApplicationListener.postProcessEnvironment(BootstrapConfigFileApplicationListener.java:205)

In this example everything works out, because the ConfigDataEnvironmentPostProcessor was executed first. But that does not need to be the case, and we have seen that in practice.

@ryanjbaxter ryanjbaxter merged commit 9ea5801 into spring-cloud:main Oct 12, 2023
ryanjbaxter pushed a commit that referenced this pull request Oct 12, 2023
…t always runs after the `ConfigDataEnvironmentPostProcessor` (#1213)

* Changed order of `BootstrapConfigFileApplicationListener`

Previously `BootstrapConfigFileApplicationListener` had the same order
as `ConfigDataEnvironmentPostProcessor`. This made it indeterminable
which one would run first. However, the
`BootstrapConfigFileApplicationListener` relies on the
`ConfigDataEnvironmentPostProcessor` to make sure the
`Environment.activeProfiles` are correctly set, so it must always run
after the `ConfigDataEnvironmentPostProcessor`.

* Using `Math.addExact` for adding one to the `ConfigDataEnvironmentPostProcessor.DEFAULT_ORDER` so that any accidental overflow results in an exception
@ryanjbaxter
Copy link
Contributor

Also cherry picked this change to 4.0.x as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants