Skip to content

chore(helm): move REST charts from rest-api/helm to helm/rest (#1755)#2155

Open
lachen-nv wants to merge 8 commits into
NVIDIA:mainfrom
lachen-nv:chore/move-rest-helm-charts-1755
Open

chore(helm): move REST charts from rest-api/helm to helm/rest (#1755)#2155
lachen-nv wants to merge 8 commits into
NVIDIA:mainfrom
lachen-nv:chore/move-rest-helm-charts-1755

Conversation

@lachen-nv

Copy link
Copy Markdown
Contributor

Closes #1755 — relocate the REST helm charts now that the repos are merged.

What moved

rest-api/helm/charts/{nico-rest,nico-rest-site-agent}helm/rest/{nico-rest,nico-rest-site-agent} (+ their shared README.md).

Why helm/rest/ and not helm/charts/

helm/ is the core nico umbrella chart; anything placed under helm/charts/ is auto-bundled and rendered as a subchart of nico. The REST charts are published standalone (each pushed to NGC individually), so they're placed under helm/rest/ and excluded from the umbrella package via .helmignore (same mechanism already used for examples/). This keeps the deployment/publishing model unchanged — just a relocation.

Changes

  • git mv the two charts (self-contained; nico-rest keeps its vendored subcharts) + README (paths updated).
  • helm/.helmignore: add rest/ so the nico umbrella package excludes the standalone REST charts.
  • rest-helm-workflows.yml: chart paths rest-api/helm/charts/*helm/rest/* (still validated + pushed standalone by REST CI).
  • CI gates kept symmetric so the charts still get CI'd at their new home:
    • REST gate (rest-ci.yml): add helm/rest/** → editing a REST chart triggers REST CI (which publishes it).
    • Core gate (ci.yaml): add !helm/rest/** → core pipeline isn't triggered redundantly (mirrors how rest-api/** is excluded).

Validation

  • helm lint helm/rest/nico-rest + helm/rest/nico-rest-site-agent → 0 failed.
  • helm package helm/ → resulting nico umbrella .tgz contains no rest/ (.helmignore confirmed working).
  • No remaining rest-api/helm references in the repo; workflow YAML parses clean.

@lachen-nv lachen-nv requested review from a team as code owners June 3, 2026 08:22
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5e5ad9fe-3734-488c-9e60-afcb571431b9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

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

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

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-06-03 08:24:13 UTC | Commit: e10f941

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
nico-flow 116 13 50 41 4 8
nico-nsm 133 11 45 66 11 0
nico-psm 118 13 52 41 4 8
nico-rest-api 182 16 84 67 7 8
nico-rest-cert-manager 95 5 47 32 3 8
nico-rest-db 116 13 50 41 4 8
nico-rest-site-agent 115 13 50 41 3 8
nico-rest-site-manager 102 6 48 37 3 8
nico-rest-workflow 118 13 52 41 4 8
TOTAL 1095 103 478 407 43 64

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

@lachen-nv lachen-nv requested a review from Coco-Ben as a code owner June 3, 2026 09:17
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

@thossain-nv thossain-nv 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.

Looks good, thanks for this @lachen-nv

@nv-dmendoza nv-dmendoza left a comment

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.

LGTM. Most of the edits were renames for the pull up of helm

@shayan1995

Copy link
Copy Markdown
Contributor

@lachen-nv could you please make sure the setup.sh path is unaffected by these changes.

@lachen-nv lachen-nv requested a review from shayan1995 as a code owner June 5, 2026 03:31
@lachen-nv

Copy link
Copy Markdown
Contributor Author

@lachen-nv could you please make sure the setup.sh path is unaffected by these changes.

Thanks @shayan1995, I verified this and updated the setup/preflight paths after merging latest main.

setup.sh now keeps the REST source tree and REST Helm chart paths separate:

  • NICO_REST_DIR still points to rest-api/ for scripts, kustomize manifests, and temporal-helm.
  • NICO_REST_HELM_DIR points to the moved charts under helm/rest/.

The nico-rest and nico-rest-site-agent installs now use NICO_REST_HELM_DIR, so the chart move should not break setup.sh. I also updated the reference install docs and checked that there are no remaining stale rest-api/helm/charts / helm/charts/nico-rest references.

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.

Merge helm-charts for rest-api into helm/

6 participants