Binder provided by ConfigDataLocationResolver does not support profiles on classpath

[MatejNedic]

When implementing custom spring.config.import solution and implementing ConfigDataLocationResolver, Binder retrieved from ConfigDataLocationResolverContext.getBinder method won't respect profiles resources loaded from classpath.

Debug context:
When ConfigDataLocationResolver.resolveProfileSpecific is called it passes instance of ConfigDataLocationResolverContext which has getBinder method. When getBinder() is called it will execute the following.

	@Override
		public Binder getBinder() {
			Binder binder = this.binder;
			if (binder == null) {
				binder = this.contributors.getBinder(this.activationContext);
				this.binder = binder;
			}
			return binder;
			

Returned binder instance will bind prefix to a given class, but it will not look for profile specific properties, only default ones are taken into account. Meaning only application.properties is taken into account.

So if user has active dev profile and application-dev.properties with a following value.

spring.cloud.aws.parameterstore.endpoint=http://localhost:4566

Endpoint will not be mapped properly to given class, to be specific it won't bind it to an endpoint field since it is not taking dev profile file into an account.

Classpath and /config ConfigDataLocationResolver are taken into account before any other custom ConfigDataLocationResolver implementation. This means that StandardConfigDataLocationResolver will be called first before anything else.

Currently /config properties are loaded first than my custom ConfigDataLocationResolver implementation. This implementation gets binder which can look only at default application.properties. After this StandardConfigDataLocationResolver is called again with classpath resource lookup. Which means Binder does not have access to resources/application-dev.yaml.

One way to go around this is to provide all values which directly affect ConfigDataLocationResolver implementation in a single file.
I can still have application-dev.properties loaded for different properties that don't impact the ConfigDataLocationResolver implementations.

spring.config.import=aws-parameterstore:/config/

!###

spring.cloud.aws.parameterstore.endpoint=http://localhost:4566
spring.config.activate.on-profile=dev

Everything works tho if file application-dev.properties is located under /config since it is loaded first.

I am willing to do contribution.

Docs I checked.

[philwebb]

This is actually by design as we only want that binder to provide access to values that have been contributed so far. We don't want to allow ConfigDataLocationResolver implementations to use profile specific properties. The reason for this is that the resolved ConfigData may themselves contribute spring.profile... properties.

[MatejNedic]

Hey @philwebb , Thanks on quick response and explanation. I was expecting this behaviour however I found it different that is why I raised the question. Reading now I have not expressed myself best so I will try to do so now.

ConfigDataLocationResolvers when loading resolvers one by one will firstly call StandardConfigDataLocationResolver.resolveProfileSpecific with location
optional:file:./;optional:file:./config/;optional:file:./config/*/. This will load profile files as well. After that my custom resolver is called it does its job. Following after that StandardConfigDataLocationResolver is again called with location optional:classpath:/;optional:classpath:/config/. This means that classpath:/resources are taken into account after file:/ and other resolvers.

If I included application-dev.yaml under project structure in same level as src (to be specific path /) it will load profile specific properties before my resolver is taken into account and binder will have access to them. However if i place application-dev.yaml under /resources it will load it after my resolver was triggered making it not taken into account.

I was expecting that classpath: and file: are firstly loaded, otherwise there is difference running locally vs running in container. I have always used dev profile and application-dev.yaml under resources for running spring boot locally. Currently I can see a difference in running locally/tests and running container since path of application.files are different, meaning loading and contributing ConfigData happens in different order.

I can provide sample application of the issue if it would explain it better.

Edit: I am running my app with env variable -> spring_profiles_active=dev

[philwebb]

Could you provide the sample application. I think it would help my understanding of the issue.

Thanks!

[MatejNedic]

Ofcourse, here you go. If you need anything more please let me know.

https://github.com/MatejNedic/Spring-boot-issue-43514

Please run with env variable spring_profiles_active=dev .

After that uncomment values application-dev.properties located under /config folder and try again.

You won't see logs since enabled will be picked up by binder.

[philwebb]

Thanks for the sample, it has helped a lot!

I still think this is working as designed and the problem might be that io.awspring.cloud.autoconfigure.config.parameterstore.ParameterStoreConfigDataLocationResolver should be using the resolve method and not resolveProfileSpecific. The resolveProfileSpecific method is really designed for the case where the profile name is considered when finding the location to load. In the ParameterStoreConfigDataLocationResolver case, the profiles parameter isn't even used.

The config data loading code is quite complicated. It attempts to build a tree of ConfigDataEnvironmentContributor elements. The initial set of imports used are intentionally split into two so property precedence of imports works. The optional:classpath:/;optional:classpath:/config/ happens first for classpath resource, then optional:file:./;optional:file:./config/;optional:file:./config/*/ happens for file resources.

If you imagine a tree structure, the initial two nodes are as follows:

- classpath config
- file config

As files are loaded, imports add nodes to the tree. So for example, if in classpath:application.properties you have spring.config.import=foo.properties the tree would look like this after that file is loaded:

- classpath config
  +- foo.properties
- file config

If in file:/config/application.properties you have spring.config.import=bar.properties then after that file is loaded it looks like this:

- classpath config
  +- foo.properties
- file config
  +- bar.properties

The important thing is that the tree represents the order that property sources are added and hence overriding happens correctly. A value in bar.properties, overrides the file config, foo.properties and classpath config. Some documentation exists on this at https://docs.spring.io/spring-boot/reference/features/external-config.html#features.external-config.files.importing

In your case, what is happening is you have a spring.config.import=optional:aws-parameterstore:/config/ value in your classpath:application.properties. So the first pass of the tree looks like this:

- classpath config
  +- aws store
- file config

Once we've done the first pass, we have profile properties and we enable the dev profile and do a second sweep. In the original version of your app we get:

- classpath config
  +- aws store
  +- classpath:application-dev.properties
- file config

I think we end up adding that application-dev.properties before calling your ConfigDataLocationResolver so you can read spring.cloud.aws.parameterstore.enabled. In the modified version of the app, we call your resolver before the standard one has even added its not to the tree.

I'm not sure what the best way to fix this is, but I think I'd recommend moving the spring.config.import=optional:aws-parameterstore:/config/ into the profile specific file rather than binding a property.

I'm also wondering if we should try and be stricter in Spring Boot and provide the same binder to resolveProfileSpecific as we do to resolve. That wouldn't fix your issue, but it would make the behavior more consistent.

[philwebb]

I've opened #43533 to see if we can pass in the same binder.

[MatejNedic]

Thanks @philwebb so much on extensive explanation!!!

Really appreciate it!

Makes sense now since our resolve method by default returned empty list. It explains why it ignores it perfectly well.

Basically bug on our side, will have to implement resolve(), since it does not align currently with spring boot!

Are you accepting PRs for #43533 ? I know it might be complex but I am willing to try it out.

[philwebb]

Thanks for the offer of a PR, but I think I might need to take it on since it's such a complicated bit of code. We also can't merge anything until we create the 3.4.x branch.