-
Notifications
You must be signed in to change notification settings - Fork 41.7k
Support a generalized way to configure Liquibase global properties through Spring Boot configuration #47289
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
base: main
Are you sure you want to change the base?
Conversation
2ab53f3 to
19761a2
Compare
wilkinsona
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, @dmiska25. I've left some comments for your consideration.
...c/main/java/org/springframework/boot/liquibase/autoconfigure/LiquibaseAutoConfiguration.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/springframework/boot/liquibase/autoconfigure/LiquibaseAutoConfiguration.java
Outdated
Show resolved
Hide resolved
| // Remove any previously registered instance of our provider class | ||
| liquibaseConfiguration.getProviders() | ||
| .stream() | ||
| .filter((provider) -> provider.getClass() == EnvironmentConfigurationValueProvider.class) | ||
| .toList() | ||
| .forEach(liquibaseConfiguration::unregisterProvider); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully this won't be necessary when using a @Bean method. If it is necessary, one scenario where the @Bean method may be called multiple times is in tests with slightly different configuration that result in the creation of multiple contexts. Removing one provider in favour of a new one may cause problems there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this mostly for tests. This is needed because the Liquibase configuration providers are static and when the LiquibaseAutoConfigurationTests run, they destroy Spring context in between tests, leaving the register with a stale configuration variable. I could handle this inside each test, but I think it might be wise to keep in source for downstream consumers too. I don't think this would be an issue in production runs, but I'll lean on your experience for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could avoid the problem by sub-classing SpringLiquibase and overriding afterPropertiesSet to create a child scope before calling super. This child scope could hold the EnvironmentConfigurationValueProvider and, hopefully, remove any problems of multiple contexts interfering with each other. Our existing subclass (DataSourceClosingSpringLiquibase) would become a sub-class of the proposed new sub-class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I understand correctly, your suggestion is to execute the Spring Liquibase logic within a Liquibase child scope with the new provider defined? If so, I created this subclass like this. But, in testing, what I found out is that Liquibase still registers its providers globally, even if the provider was registered to a child scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so I think one way it could be done is if the provider was defined by the scope(scope id passed in during construction). The provider can then request the current scope id at any point and then just return false for a lookup if its not the appropriate scope it was created with. My only thought is whether it's necessary to have this level of separation? Is it realistic that two Spring Liquibases would ever run at the same time? If it is, I could implement this, but if it's not, I'd lean to simplicity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having two run in parallel is unlikely, but having them run serially with potentially different configuration is definitely possible both in tests and in apps with a context hierarchy. There's also the Actuator endpoint to consider where we use StandardChangeLogHistoryService at that can be called at any time, not just during application context refresh. I haven't examined the code path of the history service to know if it references configuration values that would be affected by configuration value providers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, finally circling back to this. I updated as you suggested. I verified that there are a few Liquibase properties(I added a test for one) that do get referenced in the endpoint, so I wrapped that as well. I verified that at least all the tests in the module are referencing the correct env through logging. It is possible for users to still do things like execute Liquibase themselves, but that's the only edge case I can think of, otherwise, that would not be wrapped currently. I added a few other comments for some other thoughts I had, like logging and fallbacks as well, that I'd love your input on.
...springframework/boot/liquibase/autoconfigure/EnvironmentConfigurationValueProviderTests.java
Show resolved
Hide resolved
.../org/springframework/boot/liquibase/autoconfigure/EnvironmentConfigurationValueProvider.java
Outdated
Show resolved
Hide resolved
19761a2 to
8ba8d5d
Compare
…-through Signed-off-by: Dylan Miska <[email protected]>
…on and align code standards to match Spring Boot convention. Signed-off-by: Dylan Miska <[email protected]>
…pass-through Signed-off-by: Dylan Miska <[email protected]>
8ba8d5d to
8664d53
Compare
| return null; | ||
| } | ||
|
|
||
| String environmentId = liquibase.Scope.getCurrentScope().get(SPRING_ENV_ID_KEY, String.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add additional strategies here, such as inferring the env if there's only one, for example. I opted for the simplest solution for now, but would love input on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not aware of how scope works but can't we just put the Environment and not have a local cache here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that’s actually what I did originally, but @wilkinsona raised a concern around how this behaves with child Spring Boot contexts. From what I understand, the way Liquibase handles provider registration makes this tricky. Liquibase’s ConfigurationValueProvider registry is global and static, keyed only by class name, while Spring Boot’s configuration model is context-scoped.
Each Spring context can define its own properties and should override those from its parent. For example:
Parent context
spring:
liquibase:
properties:
liquibase.duplicateFileMode: 'WARN'Child context
spring:
liquibase:
properties:
liquibase.duplicateFileMode: 'ERROR'The expected Boot behavior is that the child context overrides the parent, and the parent keeps its own configuration. But because Liquibase maintains a single global provider list, all providers share the same static state. That’s the core mismatch.
Originally, I just replaced any previously registered provider when a new context started. That works fine for most cases, because Spring Boot typically initializes contexts sequentially and Liquibase itself only runs during startup. This is also the simplest and most test-friendly approach.
The edge case appears when Liquibase is accessed after startup-for example, via the Actuator Liquibase endpoint or any custom Liquibase invocation. In those cases, the provider would return values from the most recently initialized Spring context, even if the call originated from the parent context. That’s where correctness can drift.
To get ahead of your specific question: storing the Environment directly (as you suggested) works only when there’s a single context. With multiple contexts, there’s no way for Liquibase to indicate which Spring context a configuration lookup is coming from. Liquibase doesn’t expose that information. The only feasible mechanism is to pass some form of context identity through Liquibase’s Scope API, which is why the current PR uses that indirection.
And that’s really the broader issue: Liquibase assumes configuration providers are global and static, while Boot assumes configuration is hierarchical and scoped. Because those models don’t naturally align, we end up with tradeoffs.
So from what I can tell, there are a few realistic paths:
- Don’t support overriding in child contexts, and document that limitation clearly.
- Partially support it (current PR behavior): initialization + the Actuator endpoint work correctly, but any ad-hoc Liquibase usage might not resolve the right context. Practically speaking, those scenarios seem very rare.
- Decide the complexity isn’t worth it and close the PR.
I’m open to whichever direction best fits Boot’s conventions. My main goal here is just to make the tradeoffs visible so we can decide on the right level of support.
|
|
||
| String environmentId = liquibase.Scope.getCurrentScope().get(SPRING_ENV_ID_KEY, String.class); | ||
| if (environmentId == null) { | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered adding logging here if we get a request for a config from this provider but we can't infer the env but I didn't see any other logging in the module, so I skipped for now.
snicoll
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. I've added some comments for your consideration.
Also, please run the build as there are checkstyle rules failing atm. For a quick check use ./gradlew :module:spring-boot-liquibase:check.
| return null; | ||
| } | ||
|
|
||
| String environmentId = liquibase.Scope.getCurrentScope().get(SPRING_ENV_ID_KEY, String.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not aware of how scope works but can't we just put the Environment and not have a local cache here?
| */ | ||
| class EnvironmentAwareSpringLiquibase extends SpringLiquibase implements ApplicationContextAware, DisposableBean { | ||
|
|
||
| private @Nullable String environmentId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not supposed to be nullable. I know that ApplicationContext#getId can return null but that's an oversight, see spring-projects/spring-framework#35925
| scopeValues.put(EnvironmentConfigurationValueProvider.SPRING_ENV_ID_KEY, this.environmentId); | ||
| Scope.child(scopeValues, EnvironmentAwareSpringLiquibase.super::afterPropertiesSet); | ||
| } | ||
| catch (Exception ex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have two catch blocks here
| @Override | ||
| public void destroy() { | ||
| if (this.environmentId != null) { | ||
| EnvironmentConfigurationValueProvider.unregisterEnvironment(this.environmentId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really would like that the responsability of this was in Liquibase's scope, and not here.
| } | ||
|
|
||
| @Override | ||
| public void destroy() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep the original signature. Even if it's not used by the method's body, it's a breaking change.
Summary
This PR introduces support for configuring Liquibase’s global properties directly through Spring Boot’s configuration system.
A new map property has been added:
At startup, these entries are passed through to Liquibase’s
ConfigurationValueProvider. This aligns with how Spring Boot already supports pass-through properties for JPA (spring.jpa.properties.*).Motivation
Liquibase supports an expanding set of global configuration options (e.g.,
duplicateFileMode,logLevel,searchPath). Until now, these could only be set using JVM system properties or environment variables if they were not specified in the Spring Boot configuration.That approach created fragmentation: most Liquibase settings were configured under
spring.liquibase.*, while global properties had to be managed separately. This change unifies configuration, allowing Spring users to manage all Liquibase options from one place.My motivating use case: we had multiple Liquibase changelog files and frequently saw an error indicating duplicates. The recommended fix was to set
liquibase.duplicateFileMode=WARN. The workaround was to set it in Gradle system properties, but that made testing and environment alignment harder.Design
EnvironmentConfigurationValueProviderbridges Spring’sEnvironmentto Liquibase’s configuration system.spring.liquibase.properties.*are read exactly as-is (no relaxed binding or rewriting).Precedence Choice
The provider is registered with a precedence of 100, above defaults but below environment variables.
--)Reference: Liquibase parameters documentation.
Testing
application.ymlandapplication.properties.Documentation
spring.liquibase.propertiesto clarify usage, with an example key.Implementation Notes
Fixes #47212