Add native Tomcat Manager HTTP client and connector configurator - #2141
Open
zentron wants to merge 1 commit into
Open
Add native Tomcat Manager HTTP client and connector configurator#2141zentron wants to merge 1 commit into
zentron wants to merge 1 commit into
Conversation
Replaces the com.octopus.calamari.tomcat.TomcatDeploy/TomcatState and com.octopus.calamari.tomcathttps.TomcatHttpsConfig Java processes with native implementations, gated behind the tomcat-native-integration feature toggle (existing Java-based paths remain the default): - Tomcat Deploy / Tomcat State: TomcatManagerClient talks to Tomcat's Manager "/text/..." API directly over HttpClient - it was already just an authenticated HTTP call, nothing Java-specific about it. - Deploy a Certificate to Tomcat: TomcatConnectorConfigurator edits server.xml directly (XmlDocument + XPath, matching existing project convention), covering both the Tomcat 7/8.0 style (attributes directly on <Connector>) and the 8.5+ style (<SSLHostConfig><Certificate>), and both keystore-based (BIO/NIO/NIO2, reusing JavaKeystoreBuilder from the keystore branch this one is stacked on) and APR (raw PEM files) implementations. Known simplification: ConnectorIsEmpty approximates the original's connectorAttribuites whitelist (a large, version-accumulated list of non-certificate Connector attributes) by checking for the absence of the certificate-related attributes instead - simpler, and sufficient for the default-host-detection heuristic it backs, but not a byte-for-byte port of that specific list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tomcat-native-integrationtoggle needs its own Server-sideOctopusFeatureToggledefinition. Adding theRequires Server Changelabel.Stacked on #2140 (
feature/java-keystore-bouncycastle) — please merge that one first.Background
Continues the effort started in #2140 to drop the
Octopus.Dependencies.Javadependency from Calamari. This PR covers the 3 Tomcat steps: Deploy, State, and Deploy a Certificate to Tomcat, all of which currently shell out to Java processes in that package.Results
TomcatManagerClient/TomcatManagerOptions(Calamari.Shared/Integration/Tomcat/): talks to Tomcat's Manager/text/...HTTP API directly — it was always just an authenticated HTTP call, no JVM needed. Covers deploy/redeploy/undeploy/start/stop/list, matching the original's URL-building and retry policy, except 401/403 responses now throw a distinctTomcatAuthenticationExceptionthat's excluded from retry (same "don't retry deterministic failures" fix as Add BouncyCastle-based native keystore builder for the Java Keystore step #2140).TomcatConnectorConfigurator(.../Certificates/Java/Tomcat/): editsserver.xmldirectly viaXmlDocument/XPath (matching the existing convention inXmlFormatVariableReplacer) for the Deploy Certificate step. Covers Tomcat 7/8.0 (attributes on<Connector>) and 8.5+ (<SSLHostConfig><Certificate>), both keystore-based (BIO/NIO/NIO2, reusingJavaKeystoreBuilderfrom Add BouncyCastle-based native keystore builder for the Java Keystore step #2140) and APR (raw PEM files), plus version guards (NIO2 needs ≥8.0, BIO forbidden ≥8.5).TomcatFeature,TomcatStateAction,TomcatDeployCertificateAction) branch on the singletomcat-native-integrationtoggle; default off, existing path unchanged.Caveats
ConnectorIsEmptyapproximates the original's large version-accumulated attribute whitelist by checking for absence of certificate-specific attributes instead — simpler, sufficient for the default-host-detection heuristic it backs, but not a byte-for-byte port. Flagged for review.Testing
22 new NUnit tests across 3 fixtures: URL-building unit tests, a real local
HttpListenerexercising the HTTP client end-to-end (auth headers, 401/403), and 9 tests drivingTomcatConnectorConfiguratoragainst real generated certs and realserver.xmlfiles (Tomcat 7 vs 9, NIO vs APR, missing<Service>, version guards, backup zip). Same 4 pre-existing unrelatedDeployJavaArchiveFixtureJVM-dependent failures as #2140.Beyond the committed suite, this was also manually verified end-to-end against a real Tomcat 9 container (Docker, manager app enabled, Tomcat Native/APR loaded), using temporary
[Explicit]fixtures that were not committed:TomcatManagerClient: full deploy (WAR upload) → list →VerifyState→ stop → start → undeploy lifecycle against the real Manager API, plus a wrong-credentials case confirming the fail-fast (no retry) behaviour.TomcatConnectorConfigurator: generated real certs, ran the configurator against a copy of the container's actualserver.xmlfor both the NIO (keystore) and APR (raw PEM) implementations, pushed the result back into the container, and confirmed viaopenssl s_client/curlthat Tomcat started the HTTPS connector and served the exact certificate configured in each case.How to review
TomcatConnectorConfigurator.csis the core and riskiest piece — the version/implementation branching is where subtle bugs would hide.TomcatManagerClient.csis comparatively straightforward HTTP wiring. The diff shown here is scoped to Tomcat-only changes since the base is #2140's branch, notmain.