feat(pki): add OpenBao ClusterIssuer chart - #537
Conversation
📝 WalkthroughWalkthroughAdds the ChangesNVCF PKI Helm chart
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HelmValues
participant HelmRenderer
participant ClusterIssuer
HelmValues->>HelmRenderer: provide clusterIssuer settings
HelmRenderer->>ClusterIssuer: render enabled Vault-backed manifest
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
af0db12 to
d0295e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/nvcf-pki/AGENTS.md`:
- Around line 6-15: Update the AGENTS.md section around the existing Validate
commands to document applicable code-style rules for YAML, Helm templates, and
shell scripts, including a concrete pointer to the governing repository guidance
when available. Keep the existing validation commands unchanged and add only
subtree-specific conventions needed for contributors working under nvcf-pki.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4dfa64ef-d4ea-4c01-8cc6-53cebd08f0de
📒 Files selected for processing (9)
deploy/helm/nvcf-pki/AGENTS.mddeploy/helm/nvcf-pki/CLAUDE.mddeploy/helm/nvcf-pki/Chart.yamldeploy/helm/nvcf-pki/scripts/check-render.shdeploy/helm/nvcf-pki/templates/clusterissuer.yamldeploy/helm/nvcf-pki/values.yamldocs/user/manifest.mddocs/version-catalog/main.yamltools/ci/github-release-subprojects.json
Signed-off-by: Mike Camp <mcamp@nvidia.com>
d0295e4 to
03287fe
Compare
Signed-off-by: Stephanie Baum <sbaum@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/nvcf-pki/README.md`:
- Around line 94-102: The operational commands in the README hardcode
nvcf-openbao-pki instead of honoring the configurable clusterIssuer.name value.
Update the issuer verification commands and the additionally referenced deletion
commands to use a clear clusterIssuer.name placeholder or an ISSUER_NAME
variable consistently.
- Around line 148-155: Add the required git diff --check command to the Local
validation steps in the nvcf-pki README, alongside the existing Helm lint,
template, and render checks, and specify that it runs from the repository root.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f0c1f86d-5f62-4407-8379-726867171079
📒 Files selected for processing (1)
deploy/helm/nvcf-pki/README.md
|
🎉 This PR is included in version helm-nvcf-pki-v0.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
TL;DR
Add an optional
helm-nvcf-pkichart that provisions the OpenBao-backed cert-managerClusterIssuerrequired by the self-managed LLM TLS path. Register the chart for semantic release and add version0.1.0to the public artifact catalog.Additional Details
The LLM request router can create a
Certificatethat referencesClusterIssuer/nvcf-openbao-pki, but the stack does not currently publish a chart that owns that issuer. This leaves the certificate unissued when the referenced issuer is absent.The new chart:
ClusterIssuerwhen enabled;This is the publication-only PR. Stack consumption remains out of scope until
helm-nvcf-pki:0.1.0is available from the normal chart source.Customer Release Notes
Adds an optional Helm chart for provisioning the OpenBao-backed ClusterIssuer used by NVCF service TLS.
Plan Summary
The chart adds one optional cluster-scoped cert-manager resource. It adds no pods, images, Secrets, or runtime application dependencies. The default configuration renders no Kubernetes resources.
Usage
Enable
clusterIssuer.enabledand provide the issuer name, OpenBao server and signing path, JWT authentication settings, and cert-manager service account identity. The self-managed stack integration will follow in a separate PR after publication.For the Reviewer
Please focus on:
deploy/helm/nvcf-pki/templates/clusterissuer.yamlfor resource ownership and uninstall behavior;deploy/helm/nvcf-pki/scripts/check-render.shfor the render validation matrix;tools/ci/github-release-subprojects.jsonfor release registration; andFor QA
Validated on the exact pushed commit set:
helm lint deploy/helm/nvcf-pkihelm template nvcf-pki deploy/helm/nvcf-pkideploy/helm/nvcf-pki/scripts/check-render.shpython3 -m unittest tools/ci/test-github-release.pygo test -C tools/docs-version-sync ./..../tools/ci/check-doc-version-syncgit diff --check origin/main...HEADThe docs synchronization wrapper emitted its existing advisory warning because
imports.yamlis not present in the public checkout. Its Go unit tests passed.Post-merge QA is required: confirm the
0.1.0chart is published, render the published artifact, and verify the issuer reachesReady=Truein a disposable environment with cert-manager and OpenBao.Issues
Relates to #502
Notes
This is Pull Request 1 of the two-PR rollout. Do not begin stack consumption based only on a local chart override. Pull Request 2 must wait for successful publication of
helm-nvcf-pki:0.1.0.Safe rollback removes or reconfigures certificate consumers before issuer management is disabled. The retained
ClusterIssuershould only be deleted after no active certificate depends on it.References
Related Pull Requests
None.
Dependencies
None. No new third-party package is added. License review is not required, and
NOTICEis unchanged.Checklist
Summary by CodeRabbit
helm-nvcf-pki) to provision a cert-managerClusterIssuerbacked by OpenBao for service TLS.