Skip to content

feat(component-manager): add NVOS password rotation job contract#3409

Merged
jayzhudev merged 6 commits into
NVIDIA:mainfrom
jayzhudev:nvos/password-rotation-02
Jul 14, 2026
Merged

feat(component-manager): add NVOS password rotation job contract#3409
jayzhudev merged 6 commits into
NVIDIA:mainfrom
jayzhudev:nvos/password-rotation-02

Conversation

@jayzhudev

@jayzhudev jayzhudev commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

This is the second incremental step toward complete NVOS password rotation support, following #3405.

It lays the groundwork in component-manager for async rotation:

  • Backend-neutral APIs for capability checks, submission, and job polling.
  • RMS integration for async password updates.
  • Explicit handling when a submission may have succeeded but no result was returned.
  • Consistent states for pending, completed, failed, unknown, and missing jobs.
  • A default-disabled NVOS rotation config option.

Password rotation needs careful retry and state handling. If a request times out or its response is lost, the password may still have changed. Retrying immediately could start another rotation using the wrong current credential. Component-manager handles backend-specific request and job details, while future controller changes will handle durable state, safe retries, credential reconciliation, and promotion.

This PR does not enable end-to-end rotation. No rotation tasks are submitted by the existing runtime, and the capability is disabled by default. Enabling the config option only makes the backend capability available; controller orchestration is still needed before the feature can work. Existing runtime behavior remains unchanged.

Returned job IDs are preserved as handles for observing submitted operations. A missing job is not treated as proof that the rotation never ran; credential state remains the basis for reconciliation.

Planned follow-up changes will add durable rotation state, controller submission and polling, credential reconciliation and promotion, restart recovery, operator documentation, and controlled activation.

Related issues

Supports #2041 and #3522

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

@jayzhudev jayzhudev self-assigned this Jul 12, 2026
@jayzhudev jayzhudev added the rack lifecycle Issues that relate to managing the lifecycle of a full rack (compute, switches and powershelves) label Jul 12, 2026
@jayzhudev jayzhudev requested a review from a team as a code owner July 12, 2026 03:45
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

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

Adds configurable asynchronous NVOS password rotation to the RMS-backed component manager, including backend-neutral states, RMS job submission and polling, credential-safe node information, error mappings, and comprehensive tests.

Changes

NVOS Password Rotation

Layer / File(s) Summary
Rotation contracts and error mapping
crates/component-manager/src/error.rs, crates/component-manager/src/nv_switch_manager.rs, crates/api-core/src/handlers/component_manager.rs
Adds password-rotation states and trait methods, introduces unsupported and unknown-outcome errors, and maps them to gRPC statuses.
Configuration and backend wiring
crates/component-manager/src/config.rs, crates/component-manager/src/component_manager.rs, crates/component-manager/src/rms.rs
Adds the RMS enablement flag, validates TOML defaults and overrides, and passes the flag through RMS backend construction.
RMS rotation workflow
crates/component-manager/src/rms.rs
Submits password-rotation jobs, excludes BMC credentials from node information, polls RMS job status, and maps parent and child job outcomes.
Rotation behavior validation
crates/component-manager/src/rms.rs
Tests state mapping, credential exclusion, feature gating, job submission, polling, and retry or rejection semantics.

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

Sequence Diagram(s)

sequenceDiagram
  participant ComponentManager
  participant RmsBackend
  participant RmsApi
  ComponentManager->>RmsBackend: start_password_rotation(endpoint, next_password)
  RmsBackend->>RmsApi: UpdateSwitchSystemPassword(host-only node info)
  RmsApi-->>RmsBackend: durable job ID
  RmsBackend-->>ComponentManager: return job ID
  ComponentManager->>RmsBackend: get_password_rotation_job_status(job ID)
  RmsBackend->>RmsApi: GetJobStatus(include child jobs)
  RmsApi-->>RmsBackend: RMS job execution and error state
  RmsBackend-->>ComponentManager: SwitchPasswordRotationState
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.61% 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
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding the NVOS password rotation job contract.
Description check ✅ Passed The description is clearly related to the changeset and matches the async NVOS password rotation work described in the diff.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@jayzhudev jayzhudev changed the title feat(component-manager): add NVOS rotation job contract feat(component-manager): add NVOS password rotation job contract Jul 12, 2026

@coderabbitai coderabbitai Bot 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.

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 `@crates/component-manager/src/rms.rs`:
- Around line 3252-3283: The test fixture in
password_rotation_node_info_excludes_bmc_credentials must use a distinct
nvos_credentials value from bmc_credentials so the host_endpoint source is
validated. Update the shared make_sw_endpoint setup, including the
password-rotation test below that uses it, while preserving the existing
assertions and test behavior.
🪄 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: f5c1a623-fb4d-4dbc-822a-9abe00d12979

📥 Commits

Reviewing files that changed from the base of the PR and between 6a5316b and 1218e39.

📒 Files selected for processing (6)
  • crates/api-core/src/handlers/component_manager.rs
  • crates/component-manager/src/component_manager.rs
  • crates/component-manager/src/config.rs
  • crates/component-manager/src/error.rs
  • crates/component-manager/src/nv_switch_manager.rs
  • crates/component-manager/src/rms.rs

Comment thread crates/component-manager/src/rms.rs
@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
boot-artifacts-aarch64 3 0 0 3 0 0
boot-artifacts-x86_64 3 0 0 3 0 0
forge-admin-cli-x86_64 259 13 30 78 7 131
machine-validation-runner 807 40 234 288 36 209
machine_validation 807 40 234 288 36 209
machine_validation-aarch64 807 40 234 288 36 209
nvmetal-carbide 807 40 234 288 36 209
TOTAL 3493 173 966 1236 151 967

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

Comment thread crates/component-manager/src/config.rs
Comment thread crates/component-manager/src/nv_switch_manager.rs
Comment thread crates/component-manager/src/rms.rs
Comment thread crates/component-manager/src/rms.rs
Comment thread crates/component-manager/src/rms.rs

@vinodchitraliNVIDIA vinodchitraliNVIDIA 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.

LGTM.
move nvos_password_rotation_enabled variable to component_manager struct

@jayzhudev jayzhudev force-pushed the nvos/password-rotation-02 branch from 5ac7dd1 to 893c2d4 Compare July 14, 2026 14:33

@coderabbitai coderabbitai Bot 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.

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 `@crates/component-manager/src/config.rs`:
- Around line 202-211: Extend the rms_nvos_password_rotation_can_be_enabled test
to use table-driven input/output cases, including an empty or legacy
configuration whose nvos_password_rotation_enabled value deserializes to false.
Retain coverage for the explicit true configuration and assert each expected
result after deserialization.
🪄 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: ad5e974d-3e18-4422-b2a9-f19ce4b06800

📥 Commits

Reviewing files that changed from the base of the PR and between 5ac7dd1 and 893c2d4.

📒 Files selected for processing (6)
  • crates/api-core/src/handlers/component_manager.rs
  • crates/component-manager/src/component_manager.rs
  • crates/component-manager/src/config.rs
  • crates/component-manager/src/error.rs
  • crates/component-manager/src/nv_switch_manager.rs
  • crates/component-manager/src/rms.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • crates/component-manager/src/error.rs
  • crates/component-manager/src/component_manager.rs
  • crates/component-manager/src/nv_switch_manager.rs
  • crates/api-core/src/handlers/component_manager.rs
  • crates/component-manager/src/rms.rs

Comment thread crates/component-manager/src/config.rs Outdated
@github-actions

Copy link
Copy Markdown

Comment thread crates/component-manager/src/nv_switch_manager.rs
Comment thread crates/component-manager/src/nv_switch_manager.rs
Comment thread crates/component-manager/src/nv_switch_manager.rs
Comment thread crates/component-manager/src/rms.rs

@spydaNVIDIA spydaNVIDIA 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.

lgtm!

@spydaNVIDIA spydaNVIDIA 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.

lgtm

@jayzhudev

jayzhudev commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

/ok-to-test

Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
@jayzhudev jayzhudev force-pushed the nvos/password-rotation-02 branch from 36eeadf to cb62290 Compare July 14, 2026 20:49
@ajf

ajf commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

@jayzhudev does this supersede #1609?

@jayzhudev

Copy link
Copy Markdown
Contributor Author

@jayzhudev does this supersede #1609?

Yes it does.

@jayzhudev jayzhudev enabled auto-merge (squash) July 14, 2026 21:49
@jayzhudev jayzhudev merged commit 2c2ecff into NVIDIA:main Jul 14, 2026
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rack lifecycle Issues that relate to managing the lifecycle of a full rack (compute, switches and powershelves)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants