Add BouncyCastle-based native keystore builder for the Java Keystore step - #2140
Add BouncyCastle-based native keystore builder for the Java Keystore step#2140zentron wants to merge 2 commits into
Conversation
…step Replaces the com.octopus.calamari.keystore.KeystoreConfig Java process with a native BouncyCastle.Cryptography implementation, gated behind the java-keystore-native-bouncycastle feature toggle so the existing Java-based path remains the default. Builds a PKCS12 keystore rather than JKS (the original's JKS support comes from the JDK's own provider, which has no .NET equivalent; BouncyCastle there is PEM-parsing only). PKCS12 is accepted anywhere JKS currently is, provided the consuming Tomcat/WildFly config declares the keystore type explicitly.
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a JVM-free implementation of the “Deploy a Keystore to the Filesystem” Java keystore step by building a PKCS#12 keystore directly in .NET using BouncyCastle, gated behind a new java-keystore-native-bouncycastle feature toggle (off by default). This is part of the broader effort to remove the unmaintained Octopus.Dependencies.Java tooling dependency from Calamari’s Java-related steps.
Changes:
- Added
JavaKeystoreBuilder(BouncyCastle-based) to build and persist PKCS#12 keystores natively, including retry logic around the file write. - Wired
JavaKeystoreActionto branch between the legacy Java process path and the new native path based on the feature toggle. - Updated
JavaLibraryCommandto skipJavaRuntime.VerifyExists()only when executing the keystore action with the native toggle enabled; added NUnit coverage for the new builder/action behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| source/Calamari/Deployment/Features/Java/Actions/JavaKeystoreAction.cs | Branches between native keystore build vs legacy Java runner based on the toggle. |
| source/Calamari/Commands/Java/JavaLibraryCommand.cs | Skips JVM existence check only for the native keystore action when toggle enabled. |
| source/Calamari.Shared/Integration/Certificates/Java/JavaKeystoreBuilder.cs | New native PKCS#12 keystore builder + PEM parsing + retry-on-write implementation. |
| source/Calamari.Common/FeatureToggles/OctopusFeatureToggle.cs | Adds the java-keystore-native-bouncycastle toggle slug and definition. |
| source/Calamari.Tests/Java/Fixtures/JavaKeystoreBuilderFixture.cs | Tests PKCS#12 round-trip and default alias/password behavior. |
| source/Calamari.Tests/Java/Fixtures/JavaKeystoreActionFixture.cs | Verifies native toggle path does not invoke the Java command runner. |
Suppressed comments (2)
source/Calamari/Deployment/Features/Java/Actions/JavaKeystoreAction.cs:28
- The native keystore path passes privateKeyPem/certificatePem directly into the BouncyCastle parser. If either variable is missing, the parser will currently throw ArgumentNullException rather than a CommandException with a helpful message. Fetch these as mandatory variables (consistent with other Calamari commands) so failures are clear and actionable.
var privateKeyPem = variables.Get(SpecialVariables.Certificate.PrivateKeyPem(certificateId));
var certificatePem = variables.Get(SpecialVariables.Certificate.CertificatePem(certificateId));
source/Calamari.Shared/Integration/Certificates/Java/JavaKeystoreBuilder.cs:218
- ParseCertificateChain will throw ArgumentNullException if the PEM string is null (e.g., missing variable). Add an explicit null/whitespace check and throw CommandException so callers get a clear error message.
static IList<X509Certificate> ParseCertificateChain(string pem)
{
var certificates = new List<X509Certificate>();
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| var certificateId = variables.Get(SpecialVariables.Action.Java.JavaKeystore.Variable); | ||
| var password = variables.Get(SpecialVariables.Action.Java.JavaKeystore.Password); | ||
| var keystoreFilename = variables.Get(SpecialVariables.Action.Java.JavaKeystore.KeystoreFilename); | ||
| var keystoreAlias = variables.Get(SpecialVariables.Action.Java.JavaKeystore.KeystoreAlias); |
| static AsymmetricKeyParameter ParsePrivateKey(string pem) | ||
| { | ||
| var match = BeginPrivateKey.Match(pem); | ||
| if (!match.Success) | ||
| throw new CommandException("The private key does not contain a recognisable PEM private key block."); | ||
|
|
| return new ResiliencePipelineBuilder() | ||
| .AddRetry(new RetryStrategyOptions | ||
| { | ||
| ShouldHandle = new PredicateBuilder().Handle<Exception>(), |
java-keystore-native-bouncycastletoggle needs a matchingOctopusFeatureToggledefinition on the Server side before it can ever be enabled — not included in this PR. Adding theRequires Server Changelabel.Background
The "Deploy a Keystore to the Filesystem" step currently shells out to
com.octopus.calamari.keystore.KeystoreConfig, a Kotlin process bundled in theOctopus.Dependencies.JavaNuGet package (unmaintained since 2022, ships a static 2017 JDK8tools.jar).This is the first step in an effort to drop that dependency from Calamari's Java/Tomcat/WildFly steps, none of which actually need a JVM once the process-based tooling is replaced.
Results
JavaKeystoreBuilder(Calamari.Shared/Integration/Certificates/Java/) parses a PEM private key + cert chain with BouncyCastle.Cryptography and builds a PKCS#12 keystore, gated behind thejava-keystore-native-bouncycastletoggle (off by default; existing Java-based path unchanged).PemReadercan't handle that case.RetryTemplatepolicy) wraps only the file write, not PEM parsing — a malformed cert fails identically every attempt, so retrying it is pure waste.JavaKeystoreAction(branches on toggle) andJavaLibraryCommand(skipsJavaRuntime.VerifyExists()only for this action type when the toggle is on).Testing
7 new NUnit tests (
JavaKeystoreBuilderFixture,JavaKeystoreActionFixture): real BouncyCastle-generated RSA certs round-tripped through a realPkcs12Storereload, and asserting the Java command-line runner is never invoked when the toggle is on. No JVM available in the dev sandbox to exercise the existing path side-by-side; the 4 pre-existingDeployJavaArchiveFixturefailures (need a real JVM) are untouched and unrelated.How to review
Core logic is
JavaKeystoreBuilder.cs— the PEM parsing (including the EC fallback) is the riskiest part. Everything else is toggle plumbing.