spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
707 stars 704 forks source link

BootstrapConfigFileApplicationListener cant handle optional config locations #1292

Closed larsgrefer closed 11 months ago

larsgrefer commented 1 year ago

Describe the bug

BootstrapConfigFileApplicationListener can't handle spring.config.additional-locations with optional: prefixes.

When setting spring.config.additional-location to optional:file:/foo or optional:/bar, https://github.com/spring-cloud/spring-cloud-commons/blob/1a387e1e39d3ce5f2b6a32668eb8ede9e24154d4/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapConfigFileApplicationListener.java#L701-L718 will turn these locations into file:optional:file:/foo and file:optional:/bar.

The following code will then decide that the Resource file:optional:file:/foo/application.properties does not exist, (while file:/foo/application.properties would exist) https://github.com/spring-cloud/spring-cloud-commons/blob/1a387e1e39d3ce5f2b6a32668eb8ede9e24154d4/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapConfigFileApplicationListener.java#L501-L508

This can cause ordering issues between internal and external configuration files, depending on the execution order of BootstrapConfigFileApplicationListener and ConfigDataEnvironmentPostProcessor (so #1213 will hide this problem)

When BootstrapConfigFileApplicationListener runs first, we get this order:

  1. ~external properties~ (not found by spring cloud because of this bug)
  2. internal properties (found by BootstrapConfigFileApplicationListener)
  3. external-properties (found by ConfigDataEnvironmentPostProcessor)
  4. internal properties (found by ConfigDataEnvironmentPostProcessor)

When ConfigDataEnvironmentPostProcessor runs first, we get this order:

  1. external-properties (found by ConfigDataEnvironmentPostProcessor)
  2. internal properties (found by ConfigDataEnvironmentPostProcessor)
  3. ~external properties~ (not found by spring cloud because of this bug)
  4. internal properties (found by BootstrapConfigFileApplicationListener)

In the first case, the internal properites suddenly get precedence before the external ones.

ryanjbaxter commented 11 months ago

But since #1213 this should no longer matter correct?

spring-cloud-issues commented 11 months ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

larsgrefer commented 11 months ago

@ryanjbaxter #1213 will hide this, yes

ryanjbaxter commented 11 months ago

After discussing this with the team we view #1213 as a fix for the issue so this can be closed