Skip to content

style(helm): split helm template | kubeconform pipeline for readability#104

Closed
rguichard wants to merge 1 commit into
feature/pla-1455-move-app-related-deployment-config-into-application-reposfrom
fix/validate-helm-pipeline-readability
Closed

style(helm): split helm template | kubeconform pipeline for readability#104
rguichard wants to merge 1 commit into
feature/pla-1455-move-app-related-deployment-config-into-application-reposfrom
fix/validate-helm-pipeline-readability

Conversation

@rguichard

Copy link
Copy Markdown
Collaborator

Addresses an Aikido finding on PR #90 (validate-helm-charts.sh:121).

Split the single-line helm template . | kubeconform ... > file 2>&1 across continuation lines with a comment. The if ! guard is intentionally kept: under set -o pipefail it detects a failure in either stage, whereas Aikido's suggested bare-command + $? form would abort the function under set -e before the status could be captured. No behaviour change.

🤖 Generated with Claude Code

The single-line `helm template . | kubeconform ... > file 2>&1` was
hard to read. Split it across continuation lines and add a comment.
The `if !` guard is intentionally kept so that under `set -o pipefail`
a failure in either stage is detected; Aikido's suggested bare-command
+ `$?` form would abort the function under `set -e` before the status
could be captured.

No behaviour change. Addresses Aikido finding on validate-helm-charts.sh:121.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread validate-helm-charts.sh
Comment on lines +120 to +121
# resulting manifests with kubeconform. The pipeline's exit status
# reflects kubeconform (the last command), which is what we check.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new comment says pipeline status reflects only kubeconform, but with set -o pipefail failures in helm template also fail the pipeline; this comment describes impossible behavior for this script.

Suggested change
# resulting manifests with kubeconform. The pipeline's exit status
# reflects kubeconform (the last command), which is what we check.
# resulting manifests with kubeconform. With pipefail enabled, the
# pipeline fails if either helm template or kubeconform fails.
Details

✨ AI Reasoning
​The updated explanatory comment asserts that the pipeline status comes from the final stage only. That assumption cannot hold in this script because pipeline failure behavior is globally altered to fail when any stage fails. This creates a clear contradiction between documented logic and actual execution behavior, which can mislead future changes and debugging.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

@rguichard rguichard closed this Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant