Add company-internal CA extension point to eSignet Helm chart#1990
Draft
Ivanmeneges wants to merge 1 commit into
Draft
Add company-internal CA extension point to eSignet Helm chart#1990Ivanmeneges wants to merge 1 commit into
Ivanmeneges wants to merge 1 commit into
Conversation
WalkthroughAdds an interactive TLS trust helper and integrates it into installer scripts; extends Helm chart values and templates to support an optional company-internal CA bundle (Secret/ConfigMap), an init container that builds a Java truststore from PEMs, and deployment wiring to mount the truststore and CA bundle. ChangesCustom Company CA Trust Support
Sequence Diagram(s)sequenceDiagram
participant User
participant InstallerScript as Installer Script
participant TLSHelper as TLS Helper<br/>configure_tls_trust
participant Kubectl
participant HelmChart as Helm Chart
participant JavaService as Java Service
User->>InstallerScript: Run install.sh
InstallerScript->>TLSHelper: source and call configure_tls_trust($NS)
User->>TLSHelper: Select TLS mode
alt Custom CA Mode
TLSHelper->>Kubectl: Check Secret exists
alt Secret missing
User->>TLSHelper: Provide PEM CA bundle path
TLSHelper->>Kubectl: Create CA Secret in namespace
end
TLSHelper-->>InstallerScript: TRUST_HELM_ARGS with customCA
else Insecure Mode
TLSHelper-->>InstallerScript: TRUST_HELM_ARGS with enable_insecure=true
else Default Mode
TLSHelper-->>InstallerScript: TRUST_HELM_ARGS (empty)
end
InstallerScript->>HelmChart: helm install/upgrade with $TRUST_HELM_ARGS
HelmChart->>JavaService: render/init customCA init container
JavaService->>JavaService: init container builds truststore and imports certs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/configure_tls_trust.sh`:
- Around line 27-39: The script enables customCA by setting TRUST_HELM_ARGS with
customCA.enabled and customCA.secretName but doesn't validate or propagate
customCA.secretKey; update deploy/configure_tls_trust.sh to check the existing
Secret (variable ca_secret in the script) for the expected key (default "ca.crt"
or a user-provided key), fail with an informative message if the key is missing,
and append --set customCA.secretKey=<key> to TRUST_HELM_ARGS when setting
customCA.enabled=true so the Helm chart's init container (which reads
customCA.secretKey) can find the PEM bundle.
In `@helm/esignet/templates/_helpers.tpl`:
- Around line 73-82: In the template defined by
"esignet.validateValues.customCA" add a validation branch that detects when
.Values.customCA.enabled is true AND both .Values.customCA.secretName and
.Values.customCA.configMapName are non-empty, and emit an error message
indicating that both cannot be set simultaneously (explain that Secret takes
precedence and one must be removed); update the conditional logic alongside the
existing checks for missing names and enable_insecure to ensure this new
mutually-exclusive check runs when customCA.enabled is true.
- Around line 52-54: esignet.validateValues must actively block invalid flag
combinations and be invoked during chart rendering: update the validate function
(esignet.validateValues) to call {{ failf }} when .Values.enable_insecure and
.Values.customCA.enabled are both true (or otherwise enforce a single source of
truth), then include a call to that helper from top-level templates (e.g.,
deployment.yaml and other rendered templates) so validation runs; additionally,
change the init container rendering logic so the enable_insecure init container
(the initContainers block that copies /cacerts) is only rendered when
.Values.enable_insecure is true AND .Values.customCA.enabled is false, or alter
its copy behavior to avoid overwriting the cacerts volume when
customCAInitContainer is enabled, referencing the customCAInitContainer, the
initContainers block, and the cacerts volume to locate the code to change.
In `@helm/esignet/values.yaml`:
- Around line 527-549: The values.yaml default for customCA.javaHome may not
match the Java installation in the main app image, causing the init container to
mount/update the wrong truststore; update either by adding a clear comment near
customCA.javaHome (and mention mountPath and that it must match the main
container JAVA_HOME) or add a runtime validation step in the init container
entrypoint (the init container that uses initContainerImage and writes to {{
.Values.customCA.javaHome }}/lib/security/cacerts) to check that
$JAVA_HOME/lib/security/cacerts exists at the target mountPath and exit with a
clear error if not found so the mismatch is detected early.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 80f08999-cf89-4492-a069-9b0aae7e3b6a
📒 Files selected for processing (10)
deploy/configure_tls_trust.shdeploy/esignet-with-plugins/install.shdeploy/esignet-with-plugins/values.yamldeploy/esignet/install.shdeploy/esignet/values.yamlhelm/esignet/README.mdhelm/esignet/templates/_helpers.tplhelm/esignet/templates/custom-ca-initcontainer.yamlhelm/esignet/templates/deployment.yamlhelm/esignet/values.yaml
Comment on lines
+27
to
+39
| if ! kubectl -n "$ns" get secret "$ca_secret" >/dev/null 2>&1; then | ||
| read -p "Secret '$ca_secret' not found. Provide path to PEM CA bundle file: " ca_file | ||
| if [[ -z "${ca_file}" || ! -f "${ca_file}" ]]; then | ||
| echo "CA bundle file not found; EXITING" | ||
| exit 1 | ||
| fi | ||
| kubectl -n "$ns" create secret generic "$ca_secret" \ | ||
| --from-file=ca.crt="${ca_file}" \ | ||
| --dry-run=client -o yaml | kubectl apply -f - | ||
| echo "Created Secret '$ca_secret' with company CA bundle." | ||
| fi | ||
|
|
||
| TRUST_HELM_ARGS="--set customCA.enabled=true --set customCA.secretName=${ca_secret}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify helper/chart key contract alignment
rg -n 'customCA\.secretName|customCA\.secretKey|secretKey:\s*ca\.crt|configMapKey:\s*ca\.crt' \
deploy/configure_tls_trust.sh \
helm/esignet/values.yaml \
helm/esignet/templates/custom-ca-initcontainer.yamlRepository: mosip/esignet
Length of output: 509
Propagate/validate customCA.secretKey when using an existing CA Secret
The Helm init container builds CA_FILE from customCA.secretKey whenever customCA.secretName is set (defaulting to secretKey: ca.crt). deploy/configure_tls_trust.sh enables only --set customCA.secretName=... for an existing Secret, so if that Secret’s data key isn’t ca.crt (or the configured secretKey), the init container won’t find the mounted PEM file and the pod can fail to start.
Validate the Secret contains the expected key and set --set customCA.secretKey=... accordingly when enabling customCA.enabled=true.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 28-28: read without -r will mangle backslashes.
(SC2162)
[warning] 39-39: TRUST_HELM_ARGS appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/configure_tls_trust.sh` around lines 27 - 39, The script enables
customCA by setting TRUST_HELM_ARGS with customCA.enabled and
customCA.secretName but doesn't validate or propagate customCA.secretKey; update
deploy/configure_tls_trust.sh to check the existing Secret (variable ca_secret
in the script) for the expected key (default "ca.crt" or a user-provided key),
fail with an informative message if the key is missing, and append --set
customCA.secretKey=<key> to TRUST_HELM_ARGS when setting customCA.enabled=true
so the Helm chart's init container (which reads customCA.secretKey) can find the
PEM bundle.
Comment on lines
+52
to
+54
| {{- define "esignet.truststoreRequired" -}} | ||
| {{- or .Values.enable_insecure .Values.customCA.enabled -}} | ||
| {{- end -}} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for where esignet.validateValues is called and whether fail is used
rg -n 'esignet.validateValues' --type=yaml -B 2 -A 5
# Look for Helm fail patterns in templates
rg -n '\{\{-?\s*fail\s+' helm/esignet/templates/ --type=yamlRepository: mosip/esignet
Length of output: 39
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find all occurrences of validateValues (any template extension, not just yaml)
rg -n 'esignet\.validateValues|validateValues' helm/esignet/templates -S
# 2) Find the helper definition for validateValues (if it exists)
rg -n 'define\s+"esignet\.validateValues"' -S helm/esignet/templates
# 3) Check whether any template uses Helm's hard-fail ({{ fail ... }})
rg -n '\{\{-?\s*fail\s+' helm/esignet/templates -S
# 4) Check for 'required' and 'failf' style patterns too (commonly used in charts)
rg -n '\{\{-?\s*(required|failf)\s+' helm/esignet/templates -SRepository: mosip/esignet
Length of output: 662
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="helm/esignet/templates/_helpers.tpl"
echo "---- validateValues definition (lines 1-140) ----"
nl -ba "$FILE" | sed -n '1,140p'
echo "---- locate calls to validateValues helper ----"
rg -n 'include\s+"esignet\.validateValues"|esignet\.validateValues' helm/esignet/templates -S
echo "---- check for fail/failf/required usage in entire templates dir ----"
rg -n '\{\{-?\s*(fail|failf)\s+' helm/esignet/templates -S || true
rg -n '\{\{-?\s*required\s+' helm/esignet/templates -S || trueRepository: mosip/esignet
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="helm/esignet/templates/_helpers.tpl"
echo "---- validateValues definition (whole file, capped at 220 lines) ----"
# show with line numbers (cat -n available)
cat -n "$FILE" | sed -n '1,220p'
echo "---- locate calls to validateValues helper ----"
rg -n 'include\s+"esignet\.validateValues"|include\s+"esignet\.validateValues\.' helm/esignet/templates -S
echo "---- check for fail/failf/required usage in entire templates dir ----"
rg -n '\{\{-?\s*(fail|failf)\s+' helm/esignet/templates -S || true
rg -n '\{\{-?\s*required\s+' helm/esignet/templates -S || true
echo "---- check where validation messages are emitted (if any) ----"
rg -n 'messages|print|printf' helm/esignet/templates/_helpers.tpl -SRepository: mosip/esignet
Length of output: 5312
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- all occurrences of validateValues usage ----"
rg -n 'validateValues' helm/esignet/templates -S
echo "---- specifically look for including the root helper ----"
rg -n 'include\s+"esignet\.validateValues"\s' helm/esignet/templates -S
echo "---- also handle calling by name without include (rare) ----"
rg -n 'esignet\.validateValues\s' helm/esignet/templates -SRepository: mosip/esignet
Length of output: 691
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- validateValues usage anywhere under helm/esignet ----"
rg -n 'validateValues' helm/esignet -S
echo "---- check for the printed validation banner string ----"
rg -n 'VALUES VALIDATION' helm/esignet -S || true
echo "---- check for any template that might deliberately call include/esignet.validateValues indirectly ----"
rg -n 'include\s+"esignet\.' helm/esignet/templates -S | head -n 50Repository: mosip/esignet
Length of output: 1880
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="helm/esignet/templates/deployment.yaml"
echo "---- deployment.yaml around truststoreRequired usages (lines 130-220) ----"
cat -n "$FILE" | sed -n '130,230p'
echo "---- find initContainers blocks to see gating conditions (up to 260) ----"
rg -n 'initContainers:|customCA|enable_insecure|cacerts|truststoreRequired' "$FILE" -S
echo "---- deployment.yaml around cacerts mounts (lines 150-210) ----"
rg -n 'cacerts|truststoreRequired' "$FILE" -n -SRepository: mosip/esignet
Length of output: 5300
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="helm/esignet/templates/deployment.yaml"
cat -n "$FILE" | sed -n '40,120p'Repository: mosip/esignet
Length of output: 4802
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm validateValues helper is never invoked
rg -n 'include\s+"esignet\.validateValues"' -S helm/esignet || true
rg -n 'esignet\.validateValues' -S helm/esignet || true
# 2) Inspect customCA init container template to see how it uses the cacerts volume
rg -n 'define "esignet\.customCAInitContainer"' -S helm/esignet/templates/_helpers.tpl
# print around it
FILE="helm/esignet/templates/_helpers.tpl"
start=$(rg -n 'define "esignet\.customCAInitContainer"' "$FILE" -n | cut -d: -f1 | head -n1)
echo "customCAInitContainer starts at line $start"
cat -n "$FILE" | sed -n "${start},$((start+120))p"
# 3) Inspect values for enable_insecure and initContainers
VALS="helm/esignet/values.yaml"
ls -la helm/esignet | cat
rg -n 'enable_insecure|initContainers|customCA' "$VALS" -S || true
# print relevant sections if present
cat -n "$VALS" | sed -n '1,260p'Repository: mosip/esignet
Length of output: 583
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- helper definitions for customCA init container ----"
rg -n 'define "esignet\.customCAInitContainer"|customCAInitContainer' helm/esignet/templates/_helpers.tpl -S || true
FILE="helm/esignet/templates/_helpers.tpl"
# Try to print around the helper if it exists
LINE=$(rg -n 'define "esignet\.customCAInitContainer"' "$FILE" -S | cut -d: -f1 | head -n1 || true)
if [ -n "${LINE:-}" ]; then
cat -n "$FILE" | sed -n "$LINE,$((LINE+160))p"
fi
echo "---- locate values files ----"
find helm/esignet -maxdepth 1 -type f -name 'values*.yaml' -o -name 'values*.yml' -print
echo "---- grep enable_insecure/customCA-related values across chart root values files ----"
for f in $(find helm/esignet -maxdepth 1 -type f -name 'values*.yaml' -o -name 'values*.yml' -print); do
echo "### $f"
rg -n 'enable_insecure|customCA|initContainers|cacerts' "$f" -S || true
done
echo "---- inspect deployment.yaml init container gating for customCA vs enable_insecure (lines 50-110 already partly shown; show full block) ----"
cat -n helm/esignet/templates/deployment.yaml | sed -n '45,110p'Repository: mosip/esignet
Length of output: 4137
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- rg for esignet.customCAInitContainer definition/inclusions ----"
rg -n 'esignet\.customCAInitContainer' helm/esignet/templates -S
echo "---- locate values files ----"
find helm/esignet -maxdepth 2 -type f \( -name 'values.yaml' -o -name 'values.yml' -o -name 'values.*.yaml' -o -name 'values.*.yml' \) -print
# pick whatever exists
for f in $(find helm/esignet -maxdepth 2 -type f \( -name 'values.yaml' -o -name 'values.yml' -o -name 'values.*.yaml' -o -name 'values.*.yml' \) -print); do
echo "### $f"
rg -n 'enable_insecure|customCA|initContainers|cacerts' "$f" -S || true
doneRepository: mosip/esignet
Length of output: 941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- custom-ca-initcontainer.yaml (full file, likely small) ----"
cat -n helm/esignet/templates/custom-ca-initcontainer.yaml | sed -n '1,220p'
echo "---- values.yaml: initContainers section for enable_insecure ----"
# print around initContainers block
cat -n helm/esignet/values.yaml | sed -n '300,430p'
echo "---- values.yaml: customCA.enabled/related keys ----"
rg -n '^customCA:|enable_insecure:' helm/esignet/values.yaml -n || true
cat -n helm/esignet/values.yaml | sed -n '500,620p'Repository: mosip/esignet
Length of output: 9722
Fix: validation doesn’t prevent deploying conflicting enable_insecure + customCA.enabled
esignet.validateValuesonly emits text (printf "\nVALUES VALIDATION:\n%s" ...) and never calls{{ fail }}/failf, andesignet.validateValuesisn’t invoked anywhere in the chart templates—so the customCA validation at_helpers.tpl(lines 78-81) won’t block rendering.- With both flags true,
deployment.yamlrenders both init containers (customCAInitContainerunder.Values.customCA.enabledand theinitContainersblock under.Values.enable_insecure) and both write to the samecacertsvolume (via/cacerts/cacerts). Theenable_insecureinit container overwrites/cacertsby copying the keystore (cp ...cacerts /cacerts), which can wipe the imported company CA certs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/esignet/templates/_helpers.tpl` around lines 52 - 54,
esignet.validateValues must actively block invalid flag combinations and be
invoked during chart rendering: update the validate function
(esignet.validateValues) to call {{ failf }} when .Values.enable_insecure and
.Values.customCA.enabled are both true (or otherwise enforce a single source of
truth), then include a call to that helper from top-level templates (e.g.,
deployment.yaml and other rendered templates) so validation runs; additionally,
change the init container rendering logic so the enable_insecure init container
(the initContainers block that copies /cacerts) is only rendered when
.Values.enable_insecure is true AND .Values.customCA.enabled is false, or alter
its copy behavior to avoid overwriting the cacerts volume when
customCAInitContainer is enabled, referencing the customCAInitContainer, the
initContainers block, and the cacerts volume to locate the code to change.
Comment on lines
+73
to
+82
| {{- define "esignet.validateValues.customCA" -}} | ||
| {{- if and .Values.customCA.enabled (and (not .Values.customCA.secretName) (not .Values.customCA.configMapName)) -}} | ||
| esignet: customCA.enabled | ||
| When customCA.enabled is true, set either customCA.secretName or customCA.configMapName. | ||
| {{- end -}} | ||
| {{- if and .Values.enable_insecure .Values.customCA.enabled -}} | ||
| esignet: customCA.enable_insecure | ||
| enable_insecure and customCA.enabled cannot be used together. Use customCA for company-internal PKI. | ||
| {{- end -}} | ||
| {{- end -}} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add validation when both secretName and configMapName are set.
The validation at line 74 ensures at least one of secretName or configMapName is set when customCA.enabled is true. However, it doesn't validate the case where both are set simultaneously.
According to the deployment template (context snippet from deployment.yaml:187-194), when both are set, the Secret takes precedence and the ConfigMap is silently ignored:
{{- if .Values.customCA.secretName }}
secret:
secretName: {{ .Values.customCA.secretName }}
{{- else }}
configMap:
name: {{ .Values.customCA.configMapName }}This could lead to silent misconfiguration where a user sets both, expecting the ConfigMap to be used, but the Secret is selected instead.
🔒 Suggested validation enhancement
{{- define "esignet.validateValues.customCA" -}}
{{- if and .Values.customCA.enabled (and (not .Values.customCA.secretName) (not .Values.customCA.configMapName)) -}}
esignet: customCA.enabled
When customCA.enabled is true, set either customCA.secretName or customCA.configMapName.
{{- end -}}
+{{- if and .Values.customCA.enabled (and .Values.customCA.secretName .Values.customCA.configMapName) -}}
+esignet: customCA.source
+ Set either customCA.secretName or customCA.configMapName, not both.
+{{- end -}}
{{- if and .Values.enable_insecure .Values.customCA.enabled -}}
esignet: customCA.enable_insecure
enable_insecure and customCA.enabled cannot be used together. Use customCA for company-internal PKI.
{{- end -}}
{{- end -}}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/esignet/templates/_helpers.tpl` around lines 73 - 82, In the template
defined by "esignet.validateValues.customCA" add a validation branch that
detects when .Values.customCA.enabled is true AND both
.Values.customCA.secretName and .Values.customCA.configMapName are non-empty,
and emit an error message indicating that both cannot be set simultaneously
(explain that Secret takes precedence and one must be removed); update the
conditional logic alongside the existing checks for missing names and
enable_insecure to ensure this new mutually-exclusive check runs when
customCA.enabled is true.
Comment on lines
+527
to
+549
| ## Trust a company-internal CA for outbound HTTPS calls from the Java service. | ||
| ## Create a Secret or ConfigMap with PEM-encoded root/intermediate certificate(s) | ||
| ## before enabling this option. Recommended for on-prem deployments using internal PKI. | ||
| ## | ||
| customCA: | ||
| enabled: false | ||
| ## Use an existing Secret that contains PEM-encoded CA certificate(s). | ||
| secretName: "" | ||
| secretKey: ca.crt | ||
| ## Alternatively, use a ConfigMap instead of a Secret. | ||
| configMapName: "" | ||
| configMapKey: ca.crt | ||
| ## Path where the CA bundle is mounted inside the init container. | ||
| mountPath: /etc/ssl/certs/company-ca | ||
| ## Prefix used for keytool aliases when importing certificates. | ||
| aliasPrefix: company-ca | ||
| javaHome: /usr/local/openjdk-11 | ||
| keystorePassword: changeit | ||
| initContainerImage: | ||
| registry: docker.io | ||
| repository: openjdk | ||
| tag: 11-jre | ||
| pullPolicy: Always |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Document or validate javaHome compatibility.
The customCA.javaHome default assumes OpenJDK 11 at /usr/local/openjdk-11, matching the existing enable_insecure init container pattern (line 346). However, if the main application image (mosipdev/esignet-with-plugins:develop, line 55) uses a different Java installation path or version, the truststore mount at {{ .Values.customCA.javaHome }}/lib/security/cacerts will fail silently—Java will continue using the default system truststore instead of the custom one.
Consider either:
- Adding a comment in
values.yamlnoting thatjavaHomemust match the Java installation in the main container image, or - Adding runtime validation in the init container to verify
$JAVA_HOME/lib/security/cacertsexists in the target mount location
📝 Suggested documentation addition
aliasPrefix: company-ca
+ ## Must match the Java installation path in the main application container.
+ ## Verify this matches the JAVA_HOME in mosipdev/esignet-with-plugins image.
javaHome: /usr/local/openjdk-11
keystorePassword: changeit🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/esignet/values.yaml` around lines 527 - 549, The values.yaml default for
customCA.javaHome may not match the Java installation in the main app image,
causing the init container to mount/update the wrong truststore; update either
by adding a clear comment near customCA.javaHome (and mention mountPath and that
it must match the main container JAVA_HOME) or add a runtime validation step in
the init container entrypoint (the init container that uses initContainerImage
and writes to {{ .Values.customCA.javaHome }}/lib/security/cacerts) to check
that $JAVA_HOME/lib/security/cacerts exists at the target mountPath and exit
with a clear error if not found so the mismatch is detected early.
Introduce customCA values and init container to import PEM-encoded corporate CA bundles into the Java truststore. Wire extraVolumes and extraVolumeMounts in the deployment template, and update install scripts with a dedicated TLS trust configuration option. Co-authored-by: Ivanmeneges <Ivanmeneges@users.noreply.github.com> Signed-off-by: Cursor Agent <cursoragent@cursor.com>
cursor
Bot
force-pushed
the
cursor/internal-ca-helm-extension-403f
branch
from
31fd675 to
8a8e13a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@helm/esignet/templates/custom-ca-initcontainer.yaml`:
- Line 18: The cp command copying "${JAVA_HOME}/lib/security/cacerts" to
"${KEYSTORE}" can fail with an opaque "No such file or directory" when
customCA.javaHome is wrong; update the initContainer command that contains cp
"${JAVA_HOME}/lib/security/cacerts" "${KEYSTORE}" to first test for the source
file and exit with a descriptive error that includes the resolved
JAVA_HOME/customCA.javaHome (e.g., if [ -f "${JAVA_HOME}/lib/security/cacerts"
]; then cp ...; else echo "ERROR: Java cacerts not found at
${JAVA_HOME}/lib/security/cacerts — ensure customCA.javaHome matches the Java
installation in the image" >&2; exit 1; fi), so operators see the bad javaHome
value and can fix values.yaml.
In `@helm/esignet/templates/deployment.yaml`:
- Around line 75-80: Call the existing validation template and add a runtime
guard so both init containers cannot be rendered together: invoke the
esignet.validateValues template near the top of deployment.yaml (before any
conditional init-container logic) and modify the enable_insecure init-container
block (the include of "common.tplvalues.render" for .Values.initContainers) to
only render when .Values.enable_insecure is true AND .Values.customCA.enabled is
false; also update the esignet.validateValues helper in _helpers.tpl to
return/throw a failure (not just print) when customCA.enabled and
enable_insecure are both true so Helm install/upgrade fails early.
In `@helm/esignet/values.yaml`:
- Around line 371-373: The default value for extraInitContainers is currently an
empty map ({}) but must be an array; update the values.yaml entry named
extraInitContainers to be an empty list ([]) so templates that call
common.tplvalues.render on extraInitContainers produce a YAML list of init
containers instead of a dict, avoiding invalid Kubernetes manifests when
rendering initContainers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ccde635-a505-4632-b21d-183ba2977da5
📒 Files selected for processing (10)
deploy/configure_tls_trust.shdeploy/esignet-with-plugins/install.shdeploy/esignet-with-plugins/values.yamldeploy/esignet/install.shdeploy/esignet/values.yamlhelm/esignet/README.mdhelm/esignet/templates/_helpers.tplhelm/esignet/templates/custom-ca-initcontainer.yamlhelm/esignet/templates/deployment.yamlhelm/esignet/values.yaml
| ALIAS_PREFIX="{{ .Values.customCA.aliasPrefix }}" | ||
| CA_FILE="{{ .Values.customCA.mountPath }}/{{ if .Values.customCA.secretName }}{{ .Values.customCA.secretKey }}{{ else }}{{ .Values.customCA.configMapKey }}{{ end }}" | ||
|
|
||
| cp "${JAVA_HOME}/lib/security/cacerts" "${KEYSTORE}" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Improve error message when javaHome path is incorrect.
If customCA.javaHome doesn't match the actual Java installation in the init container image, the cp command at line 18 will fail with a generic "No such file or directory" error. This makes it difficult for operators to diagnose that they need to update the javaHome value in values.yaml.
💡 Proposed improvement
CA_FILE="{{ .Values.customCA.mountPath }}/{{ if .Values.customCA.secretName }}{{ .Values.customCA.secretKey }}{{ else }}{{ .Values.customCA.configMapKey }}{{ end }}"
+ if [[ ! -f "${JAVA_HOME}/lib/security/cacerts" ]]; then
+ echo "Java cacerts not found at ${JAVA_HOME}/lib/security/cacerts"
+ echo "Verify that customCA.javaHome (${JAVA_HOME}) matches the Java installation in this image"
+ exit 1
+ fi
+
cp "${JAVA_HOME}/lib/security/cacerts" "${KEYSTORE}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cp "${JAVA_HOME}/lib/security/cacerts" "${KEYSTORE}" | |
| CA_FILE="{{ .Values.customCA.mountPath }}/{{ if .Values.customCA.secretName }}{{ .Values.customCA.secretKey }}{{ else }}{{ .Values.customCA.configMapKey }}{{ end }}" | |
| if [[ ! -f "${JAVA_HOME}/lib/security/cacerts" ]]; then | |
| echo "Java cacerts not found at ${JAVA_HOME}/lib/security/cacerts" | |
| echo "Verify that customCA.javaHome (${JAVA_HOME}) matches the Java installation in this image" | |
| exit 1 | |
| fi | |
| cp "${JAVA_HOME}/lib/security/cacerts" "${KEYSTORE}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/esignet/templates/custom-ca-initcontainer.yaml` at line 18, The cp
command copying "${JAVA_HOME}/lib/security/cacerts" to "${KEYSTORE}" can fail
with an opaque "No such file or directory" when customCA.javaHome is wrong;
update the initContainer command that contains cp
"${JAVA_HOME}/lib/security/cacerts" "${KEYSTORE}" to first test for the source
file and exit with a descriptive error that includes the resolved
JAVA_HOME/customCA.javaHome (e.g., if [ -f "${JAVA_HOME}/lib/security/cacerts"
]; then cp ...; else echo "ERROR: Java cacerts not found at
${JAVA_HOME}/lib/security/cacerts — ensure customCA.javaHome matches the Java
installation in the image" >&2; exit 1; fi), so operators see the bad javaHome
value and can fix values.yaml.
Comment on lines
+75
to
80
| {{- if .Values.customCA.enabled }} | ||
| {{- include "esignet.customCAInitContainer" . | nindent 8 }} | ||
| {{- end }} | ||
| {{- if .Values.enable_insecure }} | ||
| {{- include "common.tplvalues.render" (dict "value" .Values.initContainers "context" $) | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Both customCA and enable_insecure init containers can run simultaneously and conflict.
When customCA.enabled and enable_insecure are both set to true, both init containers are rendered (line 75-77 and 78-80) and execute sequentially. Both write to the shared cacerts volume:
- The
customCAinit container (line 76) imports company CA certificates into/cacerts/cacerts. - The
enable_insecureinit container (defined invalues.yaml:351) then runscp /usr/local/openjdk-11/lib/security/cacerts /cacerts, which overwrites the file with the default JVM truststore, removing all the company CA certificates imported by the first init container.
This results in a broken configuration where the Java application cannot trust either the company-internal CA or the self-signed certificate from mosip-api-internal-host.
The validation in _helpers.tpl lines 78-81 would catch this, but as noted in previous reviews, esignet.validateValues is never invoked, so the check doesn't run.
🔧 Recommended fix
The proper fix requires two changes:
- Invoke validation in a rendered template to enforce mutual exclusivity at
helm installtime - Add a runtime guard to prevent rendering both init containers
Option A (recommended): Add validation call and guard the enable_insecure block:
In deployment.yaml:
{{- if .Values.customCA.enabled }}
{{- include "esignet.customCAInitContainer" . | nindent 8 }}
{{- end }}
- {{- if .Values.enable_insecure }}
+ {{- if and .Values.enable_insecure (not .Values.customCA.enabled) }}
{{- include "common.tplvalues.render" (dict "value" .Values.initContainers "context" $) | nindent 8 }}
{{- end }}And in _helpers.tpl, change the validation to fail instead of just printing:
{{- if and .Values.enable_insecure .Values.customCA.enabled -}}
-esignet: customCA.enable_insecure
- enable_insecure and customCA.enabled cannot be used together. Use customCA for company-internal PKI.
+{{- fail "enable_insecure and customCA.enabled cannot be used together. Use customCA for company-internal PKI." -}}
{{- end -}}Then invoke validation in deployment.yaml (add near the top, before any conditionals):
{{- include "esignet.validateValues" . -}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {{- if .Values.customCA.enabled }} | |
| {{- include "esignet.customCAInitContainer" . | nindent 8 }} | |
| {{- end }} | |
| {{- if .Values.enable_insecure }} | |
| {{- include "common.tplvalues.render" (dict "value" .Values.initContainers "context" $) | nindent 8 }} | |
| {{- end }} | |
| {{- if .Values.customCA.enabled }} | |
| {{- include "esignet.customCAInitContainer" . | nindent 8 }} | |
| {{- end }} | |
| {{- if and .Values.enable_insecure (not .Values.customCA.enabled) }} | |
| {{- include "common.tplvalues.render" (dict "value" .Values.initContainers "context" $) | nindent 8 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/esignet/templates/deployment.yaml` around lines 75 - 80, Call the
existing validation template and add a runtime guard so both init containers
cannot be rendered together: invoke the esignet.validateValues template near the
top of deployment.yaml (before any conditional init-container logic) and modify
the enable_insecure init-container block (the include of
"common.tplvalues.render" for .Values.initContainers) to only render when
.Values.enable_insecure is true AND .Values.customCA.enabled is false; also
update the esignet.validateValues helper in _helpers.tpl to return/throw a
failure (not just print) when customCA.enabled and enable_insecure are both true
so Helm install/upgrade fails early.
Comment on lines
+371
to
+373
| ## Additional init containers appended after built-in truststore init containers. | ||
| ## | ||
| extraInitContainers: {} |
There was a problem hiding this comment.
Fix: extraInitContainers should be a list, not a dict.
The deployment template at line 82 renders extraInitContainers using common.tplvalues.render, expecting a YAML list (array) of init container specs. However, the default value is set to {} (empty dict) instead of [] (empty list). If a user leaves this at the default and the template attempts to render it, Kubernetes will reject the manifest because init containers must be an array.
🛠️ Proposed fix
## Additional init containers appended after built-in truststore init containers.
##
-extraInitContainers: {}
+extraInitContainers: []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Additional init containers appended after built-in truststore init containers. | |
| ## | |
| extraInitContainers: {} | |
| ## Additional init containers appended after built-in truststore init containers. | |
| ## | |
| extraInitContainers: [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/esignet/values.yaml` around lines 371 - 373, The default value for
extraInitContainers is currently an empty map ({}) but must be an array; update
the values.yaml entry named extraInitContainers to be an empty list ([]) so
templates that call common.tplvalues.render on extraInitContainers produce a
YAML list of init containers instead of a dict, avoiding invalid Kubernetes
manifests when rendering initContainers.
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
2 participants
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.
Summary
Adds first-class Helm support for trusting a company-internal CA in eSignet deployments, addressing the gap where only the development-oriented
enable_insecureworkaround existed.Changes
Helm chart (
helm/esignet)customCAvalues block to mount a Secret/ConfigMap with PEM-encoded root/intermediate CA certificate(s)cacertstruststore before the app startsextraVolumes/extraVolumeMountsextension points in the deployment templateextraInitContainersfor additional custom init hookscustomCAandenable_insecuretogetherDeploy scripts
deploy/configure_tls_trust.shhelper used by eSignet install scriptsenable_insecure)customCA)Documentation
helm/esignet/README.mdwith usage examplescustomCAsamples in deploy values filesUsage
Notes
enable_insecureremains available for dev/self-signed environments but is mutually exclusive withcustomCASummary by CodeRabbit
New Features
Documentation