Skip to content

Refactor supply and fix possible issues in frameworks - #1397

Open
kiril-keranov wants to merge 2 commits into
cloudfoundry:mainfrom
kiril-keranov:fix_and_cleanup
Open

Refactor supply and fix possible issues in frameworks#1397
kiril-keranov wants to merge 2 commits into
cloudfoundry:mainfrom
kiril-keranov:fix_and_cleanup

Conversation

@kiril-keranov

@kiril-keranov kiril-keranov commented Aug 20, 2026

Copy link
Copy Markdown
Contributor
  • Refactor supply.go in order not to reuse already created context instead of creating new one excessively.
  • Remove hardcoded fallback versions across frameworks and containers. Eight components (new_relic, open_telemetry_javaagent, postgresql_jdbc, app_dynamics, jacoco_agent, spring_auto_reconfiguration, groovy, spring_boot_cli) silently fell back to a hardcoded version string when DefaultVersion failed a manifest lookup. This masked misconfigured or missing manifest entries, potentially installing a stale cached artifact or failing with a confusing downstream error. All eight now return a descriptive error immediately, consistent with the rest of the codebase.
  • Fix broken classpath glob in JavaMainContainer.buildClasspath() (java_main.go). The glob pattern filepath.Join(buildDir, "$HOME/.jar") produced a literal path like /tmp/build/$HOME/.jar which never matched anything, silently dropping all root-level JARs from the classpath. Fixed to filepath.Join(buildDir, "*.jar") with $HOME/ prepended to each match when constructing the runtime path, consistent with how other classpath entries are built.

@ramonskie

Copy link
Copy Markdown
Contributor

makes sense..

@stokpop

stokpop commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Looks good, got some suggestions from my ai agent:

Review suggestions

1. Classpath glob: use wildcard pattern instead of enumerating

The fix is correct, but the per-jar enumeration is inconsistent with how lib/ and BOOT-INF/lib/ are handled (both use * wildcards). Simpler approach:

jarFiles, err := filepath.Glob(filepath.Join(buildDir, "*.jar"))
if err == nil && len(jarFiles) > 0 {
    classpathEntries = append(classpathEntries, "$HOME/*.jar")
}

Same runtime result, one classpath entry instead of N, consistent with $HOME/lib/* and $HOME/BOOT-INF/lib/* patterns already used in this function.

2. Add a unit test that catches the original bug

The existing buildClasspath tests in java_main_test.go don't assert the $HOME/ prefix on root-level jars. A test like this would have caught the bug and will prevent regressions:

Context("with JARs in root directory", func() {
    BeforeEach(func() {
        os.WriteFile(filepath.Join(buildDir, "app.jar"), []byte("fake"), 0644)
        os.WriteFile(filepath.Join(buildDir, "Main.class"), []byte("fake"), 0644)
    })

    It("includes root JARs with $HOME prefix in classpath", func() {
        os.Setenv("JAVA_MAIN_CLASS", "com.example.Main")
        defer os.Unsetenv("JAVA_MAIN_CLASS")

        cmd, err := container.Release()
        Expect(err).NotTo(HaveOccurred())
        // Verify root jars are reachable at runtime via $HOME path
        Expect(cmd).To(ContainSubstring("$HOME/*.jar"))
        // Or if enumerating individually:
        // Expect(cmd).To(ContainSubstring("$HOME/app.jar"))
    })
})

This test fails on main (the $HOME/*.jar literal path in filepath.Glob never matches) and passes with the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants