feat: Add iPXE script template and new variant of operating system#2616
Open
hwadekar-nv wants to merge 1 commit into
Open
feat: Add iPXE script template and new variant of operating system#2616hwadekar-nv wants to merge 1 commit into
hwadekar-nv wants to merge 1 commit into
Conversation
Contributor
Summary by CodeRabbitRelease Notes
WalkthroughThis PR introduces a ChangesTemplated iPXE Operating System and Template API
Sequence Diagram(s)sequenceDiagram
participant Client
participant CreateOSHandler
participant IsProviderOrTenant
participant OSDAO
participant OSSiteAssocDAO
participant TemporalWorkflow
Client->>CreateOSHandler: POST /org/:org/nico/operating-system
CreateOSHandler->>IsProviderOrTenant: resolve provider/tenant role
CreateOSHandler->>CreateOSHandler: infer OS type, reject Image, enforce TemplatedIPXE=providerAdmin
CreateOSHandler->>CreateOSHandler: compute osScope and target site IDs (Global/Limited)
CreateOSHandler->>OSDAO: Create OS record (BeginTx)
CreateOSHandler->>OSSiteAssocDAO: Create OS-site associations (status=Syncing, same tx)
CreateOSHandler->>TemporalWorkflow: SynchronizeOperatingSystem (async, pre-commit)
CreateOSHandler->>CreateOSHandler: Commit()
CreateOSHandler-->>Client: 201 Created (status=Syncing)
sequenceDiagram
participant Client
participant GetAllIpxeTemplateHandler
participant IsProviderOrTenant
participant IpxeTemplateSiteAssocDAO
participant IpxeTemplateDAO
Client->>GetAllIpxeTemplateHandler: GET /org/:org/nico/ipxe-template[?siteId=...]
GetAllIpxeTemplateHandler->>IsProviderOrTenant: resolve provider/tenant role
alt siteId filter provided
GetAllIpxeTemplateHandler->>GetAllIpxeTemplateHandler: validate per-site authorization
else no siteId filter
GetAllIpxeTemplateHandler->>GetAllIpxeTemplateHandler: compute effective site set (provider ∪ tenant)
end
GetAllIpxeTemplateHandler->>IpxeTemplateSiteAssocDAO: GetAll(effectiveSiteIDs)
GetAllIpxeTemplateHandler->>IpxeTemplateDAO: GetAll(templateIDs, page)
GetAllIpxeTemplateHandler-->>Client: 200 OK with Pagination-Total header
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
hwadekar-nv
force-pushed
the
feat/ipxe-os
branch
from
79993a7 to
9e1f7c7
Compare
|
🌿 Preview your docs: https://nvidia-preview-pull-request-2616.docs.buildwithfern.com/infra-controller |
🔐 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-15 23:43:41 UTC | Commit: 9e1f7c7 |
![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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rest-api/api/pkg/api/handler/operatingsystem.go (1)
113-118:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftValidate the selected iPXE template before persisting create/update requests.
The handler only checks that
ipxeTemplateIdis non-empty, then saves the template ID, parameters, and artifacts directly. A caller can persist an OS referencing a missing/inaccessible template, omit required template parameters/artifacts, or supply reserved parameters; the first failure then shifts to synchronization/rendering. Load the template by ID/name before create/update, enforce template visibility, and validate required/reserved params/artifacts against the selected template.As per coding guidelines, authorization and validation that depends on DAO lookups belongs in handlers before persistence/translation.
Also applies to: 300-319, 1114-1128
🤖 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/operatingsystem.go` around lines 113 - 118, The operating system handler currently only validates that ipxeTemplateId is non-empty before persisting the create/update request, allowing callers to save references to missing/inaccessible templates and omit required template parameters or artifacts. Before calling the persistence layer in the create and update flows, load the selected iPXE template by ID/name to verify it exists and is accessible, enforce template visibility constraints, and validate that all required template parameters and artifacts are provided while rejecting any reserved parameters. This validation must occur in the handler before persistence to align with coding guidelines that require authorization and validation dependent on DAO lookups to be completed before persistence.Source: Coding guidelines
rest-api/sdk/standard/compat/instance_create.go (1)
19-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocstrings describe outdated migration paths in both deprecated wrappers. Both functions now delegate to constructors that accept
interfacesas a direct parameter, but the docstrings recommend migrating to a constructor-without-interfaces followed bySetInterfaces. Update both docstrings to reflect the currentstandardpackage API.
rest-api/sdk/standard/compat/instance_create.go#L19-L29: Update the migration guidance inNewInstanceCreateRequestWithInterfacesto match the 4-argumentstandard.NewInstanceCreateRequestsignature.rest-api/sdk/standard/compat/instance_create.go#L31-L40: Update the migration guidance inNewBatchInstanceCreateRequestWithInterfacesto match the 6-argumentstandard.NewBatchInstanceCreateRequestsignature.🤖 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/sdk/standard/compat/instance_create.go` around lines 19 - 29, The docstrings in both deprecated wrapper functions describe outdated migration paths that do not match the current standard package API. Update the docstring for NewInstanceCreateRequestWithInterfaces (rest-api/sdk/standard/compat/instance_create.go, lines 19-29) to accurately reflect that callers should migrate directly to the 4-argument standard.NewInstanceCreateRequest constructor that accepts interfaces as a parameter. Similarly, update the docstring for NewBatchInstanceCreateRequestWithInterfaces (rest-api/sdk/standard/compat/instance_create.go, lines 31-40) to accurately reflect the 6-argument standard.NewBatchInstanceCreateRequest constructor signature. Remove references to the two-step migration pattern (constructor followed by SetInterfaces) and instead document the single-step migration to the constructors that accept interfaces directly.
🧹 Nitpick comments (3)
rest-api/api/pkg/api/handler/ipxetemplate.go (2)
31-32: ⚡ Quick winConsolidate duplicate import aliases.
Both
cerrandsutilalias the same packagecommon/pkg/util. This creates unnecessary cognitive overhead when reading the code. Prefer a single alias and use it consistently throughout the file.♻️ Suggested consolidation
- cerr "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" - sutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" + cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util"Then update all
cerr.andsutil.references tocutil..🤖 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/ipxetemplate.go` around lines 31 - 32, The file imports the same package `github.com/NVIDIA/infra-controller/rest-api/common/pkg/util` twice with different aliases (`cerr` and `sutil`), which is redundant. Remove one of the duplicate import statements and keep a single import with a unified alias (such as `cutil`). Then find and replace all occurrences of both `cerr.` and `sutil.` prefixes throughout the file with the single consolidated alias to ensure consistent usage.
47-52: ⚡ Quick winUnused
tc tclient.Clientfield in both handler structs. BothGetAllIpxeTemplateHandlerandGetIpxeTemplateHandleraccept and store a Temporal client that is never referenced in theirHandlemethods. This is dead code that should be removed until workflow integration is actually needed.
rest-api/api/pkg/api/handler/ipxetemplate.go#L47-L52: Removetc tclient.Clientfield and update constructor signature forGetAllIpxeTemplateHandler.rest-api/api/pkg/api/handler/ipxetemplate.go#L243-L248: Removetc tclient.Clientfield and update constructor signature forGetIpxeTemplateHandler.Note: This will also require updating
routes.goto remove thetcargument from the handler constructor calls.🤖 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/ipxetemplate.go` around lines 47 - 52, The `tc tclient.Client` field is unused in both handler structs and should be removed. In rest-api/api/pkg/api/handler/ipxetemplate.go, remove the `tc tclient.Client` field from the `GetAllIpxeTemplateHandler` struct (lines 47-52) and update its constructor function to remove the `tc` parameter from the signature. Similarly, remove the `tc tclient.Client` field from the `GetIpxeTemplateHandler` struct (lines 243-248) and update its constructor function to remove the `tc` parameter. Additionally, update routes.go to remove the `tc` argument from all constructor calls to both `GetAllIpxeTemplateHandler` and `GetIpxeTemplateHandler`.Source: Coding guidelines
rest-api/api/pkg/api/handler/instancebatch.go (1)
182-197: ⚡ Quick winAdd a default case to handle unexpected OS types.
The switch statement covers the three known OS types but lacks a
defaultbranch. Ifos.Typecontains an unexpected value (e.g., a future OS type or data inconsistency), the function returns a config withnilVariant, which may cause obscure failures in the downstream Temporal workflow.Returning an explicit error for unknown types improves diagnosability and serves as a guard against incomplete updates when new OS types are introduced.
Suggested fix
case cdbm.OperatingSystemTypeImage: result.Variant = &cwssaws.InstanceOperatingSystemConfig_OsImageId{ OsImageId: &cwssaws.UUID{Value: os.ID.String()}, } + default: + logger.Error().Str("osType", os.Type).Msg("unsupported operating system type") + return nil, nil, cutil.NewAPIError(http.StatusBadRequest, fmt.Sprintf("Unsupported operating system type: %s", os.Type), 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/handler/instancebatch.go` around lines 182 - 197, The switch statement on os.Type in the instance batch handler is missing a default case to handle unexpected OS type values. Add a default case to this switch statement (which currently handles OperatingSystemTypeIPXE, OperatingSystemTypeTemplatedIPXE, and OperatingSystemTypeImage) that returns an explicit error when an unexpected os.Type is encountered. This prevents the function from returning a partially initialized config with a nil Variant field, which would cause obscure failures in downstream Temporal workflows and makes it easier to diagnose issues if new OS types are added in the future.
🤖 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/instance.go`:
- Around line 208-212: The TemplatedIPXE operating system selection is not
validating scope/site-association before sending the OS ID to Core, creating a
security issue where site-scoped templated OS can be selected outside their
allowed site. In rest-api/api/pkg/api/handler/instance.go at lines 208-212 (the
anchor case statement for cdbm.OperatingSystemTypeTemplatedIPXE), add a
scope/site-association guard before constructing the
InstanceOperatingSystemConfig_OperatingSystemId, reusing the same validation
logic already applied to Image types. Apply the identical guard at lines
2077-2081 (the sibling location in the instance update path) before allowing the
instance to switch to that TemplatedIPXE OS. This ensures that Operating System
accessibility is validated at both the create and update paths consistent with
the existing Image-type validation.
In `@rest-api/api/pkg/api/handler/operatingsystem_test.go`:
- Around line 1694-1701: This test case combines two invalid inputs (Image OS
type and missing deactivation note), making it ambiguous which validation check
causes the 400 response. To isolate the failure to the deactivation-note
validation only, change the osID from os11.ID.String() (an Image OS) to a
non-Image OS that allows deactivation, while keeping the missing deactivation
note. This ensures the test specifically validates the note-validation path, not
the image-type guard. Alternatively, if the intent is to test Image OS
rejection, rename the case descriptively and change the request body to include
a deactivation note so the failure is caused solely by the Image OS type check.
In `@rest-api/api/pkg/api/handler/operatingsystem.go`:
- Around line 406-414: The code enqueues Temporal workflows via
osWorkflow.ExecuteCreateOrUpdateOperatingSystemByIDWorkflow() before the
database transaction commits, creating a race condition where workflows may
execute against uncommitted or rolled-back data. Refactor this pattern at all
three affected sites (the ExecuteCreateOrUpdateOperatingSystemByIDWorkflow call
around line 406-414, and two additional locations around lines 1172-1178 and
1399-1410) by replacing the direct workflow enqueue with creation of a
transactional outbox or sync-job record that includes the workflow details and
target parameters. Commit this record as part of the same database transaction,
then implement a separate post-commit worker or hook that reads these committed
records and enqueues the actual workflows only after the transaction succeeds,
ensuring workflows never execute against uncommitted or rolled-back state.
- Around line 203-222: For Limited-scope tenant-owned Operating Systems, after
parsing and validating the requested site IDs in the requestedSiteIDs loop, add
authorization checks to verify that each requested site has a TenantSite
relationship with the current tenant before allowing the associations to be
created. This cross-resource ownership validation belongs in the handler and
must be applied at both affected locations:
rest-api/api/pkg/api/handler/operatingsystem.go lines 203-222 (where
requestedSiteIDs is populated) and lines 271-282 (the sibling site with the same
logic pattern). Return an appropriate authorization error if any requested site
is not accessible to the tenant.
- Around line 167-176: Refactor the manual transaction handling to use
closure-based transaction helpers from the db package. Replace the manual
BeginTx call, subsequent Commit, and the defer common.RollbackTx pattern with
cdb.WithTx or cdb.WithTxResult helpers. This applies to all instances where
BeginTx is called directly in the handler code. The closure-based approach
should wrap the database operations, with pure validation reads happening
outside the transaction and writes happening within it, and response values
either returned via WithTxResult or assigned to outer variables that persist
after the closure completes.
- Around line 1319-1323: The instance filter is being scoped by tenant ID
whenever tenant is not nil, but this is incorrect for provider-owned OS
deletions. When a dual-role caller deletes a provider-owned OS, the tenant
filter causes the check to only validate that specific tenant's instances,
missing instances from other tenants that are using the same provider OS. Modify
the logic in the InstanceFilterInput setup to only add the TenantIDs filter when
the OS being deleted is tenant-owned (not provider-owned). Check the OS object
to determine ownership (likely via os.TenantID) and only apply the tenant filter
to the instanceFilter if the OS is tenant-owned, allowing the provider-owned OS
deletion check to validate instances across all tenants.
- Around line 1436-1439: In the DeleteOsImageRequest construction in the
deleteOsImage handler, the TenantOrganizationId field is being set with
tenant.Org, which causes a panic when tenant is nil for provider-only callers
who are authorized to delete provider-owned images. Replace tenant.Org with the
org variable that is always available in this context, ensuring the request has
the correct organization ID regardless of whether the request is from a tenant
or provider-only caller.
In `@rest-api/api/pkg/api/model/operatingsystem.go`:
- Around line 736-739: The IpxeTemplateArtifacts field directly exposes the
database model cdbm.OperatingSystemIpxeArtifact in the API response, which
includes the sensitive AuthToken field that should never be returned to API
clients. Create an API-specific DTO struct in api/pkg/api/model/ that mirrors
cdbm.OperatingSystemIpxeArtifact but excludes the AuthToken field, then update
the IpxeTemplateArtifacts field to use this new API DTO type instead of the raw
database model. Apply the same DTO-based fix to all other locations where DB
artifact models are exposed in API responses.
- Around line 863-867: The validateIpxeTemplateArtifacts function currently
validates only that URLs are non-empty but does not validate their format,
allowing malformed URLs like "not-a-url" to persist. Add URL format validation
using the is.URL validator from ozzo-validation immediately after the
empty-string check in the validation logic for the artifact URL field, following
the same pattern already used for ImageURL validation elsewhere in the file.
Additionally, add a test case to operatingsystem_test.go that verifies the
validateIpxeTemplateArtifacts function rejects artifacts with malformed URLs
such as "not-a-url".
In `@rest-api/db/pkg/db/model/ipxetemplatesiteassociation.go`:
- Around line 215-227: The query filtering logic in the GetAll method fails to
distinguish between a nil filter (meaning no filter applied) and a non-nil but
empty slice (meaning match nothing). Currently, both
`len(filter.IpxeTemplateIDs) > 0` and `len(filter.SiteIDs) > 0` checks treat
empty slices as "no filter," allowing unintended data exposure when callers pass
empty authorization lists. Modify the conditions to explicitly check if the
filter is non-nil before checking its length; when a filter is non-nil but
empty, ensure the query is modified to return zero rows (for example, by adding
a WHERE clause that cannot be satisfied). Additionally, add a test case to the
DAO tests that verifies calling GetAll with
`IpxeTemplateSiteAssociationFilterInput{SiteIDs: []uuid.UUID{}}` returns zero
rows and zero total count.
In `@rest-api/db/pkg/db/model/operatingsystem_test.go`:
- Line 1244: In the operatingsystem_test.go file, the expectPhoneHomeEnabled
field is incorrectly assigned to updatedEnableBlockStorage instead of the
corresponding phone-home variable. Replace the updatedEnableBlockStorage
reference in the expectPhoneHomeEnabled assignment with the appropriate variable
that represents the updated phone-home enabled value (likely something like
updatedEnablePhoneHome or similar) to ensure the test properly validates
phone-home functionality changes rather than block-storage changes.
In `@rest-api/db/pkg/db/model/operatingsystem.go`:
- Around line 138-141: The FromProto methods in operatingsystem.go do not guard
against nil proto inputs before dereferencing, which causes panics. In the
OperatingSystemIpxeParameter.FromProto method (lines 138-141), add a nil check
at the beginning that clears the receiver to its zero value if the input
protoParam is nil, otherwise proceed with the existing logic. Apply the same nil
guard pattern to the other FromProto method located at lines 171-185 to ensure
both methods handle nil inputs consistently by resetting the receiver rather
than panicking.
- Around line 605-607: The scope filtering in the query construction (around the
filter.Scopes check where COALESCE is used) incorrectly includes image OS rows
by coalescing NULL ipxe_os_scope values to 'Local', causing them to match scope
filters they should not match. Modify the WHERE clause to apply scope filtering
only to iPXE OS rows that have a non-NULL ipxe_os_scope value, excluding image
OS rows entirely from the scope filter. Instead of using COALESCE to default
NULL to 'Local', add a condition to check that ipxe_os_scope is not NULL before
filtering on its actual value.
In `@rest-api/db/pkg/migrations/20260615120000_ipxe_os_and_templates.go`:
- Around line 159-161: The down migration function in the migration file must
not silently no-op and return nil, as this marks the migration as reverted while
the schema remains incompatible with the rolled-back application. Replace the
down migration implementation to either perform the actual schema reversals that
undo the up migration (such as recreating the dropped
controller_operating_system_id column and removing the created tables), or
return an explicit error to prevent false success. Ensure the down migration
properly reverses all schema changes made by the corresponding up migration to
maintain consistency between migration state and actual schema.
- Around line 134-149: The migration backfill logic in the
20260615120000_ipxe_os_and_templates.go migration is incomplete. The current
UPDATE statement only handles tenant-owned iPXE records (tenant_id IS NOT NULL)
by setting ipxe_os_scope to 'Global', but provider-owned iPXE records (where
tenant_id IS NULL) are left with NULL scope values. Add a second UPDATE
statement after the existing one that specifically targets provider-owned iPXE
records by checking for type = 'iPXE' AND tenant_id IS NULL, and sets their
ipxe_os_scope to 'Local' to fulfill the scope invariant defined in the migration
comment.
---
Outside diff comments:
In `@rest-api/api/pkg/api/handler/operatingsystem.go`:
- Around line 113-118: The operating system handler currently only validates
that ipxeTemplateId is non-empty before persisting the create/update request,
allowing callers to save references to missing/inaccessible templates and omit
required template parameters or artifacts. Before calling the persistence layer
in the create and update flows, load the selected iPXE template by ID/name to
verify it exists and is accessible, enforce template visibility constraints, and
validate that all required template parameters and artifacts are provided while
rejecting any reserved parameters. This validation must occur in the handler
before persistence to align with coding guidelines that require authorization
and validation dependent on DAO lookups to be completed before persistence.
In `@rest-api/sdk/standard/compat/instance_create.go`:
- Around line 19-29: The docstrings in both deprecated wrapper functions
describe outdated migration paths that do not match the current standard package
API. Update the docstring for NewInstanceCreateRequestWithInterfaces
(rest-api/sdk/standard/compat/instance_create.go, lines 19-29) to accurately
reflect that callers should migrate directly to the 4-argument
standard.NewInstanceCreateRequest constructor that accepts interfaces as a
parameter. Similarly, update the docstring for
NewBatchInstanceCreateRequestWithInterfaces
(rest-api/sdk/standard/compat/instance_create.go, lines 31-40) to accurately
reflect the 6-argument standard.NewBatchInstanceCreateRequest constructor
signature. Remove references to the two-step migration pattern (constructor
followed by SetInterfaces) and instead document the single-step migration to the
constructors that accept interfaces directly.
---
Nitpick comments:
In `@rest-api/api/pkg/api/handler/instancebatch.go`:
- Around line 182-197: The switch statement on os.Type in the instance batch
handler is missing a default case to handle unexpected OS type values. Add a
default case to this switch statement (which currently handles
OperatingSystemTypeIPXE, OperatingSystemTypeTemplatedIPXE, and
OperatingSystemTypeImage) that returns an explicit error when an unexpected
os.Type is encountered. This prevents the function from returning a partially
initialized config with a nil Variant field, which would cause obscure failures
in downstream Temporal workflows and makes it easier to diagnose issues if new
OS types are added in the future.
In `@rest-api/api/pkg/api/handler/ipxetemplate.go`:
- Around line 31-32: The file imports the same package
`github.com/NVIDIA/infra-controller/rest-api/common/pkg/util` twice with
different aliases (`cerr` and `sutil`), which is redundant. Remove one of the
duplicate import statements and keep a single import with a unified alias (such
as `cutil`). Then find and replace all occurrences of both `cerr.` and `sutil.`
prefixes throughout the file with the single consolidated alias to ensure
consistent usage.
- Around line 47-52: The `tc tclient.Client` field is unused in both handler
structs and should be removed. In rest-api/api/pkg/api/handler/ipxetemplate.go,
remove the `tc tclient.Client` field from the `GetAllIpxeTemplateHandler` struct
(lines 47-52) and update its constructor function to remove the `tc` parameter
from the signature. Similarly, remove the `tc tclient.Client` field from the
`GetIpxeTemplateHandler` struct (lines 243-248) and update its constructor
function to remove the `tc` parameter. Additionally, update routes.go to remove
the `tc` argument from all constructor calls to both `GetAllIpxeTemplateHandler`
and `GetIpxeTemplateHandler`.
🪄 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: 25a14688-954c-4cd6-9133-221195a3b5e3
⛔ Files ignored due to path filters (239)
rest-api/sdk/standard/api_allocation.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_audit.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_dpu_extension_service.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_expected_machine.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_expected_power_shelf.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_expected_switch.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_infini_band_partition.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_infrastructure_provider.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_instance.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_instance_type.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_ip_block.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_ipxe_template.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_machine.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_metadata.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_network_security_group.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_nv_link_logical_partition.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_operating_system.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_rack.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_service_account.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_site.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_sku.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_ssh_key.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_ssh_key_group.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_subnet.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_tenant.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_tenant_account.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_tray.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_user.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_vpc.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_vpc_peering.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_vpc_prefix.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/configuration.gois excluded by!rest-api/sdk/standard/configuration.gorest-api/sdk/standard/model_allocation.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_constraint.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_constraint_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_constraint_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_audit_entry.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_bring_up_rack_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_rack_firmware_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_tray_firmware_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_update_rack_power_state_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_update_tray_power_state_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_bmc_info.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_bring_up_rack_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_bring_up_rack_response.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_carbide_api_error.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_component_diff.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_deprecation.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_credentials.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_deployment.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_deployment_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_deployment_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_observability.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_observability_config.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_observability_logging.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_observability_prometheus.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_summary.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_version_info.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_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_field_diff.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_firmware_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_firmware_update_response.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_interface_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_interface_status.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_status.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_infrastructure_provider.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infrastructure_provider_stats.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_count_by_status.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_delete_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_status.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_allocation_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_capability_create_request.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_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_summary.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_interface_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_count_by_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_summary.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_usage_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ipxe_template.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ipxe_template_artifact.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ipxe_template_parameter.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_bmc_info.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_capability.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_count_by_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_dmi_data.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_gpu_info.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_gpu_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_health.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_health_issue.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_health_probe_alert.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_health_probe_success.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_infini_band_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_instance_type.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_instance_type_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_instance_type_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_instance_type_summary.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_metadata.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_network_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_status_breakdown.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_summary.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_metadata.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_propagation_details.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_propagation_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_rule.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_status.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_nv_link_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_interface_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_interface_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_logical_partition.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_logical_partition_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_logical_partition_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_logical_partition_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system_site_association.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_component.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_filter.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_location.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_task.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_validation_result.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_service_account.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_capabilities.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_contact.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_location.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats_by_allocation.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats_by_health.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats_by_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats_by_status_and_health.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_summary.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_chassis.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_components.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_cpu.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_ethernet_device.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_gpu.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_infiniband_device.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_memory.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_storage.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_tpm.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key_group.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key_group_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key_group_site_association.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key_group_site_association_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key_group_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key_group_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ssh_key_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_status_detail.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_subnet.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_subnet_count_by_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_subnet_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_subnet_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_subnet_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_count_by_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_account_status.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_tenant_instance_type_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tenant_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tray.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tray_filter.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_tray_position.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_update_power_state_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_update_power_state_response.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_user.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_count_by_status.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_peering.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_peering_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_peering_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_prefix.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_prefix_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_prefix_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_prefix_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_virtualization_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/response.gois excluded by!rest-api/sdk/standard/response.gorest-api/sdk/standard/utils.gois excluded by!rest-api/sdk/standard/utils.gorest-api/workflow-schema/schema/site-agent/workflows/v1/fmds_nico_grpc.pb.gois excluded by!**/*.pb.go,!rest-api/**/*.pb.go,!rest-api/**/*_grpc.pb.gorest-api/workflow-schema/schema/site-agent/workflows/v1/inventory.pb.gois excluded by!**/*.pb.go,!rest-api/**/*.pb.gorest-api/workflow-schema/schema/site-agent/workflows/v1/nico_nico.pb.gois excluded by!**/*.pb.go,!rest-api/**/*.pb.gorest-api/workflow-schema/schema/site-agent/workflows/v1/nico_nico_grpc.pb.gois excluded by!**/*.pb.go,!rest-api/**/*.pb.go,!rest-api/**/*_grpc.pb.gorest-api/workflow-schema/schema/site-agent/workflows/v1/nmx_c_nico_grpc.pb.gois excluded by!**/*.pb.go,!rest-api/**/*.pb.go,!rest-api/**/*_grpc.pb.gorest-api/workflow-schema/site-agent/workflows/v1/nico_nico.protois excluded by!rest-api/workflow-schema/site-agent/workflows/v1/*_nico.proto
📒 Files selected for processing (61)
rest-api/api/pkg/api/handler/instance.gorest-api/api/pkg/api/handler/instancebatch.gorest-api/api/pkg/api/handler/ipxetemplate.gorest-api/api/pkg/api/handler/ipxetemplate_test.gorest-api/api/pkg/api/handler/operatingsystem.gorest-api/api/pkg/api/handler/operatingsystem_ownership_test.gorest-api/api/pkg/api/handler/operatingsystem_test.gorest-api/api/pkg/api/handler/util/common/testing.gorest-api/api/pkg/api/model/ipxetemplate.gorest-api/api/pkg/api/model/operatingsystem.gorest-api/api/pkg/api/model/operatingsystem_test.gorest-api/api/pkg/api/routes.gorest-api/api/pkg/api/routes_test.gorest-api/db/pkg/db/model/ipxetemplate.gorest-api/db/pkg/db/model/ipxetemplate_test.gorest-api/db/pkg/db/model/ipxetemplatesiteassociation.gorest-api/db/pkg/db/model/ipxetemplatesiteassociation_test.gorest-api/db/pkg/db/model/operatingsystem.gorest-api/db/pkg/db/model/operatingsystem_test.gorest-api/db/pkg/db/model/operatingsystemsiteassociation.gorest-api/db/pkg/migrations/20260615120000_ipxe_os_and_templates.gorest-api/docs/index.htmlrest-api/openapi/spec.yamlrest-api/sdk/standard/compat/instance_create.gorest-api/site-agent/pkg/components/managers/ipxetemplate/access.gorest-api/site-agent/pkg/components/managers/ipxetemplate/cron.gorest-api/site-agent/pkg/components/managers/ipxetemplate/init.gorest-api/site-agent/pkg/components/managers/ipxetemplate/publisher.gorest-api/site-agent/pkg/components/managers/ipxetemplate/subscriber.gorest-api/site-agent/pkg/components/managers/manager.gorest-api/site-agent/pkg/components/managers/manageraccess.gorest-api/site-agent/pkg/components/managers/managerapi/ipxetemplate_api.gorest-api/site-agent/pkg/components/managers/managerapi/managerapi.gorest-api/site-agent/pkg/components/managers/operatingsystem/cron.gorest-api/site-agent/pkg/components/managers/operatingsystem/publisher.gorest-api/site-agent/pkg/components/managers/operatingsystem/subscriber.gorest-api/site-agent/pkg/components/managers/workflow/orchestrator.gorest-api/site-agent/pkg/datatypes/managertypes/workflow/workflowtypes.gorest-api/site-workflow/pkg/activity/instance_test.gorest-api/site-workflow/pkg/activity/ipxetemplate.gorest-api/site-workflow/pkg/activity/ipxetemplate_test.gorest-api/site-workflow/pkg/activity/operatingsystem.gorest-api/site-workflow/pkg/activity/operatingsystem_test.gorest-api/site-workflow/pkg/grpc/client/testing.gorest-api/site-workflow/pkg/workflow/ipxetemplate.gorest-api/site-workflow/pkg/workflow/ipxetemplate_test.gorest-api/site-workflow/pkg/workflow/operatingsystem.gorest-api/workflow-schema/site-agent/workflows/v1/inventory.protorest-api/workflow/cmd/workflow/main.gorest-api/workflow/pkg/activity/ipxetemplate/ipxetemplate.gorest-api/workflow/pkg/activity/ipxetemplate/ipxetemplate_test.gorest-api/workflow/pkg/activity/operatingsystem/operatingsystem.gorest-api/workflow/pkg/activity/operatingsystem/operatingsystem_test.gorest-api/workflow/pkg/activity/operatingsystem/push.gorest-api/workflow/pkg/util/testing.gorest-api/workflow/pkg/workflow/ipxetemplate/update.gorest-api/workflow/pkg/workflow/ipxetemplate/update_test.gorest-api/workflow/pkg/workflow/operatingsystem/delete.gorest-api/workflow/pkg/workflow/operatingsystem/delete_test.gorest-api/workflow/pkg/workflow/operatingsystem/synchronize.gorest-api/workflow/pkg/workflow/operatingsystem/update.go
Comment on lines
+208
to
+212
| case cdbm.OperatingSystemTypeTemplatedIPXE: | ||
| result.RunProvisioningInstructionsOnEveryBoot = *apiRequest.AlwaysBootWithCustomIpxe | ||
| result.Variant = &cwssaws.InstanceOperatingSystemConfig_OperatingSystemId{ | ||
| OperatingSystemId: &cwssaws.OperatingSystemId{Value: os.ID.String()}, | ||
| } |
Contributor
There was a problem hiding this comment.
Validate TemplatedIPXE scope/site access before sending the OS ID to Core. Both create and update now dispatch TemplatedIPXE via OperatingSystemId, but the visible site-association check remains Image-only, so a site-scoped templated OS can be selected outside its allowed site.
rest-api/api/pkg/api/handler/instance.go#L208-L212: add the TemplatedIPXE scope/site-association guard before constructingInstanceOperatingSystemConfig_OperatingSystemId.rest-api/api/pkg/api/handler/instance.go#L2077-L2081: reuse the same guard before allowing an instance update to switch to that TemplatedIPXE OS.
As per coding guidelines, REST API server changes must be reviewed for validation, authorization, tenant/resource ownership checks.
📍 Affects 1 file
rest-api/api/pkg/api/handler/instance.go#L208-L212(this comment)rest-api/api/pkg/api/handler/instance.go#L2077-L2081
🤖 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/instance.go` around lines 208 - 212, The
TemplatedIPXE operating system selection is not validating
scope/site-association before sending the OS ID to Core, creating a security
issue where site-scoped templated OS can be selected outside their allowed site.
In rest-api/api/pkg/api/handler/instance.go at lines 208-212 (the anchor case
statement for cdbm.OperatingSystemTypeTemplatedIPXE), add a
scope/site-association guard before constructing the
InstanceOperatingSystemConfig_OperatingSystemId, reusing the same validation
logic already applied to Image types. Apply the identical guard at lines
2077-2081 (the sibling location in the instance update path) before allowing the
instance to switch to that TemplatedIPXE OS. This ensures that Operating System
accessibility is validated at both the create and update paths consistent with
the existing Image-type validation.
Source: Coding guidelines
Comment on lines
+1694
to
1701
| name: "should reject deactivate on Image OS without Deactivation Note", | ||
| reqOrgName: ipOrg1, | ||
| user: user, | ||
| reqBody: string(okBodyDeactivateNoNote), | ||
| osID: os11.ID.String(), | ||
| expectedErr: false, | ||
| expectedStatus: http.StatusOK, | ||
|
|
||
| expectedName: updReqDeactivateNoNote.Name, | ||
| expectedDesc: updReqDeactivateNoNote.Description, | ||
| expectedIsActive: cutil.GetPtr(false), | ||
| expectedDeactivationNote: updReqDeactivateNoNote.DeactivationNote, | ||
| expectedErr: true, | ||
| expectedStatus: http.StatusBadRequest, | ||
| }, |
Contributor
There was a problem hiding this comment.
Isolate the deactivation-without-note failure branch.
This case currently combines two invalid inputs (Image OS + missing deactivation note), so the 400 can come from the image-type guard instead of the note-validation path. Please keep this case single-cause by targeting a non-Image OS (or renaming it to explicitly assert Image OS rejection only).
Proposed minimal fix
- name: "should reject deactivate on Image OS without Deactivation Note",
+ name: "should reject deactivate without Deactivation Note",
reqOrgName: ipOrg1,
user: user,
reqBody: string(okBodyDeactivateNoNote),
- osID: os11.ID.String(),
+ osID: os1.ID.String(), // iPXE OS isolates note-validation branch
expectedErr: true,
expectedStatus: http.StatusBadRequest,Based on learnings: failure-path tests should intentionally target one validation branch and satisfy all other preconditions so the asserted status is caused by that exact check.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| name: "should reject deactivate on Image OS without Deactivation Note", | |
| reqOrgName: ipOrg1, | |
| user: user, | |
| reqBody: string(okBodyDeactivateNoNote), | |
| osID: os11.ID.String(), | |
| expectedErr: false, | |
| expectedStatus: http.StatusOK, | |
| expectedName: updReqDeactivateNoNote.Name, | |
| expectedDesc: updReqDeactivateNoNote.Description, | |
| expectedIsActive: cutil.GetPtr(false), | |
| expectedDeactivationNote: updReqDeactivateNoNote.DeactivationNote, | |
| expectedErr: true, | |
| expectedStatus: http.StatusBadRequest, | |
| }, | |
| name: "should reject deactivate without Deactivation Note", | |
| reqOrgName: ipOrg1, | |
| user: user, | |
| reqBody: string(okBodyDeactivateNoNote), | |
| osID: os1.ID.String(), // iPXE OS isolates note-validation branch | |
| expectedErr: true, | |
| expectedStatus: http.StatusBadRequest, |
🤖 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/operatingsystem_test.go` around lines 1694 -
1701, This test case combines two invalid inputs (Image OS type and missing
deactivation note), making it ambiguous which validation check causes the 400
response. To isolate the failure to the deactivation-note validation only,
change the osID from os11.ID.String() (an Image OS) to a non-Image OS that
allows deactivation, while keeping the missing deactivation note. This ensures
the test specifically validates the note-validation path, not the image-type
guard. Alternatively, if the intent is to test Image OS rejection, rename the
case descriptively and change the request body to include a deactivation note so
the failure is caused solely by the Image OS type check.
Source: Learnings
Comment on lines
+167
to
+176
| // Start a db tx | ||
| tx, err := cdb.BeginTx(ctx, csh.dbSession, &sql.TxOptions{}) | ||
| if err != nil { | ||
| logger.Error().Err(err).Msg("db error retrieving TenantSite records for Tenant") | ||
| return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Site associations for Tenant, DB error", nil) | ||
| logger.Error().Err(err).Msg("unable to start transaction") | ||
| return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to create Operating System", nil) | ||
| } | ||
| for _, ts := range tss { | ||
| cts := ts | ||
| sttsmap[ts.SiteID] = &cts | ||
|
|
||
| // This variable is used in cleanup actions to indicate if this transaction committed | ||
| txCommitted := false | ||
| defer common.RollbackTx(ctx, tx, &txCommitted) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Use cdb.WithTx / cdb.WithTxResult instead of manual transactions.
These paths manually call BeginTx, Commit, and deferred rollback, and the create path opens the transaction before pure site-validation reads. Refactor to the closure helpers, start the transaction when writes begin, and return response values via WithTxResult or outer variables consistently.
As per coding guidelines, handler code that touches the database uses closure-based transaction helpers from db/pkg/db, not manual BeginTx/Commit/Rollback.
Also applies to: 1081-1088, 1334-1342
🤖 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/operatingsystem.go` around lines 167 - 176,
Refactor the manual transaction handling to use closure-based transaction
helpers from the db package. Replace the manual BeginTx call, subsequent Commit,
and the defer common.RollbackTx pattern with cdb.WithTx or cdb.WithTxResult
helpers. This applies to all instances where BeginTx is called directly in the
handler code. The closure-based approach should wrap the database operations,
with pure validation reads happening outside the transaction and writes
happening within it, and response values either returned via WithTxResult or
assigned to outer variables that persist after the closure completes.
Source: Coding guidelines
Comment on lines
+203
to
+222
| if isLimited { | ||
| // Limited-scope iPXE: resolve the explicitly requested site IDs. | ||
| if ip == nil { | ||
| ip, err = common.GetInfrastructureProviderForOrg(ctx, nil, csh.dbSession, org) | ||
| if err != nil { | ||
| logger.Error().Err(err).Msg("error retrieving Infrastructure Provider for org") | ||
| return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Infrastructure Provider for org", nil) | ||
| } | ||
| } | ||
|
|
||
| requestedSiteIDs := make([]uuid.UUID, 0, len(apiRequest.SiteIDs)) | ||
| for _, stID := range apiRequest.SiteIDs { | ||
| parsed, perr := uuid.Parse(stID) | ||
| if perr != nil { | ||
| return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, invalid Site ID: %s", stID), nil) | ||
| } | ||
| if serr == cdb.ErrDoesNotExist { | ||
| return cutil.NewAPIErrorResponse(c, http.StatusNotFound, fmt.Sprintf("Failed to create Operating System, could not find Site with ID: %s ", stID), nil) | ||
| requestedSiteIDs = append(requestedSiteIDs, parsed) | ||
| } | ||
| siteFilter.SiteIDs = requestedSiteIDs | ||
| runSiteQuery = len(requestedSiteIDs) > 0 |
Contributor
There was a problem hiding this comment.
Check TenantSite access for tenant-owned Limited-scope OSes.
For Limited scope, tenant-only callers cause the org provider to be fetched and requested sites are only checked against provider ownership. That lets a tenant admin create associations to any registered provider site, even without a TenantSite relationship. When the OS will be tenant-owned, require every requested site to be accessible to that tenant before creating associations.
Gate requested Limited-scope sites by tenant access
siteFilter.SiteIDs = requestedSiteIDs
runSiteQuery = len(requestedSiteIDs) > 0
+
+ if tenant != nil && !allowedByProvider {
+ tenantSiteIDs, tserr := getTenantSiteIDs(ctx, csh.dbSession, tenant.ID)
+ if tserr != nil {
+ logger.Error().Err(tserr).Msg("error retrieving tenant site IDs for limited-scope iPXE OS")
+ return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to determine site access for tenant", nil)
+ }
+ tenantSiteSet := make(map[uuid.UUID]struct{}, len(tenantSiteIDs))
+ for _, siteID := range tenantSiteIDs {
+ tenantSiteSet[siteID] = struct{}{}
+ }
+ for _, siteID := range requestedSiteIDs {
+ if _, ok := tenantSiteSet[siteID]; !ok {
+ return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Caller is not associated with one or more requested sites", nil)
+ }
+ }
+ }
} else if isGlobal {As per coding guidelines, authorization checks, including cross-resource ownership, belong in handlers.
Also applies to: 271-282
🤖 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/operatingsystem.go` around lines 203 - 222, For
Limited-scope tenant-owned Operating Systems, after parsing and validating the
requested site IDs in the requestedSiteIDs loop, add authorization checks to
verify that each requested site has a TenantSite relationship with the current
tenant before allowing the associations to be created. This cross-resource
ownership validation belongs in the handler and must be applied at both affected
locations: rest-api/api/pkg/api/handler/operatingsystem.go lines 203-222 (where
requestedSiteIDs is populated) and lines 271-282 (the sibling site with the same
logic pattern). Return an appropriate authorization error if any requested site
is not accessible to the tenant.
Source: Coding guidelines
Comment on lines
+406
to
+414
| // Trigger async workflow before committing so a failure to enqueue rolls back the transaction. | ||
| // Note: first run WILL fail since data is not committed so we rely on retry. We choose that initial inocuous failure vs failing to queue silently. | ||
| if cdbm.IsIPXEType(osType) && len(dbossa) > 0 { | ||
| wid, werr := osWorkflow.ExecuteCreateOrUpdateOperatingSystemByIDWorkflow(ctx, csh.tc, targetSiteIDs, os.ID) | ||
| if werr != nil { | ||
| logger.Error().Err(werr).Msg("failed to trigger SynchronizeOperatingSystem workflow for create") | ||
| return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to trigger Operating System synchronization workflow", nil) | ||
| } | ||
| logger.Info().Str("Workflow ID", *wid).Interface("Site IDs", targetSiteIDs).Msg("triggered async CreateOrUpdateOperatingSystemByID workflow for create") |
Contributor
There was a problem hiding this comment.
Do not enqueue workflows against uncommitted rows.
Each path starts a Temporal workflow before Commit() and relies on workflow retry because the row is not visible yet. If enqueue succeeds and commit later fails, a workflow is queued for rolled-back state; if the workflow observes the missing row before commit, the behavior depends on retry policy rather than the API transaction. Prefer a transactional outbox/sync-job record committed with the DB changes, then enqueue from a post-commit worker.
Also applies to: 1172-1178, 1399-1410
🤖 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/operatingsystem.go` around lines 406 - 414, The
code enqueues Temporal workflows via
osWorkflow.ExecuteCreateOrUpdateOperatingSystemByIDWorkflow() before the
database transaction commits, creating a race condition where workflows may
execute against uncommitted or rolled-back data. Refactor this pattern at all
three affected sites (the ExecuteCreateOrUpdateOperatingSystemByIDWorkflow call
around line 406-414, and two additional locations around lines 1172-1178 and
1399-1410) by replacing the direct workflow enqueue with creation of a
transactional outbox or sync-job record that includes the workflow details and
target parameters. Commit this record as part of the same database transaction,
then implement a separate post-commit worker or hook that reads these committed
records and enqueues the actual workflows only after the transaction succeeds,
ensuring workflows never execute against uncommitted or rolled-back state.
| expectedIsCloudInit: &updatedIsCloudInit, | ||
| expectedAllowOverride: &updatedAllowOverride, | ||
| expectedEnableBlockStorage: &updatedEnableBlockStorage, | ||
| expectPhoneHomeEnabled: &updatedEnableBlockStorage, |
Contributor
There was a problem hiding this comment.
Assert the updated phone-home value, not block-storage.
This expectation is wired to updatedEnableBlockStorage; it only passes because both booleans are currently false, masking regressions in PhoneHomeEnabled.
Proposed fix
- expectPhoneHomeEnabled: &updatedEnableBlockStorage,
+ expectPhoneHomeEnabled: &updatedPhoneHomeEnabled,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expectPhoneHomeEnabled: &updatedEnableBlockStorage, | |
| expectPhoneHomeEnabled: &updatedPhoneHomeEnabled, |
🤖 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/operatingsystem_test.go` at line 1244, In the
operatingsystem_test.go file, the expectPhoneHomeEnabled field is incorrectly
assigned to updatedEnableBlockStorage instead of the corresponding phone-home
variable. Replace the updatedEnableBlockStorage reference in the
expectPhoneHomeEnabled assignment with the appropriate variable that represents
the updated phone-home enabled value (likely something like
updatedEnablePhoneHome or similar) to ensure the test properly validates
phone-home functionality changes rather than block-storage changes.
Comment on lines
+138
to
+141
| // FromProto converts a proto IpxeTemplateParameter to an OperatingSystemIpxeParameter | ||
| func (osip *OperatingSystemIpxeParameter) FromProto(protoParam *ws.IpxeTemplateParameter) { | ||
| osip.Name = protoParam.Name | ||
| osip.Value = protoParam.Value |
Contributor
There was a problem hiding this comment.
Guard nil proto inputs before dereferencing.
Both FromProto methods panic on nil inputs. Clear the receiver on nil so repeated proto fields with nil elements or omitted leaf values reset cleanly instead of crashing sync.
Proposed fix
func (osip *OperatingSystemIpxeParameter) FromProto(protoParam *ws.IpxeTemplateParameter) {
+ if protoParam == nil {
+ *osip = OperatingSystemIpxeParameter{}
+ return
+ }
osip.Name = protoParam.Name
osip.Value = protoParam.Value
}
@@
func (osia *OperatingSystemIpxeArtifact) FromProto(protoArtifact *ws.IpxeTemplateArtifact) {
+ if protoArtifact == nil {
+ *osia = OperatingSystemIpxeArtifact{}
+ return
+ }
osia.Name = protoArtifact.Name
osia.URL = protoArtifact.Url
osia.SHA = protoArtifact.ShaAs per coding guidelines, FromProto on a leaf named type should mutate the receiver in place and a nil input clears the receiver to its zero value.
Also applies to: 171-185
🤖 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/operatingsystem.go` around lines 138 - 141, The
FromProto methods in operatingsystem.go do not guard against nil proto inputs
before dereferencing, which causes panics. In the
OperatingSystemIpxeParameter.FromProto method (lines 138-141), add a nil check
at the beginning that clears the receiver to its zero value if the input
protoParam is nil, otherwise proceed with the existing logic. Apply the same nil
guard pattern to the other FromProto method located at lines 171-185 to ensure
both methods handle nil inputs consistently by resetting the receiver rather
than panicking.
Source: Coding guidelines
Comment on lines
+605
to
+607
| if filter.Scopes != nil { | ||
| query = query.Where("COALESCE(os.ipxe_os_scope, 'Local') IN (?)", bun.In(filter.Scopes)) | ||
| ossd.tracerSpan.SetAttribute(operatingSystemSQLDAOSpan, "scopes", filter.Scopes) |
Contributor
There was a problem hiding this comment.
Restrict scope filtering to iPXE OS rows.
COALESCE(os.ipxe_os_scope, 'Local') makes image OS rows match Scopes: ["Local"], even though image scope is intentionally NULL and not applicable. This can leak image rows into iPXE-scope queries.
Proposed fix
if filter.Scopes != nil {
- query = query.Where("COALESCE(os.ipxe_os_scope, 'Local') IN (?)", bun.In(filter.Scopes))
+ query = query.Where(
+ "os.type IN (?) AND COALESCE(os.ipxe_os_scope, ?) IN (?)",
+ bun.In([]string{OperatingSystemTypeIPXE, OperatingSystemTypeTemplatedIPXE}),
+ OperatingSystemScopeLocal,
+ bun.In(filter.Scopes),
+ )
ossd.tracerSpan.SetAttribute(operatingSystemSQLDAOSpan, "scopes", filter.Scopes)
}🤖 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/operatingsystem.go` around lines 605 - 607, The
scope filtering in the query construction (around the filter.Scopes check where
COALESCE is used) incorrectly includes image OS rows by coalescing NULL
ipxe_os_scope values to 'Local', causing them to match scope filters they should
not match. Modify the WHERE clause to apply scope filtering only to iPXE OS rows
that have a non-NULL ipxe_os_scope value, excluding image OS rows entirely from
the scope filter. Instead of using COALESCE to default NULL to 'Local', add a
condition to check that ipxe_os_scope is not NULL before filtering on its actual
value.
Comment on lines
+134
to
+149
| // ── Backfill ipxe_os_scope for existing iPXE-type OS records ──── | ||
| // | ||
| // Tenant-owned raw iPXE → Global (preserves legacy behavior: tenant | ||
| // can use it for any Instance at any accessible site). | ||
| // Provider-owned iPXE (from nico-core inventory) → Local (single | ||
| // site, bidirectional sync). | ||
| // Image-type OS entries are left as NULL since scope does not apply. | ||
|
|
||
| _, err = tx.Exec(` | ||
| UPDATE operating_system | ||
| SET ipxe_os_scope = 'Global' | ||
| WHERE ipxe_os_scope IS NULL | ||
| AND type = 'iPXE' | ||
| AND tenant_id IS NOT NULL | ||
| AND deleted IS NULL | ||
| `) |
Contributor
There was a problem hiding this comment.
Backfill provider-owned iPXE rows to Local.
The migration comment defines provider-owned iPXE as Local, but the SQL only updates tenant-owned rows to Global. Existing provider iPXE records remain NULL, breaking the persisted scope invariant for iPXE rows and any consumer that does not apply a DAO-side COALESCE.
Proposed fix
_, err = tx.Exec(`
UPDATE operating_system
SET ipxe_os_scope = 'Global'
WHERE ipxe_os_scope IS NULL
AND type = 'iPXE'
AND tenant_id IS NOT NULL
AND deleted IS NULL
`)
handleError(tx, err)
+
+ _, err = tx.Exec(`
+ UPDATE operating_system
+ SET ipxe_os_scope = 'Local'
+ WHERE ipxe_os_scope IS NULL
+ AND type = 'iPXE'
+ AND infrastructure_provider_id IS NOT NULL
+ AND tenant_id IS NULL
+ AND deleted IS NULL
+ `)
+ handleError(tx, err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // ── Backfill ipxe_os_scope for existing iPXE-type OS records ──── | |
| // | |
| // Tenant-owned raw iPXE → Global (preserves legacy behavior: tenant | |
| // can use it for any Instance at any accessible site). | |
| // Provider-owned iPXE (from nico-core inventory) → Local (single | |
| // site, bidirectional sync). | |
| // Image-type OS entries are left as NULL since scope does not apply. | |
| _, err = tx.Exec(` | |
| UPDATE operating_system | |
| SET ipxe_os_scope = 'Global' | |
| WHERE ipxe_os_scope IS NULL | |
| AND type = 'iPXE' | |
| AND tenant_id IS NOT NULL | |
| AND deleted IS NULL | |
| `) | |
| // ── Backfill ipxe_os_scope for existing iPXE-type OS records ──── | |
| // | |
| // Tenant-owned raw iPXE → Global (preserves legacy behavior: tenant | |
| // can use it for any Instance at any accessible site). | |
| // Provider-owned iPXE (from nico-core inventory) → Local (single | |
| // site, bidirectional sync). | |
| // Image-type OS entries are left as NULL since scope does not apply. | |
| _, err = tx.Exec(` | |
| UPDATE operating_system | |
| SET ipxe_os_scope = 'Global' | |
| WHERE ipxe_os_scope IS NULL | |
| AND type = 'iPXE' | |
| AND tenant_id IS NOT NULL | |
| AND deleted IS NULL | |
| `) | |
| handleError(tx, err) | |
| _, err = tx.Exec(` | |
| UPDATE operating_system | |
| SET ipxe_os_scope = 'Local' | |
| WHERE ipxe_os_scope IS NULL | |
| AND type = 'iPXE' | |
| AND infrastructure_provider_id IS NOT NULL | |
| AND tenant_id IS NULL | |
| AND deleted IS NULL | |
| `) | |
| handleError(tx, err) |
🤖 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/migrations/20260615120000_ipxe_os_and_templates.go` around
lines 134 - 149, The migration backfill logic in the
20260615120000_ipxe_os_and_templates.go migration is incomplete. The current
UPDATE statement only handles tenant-owned iPXE records (tenant_id IS NOT NULL)
by setting ipxe_os_scope to 'Global', but provider-owned iPXE records (where
tenant_id IS NULL) are left with NULL scope values. Add a second UPDATE
statement after the existing one that specifically targets provider-owned iPXE
records by checking for type = 'iPXE' AND tenant_id IS NULL, and sets their
ipxe_os_scope to 'Local' to fulfill the scope invariant defined in the migration
comment.
Comment on lines
+159
to
+161
| }, func(ctx context.Context, db *bun.DB) error { | ||
| fmt.Print(" [down migration] No action taken") | ||
| return nil |
Contributor
There was a problem hiding this comment.
Do not silently no-op the down migration.
This up migration creates tables/columns and drops controller_operating_system_id; returning nil on down can mark the migration as reverted while leaving the schema incompatible with the rolled-back application. Implement a real reverse migration, or fail explicitly so rollback cannot proceed with false migration state.
Minimum safe guard
}, func(ctx context.Context, db *bun.DB) error {
- fmt.Print(" [down migration] No action taken")
- return nil
+ return fmt.Errorf("down migration 20260615120000_ipxe_os_and_templates is destructive and must be implemented before rollback")
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| }, func(ctx context.Context, db *bun.DB) error { | |
| fmt.Print(" [down migration] No action taken") | |
| return nil | |
| }, func(ctx context.Context, db *bun.DB) error { | |
| return fmt.Errorf("down migration 20260615120000_ipxe_os_and_templates is destructive and must be implemented before rollback") | |
| }) |
🤖 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/migrations/20260615120000_ipxe_os_and_templates.go` around
lines 159 - 161, The down migration function in the migration file must not
silently no-op and return nil, as this marks the migration as reverted while the
schema remains incompatible with the rolled-back application. Replace the down
migration implementation to either perform the actual schema reversals that undo
the up migration (such as recreating the dropped controller_operating_system_id
column and removing the created tables), or return an explicit error to prevent
false success. Ensure the down migration properly reverses all schema changes
made by the corresponding up migration to maintain consistency between migration
state and actual schema.
ianderson-nvidia
requested changes
Jun 16, 2026
Contributor
There was a problem hiding this comment.
This PR needs to be broken up into smaller PR's. I can't make heads or tails of what this PR is trying to accomplish.
The object renaming is also adding to the confusion; in some instances, the changes are from nico -> carbide, in other instances, it's from carbide to nico (see rest-api/sdk/standard/api_machine.go`). Don't do a massive rename as part of a feature branch.
Whatever AI helped author this PR also removed code comments, see rest-api/sdk/standard/model_machine_infini_band_interface.go, which seems unnecessary.
Please fix the PR to only add/change the code needed to complete the feature of the PR title. If you want to open follow-up PRs for the renaming, that's fine, but it should be its own distinct PR
The failing CI tests also need to be addressed.
hwadekar-nv
force-pushed
the
feat/ipxe-os
branch
from
9e1f7c7 to
66f7927
Compare
🔍 Container Scan SummaryNo Grype artifacts were found to aggregate. |
Contributor
There was a problem hiding this comment.
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 `@rest-api/api/pkg/api/handler/instancebatch.go`:
- Around line 182-198: The switch statement handling os.Type cases
(OperatingSystemTypeIPXE, OperatingSystemTypeTemplatedIPXE,
OperatingSystemTypeImage) is missing a default case, allowing unknown OS types
to return success with a nil Variant field, creating an invalid
InstanceOperatingSystemConfig. Add a default case to the switch statement that
returns an error for any unsupported or unknown os.Type values, preventing the
function from silently returning an incomplete configuration that would defer
failure to the workflow layer.
🪄 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: 30cd10f4-1305-4856-a0fa-456b42b90882
⛔ Files ignored due to path filters (182)
rest-api/sdk/standard/api_allocation.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_audit.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_dpu_extension_service.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_expected_machine.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_expected_power_shelf.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_expected_switch.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_infini_band_partition.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_infrastructure_provider.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_instance.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_instance_type.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_ip_block.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_ipxe_template.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_machine.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_metadata.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_network_security_group.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_nv_link_logical_partition.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_operating_system.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_rack.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_service_account.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_site.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_sku.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_ssh_key.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_ssh_key_group.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_subnet.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_tenant.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_tenant_account.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_tray.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_user.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_vpc.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_vpc_peering.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_vpc_prefix.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/configuration.gois excluded by!rest-api/sdk/standard/configuration.gorest-api/sdk/standard/model_allocation.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_constraint.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_constraint_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_constraint_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_allocation_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_audit_entry.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_bring_up_rack_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_rack_firmware_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_tray_firmware_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_update_rack_power_state_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_batch_update_tray_power_state_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_bmc_info.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_bring_up_rack_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_bring_up_rack_response.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_carbide_api_error.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_component_diff.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_deprecation.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_credentials.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_deployment.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_deployment_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_deployment_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_observability.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_observability_config.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_observability_logging.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_observability_prometheus.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_summary.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_dpu_extension_service_version_info.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_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_field_diff.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_firmware_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_firmware_update_response.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_interface_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_interface_status.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_status.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_infrastructure_provider.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infrastructure_provider_stats.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_count_by_status.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_delete_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_status.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_allocation_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_capability_create_request.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_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_summary.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_interface_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_count_by_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_summary.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ip_block_usage_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ipxe_template.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ipxe_template_artifact.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_ipxe_template_parameter.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_bmc_info.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_capability.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_count_by_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_dmi_data.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_gpu_info.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_gpu_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_health.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_health_issue.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_health_probe_alert.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_health_probe_success.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_infini_band_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_instance_type.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_instance_type_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_instance_type_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_instance_type_summary.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_metadata.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_network_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_status_breakdown.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_summary.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_metadata.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_propagation_details.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_propagation_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_rule.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_status.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_nv_link_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_interface_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_interface_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_logical_partition.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_logical_partition_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_logical_partition_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_nv_link_logical_partition_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system_site_association.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_operating_system_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_component.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_filter.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_location.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_task.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_rack_validation_result.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_service_account.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_capabilities.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_contact.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_location.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats_by_allocation.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats_by_health.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats_by_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_machine_stats_by_status_and_health.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_summary.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_site_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_chassis.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_sku_components.gois excluded by!rest-api/sdk/standard/model_*.go
📒 Files selected for processing (24)
rest-api/api/pkg/api/handler/instance.gorest-api/api/pkg/api/handler/instancebatch.gorest-api/api/pkg/api/handler/ipxetemplate.gorest-api/api/pkg/api/handler/ipxetemplate_test.gorest-api/api/pkg/api/handler/operatingsystem.gorest-api/api/pkg/api/handler/operatingsystem_ownership_test.gorest-api/api/pkg/api/handler/operatingsystem_test.gorest-api/api/pkg/api/handler/util/common/testing.gorest-api/api/pkg/api/model/ipxetemplate.gorest-api/api/pkg/api/model/operatingsystem.gorest-api/api/pkg/api/model/operatingsystem_test.gorest-api/api/pkg/api/routes.gorest-api/api/pkg/api/routes_test.gorest-api/db/pkg/db/model/ipxetemplate.gorest-api/db/pkg/db/model/ipxetemplate_test.gorest-api/db/pkg/db/model/ipxetemplatesiteassociation.gorest-api/db/pkg/db/model/ipxetemplatesiteassociation_test.gorest-api/db/pkg/db/model/operatingsystem.gorest-api/db/pkg/db/model/operatingsystem_test.gorest-api/db/pkg/db/model/operatingsystemsiteassociation.gorest-api/db/pkg/migrations/20260615120000_ipxe_os_and_templates.gorest-api/docs/index.htmlrest-api/openapi/spec.yamlrest-api/sdk/standard/compat/instance_create.go
💤 Files with no reviewable changes (1)
- rest-api/sdk/standard/compat/instance_create.go
🚧 Files skipped from review as they are similar to previous changes (20)
- rest-api/api/pkg/api/routes_test.go
- rest-api/api/pkg/api/handler/util/common/testing.go
- rest-api/api/pkg/api/routes.go
- rest-api/db/pkg/db/model/ipxetemplatesiteassociation_test.go
- rest-api/api/pkg/api/model/ipxetemplate.go
- rest-api/db/pkg/migrations/20260615120000_ipxe_os_and_templates.go
- rest-api/api/pkg/api/handler/ipxetemplate.go
- rest-api/api/pkg/api/handler/instance.go
- rest-api/db/pkg/db/model/ipxetemplate_test.go
- rest-api/db/pkg/db/model/ipxetemplate.go
- rest-api/db/pkg/db/model/operatingsystemsiteassociation.go
- rest-api/api/pkg/api/handler/operatingsystem_test.go
- rest-api/api/pkg/api/handler/operatingsystem_ownership_test.go
- rest-api/api/pkg/api/handler/ipxetemplate_test.go
- rest-api/api/pkg/api/model/operatingsystem_test.go
- rest-api/db/pkg/db/model/ipxetemplatesiteassociation.go
- rest-api/api/pkg/api/model/operatingsystem.go
- rest-api/db/pkg/db/model/operatingsystem_test.go
- rest-api/api/pkg/api/handler/operatingsystem.go
- rest-api/db/pkg/db/model/operatingsystem.go
Comment on lines
+182
to
+198
| switch os.Type { | ||
| case cdbm.OperatingSystemTypeIPXE: | ||
| result.RunProvisioningInstructionsOnEveryBoot = *apiRequest.AlwaysBootWithCustomIpxe | ||
| result.Variant = &cwssaws.InstanceOperatingSystemConfig_Ipxe{ | ||
| Ipxe: &cwssaws.InlineIpxe{IpxeScript: *apiRequest.IpxeScript}, | ||
| } | ||
| case cdbm.OperatingSystemTypeTemplatedIPXE: | ||
| result.RunProvisioningInstructionsOnEveryBoot = *apiRequest.AlwaysBootWithCustomIpxe | ||
| result.Variant = &cwssaws.InstanceOperatingSystemConfig_OperatingSystemId{ | ||
| OperatingSystemId: &cwssaws.OperatingSystemId{Value: os.ID.String()}, | ||
| } | ||
| case cdbm.OperatingSystemTypeImage: | ||
| result.Variant = &cwssaws.InstanceOperatingSystemConfig_OsImageId{ | ||
| OsImageId: &cwssaws.UUID{Value: os.ID.String()}, | ||
| } | ||
| } | ||
| return &result, osID, nil |
Contributor
There was a problem hiding this comment.
Add an explicit unsupported OS-type path in the switch.
At Line 182, the switch has no default, so an unknown os.Type returns success with Variant == nil. That produces an invalid InstanceOperatingSystemConfig and defers failure to the workflow layer.
Suggested fix
switch os.Type {
case cdbm.OperatingSystemTypeIPXE:
result.RunProvisioningInstructionsOnEveryBoot = *apiRequest.AlwaysBootWithCustomIpxe
result.Variant = &cwssaws.InstanceOperatingSystemConfig_Ipxe{
Ipxe: &cwssaws.InlineIpxe{IpxeScript: *apiRequest.IpxeScript},
}
case cdbm.OperatingSystemTypeTemplatedIPXE:
result.RunProvisioningInstructionsOnEveryBoot = *apiRequest.AlwaysBootWithCustomIpxe
result.Variant = &cwssaws.InstanceOperatingSystemConfig_OperatingSystemId{
OperatingSystemId: &cwssaws.OperatingSystemId{Value: os.ID.String()},
}
case cdbm.OperatingSystemTypeImage:
result.Variant = &cwssaws.InstanceOperatingSystemConfig_OsImageId{
OsImageId: &cwssaws.UUID{Value: os.ID.String()},
}
+ default:
+ logger.Error().Str("osType", os.Type).Msg("unsupported OperatingSystem type for batch create")
+ return nil, nil, cutil.NewAPIError(
+ http.StatusBadRequest,
+ "Unsupported OperatingSystem type specified in request data",
+ 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/handler/instancebatch.go` around lines 182 - 198, The
switch statement handling os.Type cases (OperatingSystemTypeIPXE,
OperatingSystemTypeTemplatedIPXE, OperatingSystemTypeImage) is missing a default
case, allowing unknown OS types to return success with a nil Variant field,
creating an invalid InstanceOperatingSystemConfig. Add a default case to the
switch statement that returns an error for any unsupported or unknown os.Type
values, preventing the function from silently returning an incomplete
configuration that would defer failure to the workflow layer.
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.
Introduce a new variant of Operating System based on iPXE templates
Ensure synchronisation with 'core'.
Type of Change
Testing