Skip to content

Commit 2236da8

Browse files
committed
🐛 fix: implement cross-phase config.yml contract between supply and finalize
- Merge two WriteConfigYml calls in supply.go into a single write at end of Run() with all keys (container, jre, jre_version, java_home) - Have installJRE() return (JRE, string, error) so Run() collects all values before writing once - Remove WriteConfigYml(nil) in supply/cli/main.go that wiped everything - Add NewFinalizer constructor in finalize.go that reads config.yml via libbuildpack.NewYAML().Load() following go-buildpack pattern - Replace re-detection in finalize with registry Get() lookups by name - Add Get(name) methods to containers.Registry and jres.Registry - Strengthen tests: supply test asserts all keys survive; finalize tests verify NewFinalizer reads, and fails correctly when config is absent
1 parent 5909a86 commit 2236da8

8 files changed

Lines changed: 186 additions & 138 deletions

File tree

src/java/containers/container.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ type Container interface {
2020
Release() (string, error)
2121
}
2222

23-
2423
// Registry manages available containers
2524
type Registry struct {
2625
containers []Container
@@ -75,6 +74,18 @@ func (r *Registry) DetectAll() ([]Container, []string, error) {
7574
return matched, names, nil
7675
}
7776

77+
// Get returns the container whose Detect() returns the given name, or nil if not found.
78+
// Used by the finalize phase to resolve a container by the name stored in config.yml.
79+
func (r *Registry) Get(name string) Container {
80+
for _, container := range r.containers {
81+
detected, err := container.Detect()
82+
if err == nil && detected == name {
83+
return container
84+
}
85+
}
86+
return nil
87+
}
88+
7889
// RegisterStandardContainers registers all standard containers in the correct priority order.
7990
// This ensures Supply and Finalize phases use the same detection order.
8091
// IMPORTANT: The order matters! Containers are checked in registration order.

src/java/finalize/cli/main.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,13 @@ func main() {
4747
os.Exit(10)
4848
}
4949

50-
f := finalize.Finalizer{
51-
Stager: stager,
52-
Manifest: manifest,
53-
Installer: installer,
54-
Log: logger,
55-
Command: &libbuildpack.Command{},
50+
f, err := finalize.NewFinalizer(stager, manifest, installer, logger, &libbuildpack.Command{})
51+
if err != nil {
52+
logger.Error("Unable to initialize finalizer from supply config: %s", err.Error())
53+
os.Exit(11)
5654
}
5755

58-
if err = finalize.Run(&f); err != nil {
56+
if err = finalize.Run(f); err != nil {
5957
os.Exit(12)
6058
}
6159

src/java/finalize/finalize.go

Lines changed: 84 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,61 @@ import (
1515
)
1616

1717
type Finalizer struct {
18-
Stager *libbuildpack.Stager
19-
Manifest *libbuildpack.Manifest
20-
Installer *libbuildpack.Installer
21-
Log *libbuildpack.Logger
22-
Command *libbuildpack.Command
23-
Container containers.Container
24-
JRE jres.JRE
18+
Stager *libbuildpack.Stager
19+
Manifest *libbuildpack.Manifest
20+
Installer *libbuildpack.Installer
21+
Log *libbuildpack.Logger
22+
Command *libbuildpack.Command
23+
Container containers.Container
24+
JRE jres.JRE
25+
ContainerName string
26+
JREName string
27+
}
28+
29+
// SupplyConfig holds the values written to config.yml by the supply phase.
30+
type SupplyConfig struct {
31+
Container string `yaml:"container"`
32+
JRE string `yaml:"jre"`
33+
JREVersion string `yaml:"jre_version"`
34+
JavaHome string `yaml:"java_home"`
35+
}
36+
37+
// NewFinalizer creates a Finalizer by reading the config.yml written by the supply phase.
38+
// This follows the pattern established by go-buildpack and dotnet-core-buildpack.
39+
func NewFinalizer(stager *libbuildpack.Stager, manifest *libbuildpack.Manifest,
40+
installer *libbuildpack.Installer, logger *libbuildpack.Logger,
41+
command *libbuildpack.Command) (*Finalizer, error) {
42+
43+
raw := struct {
44+
Config SupplyConfig `yaml:"config"`
45+
}{}
46+
if err := libbuildpack.NewYAML().Load(filepath.Join(stager.DepDir(), "config.yml"), &raw); err != nil {
47+
logger.Error("Unable to read supply phase config.yml: %s", err)
48+
return nil, err
49+
}
50+
51+
cfg := raw.Config
52+
if cfg.Container == "" || cfg.JRE == "" {
53+
return nil, fmt.Errorf("config.yml is missing required keys: container=%q jre=%q", cfg.Container, cfg.JRE)
54+
}
55+
56+
logger.Info("Loaded supply config: container=%s jre=%s version=%s", cfg.Container, cfg.JRE, cfg.JREVersion)
57+
58+
return &Finalizer{
59+
Stager: stager,
60+
Manifest: manifest,
61+
Installer: installer,
62+
Log: logger,
63+
Command: command,
64+
ContainerName: cfg.Container,
65+
JREName: cfg.JRE,
66+
}, nil
2567
}
2668

2769
// Run performs the finalize phase
2870
func Run(f *Finalizer) error {
2971
f.Log.BeginStep("Finalizing Java")
3072

31-
// Create container context
3273
ctx := &common.Context{
3374
Stager: f.Stager,
3475
Manifest: f.Manifest,
@@ -37,32 +78,30 @@ func Run(f *Finalizer) error {
3778
Command: f.Command,
3879
}
3980

40-
// Create and populate container registry with standard containers
41-
registry := containers.NewRegistry(ctx)
42-
registry.RegisterStandardContainers()
43-
44-
// Detect which container was used (should match supply phase)
45-
container, containerName, err := registry.Detect()
81+
// Resolve container using the name stored by supply — no re-detection needed.
82+
container, err := resolveContainer(ctx, f.ContainerName)
4683
if err != nil {
47-
f.Log.Error("Failed to detect container: %s", err.Error())
84+
f.Log.Error("Failed to resolve container %q: %s", f.ContainerName, err.Error())
4885
return err
4986
}
50-
if container == nil {
51-
f.Log.Error("No suitable container found for this application")
52-
return fmt.Errorf("no suitable container found")
53-
}
54-
55-
f.Log.Info("Finalizing container: %s", containerName)
5687
f.Container = container
5788

58-
// Finalize JRE (memory calculator, jvmkill, etc.)
59-
jre, err := f.finalizeJRE()
89+
f.Log.Info("Finalizing container: %s", f.ContainerName)
90+
91+
// Resolve JRE using the name stored by supply — no re-detection needed.
92+
jre, err := resolveJRE(ctx, f.JREName)
6093
if err != nil {
61-
f.Log.Error("Failed to finalize JRE: %s", err.Error())
94+
f.Log.Error("Failed to resolve JRE %q: %s", f.JREName, err.Error())
6295
return err
6396
}
6497
f.JRE = jre
6598

99+
// Finalize JRE (memory calculator, jvmkill, etc.)
100+
if err := f.finalizeJRE(); err != nil {
101+
f.Log.Error("Failed to finalize JRE: %s", err.Error())
102+
return err
103+
}
104+
66105
// Finalize frameworks (APM agents, etc.)
67106
if err := f.finalizeFrameworks(); err != nil {
68107
f.Log.Error("Failed to finalize frameworks: %s", err.Error())
@@ -85,56 +124,45 @@ func Run(f *Finalizer) error {
85124
return nil
86125
}
87126

88-
// finalizeJRE finalizes the JRE configuration (memory calculator, jvmkill, etc.)
89-
// Returns the finalized JRE instance for use in command generation
90-
func (f *Finalizer) finalizeJRE() (jres.JRE, error) {
91-
f.Log.BeginStep("Finalizing JRE")
92-
93-
// Create JRE context
94-
ctx := &common.Context{
95-
Stager: f.Stager,
96-
Manifest: f.Manifest,
97-
Installer: f.Installer,
98-
Log: f.Log,
99-
Command: f.Command,
127+
// resolveContainer finds the container registered under the given name.
128+
func resolveContainer(ctx *common.Context, name string) (containers.Container, error) {
129+
registry := containers.NewRegistry(ctx)
130+
registry.RegisterStandardContainers()
131+
container := registry.Get(name)
132+
if container == nil {
133+
return nil, fmt.Errorf("no container registered with name %q", name)
100134
}
135+
return container, nil
136+
}
101137

102-
// Create and populate JRE registry
103-
// This MUST match the behavior in the supply phase to ensure consistent detection.
104-
// The finalize phase re-detects the JRE (rather than reading stored config) to support:
105-
// 1. Multi-buildpack scenarios where supply and finalize may run in different contexts
106-
// 2. Environment variable overrides that occur between phases
107-
// 3. Detection of JREs installed by other buildpacks
138+
// resolveJRE finds the JRE registered under the given name.
139+
func resolveJRE(ctx *common.Context, name string) (jres.JRE, error) {
108140
registry := jres.NewRegistry(ctx)
109141
registry.RegisterStandardJREs()
110-
111-
// Detect which JRE was installed (should match supply phase)
112-
// With SetDefault(openJDK) configured, this will always return a JRE unless
113-
// an explicitly configured JRE fails detection
114-
jre, jreName, err := registry.Detect()
115-
if err != nil {
116-
f.Log.Error("Failed to detect JRE: %s", err.Error())
117-
return nil, err
142+
jre := registry.Get(name)
143+
if jre == nil {
144+
return nil, fmt.Errorf("no JRE registered with name %q", name)
118145
}
146+
return jre, nil
147+
}
119148

120-
f.Log.Info("Finalizing JRE: %s", jreName)
149+
// finalizeJRE finalizes the JRE configuration (memory calculator, jvmkill, etc.)
150+
func (f *Finalizer) finalizeJRE() error {
151+
f.Log.BeginStep("Finalizing JRE: %s", f.JREName)
121152

122-
// Call JRE finalize (this will finalize memory calculator, jvmkill, etc.)
123-
if err := jre.Finalize(); err != nil {
153+
if err := f.JRE.Finalize(); err != nil {
124154
f.Log.Warning("Failed to finalize JRE: %s (continuing)", err.Error())
125155
// Don't fail the build if JRE finalization fails
126-
return jre, nil
127156
}
128157

129158
f.Log.Info("JRE finalization complete")
130-
return jre, nil
159+
return nil
131160
}
132161

133162
// finalizeFrameworks finalizes framework components (APM agents, etc.)
134163
func (f *Finalizer) finalizeFrameworks() error {
135164
f.Log.BeginStep("Finalizing frameworks")
136165

137-
// Create framework context
138166
ctx := &common.Context{
139167
Stager: f.Stager,
140168
Manifest: f.Manifest,
@@ -143,11 +171,9 @@ func (f *Finalizer) finalizeFrameworks() error {
143171
Command: f.Command,
144172
}
145173

146-
// Create and populate framework registry
147174
registry := frameworks.NewRegistry(ctx)
148175
registry.RegisterStandardFrameworks()
149176

150-
// Detect all frameworks that were installed
151177
detectedFrameworks, frameworkNames, err := registry.DetectAll()
152178
if err != nil {
153179
f.Log.Warning("Failed to detect frameworks: %s", err.Error())
@@ -161,44 +187,35 @@ func (f *Finalizer) finalizeFrameworks() error {
161187

162188
f.Log.Info("Finalizing frameworks: %v", strings.Join(frameworkNames, ","))
163189

164-
// Finalize all detected frameworks
165190
for i, framework := range detectedFrameworks {
166191
f.Log.Info("Finalizing framework: %s", frameworkNames[i])
167192
if err := framework.Finalize(); err != nil {
168193
f.Log.Warning("Failed to finalize framework %s: %s", frameworkNames[i], err.Error())
169194
// Continue with other frameworks even if one fails
170-
continue
171195
}
172196
}
173197

174198
// After all frameworks have written their .opts files, create the centralized assembly script
175-
// This script reads all .opts files in priority order and assembles JAVA_OPTS at runtime
176199
if err := frameworks.CreateJavaOptsAssemblyScript(ctx); err != nil {
177200
f.Log.Warning("Failed to create JAVA_OPTS assembly script: %s", err.Error())
178-
// Don't fail the build, but this means JAVA_OPTS won't be assembled
179201
}
180202

181203
return nil
182204
}
183205

184206
// writeReleaseYaml writes the release configuration to a YAML file
185-
// This follows the pattern used by Ruby, Go, and Node.js buildpacks
186207
func (f *Finalizer) writeReleaseYaml(container containers.Container) error {
187208
f.Log.BeginStep("Writing release configuration")
188209

189-
// Get the container's startup command
190210
containerCommand, err := container.Release()
191211
if err != nil {
192212
return fmt.Errorf("failed to get container command: %w", err)
193213
}
194214

195-
// Prepend memory calculator command if available (Ruby buildpack parity)
196-
// The memory calculator must run before the Java command to set JAVA_OPTS
197215
var fullCommand string
198216
if f.JRE != nil {
199217
memCalcCmd := f.JRE.MemoryCalculatorCommand()
200218
if memCalcCmd != "" {
201-
// Join with && to ensure memory calculator runs before container command
202219
fullCommand = memCalcCmd + " && " + containerCommand
203220
f.Log.Debug("Prepended memory calculator command to startup")
204221
} else {
@@ -208,14 +225,11 @@ func (f *Finalizer) writeReleaseYaml(container containers.Container) error {
208225
fullCommand = containerCommand
209226
}
210227

211-
// Create tmp directory in build dir
212228
tmpDir := filepath.Join(f.Stager.BuildDir(), "tmp")
213229
if err := os.MkdirAll(tmpDir, 0755); err != nil {
214230
return fmt.Errorf("failed to create tmp directory: %w", err)
215231
}
216232

217-
// Write YAML file with release information
218-
// The command must be properly escaped for YAML - use single quotes to preserve special characters
219233
releaseYamlPath := filepath.Join(tmpDir, "java-buildpack-release-step.yml")
220234
yamlContent := fmt.Sprintf(`---
221235
default_process_types:

0 commit comments

Comments
 (0)