fix(rest-api): OS imageUrl/imageSha are immutable on update and change attempt should fail by pbreton · Pull Request #3294 · NVIDIA/infra-controller

pbreton · 2026-07-09T01:16:50Z

The operating-system PATCH validation advertised imageUrl/imageSha as patchable and forced them to be sent together, but site sync forwards those to Core's UpdateOsImage, which rejects any change to source_url/digest as read-only ("os_image update read-only attributes changed"). Callers who only wanted to rotate an image auth token were also blocked, since imageAuthToken could not be sent without imageUrl.

Align the API layer with the immutable-image design: reject changes to imageUrl/imageSha up front with a clear 400 (re-sending the current value stays a no-op), and allow imageAuthType/imageAuthToken to be updated on image-based OS without re-sending imageUrl/imageSha. Update the OpenAPI descriptions and the model/handler tests to match.

Type of Change

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

Breaking Changes

This PR contains breaking changes

Testing

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

The operating-system PATCH validation advertised imageUrl/imageSha as patchable and forced them to be sent together, but site sync forwards those to Core's UpdateOsImage, which rejects any change to source_url/digest as read-only ("os_image update read-only attributes changed"). Callers who only wanted to rotate an image auth token were also blocked, since imageAuthToken could not be sent without imageUrl. Align the API layer with the immutable-image design: reject changes to imageUrl/imageSha up front with a clear 400 (re-sending the current value stays a no-op), and allow imageAuthType/imageAuthToken to be updated on image-based OS without re-sending imageUrl/imageSha. Update the OpenAPI descriptions and the model/handler tests to match. Signed-off-by: Patrice Breton <pbreton@nvidia.com>

coderabbitai · 2026-07-09T01:17:26Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 497cedb0-ec85-4eca-a62a-e237efc2c1c8

📥 Commits

Reviewing files that changed from the base of the PR and between 47ccd6c and 13b52b5.

📒 Files selected for processing (1)

rest-api/api/pkg/api/handler/operatingsystem_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

rest-api/api/pkg/api/handler/operatingsystem_test.go

Summary by CodeRabbit

Bug Fixes
- Operating system image-based updates now reject attempts to change immutable image fields (imageUrl / imageSha) while allowing re-sending the same values unchanged.
- iPXE-based updates now consistently disallow image URL fields and enforce correct validation for image authentication updates.
Documentation
- Updated OpenAPI request documentation to clearly describe which fields are mutable vs. immutable and how imageAuthType / imageAuthToken relate to image fields.

Walkthrough

OperatingSystem update validation now rejects changes to ImageURL and ImageSHA after creation, while allowing mutable image auth and rootfs fields to be updated. Handler tests, model tests, and OpenAPI descriptions were updated to match.

Changes

Image immutability validation

Layer / File(s)	Summary
Validation logic for image immutability `rest-api/api/pkg/api/model/operatingsystem.go`	Rejects `ImageURL`/`ImageSHA` changes after creation, validates image fields only when present, and forbids `ImageURL` for iPXE-based updates.
Model validation tests `rest-api/api/pkg/api/model/operatingsystem_test.go`	Covers unchanged and changed image fields, auth-field updates, iPXE auth rejection, and rootfs update requests without unrelated image fields.
Handler update expectations `rest-api/api/pkg/api/handler/operatingsystem_test.go`	Verifies image SHA fixtures, unchanged `ImageURL` on success, 400 rejection for `ImageURL` changes, and 500 on workflow timeout with the immutable-image request shape.
OpenAPI descriptions `rest-api/openapi/spec.yaml`	Updates request and property descriptions to state immutability after creation and independent auth-field updates.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant APIOperatingSystemUpdateRequest.Validate
  participant validation
  Client->>APIOperatingSystemUpdateRequest.Validate: submit update request
  APIOperatingSystemUpdateRequest.Validate->>validation: check ImageURL/ImageSHA immutability
  APIOperatingSystemUpdateRequest.Validate->>validation: enforce Nil on ImageURL for iPXE updates

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly reflects the main change: enforcing immutable OS imageUrl/imageSha updates and rejecting changes.
Description check	✅ Passed	The description accurately matches the patch, including immutability enforcement, auth field updates, and test/OpenAPI changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

github-actions · 2026-07-09T01:17:46Z

🔍 Container Scan Summary

No Grype artifacts were found to aggregate.

github-actions · 2026-07-09T01:18:15Z

🌿 Preview your docs: https://nvidia-preview-pull-request-3294.docs.buildwithfern.com/infra-controller

github-actions · 2026-07-09T01:19:37Z

🔐 TruffleHog Secret Scan

✅ No secrets or credentials found!

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

🔗 View scan details

_{🕐 Last updated: 2026-07-09 01:19:36 UTC | Commit: b3eb803}

coderabbitai

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/operatingsystem_test.go`:
- Around line 1741-1769: The new table-driven cases in operatingsystem_test.go
depend on a valid ImageSHA fixture, but the current test data prevents the
intended update paths from being exercised. Update the relevant OS fixture/setup
used by os5/os9 so the image OS entries have a correct ImageSHA value, and keep
the unchanged-imageURL and change-rejected cases wired through
updReqValidImageUrl, okBodyImageUrl, and errBodyChangeImageUrl so the success,
400, and timeout scenarios reach the expected handlers.

🪄 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: 41aa0382-da61-406e-88aa-758fc5b515a2

📥 Commits

Reviewing files that changed from the base of the PR and between 9e512e3 and b3eb803.

📒 Files selected for processing (4)

rest-api/api/pkg/api/handler/operatingsystem_test.go
rest-api/api/pkg/api/model/operatingsystem.go
rest-api/api/pkg/api/model/operatingsystem_test.go
rest-api/openapi/spec.yaml

Signed-off-by: Patrice Breton <pbreton@nvidia.com>

nvlitagaki · 2026-07-09T14:59:49Z

Rather than simply updating the descriptions of the open API spec to tell users that updates to imageUrl and imageSha will be rejected, wouldn't it be more clear if we deprecated those fields on OperaingSystemUpdateRequest?

nvlitagaki · 2026-07-09T15:02:49Z

Consider updating the second example of the OperatingSystemUpdateRequest object so that it no longer sends imageUrl or imageSha.

index d79332b05..2a17494cf 100644 --- a/rest-api/openapi/spec.yaml +++ b/rest-api/openapi/spec.yaml @@ -16943,8 +16943,6 @@ components: allowOverride: false - name: debian-12-amd64 description: Official Debian 12 for AMD/Intel - imageUrl: 'https://saimei.ftp.acc.umu.se/images/cloud/bookworm/latest/debian-12-generic-amd64.qcow2' - imageSha: 2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae imageAuthType: Bearer imageAuthToken: acbd18db4cc2f85cedef654fccc4a4d8 imageDisk: /dev/sda```

nvlitagaki · 2026-07-09T15:48:43Z

It looks like core logic also rejects changes to the rootFsId and rootFsLabel fields, so could we make similar changes to the API for those fields as well?

Edit: Nvm, I just saw your comment about limiting the scope of this work. We should probably make sure that the need to make the fix on those fields too is documented as a separate work item if it isn't already.

thossain-nv

Looks good @pbreton, left one suggestion.

thossain-nv · 2026-07-09T16:47:11Z

 		)
 	} else {
 		err = validation.ValidateStruct(osur,
+			validation.Field(&osur.ImageURL,


This seems already covered in line 329?

pbreton requested a review from a team as a code owner July 9, 2026 01:16

coderabbitai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread rest-api/api/pkg/api/handler/operatingsystem_test.go

fix formatting

13b52b5

Signed-off-by: Patrice Breton <pbreton@nvidia.com>

pbreton force-pushed the fix/api-spec-sha-should-be-immutable branch from 47ccd6c to 13b52b5 Compare July 9, 2026 03:17

nvlitagaki reviewed Jul 9, 2026

View reviewed changes

thossain-nv approved these changes Jul 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(rest-api): OS imageUrl/imageSha are immutable on update and change attempt should fail#3294

fix(rest-api): OS imageUrl/imageSha are immutable on update and change attempt should fail#3294
pbreton wants to merge 2 commits into
NVIDIA:mainfrom
pbreton:fix/api-spec-sha-should-be-immutable

pbreton commented Jul 9, 2026

Uh oh!

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

nvlitagaki commented Jul 9, 2026 •

edited

Loading

Uh oh!

nvlitagaki Jul 9, 2026

Uh oh!

nvlitagaki commented Jul 9, 2026 •

edited

Loading

Uh oh!

thossain-nv left a comment

Uh oh!

thossain-nv Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

pbreton commented Jul 9, 2026

Type of Change

Breaking Changes

Testing

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Jul 9, 2026

🔍 Container Scan Summary

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

🔐 TruffleHog Secret Scan

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nvlitagaki commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nvlitagaki Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

nvlitagaki commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thossain-nv left a comment

Choose a reason for hiding this comment

Uh oh!

thossain-nv Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

nvlitagaki commented Jul 9, 2026 •

edited

Loading

nvlitagaki commented Jul 9, 2026 •

edited

Loading