Skip to content

OSAC-1602: add configurable CaaS inventory backend - #626

Closed
rccrdpccl wants to merge 1 commit into
mainfrom
feat/OSAC-1602-inventory-backend
Closed

rccrdpccl wants to merge 1 commit into
mainfrom
feat/OSAC-1602-inventory-backend

Conversation

@rccrdpccl

@rccrdpccl rccrdpccl commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Add BCM (NVIDIA Base Command Manager) as an optional bare metal inventory source alongside the existing discovery_hosts.

  • Schema: osacBcmEnabled toggle, API URL, mTLS client cert/key, cert validation settings. Uses title/format/enumLabels for schema-driven UI rendering.
  • Defaults: BCM disabled, safe defaults for all fields.
  • Template: derives ConfigMap import-agents from discovery_hosts when CaaS profile is active. Enables BCM import when osacBcmEnabled is set. Both can run simultaneously.

Assisted-by: Claude Code noreply@anthropic.com

Summary by CodeRabbit

  • New Features
    • Added optional NVIDIA Base Command Manager inventory integration with configurable API access, client certificates, and certificate validation.
    • Added default container image settings for core platform services and command-line tooling.
    • Enhanced automated bootstrap configuration with project source details and optional execution environment images.
    • Added support for importing discovery and BCM agents and providing host hardware and network metadata.
    • Added configuration validation to ensure required BCM connection details are supplied when integration is enabled.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@rccrdpccl, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: acddbb17-68ef-4ad5-a42c-31a7b4042375

📥 Commits

Reviewing files that changed from the base of the PR and between 0abf7cd and ae0c560.

📒 Files selected for processing (5)
  • config/plugins/osac.example.yaml
  • plugins/osac/defaults.yaml
  • plugins/osac/schemas/config.yaml
  • plugins/osac/schemas/defaults.yaml
  • plugins/osac/templates/values.yaml.j2

Walkthrough

The OSAC plugin adds BCM inventory settings, schema validation, image defaults, and conditional bootstrap rendering. It also renders discovery-agent host metadata and BCM-agent mTLS settings.

Changes

OSAC BCM integration

Layer / File(s) Summary
Configuration contracts and defaults
config/plugins/osac.example.yaml, plugins/osac/defaults.yaml, plugins/osac/schemas/config.yaml, plugins/osac/schemas/defaults.yaml
Defines BCM API, mTLS credentials, certificate controls, validation rules, and default container images.
Bootstrap and discovery rendering
plugins/osac/templates/values.yaml.j2
Adds bootstrap settings and conditionally renders discovery-agent imports with host Redfish and network metadata.
BCM agent rendering
plugins/osac/templates/values.yaml.j2
Conditionally renders BCM-agent API settings, certificate controls, and multiline client credentials. Keeps bmf.enabled: false under bootstrap.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: validation, infrastructure

Suggested reviewers: eurijon

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the addition of a configurable CaaS inventory backend, which matches the primary pull request objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
No-Hardcoded-Secrets ✅ Passed No hardcoded secrets found. BCM key fields are empty, placeholders, or template inputs; scans found no private-key blocks, long base64 literals, or credentialed URLs. Risk: none.
No-Weak-Crypto ✅ Passed PASS: The patch adds YAML/Jinja configuration only. It introduces no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons.
No-Injection-Vectors ✅ Passed The commit adds only YAML schema/defaults and Jinja value interpolation; searches found no SQL concatenation, shell=True, eval/exec, pickle.loads, unsafe yaml.load, os.system, or dangerouslySetInne...
Container-Privileges ✅ Passed The pull request adds BCM configuration and schema definitions. No Kubernetes container privilege settings, privileged modes, host access options, or capability escalations appear in the modified f...
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data exposure in logs. All credential access operations use no_log: true protection. Debug messages output only generic status, not passwords, keys, certificates, API URLs, or tokens.
Ai-Attribution ✅ Passed AI use is disclosed in the PR and HEAD commit with Assisted-by: Claude Code; the PR commit has no AI Co-Authored-By trailer.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/OSAC-1602-inventory-backend

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@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: 6

🤖 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 `@plugins/osac/defaults.yaml`:
- Around line 47-50: Replace the mutable latest tags in operator_tag,
fulfillment_service, and aap_bootstrap with approved version-pinned tags or
immutable image digests. Keep the existing image repositories unchanged and
ensure all three defaults resolve to reproducible binaries.
- Around line 45-51: Rename the osac_images.operator and
osac_images.operator_tag defaults to the schema and template key osac_operator,
using a single complete operator image reference. Preserve the existing operator
image repository and latest tag while removing the obsolete split fields so the
default override is rendered correctly.

In `@plugins/osac/schemas/config.yaml`:
- Around line 132-157: Update the schema validation for osacBcmEnabled so that
when it is true, osacBcmApiUrl, osacBcmClientCert, and osacBcmClientKey are each
required and non-empty. Preserve the existing optional behavior when BCM is
disabled, using the schema’s conditional validation mechanism.

In `@plugins/osac/templates/values.yaml.j2`:
- Around line 123-143: Update the `_has_discovery` condition in the values
template to require both a non-empty `discovery_hosts` list and `'caas' in
osacProfilesList`; use this gated value for the existing `importAgentsEnabled`
and `importAgents` blocks. Keep the BMAAS handling via `_has_bcm` unchanged.
- Around line 117-152: Update the YAML structure in the values template so
configAsCode, importAgents, importBcmAgents, bmf, and their related enable flags
are all nested under aap.bootstrap. Preserve the existing conditional rendering
and values while correcting indentation for the entire bootstrap configuration.
- Around line 144-150: Extend the _has_bcm block in the values.yaml.j2 template
to render the remaining BCM connection settings alongside importBcmAgents.cert
and importBcmAgents.key. Map osacBcmApiUrl, osacBcmValidateCerts, and
osacBcmDisableBmcCertVerification to their supported BCM-agent values fields,
preserving the existing conditional and YAML formatting.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 8f0c30e5-467f-4ecb-8e5a-0ee2c8a55319

📥 Commits

Reviewing files that changed from the base of the PR and between 3bf7350 and 5e74348.

📒 Files selected for processing (4)
  • config/plugins/osac.example.yaml
  • plugins/osac/defaults.yaml
  • plugins/osac/schemas/config.yaml
  • plugins/osac/templates/values.yaml.j2

Comment thread plugins/osac/defaults.yaml
Comment thread plugins/osac/defaults.yaml Outdated
Comment thread plugins/osac/schemas/config.yaml
Comment thread plugins/osac/templates/values.yaml.j2 Outdated
Comment thread plugins/osac/templates/values.yaml.j2 Outdated
Comment thread plugins/osac/templates/values.yaml.j2
@rccrdpccl
rccrdpccl force-pushed the feat/OSAC-1602-inventory-backend branch from 0cee31f to 6781622 Compare August 5, 2026 08:26
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

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.

@rccrdpccl
rccrdpccl force-pushed the feat/OSAC-1602-inventory-backend branch from 6781622 to 840fd9c Compare August 5, 2026 08:29

@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 `@plugins/osac/templates/values.yaml.j2`:
- Around line 135-141: Update the host fields in the values template, including
bmc_url, credentials, boot_mac, netris_server_name, and resource_class, to
serialize their dynamic Jinja values as YAML-safe scalars before rendering.
Preserve the existing values and defaults while ensuring quotes, special
characters, and newlines cannot produce invalid values.yaml output.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: fbb6dcf1-9916-4be0-a2b5-322bf4ffdcea

📥 Commits

Reviewing files that changed from the base of the PR and between 0abf7cd and 6781622.

📒 Files selected for processing (5)
  • config/plugins/osac.example.yaml
  • plugins/osac/defaults.yaml
  • plugins/osac/schemas/config.yaml
  • plugins/osac/schemas/defaults.yaml
  • plugins/osac/templates/values.yaml.j2

Comment thread plugins/osac/templates/values.yaml.j2 Outdated
Add BCM (NVIDIA Base Command Manager) as an optional bare metal
inventory source alongside the existing discovery_hosts.

Schema: osacBcmEnabled toggle, API URL, mTLS client cert/key, cert
validation settings. Conditional validation requires connection fields
when BCM is enabled.

Defaults: BCM disabled, safe defaults for all fields. Container image
defaults for OSAC operator, fulfillment-service, envoy, bootstrap, and
CLI.

Template: renders discovery-agent imports from discovery_hosts when CaaS
profile is active. Enables BCM import with full connection settings when
osacBcmEnabled is set. Both inventory sources can run simultaneously.

Assisted-by: Claude Code <noreply@anthropic.com>
@rccrdpccl
rccrdpccl force-pushed the feat/OSAC-1602-inventory-backend branch from 840fd9c to ae0c560 Compare August 5, 2026 08:56
@rccrdpccl

Copy link
Copy Markdown
Contributor Author

/retest

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Re-triggered failed runs:

  • Disconnected Dry-Run (#30991146570)
  • E2E OSAC (#30991146781)

@rccrdpccl rccrdpccl closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployment Deployment-related changes plugins

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant