Skip to content

feat(nvca): configure worker-init transport TLS trust - #718

Open
mikeyrcamp wants to merge 9 commits into
mainfrom
agent/transport-tls-worker-init-mainline
Open

feat(nvca): configure worker-init transport TLS trust#718
mikeyrcamp wants to merge 9 commits into
mainfrom
agent/transport-tls-worker-init-mainline

Conversation

@mikeyrcamp

@mikeyrcamp mikeyrcamp commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Rollout prerequisite

Publish worker-init with the installer first, then update applicable INIT_CONTAINER metadata/default/override references, and only then enable the NVCA injection change. This prevents an injected installer from selecting an older worker-init image without the executable.

Supersedes

Fixes #713

Validation

  • Installer merge, fingerprint, malformed-PEM, and output-write coverage now lives with worker-init.
  • bazel test //src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install:nvcf-trust-bundle-install_test //src/compute-plane-services/worker-init/cmd:image_contents_test
  • Focused NVCA transport-TLS, MiniService, operator reconcile, shared-library, and config tests.
  • bash src/compute-plane-services/nvca/scripts/lint_helm.sh, including both chart copies and direct-entrypoint assertions.
  • Built worker-init and NVCA OCI image archives successfully.

Dev2 live validation

Using temporary, non-release images, the worker-init image was pre-pulled to all six dev2 nodes and NVCA was configured to use it via a temporary INIT_CONTAINER override. Recreating an LLM workload verified:

  • The regular init and injected nvcf-trust-bundle-install containers used the same worker-init image digest and IfNotPresent policy.
  • The installer completed successfully.
  • The LLM worker received the read-only merged-bundle mount and STARGATE_TLS_CERT_PATH=/nvcf/transport-tls/ca-certificates.crt.
  • Pylon completed its reverse TLS tunnel connection to the Stargate router and the Pod reached Ready.

The only worker restart was an initial inference-upstream readiness race; the subsequent worker instance connected successfully.

Summary by CodeRabbit

  • New Features
    • Added workload transport TLS settings for trust-bundle Secrets, optional SHA-256 fingerprint validation, and configurable certificate mount paths.
    • Worker initialization now installs and validates merged trust bundles.
  • Bug Fixes
    • Improved certificate rotation, rollout detection, configuration updates, and recovery after invalid settings.
    • Added validation to prevent unsafe or conflicting certificate mounts.
    • Workloads now start packaged binaries directly and use their regular initialization image.
  • Documentation
    • Documented the new transport TLS options, defaults, and configuration requirements.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change adds shared trust-bundle handling, Helm transport TLS configuration, operator ConfigMap reconciliation, configurable worker certificate paths, and worker-init image packaging. It also removes /tini wrappers and replaces installer-image configuration with regular init-container image resolution.

Changes

Workload transport TLS

Layer / File(s) Summary
Shared trust-bundle library and worker image
src/libraries/go/lib/pkg/trustbundle/*, src/compute-plane-services/worker-init/cmd/*, rules/oci/private/go.bzl
Adds PEM validation, fingerprinting, merging, atomic installation APIs, and worker-init image content checks.
Transport TLS contract and Helm wiring
src/libraries/go/lib/pkg/types/nvca/config/*, src/compute-plane-services/nvca/deployments/nvca-operator/*, deploy/helm/nvca-operator/nvca-operator/*
Replaces InstallerImage with InstalledBundleMountPath. Adds Helm schema, values, documentation, and operator ConfigMap templates. Removes /tini wrappers.
Worker pod TLS injection
src/compute-plane-services/nvca/internal/transporttls/*, src/compute-plane-services/nvca/internal/miniservice/transport_tls_test.go, src/compute-plane-services/nvca/pkg/nvca/transport_tls_test.go
Validates mount paths and conflicts. Resolves the installer image from the regular init container. Injects certificate volumes, environment variables, and the installer init container for eligible workloads.
Operator configuration mapping and reconciliation
src/compute-plane-services/nvca/pkg/operator/reconcile/*
Maps operator ConfigMaps and Secrets into agent configuration. Handles PEM and fingerprint validation, source conflicts, rollout detection, ConfigMap events, and preservation of the last valid configuration.
Helm and transport regression validation
src/compute-plane-services/nvca/scripts/lint_helm.sh
Adds rendered-chart checks for direct binary execution and transport trust configuration defaults, explicit values, and reuse-values upgrades.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested reviewers: shobham-nv

Sequence Diagram(s)

sequenceDiagram
  participant OperatorConfig as nvca-operator-config
  participant Reconciler as NVCA reconciler
  participant WorkerPod as LLM worker Pod
  participant WorkerInit as worker-init

  OperatorConfig->>Reconciler: Provide transportTLS configuration
  Reconciler->>WorkerPod: Inject TLS volumes and installer init container
  WorkerPod->>WorkerInit: Run nvcf-trust-bundle-install
  WorkerInit-->>WorkerPod: Write merged certificate bundle
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.37% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits and accurately describes the primary transport TLS trust feature.
Linked Issues check ✅ Passed The changes address #713 with informer initial-list gating, configuration-change reconciliation, contextual errors, and focused regression coverage.
Out of Scope Changes check ✅ Passed The transport TLS, worker-init packaging, operator configuration, and informer changes match the stated pull request objectives.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/transport-tls-worker-init-mainline

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

@mikeyrcamp
mikeyrcamp marked this pull request as ready for review August 6, 2026 21:59
@mikeyrcamp
mikeyrcamp requested review from a team as code owners August 6, 2026 21:59
@mikeyrcamp
mikeyrcamp requested a review from shobham-nv August 6, 2026 21:59

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (5)
src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go (2)

339-342: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a negative case for configMapUpdateForcesNVCAReconcile.

The test asserts only the true cases. Add an assertion that an unrelated ConfigMap name returns false, so the default branch stays covered.

Proposed fix
 	assert.True(t, configMapUpdateForcesNVCAReconcile(nvcfBackendChartDefaultsConfigMapName))
+	assert.False(t, configMapUpdateForcesNVCAReconcile("unrelated-configmap"))
+	assert.False(t, configMapUpdateForcesNVCAReconcile(cleanup.ShutdownSentinelConfigMapName))
🤖 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
`@src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go`
around lines 339 - 342, Extend TestConfigMapChangesForceNVCAReconcile with a
negative assertion using an unrelated ConfigMap name, and verify
configMapUpdateForcesNVCAReconcile returns false so its default branch is
covered.

45-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the package constant for the ConfigMap name.

Lines 262 and 291 use nvcaOperatorConfigMapName. Line 46 hardcodes "nvca-operator-config". Use the constant so a rename keeps this test aligned with the production name.

Proposed fix
-		ObjectMeta: metav1.ObjectMeta{Name: "nvca-operator-config"},
+		ObjectMeta: metav1.ObjectMeta{Name: nvcaOperatorConfigMapName},
🤖 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
`@src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go`
around lines 45 - 46, Replace the hardcoded ConfigMap name in the test’s
ConfigMap creation with the existing nvcaOperatorConfigMapName constant,
matching the usage at the other test locations and keeping the test aligned with
the production name.
src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go (2)

596-608: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify handleConfigMapAdd.

Both branches after the type assertion return nil, so the ShutdownSentinelConfigMapName case changes nothing. The function now only validates the informer object type. Keep the type check and drop the sentinel branch, or add a short comment that Add events are ignored on purpose so a later reader does not re-add rollout logic. The receiver c is also unused.

🤖 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 `@src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go`
around lines 596 - 608, Update handleConfigMapAdd to retain only the ConfigMap
type assertion and invalid-object error handling, removing the no-op
ShutdownSentinelConfigMapName branch; optionally document that valid Add events
are intentionally ignored. Since receiver c is unused, keep the implementation
free of unnecessary receiver references.

640-659: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The two Helm-managed and self-managed cases are identical.

Both bodies compare data and call c.dispatchReconcileClusterFunc(ctx). Merge them into one case newCM.Name == nvcfBackendHelmManagedConfigMapName, newCM.Name == nvcfBackendSelfManagedConfigMapName: branch to remove the duplication.

🤖 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 `@src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go`
around lines 640 - 659, Merge the duplicate branches in the ConfigMap update
handler into a single case covering nvcfBackendHelmManagedConfigMapName and
nvcfBackendSelfManagedConfigMapName. Preserve the existing diff comparison,
conditional c.dispatchReconcileClusterFunc(ctx) call, logging, and return
behavior.
src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper_test.go (1)

270-283: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Verify the indentation contract of the appended key.

setTransportTrustInstalledBundleMountPath appends a raw line at four-space indentation to the YAML created by createTransportTrustSource. The key lands under workload.transportTLS only while the base document keeps its current nesting. decodeNVCAOperatorConfig uses KnownFields(true), so a future indentation change in the base fixture makes these tests fail with a confusing unknown-field error instead of a mount-path assertion failure. Consider building the document from one shared template that takes the mount path.

🤖 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
`@src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper_test.go`
around lines 270 - 283, Update setTransportTrustInstalledBundleMountPath and the
createTransportTrustSource fixture setup to use one shared YAML template that
accepts the mount path, rather than appending a raw indented line. Preserve the
existing workload.transportTLS nesting and ensure decodeNVCAOperatorConfig
continues validating the generated document with KnownFields(true).
🤖 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 `@src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go`:
- Around line 629-659: Update the ConfigMap update handling in the switch cases
for configMapUpdateForcesNVCAReconcile, nvcfBackendHelmManagedConfigMapName, and
nvcfBackendSelfManagedConfigMapName so success logs only report a sync or
dispatched reconcile when diff != "". For unchanged data, log that the update
was ignored or omit the success message, applying the same behavior consistently
to all three cases.
- Around line 584-594: Update the ConfigMap informer callback that invokes
syncCurrentBackendForConfigMapChange to capture its returned error and log it
once with the callback’s logger. Preserve the existing fatal-error handling and
return behavior in syncCurrentBackendForConfigMapChange, avoiding duplicate
logging there.

In
`@src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper.go`:
- Around line 182-198: Update decodeNVCAOperatorConfig to treat io.EOF from the
first decoder.Decode as an empty configuration and return the zero-value
nvcaOperatorConfigDTO without error. Preserve existing errors for non-empty
invalid YAML and retain the single-document validation for additional documents.

In
`@src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go`:
- Around line 1423-1443: Build the desired agent ConfigMap once per sync,
including the trust Secret data resolved by getAgentConfigToMerge, then pass
that same result to newAgentConfigChangedCheck and setupAgentConfigConfigMap.
Remove their independent calls that reread the operator ConfigMap and Secret,
while preserving existing change detection and write behavior.

---

Nitpick comments:
In `@src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go`:
- Around line 596-608: Update handleConfigMapAdd to retain only the ConfigMap
type assertion and invalid-object error handling, removing the no-op
ShutdownSentinelConfigMapName branch; optionally document that valid Add events
are intentionally ignored. Since receiver c is unused, keep the implementation
free of unnecessary receiver references.
- Around line 640-659: Merge the duplicate branches in the ConfigMap update
handler into a single case covering nvcfBackendHelmManagedConfigMapName and
nvcfBackendSelfManagedConfigMapName. Preserve the existing diff comparison,
conditional c.dispatchReconcileClusterFunc(ctx) call, logging, and return
behavior.

In
`@src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper_test.go`:
- Around line 270-283: Update setTransportTrustInstalledBundleMountPath and the
createTransportTrustSource fixture setup to use one shared YAML template that
accepts the mount path, rather than appending a raw indented line. Preserve the
existing workload.transportTLS nesting and ensure decodeNVCAOperatorConfig
continues validating the generated document with KnownFields(true).

In
`@src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go`:
- Around line 339-342: Extend TestConfigMapChangesForceNVCAReconcile with a
negative assertion using an unrelated ConfigMap name, and verify
configMapUpdateForcesNVCAReconcile returns false so its default branch is
covered.
- Around line 45-46: Replace the hardcoded ConfigMap name in the test’s
ConfigMap creation with the existing nvcaOperatorConfigMapName constant,
matching the usage at the other test locations and keeping the test aligned with
the production name.
🪄 Autofix

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: 78f4bf77-b095-4a7e-95e5-3e44df3a278a

📥 Commits

Reviewing files that changed from the base of the PR and between cc1b487 and 549bbf2.

⛔ Files ignored due to path filters (5)
  • deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-8e61e521-nvca-operator/helm-nvca-operator/templates/self-managed-nvcfbackend-cm.yaml is excluded by !**/testdata/**
  • src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/trustbundle/BUILD.bazel is excluded by !**/vendor/**
  • src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/trustbundle/trust_bundle.go is excluded by !**/vendor/**
  • src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/types/nvca/config/types.go is excluded by !**/vendor/**
  • src/compute-plane-services/nvca/vendor/modules.txt is excluded by !**/vendor/**
📒 Files selected for processing (42)
  • deploy/helm/nvca-operator/nvca-operator/README.md
  • deploy/helm/nvca-operator/nvca-operator/templates/deployment.yaml
  • deploy/helm/nvca-operator/nvca-operator/templates/operator-config-cm.yaml
  • deploy/helm/nvca-operator/nvca-operator/templates/pre-delete-cleanup-job.yaml
  • deploy/helm/nvca-operator/nvca-operator/values.schema.json
  • deploy/helm/nvca-operator/nvca-operator/values.yaml
  • rules/oci/private/go.bzl
  • src/compute-plane-services/nvca/cmd/nvca/BUILD.bazel
  • src/compute-plane-services/nvca/cmd/nvcf-trust-bundle-install/main_test.go
  • src/compute-plane-services/nvca/deployments/nvca-operator/README.md
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/deployment.yaml
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/operator-config-cm.yaml
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/pre-delete-cleanup-job.yaml
  • src/compute-plane-services/nvca/deployments/nvca-operator/values.schema.json
  • src/compute-plane-services/nvca/deployments/nvca-operator/values.yaml
  • src/compute-plane-services/nvca/internal/miniservice/transport_tls_test.go
  • src/compute-plane-services/nvca/internal/transporttls/BUILD.bazel
  • src/compute-plane-services/nvca/internal/transporttls/transport_tls.go
  • src/compute-plane-services/nvca/internal/transporttls/transport_tls_test.go
  • src/compute-plane-services/nvca/internal/transporttls/trustbundle/BUILD.bazel
  • src/compute-plane-services/nvca/internal/transporttls/trustbundle/trust_bundle_test.go
  • src/compute-plane-services/nvca/pkg/nvca/transport_tls_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/BUILD.bazel
  • src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/rollout.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go
  • src/compute-plane-services/nvca/scripts/lint_helm.sh
  • src/compute-plane-services/worker-init/cmd/BUILD.bazel
  • src/compute-plane-services/worker-init/cmd/image_contents_test.sh
  • src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/BUILD.bazel
  • src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/main.go
  • src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/main_test.go
  • src/libraries/go/lib/pkg/trustbundle/BUILD.bazel
  • src/libraries/go/lib/pkg/trustbundle/trust_bundle.go
  • src/libraries/go/lib/pkg/trustbundle/trust_bundle_test.go
  • src/libraries/go/lib/pkg/types/nvca/config/config_test.go
  • src/libraries/go/lib/pkg/types/nvca/config/transport_tls_installer_image_test.go
  • src/libraries/go/lib/pkg/types/nvca/config/types.go
💤 Files with no reviewable changes (10)
  • src/compute-plane-services/nvca/cmd/nvca/BUILD.bazel
  • deploy/helm/nvca-operator/nvca-operator/templates/deployment.yaml
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/pre-delete-cleanup-job.yaml
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/deployment.yaml
  • src/compute-plane-services/nvca/cmd/nvcf-trust-bundle-install/main_test.go
  • src/compute-plane-services/nvca/internal/transporttls/trustbundle/BUILD.bazel
  • deploy/helm/nvca-operator/nvca-operator/templates/pre-delete-cleanup-job.yaml
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go
  • src/libraries/go/lib/pkg/types/nvca/config/config_test.go
  • src/compute-plane-services/nvca/internal/transporttls/trustbundle/trust_bundle_test.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go`:
- Around line 603-605: The ConfigMap Add handler currently suppresses all valid
Add events instead of only informer initial-list events. Gate the no-op on
informer sync state, and after synchronization route Add events through the
existing name-specific reconciliation path used by updates; add regression
coverage for post-sync delete-and-recreate behavior.
- Around line 1120-1125: Update the error handling around
newAgentConfigConfigMap and newAgentConfigChangedCheck in SyncNVCFBackend to
remove the local log calls and return context-rich wrapped errors using the
requested operation descriptions with %w, preserving immediate returns on
failure.
🪄 Autofix

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: e2a6f557-5bf0-493e-8f00-bd3017760aa2

📥 Commits

Reviewing files that changed from the base of the PR and between 549bbf2 and 6c20478.

📒 Files selected for processing (8)
  • src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/rollout.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/rollout_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go

Comment thread src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go Outdated
@mikeyrcamp
mikeyrcamp force-pushed the agent/transport-tls-worker-init-mainline branch from f70e313 to 8caa360 Compare August 7, 2026 01:03

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go`:
- Around line 608-611: Update handleConfigMapAdd’s invalid-object branch to
remove its local log and return a context-rich error identifying the ConfigMap
informer Add handler and validation failure, without including the full object.
Apply the same change to the corresponding branch around the second referenced
location so the informer caller remains the single error logger.
- Around line 613-619: Update handleConfigMapAdd and its handler registration to
use cache.ResourceEventHandlerDetailedFuncs, distinguishing initial-list adds
from real ConfigMap add events. Defer or replay real adds when the ConfigMap
handler is synced but informersSynced is false, while continuing to ignore
initial-list adds until all required caches are ready. Add a regression test
covering this startup ordering and stale-backend prevention.
🪄 Autofix

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: 6a49b6cb-bc9c-4d56-9a49-2d352192f3c4

📥 Commits

Reviewing files that changed from the base of the PR and between 6c20478 and f70e313.

📒 Files selected for processing (2)
  • src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go

Comment thread src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go Outdated
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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/nvca-operator/nvca-operator/templates/operator-config-cm.yaml`:
- Around line 36-38: The Helm conditional around installedBundleMountPath
removes the newline after fingerprint, producing invalid YAML when the path is
set. In both
deploy/helm/nvca-operator/nvca-operator/templates/operator-config-cm.yaml lines
36-38 and
src/compute-plane-services/nvca/deployments/nvca-operator/templates/operator-config-cm.yaml
lines 36-38, remove the left-trim markers from the conditional actions so
fingerprint and installedBundleMountPath remain on separate lines.

In
`@src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/main.go`:
- Line 27: Update the go-lib dependency version in worker-init/go.mod to a
release that exports pkg/trustbundle, then regenerate worker-init/go.sum so its
checksums match the new module version.
🪄 Autofix

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: 4bea2a13-60d4-4d5f-8c8c-942125777284

📥 Commits

Reviewing files that changed from the base of the PR and between 43c439e and 8caa360.

⛔ Files ignored due to path filters (5)
  • deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-8e61e521-nvca-operator/helm-nvca-operator/templates/self-managed-nvcfbackend-cm.yaml is excluded by !**/testdata/**
  • src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/trustbundle/BUILD.bazel is excluded by !**/vendor/**
  • src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/trustbundle/trust_bundle.go is excluded by !**/vendor/**
  • src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/types/nvca/config/types.go is excluded by !**/vendor/**
  • src/compute-plane-services/nvca/vendor/modules.txt is excluded by !**/vendor/**
📒 Files selected for processing (43)
  • deploy/helm/nvca-operator/nvca-operator/README.md
  • deploy/helm/nvca-operator/nvca-operator/templates/deployment.yaml
  • deploy/helm/nvca-operator/nvca-operator/templates/operator-config-cm.yaml
  • deploy/helm/nvca-operator/nvca-operator/templates/pre-delete-cleanup-job.yaml
  • deploy/helm/nvca-operator/nvca-operator/values.schema.json
  • deploy/helm/nvca-operator/nvca-operator/values.yaml
  • rules/oci/private/go.bzl
  • src/compute-plane-services/nvca/cmd/nvca/BUILD.bazel
  • src/compute-plane-services/nvca/cmd/nvcf-trust-bundle-install/main_test.go
  • src/compute-plane-services/nvca/deployments/nvca-operator/README.md
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/deployment.yaml
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/operator-config-cm.yaml
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/pre-delete-cleanup-job.yaml
  • src/compute-plane-services/nvca/deployments/nvca-operator/values.schema.json
  • src/compute-plane-services/nvca/deployments/nvca-operator/values.yaml
  • src/compute-plane-services/nvca/internal/miniservice/transport_tls_test.go
  • src/compute-plane-services/nvca/internal/transporttls/BUILD.bazel
  • src/compute-plane-services/nvca/internal/transporttls/transport_tls.go
  • src/compute-plane-services/nvca/internal/transporttls/transport_tls_test.go
  • src/compute-plane-services/nvca/internal/transporttls/trustbundle/BUILD.bazel
  • src/compute-plane-services/nvca/internal/transporttls/trustbundle/trust_bundle_test.go
  • src/compute-plane-services/nvca/pkg/nvca/transport_tls_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/BUILD.bazel
  • src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/rollout.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/rollout_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go
  • src/compute-plane-services/nvca/scripts/lint_helm.sh
  • src/compute-plane-services/worker-init/cmd/BUILD.bazel
  • src/compute-plane-services/worker-init/cmd/image_contents_test.sh
  • src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/BUILD.bazel
  • src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/main.go
  • src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/main_test.go
  • src/libraries/go/lib/pkg/trustbundle/BUILD.bazel
  • src/libraries/go/lib/pkg/trustbundle/trust_bundle.go
  • src/libraries/go/lib/pkg/trustbundle/trust_bundle_test.go
  • src/libraries/go/lib/pkg/types/nvca/config/config_test.go
  • src/libraries/go/lib/pkg/types/nvca/config/transport_tls_installer_image_test.go
  • src/libraries/go/lib/pkg/types/nvca/config/types.go
💤 Files with no reviewable changes (9)
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/deployment.yaml
  • deploy/helm/nvca-operator/nvca-operator/templates/deployment.yaml
  • src/compute-plane-services/nvca/internal/transporttls/trustbundle/BUILD.bazel
  • src/compute-plane-services/nvca/cmd/nvcf-trust-bundle-install/main_test.go
  • src/compute-plane-services/nvca/deployments/nvca-operator/templates/pre-delete-cleanup-job.yaml
  • deploy/helm/nvca-operator/nvca-operator/templates/pre-delete-cleanup-job.yaml
  • src/compute-plane-services/nvca/internal/transporttls/trustbundle/trust_bundle_test.go
  • src/compute-plane-services/nvca/cmd/nvca/BUILD.bazel
  • src/libraries/go/lib/pkg/types/nvca/config/config_test.go
🚧 Files skipped from review as they are similar to previous changes (30)
  • src/compute-plane-services/nvca/deployments/nvca-operator/values.yaml
  • src/compute-plane-services/nvca/scripts/lint_helm.sh
  • src/compute-plane-services/nvca/deployments/nvca-operator/README.md
  • src/libraries/go/lib/pkg/types/nvca/config/transport_tls_installer_image_test.go
  • src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/BUILD.bazel
  • src/compute-plane-services/nvca/pkg/operator/reconcile/BUILD.bazel
  • src/compute-plane-services/worker-init/cmd/image_contents_test.sh
  • deploy/helm/nvca-operator/nvca-operator/README.md
  • src/compute-plane-services/worker-init/cmd/BUILD.bazel
  • rules/oci/private/go.bzl
  • src/compute-plane-services/nvca/pkg/operator/reconcile/rollout_test.go
  • src/libraries/go/lib/pkg/trustbundle/BUILD.bazel
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper.go
  • src/compute-plane-services/nvca/pkg/nvca/transport_tls_test.go
  • deploy/helm/nvca-operator/nvca-operator/values.yaml
  • src/compute-plane-services/nvca/internal/transporttls/BUILD.bazel
  • src/compute-plane-services/nvca/internal/transporttls/transport_tls.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go
  • src/libraries/go/lib/pkg/trustbundle/trust_bundle.go
  • src/compute-plane-services/nvca/deployments/nvca-operator/values.schema.json
  • src/compute-plane-services/nvca/internal/miniservice/transport_tls_test.go
  • deploy/helm/nvca-operator/nvca-operator/values.schema.json
  • src/compute-plane-services/nvca/pkg/operator/reconcile/nvca_config_mapper_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/rollout.go
  • src/libraries/go/lib/pkg/types/nvca/config/types.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/transport_tls_config_test.go
  • src/compute-plane-services/nvca/internal/transporttls/transport_tls_test.go
  • src/compute-plane-services/nvca/pkg/operator/reconcile/backendk8scache.go
  • src/compute-plane-services/worker-init/cmd/nvcf-trust-bundle-install/main_test.go
  • src/libraries/go/lib/pkg/trustbundle/trust_bundle_test.go

@mikeyrcamp
mikeyrcamp enabled auto-merge August 7, 2026 01:17
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.

fix(nvca): address ConfigMap informer follow-ups from #672

3 participants