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.18k stars 40.68k forks source link

SystemProperties ignored in spring-boot-maven-plugin if -Dspring.context.exit=onRefresh is specified #41078

Closed patbaumgartner closed 4 months ago

patbaumgartner commented 5 months ago

When using properties through CDS_TRAINING_JAVA_TOOL_OPTIONS in spring-boot-maven-plugin (pom.xml), but also when providing mvn spring-boot:run -Dspring-boot.run.arguments="-Dspring.profiles.active=debug -Dspring.context.exit=onRefresh" on the command line, the properties are ignored.

When running the fatJAR directly via java -Dspring.profiles.active=debug -Dspring.context.exit=onRefresh -jar target/*.jar the properties are taken into account.

So far, It looks like an issue in the spring-boot-maven-plugin but looks good from the spring-boot buildpack perspective.

scottfrederick commented 5 months ago

@patbaumgartner It's not clear to me what you are trying to do. The CDS_TRAINING_JAVA_TOOL_OPTIONS environment variable can be configured on the spring-boot:build-image goal and passed to the Paketo Spring Boot buildpack. It should have no effect on the spring-boot:run goal, so I don't see how it could interfere with setting spring-boot.run.arguments on spring-boot:run.

Can you provide a more complete example of your pom.xml, the mvn commands you are running, and the behavior you expect?

patbaumgartner commented 5 months ago

Hi @scottfrederick

my main goal was to use App CDS with the spring-boot-maven plugin as it is described in the Buildpacks documentation. I realized that the parameter CDS_TRAINING_JAVA_TOOL_OPTIONS gets passed as JAVA_TOOL_OPTIONS successfully and was ignored. So I tried to find out where the issue might be.

            <plugin>
                <groupId>org.springframework.boot</groupId>
                <artifactId>spring-boot-maven-plugin</artifactId>
                <executions>
                    <execution>
                        <id>process-aot</id>
                        <goals>
                            <goal>process-aot</goal>
                        </goals>
                    </execution>
                </executions>
                <configuration>
                    <image>
                        <env>
                            <BP_SPRING_AOT_ENABLED>true</BP_SPRING_AOT_ENABLED>
                            <BP_JVM_CDS_ENABLED>true</BP_JVM_CDS_ENABLED>
                            <CDS_TRAINING_JAVA_TOOL_OPTIONS>-Dtelegrambots.enabled=false</CDS_TRAINING_JAVA_TOOL_OPTIONS>
                        </env>
                    </image>
                </configuration>
            </plugin>

Then I tried to run the app with java -Dtelegrambots.enabled=true -jar my.jar and it worked. Next step was the maven plugin which I got wrong above. It should have been mvn spring-boot:run -Dspring-boot.run.arguments="-Dreproducer.enabled=true" which passes the args to the app.

After that I realized that when adding -Dspring.context.exit=onRefresh my enabling property gets ignored. It doesn't matter if I do it via maven, buildpacks or java -jar.

I remember that I saw somewhere @sdeleuze used spring.flyway.enabled=false of FlywayAutoConfiguration for a training run but since the property gets ignored, it would not work as expected, which means to not creating a DB connection and migrating the data.

Since this is a pretty new feature, there is not much documentation and answers on SO. I hope you could follow my thoughts.

Best, Patrick

mhalbritter commented 5 months ago

spring-boot.run.arguments are for arguments to the main class. I think jvmArguments is what you're looking for?

sdeleuze commented 5 months ago

FYI I did a quick test on aarch64 with the buildpacks branch of petclinic-efficient-container. If I remove spring.data.jdbc.dialect=postgresql from application.properties, the application training run fails as expected because it tries to connect to the database when building the container.

If I bring it back in the Spring Boot Maven configuration, it works again:

                <configuration>
                    <image>
                        <env>
                            <BP_SPRING_AOT_ENABLED>true</BP_SPRING_AOT_ENABLED>
                            <BP_JVM_CDS_ENABLED>true</BP_JVM_CDS_ENABLED>
                            <CDS_TRAINING_JAVA_TOOL_OPTIONS>-Dspring.data.jdbc.dialect=postgresql</CDS_TRAINING_JAVA_TOOL_OPTIONS>
                        </env>
                    </image>
                </configuration>

I did another test on x86 without the Spring Framework version customization, builder and image customization, it works as well. So the feature seems to work as expected with Buildacks. For Spring Boot run, the configuration is done differently as pointed out by @scottfrederick and @mhalbritter.

spring-projects-issues commented 4 months ago

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.

spring-projects-issues commented 4 months ago

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.

patbaumgartner commented 4 months ago

@mhalbritter @sdeleuze Thank you very much for your help.

It's interesting to see that when disabling the starter in the contextLoads test with

@SpringBootTest(properties = { "lovebox.enabled=false", "telegrambots.enabled=false" }) 

The test is successful because it recognizes the properties.

Assuming that I could do the same in the CDS_TRAINING_JAVA_TOOL_OPTIONS to get the same results

                            <CDS_TRAINING_JAVA_TOOL_OPTIONS>-Dlovebox.enabled=false
                                -Dtelegrambots.enabled=false</CDS_TRAINING_JAVA_TOOL_OPTIONS>

The training run fails, and therefore, the build too.

I think that @ConfigurationProperties is considered, but @ConditionalOnProperty is ignored.

To showcase my research, I created a reproducer project here: https://github.com/patbaumgartner/spring-boot-appcds-training-run-reproducer

mhalbritter commented 4 months ago

Your problem appears because you're using AOT and CDS. For CDS, you specified -Dexception.throwing.enabled=false, but for AOT you didn't. @ConditionalOnProperty is evaluated when AOT is happening, and there exception.throwing.enabled hasn't been set, to the condition matches (because matchIfMissing = true), and the bean is registered in the AOT code.

When you now run your CDS training run, you set exception.throwing.enabled=false but the bean is created nonetheless because of AOT (remember: changing the bean arrangement is not possible after the AOT phase).

The fix for your problem:

either disable AOT with

<BP_SPRING_AOT_ENABLED>false</BP_SPRING_AOT_ENABLED>

or pass -Dexception.throwing.enabled=false to the AOT phase, too. This can be done with something like this:

<executions>
  <execution>
    <id>process-aot</id>
    <goals>
      <goal>process-aot</goal>
    </goals>
    <configuration>
      <systemPropertyVariables>
        <exception.throwing.enabled>false</exception.throwing.enabled>
      </systemPropertyVariables>
    </configuration>
  </execution>
</executions>