Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ajf merged 4 commits into from
Jul 10, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
07b51f2
fix(rest-api): treat OS imageUrl/imageSha as immutable on update
pbreton Jul 9, 2026
86526d4
fix formatting
pbreton Jul 9, 2026
01025e4
refactor(rest-api): consolidate OS update imageURL guard and fix example
pbreton Jul 9, 2026
b1d5a07
generated sdk and doc
pbreton Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions rest-api/api/pkg/api/handler/operatingsystem_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,7 @@ func TestOperatingSystemHandler_Update(t *testing.T) {
TenantID: &tenant1.ID,
OsType: cdbm.OperatingSystemTypeImage,
ImageURL: cutil.GetPtr("https://oldimagepath.iso"),
ImageSHA: cutil.GetPtr("10886660c5b2746ff48224646c5094ebcf88c889"),
AllowOverride: false,
EnableBlockStorage: true,
PhoneHomeEnabled: false,
Expand Down Expand Up @@ -1345,6 +1346,7 @@ func TestOperatingSystemHandler_Update(t *testing.T) {
TenantID: &tenant2.ID,
OsType: cdbm.OperatingSystemTypeImage,
ImageURL: cutil.GetPtr("https://oldimagepath.iso"),
ImageSHA: cutil.GetPtr("10886660c5b2746ff48224646c5094ebcf88c889"),
AllowOverride: false,
EnableBlockStorage: true,
PhoneHomeEnabled: false,
Expand Down Expand Up @@ -1476,10 +1478,20 @@ func TestOperatingSystemHandler_Update(t *testing.T) {
errBodyImageUrlIpxe, err := json.Marshal(updReqImageUrl)
assert.Nil(t, err)

updReqValidImageUrl := model.APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("test-os-updated-3"), Description: cutil.GetPtr("Updated Description"), ImageURL: cutil.GetPtr("http://newimagepath.iso"), ImageSHA: cutil.GetPtr("10886660c5b2746ff48224646c5094ebcf88c889"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e"), ImageDisk: cutil.GetPtr("/dev/nvme2n1")}
// imageUrl/imageSha are immutable, so a valid update re-sends the
// existing imageUrl (matching the os5/os9 fixtures) unchanged while
// updating mutable image attributes such as the root filesystem and
// image disk.
updReqValidImageUrl := model.APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("test-os-updated-3"), Description: cutil.GetPtr("Updated Description"), ImageURL: cutil.GetPtr("https://oldimagepath.iso"), ImageSHA: cutil.GetPtr("10886660c5b2746ff48224646c5094ebcf88c889"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e"), ImageDisk: cutil.GetPtr("/dev/nvme2n1")}
okBodyImageUrl, err := json.Marshal(updReqValidImageUrl)
assert.Nil(t, err)

// Changing imageUrl is rejected up front because the underlying image
// content is immutable after creation.
updReqChangeImageUrl := model.APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("test-os-updated-change-url"), ImageURL: cutil.GetPtr("http://newimagepath.iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980")}
errBodyChangeImageUrl, err := json.Marshal(updReqChangeImageUrl)
assert.Nil(t, err)

updReqDeactivate := model.APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("test-os-updated-deactivate"), Description: cutil.GetPtr("Updated Description for deactivation"), IsActive: cutil.GetPtr(false), DeactivationNote: cutil.GetPtr("Deactivated for a valid reason")}
okBodyDeactivate, err := json.Marshal(updReqDeactivate)
assert.Nil(t, err)
Expand Down Expand Up @@ -1728,22 +1740,31 @@ func TestOperatingSystemHandler_Update(t *testing.T) {
expectedDeactivationNote: nil,
},
{
name: "success when updated with required valid imageURL attribute",
name: "success when image OS updated with unchanged imageURL and mutable attributes",
reqOrgName: ipOrg1,
user: user,
reqBody: string(okBodyImageUrl),
reqUpdateModel: &updReqImageUrl,
reqUpdateModel: &updReqValidImageUrl,
osID: os5.ID.String(),
expectedErr: false,
expectedStatus: http.StatusOK,
expectedImageURL: cutil.GetPtr("http://newimagepath.iso"),
expectedImageURL: cutil.GetPtr("https://oldimagepath.iso"),
},
{
name: "error when image OS update tries to change imageURL",
reqOrgName: ipOrg1,
user: user,
reqBody: string(errBodyChangeImageUrl),
osID: os5.ID.String(),
expectedErr: true,
expectedStatus: http.StatusBadRequest,
},
{
name: "error when updated with required valid imageURL attribute failed with context deadline error",
name: "error when image OS update workflow fails with context deadline error",
reqOrgName: ipOrg2,
user: user,
reqBody: string(okBodyImageUrl),
reqUpdateModel: &updReqImageUrl,
reqUpdateModel: &updReqValidImageUrl,
osID: os9.ID.String(),
expectedErr: true,
expectedStatus: http.StatusInternalServerError,
Expand Down
47 changes: 38 additions & 9 deletions rest-api/api/pkg/api/model/operatingsystem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,19 +325,19 @@ func (osur *APIOperatingSystemUpdateRequest) Validate(existingOS *cdbm.Operating
}
}

// verify if os created with ipxe script, if yes reject the update if imageURL provided
if existingOS.Type == cdbm.OperatingSystemTypeIPXE && osur.ImageURL != nil {
isImageBased := existingOS.Type == cdbm.OperatingSystemTypeImage

// verify if os was not created as image-based, reject the update if imageURL provided
if !isImageBased && osur.ImageURL != nil {
return validation.Errors{
"imageURL": errors.New("unable to set image URL for iPXE based Operating System"),
"imageURL": errors.New("unable to set image URL for non-image based Operating System"),
}
} else if existingOS.Type == cdbm.OperatingSystemTypeImage && osur.IpxeScript != nil {
} else if isImageBased && osur.IpxeScript != nil {
return validation.Errors{
"ipxeScript": errors.New("unable to set iPXE script for image based Operating System"),
}
}

isImageBased := existingOS.Type == cdbm.OperatingSystemTypeImage

if !util.IsNilOrEmptyStrPtr(osur.RootFsID) && osur.RootFsLabel == nil && !util.IsNilOrEmptyStrPtr(existingOS.RootFsLabel) {
return validation.Errors{
"rootFsId": errors.New("unable to set root filesystem id for Operating System with root filesystem label specified"),
Expand All @@ -356,11 +356,40 @@ func (osur *APIOperatingSystemUpdateRequest) Validate(existingOS *cdbm.Operating
}
}

if osur.ImageURL != nil {
// imageUrl and imageSha identify the underlying image content and are
// immutable after creation. The Site treats source_url/digest as
// read-only and rejects any change during sync with
// "os_image update read-only attributes changed" (see api-core
// update_os_image); rejecting the change here gives the caller a clear,
// actionable error up front instead of a cryptic site-sync failure.
// Re-sending the current value is accepted as a no-op so clients that
// echo back the full resource still succeed.
if osur.ImageURL != nil && !util.IsNilOrEmptyStrPtr(existingOS.ImageURL) && *osur.ImageURL != *existingOS.ImageURL {
return validation.Errors{
"imageUrl": errors.New("imageUrl cannot be changed after creation; create a new Operating System to use a different image"),
}
}
if osur.ImageSHA != nil && !util.IsNilOrEmptyStrPtr(existingOS.ImageSHA) && *osur.ImageSHA != *existingOS.ImageSHA {
return validation.Errors{
"imageSha": errors.New("imageSha cannot be changed after creation; create a new Operating System to use a different image"),
}
}

if isImageBased {
// Image auth credentials can be updated on their own — the caller
// does not have to re-send the immutable imageUrl/imageSha to
// rotate a token. Those fields, when present, are validated for
// format only; the immutability guard above has already rejected
// any attempt to change them.
//
// TODO: rootFsId/rootFsLabel are also read-only on the Site (see
// api-core update_os_image), so changing them still fails at sync.
// Left as-is to keep this fix scoped to the reported
// imageUrl/imageSha/imageAuthToken behavior.
err = validation.ValidateStruct(osur,
validation.Field(&osur.ImageURL, is.URL),
validation.Field(&osur.ImageURL,
validation.When(osur.ImageURL != nil, is.URL)),
validation.Field(&osur.ImageSHA,
validation.Required.Error(validationErrorValueRequired),
validation.When(osur.ImageSHA != nil, validation.Match(util.ShaHashRegex).Error(errMsgInvalidImageSHA))),
validation.Field(&osur.ImageAuthType,
validation.When(!(util.IsNilOrEmptyStrPtr(osur.ImageAuthType)) && util.IsNilOrEmptyStrPtr(osur.ImageAuthToken), validation.Required.Error("imageAuthType cannot be specified if imageAuthToken is not specified")),
Expand Down
49 changes: 25 additions & 24 deletions rest-api/api/pkg/api/model/operatingsystem_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func TestAPIOperatingSystemUpdateRequest_Validate(t *testing.T) {
ID: uuid.New(),
Name: "ab",
ImageURL: cutil.GetPtr("https://oldimagepath.iso"),
ImageSHA: cutil.GetPtr("tttt"),
ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"),
RootFsID: cutil.GetPtr("fsID"),
Status: cdbm.OperatingSystemStatusPending,
Type: cdbm.OperatingSystemTypeImage,
Expand All @@ -219,7 +219,7 @@ func TestAPIOperatingSystemUpdateRequest_Validate(t *testing.T) {
ID: uuid.New(),
Name: "abc",
ImageURL: cutil.GetPtr("https://oldimagepath.iso"),
ImageSHA: cutil.GetPtr("tttt"),
ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"),
RootFsLabel: cutil.GetPtr("10886660c5b2746ff48224646c5094ebcf88c889"),
Status: cdbm.OperatingSystemStatusPending,
Type: cdbm.OperatingSystemTypeImage,
Expand Down Expand Up @@ -252,34 +252,35 @@ func TestAPIOperatingSystemUpdateRequest_Validate(t *testing.T) {
expectErr: true,
},
{
desc: "error when imageURL is not valid",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("imagenet")},
desc: "error when imageUrl is changed",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("https://newimagepath.iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980")},
expectErr: true,
},
{
desc: "error when imageURL and imageAuthType are specified but no imageAuthToken",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://image.net/iso"), ImageAuthType: cutil.GetPtr("Bearer")},
desc: "error when imageSha is changed",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("https://oldimagepath.iso"), ImageSHA: cutil.GetPtr("b2efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7981")},
expectErr: true,
},
{
desc: "error when imageURL and imageAuthType are specified but imageAuthToken is empty",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://image.net/iso"), ImageAuthType: cutil.GetPtr("Bearer"), ImageAuthToken: cutil.GetPtr("")},
expectErr: true,
desc: "ok when imageUrl and imageSha are re-sent unchanged",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("https://oldimagepath.iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980")},
expectErr: false,
},
{
desc: "error when imageURL and imageAuthToken are specified but no imageAuthType",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://image.net/iso"), ImageAuthToken: cutil.GetPtr("rsa")},
expectErr: true,
desc: "ok when imageAuthType and imageAuthToken are updated without imageUrl",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageAuthType: cutil.GetPtr("Bearer"), ImageAuthToken: cutil.GetPtr("rotated-token")},
expectErr: false,
},
{
desc: "error when imageURL and imageAuthToken are specified but imageAuthType is invalid",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://image.net/iso"), ImageAuthToken: cutil.GetPtr("rsa"), ImageAuthType: cutil.GetPtr("VAPID")},
desc: "error when imageAuthType is invalid",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageAuthType: cutil.GetPtr("VAPID"), ImageAuthToken: cutil.GetPtr("rsa")},
expectErr: true,
},
{
desc: "error when imageURL and imageAuthToken are specified but imageAuthType is empty",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://image.net/iso"), ImageAuthToken: cutil.GetPtr("rsa"), ImageAuthType: cutil.GetPtr("")},
expectErr: true,
desc: "error when imageAuthToken is updated on an iPXE based Operating System",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageAuthType: cutil.GetPtr("Bearer"), ImageAuthToken: cutil.GetPtr("rsa")},
existingOS: existingIpxeBasedOS,
expectErr: true,
},
{
desc: "error when imageURL and ipxeScript both specified",
Expand All @@ -288,28 +289,28 @@ func TestAPIOperatingSystemUpdateRequest_Validate(t *testing.T) {
},
{
desc: "error when both RootFsID and RootFsLabel are populated",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://iso.net/iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e"), RootFsLabel: cutil.GetPtr("test-label")},
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e"), RootFsLabel: cutil.GetPtr("test-label")},
expectErr: true,
},
{
desc: "error when os created with rootFsID but request to update rootFsLabel",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://iso.net/iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsLabel: cutil.GetPtr("test-label")},
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), RootFsLabel: cutil.GetPtr("test-label")},
expectErr: true,
},
{
desc: "error when os created with rootFsID and try to clear it without specifying rootFsLabel",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://iso.net/iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("")},
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), RootFsID: cutil.GetPtr("")},
expectErr: true,
},
{
desc: "error when os created with rootFsLabel but request to update rootFsID",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://iso.net/iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e")},
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e")},
existingOS: existingImageBasedOSWithFSLabel,
expectErr: true,
},
{
desc: "error when os created with rootFsLabel and try to clear it without specifying rootFsID",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), ImageURL: cutil.GetPtr("http://iso.net/iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsLabel: cutil.GetPtr("")},
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("abc"), RootFsLabel: cutil.GetPtr("")},
existingOS: existingImageBasedOSWithFSLabel,
expectErr: true,
},
Expand All @@ -332,12 +333,12 @@ func TestAPIOperatingSystemUpdateRequest_Validate(t *testing.T) {
},
{
desc: "ok when all valid image fields provided",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("ab"), ImageURL: cutil.GetPtr("http://iso.net/iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e")},
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("ab"), ImageURL: cutil.GetPtr("https://oldimagepath.iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e")},
expectErr: false,
},
{
desc: "ok when optional image fields are empty",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("ab"), ImageURL: cutil.GetPtr("http://iso.net/iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e"), ImageDisk: cutil.GetPtr(""), ImageAuthType: cutil.GetPtr(""), ImageAuthToken: cutil.GetPtr("")},
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("ab"), ImageURL: cutil.GetPtr("https://oldimagepath.iso"), ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e"), ImageDisk: cutil.GetPtr(""), ImageAuthType: cutil.GetPtr(""), ImageAuthToken: cutil.GetPtr("")},
expectErr: false,
},
}
Expand Down
Loading
Loading