Skip to content

feat: Deploy necessary Argo CD components via Argo CD Agent helm chart#887

Open
anandrkskd wants to merge 8 commits into
argoproj-labs:mainfrom
anandrkskd:embedding-argocd-in-agent-helm-chart
Open

feat: Deploy necessary Argo CD components via Argo CD Agent helm chart#887
anandrkskd wants to merge 8 commits into
argoproj-labs:mainfrom
anandrkskd:embedding-argocd-in-agent-helm-chart

Conversation

@anandrkskd

@anandrkskd anandrkskd commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

assisted-by: cursor, claude-code Opus 4.6 model for generating values.schema.json‎,documentation reviewed by author.

What does this PR do / why we need it:

This change adds an optional argo-cd dependency to the argocd-agent-agent chart (Helm alias argocd), gated by argoCD.enabled. The Argo CD Helm chart version is aligned with github.com/argoproj/argo-cd/v3 in go.mod via a small sync script and CI check.
Maintainer docs and make targets document how to refresh Chart.lock / charts/*.tgz safely.

Which issue(s) this PR fixes:

Fixes #?

How to test changes / Special notes to the reviewer:

  cd install/helm-repo/argocd-agent-agent
  helm dependency build
  helm upgrade --install test-agent . -n argocd --create-namespace \
    --set argoCD.enabled=false
  kubectl get pods,deploy,svc -n argocd
  
  
  helm upgrade --install test-stack . -n argocd --create-namespace \
    --set argoCD.enabled=true
  kubectl get deploy,pods -n argocd

• Confirm redis, repo-server, application-controller, etc. become Running.
• Confirm argocd-server matches intended replica count from argocd.server.replicas in default values.yaml (e.g. 0 if UI/API is intentionally scaled down).

Checklist

  • Documentation update is required by this PR (and has been updated) OR no documentation update is required.

Summary by CodeRabbit

  • New Features

    • Optional bundled Argo CD chart (toggle via argoCD.enabled) and global passthrough settings (global.namespaceOverride, global.imagePullSecrets)
  • Configuration

    • Chart metadata updated with bundled dependency, annotations and appVersion formatting; schema and values extended for argoCD/global options; release notes/templates updated; service account now honors global.imagePullSecrets; helm packaging ignores docs/build artifacts
  • Documentation

    • Install guide updated with bundled Argo CD examples; new maintainer doc on dependency pinning and upgrade workflow
  • Chores

    • Added scripts and Make targets for syncing/updating/validating Helm dependencies and a CI Helm validation step

@coderabbitai

coderabbitai Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds optional bundled Argo CD as a conditional Helm subchart with automated chart-version syncing from go.mod; introduces scripts and Make targets for syncing, dependency management and chart validation; updates chart metadata, templates, schema, values, docs, and CI steps to support and validate the bundled subchart.

Changes

Cohort / File(s) Summary
CI & Build
/.github/workflows/ci.yaml, Makefile
CI now sets up Helm via azure/setup-helm and runs make validate-helm-chart; Makefile adds validate-helm-chart and helm-dependency-update targets.
Helm sync & validation scripts
hack/sync-argo-cd-helm-chart-version.sh, hack/validate-helm-chart.sh, hack/helm-dependency-update.sh
New executable scripts to resolve/sync Argo CD chart version from go.mod, validate chart lint/template rendering (--check mode), and run helm dependency update/build.
Chart metadata & packaging
install/helm-repo/argocd-agent-agent/Chart.yaml, install/helm-repo/argocd-agent-agent/.helmignore
Added argo-cd dependency entry, annotations recording module/chart versions, changed appVersion format, and added .helmignore exclusions.
Chart templates
install/helm-repo/argocd-agent-agent/templates/_helpers.tpl, .../templates/agent-sa.yaml, .../templates/NOTES.txt
Namespace helper now prefers global.namespaceOverride; added argo-cd.namespace helper; ServiceAccount may include global.imagePullSecrets; conditional NOTES block when bundled Argo CD enabled.
Schema & values
install/helm-repo/argocd-agent-agent/values.schema.json, install/helm-repo/argocd-agent-agent/values.yaml
Added argoCD.enabled (default false) and argocd passthrough values; expanded global with namespaceOverride and imagePullSecrets; updated required fields and deprecated top-level namespaceOverride.
Docs
install/helm-repo/argocd-agent-agent/README.md, install/helm-repo/docs/helm-dependencies-and-upgrades.md, install/helm-repo/docs/install-agent.md
Documented bundled Argo CD usage, global value propagation and imagePullSecret guidance, maintainer workflows and upgrade steps, and updated install examples/version.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Maintainer
    participant CI as CI (GitHub Actions)
    participant Script as sync/check script
    participant Helm as Helm CLI
    participant Chart as Chart files

    Dev->>CI: push changes / PR
    CI->>Helm: setup Helm (azure/setup-helm)
    CI->>Script: run sync script (--check or update)
    Script->>Chart: read go.mod and Chart.yaml
    Script->>Helm: query argo Helm repo / resolve chart version
    Script->>Chart: update Chart.yaml annotations/dependency (when updating)
    CI->>Helm: helm dependency build / helm lint / helm template
    Helm->>CI: success/failure
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • jannfis
  • jgwest
  • chetan-rns
  • mikeshng

Poem

🐇 I hopped through go.mod and Chart with care,
I nudged dependencies into their lair,
Secrets tucked, namespaces found their place,
Lint and template checks — a sprightly race,
🥕 CI cheers as charts align with grace.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding optional Argo CD components via the Helm chart with an argoCD.enabled flag.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@anandrkskd anandrkskd changed the title Deploy necessary Argo CD components via Argo CD Agent helm chart feat: Deploy necessary Argo CD components via Argo CD Agent helm chart Apr 13, 2026
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
…on options

Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
…ret with key server.secretKey

Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
@anandrkskd anandrkskd force-pushed the embedding-argocd-in-agent-helm-chart branch from e1d4632 to 64881bd Compare April 20, 2026 08:02
@codecov-commenter

codecov-commenter commented Apr 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.84%. Comparing base (2331cd0) to head (b3dfe6e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #887      +/-   ##
==========================================
+ Coverage   46.83%   46.84%   +0.01%     
==========================================
  Files         122      122              
  Lines       17520    17520              
==========================================
+ Hits         8206     8208       +2     
  Misses       8555     8555              
+ Partials      759      757       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
@anandrkskd anandrkskd force-pushed the embedding-argocd-in-agent-helm-chart branch 2 times, most recently from d956088 to 8d131e8 Compare April 20, 2026 08:31
@anandrkskd anandrkskd marked this pull request as ready for review April 20, 2026 08:35

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
install/helm-repo/argocd-agent-agent/Chart.yaml (1)

13-31: ⚠️ Potential issue | 🟡 Minor

Update the argo-cd-chart-version annotation to match the dependency version.

The annotation argocd-agent.argoproj-labs.io/argo-cd-chart-version is 9.5.1, but dependencies[0].version is 9.5.2. Running hack/sync-argo-cd-helm-chart-version.sh will align both the dependency version and this annotation to the same resolved chart version based on the Argo CD module version in go.mod (currently v3.3.7).

📝 Proposed fix
-  argocd-agent.argoproj-labs.io/argo-cd-chart-version: 9.5.1
+  argocd-agent.argoproj-labs.io/argo-cd-chart-version: 9.5.2
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install/helm-repo/argocd-agent-agent/Chart.yaml` around lines 13 - 31, The
annotation argocd-agent.argoproj-labs.io/argo-cd-chart-version in Chart.yaml is
out of sync (9.5.1) with dependencies[0].version (9.5.2); update that annotation
to 9.5.2 or run hack/sync-argo-cd-helm-chart-version.sh to reconcile the
annotation with the resolved dependency version based on the Argo CD module
version (v3.3.7).
🤖 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/sync-argo-cd-helm-chart-version.sh`:
- Around line 113-125: The --check branch currently validates the pinned
dependency (CURRENT_VER vs RESOLVED_CHART_VER) and the module-version annotation
(argocd-agent.argoproj-labs.io/argo-cd-module-version) but does not validate the
chart-version annotation; add a read of the chart annotation
(argocd-agent.argoproj-labs.io/argo-cd-chart-version), compare it to
RESOLVED_CHART_VER, and fail with a clear error message if they differ (similar
to the existing checks and exit 1), updating the success message to reflect both
checks; reference the CHECK conditional, CURRENT_VER/RESOLVED_CHART_VER
comparison and the ANN-like variable used for annotations to locate where to add
this check.
- Around line 86-87: The helm repo addition in
hack/sync-argo-cd-helm-chart-version.sh currently suppresses errors with "||
true" and can leave a stale alias; update the helm repo add invocation for the
"argo" repo to include --force-update and remove the "|| true" fallback so the
command always replaces any existing "argo" alias (keep the redirected output to
/dev/null 2>&1 as before) and then continue using the existing helm repo update
argo call.

In `@install/helm-repo/argocd-agent-agent/Chart.yaml`:
- Around line 30-31: The Chart.yaml annotation
argocd-agent.argoproj-labs.io/argo-cd-module-version (currently v3.3.7) is out
of sync with the default image tag in values.yaml (argocd.global.image.tag
v3.3.6) while go.mod pins github.com/argoproj/argo-cd/v3 to v3.3.7; fix by
either updating argocd.global.image.tag in values.yaml to v3.3.7 and regenerate
helm-docs (to keep Chart.yaml and values in sync with the module), or downgrade
the dependency in go.mod from v3.3.7 to v3.3.6 so the module and bundled image
match—ensure the chosen change updates both the annotation or values and any
generated docs so all three (Chart.yaml annotation, values.yaml tag, go.mod)
remain consistent.

In `@install/helm-repo/argocd-agent-agent/templates/argocd-secret.yaml`:
- Around line 17-18: The template currently falls back to a non-deterministic
randAlphaNum 20 when lookup fails ($serverSecretKey / randAlphaNum 20), causing
perpetual diffs; change the logic to require an explicit value
(argocd.configs.secret.extra.server.secretkey) or fail render/helm validation
instead of generating randomness—i.e., remove or disable the randAlphaNum
fallback and add a clear validation/error message (or documented deterministic
fallback from .Values) when lookup() does not find an existing Secret so renders
remain idempotent.

In `@install/helm-repo/argocd-agent-agent/values.yaml`:
- Around line 19-20: The Argo CD image tag in values.yaml is pinned to "tag:
v3.3.6" which is out of sync with the chart dependency
(github.com/argoproj/argo-cd/v3) resolved at v3.3.7; update the tag value from
v3.3.6 to v3.3.7 so the bundled image tag matches the dependency version.
- Around line 56-61: The default empty list for global.imagePullSecrets is
overriding subchart values; remove or comment out the line setting
global.imagePullSecrets: [] in values.yaml so the parent chart does not
propagate an empty list to subcharts (leave imagePullSecrets undefined at the
global level and set argocd.global.imagePullSecrets or agent-level
imagePullSecrets where needed).

In `@install/helm-repo/docs/install-agent.md`:
- Around line 91-95: The doc's statement about namespace precedence is
incorrect: update the text so it explains that the chart's namespace helper (see
_helpers.tpl checking global.namespaceOverride) uses global.namespaceOverride
first and only falls back to .Release.Namespace (the value set by helm install
--namespace) when global.namespaceOverride is empty; change the sentence on line
95 to reflect that the --namespace flag is used only when
global.namespaceOverride is not set.

---

Outside diff comments:
In `@install/helm-repo/argocd-agent-agent/Chart.yaml`:
- Around line 13-31: The annotation
argocd-agent.argoproj-labs.io/argo-cd-chart-version in Chart.yaml is out of sync
(9.5.1) with dependencies[0].version (9.5.2); update that annotation to 9.5.2 or
run hack/sync-argo-cd-helm-chart-version.sh to reconcile the annotation with the
resolved dependency version based on the Argo CD module version (v3.3.7).
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 816ef54c-c72b-47dd-93e3-bb1e00fad0ad

📥 Commits

Reviewing files that changed from the base of the PR and between 2331cd0 and 8d131e8.

⛔ Files ignored due to path filters (1)
  • install/helm-repo/argocd-agent-agent/Chart.lock is excluded by !**/*.lock
📒 Files selected for processing (17)
  • .github/workflows/ci.yaml
  • Makefile
  • hack/helm-dependency-update.sh
  • hack/sync-argo-cd-helm-chart-version.sh
  • hack/validate-helm-chart.sh
  • install/helm-repo/argocd-agent-agent/.helmignore
  • install/helm-repo/argocd-agent-agent/Chart.yaml
  • install/helm-repo/argocd-agent-agent/README.md
  • install/helm-repo/argocd-agent-agent/charts/argo-cd-9.5.2.tgz
  • install/helm-repo/argocd-agent-agent/templates/NOTES.txt
  • install/helm-repo/argocd-agent-agent/templates/_helpers.tpl
  • install/helm-repo/argocd-agent-agent/templates/agent-sa.yaml
  • install/helm-repo/argocd-agent-agent/templates/argocd-secret.yaml
  • install/helm-repo/argocd-agent-agent/values.schema.json
  • install/helm-repo/argocd-agent-agent/values.yaml
  • install/helm-repo/docs/helm-dependencies-and-upgrades.md
  • install/helm-repo/docs/install-agent.md

Comment thread hack/sync-argo-cd-helm-chart-version.sh Outdated
Comment thread hack/sync-argo-cd-helm-chart-version.sh
Comment thread install/helm-repo/argocd-agent-agent/Chart.yaml Outdated
Comment thread install/helm-repo/argocd-agent-agent/templates/argocd-secret.yaml Outdated
Comment thread install/helm-repo/argocd-agent-agent/values.yaml Outdated
Comment on lines +56 to +61
# -- Helm `global` values merged into subcharts. Set `global.imagePullSecrets` when you need registry credentials on the agent ServiceAccount and on the bundled argo-cd chart (`argoCD.enabled` is true).
global:
# -- Override namespace for the agent AND the bundled argo-cd subchart.
# This is the recommended way to deploy both charts into a non-release namespace.
namespaceOverride: ""
imagePullSecrets: []

@coderabbitai coderabbitai Bot Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

How does Helm merge parent chart global values into aliased subcharts when both parent global.imagePullSecrets and argocd.global.imagePullSecrets are set?

💡 Result:

Helm merges parent chart global values into aliased subcharts by making .Values.global available to all subcharts, including aliased ones. The parent's global.imagePullSecrets is accessible in the subchart templates as {{ .Values.global.imagePullSecrets }}. When both parent global.imagePullSecrets and argocd (aliased subchart) global.imagePullSecrets are set, they are separate keys in the values tree: - Parent's .Values.global.imagePullSecrets - Subchart's .Values.global.imagePullSecrets (from argocd subchart's own global values, merged with any alias-specific overrides under the alias name) There is no automatic merge between them; they coexist independently. Templates must explicitly reference .Values.global.imagePullSecrets, which refers to the global values scoped to that chart (parent globals for parent templates, subchart globals for subchart templates). For aliased subcharts, values under the alias name in parent values.yaml (e.g., argocd: { global: { imagePullSecrets: [...] } }) override/merge with the subchart's own values using Helm's standard deep-merge strategy (maps merge recursively, lists replace entirely). In the Argo CD Helm chart (which uses global.imagePullSecrets), setting it in the parent chart's global propagates to all components including subcharts if their templates use {{ .Values.global.imagePullSecrets }}. Setting it under the alias provides alias-specific overrides. To combine both, templates would need custom logic like append or set functions, as Helm does not auto-merge lists across these scopes.

Citations:


🏁 Script executed:

# First, let's find and examine the values.yaml file
find . -path "*/install/helm-repo/argocd-agent-agent/values.yaml" -type f

Repository: argoproj-labs/argocd-agent

Length of output: 120


🏁 Script executed:

# Let's also search for any references to imagePullSecrets in the helm chart files
rg "imagePullSecrets" --type yaml -B 2 -A 2

Repository: argoproj-labs/argocd-agent

Length of output: 1541


🏁 Script executed:

# Check if there's any Chart.yaml or dependencies definition
find . -path "*/install/helm-repo/argocd-agent-agent/*" -type f -name "*.yaml" | head -20

Repository: argoproj-labs/argocd-agent

Length of output: 1465


🏁 Script executed:

# Check Chart.yaml for dependencies
cat install/helm-repo/argocd-agent-agent/Chart.yaml

Repository: argoproj-labs/argocd-agent

Length of output: 1199


🏁 Script executed:

# Let's also check if there's a Chart.lock file
find . -path "*/install/helm-repo/argocd-agent-agent/*" -name "Chart.lock"

Repository: argoproj-labs/argocd-agent

Length of output: 119


🏁 Script executed:

# Check the argocd subchart values usage in templates
rg "argocd\.global" . --type yaml -B 2 -A 2

Repository: argoproj-labs/argocd-agent

Length of output: 52


🏁 Script executed:

# Look for any documentation about this behavior
find . -name "*.md" -o -name "*.txt" | xargs grep -l "imagePullSecrets" 2>/dev/null | head -5

Repository: argoproj-labs/argocd-agent

Length of output: 219


🏁 Script executed:

# Check the documentation about imagePullSecrets
cat install/helm-repo/docs/install-agent.md | grep -A 10 -B 10 "imagePullSecrets"

Repository: argoproj-labs/argocd-agent

Length of output: 2596


🏁 Script executed:

# Check the helm dependencies documentation
cat install/helm-repo/docs/helm-dependencies-and-upgrades.md | grep -A 10 -B 10 "imagePullSecrets"

Repository: argoproj-labs/argocd-agent

Length of output: 2858


🏁 Script executed:

# Let's look at the full values.yaml to understand the argoCD structure
head -150 install/helm-repo/argocd-agent-agent/values.yaml

Repository: argoproj-labs/argocd-agent

Length of output: 5515


🏁 Script executed:

# Check how argoCD values are structured in values.yaml
rg "^argoCD:" . --type yaml -A 30

Repository: argoproj-labs/argocd-agent

Length of output: 3027


Remove the default empty list from global.imagePullSecrets.

The chart's documentation explicitly warns against setting global.imagePullSecrets: [] as a default because it overrides argocd.global.imagePullSecrets when the bundled Argo CD chart is enabled. Helm propagates parent global values to subcharts, and lists are replaced entirely (not merged). With the current empty list default, users who set only argocd.global.imagePullSecrets for the bundled chart will find that setting ignored due to the empty parent override—contradicting the documented "bundled chart only" pull-secret path explained in helm-dependencies-and-upgrades.md and install-agent.md.

Comment out the line or remove it entirely so that when undefined, it does not propagate an empty override to the subchart.

Proposed fix
 global:
   # -- Override namespace for the agent AND the bundled argo-cd subchart.
   # This is the recommended way to deploy both charts into a non-release namespace.
   namespaceOverride: ""
-  imagePullSecrets: []
+  # imagePullSecrets:
+  #   - name: my-registry-secret
📝 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
# -- Helm `global` values merged into subcharts. Set `global.imagePullSecrets` when you need registry credentials on the agent ServiceAccount and on the bundled argo-cd chart (`argoCD.enabled` is true).
global:
# -- Override namespace for the agent AND the bundled argo-cd subchart.
# This is the recommended way to deploy both charts into a non-release namespace.
namespaceOverride: ""
imagePullSecrets: []
# -- Helm `global` values merged into subcharts. Set `global.imagePullSecrets` when you need registry credentials on the agent ServiceAccount and on the bundled argo-cd chart (`argoCD.enabled` is true).
global:
# -- Override namespace for the agent AND the bundled argo-cd subchart.
# This is the recommended way to deploy both charts into a non-release namespace.
namespaceOverride: ""
# imagePullSecrets:
# - name: my-registry-secret
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install/helm-repo/argocd-agent-agent/values.yaml` around lines 56 - 61, The
default empty list for global.imagePullSecrets is overriding subchart values;
remove or comment out the line setting global.imagePullSecrets: [] in
values.yaml so the parent chart does not propagate an empty list to subcharts
(leave imagePullSecrets undefined at the global level and set
argocd.global.imagePullSecrets or agent-level imagePullSecrets where needed).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@anandrkskd Is this comment actually legit?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes but we will be using global.imagepullsecret, this will propagate both agent and argocd sub-chart at the same time.
We will not expose argocd.global.imagepullsecret for users to use.

I haven't tested it with imagepullsecret vs global.imagepullsecret. But we can document it.

Comment thread install/helm-repo/docs/install-agent.md Outdated
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
@anandrkskd anandrkskd force-pushed the embedding-argocd-in-agent-helm-chart branch from 8d131e8 to b160cc2 Compare April 20, 2026 11:25

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
install/helm-repo/argocd-agent-agent/values.yaml (1)

50-60: ⚠️ Potential issue | 🟠 Major

Leave shared global.imagePullSecrets unset by default.

The default empty list still contradicts Lines 57-59: Helm propagates parent global values into subcharts, so global.imagePullSecrets: [] can mask argocd.global.imagePullSecrets when users only want bundled Argo CD credentials. This was already flagged; the current file still has the same default.

Proposed fix
 global:
   # -- Override namespace for the agent AND the bundled argo-cd subchart.
   # This is the recommended way to deploy both charts into a non-release namespace.
   namespaceOverride: ""
   # -- Image pull secrets applied to the agent AND the bundled argo-cd subchart.
   # Helm propagates global values into subcharts, so this list overrides
   # argocd.global.imagePullSecrets. Set credentials here when both charts
   # need the same pull secret; use argocd.global.imagePullSecrets only if the
   # subchart requires a different secret than the agent.
-  imagePullSecrets: []
+  # imagePullSecrets:
+  #   - name: my-registry-secret

Helm documents parent globals as shared with subcharts: https://helm.sh/docs/chart_template_guide/subcharts_and_globals/.

In Helm 3/4, if a parent chart defines `global.imagePullSecrets: []` and an aliased subchart is configured with `argocd.global.imagePullSecrets: [{name: my-secret}]`, which value is visible to the subchart at `.Values.global.imagePullSecrets`?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install/helm-repo/argocd-agent-agent/values.yaml` around lines 50 - 60, The
default empty list for global.imagePullSecrets is masking subchart overrides;
remove or unset global.imagePullSecrets in values.yaml (instead of leaving
imagePullSecrets: []) so argocd.global.imagePullSecrets can be seen by the
aliased argo-cd subchart; locate the global block (global.namespaceOverride and
global.imagePullSecrets) and either delete the imagePullSecrets key or set it to
null and add a comment explaining that subcharts (e.g.,
argocd.global.imagePullSecrets) should be used when only the bundled Argo CD
needs credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@install/helm-repo/docs/install-agent.md`:
- Around line 36-44: The fenced code blocks around the two helm install commands
are missing a language tag and should be marked as shell to satisfy markdownlint
MD040; update the two triple-backtick fences surrounding the lines containing
"helm install argocd-agent
ghcr.io/argoproj-labs/argocd-agent/argocd-agent-agent-helm --version 0.2.0
--namespace=argocd --create-namespace" and the variant with "--set
global.namespaceOverride=argocd" to start with ```shell instead of ```, leaving
the command text unchanged.
- Line 19: The example uses the wrong values path for the bundled argo-helm
subchart: replace the invalid argocd.server.insecure with the correct
configs.params.server.insecure under the argocd key and also enable a server
replica (server.replicas) for a lab setup; specifically, when toggling
argoCD.enabled=true, pass Helm values under argocd with
configs.params.server.insecure=true and set argocd.server.replicas=1 so the Argo
CD server actually runs.

---

Duplicate comments:
In `@install/helm-repo/argocd-agent-agent/values.yaml`:
- Around line 50-60: The default empty list for global.imagePullSecrets is
masking subchart overrides; remove or unset global.imagePullSecrets in
values.yaml (instead of leaving imagePullSecrets: []) so
argocd.global.imagePullSecrets can be seen by the aliased argo-cd subchart;
locate the global block (global.namespaceOverride and global.imagePullSecrets)
and either delete the imagePullSecrets key or set it to null and add a comment
explaining that subcharts (e.g., argocd.global.imagePullSecrets) should be used
when only the bundled Argo CD needs credentials.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: a5007d44-a8be-4a17-b807-2693342cf515

📥 Commits

Reviewing files that changed from the base of the PR and between 8d131e8 and b160cc2.

📒 Files selected for processing (5)
  • hack/sync-argo-cd-helm-chart-version.sh
  • install/helm-repo/argocd-agent-agent/README.md
  • install/helm-repo/argocd-agent-agent/templates/NOTES.txt
  • install/helm-repo/argocd-agent-agent/values.yaml
  • install/helm-repo/docs/install-agent.md
✅ Files skipped from review due to trivial changes (2)
  • install/helm-repo/argocd-agent-agent/templates/NOTES.txt
  • install/helm-repo/argocd-agent-agent/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • hack/sync-argo-cd-helm-chart-version.sh

Comment thread install/helm-repo/docs/install-agent.md Outdated
Comment thread install/helm-repo/docs/install-agent.md Outdated
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
install/helm-repo/docs/install-agent.md (1)

19-19: ⚠️ Potential issue | 🟡 Minor

Bundled Argo CD “lab setup” example is incomplete for a usable UI/API.

At Line 19, the example enables bundled Argo CD but does not set argocd.server.replicas; default values keep it at 0, so argocd-server won’t run. Add --set argocd.server.replicas=1 to the lab example to avoid a confusing first install experience.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install/helm-repo/docs/install-agent.md` at line 19, The bundled Argo CD lab
example enables the subchart via argoCD.enabled=true but omits
argocd.server.replicas, which defaults to 0 and prevents argocd-server from
running; update the example invocation that sets argoCD.enabled to also set
argocd.server.replicas=1 (e.g., include --set argocd.server.replicas=1 alongside
--set argoCD.enabled=true and any argocd.* overrides) so the server pod is
created for a usable UI/API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@install/helm-repo/docs/install-agent.md`:
- Around line 29-33: Clarify that the Helm flag --create-namespace only creates
the release namespace passed via --namespace and does NOT create the namespace
referenced by --set global.namespaceOverride; update the sentences around `--set
global.namespaceOverride=agent-namespace`, `--namespace`, and
`--create-namespace` to state that users must either pre-create the override
namespace (the value passed to global.namespaceOverride) or make the Helm
release namespace match the override value so no separate pre-creation is
required.

---

Duplicate comments:
In `@install/helm-repo/docs/install-agent.md`:
- Line 19: The bundled Argo CD lab example enables the subchart via
argoCD.enabled=true but omits argocd.server.replicas, which defaults to 0 and
prevents argocd-server from running; update the example invocation that sets
argoCD.enabled to also set argocd.server.replicas=1 (e.g., include --set
argocd.server.replicas=1 alongside --set argoCD.enabled=true and any argocd.*
overrides) so the server pod is created for a usable UI/API.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d4f8d7d-7a28-4668-90a4-2b1f7a13b7a2

📥 Commits

Reviewing files that changed from the base of the PR and between b160cc2 and f83d5db.

📒 Files selected for processing (5)
  • hack/sync-argo-cd-helm-chart-version.sh
  • install/helm-repo/argocd-agent-agent/Chart.yaml
  • install/helm-repo/argocd-agent-agent/README.md
  • install/helm-repo/argocd-agent-agent/templates/NOTES.txt
  • install/helm-repo/docs/install-agent.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • install/helm-repo/argocd-agent-agent/templates/NOTES.txt
  • install/helm-repo/argocd-agent-agent/README.md
  • hack/sync-argo-cd-helm-chart-version.sh

Comment thread install/helm-repo/docs/install-agent.md Outdated
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
install/helm-repo/docs/install-agent.md (2)

88-92: Clarify the complete namespace precedence order.

The documentation states that global.namespaceOverride takes precedence over .Release.Namespace, which is correct. However, it doesn't mention that the deprecated top-level namespaceOverride takes precedence over global.namespaceOverride. According to the helper template in _helpers.tpl (lines 156-163), the precedence is:

  1. namespaceOverride (top-level, deprecated)
  2. global.namespaceOverride
  3. .Release.Namespace (from --namespace flag)

Consider clarifying this to help users understand what happens if they set both values.

Suggested revision
-The Kubernetes namespace where the agent's resources (Deployment, Service, ConfigMap, etc.) and the bundled Argo CD subchart will be deployed. When set, this takes precedence over the `helm install --namespace` flag (`.Release.Namespace`), which is only used as a fallback when `global.namespaceOverride` is empty. Preferred over the top-level `namespaceOverride` which only affects the agent chart.
+The Kubernetes namespace where the agent's resources (Deployment, Service, ConfigMap, etc.) and the bundled Argo CD subchart will be deployed. When set, this takes precedence over the `helm install --namespace` flag (`.Release.Namespace`). Note: the deprecated top-level `namespaceOverride` takes precedence over `global.namespaceOverride` for agent resources (but does not propagate to the bundled Argo CD subchart), so `global.namespaceOverride` is preferred when using the bundled subchart.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install/helm-repo/docs/install-agent.md` around lines 88 - 92, Update the
namespace precedence documentation to list the full order: first the deprecated
top-level variable namespaceOverride, then global.namespaceOverride, then
.Release.Namespace (the helm --namespace fallback); reference the helper
template (the helper in _helpers.tpl that implements this precedence) and
mention that namespaceOverride is deprecated so users should prefer
global.namespaceOverride to avoid confusion when both are set.

19-19: Complete the registry path example.

The example includes --set argocd.redis.image.repository=quay.io/.. with an incomplete placeholder path. Either provide a complete example (e.g., quay.io/redis/redis) or simplify the example to avoid confusion.

Suggested revision
-The chart can install the upstream [argo-helm](https://github.com/argoproj/argo-helm) `argo-cd` chart in the same release by setting `argoCD.enabled=true`. Pass Helm values for that subchart under the `argocd` key (for example `--set argoCD.enabled=true --set argocd.redis.image.repository=quay.io/..` for a lab setup). When `argoCD.enabled` is false (default), install Argo CD separately and keep the agent pointed at its Redis service.
+The chart can install the upstream [argo-helm](https://github.com/argoproj/argo-helm) `argo-cd` chart in the same release by setting `argoCD.enabled=true`. Pass Helm values for that subchart under the `argocd` key (for example `--set argoCD.enabled=true` for a lab setup, or `--set argoCD.enabled=true --set argocd.redis.image.repository=quay.io/redis/redis` to override the Redis image). When `argoCD.enabled` is false (default), install Argo CD separately and keep the agent pointed at its Redis service.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install/helm-repo/docs/install-agent.md` at line 19, The example CLI flag
uses an incomplete registry path for argocd.redis.image.repository; update the
example where argoCD.enabled and argocd are discussed to use a complete image
repository (e.g., replace `--set argocd.redis.image.repository=quay.io/..` with
a concrete path like `--set argocd.redis.image.repository=quay.io/redis/redis`),
or remove the specific registry fragment and show a generic placeholder such as
`--set argocd.redis.image.repository=<registry>/<repo>` so `argoCD.enabled` and
`argocd.redis.image.repository` are clear and unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@install/helm-repo/docs/install-agent.md`:
- Around line 88-92: Update the namespace precedence documentation to list the
full order: first the deprecated top-level variable namespaceOverride, then
global.namespaceOverride, then .Release.Namespace (the helm --namespace
fallback); reference the helper template (the helper in _helpers.tpl that
implements this precedence) and mention that namespaceOverride is deprecated so
users should prefer global.namespaceOverride to avoid confusion when both are
set.
- Line 19: The example CLI flag uses an incomplete registry path for
argocd.redis.image.repository; update the example where argoCD.enabled and
argocd are discussed to use a complete image repository (e.g., replace `--set
argocd.redis.image.repository=quay.io/..` with a concrete path like `--set
argocd.redis.image.repository=quay.io/redis/redis`), or remove the specific
registry fragment and show a generic placeholder such as `--set
argocd.redis.image.repository=<registry>/<repo>` so `argoCD.enabled` and
`argocd.redis.image.repository` are clear and unambiguous.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4e5c4828-246c-446d-b8f9-10a368521e45

📥 Commits

Reviewing files that changed from the base of the PR and between f83d5db and b3dfe6e.

📒 Files selected for processing (1)
  • install/helm-repo/docs/install-agent.md

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.

3 participants