fix(chart): mount mTLS CA for auth-callout - #28
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
64c8d64 to
5240a62
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds tpl-driven extraVolumes/extraVolumeMounts to auth-callout, wires conditional mTLS CA mounting from the event-bus chart, updates deployment/auth docs and local dev values, and rewrites mTLS tests to generate certificates at runtime. ChangesmTLS CA Automation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
auth-callout/deploy/values.yaml (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing SPDX license header.
This YAML source file is missing the required SPDX header. Note
deploy/nats-event-bus/values.yamlalready carries it.📝 Proposed fix
+# Copyright 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + # Override the name and fullname of the chartAs per coding guidelines: "Every source file must include an SPDX license header:
# Copyright 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.followed by# SPDX-License-Identifier: Apache-2.0".🤖 Prompt for 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. In `@auth-callout/deploy/values.yaml` at line 1, Add the required SPDX license header to auth-callout/deploy/values.yaml by inserting the two YAML comment lines at the very top: the copyright line "Copyright 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." and the SPDX identifier "SPDX-License-Identifier: Apache-2.0" (each preceded by #), ensuring they appear before any other file content.
🤖 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.
Outside diff comments:
In `@auth-callout/deploy/values.yaml`:
- Line 1: Add the required SPDX license header to
auth-callout/deploy/values.yaml by inserting the two YAML comment lines at the
very top: the copyright line "Copyright 2026 NVIDIA CORPORATION & AFFILIATES.
All rights reserved." and the SPDX identifier "SPDX-License-Identifier:
Apache-2.0" (each preceded by #), ensuring they appear before any other file
content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c5859c46-03a2-4c7c-8b49-5aeb55830323
📒 Files selected for processing (9)
auth-callout/deploy/README.mdauth-callout/deploy/templates/_helpers.tplauth-callout/deploy/templates/configmap.yamlauth-callout/deploy/templates/deployment.yamlauth-callout/deploy/values.yamldeploy/README.mddeploy/nats-event-bus/values.yamldocs/authentication.mdlocal/nats/k8s/local-dev-values.yaml
💤 Files with no reviewable changes (1)
- local/nats/k8s/local-dev-values.yaml
ca580eb to
d7897b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
auth-callout/deploy/values.yaml (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing SPDX license header.
This YAML file lacks the required license header (the sibling
deploy/nats-event-bus/values.yamlincludes it). Add it at the top of the file.📝 Proposed header
+# Copyright 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + # Override the name and fullname of the chart nameOverride: ""As per coding guidelines: "Every source file must include an SPDX license header" for
**/*.{go,ts,tsx,js,jsx,yaml,yml,sh}.🤖 Prompt for 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. In `@auth-callout/deploy/values.yaml` at line 1, This file is missing the required SPDX license header; fix it by inserting the same SPDX header used in the sibling deploy/nats-event-bus/values.yaml at the very top of this file (above the existing "# Override the name and fullname of the chart" line) so the YAML begins with the project's standard SPDX-License-Identifier comment.
🧹 Nitpick comments (1)
auth-callout/deploy/templates/_helpers.tpl (1)
92-113: ⚖️ Poor tradeoffAccount for templated
extraVolumeTemplates/extraVolumeMountTemplatesin mTLS duplication checks (or document the limitation).
auth-callout.hasExtraVolume/auth-callout.hasExtraVolumeMountonly scan.Values.extraVolumes/.Values.extraVolumeMountsand ignoreextraVolumeTemplates/extraVolumeMountTemplates, so enablingauth-callout.mtlsCA.enabled: truewhile also adding anmtls-caentry via the templated lists can produce duplicatemtls-cavolume/mount definitions.No in-repo values combine these today:
auth-callout/deploy/values.yamldefaultsmtlsCA.enabled: false, anddeploy/nats-event-bus/values.yamlinjectsmtls-caviaextraVolumeTemplates/extraVolumeMountTemplatesgated byglobal.eventBus.mtls.enabled.🤖 Prompt for 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. In `@auth-callout/deploy/templates/_helpers.tpl` around lines 92 - 113, The hasExtraVolume and hasExtraVolumeMount template helpers currently only inspect .Values.extraVolumes and .Values.extraVolumeMounts; update each helper (auth-callout.hasExtraVolume and auth-callout.hasExtraVolumeMount) to also iterate over .root.Values.extraVolumeTemplates and .root.Values.extraVolumeMountTemplates (using | default list) and include those entries in the same existence checks (compare get "name" for volumes, and get "name" or get "mountPath" for mounts) so templated additions are detected, or alternatively add a clear comment/docstring in each helper noting the limitation if you opt not to handle templates.
🤖 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 `@auth-callout/deploy/templates/_helpers.tpl`:
- Around line 132-152: The serviceConfig template ("auth-callout.serviceConfig")
currently uses kindIs "map" on the result of fromYaml ($fragment), which does
not detect YAML parse failures because fromYaml returns a map with an "Error"
key on failure; update the validation to check if (hasKey $fragment "Error") or
the equivalent and fail with a clear message when present (e.g.,
"serviceConfigTemplates entries must render to valid YAML maps: {{
$fragment.Error }}"), so malformed rendered templates do not silently merge into
$config; keep the rest of the merge logic ($config, serviceConfigTemplates, mtls
handling) unchanged.
---
Outside diff comments:
In `@auth-callout/deploy/values.yaml`:
- Line 1: This file is missing the required SPDX license header; fix it by
inserting the same SPDX header used in the sibling
deploy/nats-event-bus/values.yaml at the very top of this file (above the
existing "# Override the name and fullname of the chart" line) so the YAML
begins with the project's standard SPDX-License-Identifier comment.
---
Nitpick comments:
In `@auth-callout/deploy/templates/_helpers.tpl`:
- Around line 92-113: The hasExtraVolume and hasExtraVolumeMount template
helpers currently only inspect .Values.extraVolumes and
.Values.extraVolumeMounts; update each helper (auth-callout.hasExtraVolume and
auth-callout.hasExtraVolumeMount) to also iterate over
.root.Values.extraVolumeTemplates and .root.Values.extraVolumeMountTemplates
(using | default list) and include those entries in the same existence checks
(compare get "name" for volumes, and get "name" or get "mountPath" for mounts)
so templated additions are detected, or alternatively add a clear
comment/docstring in each helper noting the limitation if you opt not to handle
templates.
🪄 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: 40b996cb-272f-4232-baa4-f636698163ad
📒 Files selected for processing (10)
auth-callout/deploy/README.mdauth-callout/deploy/templates/_helpers.tplauth-callout/deploy/templates/configmap.yamlauth-callout/deploy/templates/deployment.yamlauth-callout/deploy/values.yamlauth-callout/src/internal/auth/auth_test.godeploy/README.mddeploy/nats-event-bus/values.yamldocs/authentication.mdlocal/nats/k8s/local-dev-values.yaml
💤 Files with no reviewable changes (1)
- local/nats/k8s/local-dev-values.yaml
✅ Files skipped from review due to trivial changes (3)
- docs/authentication.md
- auth-callout/deploy/README.md
- deploy/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- auth-callout/deploy/templates/configmap.yaml
d7897b2 to
ea2d147
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
auth-callout/deploy/values.yaml (1)
1-1:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd required SPDX license header.
This YAML file is missing the required SPDX license header. As per coding guidelines, all
.yamlfiles must include the copyright and license identifier at the top.📄 Proposed fix
+# Copyright 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + # Override the name and fullname of the chartAs per coding guidelines:
**/*.{go,ts,tsx,js,jsx,yaml,yml,sh}: Every source file must include an SPDX license header.🤖 Prompt for 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. In `@auth-callout/deploy/values.yaml` at line 1, Add the required SPDX license header to the top of this YAML file: insert a standard comment block containing the copyright owner and an SPDX-License-Identifier (e.g., "SPDX-License-Identifier: Apache-2.0" or the project's chosen license) above the existing first line ("# Override the name and fullname of the chart") so the file complies with the repository rule requiring SPDX headers for all .yaml/.yml files.
🤖 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.
Outside diff comments:
In `@auth-callout/deploy/values.yaml`:
- Line 1: Add the required SPDX license header to the top of this YAML file:
insert a standard comment block containing the copyright owner and an
SPDX-License-Identifier (e.g., "SPDX-License-Identifier: Apache-2.0" or the
project's chosen license) above the existing first line ("# Override the name
and fullname of the chart") so the file complies with the repository rule
requiring SPDX headers for all .yaml/.yml files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fceb96e1-cec0-4370-b798-91998b4ce199
📒 Files selected for processing (10)
auth-callout/deploy/README.mdauth-callout/deploy/templates/_helpers.tplauth-callout/deploy/templates/configmap.yamlauth-callout/deploy/templates/deployment.yamlauth-callout/deploy/values.yamlauth-callout/src/internal/auth/auth_test.godeploy/README.mddeploy/nats-event-bus/values.yamldocs/authentication.mdlocal/nats/k8s/local-dev-values.yaml
💤 Files with no reviewable changes (1)
- local/nats/k8s/local-dev-values.yaml
✅ Files skipped from review due to trivial changes (2)
- docs/authentication.md
- deploy/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- auth-callout/deploy/templates/configmap.yaml
- auth-callout/deploy/templates/deployment.yaml
- auth-callout/src/internal/auth/auth_test.go
ea2d147 to
064ed2d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
auth-callout/deploy/values.yaml (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing SPDX license header.
This
.yamlfile lacks the required SPDX header (present indeploy/nats-event-bus/values.yaml).📝 Proposed header
+# Copyright 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + # Override the name and fullname of the chartAs per coding guidelines: "Every source file must include an SPDX license header" for
**/*.{go,ts,tsx,js,jsx,yaml,yml,sh}.🤖 Prompt for 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. In `@auth-callout/deploy/values.yaml` at line 1, Add the required SPDX license header to auth-callout/deploy/values.yaml (same header style used in deploy/nats-event-bus/values.yaml) at the top of the file; ensure the header matches project policy for YAML files (SPDX identifier and copyright line) so every source file in the specified extensions includes the SPDX license header.auth-callout/deploy/templates/deployment.yaml (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing SPDX license header.
This template
.yamlfile lacks the required SPDX header.📝 Proposed header
+# Copyright 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 apiVersion: apps/v1As per coding guidelines: "Every source file must include an SPDX license header" for
**/*.{go,ts,tsx,js,jsx,yaml,yml,sh}.🤖 Prompt for 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. In `@auth-callout/deploy/templates/deployment.yaml` at line 1, Add the required SPDX license header as the first line of this YAML file (before the existing apiVersion: apps/v1 line) using the project's chosen identifier (for example "SPDX-License-Identifier: Apache-2.0") so every source file meets the SPDX requirement; update the deployment.yaml template to include that header above the apiVersion: apps/v1 entry.
🤖 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.
Outside diff comments:
In `@auth-callout/deploy/templates/deployment.yaml`:
- Line 1: Add the required SPDX license header as the first line of this YAML
file (before the existing apiVersion: apps/v1 line) using the project's chosen
identifier (for example "SPDX-License-Identifier: Apache-2.0") so every source
file meets the SPDX requirement; update the deployment.yaml template to include
that header above the apiVersion: apps/v1 entry.
In `@auth-callout/deploy/values.yaml`:
- Line 1: Add the required SPDX license header to
auth-callout/deploy/values.yaml (same header style used in
deploy/nats-event-bus/values.yaml) at the top of the file; ensure the header
matches project policy for YAML files (SPDX identifier and copyright line) so
every source file in the specified extensions includes the SPDX license header.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f3b9adc4-5b9c-461e-8a51-54b92a8a085c
📒 Files selected for processing (8)
auth-callout/deploy/README.mdauth-callout/deploy/templates/deployment.yamlauth-callout/deploy/values.yamlauth-callout/src/internal/auth/auth_test.godeploy/README.mddeploy/nats-event-bus/values.yamldocs/authentication.mdlocal/nats/k8s/local-dev-values.yaml
💤 Files with no reviewable changes (1)
- local/nats/k8s/local-dev-values.yaml
✅ Files skipped from review due to trivial changes (2)
- docs/authentication.md
- deploy/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- auth-callout/src/internal/auth/auth_test.go
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-06-01 22:26:37 UTC | Commit: 064ed2d |
🛡️ CodeQL Analysis✅ No security issues found! 💡 Note: Enable GitHub Advanced Security to see full details in the Security tab. 🕐 Last updated: 2026-06-01 22:27:34 UTC | Commit: 064ed2d |
Signed-off-by: Frank Spitulski <fspitulski@nvidia.com>
064ed2d to
6056a65
Compare
|
🌿 Preview your docs: https://nvidia-preview-pr-28.docs.buildwithfern.com/dsx-exchange |
2 participants
Summary
AUTH_CALLOUT_MTLS_CA_PATHand mount its documentednats-mtls-server-tlsSecret only whenglobal.eventBus.mtls.enabledis trueGoal
NVBug 6230725 reports that the event-bus mTLS endpoint can require client
certificates while auth-callout does not get the CA path it needs to validate
those certificates. The intended behavior is that the event-bus chart produces
a working mTLS deployment without local-only overrides, while the auth-callout
chart still works standalone.
Chart Boundary
serviceConfig.mtls.ca-pathplusextraVolumesandextraVolumeMounts.global.eventBus.global.nats-mtls-server-tlsSecret name and theglobal.eventBus.mtls.enableddecision.Validation
git diff --check origin/main...HEADgo test -run TestMTLSAuthentication ./src/internal/authhelm lint auth-callout/deployhelm lint deploy/nats-event-bustools/check-docs-mdxmake checkmake -C local deploy-natsmake -C local test-functionalThe functional run passed the mTLS behavior cases, including connection with a
client certificate, rejection without a client certificate, mTLS pub/sub, and
TCP/mTLS routing.
Summary by CodeRabbit
New Features
Documentation
Tests