Skip to content

feat(stack): surface api-keys service env in self-managed stack - #1396

Open
sparve-nv wants to merge 1 commit into
NVIDIA:mainfrom
sparve-nv:feat/surface-apikeys-env
Open

feat(stack): surface api-keys service env in self-managed stack#1396
sparve-nv wants to merge 1 commit into
NVIDIA:mainfrom
sparve-nv:feat/surface-apikeys-env

Conversation

@sparve-nv

@sparve-nv sparve-nv commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Surface the api-keys service environment (apikeys.env, e.g. NVCF_NCA_ID) in the self-managed stack so it can be set from environments/<env>.yaml, instead of only via the secrets file or a patched global.yaml.gotmpl. When apikeys.env is unset the stack emits nothing, so existing deployments are unchanged.

Additional Details

The api-keys release only receives ../global.yaml.gotmpl + ../secrets/<env>-secrets.yaml as chart values; the plain environment file reaches a release only where global.yaml.gotmpl re-emits it via dig … .Values. The apikeys: block wires image, podDisruptionBudget, and startupProbe but had no passthrough for apikeys.env, so apikeys.env.NVCF_NCA_ID set in environments/<env>.yaml was read into .Values but silently dropped. NVCF_NCA_ID is deployment-specific config an operator running the stack for their own tenant must set (it is the NCA the api-keys service authorizes keys against).

This adds the missing stack passthrough. No chart change.

  • deploy/stacks/self-managed/global.yaml.gotmpl — add a dig-based apikeys.env passthrough in the apikeys: block, mirroring the existing podDisruptionBudget / startupProbe passthroughs.
  • deploy/stacks/self-managed/environments/base.yaml — document the knob with a commented apikeys.env example.

Example — set the operator's NCA:

# environments/<env>.yaml
apikeys:
  env:
    NVCF_NCA_ID: "nca-xxxxxxxx"

Helm deep-merges the release values over the chart's values.yaml, so NVCF_NCA_ID lands in the env ConfigMap alongside the chart defaults (AWS_REGION / SPRING_PROFILES_ACTIVE) without clobbering them.

For the Reviewer

  • Backward-compatible: the with dig "apikeys" "env" dict .Values emits nothing when unset, so an unset flag renders byte-identical and the chart default env still applies.
  • This is the same dig … .Values shape already used in the same apikeys: block for podDisruptionBudget and startupProbe — add the knob, forward only when the operator supplies it.

For QA

Automated test (make test). Added deploy/stacks/self-managed/tests/apikeys-env-wiring.sh, wired into the offline make test target next to api-env-wiring.sh. It follows the existing value-wiring pattern (pdb-value-wiring.sh / api-env-wiring.sh): it renders the api-keys release values with helmfile write-values and asserts, via yq, that (a) apikeys.env is absent by default (no override emitted), and (b) apikeys.env.NVCF_NCA_ID (plus an arbitrary CUSTOM_APIKEYS_ENV key) is threaded through when the environment file sets apikeys.env.*.

Manual render. Also rendered the real api-keys chart env ConfigMap with helm template … --show-only templates/configmap-env.yaml:

  • Case A — default (apikeys.env unset): the ConfigMap carries the chart defaults only — AWS_REGION + SPRING_PROFILES_ACTIVE — confirming byte-identical behavior when the knob is unset.
  • Case B — override (apikeys.env.NVCF_NCA_ID set): the ConfigMap carries AWS_REGION + SPRING_PROFILES_ACTIVE + NVCF_NCA_ID, confirming the deep-merge adds the operator value without clobbering the defaults (and changes the checksum/config-env annotation so the pod rolls on change).

Issues

Fixes #1356

Checklist

  • I am familiar with the Contributing Guidelines.
  • I have signed off my commits for DCO compliance.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Add a dig-based apikeys.env passthrough in global.yaml.gotmpl mirroring the
existing podDisruptionBudget/startupProbe passthroughs, so operator config such
as NVCF_NCA_ID set in environments/<env>.yaml reaches the api-keys release
instead of being silently dropped. Helm deep-merges over the chart default env
(AWS_REGION / SPRING_PROFILES_ACTIVE); unset renders byte-identical. Document
the knob in base.yaml and add tests/apikeys-env-wiring.sh.

Closes NVIDIA#1356

Signed-off-by: sparve <sparve@nvidia.com>
@sparve-nv
sparve-nv requested a review from a team as a code owner August 31, 2026 13:04
@sparve-nv
sparve-nv requested a review from along-2017 August 31, 2026 13:04
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f25f9667-172c-4d3e-b057-a5a6870869f0

📥 Commits

Reviewing files that changed from the base of the PR and between 6c27bd1 and 26b539e.

📒 Files selected for processing (4)
  • deploy/stacks/self-managed/Makefile
  • deploy/stacks/self-managed/environments/base.yaml
  • deploy/stacks/self-managed/global.yaml.gotmpl
  • deploy/stacks/self-managed/tests/apikeys-env-wiring.sh

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The self-managed stack now passes optional apikeys.env values from environment files into the rendered api-keys release. The change documents the configuration and adds an integration test for unset and configured values.

Changes

Self-managed api-keys environment overrides

Layer / File(s) Summary
Render api-keys environment overrides
deploy/stacks/self-managed/global.yaml.gotmpl, deploy/stacks/self-managed/environments/base.yaml
The template conditionally renders apikeys.env. The base environment file documents overrides such as NVCF_NCA_ID.
Validate and register environment wiring
deploy/stacks/self-managed/tests/apikeys-env-wiring.sh, deploy/stacks/self-managed/Makefile
The integration test checks unset and configured values. The Makefile runs the test from the self-managed test target.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 26b53

This localized change allows operators to pass optional api-keys environment settings through the self-managed stack while preserving existing defaults when unset; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: along-2017

🚥 Pre-merge checks | ✅ 4 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy issue #1356 by wiring apikeys.env through global.yaml.gotmpl, documenting the operator configuration, preserving chart defaults when unset, and adding coverage for unset and config…
Out of Scope Changes check ✅ Passed All changes support issue #1356. The template update, configuration guidance, test registration, and wiring test are directly related to the requested apikeys.env passthrough.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits format and accurately describes the feature that exposes api-keys service environment variables in the self-managed stack.
Full details: Linked Issues check

Explanation

The changes satisfy issue #1356 by wiring apikeys.env through global.yaml.gotmpl, documenting the operator configuration, preserving chart defaults when unset, and adding coverage for unset and configured cases.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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.

Allow setting the api-keys service env (NVCF_NCA_ID) from the environment file in the self-managed stack

1 participant