spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.33k stars 40.72k forks source link

Propose new spring.autoconfigure.exclusion property binding as Map to support multiple property sources #41669

Open tmoschou opened 3 months ago

tmoschou commented 3 months ago

Currently the spring.autoconfigure.exclude property is bound as a List<Class<?>>. This is a problem as collections cannot be merged across different property sources or individual elements removed. Additionally it requires property sources with higher precedence to have global knowledge of all preexisting exclusion from other sources in order to append a new exclusion.

This is a frequent pain-point for us and others. See #27414 - Consider merging spring.autoconfigure.exclude from multiple profiles and https://github.com/spring-projects/spring-boot/issues/9137

Understandably merging / overriding logic for collections is difficult - refer to https://github.com/spring-projects/spring-boot/issues/12444#issuecomment-372410949

I propose a new configuration property (E.g. spring.autoconfigure.exclusions or perhaps some other property if too similarly named) that is bound as a Map<Class<?>, Boolean>. Binding to a map will allow merging from different property sources and profiles. E.g.

---
spring:
  config.activate.on-profile: nosecurity
  autoconfigure:
    exclusions: 
      org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration: true
      org.springframework.boot.actuate.autoconfigure.security.servlet.ManagementWebSecurityAutoConfiguration: true

---
spring:
  config.activate.on-profile: noredis
  autoconfigure:
    exclusions: 
      org.springframework.boot.autoconfigure.data.redis.RedisAutoConfiguration: true

Including the class name in the key is not too different from the logging.level.<class> properties and similar to management.health.<name>.enabled.

I have a custom EnvironmentPostProcessor that I think achieves my goal by rebinding the spring.autoconfigure.exclude property from my custom spring.autoconfigure.exclusions property? But it would be nice if spring-boot had first class support for this. For posterity here it is

@Order(Ordered.LOWEST_PRECEDENCE)
public class AutoConfigureExcludeEnvironmentPostProcessor implements EnvironmentPostProcessor {

    private static final String SPRING_AUTOCONFIGURE_EXCLUDE = "spring.autoconfigure.exclude";
    private static final String SPRING_AUTOCONFIGURE_EXCLUDEMAP = "spring.autoconfigure.exclusion";

    @Override
    public void postProcessEnvironment(ConfigurableEnvironment environment, SpringApplication application) {
        Binder binder = Binder.get(environment);
        Set<String> disabled = new HashSet<>();
        binder.bind(SPRING_AUTOCONFIGURE_EXCLUDE, Bindable.listOf(String.class)).ifBound(disabled::addAll);
        Map<String, Boolean> excludeMap =
            binder.bind(SPRING_AUTOCONFIGURE_EXCLUDEMAP, Bindable.mapOf(String.class, Boolean.class))
                .orElseGet(Collections::emptyMap);

        for (Map.Entry<String, Boolean> entry : excludeMap.entrySet()) {
            if (entry.getValue()) {
                disabled.add(entry.getKey());
            } else {
                // override if class is present in 'spring.autoconfigure.exclude' property
                disabled.remove(entry.getKey());
            }
        }

        if (!disabled.isEmpty() || environment.containsProperty(SPRING_AUTOCONFIGURE_EXCLUDE)) {
            environment.getPropertySources().addFirst(new MapPropertySource(
                AutoConfigureExcludeEnvironmentPostProcessor.class.getSimpleName(),
                Map.of(SPRING_AUTOCONFIGURE_EXCLUDE, disabled)
            ));
        }
    }
}
quaff commented 3 months ago

It's a good idea, It would be great if Spring Boot introduce the new spring.autoconfigure.exclusion and deprecate the painful spring.autoconfigure.exclude.

philwebb commented 3 months ago

We'd like to investigate #41830 to see if we can offer a general purpose solution for merging lists. I'll put this one on hold until we've done that.

quaff commented 3 months ago

We'd like to investigate #41830 to see if we can offer a general purpose solution for merging lists. I'll put this one on hold until we've done that.

This proposal follows the way of Spring Boot, could you elaborate the disadvantage of this proposal? I'm not against #41830, but I think this proposal is better for autoconfigure exclusions, that one introduce new magic makes thing more complex and looks strange.

philwebb commented 3 months ago

I like the new proposal as well but I think it's prudent to investigate if a general purpose solution is viable before we offer too many ways of doing the same thing.

One concern I have with the new proposal is the confusion that might arise by having two very similar properties that work in slightly different ways. We'd need spring.autoconfigure.exclude for the List, and spring.autoconfigure.excludes for the Map. That's a very subtle distinction and one that's likely to cause some confusion I think. Perhaps we can come up with a better property name, or deprecate spring.autoconfigure.exclude. The problem with deprecation is that we then force folks that don't need to merge exclusions into using a more verbose syntax.

quaff commented 3 months ago

Perhaps we can come up with a better property name.

I'm vote for spring.autoconfigure.exclusions.

deprecate spring.autoconfigure.exclude.

I suggest deprecating it in next minor release and remove it next major release with a dedicated FailureAnalyzer to guide folks to migrate.

tmoschou commented 3 months ago

Thanks for the update!

That's a very subtle distinction and one that's likely to cause some confusion I think

I agree could be confusing, I think I initially had spring.autoconfigure.exclusions-map when prototyping with my EnvironmentPostProcessor and added resources/META-INF/additional-spring-configuration-metadata.json with

{
  "properties": [
    {
      "name": "spring.autoconfigure.exclusions-map",
      "type": "java.util.Map<java.lang.Class, java.lang.Boolean>",
      "description": "Auto-configuration classes to exclude."
    }
  ]
}

for type hints, which give IDE assistance (IntelliJ)

Screenshot 2024-08-22 at 5 13 16 PM Screenshot 2024-08-22 at 5 12 28 PM

I did consider whether should deprecate too, I don't have any overly strong opinions either way should the new property be ultimately agreed too. However if the old is not deprecated, then both should be supported simultaneously (not mutually exclusive) IMO, as my env post processor does.

Also not against, https://github.com/spring-projects/spring-boot/issues/41830, but I am interested how this might work with collections of complex types that themselves might have nested collections - I'll repost that last question on that ticket too.