Skip to content

Commit a40d1fa

Browse files
authored
Merge pull request #1333 from stokpop/fix/tomcat-context-path
fix(tomcat): restore context_path support from Ruby buildpack
2 parents 7117587 + 9ffd563 commit a40d1fa

4 files changed

Lines changed: 341 additions & 18 deletions

File tree

RUBY_VS_GO_BUILDPACK_COMPARISON.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ func (t *TomcatContainer) Supply() error {
445445
| **Logging Support** | `TomcatLoggingSupport` | `installTomcatLoggingSupport()` | ✅ Complete | Installs `tomcat-logging-support.jar` (CloudFoundryConsoleHandler) |
446446
| **setenv.sh Generation** | `TomcatSetenv` | `createSetenvScript()` | ✅ Complete | Creates `bin/setenv.sh` for CLASSPATH |
447447
| **Utils (XML helpers)** | `TomcatUtils` | N/A | ✅ Complete | Go uses standard library XML parsing |
448+
| **Context Path** | `TomcatInstance#root` (webapps dir rename) | `contextXMLFilename()` + context descriptor | ✅ Complete | Same config key; different mechanism — see §2A.11 |
448449
| **Geode/GemFire Session Store** | `TomcatGeodeStore` (199 lines) | **❌ Missing** | ❌ Not Implemented | Session clustering for Tanzu GemFire |
449450
| **Redis Session Store** | `TomcatRedisStore` (118 lines) | **❌ Missing** | ❌ Not Implemented | Session clustering for Redis |
450451
| **Spring Insight Support** | `TomcatInsightSupport` (51 lines) | **❌ Missing** | ⚠️ Deprecated | Spring Insight deprecated by VMware |
@@ -849,6 +850,7 @@ cf set-env myapp JBP_CONFIG_TOMCAT '{tomcat: {version: 9.0.+}}'
849850
| **External Configuration** | ⚠️ 90% | Go requires manifest (no runtime repository_root) |
850851
| **Lifecycle Support** | ✅ 100% | Both detect startup failures |
851852
| **Logging Support** | ✅ 100% | Both use CloudFoundryConsoleHandler |
853+
| **Context Path** | ✅ 100% | Same config key and URL behavior; implementation differs (see §2A.11) |
852854
| **Session Store Auto-Config** | ⚠️ 0% | Go missing convenience auto-configuration (manual setup possible) |
853855
| **Overall** | ⚠️ **95%** | Core features complete; auto-config conveniences missing |
854856

@@ -875,6 +877,22 @@ cf set-env myapp JBP_CONFIG_TOMCAT '{tomcat: {version: 9.0.+}}'
875877
3. Read `VCAP_SERVICES` in application code (if needed)
876878
4. Test with Go buildpack → Deploy
877879

880+
### 2A.11 Context Path: Implementation Difference
881+
882+
Both buildpacks support `context_path` via `JBP_CONFIG_TOMCAT`, but use different Tomcat mechanisms:
883+
884+
| Aspect | Ruby (4.x) | Go (5.x) |
885+
|--------|-----------|---------|
886+
| **Mechanism** | Deploys app into `tomcat/webapps/<name>/` | Writes `conf/Catalina/localhost/<name>.xml` context descriptor |
887+
| **App location** | `tomcat/webapps/foo#bar/` | `${user.home}/app` (unchanged) |
888+
| **Root context prevention** | No `ROOT` directory in webapps | `ROOT.xml` explicitly removed when non-root path set |
889+
| **Config key** | `tomcat.context_path` | `tomcat.context_path` (identical) |
890+
| **URL result** | App at `/foo/bar` only | App at `/foo/bar` only (identical) |
891+
892+
**Migration impact**: None for users. The `context_path` config key and resulting URL routing are identical between 4.x and 5.x. This is a smooth transition.
893+
894+
**Caveat**: `ServletContext.getRealPath()` and webapps-relative path assumptions may behave differently — the Go buildpack serves from `${user.home}/app` rather than a webapps subdirectory. This is a pre-existing structural difference between the two buildpacks, not specific to `context_path`.
895+
878896
---
879897

880898
## 2B. Container Feature Parity: Complete Analysis

src/integration/tomcat_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,5 +441,20 @@ func testTomcat(platform switchblade.Platform, fixtures string) func(*testing.T,
441441
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")))
442442
})
443443
})
444+
445+
context("with context_path configured", func() {
446+
it("serves app at configured path and not at root", func() {
447+
deployment, logs, err := platform.Deploy.
448+
WithEnv(map[string]string{
449+
"BP_JAVA_VERSION": "11",
450+
"JBP_CONFIG_TOMCAT": `{tomcat: {context_path: /my/app}}`,
451+
}).
452+
Execute(name, filepath.Join(fixtures, "containers", "tomcat_jakarta"))
453+
Expect(err).NotTo(HaveOccurred(), logs.String())
454+
455+
Eventually(deployment).Should(matchers.Serve(ContainSubstring("OK")).WithEndpoint("/my/app"))
456+
Eventually(deployment).ShouldNot(matchers.Serve(ContainSubstring("OK")).WithEndpoint("/"))
457+
})
458+
})
444459
}
445460
}

src/java/containers/tomcat.go

Lines changed: 107 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package containers
22

33
import (
4+
"bytes"
5+
"encoding/xml"
46
"fmt"
57
"io"
68
"net/http"
@@ -553,15 +555,39 @@ func injectDocBase(xmlContent string, docBase string) string {
553555
return xmlContent[:idx] + newContextTag + xmlContent[endIdx:]
554556
}
555557

558+
// contextXMLFilename converts a context path to a Tomcat context XML filename.
559+
// Tomcat convention: /foo/bar → foo#bar.xml, / or empty → ROOT.xml
560+
func contextXMLFilename(contextPath string) string {
561+
name := strings.Trim(contextPath, "/")
562+
if name == "" {
563+
return "ROOT.xml"
564+
}
565+
name = strings.ReplaceAll(name, "/", "#")
566+
return name + ".xml"
567+
}
568+
556569
// Finalize performs final Tomcat configuration
557570
func (t *TomcatContainer) Finalize() error {
558571
t.context.Log.BeginStep("Finalizing Tomcat")
559572

573+
if t.config == nil {
574+
var err error
575+
t.config, err = t.loadConfig()
576+
if err != nil {
577+
return fmt.Errorf("failed to load tomcat config: %w", err)
578+
}
579+
}
580+
560581
buildDir := t.context.Stager.BuildDir()
561-
contextXMLPath := filepath.Join(t.tomcatDir(), "conf", "Catalina", "localhost", "ROOT.xml")
582+
contextXMLName := contextXMLFilename(t.config.Tomcat.ContextPath)
583+
contextXMLPath := filepath.Join(t.tomcatDir(), "conf", "Catalina", "localhost", contextXMLName)
562584

563585
webInf := filepath.Join(buildDir, "WEB-INF")
564-
if _, err := os.Stat(webInf); err == nil {
586+
_, webInfErr := os.Stat(webInf)
587+
if webInfErr != nil && !os.IsNotExist(webInfErr) {
588+
return fmt.Errorf("failed to check WEB-INF directory: %w", webInfErr)
589+
}
590+
if webInfErr == nil {
565591
// the script name is prefixed with 'zzz' as it is important to be the last script sourced from profile.d
566592
// so that the previous scripts assembling the CLASSPATH variable(left from frameworks) are sourced previous to it.
567593
if err := t.context.Stager.WriteProfileD("zzz_classpath_symlinks.sh", fmt.Sprintf(symlinkScript, filepath.Join("WEB-INF", "lib"))); err != nil {
@@ -573,27 +599,89 @@ func (t *TomcatContainer) Finalize() error {
573599
return fmt.Errorf("failed to create context directory: %w", err)
574600
}
575601

576-
appContextXML := filepath.Join(buildDir, "META-INF", "context.xml")
577-
var contextContent string
578-
579-
if _, err := os.Stat(appContextXML); err == nil {
580-
xmlBytes, err := os.ReadFile(appContextXML)
581-
if err != nil {
582-
return fmt.Errorf("failed to read META-INF/context.xml: %w", err)
602+
_, statErr := os.Stat(contextXMLPath)
603+
if statErr != nil && !os.IsNotExist(statErr) {
604+
return fmt.Errorf("failed to check context XML %s: %w", contextXMLName, statErr)
605+
}
606+
if os.IsNotExist(statErr) {
607+
appContextXML := filepath.Join(buildDir, "META-INF", "context.xml")
608+
var contextContent string
609+
610+
_, appStatErr := os.Stat(appContextXML)
611+
if appStatErr != nil && !os.IsNotExist(appStatErr) {
612+
// A non-IsNotExist error (permissions/IO) must not be silently
613+
// treated as "no context.xml" — that would drop user-provided
614+
// Realm/Resource config. Surface it.
615+
return fmt.Errorf("failed to check META-INF/context.xml: %w", appStatErr)
616+
}
617+
if appStatErr == nil {
618+
xmlBytes, err := os.ReadFile(appContextXML)
619+
if err != nil {
620+
return fmt.Errorf("failed to read META-INF/context.xml: %w", err)
621+
}
622+
623+
xmlStr := string(xmlBytes)
624+
xmlStr = strings.TrimSpace(xmlStr)
625+
626+
contextContent = injectDocBase(xmlStr, "${user.home}/app")
627+
t.context.Log.Info("Merged META-INF/context.xml with %s - realm and resource configurations preserved", contextXMLName)
628+
} else {
629+
contextContent = fmt.Sprintf("<Context docBase=\"${user.home}/app\" reloadable=\"false\">\n</Context>\n")
630+
t.context.Log.Info("Created %s with docBase pointing to application directory", contextXMLName)
583631
}
584632

585-
xmlStr := string(xmlBytes)
586-
xmlStr = strings.TrimSpace(xmlStr)
587-
588-
contextContent = injectDocBase(xmlStr, "${user.home}/app")
589-
t.context.Log.Info("Merged META-INF/context.xml with ROOT.xml - realm and resource configurations preserved")
633+
if err := os.WriteFile(contextXMLPath, []byte(contextContent), 0644); err != nil {
634+
return fmt.Errorf("failed to write %s: %w", contextXMLName, err)
635+
}
590636
} else {
591-
contextContent = fmt.Sprintf("<Context docBase=\"${user.home}/app\" reloadable=\"false\">\n</Context>\n")
592-
t.context.Log.Info("Created ROOT.xml with docBase pointing to application directory")
637+
t.context.Log.Info("Context XML %s already exists (e.g. from external config), skipping generation", contextXMLName)
638+
}
639+
if contextXMLName != "ROOT.xml" {
640+
rootXMLPath := filepath.Join(t.tomcatDir(), "conf", "Catalina", "localhost", "ROOT.xml")
641+
if err := os.Remove(rootXMLPath); err != nil && !os.IsNotExist(err) {
642+
return fmt.Errorf("failed to remove ROOT.xml: %w", err)
643+
}
644+
}
645+
} else {
646+
warMatches, err := filepath.Glob(filepath.Join(buildDir, "*.war"))
647+
if err != nil {
648+
return fmt.Errorf("failed to find WAR files in build directory: %w", err)
593649
}
650+
if len(warMatches) == 1 {
651+
warFilename := filepath.Base(warMatches[0])
652+
// Escape XML-sensitive characters (&, <, >, ", ') so a WAR filename
653+
// containing them cannot produce an invalid Tomcat context descriptor.
654+
var escapedWarFilename bytes.Buffer
655+
if err := xml.EscapeText(&escapedWarFilename, []byte(warFilename)); err != nil {
656+
return fmt.Errorf("failed to XML-escape WAR filename %q: %w", warFilename, err)
657+
}
658+
contextContent := fmt.Sprintf("<Context docBase=\"${user.home}/app/%s\" reloadable=\"false\">\n</Context>\n", escapedWarFilename.String())
659+
660+
contextXMLDir := filepath.Dir(contextXMLPath)
661+
if err := os.MkdirAll(contextXMLDir, 0755); err != nil {
662+
return fmt.Errorf("failed to create context directory: %w", err)
663+
}
594664

595-
if err := os.WriteFile(contextXMLPath, []byte(contextContent), 0644); err != nil {
596-
return fmt.Errorf("failed to write ROOT.xml: %w", err)
665+
_, statErr := os.Stat(contextXMLPath)
666+
if statErr != nil && !os.IsNotExist(statErr) {
667+
return fmt.Errorf("failed to check context XML %s: %w", contextXMLName, statErr)
668+
}
669+
if os.IsNotExist(statErr) {
670+
if err := os.WriteFile(contextXMLPath, []byte(contextContent), 0644); err != nil {
671+
return fmt.Errorf("failed to write %s: %w", contextXMLName, err)
672+
}
673+
t.context.Log.Info("Created %s with docBase pointing to %s", contextXMLName, warFilename)
674+
} else {
675+
t.context.Log.Info("Context XML %s already exists (e.g. from external config), skipping generation", contextXMLName)
676+
}
677+
if contextXMLName != "ROOT.xml" {
678+
rootXMLPath := filepath.Join(t.tomcatDir(), "conf", "Catalina", "localhost", "ROOT.xml")
679+
if err := os.Remove(rootXMLPath); err != nil && !os.IsNotExist(err) {
680+
return fmt.Errorf("failed to remove ROOT.xml: %w", err)
681+
}
682+
}
683+
} else if len(warMatches) > 1 {
684+
t.context.Log.Warning("Multiple WAR files found in build directory; cannot determine which to deploy, skipping context descriptor generation")
597685
}
598686
}
599687

@@ -647,6 +735,7 @@ type tomcatConfig struct {
647735
type Tomcat struct {
648736
Version string `yaml:"version"`
649737
ExternalConfigurationEnabled bool `yaml:"external_configuration_enabled"`
738+
ContextPath string `yaml:"context_path"`
650739
}
651740

652741
type ExternalConfiguration struct {

0 commit comments

Comments
 (0)