Skip to content

feat: Tenant TargetedInstanceCreation capability initial implemnetation#590

Draft
hwadekar-nv wants to merge 2 commits into
mainfrom
feat/tenant-capabilities-update
Draft

feat: Tenant TargetedInstanceCreation capability initial implemnetation#590
hwadekar-nv wants to merge 2 commits into
mainfrom
feat/tenant-capabilities-update

Conversation

@hwadekar-nv
Copy link
Copy Markdown
Contributor

@hwadekar-nv hwadekar-nv commented May 29, 2026

Description

This is in a draft mode PR, still evaluating and working through design and correct way to implement Provider and Site based Tenant Config update

  • Tenant TargetedInstanceCreation capability

Type of Change

  • Feature - New feature or functionality (feat:)
  • Fix - Bug fixes (fix:)
  • Chore - Modification or removal of existing functionality (chore:)
  • Refactor - Refactoring of existing functionality (refactor:)
  • Docs - Changes in documentation or OpenAPI schema (docs:)
  • CI - Changes in GitHub workflows. Requires additional scrutiny (ci:)
  • Version - Issuing a new release version (version:)

Services Affected

  • API - API models or endpoints updated
  • Workflow - Workflow service updated
  • DB - DB DAOs or migrations updated
  • Site Manager - Site Manager updated
  • Cert Manager - Cert Manager updated
  • Site Agent - Site Agent updated
  • Flow - Flow service updated
  • Powershelf Manager - Powershelf Manager updated
  • NVSwitch Manager - NVSwitch Manager updated

Related Issues (Optional)

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.)

Additional Notes

@hwadekar-nv hwadekar-nv self-assigned this May 29, 2026
@hwadekar-nv hwadekar-nv requested a review from a team as a code owner May 29, 2026 22:48
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

Review Change Stack

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5c2a660d-4e22-4f26-970b-ce6802de70c3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

This PR consolidates repeated tenant capability authorization logic by introducing a nil-safe helper function TenantHasTargetedInstanceCreation and propagating its use across nine handler locations. No behavioral changes occur; the refactoring improves consistency and eliminates redundant nil guards.

Changes

Targeted Instance Creation Authorization Refactoring

Layer / File(s) Summary
Helper function definition and test coverage
api/pkg/api/handler/util/common/common.go, api/pkg/api/handler/util/common/common_test.go
New exported function TenantHasTargetedInstanceCreation(tenant *cdbm.Tenant) bool encapsulates nil-safe checks on both tenant and config. Comprehensive test covers nil tenant, nil config, disabled, and enabled states.
Core authorization function refactoring
api/pkg/api/handler/util/common/common.go
Authorization checks in IsTenant and IsProviderOrTenant are updated to use the new helper instead of direct config field inspection.
Handler-wide authorization check updates
api/pkg/api/handler/expectedmachine.go, api/pkg/api/handler/instance.go, api/pkg/api/handler/machine.go, api/pkg/api/handler/site.go, api/pkg/api/handler/sku.go, api/pkg/api/handler/vpc.go
Nine authorization checks across handlers for machines, instances, sites, SKUs, and VPCs are updated to use the nil-safe helper, replacing direct tenant config field access.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/infra-controller-rest#568: Both PRs refactor privileged tenant authorization in machine retrieval handlers, with this PR introducing the nil-safe helper that aligns with #568's changes to allow privileged tenants unrestricted machine access.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The pull request description is vague and generic, using non-descriptive language like 'still evaluating and working through design' that does not clearly convey the specific changes being made. Provide a more specific description: clearly state the refactoring objective (centralizing TargetedInstanceCreation checks via TenantHasTargetedInstanceCreation helper), list affected handlers, and clarify the design decision rationale.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: introducing a helper function to centralize tenant TargetedInstanceCreation capability checks across multiple handlers.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/tenant-capabilities-update

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

@hwadekar-nv hwadekar-nv marked this pull request as draft May 29, 2026 22:48
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 29, 2026

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.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 29, 2026

Test Results

9 811 tests  +32   9 811 ✅ +32   7m 21s ⏱️ -3s
  161 suites ± 0       0 💤 ± 0 
   14 files   ± 0       0 ❌ ± 0 

Results for commit 2707d63. ± Comparison against base commit e75b57a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-05-29 22:49:21 UTC | Commit: 14bbaeb

@github-actions
Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
nico-flow 66 4 34 18 2 8
nico-nsm 82 2 28 43 9 0
nico-psm 67 4 35 18 2 8
nico-rest-api 100 6 53 30 3 8
nico-rest-cert-manager 65 4 34 18 1 8
nico-rest-db 66 4 34 18 2 8
nico-rest-site-agent 65 4 34 18 1 8
nico-rest-site-manager 65 4 34 18 1 8
nico-rest-workflow 67 4 35 18 2 8
TOTAL 643 36 321 199 23 64

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

…a shared helper

Signed-off-by: Hitesh Wadekar <hwadekar@nvidia.com>
…tive backfill migration

Signed-off-by: Hitesh Wadekar <hwadekar@nvidia.com>
@hwadekar-nv hwadekar-nv force-pushed the feat/tenant-capabilities-update branch from 6d74573 to 2707d63 Compare May 29, 2026 23:45
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.

1 participant