From d2369b9852c53c4970ee91395d3ac9619ef54742 Mon Sep 17 00:00:00 2001 From: Anthony Dahanne Date: Mon, 15 Jul 2024 22:43:01 -0400 Subject: [PATCH] Fail the build if CDS_TRAINING_JAVA_TOOL_OPTIONS is set with BP_SPRING_AOT_ENABLED=true * those 2 options are not compatible, see https://github.com/spring-projects/spring-boot/issues/41348 --- README.md | 3 ++ boot/build.go | 16 +++++- boot/build_test.go | 87 +++++++++++++++++++++++++++++++++ boot/spring_performance.go | 47 +++++++++--------- boot/spring_performance_test.go | 49 +++---------------- 5 files changed, 136 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 39c0ba2..bac8ef6 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,9 @@ The buildpack will do the following: * If the application is AOT instrumented (presence of `META-INF/native-image` folder) AND `BP_SPRING_AOT_ENABLED` is set to `true` * set `BPL_SPRING_AOT_ENABLED` to true * add `-Dspring.aot.enabled=true` to `JAVA_TOOL_OPTIONS` at runtime + * If the application is AOT instrumented (presence of `META-INF/native-image` folder) AND `BP_SPRING_AOT_ENABLED` is set to `true` AND `CDS_TRAINING_JAVA_TOOL_OPTIONS` is set + * fail the build with "build failed because of invalid user configuration" - the reason being is that the AOT classes used during training run won't be compatible with a different set of `JAVA_TOOL_OPTIONS` at runtime + * the Spring team explains this issue in detail here: https://github.com/spring-projects/spring-boot/issues/41348 * If `/META-INF/MANIFEST.MF` contains a `Spring-Boot-Native-Processed` entry OR if `$BP_MAVEN_ACTIVE_PROFILES` contains the `native` profile: * A build plan entry is provided, `native-image-application`, which can be required by the `native-image` [buildpack](https://github.com/paketo-buildpacks/native-image) to automatically trigger a native image build * When contributing to a native image application: diff --git a/boot/build.go b/boot/build.go index 5674f73..b7fc664 100644 --- a/boot/build.go +++ b/boot/build.go @@ -21,6 +21,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/heroku/color" "github.com/paketo-buildpacks/libpak/crush" "io/fs" "os" @@ -211,10 +212,23 @@ func (b Build) Build(context libcnb.BuildContext) (libcnb.BuildResult, error) { b.Logger.Bodyf("unable to find AOT processed dir %s, however BP_SPRING_AOT_ENABLED has been set to true. Ensure that your app is AOT processed", dir) } + cdsTrainingJavaToolOptions := sherpa.GetEnvWithDefault("CDS_TRAINING_JAVA_TOOL_OPTIONS", "") if trainingRun || aotEnabled { helpers = append(helpers, "performance") + cdsTrainingJavaToolOptionsProvided := cdsTrainingJavaToolOptions != "" + if cdsTrainingJavaToolOptionsProvided && trainingRun && aotEnabled { + b.Logger.Infof(color.RedString("ERROR: CDS_TRAINING_JAVA_TOOL_OPTIONS is not compatible with BP_SPRING_AOT_ENABLED - as the AOT classes used during training run won't be compatible with a different set of JAVA_TOOL_OPTIONS at runtime \n" + + "The Spring team explains this issue in detail here: https://github.com/spring-projects/spring-boot/issues/41348 \n" + + "If you need to provide CDS_TRAINING_JAVA_TOOL_OPTIONS (to disable a connection to a remote service for example), you need to disable BP_SPRING_AOT_ENABLED ")) + return libcnb.BuildResult{}, fmt.Errorf("build failed because of invalid user configuration") + } + if !cdsTrainingJavaToolOptionsProvided { + // in case CDS_TRAINING_JAVA_TOOL_OPTIONS was not set, we pick up JAVA_TOOL_OPTIONS by default + cdsTrainingJavaToolOptions = sherpa.GetEnvWithDefault("JAVA_TOOL_OPTIONS", "") + } + if trainingRun { mainClass, _ = manifest.Get("Start-Class") classpathString = "runner.jar" @@ -227,7 +241,7 @@ func (b Build) Build(context libcnb.BuildContext) (libcnb.BuildResult, error) { } } - cdsLayer := NewSpringPerformance(dc, context.Application.Path, manifest, aotEnabled, trainingRun, classpathString, reZipExplodedJar) + cdsLayer := NewSpringPerformance(dc, context.Application.Path, manifest, aotEnabled, trainingRun, classpathString, reZipExplodedJar, cdsTrainingJavaToolOptions) cdsLayer.Logger = b.Logger result.Layers = append(result.Layers, cdsLayer) diff --git a/boot/build_test.go b/boot/build_test.go index bd75207..48918ef 100644 --- a/boot/build_test.go +++ b/boot/build_test.go @@ -669,6 +669,8 @@ Spring-Boot-Lib: BOOT-INF/lib it.After(func() { os.Unsetenv("BP_JVM_CDS_ENABLED") os.Unsetenv("BP_SPRING_CLOUD_BINDINGS_DISABLED") + os.Unsetenv("BP_SPRING_AOT_ENABLED") + os.Unsetenv("CDS_TRAINING_JAVA_TOOL_OPTIONS") }) ctx.Buildpack.API = "0.6" @@ -716,6 +718,91 @@ Spring-Boot-Lib: BOOT-INF/lib Expect(result.Layers[0].Name()).To(Equal("web-application-type")) }) + it("contributes CDS layer & helper for Boot 3.3+ apps with BP_SPRING_AOT_ENABLED and CDS_TRAINING_JAVA_TOOL_OPTIONS not set", func() { + t.Setenv("BP_SPRING_AOT_ENABLED", "true") + Expect(os.WriteFile(filepath.Join(ctx.Application.Path, "META-INF", "MANIFEST.MF"), []byte(` + Spring-Boot-Version: 3.3.1 + Start-Class: test-class + Spring-Boot-Classes: BOOT-INF/classes + Spring-Boot-Lib: BOOT-INF/lib + `), 0644)).To(Succeed()) + + result, err := build.Build(ctx) + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Layers).To(HaveLen(3)) + Expect(result.Layers[2].Name()).To(Equal("helper")) + Expect(result.Layers[2].(libpak.HelperLayerContributor).Names).To(Equal([]string{"performance"})) + }) + + it("fails the build because CDS_TRAINING_JAVA_TOOL_OPTIONS was provided with BP_SPRING_AOT_ENABLED", func() { + t.Setenv("BP_SPRING_AOT_ENABLED", "true") + t.Setenv("CDS_TRAINING_JAVA_TOOL_OPTIONS", "user-cds-opt") + + Expect(os.WriteFile(filepath.Join(ctx.Application.Path, "META-INF", "MANIFEST.MF"), []byte(` + Spring-Boot-Version: 3.3.1 + Start-Class: test-class + Spring-Boot-Classes: BOOT-INF/classes + Spring-Boot-Lib: BOOT-INF/lib + `), 0644)).To(Succeed()) + + Expect(os.Mkdir(filepath.Join(ctx.Application.Path, "META-INF", "native-image"), 0755)).To(Succeed()) + + _, err := build.Build(ctx) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("build failed because of invalid user configuration")) + }) + + it("contributes CDS layer & helper for Boot 3.3+ apps with CDS_TRAINING_JAVA_TOOL_OPTIONS but BP_SPRING_AOT_ENABLED is disabled", func() { + t.Setenv("BP_SPRING_AOT_ENABLED", "false") + t.Setenv("CDS_TRAINING_JAVA_TOOL_OPTIONS", "user-cds-opt") + + Expect(os.WriteFile(filepath.Join(ctx.Application.Path, "META-INF", "MANIFEST.MF"), []byte(` + Spring-Boot-Version: 3.3.1 + Start-Class: test-class + Spring-Boot-Classes: BOOT-INF/classes + Spring-Boot-Lib: BOOT-INF/lib + `), 0644)).To(Succeed()) + + Expect(os.Mkdir(filepath.Join(ctx.Application.Path, "META-INF", "native-image"), 0755)).To(Succeed()) + + result, err := build.Build(ctx) + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Layers).To(HaveLen(3)) + + springPerformanceLayer := result.Layers[0].(boot.SpringPerformance) + // specific CDS_TRAINING_JAVA_TOOL_OPTIONS was set, so we set it in the training run + Expect(springPerformanceLayer.TrainingRunJavaToolOptions).To(Equal("user-cds-opt")) + + Expect(result.Layers[2].Name()).To(Equal("helper")) + Expect(result.Layers[2].(libpak.HelperLayerContributor).Names).To(Equal([]string{"performance"})) + }) + + it("contributes CDS layer & helper for Boot 3.3+ apps with BP_SPRING_AOT_ENABLED and JAVA_TOOL_OPTIONS set", func() { + t.Setenv("BP_SPRING_AOT_ENABLED", "true") + t.Setenv("CDS_TRAINING_JAVA_TOOL_OPTIONS", "default-opt") + + Expect(os.WriteFile(filepath.Join(ctx.Application.Path, "META-INF", "MANIFEST.MF"), []byte(` + Spring-Boot-Version: 3.3.1 + Start-Class: test-class + Spring-Boot-Classes: BOOT-INF/classes + Spring-Boot-Lib: BOOT-INF/lib + `), 0644)).To(Succeed()) + + result, err := build.Build(ctx) + Expect(err).NotTo(HaveOccurred()) + + Expect(result.Layers).To(HaveLen(3)) + + springPerformanceLayer := result.Layers[0].(boot.SpringPerformance) + // no specific CDS_TRAINING_JAVA_TOOL_OPTIONS was set, but JAVA_TOOL_OPTIONS was, so we set it in the training run as well + Expect(springPerformanceLayer.TrainingRunJavaToolOptions).To(Equal("default-opt")) + + Expect(result.Layers[2].Name()).To(Equal("helper")) + Expect(result.Layers[2].(libpak.HelperLayerContributor).Names).To(Equal([]string{"performance"})) + }) + }) context("when there is a non-exploded jar passed to the buildpack", func() { diff --git a/boot/spring_performance.go b/boot/spring_performance.go index 5cee61b..cd2ec5b 100644 --- a/boot/spring_performance.go +++ b/boot/spring_performance.go @@ -35,32 +35,34 @@ import ( ) type SpringPerformance struct { - Dependency libpak.BuildpackDependency - LayerContributor libpak.LayerContributor - Logger bard.Logger - Executor effect.Executor - AppPath string - Manifest *properties.Properties - AotEnabled bool - DoTrainingRun bool - ClasspathString string - ReZip bool + Dependency libpak.BuildpackDependency + LayerContributor libpak.LayerContributor + Logger bard.Logger + Executor effect.Executor + AppPath string + Manifest *properties.Properties + AotEnabled bool + DoTrainingRun bool + ClasspathString string + ReZip bool + TrainingRunJavaToolOptions string } -func NewSpringPerformance(cache libpak.DependencyCache, appPath string, manifest *properties.Properties, aotEnabled bool, doTrainingRun bool, classpathString string, reZip bool) SpringPerformance { +func NewSpringPerformance(cache libpak.DependencyCache, appPath string, manifest *properties.Properties, aotEnabled bool, doTrainingRun bool, classpathString string, reZip bool, trainingRunJavaToolOptions string) SpringPerformance { contributor := libpak.NewLayerContributor("Performance", cache, libcnb.LayerTypes{ Build: true, Launch: true, }) return SpringPerformance{ - LayerContributor: contributor, - Executor: effect.NewExecutor(), - AppPath: appPath, - Manifest: manifest, - AotEnabled: aotEnabled, - DoTrainingRun: doTrainingRun, - ClasspathString: classpathString, - ReZip: reZip, + LayerContributor: contributor, + Executor: effect.NewExecutor(), + AppPath: appPath, + Manifest: manifest, + AotEnabled: aotEnabled, + DoTrainingRun: doTrainingRun, + TrainingRunJavaToolOptions: trainingRunJavaToolOptions, + ClasspathString: classpathString, + ReZip: reZip, } } @@ -138,10 +140,9 @@ func (s SpringPerformance) Contribute(layer libcnb.Layer) (libcnb.Layer, error) var trainingRunEnvVariables []string - javaToolOptions := sherpa.GetEnvWithDefault("CDS_TRAINING_JAVA_TOOL_OPTIONS", sherpa.GetEnvWithDefault("JAVA_TOOL_OPTIONS", "")) - if javaToolOptions != "" { - s.Logger.Bodyf("Training run will use this value as JAVA_TOOL_OPTIONS: %s", javaToolOptions) - trainingRunEnvVariables = append(trainingRunEnvVariables, fmt.Sprintf("JAVA_TOOL_OPTIONS=%s", javaToolOptions)) + if s.TrainingRunJavaToolOptions != "" { + s.Logger.Bodyf("Training run will use this value as JAVA_TOOL_OPTIONS: %s", s.TrainingRunJavaToolOptions) + trainingRunEnvVariables = append(trainingRunEnvVariables, fmt.Sprintf("JAVA_TOOL_OPTIONS=%s", s.TrainingRunJavaToolOptions)) } // perform the training run, application.dsa, the cache file, will be created diff --git a/boot/spring_performance_test.go b/boot/spring_performance_test.go index d8a3547..776bc62 100644 --- a/boot/spring_performance_test.go +++ b/boot/spring_performance_test.go @@ -79,7 +79,7 @@ Spring-Boot-Lib: BOOT-INF/lib props, err := libjvm.NewManifest(ctx.Application.Path) Expect(err).NotTo(HaveOccurred()) - s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true) + s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true, "") s.Executor = executor layer, err := ctx.Layers.Layer("test-layer") @@ -114,7 +114,7 @@ Spring-Boot-Lib: BOOT-INF/lib props, err := libjvm.NewManifest(ctx.Application.Path) Expect(err).NotTo(HaveOccurred()) - s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true) + s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true, "") s.Executor = executor layer, err := ctx.Layers.Layer("test-layer") @@ -144,7 +144,7 @@ Spring-Boot-Lib: BOOT-INF/lib props, err := libjvm.NewManifest(ctx.Application.Path) Expect(err).NotTo(HaveOccurred()) - s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true) + s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true, "") s.Executor = executor layer, err := ctx.Layers.Layer("test-layer") @@ -169,9 +169,8 @@ Spring-Boot-Lib: BOOT-INF/lib it("contributes user-provided JAVA_TOOL_OPTIONS to training run", func() { Expect(os.Setenv("JAVA_TOOL_OPTIONS", "default-opt")).To(Succeed()) - Expect(os.Setenv("CDS_TRAINING_JAVA_TOOL_OPTIONS", "user-cds-opt")).To(Succeed()) - aotEnabled, cdsEnabled = true, true + aotEnabled, cdsEnabled = false, true dc := libpak.DependencyCache{CachePath: "testdata"} executor.On("Execute", mock.Anything).Return(nil) @@ -183,7 +182,7 @@ Spring-Boot-Lib: BOOT-INF/lib props, err := libjvm.NewManifest(ctx.Application.Path) Expect(err).NotTo(HaveOccurred()) - s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true) + s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true, "user-cds-opt") s.Executor = executor layer, err := ctx.Layers.Layer("test-layer") @@ -203,40 +202,6 @@ Spring-Boot-Lib: BOOT-INF/lib Expect(os.Unsetenv("CDS_TRAINING_JAVA_TOOL_OPTIONS")).To(Succeed()) }) - it("contributes default JAVA_TOOL_OPTIONS to training run", func() { - Expect(os.Setenv("JAVA_TOOL_OPTIONS", "default-opt")).To(Succeed()) - - aotEnabled, cdsEnabled = true, true - dc := libpak.DependencyCache{CachePath: "testdata"} - executor.On("Execute", mock.Anything).Return(nil) - - Expect(os.WriteFile(filepath.Join(ctx.Application.Path, "META-INF", "MANIFEST.MF"), []byte(` -Spring-Boot-Version: 3.3.1 -Spring-Boot-Classes: BOOT-INF/classes -Spring-Boot-Lib: BOOT-INF/lib -`), 0644)).To(Succeed()) - props, err := libjvm.NewManifest(ctx.Application.Path) - Expect(err).NotTo(HaveOccurred()) - - s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true) - s.Executor = executor - - layer, err := ctx.Layers.Layer("test-layer") - Expect(err).NotTo(HaveOccurred()) - - layer, err = s.Contribute(layer) - Expect(err).NotTo(HaveOccurred()) - - Expect(executor.Calls).To(HaveLen(2)) - e, ok := executor.Calls[1].Arguments[0].(effect.Execution) - Expect(ok).To(BeTrue()) - - Expect(e.Env).To(ContainElement("JAVA_TOOL_OPTIONS=default-opt")) - Expect(layer.Build).To(BeTrue()) - - Expect(os.Unsetenv("JAVA_TOOL_OPTIONS")).To(Succeed()) - }) - it("contributes Spring Performance for Boot 3.3+, both CDS & AOT enabled - with SCB symlink", func() { aotEnabled, cdsEnabled = true, true dc := libpak.DependencyCache{CachePath: "testdata"} @@ -256,7 +221,7 @@ Spring-Boot-Lib: BOOT-INF/lib props, err := libjvm.NewManifest(ctx.Application.Path) Expect(err).NotTo(HaveOccurred()) - s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true) + s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true, "") s.Executor = executor layer, err := ctx.Layers.Layer("test-layer") @@ -299,7 +264,7 @@ Spring-Boot-Lib: BOOT-INF/lib props, err := libjvm.NewManifest(ctx.Application.Path) Expect(err).NotTo(HaveOccurred()) - s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true) + s := boot.NewSpringPerformance(dc, ctx.Application.Path, props, aotEnabled, cdsEnabled, "", true, "") s.Executor = executor layer, err := ctx.Layers.Layer("test-layer")