feat(rest-api): Tenant TargetedInstanceCreation capability implementation#2134
Open
hwadekar-nv wants to merge 29 commits into
Open
feat(rest-api): Tenant TargetedInstanceCreation capability implementation#2134hwadekar-nv wants to merge 29 commits into
hwadekar-nv wants to merge 29 commits into
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. |
🔐 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-02 21:53:17 UTC | Commit: 1feb71c |
🔍 Container Scan SummaryNo Grype artifacts were found to aggregate. |
hwadekar-nv
changed the title
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
from
June 3, 2026 22:10
2fff422 to
7d6bcc6
Compare
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis change moves ChangesTargeted instance creation capability
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ProviderAdmin
participant UpdateTenantAccountHandler
participant TenantAccountDAO
participant TenantSiteDAO
ProviderAdmin->>UpdateTenantAccountHandler: PATCH siteCapabilities
UpdateTenantAccountHandler->>TenantAccountDAO: update account default in transaction
UpdateTenantAccountHandler->>TenantSiteDAO: apply or clear site overrides
UpdateTenantAccountHandler-->>ProviderAdmin: return tenant account siteCapabilities
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
2 times, most recently
from
June 4, 2026 20:23
ec337d8 to
eeb42e0
Compare
Contributor
Author
|
Will start working on this PR after 2.0. We have new strategies to allow tenants to update capabilities via TenantAccount and TenantSite across all Sites or specific Site. |
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
2 times, most recently
from
July 2, 2026 18:32
685f8f1 to
c3402d9
Compare
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
2 times, most recently
from
July 7, 2026 21:31
482fc88 to
d20920e
Compare
hwadekar-nv
marked this pull request as ready for review
July 7, 2026 21:37
Contributor
Author
|
@CodeRabbit review |
Contributor
✅ Action performedReview finished.
|
|
🌿 Preview your docs: https://nvidia-preview-pull-request-2134.docs.buildwithfern.com/infra-controller |
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
from
July 7, 2026 21:46
d20920e to
3c8f601
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rest-api/db/pkg/db/model/tenantaccount.go (1)
546-564: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake mixed
TenantAccountupdates atomic.
Updatecan run the scalar-column update and the config merge as separate statements. If a caller passestx == nilwith both normal fields andConfig, the first update can commit before the config merge fails. Prefer building oneUPDATE, or require/wrap mixed updates incdb.WithTx.🤖 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 `@rest-api/db/pkg/db/model/tenantaccount.go` around lines 546 - 564, The TenantAccount update path in the update method currently splits scalar field changes and Config merging into separate UPDATE statements, which can leave mixed updates partially applied when tx is nil. Refactor the logic around updatedFields/input.Config in TenantAccount.Update to perform a single atomic update when both kinds of changes are present, or ensure mixed updates are always executed inside cdb.WithTx by wrapping/requiring a transaction before running db.GetIDB(...).NewUpdate().Exec.Source: Path instructions
🧹 Nitpick comments (3)
rest-api/api/pkg/api/model/tenantaccountsitecapability.go (1)
199-206: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a receiver method over a free function on the named type.
GlobalTargetedInstanceCreationFromRequestoperates entirely onAPITenantAccountSiteCapabilitiesUpdateRequest; expressing it as a method (e.g.caps.GlobalTargetedInstanceCreation()) keeps behavior colocated with the type and reads more naturally at the call site.As per path instructions: "discourage scattered independent functions when a receiver method would make ownership and responsibilities clearer."
♻️ Proposed conversion to a receiver method
-func GlobalTargetedInstanceCreationFromRequest(caps APITenantAccountSiteCapabilitiesUpdateRequest) *bool { - for _, cap := range caps { +func (caps APITenantAccountSiteCapabilitiesUpdateRequest) GlobalTargetedInstanceCreation() *bool { + for _, cap := range caps { if cap.Scope == TenantAccountSiteCapabilityScopeGlobal { return cutil.GetPtr(cap.TargetedInstanceCreation) } } return nil }🤖 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 `@rest-api/api/pkg/api/model/tenantaccountsitecapability.go` around lines 199 - 206, Convert GlobalTargetedInstanceCreationFromRequest into a receiver method on APITenantAccountSiteCapabilitiesUpdateRequest so the behavior lives with the type and can be called as caps.GlobalTargetedInstanceCreation(). Keep the same logic that scans for TenantAccountSiteCapabilityScopeGlobal and returns the targeted instance creation value, but move it onto the named type and update any call sites accordingly.Source: Path instructions
rest-api/db/pkg/db/model/tenantsite_test.go (2)
287-293: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse range-over-integer for this counting loop.
- for i := 0; i < siteCount; i++ { + for i := range siteCount {As per coding guidelines, “Counting loop → range-over-integer.” As per path instructions, prefer range-based iteration over C-style
forloops.🤖 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 `@rest-api/db/pkg/db/model/tenantsite_test.go` around lines 287 - 293, The counting loop in the tenant site test should use range-over-integer instead of a C-style for loop. Update the iteration in the site-building block within the test that uses TestBuildSite and TestBuildTenantSite so it iterates with a range-based integer loop over siteCount, while preserving the existing indexing and even/odd behavior.Sources: Coding guidelines, Path instructions
602-701: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test case for
RemoveTargetedInstanceCreation.The DAO now has a clear-override branch, but this table only verifies config merge. Add a case that starts with
targetedInstanceCreation=true, updates withRemoveTargetedInstanceCreation: true, and asserts the pointer is cleared.🤖 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 `@rest-api/db/pkg/db/model/tenantsite_test.go` around lines 602 - 701, Add a missing Update test for the clear-override path in TenantSiteSQLDAO.Update: the current TestTenantSiteSQLDAO_Update only covers config merge, so add a case that seeds TenantSiteConfig.TargetedInstanceCreation as true, calls Update with TenantSiteConfigUpdateInput.RemoveTargetedInstanceCreation set to true, and asserts the resulting TenantSite.Config.TargetedInstanceCreation is cleared/nil. Use the existing TenantSiteSQLDAO.Update, TenantSiteUpdateInput, and TenantSiteConfigUpdateInput symbols to locate the right spot in the table-driven test.Source: Path instructions
🤖 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 `@rest-api/api/pkg/api/handler/machine.go`:
- Line 245: Update the privileged machine read authorization to use the
site-effective targeted-instance capability instead of the legacy tenant
flag/provider-account check. In the machine read paths around the
TenantHasTargetedInstanceCreation logic, switch single-machine reads to
EffectiveTargetedInstanceCreation(ctx, nil, ..., tenant, machine.SiteID), and
for list reads apply the same site-aware rule when siteId is present so
disabled-site machines are not returned in tenant-privileged results. Make sure
the behavior in the machine handler keeps honoring TenantAccount.config, Ready
status, and TenantSite.config overrides.
- Around line 762-768: The site-scoped capability check in IsProviderOrTenant is
being bypassed by the early requirePrivilegedTenant gate, which can reject
legacy TenantAccount tenants before machine.SiteID is evaluated. Update the
IsProviderOrTenant flow to defer the privileged-tenant rejection until after the
EffectiveTargetedInstanceCreation check runs, keeping the machine.SiteID-based
logic as the authoritative permission decision and removing the premature
pre-gate.
In `@rest-api/api/pkg/api/handler/util/common/common.go`:
- Around line 1467-1483: The privileged pre-check is still using the deprecated
tenant-level flag in TenantHasTargetedInstanceCreation, which can incorrectly
reject tenants enabled via the new TenantAccount.config path. Update the
requirePrivileged / requirePrivilegedTenant flow in common.go so non-site-scoped
privilege is resolved from the ready TenantAccount config, or remove this early
gate for site-aware callers and let EffectiveTargetedInstanceCreation be the
authoritative check. Keep the fix localized around
TenantHasTargetedInstanceCreation and the caller(s) that enforce the privileged
gate.
In `@rest-api/db/pkg/db/model/tenantsite.go`:
- Around line 314-334: The TenantSite update path currently applies Config
merging and RemoveTargetedInstanceCreation as separate updates, which can leave
partial state and makes conflicting input ambiguous. Update the logic in the
TenantSite model update flow to validate that both inputs are not used together,
then build a single NewUpdate/Exec path that applies either the JSONB merge or
the targetedInstanceCreation removal, not both. Use the existing update block
around input.Config and input.RemoveTargetedInstanceCreation to centralize the
decision and preserve one atomic update per request.
In
`@rest-api/db/pkg/migrations/20260701120000_tenant_account_site_capabilities.go`:
- Around line 26-33: The backfill in the migration only applies to Ready
tenant_account rows, so invited or pending accounts that later become Ready will
miss the legacy targetedInstanceCreation capability. Update the migration logic
in the tenant_account/tenant backfill to copy the capability for all matching
tenant_account rows tied to tenants with
tenant.config.targetedInstanceCreation=true, and if the status filter must
remain elsewhere, ensure the status transition path in tenant account handling
also propagates this config field when moving to Ready.
---
Outside diff comments:
In `@rest-api/db/pkg/db/model/tenantaccount.go`:
- Around line 546-564: The TenantAccount update path in the update method
currently splits scalar field changes and Config merging into separate UPDATE
statements, which can leave mixed updates partially applied when tx is nil.
Refactor the logic around updatedFields/input.Config in TenantAccount.Update to
perform a single atomic update when both kinds of changes are present, or ensure
mixed updates are always executed inside cdb.WithTx by wrapping/requiring a
transaction before running db.GetIDB(...).NewUpdate().Exec.
---
Nitpick comments:
In `@rest-api/api/pkg/api/model/tenantaccountsitecapability.go`:
- Around line 199-206: Convert GlobalTargetedInstanceCreationFromRequest into a
receiver method on APITenantAccountSiteCapabilitiesUpdateRequest so the behavior
lives with the type and can be called as caps.GlobalTargetedInstanceCreation().
Keep the same logic that scans for TenantAccountSiteCapabilityScopeGlobal and
returns the targeted instance creation value, but move it onto the named type
and update any call sites accordingly.
In `@rest-api/db/pkg/db/model/tenantsite_test.go`:
- Around line 287-293: The counting loop in the tenant site test should use
range-over-integer instead of a C-style for loop. Update the iteration in the
site-building block within the test that uses TestBuildSite and
TestBuildTenantSite so it iterates with a range-based integer loop over
siteCount, while preserving the existing indexing and even/odd behavior.
- Around line 602-701: Add a missing Update test for the clear-override path in
TenantSiteSQLDAO.Update: the current TestTenantSiteSQLDAO_Update only covers
config merge, so add a case that seeds TenantSiteConfig.TargetedInstanceCreation
as true, calls Update with
TenantSiteConfigUpdateInput.RemoveTargetedInstanceCreation set to true, and
asserts the resulting TenantSite.Config.TargetedInstanceCreation is cleared/nil.
Use the existing TenantSiteSQLDAO.Update, TenantSiteUpdateInput, and
TenantSiteConfigUpdateInput symbols to locate the right spot in the table-driven
test.
🪄 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: aef13451-a675-454a-9c90-57bbe59434be
⛔ Files ignored due to path filters (40)
rest-api/sdk/standard/api_tenant_account.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/client.gois excluded by!rest-api/sdk/standard/client.gorest-api/sdk/standard/model_batch_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_interface_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_site_capability.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_site_capability_scope.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_capabilities.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_update_request.gois excluded by!rest-api/sdk/standard/model_*.go
📒 Files selected for processing (21)
rest-api/api/pkg/api/handler/instance.gorest-api/api/pkg/api/handler/machine.gorest-api/api/pkg/api/handler/serviceaccount.gorest-api/api/pkg/api/handler/tenantaccount.gorest-api/api/pkg/api/handler/util/common/common.gorest-api/api/pkg/api/handler/util/common/common_test.gorest-api/api/pkg/api/handler/vpc.gorest-api/api/pkg/api/model/operatingsystem_test.gorest-api/api/pkg/api/model/sshkeygroup_test.gorest-api/api/pkg/api/model/tenant.gorest-api/api/pkg/api/model/tenant_test.gorest-api/api/pkg/api/model/tenantaccount.gorest-api/api/pkg/api/model/tenantaccount_test.gorest-api/api/pkg/api/model/tenantaccountsitecapability.gorest-api/db/pkg/db/model/tenantaccount.gorest-api/db/pkg/db/model/tenantsite.gorest-api/db/pkg/db/model/tenantsite_test.gorest-api/db/pkg/db/model/testing.gorest-api/db/pkg/migrations/20230412224951_tenant_site.gorest-api/db/pkg/migrations/20260701120000_tenant_account_site_capabilities.gorest-api/openapi/spec.yaml
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
from
July 8, 2026 00:01
4a12952 to
b0ee8a6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
rest-api/db/pkg/db/model/tenantsite_test.go (1)
287-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer range-over-integer for this counting loop.
The index is used solely as a counter (
i%2,Sprintf), so this is a textbook candidate for Go's range-over-integer form.♻️ Suggested refactor
- sites := []*Site{} - siteCount := 30 - for i := 0; i < siteCount; i++ { + sites := []*Site{} + siteCount := 30 + for i := range siteCount {As per path instructions: "Counting loop → range-over-integer. When the bound is a count ... use
for i := range n".🤖 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 `@rest-api/db/pkg/db/model/tenantsite_test.go` around lines 287 - 297, The counting loop in the tenant site test uses an index only as a counter for naming and parity checks, so refactor the loop to use Go’s range-over-integer form instead of a classic three-part loop. Update the loop in the test setup that builds sites and calls TestBuildSite/TestBuildTenantSite so the iteration variable still supports the Sprintf name generation and i%2 branch, but the loop itself follows the preferred range-over-integer pattern.Source: Path instructions
rest-api/api/pkg/api/handler/machine.go (1)
305-331: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftRedundant per-site resolution creates an N+1 on the machine-list hot path.
EffectiveTargetedInstanceCreationinternally re-fetches theSiteby ID and re-queries ReadyTenantAccounts per provider, yet this loop already holds eachsitefromsiteDAO.GetAlland the provider is fixed for the inner iteration. The cost isO(providers × sites)with three DB round-trips per site. Consider resolving the provider's ReadyTenantAccountonce outside the inner loop and evaluating the per-siteTenantSiteoverride directly against the already-loadedsite.🤖 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 `@rest-api/api/pkg/api/handler/machine.go` around lines 305 - 331, The machine list path has an N+1 problem because the loop in the handler around siteDAO.GetAll and common.EffectiveTargetedInstanceCreation re-fetches each Site and re-queries TenantAccount data for every site/provider pair. Refactor this block so the provider’s Ready TenantAccount is resolved once per provider outside the inner site loop, then use the already-loaded site data to check the per-site TenantSite override directly instead of calling EffectiveTargetedInstanceCreation for every site. Keep the existing error handling/logging structure in the same handler flow.
🤖 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 `@rest-api/api/pkg/api/handler/machine.go`:
- Around line 294-337: The machine list filtering in the tenant branch is using
provider IDs even though EffectiveTargetedInstanceCreation is evaluated per
site, which can widen results to disabled sites. Update the logic in machine.go
around the tenant/filterSite handling to collect and append enabled site IDs
instead of provider IDs, and apply them through MachineFilterInput.SiteIDs
rather than InfrastructureProviderIDs. Keep the existing
EffectiveTargetedInstanceCreation and SiteDAO.GetAll flow, but change the
aggregation variable and final filter assignment so the list stays scoped to
sites where targeted instance creation is enabled.
---
Nitpick comments:
In `@rest-api/api/pkg/api/handler/machine.go`:
- Around line 305-331: The machine list path has an N+1 problem because the loop
in the handler around siteDAO.GetAll and
common.EffectiveTargetedInstanceCreation re-fetches each Site and re-queries
TenantAccount data for every site/provider pair. Refactor this block so the
provider’s Ready TenantAccount is resolved once per provider outside the inner
site loop, then use the already-loaded site data to check the per-site
TenantSite override directly instead of calling
EffectiveTargetedInstanceCreation for every site. Keep the existing error
handling/logging structure in the same handler flow.
In `@rest-api/db/pkg/db/model/tenantsite_test.go`:
- Around line 287-297: The counting loop in the tenant site test uses an index
only as a counter for naming and parity checks, so refactor the loop to use Go’s
range-over-integer form instead of a classic three-part loop. Update the loop in
the test setup that builds sites and calls TestBuildSite/TestBuildTenantSite so
the iteration variable still supports the Sprintf name generation and i%2
branch, but the loop itself follows the preferred range-over-integer pattern.
🪄 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: 8bd15369-c9f6-45e7-b4dd-b1d7c80569ca
⛔ Files ignored due to path filters (40)
rest-api/sdk/standard/api_tenant_account.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/client.gois excluded by!rest-api/sdk/standard/client.gorest-api/sdk/standard/model_batch_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_interface_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_site_capability.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_site_capability_scope.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_capabilities.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_update_request.gois excluded by!rest-api/sdk/standard/model_*.go
📒 Files selected for processing (23)
rest-api/api/pkg/api/handler/instance.gorest-api/api/pkg/api/handler/machine.gorest-api/api/pkg/api/handler/machine_test.gorest-api/api/pkg/api/handler/serviceaccount.gorest-api/api/pkg/api/handler/tenantaccount.gorest-api/api/pkg/api/handler/util/common/common.gorest-api/api/pkg/api/handler/util/common/common_test.gorest-api/api/pkg/api/handler/vpc.gorest-api/api/pkg/api/model/operatingsystem_test.gorest-api/api/pkg/api/model/sshkeygroup_test.gorest-api/api/pkg/api/model/tenant.gorest-api/api/pkg/api/model/tenant_test.gorest-api/api/pkg/api/model/tenantaccount.gorest-api/api/pkg/api/model/tenantaccount_test.gorest-api/api/pkg/api/model/tenantaccountsitecapability.gorest-api/db/pkg/db/model/tenantaccount.gorest-api/db/pkg/db/model/tenantaccount_test.gorest-api/db/pkg/db/model/tenantsite.gorest-api/db/pkg/db/model/tenantsite_test.gorest-api/db/pkg/db/model/testing.gorest-api/db/pkg/migrations/20230412224951_tenant_site.gorest-api/db/pkg/migrations/20260701120000_tenant_account_site_capabilities.gorest-api/openapi/spec.yaml
✅ Files skipped from review due to trivial changes (1)
- rest-api/api/pkg/api/model/sshkeygroup_test.go
🚧 Files skipped from review as they are similar to previous changes (14)
- rest-api/db/pkg/migrations/20230412224951_tenant_site.go
- rest-api/db/pkg/db/model/testing.go
- rest-api/api/pkg/api/model/tenant_test.go
- rest-api/api/pkg/api/model/operatingsystem_test.go
- rest-api/openapi/spec.yaml
- rest-api/api/pkg/api/model/tenantaccount_test.go
- rest-api/api/pkg/api/handler/instance.go
- rest-api/api/pkg/api/handler/vpc.go
- rest-api/db/pkg/db/model/tenantaccount.go
- rest-api/api/pkg/api/model/tenantaccount.go
- rest-api/api/pkg/api/model/tenantaccountsitecapability.go
- rest-api/api/pkg/api/model/tenant.go
- rest-api/db/pkg/db/model/tenantsite.go
- rest-api/api/pkg/api/handler/tenantaccount.go
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
from
July 8, 2026 16:38
fdc9a2d to
ffd8c7b
Compare
Contributor
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 (3)
rest-api/api/pkg/api/handler/tenantaccount.go (2)
397-464: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
GetAllTenantAccountHandlersilently drops per-site capability overrides.Every list-response
APITenantAccountis built withtenantSites=nil(Line 447), whileGetTenantAccountHandler(Line 585) and the new PUT response (Line 932) now fetch and pass the actualTenantSiteslice. SinceNewAPITenantAccountderivesSiteCapabilitiesfromtenantSites, the list endpoint will only ever report the global scope and silently omit anylimitedper-site overrides — producing an inconsistent contract betweenGET /tenant-account(list) andGET /tenant-account/{id}for the same brand-new field.Consider batch-fetching
TenantSiterows grouped by tenant ID (mirroring the existing allocation-count batching pattern already in this handler) so list responses stay consistent with the detail view.🤖 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 `@rest-api/api/pkg/api/handler/tenantaccount.go` around lines 397 - 464, GetAllTenantAccountHandler is building APITenantAccount list items with tenantSites left nil, so NewAPITenantAccount cannot compute SiteCapabilities and per-site limited overrides are lost. Update the list path in tenantaccount.go to batch-load TenantSite rows for the tenants in tas, grouped by tenant ID similar to the existing allocation-count fetch, and pass the matching tenantSites into model.NewAPITenantAccount just like GetTenantAccountHandler and the PUT response do.
907-937: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHardcoded
allocationCount: 0in the siteCapabilities update response.
model.NewAPITenantAccount(ta, ssds, 0, tenantSites)always reports zero allocations, even thoughGetTenantAccountHandlercomputes the real value viaaDAO.GetCount(...)for the exact same entity. A Provider Admin updatingsiteCapabilitieson an active TenantAccount with existing allocations will get back a response claiming zero allocations.🐛 Proposed fix
+ aDAO := cdbm.NewAllocationDAO(utah.dbSession) + total := 0 + if ta.TenantID != nil { + cnt, cerr := aDAO.GetCount(ctx, nil, cdbm.AllocationFilterInput{ + InfrastructureProviderIDs: []uuid.UUID{ta.InfrastructureProviderID}, + TenantIDs: []uuid.UUID{*ta.TenantID}, + }) + if cerr != nil { + logger.Error().Err(cerr).Msg("error retrieving allocation count for Tenant Account from DB") + return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Allocations to determine total allocation for tenant account", nil) + } + total = cnt + } + sdDAO := cdbm.NewStatusDetailDAO(utah.dbSession) ssds, _, err := sdDAO.GetAll(ctx, nil, cdbm.StatusDetailFilterInput{EntityIDs: []string{ta.ID.String()}}, cdbp.PageInput{Limit: cutil.GetPtr(pagination.MaxPageSize)}) if err != nil { logger.Error().Err(err).Msg("error retrieving Status Details for TenantAccount from DB") return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Status Details for TenantAccount", nil) } - apiInstance := model.NewAPITenantAccount(ta, ssds, 0, tenantSites) + apiInstance := model.NewAPITenantAccount(ta, ssds, total, tenantSites)🤖 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 `@rest-api/api/pkg/api/handler/tenantaccount.go` around lines 907 - 937, The update response is hardcoding allocationCount to 0 in the call to model.NewAPITenantAccount, so the returned TenantAccount can incorrectly show no allocations. Reuse the same allocation-count logic already used in GetTenantAccountHandler by fetching the real count for ta.ID before building the response, then pass that value instead of 0 when constructing apiInstance.rest-api/api/pkg/api/handler/expectedmachine.go (1)
351-365: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMirror the TenantAccount fallback in the ExpectedMachine list filter.
GetAllExpectedMachineHandleronly addsTenantSite.SiteIDvalues tofilterInput.SiteIDs. A privileged tenant that is authorized through a readyTenantAccountbut has no explicitTenantSiterow for a provider site will get an empty result set, and asiteIdquery for that site will be rejected, even thoughValidateProviderOrTenantSiteAccessallows the same access path. Reuse the same access resolution here or extract a shared helper.🤖 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 `@rest-api/api/pkg/api/handler/expectedmachine.go` around lines 351 - 365, The ExpectedMachine list filter only uses TenantSite rows, so privileged tenants can miss access granted through a ready TenantAccount. Update GetAllExpectedMachineHandler to resolve site access the same way ValidateProviderOrTenantSiteAccess does, or extract and reuse a shared helper, so filterInput.SiteIDs includes both TenantSite.SiteID values and any provider sites reachable via the TenantAccount fallback before applying the list filter.Source: Path instructions
🧹 Nitpick comments (1)
rest-api/api/pkg/api/handler/tenantaccount.go (1)
785-932: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftAdd handler-level test coverage for the new provider siteCapabilities flow.
This introduces a non-trivial, security-sensitive authorization path (advisory-locked transaction, global vs. limited scope resolution, stale-override cleanup, cross-provider site scoping) with no accompanying
tenantaccount_test.gochanges in this layer. Given the privilege-granting nature of this endpoint, targeted tests (ownership rejection, stale-override cleanup, limited-site-not-owned-by-provider rejection) would materially reduce regression risk.Based on path instructions: "Review Go code for correctness, clean control flow, error handling, context propagation, test coverage, performance, and cohesive organization..."
🤖 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 `@rest-api/api/pkg/api/handler/tenantaccount.go` around lines 785 - 932, Add handler-level tests for handleProviderSiteCapabilitiesUpdate in tenantaccount.go to cover the new provider siteCapabilities flow. Exercise the security-sensitive branches by verifying ownership/provider rejection, limited-site validation when a Site is not owned by the Tenant Account Infrastructure Provider, and stale TenantSite override cleanup when a site is removed from the limited set. Use the existing UpdateTenantAccountHandler, TenantAccountDAO/TenantSiteDAO behavior, and the advisory-locked transaction path to locate the right handler logic.Source: Path instructions
🤖 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 `@rest-api/api/pkg/api/handler/expectedmachine.go`:
- Around line 351-365: The ExpectedMachine list filter only uses TenantSite
rows, so privileged tenants can miss access granted through a ready
TenantAccount. Update GetAllExpectedMachineHandler to resolve site access the
same way ValidateProviderOrTenantSiteAccess does, or extract and reuse a shared
helper, so filterInput.SiteIDs includes both TenantSite.SiteID values and any
provider sites reachable via the TenantAccount fallback before applying the list
filter.
In `@rest-api/api/pkg/api/handler/tenantaccount.go`:
- Around line 397-464: GetAllTenantAccountHandler is building APITenantAccount
list items with tenantSites left nil, so NewAPITenantAccount cannot compute
SiteCapabilities and per-site limited overrides are lost. Update the list path
in tenantaccount.go to batch-load TenantSite rows for the tenants in tas,
grouped by tenant ID similar to the existing allocation-count fetch, and pass
the matching tenantSites into model.NewAPITenantAccount just like
GetTenantAccountHandler and the PUT response do.
- Around line 907-937: The update response is hardcoding allocationCount to 0 in
the call to model.NewAPITenantAccount, so the returned TenantAccount can
incorrectly show no allocations. Reuse the same allocation-count logic already
used in GetTenantAccountHandler by fetching the real count for ta.ID before
building the response, then pass that value instead of 0 when constructing
apiInstance.
---
Nitpick comments:
In `@rest-api/api/pkg/api/handler/tenantaccount.go`:
- Around line 785-932: Add handler-level tests for
handleProviderSiteCapabilitiesUpdate in tenantaccount.go to cover the new
provider siteCapabilities flow. Exercise the security-sensitive branches by
verifying ownership/provider rejection, limited-site validation when a Site is
not owned by the Tenant Account Infrastructure Provider, and stale TenantSite
override cleanup when a site is removed from the limited set. Use the existing
UpdateTenantAccountHandler, TenantAccountDAO/TenantSiteDAO behavior, and the
advisory-locked transaction path to locate the right handler logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 337a24c6-0bfa-4a62-ab56-b6e48d7222c7
⛔ Files ignored due to path filters (40)
rest-api/sdk/standard/api_tenant_account.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/client.gois excluded by!rest-api/sdk/standard/client.gorest-api/sdk/standard/model_batch_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_interface_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_site_capability.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_site_capability_scope.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_capabilities.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_update_request.gois excluded by!rest-api/sdk/standard/model_*.go
📒 Files selected for processing (31)
rest-api/api/pkg/api/handler/dpureprovision_test.gorest-api/api/pkg/api/handler/expectedmachine.gorest-api/api/pkg/api/handler/expectedmachine_test.gorest-api/api/pkg/api/handler/instance.gorest-api/api/pkg/api/handler/instance_test.gorest-api/api/pkg/api/handler/machine.gorest-api/api/pkg/api/handler/machine_test.gorest-api/api/pkg/api/handler/machinehealth_test.gorest-api/api/pkg/api/handler/serviceaccount.gorest-api/api/pkg/api/handler/sku_test.gorest-api/api/pkg/api/handler/tenantaccount.gorest-api/api/pkg/api/handler/util/common/common.gorest-api/api/pkg/api/handler/util/common/common_test.gorest-api/api/pkg/api/handler/util/common/testing.gorest-api/api/pkg/api/handler/vpc.gorest-api/api/pkg/api/handler/vpc_test.gorest-api/api/pkg/api/model/operatingsystem_test.gorest-api/api/pkg/api/model/sshkeygroup_test.gorest-api/api/pkg/api/model/tenant.gorest-api/api/pkg/api/model/tenant_test.gorest-api/api/pkg/api/model/tenantaccount.gorest-api/api/pkg/api/model/tenantaccount_test.gorest-api/api/pkg/api/model/tenantaccountsitecapability.gorest-api/db/pkg/db/model/tenantaccount.gorest-api/db/pkg/db/model/tenantaccount_test.gorest-api/db/pkg/db/model/tenantsite.gorest-api/db/pkg/db/model/tenantsite_test.gorest-api/db/pkg/db/model/testing.gorest-api/db/pkg/migrations/20230412224951_tenant_site.gorest-api/db/pkg/migrations/20260701120000_tenant_account_site_capabilities.gorest-api/openapi/spec.yaml
✅ Files skipped from review due to trivial changes (2)
- rest-api/api/pkg/api/model/sshkeygroup_test.go
- rest-api/db/pkg/migrations/20230412224951_tenant_site.go
🚧 Files skipped from review as they are similar to previous changes (20)
- rest-api/api/pkg/api/model/tenantaccount_test.go
- rest-api/api/pkg/api/model/operatingsystem_test.go
- rest-api/api/pkg/api/model/tenant_test.go
- rest-api/db/pkg/migrations/20260701120000_tenant_account_site_capabilities.go
- rest-api/db/pkg/db/model/testing.go
- rest-api/openapi/spec.yaml
- rest-api/api/pkg/api/handler/vpc.go
- rest-api/api/pkg/api/handler/instance.go
- rest-api/api/pkg/api/handler/machine_test.go
- rest-api/api/pkg/api/model/tenantaccountsitecapability.go
- rest-api/api/pkg/api/handler/util/common/common.go
- rest-api/api/pkg/api/handler/util/common/common_test.go
- rest-api/api/pkg/api/model/tenant.go
- rest-api/api/pkg/api/handler/serviceaccount.go
- rest-api/db/pkg/db/model/tenantaccount_test.go
- rest-api/db/pkg/db/model/tenantsite.go
- rest-api/api/pkg/api/handler/machine.go
- rest-api/api/pkg/api/model/tenantaccount.go
- rest-api/db/pkg/db/model/tenantaccount.go
- rest-api/db/pkg/db/model/tenantsite_test.go
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
from
July 8, 2026 17:09
1375da6 to
04ce4d9
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Move privileged tenant capability configuration to TenantAccount.config and TenantSite.config, expose it on TenantAccount GET/PATCH, and enforce effective site-scoped checks on instance, machine, and VPC actions while deprecating the legacy Tenant.capabilities field.
… responses, and require explicit siteCapabilities updates
…sence, invite response overrides, and capability auth cleanup
hwadekar-nv
force-pushed
the
feat/tenant-capabilities-update
branch
from
July 22, 2026 02:35
50e13e0 to
7ee3450
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
Privileged tenant access (TargetedInstanceCreation) was stored on the global Tenant.config and surfaced as a single Tenant.capabilities.targetedInstanceCreation flag. That model is too coarse: the capability can differ by Provider and Site, and tenants may need per-site overrides. A tenant-wide flag also made it easy to over-grant access on list endpoints (e.g. machines/sites) when only some sites should be privileged.
What this PR does
Moves capability configuration and enforcement to the account/site model and makes authorization site-aware end to end.
Configuration model
TenantAccount API (siteCapabilities)
Provider Admin + siteCapabilities — replace capability configuration globally (scope: global) or for explicit sites (scope: limited + siteIds). Server validates: non-empty array, exactly one global entry, no duplicate siteIds, siteIds required for limited / omitted for global.
Tenant Admin + tenantContactId — accept an Invited account (existing flow). Cannot be combined with siteCapabilities in the same request (400).
Authorization / enforcement
TenantHasTargetedInstanceCreation(tenant, site?) — tenant-wide or effective per-site check (account default + TenantSite override)
GetPrivilegedAccessSiteIDsForTenant — returns the exact set of privileged site IDs
Sites, Machines, Expected Machines, Instances, VPCs, SKUs, etc.
API / docs
Typical capability setup flow
Reviewer focus areas
Closes #2104
Type of Change
Related Issues (Optional)
#2104
Breaking Changes
Testing
Additional Notes