add script/docs to download helm-docs binart and guideline to for updating helm charts#769
Open
anandrkskd wants to merge 1 commit intoargoproj-labs:mainfrom
Open
add script/docs to download helm-docs binart and guideline to for updating helm charts#769anandrkskd wants to merge 1 commit intoargoproj-labs:mainfrom
anandrkskd wants to merge 1 commit intoargoproj-labs:mainfrom
Conversation
…ating helm charts Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
anandrkskd
requested review from
chetan-rns,
jannfis,
jgwest and
mikeshng
as code owners
Contributor
📝 WalkthroughWalkthroughThis PR introduces an automated helm-docs installation system by adding a Makefile build target and corresponding shell script that downloads, installs, and validates helm-docs binaries locally, along with contribution guidelines for Helm chart modifications. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
install/helm-repo/docs/contributing.md (1)
150-150: Prefer referencing the pinned version source instead of hardcoding it in prose.Line 150 hardcodes
v1.13.1; this can drift from the installer default. Consider referencingHELM_DOCS_VERSIONinhack/install/install-helm-docs.shinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install/helm-repo/docs/contributing.md` at line 150, The README text hardcodes the helm-docs version ("v1.13.1"); update the contributing docs to reference the pinned variable HELM_DOCS_VERSION instead of a literal version string so it stays in sync with the installer script; replace the hardcoded `v1.13.1` mention with a reference like `HELM_DOCS_VERSION` (from the install-helm-docs.sh script) and add a short note instructing readers to check that variable when changing the pinned helm-docs release.hack/install/install-helm-docs.sh (1)
18-21: Quote expansions and use isolated temp dirs for safer script behavior.Line 18 uses
${BASH_SOURCE}without index/quoting, and lines 46, 50 use unquoted paths. Lines 49–50 and 53 use a fixed/tmpextraction path without a cleanup trap. This can lead to word-splitting issues with filenames containing spaces and leaves temporary files behind.🧹 Suggested refactor
-PROJECT_ROOT=$(cd $(dirname ${BASH_SOURCE})/../..; pwd) +PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" DIST_PATH="${PROJECT_ROOT}/build/bin" mkdir -p "${DIST_PATH}" PATH="${DIST_PATH}:${PATH}" @@ -DOWNLOADS=$(mktemp -d /tmp/downloads.XXXXXXXXX) +DOWNLOADS="$(mktemp -d /tmp/downloads.XXXXXXXXX)" +EXTRACT_DIR="$(mktemp -d /tmp/helm-docs.XXXXXXXXX)" +trap 'rm -rf "${DOWNLOADS}" "${EXTRACT_DIR}"' EXIT @@ -[ -e $DOWNLOADS/${TARGET_FILE} ] || curl -sLf --retry 3 -o $DOWNLOADS/${TARGET_FILE} ${url} +[ -e "${DOWNLOADS}/${TARGET_FILE}" ] || curl -sLf --retry 3 -o "${DOWNLOADS}/${TARGET_FILE}" "${url}" @@ -mkdir -p /tmp/helm-docs-${HELM_DOCS_VERSION#v} -tar -xzf $DOWNLOADS/${TARGET_FILE} -C /tmp/helm-docs-${HELM_DOCS_VERSION#v} +tar -xzf "${DOWNLOADS}/${TARGET_FILE}" -C "${EXTRACT_DIR}" @@ -install -m 0755 /tmp/helm-docs-${HELM_DOCS_VERSION#v}/helm-docs ${DIST_PATH}/helm-docs +install -m 0755 "${EXTRACT_DIR}/helm-docs" "${DIST_PATH}/helm-docs"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/install/install-helm-docs.sh` around lines 18 - 21, PROJECT_ROOT and DIST_PATH variable usages must be made safe by quoting and using the indexed BASH_SOURCE: change $(dirname ${BASH_SOURCE}) to $(dirname "${BASH_SOURCE[0]}"), wrap "${PROJECT_ROOT}/build/bin" and mkdir -p and PATH modifications in quotes, and ensure PATH assignment uses the quoted DIST_PATH. Replace the fixed /tmp extraction directory with a uniquely created temp dir via mktemp -d (assign to a TMPDIR variable), use that TMPDIR for extraction and other temp files, and add a trap to remove TMPDIR on EXIT to guarantee cleanup; update any references used in extraction commands and cleanup logic (the temp dir and extraction calls around the tar/unpack steps) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/install/install-helm-docs.sh`:
- Around line 43-53: Add checksum verification and quote all variable
expansions: download and also fetch checksums.txt (and optionally
checksums.txt.sig) for the release identified by
HELM_DOCS_VERSION/OS/ARCHITECTURE, verify the downloaded ${TARGET_FILE} against
the checksums (and validate checksums.txt with GPG if available), and only
extract/install if the checksum (and signature) verification succeeds; also
change unquoted expansions like $DOWNLOADS, ${TARGET_FILE}, and ${url} (and
other path vars such as ${DIST_PATH}) to quoted forms to prevent
word-splitting/globbing. Ensure the verification step runs before tar -xzf and
install -m 0755 so you only install verified artifacts.
In `@install/helm-repo/docs/contributing.md`:
- Around line 9-19: Add a language hint to the fenced code block containing the
directory tree (the triple-backtick block that starts with
"argocd-agent-agent/") to satisfy markdown lint MD040; change the opening fence
to specify "text" (i.e., ```text) so the directory tree is treated as plain
text.
---
Nitpick comments:
In `@hack/install/install-helm-docs.sh`:
- Around line 18-21: PROJECT_ROOT and DIST_PATH variable usages must be made
safe by quoting and using the indexed BASH_SOURCE: change $(dirname
${BASH_SOURCE}) to $(dirname "${BASH_SOURCE[0]}"), wrap
"${PROJECT_ROOT}/build/bin" and mkdir -p and PATH modifications in quotes, and
ensure PATH assignment uses the quoted DIST_PATH. Replace the fixed /tmp
extraction directory with a uniquely created temp dir via mktemp -d (assign to a
TMPDIR variable), use that TMPDIR for extraction and other temp files, and add a
trap to remove TMPDIR on EXIT to guarantee cleanup; update any references used
in extraction commands and cleanup logic (the temp dir and extraction calls
around the tar/unpack steps) accordingly.
In `@install/helm-repo/docs/contributing.md`:
- Line 150: The README text hardcodes the helm-docs version ("v1.13.1"); update
the contributing docs to reference the pinned variable HELM_DOCS_VERSION instead
of a literal version string so it stays in sync with the installer script;
replace the hardcoded `v1.13.1` mention with a reference like
`HELM_DOCS_VERSION` (from the install-helm-docs.sh script) and add a short note
instructing readers to check that variable when changing the pinned helm-docs
release.
Comment on lines
+43
to
+53
| url=https://github.com/norwoodj/helm-docs/releases/download/${HELM_DOCS_VERSION}/helm-docs_${HELM_DOCS_VERSION#v}_${OS}_${ARCHITECTURE}.tar.gz | ||
|
|
||
| echo "Downloading helm-docs ${HELM_DOCS_VERSION} from ${url}..." | ||
| [ -e $DOWNLOADS/${TARGET_FILE} ] || curl -sLf --retry 3 -o $DOWNLOADS/${TARGET_FILE} ${url} | ||
|
|
||
| echo "Extracting helm-docs..." | ||
| mkdir -p /tmp/helm-docs-${HELM_DOCS_VERSION#v} | ||
| tar -xzf $DOWNLOADS/${TARGET_FILE} -C /tmp/helm-docs-${HELM_DOCS_VERSION#v} | ||
|
|
||
| # The binary is extracted directly, move it to the destination | ||
| install -m 0755 /tmp/helm-docs-${HELM_DOCS_VERSION#v}/helm-docs ${DIST_PATH}/helm-docs |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the current state of the install-helm-docs.sh file
cat -n hack/install/install-helm-docs.sh | head -60Repository: argoproj-labs/argocd-agent
Length of output: 2801
🏁 Script executed:
# Search for any existing integrity checks in the file
rg -n 'checksums|sha256sum|sha512sum|cosign|gpg' hack/install/install-helm-docs.shRepository: argoproj-labs/argocd-agent
Length of output: 52
🏁 Script executed:
# Check what files/assets are available from helm-docs releases to understand if checksums.txt exists
# Let's also check if there are similar patterns in other install scripts that might provide guidance
find hack/install -name "*.sh" -type f | head -10Repository: argoproj-labs/argocd-agent
Length of output: 175
🏁 Script executed:
# Check if there are other security/integrity checks in similar install scripts
rg -l 'sha256|checksum|gpg|cosign' hack/install/Repository: argoproj-labs/argocd-agent
Length of output: 52
🌐 Web query:
helm-docs releases checksums.txt github norwoodj security verification
💡 Result:
norwoodj/helm-docs GitHub releases publish:
checksums.txt(hashes for the release artifacts) andchecksums.txt.sig(a GPG signature overchecksums.txt). (github.com)- The public key to verify that signature is in the repo at
.github/signature.asc. (raw.githubusercontent.com)
Verify a helm-docs release download (recommended)
-
Download from the GitHub Release page the artifact you want plus
checksums.txtandchecksums.txt.sig. (github.com) -
Import the maintainer’s public key:
curl -fsSL https://raw.githubusercontent.com/norwoodj/helm-docs/master/.github/signature.asc | gpg --import- Verify the checksum file is authentic (signed by that key):
gpg --verify checksums.txt.sig checksums.txt- Verify your downloaded artifact matches the signed checksum:
# pick the exact filename you downloaded
sha256sum -c checksums.txt --ignore-missing(You should see OK for your file.)
Extra GitHub-side integrity checks (optional)
- GitHub can show SHA256 digests for release assets (computed at upload time / immutable). (github.blog)
- You can also verify with the GitHub CLI (
gh release verify/gh release verify-asset). (docs.github.com)
Citations:
- 1: https://github.com/norwoodj/helm-docs/releases?utm_source=openai
- 2: https://raw.githubusercontent.com/norwoodj/helm-docs/master/.github/signature.asc
- 3: https://github.com/norwoodj/helm-docs/releases?utm_source=openai
- 4: https://raw.githubusercontent.com/norwoodj/helm-docs/master/.github/signature.asc
- 5: https://github.com/norwoodj/helm-docs/releases?utm_source=openai
- 6: https://github.blog/changelog/2025-06-03-releases-now-expose-digests-for-release-assets/?utm_source=openai
- 7: https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/verifying-the-integrity-of-a-release?utm_source=openai
🏁 Script executed:
# Let's also check if there are any cleanup mechanisms for temp directories
rg -n 'trap|cleanup|rm -rf' hack/install/install-helm-docs.shRepository: argoproj-labs/argocd-agent
Length of output: 52
🏁 Script executed:
# Check the actual quoting state on the flagged lines
sed -n '42,54p' hack/install/install-helm-docs.sh | cat -nRepository: argoproj-labs/argocd-agent
Length of output: 849
Verify release artifact integrity before installing the binary.
Lines 43–53 download and install an executable without checksum verification. The helm-docs upstream publishes both checksums.txt and GPG-signed attestations (checksums.txt.sig) with each release, enabling supply-chain verification. Implement checksum validation at minimum; GPG verification is recommended for stronger assurance.
Additionally, unquoted variable expansions on line 46 ($DOWNLOADS, ${TARGET_FILE}, ${url}) create word-splitting and glob-expansion hazards and should be quoted.
🔐 Suggested hardening
TARGET_FILE=helm-docs_${HELM_DOCS_VERSION#v}_${OS}_${ARCHITECTURE}.tar.gz
url=https://github.com/norwoodj/helm-docs/releases/download/${HELM_DOCS_VERSION}/helm-docs_${HELM_DOCS_VERSION#v}_${OS}_${ARCHITECTURE}.tar.gz
+CHECKSUMS_URL=https://github.com/norwoodj/helm-docs/releases/download/${HELM_DOCS_VERSION}/checksums.txt
echo "Downloading helm-docs ${HELM_DOCS_VERSION} from ${url}..."
-[ -e $DOWNLOADS/${TARGET_FILE} ] || curl -sLf --retry 3 -o $DOWNLOADS/${TARGET_FILE} ${url}
+[ -e "${DOWNLOADS}/${TARGET_FILE}" ] || curl -sLf --retry 3 -o "${DOWNLOADS}/${TARGET_FILE}" "${url}"
+curl -sLf --retry 3 -o "${DOWNLOADS}/checksums.txt" "${CHECKSUMS_URL}"
+grep " ${TARGET_FILE}$" "${DOWNLOADS}/checksums.txt" | sha256sum -c -
echo "Extracting helm-docs..."
-mkdir -p /tmp/helm-docs-${HELM_DOCS_VERSION#v}
-tar -xzf $DOWNLOADS/${TARGET_FILE} -C /tmp/helm-docs-${HELM_DOCS_VERSION#v}
+mkdir -p /tmp/helm-docs-${HELM_DOCS_VERSION#v}
+tar -xzf "${DOWNLOADS}/${TARGET_FILE}" -C /tmp/helm-docs-${HELM_DOCS_VERSION#v}For maximum security, consider verifying the GPG signature on checksums.txt before validating the checksum.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/install/install-helm-docs.sh` around lines 43 - 53, Add checksum
verification and quote all variable expansions: download and also fetch
checksums.txt (and optionally checksums.txt.sig) for the release identified by
HELM_DOCS_VERSION/OS/ARCHITECTURE, verify the downloaded ${TARGET_FILE} against
the checksums (and validate checksums.txt with GPG if available), and only
extract/install if the checksum (and signature) verification succeeds; also
change unquoted expansions like $DOWNLOADS, ${TARGET_FILE}, and ${url} (and
other path vars such as ${DIST_PATH}) to quoted forms to prevent
word-splitting/globbing. Ensure the verification step runs before tar -xzf and
install -m 0755 so you only install verified artifacts.
Comment on lines
+9
to
+19
| ``` | ||
| argocd-agent-agent/ | ||
| ├── Chart.yaml # Chart metadata and version | ||
| ├── values.yaml # Default configuration values | ||
| ├── values.schema.json # JSON schema for values validation | ||
| ├── README.md # Auto-generated documentation | ||
| └── templates/ # Kubernetes resource templates | ||
| ├── _helpers.tpl # Template helpers | ||
| ├── *.yaml # Resource templates | ||
| └── tests/ # Helm test templates | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language hint to the fenced code block to satisfy markdown linting.
Line 9 opens a fence without language (MD040). Use text for the directory tree block.
📝 Suggested fix
-```
+```text
argocd-agent-agent/
├── Chart.yaml # Chart metadata and version
├── values.yaml # Default configuration values
├── values.schema.json # JSON schema for values validation
├── README.md # Auto-generated documentation
└── templates/ # Kubernetes resource templates
├── _helpers.tpl # Template helpers
├── *.yaml # Resource templates
└── tests/ # Helm test templates</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>
[warning] 9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @install/helm-repo/docs/contributing.md around lines 9 - 19, Add a language
hint to the fenced code block containing the directory tree (the triple-backtick
block that starts with "argocd-agent-agent/") to satisfy markdown lint MD040;
change the opening fence to specify "text" (i.e., ```text) so the directory tree
is treated as plain text.
</details>
<!-- fingerprinting:phantom:medusa:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
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
1 participant
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.
assisted-by: cursor
What does this PR do / why we need it:
add script/docs to download helm-docs binart and guideline to for updating helm charts
Which issue(s) this PR fixes:
Fixes #?
How to test changes / Special notes to the reviewer:
Checklist
Summary by CodeRabbit
Release Notes
Documentation
Chores