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

Cannot add propertySource with profiles #27000

Closed
nycza opened this issue Jun 21, 2021 · 3 comments
Closed

Cannot add propertySource with profiles #27000

nycza opened this issue Jun 21, 2021 · 3 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply theme: config-data Issues related to the configuration theme

Comments

@nycza
Copy link

nycza commented Jun 21, 2021

Following version 2.5 update (specifically, modification of Environment in #24892), we cannot add propertySource files correctly with an EnvironmentPostProcessor.

Here is what we did previously: in a common library for several applications (mainly of webApplicationType = REACTIVE), an EnvironmentPostProcessor was used to:

  • Add a yaml files with multiple profiles to define some common properties. These common properties sometimes refer to environment variables defined in real environments (eg. api.port in the example). This file also defines some profiles groups. Here is a shortened example:
server:
  port: ${api.port:80}
  max-http-header-size: ${project.max-http-header-size:16KB}
  shutdown: ${project.shutdown:graceful}
spring:
  profiles:
    group:
      full_debug:
      - http_trace
      - messaging_trace
---
spring:
  config:
    activate:
      on-profile: http_trace
okhttp:
  log:
    enabled: true
    content: BODY
    level: debug
  • To enable easier work locally, whenever some profile is active, another file is added with some default values for the properties matching environment variables. Here is another shortened example file for this case.
api:
  port: ${env.api.port:8080}

The class adding this behaviour looks something like this:

class DxpPropertiesEnvironmentPostProcessor : EnvironmentPostProcessor, Ordered {

    companion object {
        private const val CORE_FILE = "core.yml"
        private const val ENV_DEV_PROPERTY_FILE = "env-dev.yml"
        private const val DEV_PROFILE = "dev"
        private val propertySourceLoader = YamlPropertySourceLoader()
    }

    override fun postProcessEnvironment(
            environment: ConfigurableEnvironment,
            application: SpringApplication,
        environment.addPropertiesFileToResourceResolvers(CORE_FILE)
    }

    private fun ConfigurableEnvironment.addPropertiesFileToResourceResolvers(filename: String) {
        load(filename)
        if (isDev()) {
            load(ENV_DEV_PROPERTY_FILE)
        }
    }

    private fun ConfigurableEnvironment.load(filename: String) {
        propertySourceLoader.load(filename, ClassPathResource(filename)).filterProfiles(activeProfiles)
                .forEach {
                    propertySources.addBefore(RANDOM_PROPERTY_SOURCE_NAME, it)
                }
    }

    private fun ConfigurableEnvironment.isDev(): Boolean {
        return acceptsProfiles { it.test(DEV_PROFILE) }
    }

    override fun getOrder() = Ordered.HIGHEST_PRECEDENCE + 1
}

We now have several issues with the new Environment implementation used (ApplicationReactiveWebEnvironment), as profiles are not resolved before ConfigDataEnvironmentPostProcessor anymore:

  • The if(isDev()) clause cannot resolve correctly as profiles are defined after our class
  • The filtering of profiles in the "core" yaml file cannot work correctly, only the "default" part being added

If we move the post processor after ConfigDataEnvironmentPostProcessor, properties group defined via spring.profiles.group in the file were not present yet, and the associated profiles aren't added by ConfigDataEnvironmentPostProcessor.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 21, 2021
@nycza
Copy link
Author

nycza commented Jun 21, 2021

Note: we're not using Spring Cloud Config

Note 2: Creating multiple EnvironmentPostProcessors, splitting the files (one with only spring.profiles.include/spring.profiles.group properties, another with the rest of the content), and applying EnvironmentPostProcessors for profiles only files before ConfigDataEnvironmentPostProcessor / the other EnvironmentPostProcessors after ConfigDataEnvironmentPostProcessor seems to work, but I'm not sure that's the cleanest way to go about it.

@philwebb
Copy link
Member

I think splitting your EnvironmentPostProcessor is probably the quickest way to get your application working again. I can't think of how we could change things on our end to make things work as they did before without re-introducing a lot of the complications that we were trying to remove.

We need the profile groups to be setup before profiles are activated, and you can only check the dev profile after they have been activated.

The only other suggestion I have is to look at the new ConfigDataLocationResolver and ConfigDataLoader classes. They might allow you to contribute your own properties in a more natural way. The only downside is that users of your library would need to remember to add a spring.config.import entry to their application.yaml files:

application.yaml

spring:
  config:
    import: dxp:common

You might also want to track issue #24688 where we're trying to find a better general solution to this problem.

@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 21, 2021
@nycza
Copy link
Author

nycza commented Jun 21, 2021

Thanks for your quick answer, I'll keep #24688 in mind when upgrading in the future it indeed looks like it fits our usecase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply theme: config-data Issues related to the configuration theme
Projects
None yet
Development

No branches or pull requests

3 participants