diff --git a/CHANGELOG.md b/CHANGELOG.md index 0676be3d425..00edb588ae8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,9 @@ - New optional `version` field to PUT requests towards `/mmds/config` to configure MMDS version. Accepted values are `V1` and `V2` and default is `V1`. Known limitation: `V2` does not currently work after snapshot load. +- Mandatory `network_interfaces` field to PUT requests towards + `/mmds/config` which contains a list of network interface IDs capable of + forwarding packets to MMDS. ### Changed @@ -49,6 +52,10 @@ `X-metadata-token` header when using V2. - Allow `PUT` requests to MMDS in order to generate a session token to be used for future `GET` requests when version 2 is used. +- Remove `allow_mmds_requests` field from the request body that attaches network + interfaces. Specifying interfaces that allow forwarding requests to MMDS is done + by adding the network interface's ID to the `network_interfaces` field of PUT + `/mmds/config` request's body. ### Fixed diff --git a/src/api_server/src/parsed_request.rs b/src/api_server/src/parsed_request.rs index 3ccad86536d..cecd796d429 100644 --- a/src/api_server/src/parsed_request.rs +++ b/src/api_server/src/parsed_request.rs @@ -836,7 +836,10 @@ pub(crate) mod tests { assert!(ParsedRequest::try_from_request(&req).is_ok()); // `/mmds/config` - let body = "{\"ipv4_address\":\"169.254.170.2\"}"; + let body = "{ \ + \"ipv4_address\": \"169.254.170.2\", \ + \"network_interfaces\": [\"iface0\"] \ + }"; sender .write_all(http_request("PUT", "/mmds/config", Some(&body)).as_bytes()) .unwrap(); @@ -853,7 +856,6 @@ pub(crate) mod tests { \"iface_id\": \"string\", \ \"guest_mac\": \"12:34:56:78:9a:BC\", \ \"host_dev_name\": \"string\", \ - \"allow_mmds_requests\": true, \ \"rx_rate_limiter\": { \ \"bandwidth\": { \ \"size\": 0, \ diff --git a/src/api_server/src/request/mmds.rs b/src/api_server/src/request/mmds.rs index 0e2cb85c683..597be420b39 100644 --- a/src/api_server/src/request/mmds.rs +++ b/src/api_server/src/request/mmds.rs @@ -73,19 +73,38 @@ mod tests { // Test `config` path. let body = r#"{ - "ipv4_address": "169.254.170.2" + "version": "V2", + "ipv4_address": "169.254.170.2", + "network_interfaces": [] }"#; let config_path = "config"; assert!(parse_put_mmds(&Body::new(body), Some(&config_path)).is_ok()); let body = r#"{ - "ipv4_address": "" + "network_interfaces": [] }"#; + let config_path = "config"; + assert!(parse_put_mmds(&Body::new(body), Some(&config_path)).is_ok()); + + let body = r#"{ + "version": "foo", + "ipv4_address": "169.254.170.2", + "network_interfaces": [] + }"#; + let config_path = "config"; + assert!(parse_put_mmds(&Body::new(body), Some(&config_path)).is_err()); + + let body = r#"{ + "version": "V2" + }"#; + let config_path = "config"; assert!(parse_put_mmds(&Body::new(body), Some(&config_path)).is_err()); - // Equivalent to reset the mmds configuration. - let empty_body = r#"{}"#; - assert!(parse_put_mmds(&Body::new(empty_body), Some(&config_path)).is_ok()); + let body = r#"{ + "ipv4_address": "", + "network_interfaces": [] + }"#; + assert!(parse_put_mmds(&Body::new(body), Some(&config_path)).is_err()); let invalid_config_body = r#"{ "invalid_config": "invalid_value" diff --git a/src/api_server/src/request/net.rs b/src/api_server/src/request/net.rs index d6ee93f6243..c4fcd8ec511 100644 --- a/src/api_server/src/request/net.rs +++ b/src/api_server/src/request/net.rs @@ -74,8 +74,7 @@ mod tests { let body = r#"{ "iface_id": "foo", "host_dev_name": "bar", - "guest_mac": "12:34:56:78:9A:BC", - "allow_mmds_requests": false + "guest_mac": "12:34:56:78:9A:BC" }"#; // 1. Exercise infamous "The id from the path does not match id from the body!". assert!(parse_put_net(&Body::new(body), Some(&"bar")).is_err()); diff --git a/src/api_server/swagger/firecracker.yaml b/src/api_server/swagger/firecracker.yaml index c97ea3adfa9..d6c6cb17908 100644 --- a/src/api_server/swagger/firecracker.yaml +++ b/src/api_server/swagger/firecracker.yaml @@ -963,6 +963,8 @@ definitions: type: object description: Defines the MMDS configuration. + required: + - network_interfaces properties: version: description: Enumeration indicating the MMDS version to be configured. @@ -971,6 +973,18 @@ definitions: - V1 - V2 default: V1 + network_interfaces: + description: + List of the network interface IDs capable of forwarding packets to + the MMDS. Network interface IDs mentioned must be valid at the time + of this request. The net device model will reply to HTTP GET requests + sent to the MMDS address via the interfaces mentioned. In this + case, both ARP requests and TCP segments heading to `ipv4_address` + are intercepted by the device model, and do not reach the associated + TAP device. + type: array + items: + type: string ipv4_address: type: string format: "169.254.([1-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-4]).([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])" @@ -985,14 +999,6 @@ definitions: - host_dev_name - iface_id properties: - allow_mmds_requests: - type: boolean - description: - If this field is set, the device model will reply to HTTP GET - requests sent to the MMDS address via this interface. In this case, - both ARP requests for 169.254.169.254 and TCP segments heading to the - same address are intercepted by the device model, and do not reach - the associated TAP device. guest_mac: type: string host_dev_name: diff --git a/src/devices/src/virtio/net/device.rs b/src/devices/src/virtio/net/device.rs index 84904fa5fdd..f0e9754f165 100755 --- a/src/devices/src/virtio/net/device.rs +++ b/src/devices/src/virtio/net/device.rs @@ -220,18 +220,21 @@ impl Net { self.tap.if_name_as_str().to_string() } - /// Says if this device supports MMDS. - pub fn mmds_enabled(&self) -> bool { - self.mmds_ns.is_some() - } - - /// Sets MMDS endpoint IPv4 address, if the device supports MMDS. - pub fn set_mmds_ipv4_addr(&mut self, ipv4_addr: Ipv4Addr) { + /// Configures the `MmdsNetworkStack` to allow device to forward MMDS requests. + /// If the device already supports MMDS, updates the IPv4 address. + pub fn configure_mmds_network_stack(&mut self, ipv4_addr: Ipv4Addr) { if let Some(mmds_ns) = self.mmds_ns.as_mut() { mmds_ns.set_ipv4_addr(ipv4_addr); + } else { + self.mmds_ns = Some(MmdsNetworkStack::new_with_defaults(Some(ipv4_addr))) } } + /// Disables the `MmdsNetworkStack` to prevent device to forward MMDS requests. + pub fn disable_mmds_network_stack(&mut self) { + self.mmds_ns = None + } + /// Provides a reference to the configured RX rate limiter. pub fn rx_rate_limiter(&self) -> &RateLimiter { &self.rx_rate_limiter diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs index 9cd1abb81dd..5267408732b 100644 --- a/src/vmm/src/builder.rs +++ b/src/vmm/src/builder.rs @@ -1273,7 +1273,6 @@ pub mod tests { guest_mac: None, rx_rate_limiter: None, tx_rate_limiter: None, - allow_mmds_requests: true, }; let mut cmdline = default_kernel_cmdline(); diff --git a/src/vmm/src/device_manager/persist.rs b/src/vmm/src/device_manager/persist.rs index ba2fc7546aa..af961323c1d 100644 --- a/src/vmm/src/device_manager/persist.rs +++ b/src/vmm/src/device_manager/persist.rs @@ -569,7 +569,6 @@ mod tests { guest_mac: None, rx_rate_limiter: None, tx_rate_limiter: None, - allow_mmds_requests: true, }; insert_net_device( &mut vmm, diff --git a/src/vmm/src/persist.rs b/src/vmm/src/persist.rs index 3acb93bedb2..26317225fdd 100644 --- a/src/vmm/src/persist.rs +++ b/src/vmm/src/persist.rs @@ -564,7 +564,6 @@ mod tests { guest_mac: None, rx_rate_limiter: None, tx_rate_limiter: None, - allow_mmds_requests: true, }; insert_net_device( &mut vmm, diff --git a/src/vmm/src/resources.rs b/src/vmm/src/resources.rs index 169edb0192d..2b13e83f8d3 100644 --- a/src/vmm/src/resources.rs +++ b/src/vmm/src/resources.rs @@ -301,18 +301,8 @@ impl VmResources { &mut self, body: NetworkInterfaceConfig, ) -> Result { - self.net_builder.build(body).map(|net_device| { - // Update `Net` device `MmdsNetworkStack` IPv4 address. - match &self.mmds_config { - Some(cfg) => cfg.ipv4_addr().map_or((), |ipv4_addr| { - net_device - .lock() - .expect("Poisoned lock") - .set_mmds_ipv4_addr(ipv4_addr); - }), - None => (), - }; - }) + let _ = self.net_builder.build(body)?; + Ok(()) } /// Sets a vsock device to be attached when the VM starts. @@ -326,21 +316,7 @@ impl VmResources { config: MmdsConfig, instance_id: &str, ) -> Result { - // Check IPv4 address validity. - let ipv4_addr = match config.ipv4_addr() { - Some(ipv4_addr) if is_link_local_valid(ipv4_addr) => Ok(ipv4_addr), - None => Ok(MmdsNetworkStack::default_ipv4_addr()), - _ => Err(MmdsConfigError::InvalidIpv4Addr), - }?; - - // Update existing built network device `MmdsNetworkStack` IPv4 address. - for net_device in self.net_builder.iter_mut() { - net_device - .lock() - .expect("Poisoned lock") - .set_mmds_ipv4_addr(ipv4_addr); - } - + self.set_mmds_network_stack_config(&config)?; self.set_mmds_version(config.version, instance_id)?; self.mmds_config = Some(MmdsConfig { @@ -348,6 +324,7 @@ impl VmResources { ipv4_address: config .ipv4_addr() .or_else(|| Some(MmdsNetworkStack::default_ipv4_addr())), + network_interfaces: config.network_interfaces, }); Ok(()) @@ -366,6 +343,47 @@ impl VmResources { mmds_lock.set_aad(instance_id); Ok(()) } + + // Updates MMDS Network Stack for network interfaces to allow forwarding + // requests to MMDS (or not). + fn set_mmds_network_stack_config(&mut self, config: &MmdsConfig) -> Result { + // Check IPv4 address validity. + let ipv4_addr = match config.ipv4_addr() { + Some(ipv4_addr) if is_link_local_valid(ipv4_addr) => Ok(ipv4_addr), + None => Ok(MmdsNetworkStack::default_ipv4_addr()), + _ => Err(MmdsConfigError::InvalidIpv4Addr), + }?; + + let network_interfaces = config.network_interfaces(); + // Ensure that at least one network ID is specified. + if network_interfaces.is_empty() { + return Err(MmdsConfigError::EmptyNetworkIfaceList); + } + + // Ensure all interface IDs specified correspond to existing net devices. + if !network_interfaces.iter().all(|id| { + self.net_builder + .iter() + .map(|device| device.lock().expect("Poisoned lock").id().clone()) + .any(|x| &x == id) + }) { + return Err(MmdsConfigError::InvalidNetworkInterfaceId); + } + + // Create `MmdsNetworkStack` and configure the IPv4 address for + // existing built network devices whose names are defined in the + // network interface ID list. + for net_device in self.net_builder.iter_mut() { + let mut net_device_lock = net_device.lock().expect("Poisoned lock"); + if network_interfaces.contains(net_device_lock.id()) { + net_device_lock.configure_mmds_network_stack(ipv4_addr); + } else { + net_device_lock.disable_mmds_network_stack(); + } + } + + Ok(()) + } } impl From<&VmResources> for VmmConfig { @@ -421,7 +439,6 @@ mod tests { guest_mac: Some(MacAddr::parse_str("01:23:45:67:89:0a").unwrap()), rx_rate_limiter: Some(RateLimiterConfig::default()), tx_rate_limiter: Some(RateLimiterConfig::default()), - allow_mmds_requests: false, } } @@ -751,8 +768,7 @@ mod tests { "network-interfaces": [ {{ "iface_id": "netif", - "host_dev_name": "hostname8", - "allow_mmds_requests": true + "host_dev_name": "hostname8" }} ], "machine-config": {{ @@ -761,7 +777,9 @@ mod tests { "ht_enabled": false }}, "mmds-config": {{ - "ipv4_address": "169.254.170.2" + "version": "V2", + "ipv4_address": "169.254.170.2", + "network_interfaces": ["netif"] }} }}"#, kernel_file.as_path().to_str().unwrap(), @@ -769,9 +787,8 @@ mod tests { ); assert!(VmResources::from_json(json.as_str(), &default_instance_info).is_ok()); - // Test all configuration, this time trying to configure the MMDS with an - // empty body. It will make it access the code path in which it sets the - // default MMDS configuration. + // Test all configuration, this time trying to set default configuration + // for version and IPv4 address. let kernel_file = TempFile::new().unwrap(); json = format!( r#"{{ @@ -795,8 +812,7 @@ mod tests { "network-interfaces": [ {{ "iface_id": "netif", - "host_dev_name": "hostname9", - "allow_mmds_requests": true + "host_dev_name": "hostname9" }} ], "machine-config": {{ @@ -804,7 +820,9 @@ mod tests { "mem_size_mib": 1024, "ht_enabled": false }}, - "mmds-config": {{}} + "mmds-config": {{ + "network_interfaces": ["netif"] + }} }}"#, kernel_file.as_path().to_str().unwrap(), rootfs_file.as_path().to_str().unwrap(), diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index 2eb1358472c..f4e7e137d2a 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -1155,7 +1155,6 @@ mod tests { guest_mac: None, rx_rate_limiter: None, tx_rate_limiter: None, - allow_mmds_requests: false, }); check_preboot_request(req, |result, vm_res| { assert_eq!(result, Ok(VmmData::Empty)); @@ -1168,7 +1167,6 @@ mod tests { guest_mac: None, rx_rate_limiter: None, tx_rate_limiter: None, - allow_mmds_requests: false, }); check_preboot_request_err( req, @@ -1208,6 +1206,7 @@ mod tests { let req = VmmAction::SetMmdsConfiguration(MmdsConfig { ipv4_address: None, version: MmdsVersion::default(), + network_interfaces: Vec::new(), }); check_preboot_request(req, |result, vm_res| { assert_eq!(result, Ok(VmmData::Empty)); @@ -1217,6 +1216,7 @@ mod tests { let req = VmmAction::SetMmdsConfiguration(MmdsConfig { ipv4_address: None, version: MmdsVersion::default(), + network_interfaces: Vec::new(), }); check_preboot_request_err( req, @@ -1588,7 +1588,6 @@ mod tests { guest_mac: None, rx_rate_limiter: None, tx_rate_limiter: None, - allow_mmds_requests: false, }), VmmActionError::OperationNotSupportedPostBoot, ); @@ -1616,6 +1615,7 @@ mod tests { VmmAction::SetMmdsConfiguration(MmdsConfig { ipv4_address: None, version: MmdsVersion::default(), + network_interfaces: Vec::new(), }), VmmActionError::OperationNotSupportedPostBoot, ); @@ -1682,7 +1682,6 @@ mod tests { guest_mac: None, rx_rate_limiter: None, tx_rate_limiter: None, - allow_mmds_requests: false, }); verify_load_snap_disallowed_after_boot_resources(req, "InsertNetworkDevice"); @@ -1702,6 +1701,7 @@ mod tests { let req = VmmAction::SetMmdsConfiguration(MmdsConfig { ipv4_address: None, version: MmdsVersion::default(), + network_interfaces: Vec::new(), }); verify_load_snap_disallowed_after_boot_resources(req, "SetMmdsConfiguration"); } diff --git a/src/vmm/src/vmm_config/mmds.rs b/src/vmm/src/vmm_config/mmds.rs index b1e8f651164..d4cd3da87d2 100644 --- a/src/vmm/src/vmm_config/mmds.rs +++ b/src/vmm/src/vmm_config/mmds.rs @@ -14,6 +14,8 @@ pub struct MmdsConfig { /// MMDS version. #[serde(default)] pub version: MmdsVersion, + /// Network interfaces that allow forwarding packets to MMDS. + pub network_interfaces: Vec, /// MMDS IPv4 configured address. pub ipv4_address: Option, } @@ -24,6 +26,11 @@ impl MmdsConfig { self.version } + /// Returns the network interfaces that accept MMDS requests. + pub fn network_interfaces(&self) -> Vec { + self.network_interfaces.clone() + } + /// Returns the MMDS IPv4 address if one was configured. /// Otherwise returns None. pub fn ipv4_addr(&self) -> Option { @@ -34,8 +41,13 @@ impl MmdsConfig { /// MMDS configuration related errors. #[derive(Debug)] pub enum MmdsConfigError { + /// The network interfaces list provided is empty. + EmptyNetworkIfaceList, /// The provided IPv4 address is not link-local valid. InvalidIpv4Addr, + /// The network interfaces list provided contains IDs that + /// does not correspond to any existing network interface. + InvalidNetworkInterfaceId, /// MMDS version could not be configured. MmdsVersion(MmdsVersion, data_store::Error), } @@ -43,9 +55,23 @@ pub enum MmdsConfigError { impl Display for MmdsConfigError { fn fmt(&self, f: &mut Formatter<'_>) -> Result { match self { + MmdsConfigError::EmptyNetworkIfaceList => { + write!( + f, + "The list of network interface IDs that allow \ + forwarding MMDS requests is empty." + ) + } MmdsConfigError::InvalidIpv4Addr => { write!(f, "The MMDS IPv4 address is not link local.") } + MmdsConfigError::InvalidNetworkInterfaceId => { + write!( + f, + "The list of network interface IDs provided contains at least one ID \ + that does not correspond to any existing network interface." + ) + } MmdsConfigError::MmdsVersion(version, err) => { write!( f, diff --git a/src/vmm/src/vmm_config/net.rs b/src/vmm/src/vmm_config/net.rs index 8be98d2d26b..fa89921b7be 100644 --- a/src/vmm/src/vmm_config/net.rs +++ b/src/vmm/src/vmm_config/net.rs @@ -30,13 +30,6 @@ pub struct NetworkInterfaceConfig { pub rx_rate_limiter: Option, /// Rate Limiter for transmitted packages. pub tx_rate_limiter: Option, - #[serde(default = "default_allow_mmds_requests")] - /// If this field is set, the device model will reply to HTTP GET - /// requests sent to the MMDS address via this interface. In this case, - /// both ARP requests for `169.254.169.254` and TCP segments heading to the - /// same address are intercepted by the device model, and do not reach - /// the associated TAP device. - pub allow_mmds_requests: bool, } impl From<&Net> for NetworkInterfaceConfig { @@ -49,18 +42,10 @@ impl From<&Net> for NetworkInterfaceConfig { guest_mac: net.guest_mac().copied(), rx_rate_limiter: rx_rl.into_option(), tx_rate_limiter: tx_rl.into_option(), - allow_mmds_requests: net.mmds_enabled(), } } } -// Serde does not allow specifying a default value for a field -// that is not required. The workaround is to specify a function -// that returns the value. -fn default_allow_mmds_requests() -> bool { - false -} - /// The data fed into a network iface update request. Currently, only the RX and TX rate limiters /// can be updated. #[derive(Debug, Deserialize, PartialEq, Clone)] @@ -202,7 +187,7 @@ impl NetBuilder { cfg.guest_mac.as_ref(), rx_rate_limiter.unwrap_or_default(), tx_rate_limiter.unwrap_or_default(), - cfg.allow_mmds_requests, + false, ) .map_err(NetworkInterfaceError::CreateNetworkDevice) } @@ -240,7 +225,6 @@ mod tests { guest_mac: Some(MacAddr::parse_str(mac).unwrap()), rx_rate_limiter: RateLimiterConfig::default().into_option(), tx_rate_limiter: RateLimiterConfig::default().into_option(), - allow_mmds_requests: false, } } @@ -252,7 +236,6 @@ mod tests { guest_mac: self.guest_mac, rx_rate_limiter: None, tx_rate_limiter: None, - allow_mmds_requests: self.allow_mmds_requests, } } } @@ -383,7 +366,6 @@ mod tests { net_if_cfg.guest_mac.unwrap(), MacAddr::parse_str(guest_mac).unwrap() ); - assert_eq!(net_if_cfg.allow_mmds_requests, false); let mut net_builder = NetBuilder::new(); assert!(net_builder.build(net_if_cfg.clone()).is_ok()); diff --git a/tests/framework/builder.py b/tests/framework/builder.py index f4d99e5e3d2..bc4d318fdca 100644 --- a/tests/framework/builder.py +++ b/tests/framework/builder.py @@ -124,8 +124,7 @@ def build(self, response = vm.network.put( iface_id=iface.dev_name, host_dev_name=iface.tap_name, - guest_mac=guest_mac, - allow_mmds_requests=True, + guest_mac=guest_mac ) assert vm.api_session.is_status_no_content(response.status_code) diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py index d4d8469b2af..d4726445612 100644 --- a/tests/framework/microvm.py +++ b/tests/framework/microvm.py @@ -762,7 +762,6 @@ def ssh_network_config( self, network_config, iface_id, - allow_mmds_requests=False, tx_rate_limiter=None, rx_rate_limiter=None, tapname=None @@ -774,7 +773,6 @@ def ssh_network_config( ssh_config dictionary. :param network_config: UniqueIPv4Generator instance :param iface_id: the interface id for the API request - :param allow_mmds_requests: specifies whether requests sent from the guest on this interface towards the MMDS address are intercepted and processed by the device model. :param tx_rate_limiter: limit the tx rate @@ -795,7 +793,6 @@ def ssh_network_config( iface_id=iface_id, host_dev_name=tapname, guest_mac=guest_mac, - allow_mmds_requests=allow_mmds_requests, tx_rate_limiter=tx_rate_limiter, rx_rate_limiter=rx_rate_limiter ) diff --git a/tests/framework/resources.py b/tests/framework/resources.py index e65e0a242d3..d8ce7f4a076 100644 --- a/tests/framework/resources.py +++ b/tests/framework/resources.py @@ -706,7 +706,6 @@ def create_json( iface_id=None, host_dev_name=None, guest_mac=None, - allow_mmds_requests=None, rx_rate_limiter=None, tx_rate_limiter=None): """Create the json for the net specific API request.""" @@ -720,9 +719,6 @@ def create_json( if guest_mac is not None: datax['guest_mac'] = guest_mac - if allow_mmds_requests is not None: - datax['allow_mmds_requests'] = allow_mmds_requests - if tx_rate_limiter is not None: datax['tx_rate_limiter'] = tx_rate_limiter diff --git a/tests/framework/vm_config.json b/tests/framework/vm_config.json index 74b40726921..09fb2c0eafe 100644 --- a/tests/framework/vm_config.json +++ b/tests/framework/vm_config.json @@ -27,8 +27,5 @@ "vsock": null, "logger": null, "metrics": null, - "mmds-config": { - "version": "V2", - "ipv4_address": "169.254.169.250" - } + "mmds-config": null } diff --git a/tests/integration_tests/build/test_coverage.py b/tests/integration_tests/build/test_coverage.py index 9d71e404182..30c79e06877 100644 --- a/tests/integration_tests/build/test_coverage.py +++ b/tests/integration_tests/build/test_coverage.py @@ -29,9 +29,9 @@ # Checkout the cpuid crate. In the future other # differences may appear. if utils.is_io_uring_supported(): - COVERAGE_DICT = {"Intel": 85.09, "AMD": 84.51, "ARM": 84.13} + COVERAGE_DICT = {"Intel": 85.09, "AMD": 84.51, "ARM": 84.04} else: - COVERAGE_DICT = {"Intel": 82.03, "AMD": 81.5, "ARM": 81.07} + COVERAGE_DICT = {"Intel": 82.03, "AMD": 81.5, "ARM": 80.98} PROC_MODEL = proc.proc_type() diff --git a/tests/integration_tests/functional/test_api.py b/tests/integration_tests/functional/test_api.py index d15731b2e8c..1da71383f73 100644 --- a/tests/integration_tests/functional/test_api.py +++ b/tests/integration_tests/functional/test_api.py @@ -226,27 +226,6 @@ def test_api_put_update_pre_boot(test_microvm_with_api): track_dirty_pages = microvm_config_json['track_dirty_pages'] assert response_json['track_dirty_pages'] == track_dirty_pages - # Updates to MMDS version with invalid value are not allowed. - response = test_microvm.mmds.put_config(json={'version': 'foo'}) - assert test_microvm.api_session.is_status_bad_request(response.status_code) - assert "An error occurred when deserializing " \ - "the json body of a request: unknown variant " \ - "`foo`, expected `V1` or `V2`" in response.text - - # Updates to MMDS config pre-boot are allowed. - response = test_microvm.mmds.put_config(json={ - 'version': 'V2', - 'ipv4_address': '169.254.169.250' - }) - assert test_microvm.api_session.is_status_no_content(response.status_code) - - # Updates to `ipv4_address` without version are allowed because - # default version is V1. - response = test_microvm.mmds.put_config(json={ - 'ipv4_address': '169.254.169.250' - }) - assert test_microvm.api_session.is_status_no_content(response.status_code) - def test_net_api_put_update_pre_boot(test_microvm_with_api): """ @@ -319,6 +298,101 @@ def test_net_api_put_update_pre_boot(test_microvm_with_api): assert test_microvm.api_session.is_status_no_content(response.status_code) +def test_api_mmds_config(test_microvm_with_api): + """ + Test /mmds/config PUT scenarios that unit tests can't cover. + + Tests updates on MMDS config before and after attaching a network device. + + @type: negative + """ + test_microvm = test_microvm_with_api + test_microvm.spawn() + + # Set up the microVM with 2 vCPUs, 256 MiB of RAM and + # a root file system with the rw permission. + test_microvm.basic_config() + + # Setting MMDS config with empty network interface IDs list is not allowed. + response = test_microvm.mmds.put_config(json={ + 'network_interfaces': [] + }) + err_msg = "The list of network interface IDs that allow " \ + "forwarding MMDS requests is empty." + assert test_microvm.api_session.is_status_bad_request(response.status_code) + assert err_msg in response.text + + # Setting MMDS config when no network device has been attached + # is not allowed. + response = test_microvm.mmds.put_config(json={ + 'network_interfaces': ['foo'] + }) + err_msg = "The list of network interface IDs provided contains " \ + "at least one ID that does not correspond to any " \ + "existing network interface." + assert test_microvm.api_session.is_status_bad_request(response.status_code) + assert err_msg in response.text + + # Attach network interface. + tap = net_tools.Tap('tap1', test_microvm.jailer.netns) + response = test_microvm.network.put( + iface_id='1', + guest_mac='06:00:00:00:00:01', + host_dev_name=tap.name + ) + assert test_microvm.api_session.is_status_no_content(response.status_code) + + # Setting MMDS config with an ID that does not correspond to an already + # attached network device is not allowed. + response = test_microvm.mmds.put_config(json={ + 'network_interfaces': ['1', 'foo'] + }) + err_msg = "The list of network interface IDs provided contains" \ + " at least one ID that does not correspond to any " \ + "existing network interface." + assert test_microvm.api_session.is_status_bad_request(response.status_code) + assert err_msg in response.text + + # Updates to MMDS version with invalid value are not allowed. + response = test_microvm.mmds.put_config(json={ + 'version': 'foo', + 'network_interfaces': ['1'] + }) + err_msg = "An error occurred when deserializing the json body of a " \ + "request: unknown variant `foo`, expected `V1` or `V2`" + assert test_microvm.api_session.is_status_bad_request(response.status_code) + assert err_msg in response.text + + # Valid MMDS config not specifying version or IPv4 address. + response = test_microvm.mmds.put_config(json={ + 'network_interfaces': ['1'] + }) + assert test_microvm.api_session.is_status_no_content(response.status_code) + assert test_microvm.full_cfg.get().json( + )['mmds-config']['version'] == "V1" + + # Valid MMDS config not specifying version. + mmds_config = { + 'ipv4_address': '169.254.169.250', + 'network_interfaces': ['1'] + } + response = test_microvm.mmds.put_config(json=mmds_config) + assert test_microvm.api_session.is_status_no_content(response.status_code) + assert test_microvm.full_cfg.get().json( + )['mmds-config']['ipv4_address'] == "169.254.169.250" + + # Valid MMDS config. + mmds_config = { + 'version': 'V2', + 'ipv4_address': '169.254.169.250', + 'network_interfaces': ['1'] + } + response = test_microvm.mmds.put_config(json=mmds_config) + assert test_microvm.api_session.is_status_no_content(response.status_code) + assert test_microvm.full_cfg.get().json( + )['mmds-config']['version'] == "V2" + + def test_api_machine_config(test_microvm_with_api): """ Test /machine_config PUT/PATCH scenarios that unit tests can't cover. @@ -507,7 +581,8 @@ def test_api_put_update_post_boot(test_microvm_with_api): # MMDS config is not allowed post-boot. mmds_config = { 'version': 'V2', - 'ipv4_address': '169.254.169.250' + 'ipv4_address': '169.254.169.250', + 'network_interfaces': ['1'] } response = test_microvm.mmds.put_config(json=mmds_config) assert test_microvm.api_session.is_status_bad_request(response.status_code) @@ -1291,8 +1366,7 @@ def test_get_full_config(test_microvm_with_api): iface_id=iface_id, guest_mac=guest_mac, host_dev_name=tap1.name, - tx_rate_limiter=tx_rl, - allow_mmds_requests=True + tx_rate_limiter=tx_rl ) assert test_microvm.api_session.is_status_no_content(response.status_code) expected_cfg['network-interfaces'] = [{ @@ -1300,14 +1374,14 @@ def test_get_full_config(test_microvm_with_api): 'host_dev_name': tap1.name, 'guest_mac': '06:00:00:00:00:01', 'rx_rate_limiter': None, - 'tx_rate_limiter': tx_rl, - 'allow_mmds_requests': True + 'tx_rate_limiter': tx_rl }] # Update MMDS config. mmds_config = { 'version': 'V2', - 'ipv4_address': '169.254.169.250' + 'ipv4_address': '169.254.169.250', + 'network_interfaces': ['1'] } response = test_microvm.mmds.put_config(json=mmds_config) assert test_microvm.api_session.is_status_no_content(response.status_code) @@ -1316,7 +1390,8 @@ def test_get_full_config(test_microvm_with_api): expected_cfg['metrics'] = None expected_cfg['mmds-config'] = { 'version': 'V2', - 'ipv4_address': '169.254.169.250' + 'ipv4_address': '169.254.169.250', + 'network_interfaces': ['1'] } # Getting full vm configuration should be available pre-boot. diff --git a/tests/integration_tests/functional/test_mmds.py b/tests/integration_tests/functional/test_mmds.py index 0e405f9a01d..2ac7bd7dbbc 100644 --- a/tests/integration_tests/functional/test_mmds.py +++ b/tests/integration_tests/functional/test_mmds.py @@ -7,6 +7,7 @@ import string import time import pytest +from framework.artifacts import DEFAULT_DEV_NAME, NetIfaceConfig from framework.builder import MicrovmBuilder, SnapshotBuilder, SnapshotType import host_tools.network as net_tools @@ -61,6 +62,19 @@ def _generate_mmds_v2_get_request(ipv4_address, token, app_json=True): return cmd +def _configure_mmds(test_microvm, iface_id, version, ipv4_address=None): + mmds_config = { + 'version': version, + 'network_interfaces': [iface_id] + } + + if ipv4_address: + mmds_config['ipv4_address'] = ipv4_address + + response = test_microvm.mmds.put_config(json=mmds_config) + assert test_microvm.api_session.is_status_no_content(response.status_code) + + @pytest.mark.parametrize( "version", MMDS_VERSIONS @@ -97,41 +111,36 @@ def test_custom_ipv4(test_microvm_with_api, network_config, version): } _populate_data_store(test_microvm, data_store) - config_data = { - 'ipv4_address': '' - } - response = test_microvm.mmds.put_config(json=config_data) + # Attach network device. + _tap = test_microvm.ssh_network_config(network_config, '1') + + # Invalid values IPv4 address. + response = test_microvm.mmds.put_config(json={ + 'ipv4_address': '', + 'network_interfaces': ['1'] + }) assert test_microvm.api_session.is_status_bad_request(response.status_code) - config_data = { - 'ipv4_address': '1.1.1.1' - } - response = test_microvm.mmds.put_config(json=config_data) + response = test_microvm.mmds.put_config(json={ + 'ipv4_address': '1.1.1.1', + 'network_interfaces': ['1'] + }) assert test_microvm.api_session.is_status_bad_request(response.status_code) + ipv4_address = '169.254.169.250' # Configure MMDS with custom IPv4 address. - config_data = { - 'ipv4_address': '169.254.169.250', - 'version': version - } - response = test_microvm.mmds.put_config(json=config_data) - assert test_microvm.api_session.is_status_no_content(response.status_code) - - test_microvm.basic_config(vcpu_count=1) - _tap = test_microvm.ssh_network_config( - network_config, - '1', - allow_mmds_requests=True + _configure_mmds( + test_microvm, + iface_id='1', + version=version, + ipv4_address=ipv4_address ) + test_microvm.basic_config(vcpu_count=1) test_microvm.start() ssh_connection = net_tools.SSHConnection(test_microvm.ssh_config) - # Requests to `/mmds/config` are rejected after boot. - response = test_microvm.mmds.put_config(json=config_data) - assert test_microvm.api_session.is_status_bad_request(response.status_code) - - cmd = 'ip route add 169.254.169.250 dev eth0' + cmd = 'ip route add {} dev eth0'.format(ipv4_address) _, stdout, stderr = ssh_connection.execute_command(cmd) _assert_out(stdout, stderr, '') @@ -139,17 +148,17 @@ def test_custom_ipv4(test_microvm_with_api, network_config, version): # Generate token. token = _generate_mmds_session_token( ssh_connection, - ipv4_address="169.254.169.250", + ipv4_address=ipv4_address, token_ttl=60 ) pre = _generate_mmds_v2_get_request( - ipv4_address='169.254.169.250', + ipv4_address=ipv4_address, token=token ) else: pre = 'curl -s -H "Accept: application/json" ' \ - 'http://169.254.169.250/' + 'http://{}/'.format(ipv4_address) cmd = pre + 'latest/meta-data/ami-id' _, stdout, _ = ssh_connection.execute_command(cmd) @@ -209,23 +218,16 @@ def test_json_response(test_microvm_with_api, network_config, version): } } - # Configure MMDS version if V2, default is V1. - if version == 'V2': - response = test_microvm.mmds.put_config(json={'version': version}) - assert test_microvm.api_session.is_status_no_content( - response.status_code - ) + # Attach network device. + _tap = test_microvm.ssh_network_config(network_config, '1') + + # Configure MMDS version. + _configure_mmds(test_microvm, iface_id='1', version=version) # Populate data store with contents. _populate_data_store(test_microvm, data_store) test_microvm.basic_config(vcpu_count=1) - _tap = test_microvm.ssh_network_config( - network_config, - '1', - allow_mmds_requests=True - ) - test_microvm.start() ssh_connection = net_tools.SSHConnection(test_microvm.ssh_config) @@ -304,22 +306,16 @@ def test_mmds_response(test_microvm_with_api, network_config, version): } } } - # Configure MMDS version. - response = test_microvm.mmds.put_config(json={'version': version}) - assert test_microvm.api_session.is_status_no_content( - response.status_code - ) + # Attach network device. + _tap = test_microvm.ssh_network_config(network_config, '1') + + # Configure MMDS version. + _configure_mmds(test_microvm, iface_id='1', version=version) # Populate data store with contents. _populate_data_store(test_microvm, data_store) test_microvm.basic_config(vcpu_count=1) - _tap = test_microvm.ssh_network_config( - network_config, - '1', - allow_mmds_requests=True - ) - test_microvm.start() ssh_connection = net_tools.SSHConnection(test_microvm.ssh_config) @@ -389,9 +385,10 @@ def test_larger_than_mss_payloads( test_microvm = test_microvm_with_api test_microvm.spawn() + # Attach network device. + _tap = test_microvm.ssh_network_config(network_config, '1') # Configure MMDS version. - response = test_microvm.mmds.put_config(json={'version': version}) - assert test_microvm.api_session.is_status_no_content(response.status_code) + _configure_mmds(test_microvm, iface_id='1', version=version) # The MMDS is empty at this point. response = test_microvm.mmds.get() @@ -399,12 +396,6 @@ def test_larger_than_mss_payloads( assert response.json() == {} test_microvm.basic_config(vcpu_count=1) - _tap = test_microvm.ssh_network_config( - network_config, - '1', - allow_mmds_requests=True - ) - test_microvm.start() # Make sure MTU is 1500 bytes. @@ -478,7 +469,7 @@ def test_larger_than_mss_payloads( "version", MMDS_VERSIONS ) -def test_mmds_dummy(test_microvm_with_api, version): +def test_mmds_dummy(test_microvm_with_api, network_config, version): """ Test the API and guest facing features of the microVM MetaData Service. @@ -487,9 +478,10 @@ def test_mmds_dummy(test_microvm_with_api, version): test_microvm = test_microvm_with_api test_microvm.spawn() + # Attach network device. + _tap = test_microvm.ssh_network_config(network_config, '1') # Configure MMDS version. - response = test_microvm.mmds.put_config(json={'version': version}) - assert test_microvm.api_session.is_status_no_content(response.status_code) + _configure_mmds(test_microvm, iface_id='1', version=version) # The MMDS is empty at this point. response = test_microvm.mmds.get() @@ -552,9 +544,10 @@ def test_guest_mmds_hang(test_microvm_with_api, network_config, version): test_microvm = test_microvm_with_api test_microvm.spawn() + # Attach network device. + _tap = test_microvm.ssh_network_config(network_config, '1') # Configure MMDS version. - response = test_microvm.mmds.put_config(json={'version': version}) - assert test_microvm.api_session.is_status_no_content(response.status_code) + _configure_mmds(test_microvm, iface_id='1', version=version) data_store = { 'latest': { @@ -566,12 +559,6 @@ def test_guest_mmds_hang(test_microvm_with_api, network_config, version): _populate_data_store(test_microvm, data_store) test_microvm.basic_config(vcpu_count=1) - _tap = test_microvm.ssh_network_config( - network_config, - '1', - allow_mmds_requests=True - ) - test_microvm.start() ssh_connection = net_tools.SSHConnection(test_microvm.ssh_config) @@ -618,7 +605,7 @@ def test_guest_mmds_hang(test_microvm_with_api, network_config, version): "version", MMDS_VERSIONS ) -def test_patch_dos_scenario(test_microvm_with_api, version): +def test_patch_dos_scenario(test_microvm_with_api, network_config, version): """ Test the MMDS json endpoint when data store size reaches the limit. @@ -627,9 +614,10 @@ def test_patch_dos_scenario(test_microvm_with_api, version): test_microvm = test_microvm_with_api test_microvm.spawn() + # Attach network device. + _tap = test_microvm.ssh_network_config(network_config, '1') # Configure MMDS version. - response = test_microvm.mmds.put_config(json={'version': version}) - assert test_microvm.api_session.is_status_no_content(response.status_code) + _configure_mmds(test_microvm, iface_id='1', version=version) dummy_json = { 'latest': { @@ -720,14 +708,23 @@ def test_mmds_snapshot(bin_cloner_path): @type: functional """ vm_builder = MicrovmBuilder(bin_cloner_path) - vm_instance = vm_builder.build_vm_nano(diff_snapshots=True) + net_iface = NetIfaceConfig() + vm_instance = vm_builder.build_vm_nano( + net_ifaces=[net_iface], + diff_snapshots=True + ) test_microvm = vm_instance.vm root_disk = vm_instance.disks[0] ssh_key = vm_instance.ssh_key - # Configure MMDS version. - response = test_microvm.mmds.put_config(json={'version': 'V2'}) - assert test_microvm.api_session.is_status_no_content(response.status_code) + ipv4_address = '169.254.169.250' + # Configure MMDS version with custom IPv4 address. + _configure_mmds( + test_microvm, + version='V2', + iface_id=DEFAULT_DEV_NAME, + ipv4_address=ipv4_address + ) data_store = { 'latest': { @@ -744,21 +741,21 @@ def test_mmds_snapshot(bin_cloner_path): disks = [root_disk.local_path()] ssh_connection = net_tools.SSHConnection(test_microvm.ssh_config) - cmd = 'ip route add 169.254.169.254 dev eth0' + cmd = 'ip route add {} dev eth0'.format(ipv4_address) _, stdout, stderr = ssh_connection.execute_command(cmd) _assert_out(stdout, stderr, '') # Generate token. token = _generate_mmds_session_token( ssh_connection, - ipv4_address="169.254.169.254", + ipv4_address=ipv4_address, token_ttl=60 ) pre = 'curl -m 2 -s' pre += ' -X GET' pre += ' -H "X-metadata-token: {}"'.format(token) - pre += ' http://169.254.169.254/' + pre += ' http://{}/'.format(ipv4_address) # Fetch metadata. cmd = pre + 'latest/meta-data/' @@ -794,13 +791,14 @@ def test_mmds_snapshot(bin_cloner_path): cmd = 'curl -m 2 -s' cmd += ' -X PUT' cmd += ' -H "X-metadata-token-ttl-seconds: 1"' - cmd += ' http://169.254.169.254/latest/api/token' + cmd += ' http://{}/latest/api/token'.format(ipv4_address) _, stdout, stderr = ssh_connection.execute_command(cmd) expected = "Not allowed HTTP method." _assert_out(stdout, stderr, expected) - # Fetch metadata using V1 requests. - cmd = 'curl -s http://169.254.169.254/latest/meta-data/ami-id/' + # Fetch metadata using V1 requests and ensure IPv4 configuration + # is persistent between snapshots. + cmd = 'curl -s http://{}/latest/meta-data/ami-id/'.format(ipv4_address) _, stdout, stderr = ssh_connection.execute_command(cmd) _assert_out(stdout, stderr, 'ami-12345678') @@ -814,9 +812,10 @@ def test_mmds_v2_negative(test_microvm_with_api, network_config): test_microvm = test_microvm_with_api test_microvm.spawn() + # Attach network device. + _tap = test_microvm.ssh_network_config(network_config, '1') # Configure MMDS version. - response = test_microvm.mmds.put_config(json={'version': 'V2'}) - assert test_microvm.api_session.is_status_no_content(response.status_code) + _configure_mmds(test_microvm, version='V2', iface_id='1') data_store = { 'latest': { @@ -831,12 +830,6 @@ def test_mmds_v2_negative(test_microvm_with_api, network_config): _populate_data_store(test_microvm, data_store) test_microvm.basic_config(vcpu_count=1) - _tap = test_microvm.ssh_network_config( - network_config, - '1', - allow_mmds_requests=True - ) - test_microvm.start() ssh_connection = net_tools.SSHConnection(test_microvm.ssh_config)