fix(tomcat): restore context_path support from Ruby buildpack by stokpop · Pull Request #1333 · cloudfoundry/java-buildpack

stokpop · 2026-06-25T07:36:32Z

Summary

Restores context_path\ support from the Ruby buildpack. Users setting JBP_CONFIG_TOMCAT: {tomcat: {context_path: /foo/bar}} always got ROOT.xml and their app deployed at / — the field was never ported to the Go rewrite.

Changes

Adds ContextPath field to Tomcat config struct
contextXMLFilename() helper: /foo/bar → foo#bar.xml (Tomcat convention)
Finalize() reads config and writes the correct context descriptor:
- Exploded WAR (WEB-INF/ present): docBase="${user.home}/app"
- Packaged WAR (*.war present): docBase="${user.home}/app/<name>.war"
- Empty or / context path → ROOT.xml (unchanged default)
- Trailing slash in context_path normalised
- Pre-existing non-root ROOT.xml removed to avoid ambiguous deployment
External config ordering fix: if a context XML descriptor already exists (e.g. placed by external_configuration_enabled), the generated one is skipped — external config wins
RUBY_VS_GO_BUILDPACK_COMPARISON.md updated with context_path parity note and mechanism difference (§2A.2, §2A.11)

Fixes #1331.

Tests

Unit tests (tomcat_test.go, TDD):

context_path: /the/intended/path → the#intended#path.xml created, ROOT.xml absent
Empty or / context path → ROOT.xml
Trailing slash normalised
Pre-existing ROOT.xml removed when non-root context path set
Explicit empty string via JBP_CONFIG_TOMCAT → ROOT.xml
Packaged WAR + context_path → context XML with docBase pointing to WAR file
Packaged WAR + no context_path → ROOT.xml with WAR docBase
Packaged WAR + non-root removes pre-existing ROOT.xml
External config descriptor present → write skipped, file unchanged

Integration test (src/integration/tomcat_test.go):

Deploys tomcat_jakarta fixture with context_path: /my/app, verifies /my/app serves app content and / does not

context_path was implemented in the Ruby buildpack (commit 0186270, 2015) but never ported to Go. Users setting JBP_CONFIG_TOMCAT context_path always got ROOT.xml regardless of config. Tomcat convention: /foo/bar → foo#bar.xml (strip leading /, replace / with #). Empty or / → ROOT.xml (root context, existing default). Changes: - Add ContextPath field to Tomcat config struct (yaml:"context_path") - Add contextXMLFilename() helper implementing the path→filename transform - Finalize() loads config if not already loaded (Supply() may not have run in all call paths), computes XML filename from context_path - Three new tests (TDD): context_path set, empty, and / Fixes cloudfoundry#1331.

Copilot

Pull request overview

Restores support for configuring Tomcat’s deployment context via JBP_CONFIG_TOMCAT.tomcat.context_path, matching the legacy Ruby buildpack behavior (and fixing #1331) by creating the appropriate context XML filename instead of always writing ROOT.xml.

Changes:

Add ContextPath to the Tomcat config struct and derive the context XML filename from it.
Update Finalize() to write the context XML to the computed filename.
Add unit tests covering configured, empty, and / context paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/java/containers/tomcat.go`	Adds `context_path` config support and uses it during `Finalize()` to select the context XML filename.
`src/java/containers/tomcat_test.go`	Adds `Finalize()` tests validating filename selection for configured/empty/root context paths.

Comments suppressed due to low confidence (1)

src/java/containers/tomcat.go:617

The write error message still says failed to write ROOT.xml, but the target file may be something else when context_path is configured. This will make failures harder to diagnose.

		if err := os.WriteFile(contextXMLPath, []byte(contextContent), 0644); err != nil {
			return fmt.Errorf("failed to write ROOT.xml: %w", err)
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ed ROOT.xml in log

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

…igured; add explicit empty context_path test

stokpop · 2026-06-25T12:51:41Z

Note: context_path deployment mechanism differs between Ruby and Go buildpack

Ruby buildpack (4.x) deploys the app into tomcat/webapps/<name>/ (e.g. webapps/foo#bar/) — Tomcat autodeploys from the webapps directory. No context descriptor XML is involved.

Go buildpack (5.x) uses Tomcat context descriptor files (conf/Catalina/localhost/<name>.xml) with docBase pointing at ${user.home}/app. This means the app is never placed in the webapps/ directory at all — even for the default root context (ROOT.xml points docBase at the app dir).

User-visible URL routing is identical: the application is accessible only at the configured path.

Potential behavioral difference: ServletContext.getRealPath() and any webapps-relative path assumptions will behave differently. In Ruby 4.x, getRealPath("/") resolves under tomcat/webapps/ROOT/ (or webapps/foo#bar/). In Go 5.x, it resolves under ${user.home}/app. This is a structural difference introduced by the Ruby→Go migration, not specific to this context_path PR — but this PR makes it visible for the non-root context case.

Question for reviewers: Is this getRealPath() behavioral difference expected/acceptable, or should the Go buildpack also place (or symlink) the app under tomcat/webapps/<name>/ to maintain full parity?

Details added to RUBY_VS_GO_BUILDPACK_COMPARISON.md §2A.11.

…d Go buildpack (§2A.11)

ramonskie · 2026-06-25T15:54:38Z

Finding: context_path fix only covers exploded WARs.

Current Detect() accepts both:

exploded WAR: WEB-INF/
packaged WAR: *.war

But Finalize() only writes context descriptor when WEB-INF/ exists. So app.war + JBP_CONFIG_TOMCAT='{tomcat:{context_path:/foo}}' still no-op: no foo.xml, no ROOT.xml rewrite/removal.

Also integration test named “WAR file” does not deploy real .war; it deploys exploded tomcat_jakarta fixture (WEB-INF/...). Repo has no .war fixture.

ROOT.xml history matters here: current buildpack writes ROOT.xml after external config, so external config cannot reliably override root context. This PR helps that for exploded apps, but we need coverage proving:

/foo serves OK
/ does not serve app
packaged .war behavior fixed or explicitly unsupported
external config + pre-existing ROOT.xml does not double-deploy

Without that, PR fixes happy path only, not full ROOT.xml/context_path problem.

…r context_path - Finalize() now writes context XML for packaged .war apps (docBase points to the WAR file), so context_path works for both exploded and packaged WARs - Skip writing context XML if the file already exists (external config via external_configuration_enabled provides its own context descriptor) - Unit tests (TDD): 4 new tests covering WAR path, external-config guard - Integration test: verify context_path routes /my/app correctly and / returns 404

…s Tomcat default page not 404

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

…on-root context_path - os.Stat unexpected errors (non-IsNotExist) now returned as failures instead of silently treating the context XML as already existing - ROOT.xml removal now happens regardless of whether context XML was generated or already existed from external config, so non-root context_path always removes a stale ROOT.xml - Unit test added (TDD) for external-config + non-root ROOT.xml removal

stokpop · 2026-06-26T13:50:12Z

@ramonskie thanks for the info, updates the PR and code based on your feedback, please have a look

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

+				return fmt.Errorf("failed to remove ROOT.xml: %w", err)
+			}
+		}
+	} else {


+		}
+	} else {
+		warMatches, err := filepath.Glob(filepath.Join(buildDir, "*.war"))
+		if err == nil && len(warMatches) == 1 {


+				}
+			}
+		} else if len(warMatches) > 1 {
+			t.context.Log.Warning("Multiple WAR files found in build directory; context_path not applied")


stokpop requested a review from Copilot June 25, 2026 09:49

Copilot started reviewing on behalf of stokpop June 25, 2026 09:50 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread src/java/containers/tomcat.go

Comment thread src/java/containers/tomcat.go Outdated

stokpop added 2 commits June 25, 2026 11:38

fix(tomcat): use actual context XML filename in write error message

3b4b434

fix(tomcat): normalize trailing slash in context_path and fix hardcod…

4a16504

…ed ROOT.xml in log

stokpop requested a review from Copilot June 25, 2026 12:09

Copilot started reviewing on behalf of stokpop June 25, 2026 12:09 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread src/java/containers/tomcat_test.go

Comment thread src/java/containers/tomcat_test.go

Comment thread src/java/containers/tomcat_test.go

stokpop requested a review from Copilot June 25, 2026 12:16

Copilot started reviewing on behalf of stokpop June 25, 2026 12:17 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread src/java/containers/tomcat.go

Comment thread src/java/containers/tomcat_test.go

fix(tomcat): remove stale ROOT.xml when non-root context_path is conf…

937509c

…igured; add explicit empty context_path test

docs: document context_path implementation difference between Ruby an…

267ada6

…d Go buildpack (§2A.11)

stokpop mentioned this pull request Jun 25, 2026

Tomcat JBP_CONFIG_TOMCAT context_path not applied in v5 #1331

Open

stokpop added 3 commits June 26, 2026 11:51

Merge remote-tracking branch 'origin/main' into fix/tomcat-context-path

12b80a9

fix(tomcat): fix context_path integration test assertion - root serve…

02f0fe1

…s Tomcat default page not 404

stokpop requested a review from Copilot June 26, 2026 12:28

Copilot started reviewing on behalf of stokpop June 26, 2026 12:28 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Comment thread src/java/containers/tomcat.go Outdated

Comment thread src/java/containers/tomcat.go

Comment thread src/java/containers/tomcat.go Outdated

Comment thread src/java/containers/tomcat.go

stokpop requested a review from Copilot June 26, 2026 13:49

Copilot started reviewing on behalf of stokpop June 26, 2026 13:49 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Uh oh!

Conversation

stokpop commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

stokpop commented Jun 25, 2026

Uh oh!

ramonskie commented Jun 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stokpop commented Jun 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stokpop commented Jun 25, 2026 •

edited

Loading