spring-cloud/spring-cloud-config

Incorrect prioritizing of profile specific property sources

AvrahamNissimov opened this issue · 20 comments

We are trying to upgrade from Spring 2.7.2 towards Spring 3.

In commit 6ec9c43, function “ConfigServerConfigDataLocationResolver.resolve(ConfigDataLocationResolverContext, ConfigDataLocation)” was changed:

public List<ConfigServerConfigDataResource> resolve(ConfigDataLocationResolverContext context,
                ConfigDataLocation location)
                throws ConfigDataLocationNotFoundException, ConfigDataResourceNotFoundException {
        return resolveProfileSpecific(context, location, null);
}

This leads to the following problem: Configuration sources that are profile specific become prioritized over ones that are application specific. This happens because it brings sources for default profile, application specific arrive there as well, and they are added last, despite the fact they arrived also when the profile was specified and were higher prioritized.

Is there any way to receive back the previous behavior? Or any explanation what was the intention?

Thanks.

Can you provide a concrete example so I can make sure I understand exactly the behavior you are observing?

Thanks for your prompt response,

The search paths of the configuration server are set as follows:

spring.cloud.config.server.composite:
-
searchPaths:
- 'envType/{profile}'
- 'app/{application}'

Meaning, a definition in app/{application} should override the one in envType/{profile}.

If the application name is “mock” and the profile is “build”
After the change, it fetches first “/mock/default/{label}”, and then “/mock/build/{label}”.
The first fetch brings [“/app/mock/application.yml”].
The second fetch brings [“/app/mock/application.yml”, “/envType/build/application.yml”]

The resulting list of sources is composed first with the second result.
And then it addLast() for items of the first result.

  /**
  * Add the given property source object with the lowest precedence.
  */
  public void addLast(PropertySource<?> propertySource) {
        synchronized (this.propertySourceList) {
              removeIfPresent(propertySource);
              this.propertySourceList.add(propertySource);
        }
  }

So it results with [“/envType/build/application.yml”, “/app/mock/application.yml”]
The order is opposite, the definitions from envType/build override the ones in app/mock, and this is not the desired behavior.

Where is this code?

  /**
  * Add the given property source object with the lowest precedence.
  */
  public void addLast(PropertySource<?> propertySource) {
        synchronized (this.propertySourceList) {
              removeIfPresent(propertySource);
              this.propertySourceList.add(propertySource);
        }
  }

Sorry for the delay,

It comes from MutablePropertySources.
Here is the stack how it arrives there:

MutablePropertySources.addLast(PropertySource<?>) line: 116
ConfigDataEnvironment.applyContributor(ConfigDataEnvironmentContributors, ConfigDataActivationContext, MutablePropertySources) line: 354
ConfigDataEnvironment.applyToEnvironment(ConfigDataEnvironmentContributors, ConfigDataActivationContext, Set, Set) line: 331
ConfigDataEnvironment.processAndApply() line: 235

The list of contributors contains the application specific YML file twice, for each of the fetches.

Any updates on this please?
Thanks.

No. It’s on my list to look into but I am working on something else at the moment

I have looked into this a bit more and I think the current behavior is correct. The prior behavior was a side effect of the fact that everything from the config server was considered to be "profile specific".

Anything that is returned in the second request to the config server and does not already exist in the list of property sources from the first request is considered to be a profile specific property source, so it should take priority over anything that was already in the list. Property sources from active profiles will always have priority over property sources without active profiles.

I am sorry, but this does not make any sense. If files arrive in some prioritization when asked with profile, there is no justification to switch the order just because the file arrived also when profile was not set. Just like in the example above, when profile was set, application specific YML arrived, and it was higher prioritized; the priority was dropped just because it also arrived when the profile was not specified. And then it was removed and added last.

I would expect there to be a condition, if the file already exists, don't touch it. If it does not exist, add last. But if the file is moved to the bottom just because it also arrives when profile=null, this makes no sense.

We have many applications in our eco system. If something is overridden on application level, it is more important, than environment or profile. I believe it is quite a common practice: there are many microservices, and the setting on a specific microservice is more important than such setting as default for environment.

Anyway, is there any chance to receive an old behavior? Or to workaround the problem?

Thanks for your assistence.

The decision to not include a property source and the order in which they are added is actually not something Spring Cloud Config controls. This is behavior defined within Spring Boot. We just provide the property sources to Spring Boot from the config server.

In fact you can reproduce something similar with just Spring Boot.
If in src/main/resources you have 3 yaml files, application.yaml, application-build.yaml and mock.yaml. In application.yaml you have spring.config.import=classpath:mock.yaml. If you start this application the property sources will be ordered as follows

application-build.yaml
mock.yaml
application.yaml

The profile specific property source is always higher.

Sorry, this is not as good as it was. Now I have to define for each of profiles that may arrive the definitions for the application. Meaning, I have to actually know in advance what profiles there are in the system (I hope it is clear that other than profile "build" there are also profiles "testEnv1", "testEnv2", ... "prod" etc). This is simply not a feasible solution to define this way for all the profiles, if all I want is my application to start with this definition and override whatever default a profile may set.

Please correct me if I am wrong: The goal of your change was to enable adding settings from default profile, as lower prioritized than the given profile. Am I right?

If this is the case, the code had to check whether the file arrived by the given profile, and add it last only in cases it did not. Do we agree on this?

Please correct me if I am wrong: The goal of your change was to enable adding settings from default profile, as lower prioritized than the given profile. Am I right?

No that was not the goal.

Originally when Spring Cloud Conifig supported spring.config.import introduced in Spring Boot 3 we took a "shortcut" to implementing support and any profiles activated in configuration from the config server would not end up being activated in the application. This is because in Spring Boot 3 Spring Boot loads configuration in 2 phases...

In phase 1 Spring Boot loads all non-profile specific configuration. It then goes through and applies those configuration files to the environment and activate any profiles from this configuration. This is why you see an initial request to the config server with /{appname}/default.

With this configuration in place and all possible profiles activated from it a second pass will be made loading configuration with all the active profiles. Now we see a second request /{appname}/{activeprofiles}. This will return configuration not only for active profiles but also for the default profile. All of the configuration for the default profile should have been returned in the first request and are therefore not readded to the environment. All new configuration for active profiles will be added to the environment at this point.

If this is the case, the code had to check whether the file arrived by the given profile, and add it last only in cases it did not. Do we agree on this?

No, as I said above, the order in which these configuration files are added and whether a configuration is ignored is really up to Spring Boot.. Its not something Spring Cloud has any control over.

I actually think if you don't use spring.config.import and instead use Boostrap to load the configuration you might be able to return to the behavior from Boot 2.x.

All you would need to do is to add the following dependency

<dependency>
  <groupId>org.springframework.cloud</groupId>
  <artifactId>spring-cloud-starter-bootstrap</artifactId>
</dependency>

Then remove spring.config.import=configserver... and go back to using spring.cloud.config.uri.

Seems it indeed solved the problem. Many thanks for this!
Is there any way to get any reference on differences between spring.config.import and spring.cloud.config.uri? Thanks.

I am glad that unblocked things.

Essentially when adding spring-cloud-starter-bootstrap and using spring.cloud.config.uri you are loading configuration from the config server during the bootstrap phase. Spring Cloud is adding the configuration to the bootstrap application and then promoting that to the main application when it starts. This is how configuration was loaded from the config server before Spring Boot introduced spring.config.import.

https://docs.spring.io/spring-cloud-config/docs/current/reference/html/#config-first-bootstrap

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.

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

Hi @ryanjbaxter, we face kind of similar issue. It is specific to "overrides" returned by config server. The behaviour was always that overrides take precedence. Now with the call to config server with de default profile. The reordering makes the overriding having less precedence than the profile specific files.

I assume the workaround above works for you?

Indeed the "workaround" works for me as well. But I have a few concerns:

  • How longer this bootstrap will be supported ?
  • I am a bit lost reading documentation since the spring config import is now the default implementation so how is it possible that a key feature of config server (overrides) does not do what it is supposed to do.
  • I can't help thinking that there is something wrong either on implementation or in documentation

Please open a separate issue for this since it is different from the original issue and I can take a look.

How longer this bootstrap will be supported ?

We have no plans to remove it at the moment.