Skip to content

Allow REST to support disk/by-id and "smallest" for image disk. #5542

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

Open
stoo-davies wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Allow REST to support disk/by-id and "smallest" for image disk. #5542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c39eaab
Allow REST to support disk/by-id and "smallest" for image disk.
stoo-davies Aug 28, 2026
ca734ff
Merge remote-tracking branch 'origin/main' into stoo-4990-rest
stoo-davies Aug 31, 2026
4f27841
Addressing feedback.
stoo-davies Aug 31, 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
137 changes: 68 additions & 69 deletions pxe/common_files/disk_imaging.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,69 @@ function modify_grub_template() {
cat $new_grub_template > /mnt/etc/default/grub
}

function parse_kernel_cmdline_argument() {
local argument=$1

case "$argument" in
image_url=*)
image_url=${argument#image_url=}
;;
image_sha=*)
image_sha=${argument#image_sha=}
;;
image_auth_type=*)
image_auth_type=${argument#image_auth_type=}
;;
image_auth_token=*)
image_auth_token=${argument#image_auth_token=}
;;
image_disk=*)
image_disk=${argument#image_disk=}
;;
image_distro_name=*)
distro_name=${argument#image_distro_name=}
distro_name=${distro_name,,}
;;
image_distro_version=*)
distro_version=${argument#image_distro_version=}
;;
image_distro_release=*)
distro_release=${argument#image_distro_release=}
;;
"ds=nocloud;s="*|"ds=nocloud-net;s="*)
cloud_init_url=${argument#*;s=}
;;
create_forge_test_user=*)
user_pass=${argument#create_forge_test_user=}
forge_test_user=${user_pass%%:*}
forge_test_pass=${user_pass#*:}
;;
rootfs_uuid=*)
rootfs_uuid=${argument#rootfs_uuid=}
;;
rootfs_label=*)
rootfs_label=${argument#rootfs_label=}
;;
bootfs_uuid=*)
bootfs_uuid=${argument#bootfs_uuid=}
;;
efifs_uuid=*)
efifs_uuid=${argument#efifs_uuid=}
;;
update_grub_template=*)
update_grub_template=${argument#update_grub_template=}
;;
update_grub_cfg=*)
update_grub_cfg=${argument#update_grub_cfg=}
;;
*)
return 1
;;
esac

return 0
}

function main() {

get_serial_port
Expand All @@ -971,76 +1034,12 @@ function main() {
# use the disk the tenant specified optionally
# image_disk=/dev/nvme0n1
# image_disk=smallest
for i in `cat /proc/cmdline`
local -a kernel_arguments
local argument
read -r -a kernel_arguments < /proc/cmdline
for argument in "${kernel_arguments[@]}"
do
#echo $line
line=$(echo $i|grep image_url)
if [ ! -z "$line" ]; then
image_url=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep image_sha)
if [ ! -z "$line" ]; then
image_sha=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep image_auth_type)
if [ ! -z "$line" ]; then
image_auth_type=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep image_auth_token)
if [ ! -z "$line" ]; then
image_auth_token=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep image_disk)
if [ ! -z "$line" ]; then
image_disk=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep image_distro_name)
if [ ! -z "$line" ]; then
distro_name=$(echo $line|cut -d'=' -f2|tr '[:upper:]' '[:lower:]')
fi
line=$(echo $i|grep image_distro_version)
if [ ! -z "$line" ]; then
distro_version=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep image_distro_release)
if [ ! -z "$line" ]; then
distro_release=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep 'ds=nocloud')
if [ ! -z "$line" ]; then
cloud_init_url=$(echo $line|cut -d'=' -f3)
fi
line=$(echo $i|grep 'create_forge_test_user')
if [ ! -z "$line" ]; then
user_pass=$(echo $line|cut -d'=' -f2)
forge_test_user=$(echo $user_pass|cut -d':' -f1)
forge_test_pass=$(echo $user_pass|cut -d':' -f2)
fi
line=$(echo $i|grep 'rootfs_uuid')
if [ ! -z "$line" ]; then
rootfs_uuid=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep 'rootfs_label')
if [ ! -z "$line" ]; then
rootfs_label=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep 'bootfs_uuid')
if [ ! -z "$line" ]; then
bootfs_uuid=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep 'efifs_uuid')
if [ ! -z "$line" ]; then
efifs_uuid=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep 'update_grub_template')
if [ ! -z "$line" ]; then
update_grub_template=$(echo $line|cut -d'=' -f2)
fi
line=$(echo $i|grep 'update_grub_cfg')
if [ ! -z "$line" ]; then
update_grub_cfg=$(echo $line|cut -d'=' -f2)
fi

parse_kernel_cmdline_argument "$argument" || true
done
if [ -z "$rootfs_uuid" ] && [ -z "$rootfs_label" ]; then
rootfs_label="cloudimg-rootfs" #default rootfs name for cloud images
Expand Down
64 changes: 64 additions & 0 deletions pxe/common_files/disk_imaging_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,68 @@ fi
assert_log_contains \
"blkid failed while looking up UUID=root with status 4: stdout=<empty>; stderr=command failed"

assert_parsed_argument() {
local argument=$1
local variable_name=$2
local expected=$3

printf -v "$variable_name" '%s' "unparsed"
parse_kernel_cmdline_argument "$argument" ||
fail "argument was not recognized: $argument"
assert_eq "$argument" "$expected" "${!variable_name}"
}

echo "kernel command-line arguments"
image_disk=unchanged-disk
assert_parsed_argument \
'image_url=https://images.example/image.qcow2?token=a=b-image_disk' \
image_url \
'https://images.example/image.qcow2?token=a=b-image_disk'
assert_eq "image_url does not alter image_disk" "unchanged-disk" "$image_disk"

image_url=unchanged-url
assert_parsed_argument \
'image_disk=/dev/disk/by-id/foo=bar-image_url' \
image_disk \
'/dev/disk/by-id/foo=bar-image_url'
assert_eq "image_disk does not alter image_url" "unchanged-url" "$image_url"

assert_parsed_argument 'image_sha=sha=value' image_sha 'sha=value'
assert_parsed_argument 'image_auth_type=Bearer' image_auth_type 'Bearer'
assert_parsed_argument 'image_auth_token=token=value' image_auth_token 'token=value'
assert_parsed_argument 'image_distro_name=UbUnTu' distro_name 'ubuntu'
assert_parsed_argument 'image_distro_version=24.04' distro_version '24.04'
assert_parsed_argument 'image_distro_release=server' distro_release 'server'
assert_parsed_argument 'rootfs_uuid=root=value' rootfs_uuid 'root=value'
assert_parsed_argument 'rootfs_label=root-label' rootfs_label 'root-label'
assert_parsed_argument 'bootfs_uuid=boot=value' bootfs_uuid 'boot=value'
assert_parsed_argument 'efifs_uuid=efi=value' efifs_uuid 'efi=value'
assert_parsed_argument 'update_grub_template=yes' update_grub_template 'yes'
assert_parsed_argument 'update_grub_cfg=yes' update_grub_cfg 'yes'
assert_parsed_argument \
'ds=nocloud;s=https://cloud.example/data?token=a=b' \
cloud_init_url \
'https://cloud.example/data?token=a=b'
assert_parsed_argument \
'ds=nocloud-net;s=https://cloud.example/data?token=c=d' \
cloud_init_url \
'https://cloud.example/data?token=c=d'

forge_test_user=
forge_test_pass=
parse_kernel_cmdline_argument \
'create_forge_test_user=tester:pass:word=value' ||
fail "test-user argument was not recognized"
assert_eq "test-user name" "tester" "$forge_test_user"
assert_eq "test-user password preserves delimiters" "pass:word=value" "$forge_test_pass"

image_url=unchanged-url
image_disk=unchanged-disk
if parse_kernel_cmdline_argument \
'other=image_disk=/dev/disk/by-id/foo-image_url'; then
fail "unknown argument was accepted"
fi
assert_eq "unknown argument leaves image_url unchanged" "unchanged-url" "$image_url"
assert_eq "unknown argument leaves image_disk unchanged" "unchanged-disk" "$image_disk"

echo "disk imaging identifier tests passed"
10 changes: 10 additions & 0 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 @@ -178,6 +178,11 @@ func TestAPIOperatingSystemCreateRequest_Validate(t *testing.T) {
obj: APIOperatingSystemCreateRequest{Name: "abc", TenantID: cutil.GetPtr(uuid.New().String()), ImageURL: cutil.GetPtr("http://iso.net/iso"), SiteIDs: []string{uuid.NewString()}, ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e"), IsCloudInit: true, AllowOverride: false},
expectErr: false,
},
{
desc: "ok when ImageDisk is a by-id path",
obj: APIOperatingSystemCreateRequest{Name: "abc", TenantID: cutil.GetPtr(uuid.New().String()), ImageURL: cutil.GetPtr("http://iso.net/iso"), SiteIDs: []string{uuid.NewString()}, ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), ImageDisk: cutil.GetPtr("/dev/disk/by-id/nvme-Dell_DC_NVMe_CD7_U.2_960GB_Z3W0A01DTXBH-extra-long"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e")},
expectErr: false,
},
{
desc: "ok when empty strings specified for optional image fields",
obj: APIOperatingSystemCreateRequest{Name: "abc", TenantID: cutil.GetPtr(uuid.New().String()), ImageURL: cutil.GetPtr("http://iso.net/iso"), SiteIDs: []string{uuid.NewString()}, ImageSHA: cutil.GetPtr("a1efca12ea51069abb123bf9c77889fcc2a31cc5483fc14d115e44fdf07c7980"), RootFsID: cutil.GetPtr("666c2eee-193d-42db-a490-4c444342bd4e"), IsCloudInit: true, AllowOverride: false, ImageDisk: cutil.GetPtr(""), ImageAuthType: cutil.GetPtr(""), ImageAuthToken: cutil.GetPtr("")},
Expand Down Expand Up @@ -337,6 +342,11 @@ func TestAPIOperatingSystemUpdateRequest_Validate(t *testing.T) {
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 ImageDisk selects the smallest disk",
obj: APIOperatingSystemUpdateRequest{Name: cutil.GetPtr("ab"), ImageDisk: cutil.GetPtr("smallest")},
expectErr: false,
},
{
desc: "ok when optional image fields are empty",
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("")},
Expand Down
2 changes: 1 addition & 1 deletion rest-api/api/pkg/api/model/util/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var (
NotAllWhitespaceRegexp = regexp.MustCompile("[^\\s]+")
ShaHashRegex = regexp.MustCompile("^[A-Fa-f0-9]+$")
Sha256LowercaseHexRegex = regexp.MustCompile("^[a-f0-9]{64}$")
DiskImagePathRegex = regexp.MustCompile("^/dev/(:?nvme\\d+n\\d+|sd*)")
DiskImagePathRegex = regexp.MustCompile(`^(smallest|/dev/(nvme[0-9]+n[0-9]+|sd[a-z]+|disk/by-id/[^/[:space:]]+))$`)
Comment thread
stoo-davies marked this conversation as resolved.

ValidationErrorNameHasLeadingWhitespace = errors.New("name field has leading whitespace")
ValidationErrorNameHasTrailingWhitespace = errors.New("name field has trailing whitespace")
Expand Down
1 change: 1 addition & 0 deletions rest-api/db/pkg/db/model/operatingsystem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ func (os *OperatingSystem) ToImageAttributesProto(tenantOrg string) *corev1.OsIm
AuthToken: os.ImageAuthToken,
RootfsId: os.RootFsID,
RootfsLabel: os.RootFsLabel,
BootDisk: os.ImageDisk,
}
}

Expand Down
3 changes: 3 additions & 0 deletions rest-api/db/pkg/db/model/operatingsystem_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func TestOperatingSystem_ToImageAttributesProto(t *testing.T) {
authToken := "token"
rootFsID := "fs-1"
rootFsLabel := "label"
imageDisk := "/dev/disk/by-id/nvme-Dell_DC_NVMe_CD7_U.2_960GB_Z3W0A01DTXBH-extra-long"
os := &OperatingSystem{
ID: id,
Name: "ubuntu",
Expand All @@ -114,6 +115,7 @@ func TestOperatingSystem_ToImageAttributesProto(t *testing.T) {
ImageAuthToken: &authToken,
RootFsID: &rootFsID,
RootFsLabel: &rootFsLabel,
ImageDisk: &imageDisk,
EnableBlockStorage: true,
}
got := os.ToImageAttributesProto("org-1")
Expand All @@ -131,6 +133,7 @@ func TestOperatingSystem_ToImageAttributesProto(t *testing.T) {
assert.Equal(t, &authToken, got.AuthToken)
assert.Equal(t, &rootFsID, got.RootfsId)
assert.Equal(t, &rootFsLabel, got.RootfsLabel)
assert.Equal(t, &imageDisk, got.BootDisk)
}

func TestOperatingSystem_ToImageDeletionRequestProto(t *testing.T) {
Expand Down
Loading
Loading