Conversation
WalkthroughAdds three new Helm charts—othnode, performer, and validator—each with Chart.yaml, README, values, helper templates, and Kubernetes manifests (StatefulSet, Service(s), ServiceAccount, SecretProviderClass for Vault CSI, and Ingress). Templates include conditional logic for secrets, probes, ports, and optional persistent storage. Changes
Sequence Diagram(s)sequenceDiagram
participant Helm as Helm (templates)
participant K8s as Kubernetes API
participant CSI as Secrets Store CSI
participant Vault as Vault
participant Pod as Pod (init + main)
rect rgb(240,248,255)
Helm->>K8s: Render & apply manifests (StatefulSet, Service(s), SecretProviderClass, ServiceAccount, Ingress)
end
rect rgb(245,255,240)
K8s->>CSI: Register SecretProviderClass (provider=vault, role)
CSI->>Vault: Fetch secrets per object path
CSI->>K8s: Expose secrets via projected volume
end
rect rgb(255,250,240)
K8s->>Pod: Start Pod with CSI volume
Pod->>Pod: init container reads CSI secrets, writes /etc/env
Pod->>Pod: main container sources /etc/env and starts app (flags: rpc/p2p/metrics, persistent, etc.)
end
Note right of Pod: PVC attached if persistent storage enabled
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 10
♻️ Duplicate comments (1)
charts/performer/templates/secretproviderclass.yaml (1)
1-25: This is the third identical SecretProviderClass template—consolidate across all charts.This template duplicates:
- charts/validator/templates/secretproviderclass.yaml
- charts/othnode/templates/secretproviderclass.yaml
- charts/performer/templates/secretproviderclass.yaml
All three are identical and should be consolidated into a shared library chart or template. Additionally, apply the same Vault configuration refactor noted in the validator chart review (move hardcoded values to values.yaml).
🧹 Nitpick comments (12)
charts/performer/Chart.yaml (1)
1-8: PinappVersionto a specific release version instead of "latest".Using
appVersion: "latest"prevents reproducible deployments and makes it difficult to track which application version is running in production. Replace with a specific semantic version.apiVersion: v2 name: performer description: A Helm chart for performer type: application version: 0.0.1 maintainers: - name: "mikolajsobolewski" -appVersion: "latest" +appVersion: "0.0.8"charts/othnode/Chart.yaml (1)
1-8: PinappVersionto a specific release version instead of "latest".Using
appVersion: "latest"prevents reproducible deployments and makes it difficult to track which application version is running in production. Replace with a specific semantic version.apiVersion: v2 name: othnode description: A Helm chart for othnode type: application version: 0.0.1 maintainers: - name: "mikolajsobolewski" -appVersion: "latest" +appVersion: "0.0.4"charts/validator/Chart.yaml (1)
1-8: PinappVersionto a specific release version instead of "latest".Using
appVersion: "latest"prevents reproducible deployments and makes it difficult to track which application version is running in production. Replace with a specific semantic version.apiVersion: v2 name: validator description: A Helm chart for validator type: application version: 0.0.1 maintainers: - name: "mikolajsobolewski" -appVersion: "latest" +appVersion: "0.0.8"charts/performer/templates/serviceaccount.yaml (1)
1-10: Consolidate duplicate ServiceAccount template across charts.The ServiceAccount template is identical across validator, performer, and othnode charts. Consider extracting this into a library chart or shared Helm template to reduce duplication and maintenance burden.
This pattern appears in:
- charts/validator/templates/serviceaccount.yaml
- charts/performer/templates/serviceaccount.yaml
- charts/othnode/templates/serviceaccount.yaml (inferred from summary)
You could create a shared library chart or move this template to a common location that all three charts can reference via dependencies.
charts/validator/templates/service.yaml (1)
20-22: Simplify redundant selector labels.Lines 20-21 set both app.kubernetes.io/name and app.kubernetes.io/instance to the same value ($.Release.Name), which is redundant. The statefulset.kubernetes.io/pod-name selector on line 22 is sufficient and more precise for headless per-pod Services.
Consider simplifying:
selector: - app.kubernetes.io/name: {{ $.Release.Name }} - app.kubernetes.io/instance: {{ $.Release.Name }} statefulset.kubernetes.io/pod-name: {{ $.Release.Name }}-{{ $i }}charts/performer/templates/statefulset.yaml (2)
37-44: Document Vault KV v2 secret structure requirement.The jq command on line 43 assumes a specific Vault KV v2 secret structure with nested
.data.datapaths. This should be documented in the chart README or values.yaml to clarify the expected secret format for users.
136-138: Consider adding error handling for the eval command.The command evaluates all files in
/etc/env/*without checking if they exist or contain valid data. If the initContainer fails or produces unexpected output, this could cause the main container to fail silently or behave unexpectedly.Consider adding a check:
- | if [ -f /etc/env/secrets ]; then eval $(cat /etc/env/* | sed 's/^/export /') fi exec node dist/index.jscharts/othnode/templates/statefulset.yaml (3)
37-44: Document Vault KV v2 secret structure requirement.The jq command assumes a specific Vault KV v2 secret structure with nested
.data.datapaths. This should be documented in the chart README to clarify the expected secret format.
204-207: Consider simplifying the complex command line construction.The command line on line 207 concatenates many conditional flags inline within the Helm template. This reduces readability and makes it harder to maintain.
Consider building the command in the shell script itself where you can use more readable conditionals, or break the command into multiple lines for clarity.
32-56: Consider extracting the initContainer to a shared helper.The secret-copy-init initContainer logic is duplicated across all three charts (performer, othnode, validator) with nearly identical implementations. Consider extracting this to a shared Helm library chart or a named template to reduce duplication and improve maintainability.
charts/validator/templates/statefulset.yaml (2)
37-44: Document Vault KV v2 secret structure requirement.The jq command assumes a specific Vault KV v2 secret structure with nested
.data.datapaths. This should be documented in the chart README to clarify the expected secret format.
32-56: Consider extracting the initContainer to a shared helper.This initContainer logic is duplicated across all three charts (performer, othnode, validator). Consider extracting it to a shared Helm library chart or creating a reusable named template to reduce duplication and simplify maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
charts/othnode/Chart.yaml(1 hunks)charts/othnode/README.md(1 hunks)charts/othnode/templates/_helpers.tpl(1 hunks)charts/othnode/templates/ingress.yaml(1 hunks)charts/othnode/templates/secretproviderclass.yaml(1 hunks)charts/othnode/templates/service.yaml(1 hunks)charts/othnode/templates/serviceaccount.yaml(1 hunks)charts/othnode/templates/statefulset.yaml(1 hunks)charts/othnode/values.yaml(1 hunks)charts/performer/Chart.yaml(1 hunks)charts/performer/README.md(1 hunks)charts/performer/templates/_helpers.tpl(1 hunks)charts/performer/templates/secretproviderclass.yaml(1 hunks)charts/performer/templates/serviceaccount.yaml(1 hunks)charts/performer/templates/statefulset.yaml(1 hunks)charts/performer/values.yaml(1 hunks)charts/validator/Chart.yaml(1 hunks)charts/validator/README.md(1 hunks)charts/validator/templates/_helpers.tpl(1 hunks)charts/validator/templates/secretproviderclass.yaml(1 hunks)charts/validator/templates/service.yaml(1 hunks)charts/validator/templates/serviceaccount.yaml(1 hunks)charts/validator/templates/statefulset.yaml(1 hunks)charts/validator/values.yaml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: charts/all
charts/othnode/values.yaml
[error] 9-9: Trailing spaces detected (trailing-spaces) in charts/othnode/values.yaml.
🪛 GitHub Check: test
charts/othnode/values.yaml
[failure] 9-9:
9:27 [trailing-spaces] trailing spaces
charts/validator/values.yaml
[failure] 9-9:
9:27 [trailing-spaces] trailing spaces
[failure] 7-7:
7:62 [trailing-spaces] trailing spaces
charts/performer/values.yaml
[failure] 9-9:
9:27 [trailing-spaces] trailing spaces
[failure] 7-7:
7:61 [trailing-spaces] trailing spaces
🪛 LanguageTool
charts/othnode/README.md
[uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”.
Context: ...| | imagePullSecrets[0].name | string | "github" | | | nameOverride | string | "" |...
(GITHUB)
charts/performer/README.md
[uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”.
Context: ...| | imagePullSecrets[0].name | string | "github" | | | nameOverride | string | "" |...
(GITHUB)
charts/validator/README.md
[uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”.
Context: ...| | imagePullSecrets[0].name | string | "github" | | | nameOverride | string | "" |...
(GITHUB)
🪛 YAMLlint (1.37.1)
charts/performer/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/performer/templates/secretproviderclass.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/validator/templates/secretproviderclass.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/othnode/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/othnode/templates/service.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/performer/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/validator/templates/service.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/validator/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/othnode/templates/ingress.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/othnode/templates/secretproviderclass.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/validator/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/othnode/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (10)
charts/othnode/templates/serviceaccount.yaml (1)
1-10: Conditional ServiceAccount rendering is well-structured.The gating logic properly checks
secretManager.enabledandsecretManager.serviceAccount.create, and the resource correctly references the values and uses the helper template for labels.charts/validator/templates/serviceaccount.yaml (1)
1-10: YAMLlint error is a false positive for Helm template syntax.The error "syntax error: expected the node content, but found '-'" is reported by YAMLlint on
{{- ifdirectives, which is a known limitation of the tool when parsing Helm template syntax. This template is valid.The nested conditions (line 1-2) are intentional: the ServiceAccount is created only if both secret manager is enabled AND the create flag is set.
charts/validator/templates/secretproviderclass.yaml (1)
14-24: Verify secretManager.secrets structure matches values.yaml.The nested loop (lines 17-24) builds the objects list by iterating over
secretManager.secretsexpecting each entry to have.nameand.pathfields. Ensure the corresponding values.yaml defines these fields consistently.Can you verify that all three charts (validator, performer, othnode) define
secretManager.secretswith the expected schema (name and path fields)?charts/othnode/templates/ingress.yaml (1)
1-43: Ingress template is well-structured.The template properly handles:
- Conditional optional fields (annotations, ingressClassName, TLS)
- Nested loops with correct context preservation using
$- Optional pathType support (line 33-35)
- Proper use of helpers for naming and labeling
The YAMLlint error on line 1 is a false positive for Helm template syntax.
Verify that the values.yaml provides the ingress structure with fields: enabled, annotations, className, tls, and hosts (with nested host/path/pathType structure).
charts/performer/values.yaml (2)
50-54: Clarify serviceAccount creation logic.Line 54 sets
create: false, but line 52 definesname: "performer-sa". If creation is disabled, the ServiceAccount must already exist in the target namespace. This could lead to failures if the account doesn't exist.Either:
- Set
create: trueto let Helm create it, or- Document that the ServiceAccount must be pre-created with the specified name
Is the intent to have the ServiceAccount pre-created, or should this be
create: true?
1-59: Overall values.yaml structure is reasonable.The configuration covers necessary deployment aspects: replicas, image, pull secrets, resources, security context, and secret management integration. The structure aligns with the patterns used in validator and othnode charts.
Minor: The CPU and memory requests/limits (100m/128Mi requests, 100m/256Mi limits) are quite low—verify this is appropriate for the performer workload.
charts/othnode/templates/service.yaml (1)
1-72: LGTM!The service template correctly implements both per-pod headless services for StatefulSet pods and a main ClusterIP service. The conditional port exposure based on feature flags (rpc, p2p, metrics) provides good flexibility.
charts/validator/templates/_helpers.tpl (1)
1-49: LGTM!The helper templates follow standard Helm chart patterns and correctly implement naming, labeling, and selector logic with appropriate truncation for Kubernetes label limits.
charts/performer/templates/_helpers.tpl (1)
1-50: LGTM!The helper templates are consistent with standard Helm practices and align well with the validator chart helpers, ensuring consistency across the charts.
charts/othnode/templates/_helpers.tpl (1)
1-60: LGTM!The helper templates follow consistent patterns with the other charts and include an additional
othnode.secretNamehelper for pod-specific secret naming, which is appropriate for this chart's requirements.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
charts/validator/values.yaml (1)
7-7: Trailing whitespace issues remain unresolved from previous review.The GitHub Check (test) previously flagged trailing spaces on lines 7 and 9. These should be removed to maintain clean formatting:
- Line 7: Remove trailing space after
validation-service- Line 9: Remove trailing space after
IfNotPresent- repository: ghcr.io/eq-lab/enjoyoors-avs/validation-service + repository: ghcr.io/eq-lab/enjoyoors-avs/validation-service- pullPolicy: IfNotPresent + pullPolicy: IfNotPresentAlso applies to: 9-9
🧹 Nitpick comments (1)
charts/validator/values.yaml (1)
41-61: Provide guidance on required validator environment variables.The validator section has several empty placeholder strings (lines 46, 48, 50, 52, 54, 58, 60). Consider adding inline documentation or validation to clarify which of these are truly required at deployment time vs. optional configuration.
For example,
MAINNET_CHAIN_ID,MAINNET_GATEWAY_CONTRACT_ADDRESS,L2_CHAIN_ID, andL2_GATEWAY_CONTRACT_ADDRESSlikely need values. Consider:
- Adding a comment indicating which fields are mandatory
- Using comments to document expected formats/value sources
- Potentially adding Helm chart-level validation in
templates/_helpers.tplor validation hooks
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
charts/othnode/values.yaml(1 hunks)charts/performer/README.md(1 hunks)charts/performer/values.yaml(1 hunks)charts/validator/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/othnode/values.yaml
🧰 Additional context used
🪛 LanguageTool
charts/performer/README.md
[uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”.
Context: ...| | imagePullSecrets[0].name | string | "github" | | | nameOverride | string | "" |...
(GITHUB)
🔇 Additional comments (2)
charts/performer/README.md (1)
1-42: README is auto-generated; static analysis flag is expected.The README was generated using helm-docs v1.14.2. The static analysis tool flags line 21 regarding "github" capitalization as "GitHub"—this is an expected artifact of auto-generated documentation and can be safely ignored unless your team regenerates the README and explicitly applies custom capitalization rules to helm-docs.
charts/performer/values.yaml (1)
7-9: Trailing spaces have been resolved.Verification confirms that lines 7 and 9 are now clean with no trailing whitespace.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
charts/performer/templates/statefulset.yaml (2)
37-44: Clarify the secret extraction logic.The initContainer script extracts pod index, conditionally derives a secret name, and uses jq to convert nested JSON to flat key=value pairs. While functional, the logic is somewhat opaque:
- Pod index extraction via sed assumes pod names follow the StatefulSet naming convention.
- The secret name fallback to
performer-secretsis implicit; document this assumption or make it explicit via a values default.- The jq path
.data.data | to_entries[]assumes a specific secret structure; add an inline comment explaining the expected format.Consider adding brief inline comments to explain the contract:
POD_INDEX=$(echo $POD_NAME | sed 's/.*-//') - SECRET_NAME="{{ if and .Values.secretManager.secrets (gt (len .Values.secretManager.secrets) 0) }}{{ (index .Values.secretManager.secrets 0).name }}{{ else }}performer-secrets{{ end }}" + # Default to first secret name from config, or fall back to performer-secrets + SECRET_NAME="{{ if and .Values.secretManager.secrets (gt (len .Values.secretManager.secrets) 0) }}{{ (index .Values.secretManager.secrets 0).name }}{{ else }}performer-secrets{{ end }}" SECRET_FILE="/tmp/secrets/${SECRET_NAME}-${POD_INDEX}-env" mkdir -p /etc/env if [ -f "$SECRET_FILE" ]; then + # Extract nested .data.data object and convert to KEY=VALUE lines jq -r '.data.data | to_entries[] | "\(.key)=\(.value)"' "$SECRET_FILE" > "/etc/env/secrets" fi
154-163: Verify storage configuration defaults in values.yaml.The volumeClaimTemplate references
{{ .Values.storage.storageClassName }}and{{ .Values.storage.size }}without fallback defaults. If these are undefined in values.yaml, the manifest will render with empty values, causing Kubernetes validation failures.Ensure values.yaml provides explicit defaults or add fallback logic:
storageClassName: {{ .Values.storage.storageClassName }} + # storageClassName: {{ .Values.storage.storageClassName | default "standard" }} resources: requests: - storage: {{ .Values.storage.size }} + storage: {{ .Values.storage.size | default "10Gi" }}charts/othnode/templates/statefulset.yaml (1)
202-207: Refactor long command line for maintainability.The command at line 207 is a single long line concatenating many conditional flags, making it difficult to read and maintain. Consider using a multi-line format or a separate configuration helper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/othnode/templates/statefulset.yaml(1 hunks)charts/performer/templates/statefulset.yaml(1 hunks)charts/validator/templates/statefulset.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/othnode/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/performer/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/validator/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (6)
charts/performer/templates/statefulset.yaml (3)
32-34: Good: jq image version is now pinned.The image
ghcr.io/jqlang/jq:1.8.1is pinned to a specific version, addressing the supply-chain risk flagged in the previous review. ✅
65-84: Liveness and readiness probe conditionals are well-implemented.Both probes use nested
andconditionals to ensure they're only rendered when the probe config exists and is explicitly enabled. The default values (paths, ports, delays) follow Kubernetes conventions and provide sensible fallbacks. ✅
23-23: The review comment's premise is incorrect—the ServiceAccount is not conditionally created.The chart does not create the ServiceAccount at all (
serviceAccount.create: falsein values.yaml). This is a valid pattern where the operator provisions the account separately. The review comment incorrectly claims the account is "created conditionally only whensecretManageris enabled," which is not true.The actual question is whether
serviceAccountNameshould be conditional onsecretManager.enabled. Currently:
- StatefulSet is gated on
performer.enabled(line 2)- serviceAccountName references a static value that defaults to
"performer-sa"- If
secretManager.enabled: false, the StatefulSet still references this accountHowever, this depends on the intended design: Is the service account needed only when secretManager is enabled, or is it always required? The review comment does not establish this design intent, only an incorrect assumption about how the account is provisioned.
Likely an incorrect or invalid review comment.
charts/validator/templates/statefulset.yaml (3)
34-34: Good fix: Image version now pinned.The previous review flagged using
stedolan/jq:latestfor supply chain risk. This has been properly addressed by pinning toghcr.io/jqlang/jq:1.8.1.
2-2: YAML linting error is a false positive.The static analysis tool flagged a syntax error at line 2, but this is a known limitation of YAMLlint—it does not understand Helm templating syntax. The conditional
{{- if .Values.validator.enabled }}at the start of the document is valid Helm syntax and will render correctly.
23-23: The review comment is incorrect and should be disregarded.The value
.Values.secretManager.serviceAccount.nameis guaranteed to exist invalues.yamlwith a default of"validator-sa". Additionally, the suggested fix references a helper templatevalidator.serviceAccountNamethat does not exist in the_helpers.tplfile—using it would cause template rendering to fail.The current implementation is safe as-is: the value is always defined with a sensible default, and the ServiceAccount is conditionally created based on the chart configuration in
serviceaccount.yaml.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
charts/performer/templates/statefulset.yaml (2)
137-153: Verify the quote removal logic in the main container entrypoint.Lines 141–152 safely load environment variables from files. However, line 147 removes quotes (
sed 's/^['\''"]//; s/['\''"]$//') from values that jq has already escaped with@sh. Since jq's@shfilter wraps values in quotes for proper escaping, the sed removes the outer quotes.Consider documenting why this step is necessary or simplifying by trusting jq's escaping directly:
# Current approach value=$(echo "$value" | sed 's/^['\''"]//; s/['\''"]$//') export "$key"="$value" # Alternative: Trust jq's @sh escaping export "$key"="$value"If the sed removal is intentional (e.g., to handle a specific secret format), a brief inline comment would clarify intent.
169-178: Verify storage configuration at deployment time.The VolumeClaimTemplate references
.Values.storage.storageClassNameand.Values.storage.size, but there is no validation that these values are set or valid. If a user deploys without configuring storage, the StatefulSet will fail to schedule. Consider adding a chart validation note in values.yaml or the README indicating that storage configuration is required.charts/validator/templates/statefulset.yaml (1)
36-36: Alpine image pinning and YAMLlint false positive.
- Line 36: Alpine 3.18 is reasonably pinned to a point release (not
latest), which is acceptable for reproducible deployments. Consider pinning to a digest (e.g.,alpine:3.18@sha256:...) if your supply-chain hardening policy requires it.- Line 2: The YAMLlint syntax error is a false positive.
{{- if .Values.validator.enabled }}is valid Helm template syntax using whitespace-stripping dashes; YAMLlint doesn't parse Helm templating.Also applies to: 2-2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/othnode/templates/statefulset.yaml(1 hunks)charts/performer/templates/statefulset.yaml(1 hunks)charts/validator/templates/statefulset.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/performer/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/othnode/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/validator/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (7)
charts/performer/templates/statefulset.yaml (3)
1-2: YAMLlint error is likely a false positive.The static analysis tool reports a syntax error at line 2, but this is almost certainly a false positive because YAMLlint does not understand Helm template syntax (
{{-). The YAML structure is correct. You can safely ignore this.
34-59: Environment loading has been properly hardened.The initContainer now safely extracts secrets and exports them as environment variables (lines 141–152), avoiding the previous eval-based vulnerability. The loop properly handles IFS parsing, skips comments/empty lines, and exports variables safely. This is a solid improvement.
45-47: Validate the secret file format assumption.Line 46 uses the jq filter
.data.data | to_entries[], which assumes a nested secret structure. If the secret is structured differently, this will fail silently or produce incorrect results. Ensure that whoever configures the SecretProviderClass and populates secrets adheres to this nested format.charts/validator/templates/statefulset.yaml (1)
34-58: ✅ Shell injection vulnerability properly remediated.The previous CRITICAL issue—unsafe
eval $(cat /etc/env/*)pattern—has been correctly addressed. The new implementation safely loads environment variables:
- Init container (lines 40–47): Uses
jq -rwith the@shfilter to properly shell-escape secret values when writing to/etc/env/secrets.- Main container (lines 137–148): Reads env files line-by-line using
while IFS='=' read -r key valueto parse key=value pairs without invoking a shell on the values, skips empty lines and comments, and uses safeexport "$key"="$value"instead of eval.- Process execution (line 149): Uses
exec node dist/server.jsto ensure the node process replaces the shell without allowing prior command injection.This pattern is secure and matches the safe alternative suggested in the prior review.
Also applies to: 133-149
charts/othnode/templates/statefulset.yaml (3)
42-46: Verify secret file path and structure match SecretProviderClass configuration.Lines 42-43 construct the SECRET_FILE path as
/tmp/secrets/${SECRET_NAME}-${POD_INDEX}-env, and line 46 extracts data from.data.data. These assumptions depend on how the SecretProviderClass is configured and the secret backend structure (Vault, AWS Secrets Manager, etc.).Please verify that:
- The per-replica secret file naming convention (
{secret-name}-{pod-index}-env) matches your SecretProviderClass configuration- The JSON structure
.data.datais correct for your secret backend (it may be.dataor a different path depending on the backend)- If files aren't found or the jq path fails, the init container should report an error instead of silently continuing with no environment variables set
34-36: Good: Safer jq integration approach.Using
alpine:3.18withapk add --no-cache jqis safer than the previous hardcodedstedolan/jq:latestimage and avoids supply chain risks from floating tags.
23-25: Good: ServiceAccount properly conditionally guarded.The serviceAccountName is correctly wrapped in
{{- if .Values.secretManager.enabled }}, preventing deployment failures when secretManager is disabled.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/othnode/templates/statefulset.yaml (1)
36-36: Consider upgrading Alpine base image for init container.Alpine 3.18 (released May 2023) is getting older. Consider updating to a more recent stable version (e.g., 3.20 or latest stable) to benefit from security patches and dependency updates in the apk registry.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/othnode/templates/statefulset.yaml(1 hunks)charts/performer/templates/statefulset.yaml(1 hunks)charts/validator/templates/statefulset.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/othnode/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/validator/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
charts/performer/templates/statefulset.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (8)
charts/performer/templates/statefulset.yaml (4)
1-2: YAMLlint syntax error on line 2 is likely a false positive.The error "expected the node content, but found '-'" is likely because YAMLlint is parsing the raw Helm template as pure YAML without understanding Go template syntax (
{{- if ...}}). This is expected behavior for Helm chart templates. Verify your CI/CD pipeline is configured to skip YAML linting on Helm template files or uses a Helm-aware linter.
34-47: Init container secret extraction logic looks sound, but verify secret file naming assumptions.The init container correctly:
- Installs jq and safely quotes values with
@sh- Extracts pod index from StatefulSet pod name
- Handles missing secret files gracefully (line 45)
- Uses emptyDir for shared /etc/env directory with main container
Verify that the secret file naming pattern
${SECRET_NAME}-${POD_INDEX}-envmatches how secrets are actually stored by your secret provider (CSI volume mounter). This is critical for secrets to be loaded correctly.
137-147: Environment loading logic is safe and idiomatic.The command properly uses shell source (
. "$env_file") instead ofeval, with file existence checks. This is more resilient than the previous unsafe approach and correctly handles jq-quoted values.
163-171: Storage class name has no default value.Line 168 references
{{ .Values.storage.storageClassName }}without a fallback. If this value is not provided invalues.yaml, the rendered template will have an empty string, which may cause the PVC to use the cluster's default storage class (if one exists) or fail to provision storage.Consider either:
- Adding a default in this template:
{{ .Values.storage.storageClassName | default "" }}(uses cluster default)- Documenting that
storage.storageClassNameis required invalues.yaml- Adding a default storage class name in
values.yamlitselfcharts/othnode/templates/statefulset.yaml (1)
1-248: Well-structured StatefulSet template with all past critical issues resolved.This template correctly implements a Kubernetes StatefulSet for the othnode chart with proper security hardening. All five critical issues from previous reviews have been successfully addressed:
- ✅ Init container uses a specific Alpine version (3.18) instead of
stedolan/jq:latest- ✅
serviceAccountNameis properly guarded with a conditional (lines 23–25)- ✅
POD_INDEXextraction is simplified and non-redundant (line 204)- ✅ Shell injection risk eliminated via direct sourcing instead of
eval(line 209)- ✅ Environment variable quoting handled correctly via jq
@sh(line 46) + direct sourceThe volume claim template logic is consistent (lines 216, 237), and conditional resource creation aligns correctly with the enable/disable flags. The init container's graceful handling of missing secrets (line 45) is appropriate.
charts/validator/templates/statefulset.yaml (3)
35-58: Shell injection vulnerability fix is properly implemented.The critical security issue flagged in previous reviews (unsafe
eval $(cat /etc/env/*)pattern) has been successfully addressed. The current implementation:
- Replaces
evalwith a safer loop +sourcepattern (lines 137-142)- Uses jq's
@shfilter to properly escape all secret values (line 46)- Maintains readOnly permissions on secret volumes throughout the chain
This approach prevents shell metacharacters in secret values from being interpreted as commands.
Also applies to: 133-143
60-66: Properly pinned alpine image version and good security defaults.The init container uses
alpine:3.18(specific version, not :latest) and includes proper image pull policy configuration. This addresses the previous version-pinning concern and supports reproducible deployments.
1-2: Now let me inspect the actual file to confirm the context:Based on my verification, the original review comment is accurate and well-reasoned. The
{{-syntax is valid Helm template syntax for whitespace control, and YAMLlint is known to produce errors when linting Helm charts that use this template language, particularly flagging{{-as a parse error even though the YAML is valid.Since it's not possible to perform pure YAML linting on Helm templates, the standard solution is to either ignore the templates folder in the YAMLlint configuration or use Helm-aware linting tools.
Ignore the YAMLlint error—it's a known limitation of linting unprocessed Helm templates.
The syntax at line 2 (
{{- if .Values.validator.enabled }}) is valid Helm template syntax. To resolve this in your CI pipeline, configure your linting to either:
- Exclude the
charts/validator/templates/directory from YAMLlint, or- Use
helm lintinstead of raw YAML validation for template files
1 participant
Summary by CodeRabbit
New Features
Chores