[component_manager] Forward path correctly for storage capabilities Fixed an issue where the storage capability was ignoring the relative path being passed in. This affected how storage capabilities worked in `ffx component explore` Fixed: 103669 Multiply: component_manager_tests Test: fx test component_manager_tests Change-Id: I5f1bcee8fbf8150726d1057ccef3ab82d010a0c7 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/695837 Reviewed-by: Miguel Flores <miguelfrde@google.com> Commit-Queue: Xyan Bhatnagar <xbhatnag@google.com>
diff --git a/src/sys/component_manager/src/model/namespace.rs b/src/sys/component_manager/src/model/namespace.rs index 02a3094..cec29a7 100644 --- a/src/sys/component_manager/src/model/namespace.rs +++ b/src/sys/component_manager/src/model/namespace.rs
@@ -308,6 +308,7 @@ OpenOptions::Storage(OpenStorageOptions { flags, open_mode: fio::MODE_TYPE_DIRECTORY, + relative_path: ".".into(), server_chan: &mut server_end, }), ),
diff --git a/src/sys/component_manager/src/model/routing/mod.rs b/src/sys/component_manager/src/model/routing/mod.rs index e5ddbc4..ba8b02b 100644 --- a/src/sys/component_manager/src/model/routing/mod.rs +++ b/src/sys/component_manager/src/model/routing/mod.rs
@@ -414,7 +414,13 @@ let dir_source = source.storage_provider.clone(); let relative_moniker_2 = relative_moniker.clone(); match options { - OpenOptions::Storage(OpenStorageOptions { flags, open_mode, server_chan, .. }) => { + OpenOptions::Storage(OpenStorageOptions { + flags, + open_mode, + relative_path, + server_chan, + .. + }) => { let storage_dir_proxy = storage::open_isolated_storage( source, target.persistent_storage, @@ -424,11 +430,17 @@ .await .map_err(|e| ModelError::from(e))?; - // Open the storage with the provided flags, mode and server_chan. - // We don't clone the directory because we can't specify the mode that way. + // Open the storage with the provided flags, mode, relative_path and server_chan. + // We don't clone the directory because we can't specify the mode or path that way. let server_chan = channel::take_channel(server_chan); - storage_dir_proxy.open(flags, open_mode, ".", ServerEnd::new(server_chan)).map_err( - |e| { + + // If there is no relative path, assume it is the current directory. We use "." + // because `fuchsia.io/Directory.Open` does not accept empty paths. + let relative_path = if relative_path.is_empty() { "." } else { &relative_path }; + + storage_dir_proxy + .open(flags, open_mode, relative_path, ServerEnd::new(server_chan)) + .map_err(|e| { let moniker = match &dir_source { Some(r) => InstancedExtendedMoniker::ComponentInstance( r.instanced_moniker().clone(), @@ -441,8 +453,7 @@ "", e, )) - }, - )?; + })?; return Ok(()); } _ => unreachable!("expected OpenStorageOptions"),
diff --git a/src/sys/component_manager/src/model/routing/open.rs b/src/sys/component_manager/src/model/routing/open.rs index cd1280a..26167ca 100644 --- a/src/sys/component_manager/src/model/routing/open.rs +++ b/src/sys/component_manager/src/model/routing/open.rs
@@ -68,9 +68,12 @@ relative_path, server_chan, })), - RouteRequest::UseStorage(_) => { - Ok(Self::Storage(OpenStorageOptions { flags, open_mode, server_chan })) - } + RouteRequest::UseStorage(_) => Ok(Self::Storage(OpenStorageOptions { + flags, + open_mode, + relative_path, + server_chan, + })), _ => Err(ModelError::unsupported("capability cannot be installed in a namespace")), } } @@ -112,6 +115,7 @@ pub struct OpenStorageOptions<'a> { pub flags: fio::OpenFlags, pub open_mode: u32, + pub relative_path: String, pub server_chan: &'a mut zx::Channel, }
diff --git a/src/sys/component_manager/src/model/tests/storage.rs b/src/sys/component_manager/src/model/tests/storage.rs index 12199ed..23ac719 100644 --- a/src/sys/component_manager/src/model/tests/storage.rs +++ b/src/sys/component_manager/src/model/tests/storage.rs
@@ -666,6 +666,7 @@ OpenOptions::Storage(OpenStorageOptions { flags: fio::OpenFlags::RIGHT_READABLE | fio::OpenFlags::RIGHT_WRITABLE, open_mode: fio::MODE_TYPE_DIRECTORY, + relative_path: ".".into(), server_chan: &mut server_end, }), ) @@ -702,6 +703,7 @@ OpenOptions::Storage(OpenStorageOptions { flags: fio::OpenFlags::RIGHT_READABLE | fio::OpenFlags::RIGHT_WRITABLE, open_mode: fio::MODE_TYPE_DIRECTORY, + relative_path: ".".into(), server_chan: &mut server_end, }), ) @@ -712,6 +714,123 @@ )); } +/// component manager's namespace +/// | +/// provider (provides storage capability, restricted to component ID index) +/// | +/// parent_consumer (in component ID index) +/// +/// Test that a component can open a subdirectory of a storage successfully +#[fuchsia::test] +async fn open_storage_subdirectory() { + let parent_consumer_instance_id = Some(gen_instance_id(&mut rand::thread_rng())); + let component_id_index_path = make_index_file(component_id_index::Index { + instances: vec![component_id_index::InstanceIdEntry { + instance_id: parent_consumer_instance_id.clone(), + appmgr_moniker: None, + moniker: Some(AbsoluteMoniker::parse_str("/consumer").unwrap()), + }], + ..component_id_index::Index::default() + }) + .unwrap(); + let components = vec![ + ( + "provider", + ComponentDeclBuilder::new() + .directory( + DirectoryDeclBuilder::new("data") + .path("/data") + .rights(*routing::rights::READ_RIGHTS | *routing::rights::WRITE_RIGHTS) + .build(), + ) + .storage(StorageDecl { + name: "cache".into(), + backing_dir: "data".try_into().unwrap(), + source: StorageDirectorySource::Self_, + subdir: None, + storage_id: fdecl::StorageId::StaticInstanceIdOrMoniker, + }) + .offer(OfferDecl::Storage(OfferStorageDecl { + source: OfferSource::Self_, + target: OfferTarget::static_child("consumer".to_string()), + source_name: "cache".into(), + target_name: "cache".into(), + availability: Availability::Required, + })) + .add_lazy_child("consumer") + .build(), + ), + ( + "consumer", + ComponentDeclBuilder::new() + .use_(UseDecl::Storage(UseStorageDecl { + source_name: "cache".into(), + target_path: "/storage".try_into().unwrap(), + availability: Availability::Required, + })) + .build(), + ), + ]; + let test = RoutingTestBuilder::new("provider", components) + .set_component_id_index_path(component_id_index_path.path().to_str().unwrap().to_string()) + .build() + .await; + + let consumer_moniker = AbsoluteMoniker::parse_str("/consumer").unwrap(); + let (consumer_instance, _) = test + .start_and_get_instance(&consumer_moniker, StartReason::Eager, false) + .await + .expect("could not resolve state"); + + // `consumer` should be able to open its storage at the root dir + let (root_dir, server_end) = fidl::endpoints::create_proxy::<fio::DirectoryMarker>().unwrap(); + let mut server_end = server_end.into_channel(); + route_and_open_capability( + RouteRequest::UseStorage(UseStorageDecl { + source_name: "cache".into(), + target_path: "/storage".try_into().unwrap(), + availability: cm_rust::Availability::Required, + }), + &consumer_instance, + OpenOptions::Storage(OpenStorageOptions { + flags: fio::OpenFlags::RIGHT_READABLE | fio::OpenFlags::RIGHT_WRITABLE, + open_mode: fio::MODE_TYPE_DIRECTORY, + relative_path: ".".into(), + server_chan: &mut server_end, + }), + ) + .await + .expect("Unable to route. oh no!!"); + + // Create the subdirectories we will open later + let bar_dir = fuchsia_fs::create_sub_directories(&root_dir, &PathBuf::from("foo/bar")).unwrap(); + let entries = fuchsia_fs::directory::readdir(&bar_dir).await.unwrap(); + assert!(entries.is_empty()); + + // `consumer` should be able to open its storage at "foo/bar" + let (bar_dir, server_end) = fidl::endpoints::create_proxy::<fio::DirectoryMarker>().unwrap(); + let mut server_end = server_end.into_channel(); + route_and_open_capability( + RouteRequest::UseStorage(UseStorageDecl { + source_name: "cache".into(), + target_path: "/storage".try_into().unwrap(), + availability: cm_rust::Availability::Required, + }), + &consumer_instance, + OpenOptions::Storage(OpenStorageOptions { + flags: fio::OpenFlags::RIGHT_READABLE | fio::OpenFlags::RIGHT_WRITABLE, + open_mode: fio::MODE_TYPE_DIRECTORY, + relative_path: "foo/bar".into(), + server_chan: &mut server_end, + }), + ) + .await + .expect("Unable to route. oh no!!"); + + let entries = fuchsia_fs::directory::readdir(&bar_dir).await.unwrap(); + assert!(entries.is_empty()); +} + /// a /// | /// b