Skip to content

Allow PUT updates before guest boot and deny them post boot #263

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.

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
merged 4 commits into from
May 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 11 additions & 1 deletion api_server/src/request/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,15 @@ impl fmt::Debug for SyncRequest {

pub enum OkStatus {
Created,
Updated,
}

impl GenerateResponse for OkStatus {
fn generate_response(&self) -> hyper::Response {
use self::OkStatus::*;
match *self {
Created => empty_response(StatusCode::Created),
Updated => empty_response(StatusCode::NoContent),
}
}
}
Expand All @@ -90,6 +92,7 @@ pub enum Error {
GuestCIDAlreadyInUse,
GuestMacAddressInUse,
OpenTap(TapError),
UpdateNotAllowedPostBoot,
UpdateNotImplemented,
}

Expand All @@ -109,6 +112,10 @@ impl GenerateResponse for Error {
StatusCode::BadRequest,
json_fault_message("Could not open TAP device."),
),
UpdateNotAllowedPostBoot => json_response(
StatusCode::Forbidden,
json_fault_message("The update operation is not allowed"),
),
UpdateNotImplemented => json_response(
StatusCode::InternalServerError,
json_fault_message("Update operation is not implemented yet."),
Expand Down Expand Up @@ -156,8 +163,11 @@ mod tests {

#[test]
fn test_generate_response_okstatus() {
let ret = OkStatus::Created.generate_response();
let mut ret = OkStatus::Created.generate_response();
assert_eq!(ret.status(), StatusCode::Created);

ret = OkStatus::Updated.generate_response();
assert_eq!(ret.status(), StatusCode::NoContent);
}

#[test]
Expand Down
197 changes: 195 additions & 2 deletions tests/functional/test_api.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to change the commit message, you misspelled updates

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Tests that ensure the corectness of the Firecracker API.
Tests that ensure the correctness of the Firecracker API.

# TODO

- Add many more API tests!
"""

import pytest

import shutil

@pytest.mark.timeout(240)
def test_api_happy_start(test_microvm_any, uhttp):
Expand Down Expand Up @@ -91,3 +91,196 @@ def test_api_happy_start(test_microvm_any, uhttp):
)
""" Issues a power-on command to the microvm. """
assert(uhttp.is_good_response(response.status_code))


def test_api_put_update_pre_boot(test_microvm_any, uhttp):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to add a timeout limit? Similar to what we have for test_api_happy_start?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also a default timeout of 300s that applies to all test cases.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm leaving it to the default of 300 as files are moved around to validate correct updates and this takes a while. I haven't measured how long it would take on an i3p, but on my dev machine I got between 10-30s extra time for copying the rootfs. Of course this varies with the size of the rootfs so I left the timeout at a generous 300s.

""" Tests that PUT updates are allowed before the microvm boots. """

test_microvm = test_microvm_any

_setup_microvm(test_microvm, uhttp)

response = uhttp.put(
test_microvm.boot_cfg_url,
json={
'boot_source_id': '1',
'source_type': 'LocalImage',
'local_image': {'kernel_image_path': 'foo.bar'}
}
)
""" Updating the kernel with an invalid path is not allowed. """
assert(not uhttp.is_good_response(response.status_code))

kernel_copy = test_microvm.slot.kernel_file + '.copy'
# The copy will be cleaned up by the microvm fixture's teardown() function.
shutil.copy(test_microvm.slot.kernel_file, kernel_copy)
response = uhttp.put(
test_microvm.boot_cfg_url,
json={
'boot_source_id': '1',
'source_type': 'LocalImage',
'local_image': {'kernel_image_path': kernel_copy}
}
)
""" Updates the kernel. """
assert(uhttp.is_good_response(response.status_code))

response = uhttp.put(
test_microvm.blk_cfg_url + '/root',
json={
'drive_id': 'root',
'path_on_host': 'foo.bar',
'is_root_device': True,
'permissions': 'ro',
'state': 'Attached'
}
)
""" Updating a block device with an invalid path is not allowed. """
assert(not uhttp.is_good_response(response.status_code))

response = uhttp.put(
test_microvm.blk_cfg_url + '/scratch',
json={
'drive_id': 'scratch',
'path_on_host': test_microvm.slot.make_fsfile(name='scratch'),
'is_root_device': True,
'permissions': 'rw',
'state': 'Attached'
}
)
""" An update that would result in two root block devices is not allowed."""
assert(not uhttp.is_good_response(response.status_code))

response = uhttp.put(
test_microvm.blk_cfg_url + '/scratch',
json={
'drive_id': 'scratch',
'path_on_host': test_microvm.slot.make_fsfile(name='scratch'),
'is_root_device': False,
'permissions': 'ro',
'state': 'Attached'
}
)
""" Updates a block device."""
assert(uhttp.is_good_response(response.status_code))

response = uhttp.put(test_microvm.microvm_cfg_url, json={'vcpu_count': 2})
""" Updates the vcpu count in the machine configuration.
The machine configuration has a default value, so all PUTs are updates.
"""
assert(uhttp.is_good_response(response.status_code))

response = uhttp.put(
test_microvm.net_cfg_url + '/1',
json={
'iface_id': '1',
'host_dev_name': test_microvm.slot.make_tap(name='dummytap'),
'guest_mac': '06:00:00:00:00:01',
'state': 'Attached'
}
)
""" Updates the network interface."""
assert(uhttp.is_good_response(response.status_code))


def test_api_put_update_post_boot(test_microvm_any, uhttp):
""" Tests that PUT updates are rejected after the microvm boots. """

test_microvm = test_microvm_any

_setup_microvm(test_microvm, uhttp)

uhttp.put(
test_microvm.actions_url + '/1',
json={'action_id': '1', 'action_type': 'InstanceStart'}
)

response = uhttp.put(
test_microvm.boot_cfg_url,
json={
'boot_source_id': '1',
'source_type': 'LocalImage',
'local_image': {'kernel_image_path': test_microvm.slot.kernel_file}
}
)
""" Kernel update is not allowed after boot. """
assert(not uhttp.is_good_response(response.status_code))

""" TODO
Uncomment this block after the block device update is implemented properly. Until then, the PUT triggers a rescan.
"""
# response = uhttp.put(
# test_microvm.blk_cfg_url + '/scratch',
# json={
# 'drive_id': 'scratch',
# 'path_on_host': test_microvm.slot.make_fsfile(name='scratch'),
# 'is_root_device': False,
# 'permissions': 'ro',
# 'state': 'Attached'
# }
# )
# """ Block device updates are not allowed via PUT after boot."""
# assert(not uhttp.is_good_response(response.status_code))

response = uhttp.put(test_microvm.microvm_cfg_url, json={'vcpu_count': 2})
""" Machine configuration update is not allowed after boot."""
assert(not uhttp.is_good_response(response.status_code))

response = uhttp.put(
test_microvm.net_cfg_url + '/1',
json={
'iface_id': '1',
'host_dev_name': test_microvm.slot.make_tap(name='dummytap'),
'guest_mac': '06:00:00:00:00:01',
'state': 'Attached'
}
)
""" Network interface update is not allowed after boot."""
assert(not uhttp.is_good_response(response.status_code))


def _setup_microvm(test_microvm_any, uhttp):
""" Sets up a microvm with a kernel, 2 block devices and a network interface. """

test_microvm = test_microvm_any

uhttp.put(
test_microvm.boot_cfg_url,
json={
'boot_source_id': '1',
'source_type': 'LocalImage',
'local_image': {'kernel_image_path': test_microvm.slot.kernel_file}
}
)

uhttp.put(
test_microvm.blk_cfg_url + '/root',
json={
'drive_id': 'root',
'path_on_host': test_microvm.slot.rootfs_file,
'is_root_device': True,
'permissions': 'ro',
'state': 'Attached'
}
)

uhttp.put(
test_microvm.blk_cfg_url + '/scratch',
json={
'drive_id': 'scratch',
'path_on_host': test_microvm.slot.make_fsfile(name='scratch'),
'is_root_device': False,
'permissions': 'rw',
'state': 'Attached'
}
)

uhttp.put(
test_microvm.net_cfg_url + '/1',
json={
'iface_id': '1',
'host_dev_name': test_microvm.slot.make_tap(),
'guest_mac': '06:00:00:00:00:01',
'state': 'Attached'
}
)
Loading