From ca94fc306113881b7250595dcd6c1567c2c91392 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Wed, 29 Jul 2026 10:51:30 +0200 Subject: [PATCH] refactor(event): cleanup getter methods This is a small refactor done ahead of stackrox/fact#1059, cleaning up getters in the `FileData` type by using better pattern matching. The `Rename` variant is also streamlined by using a struct variant instead of a tuple holding `RenameFileData`. --- fact/src/event/mod.rs | 237 ++++++++++++++++++++---------------------- 1 file changed, 110 insertions(+), 127 deletions(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 98808af8..01ce7c2c 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -114,16 +114,13 @@ impl Event { }; FileData::Chmod(data) } - EventTestData::Rename(old_path) => { - let data = RenameFileData { - new: inner, - old: BaseFileData { - filename: old_path, - ..Default::default() - }, - }; - FileData::Rename(data) - } + EventTestData::Rename(old_path) => FileData::Rename { + new: inner, + old: BaseFileData { + filename: old_path, + ..Default::default() + }, + }, }; Ok(Event { @@ -155,7 +152,7 @@ impl Event { } pub fn is_rename(&self) -> bool { - matches!(self.file, FileData::Rename(_)) + matches!(self.file, FileData::Rename { .. }) } /// Unwrap the inner FileData and return the inode that triggered @@ -165,34 +162,34 @@ impl Event { /// the 'new' inode will be returned. pub fn get_inode(&self) -> &inode_key_t { match &self.file { - FileData::Open(data) => &data.inode, - FileData::Creation(data) => &data.inode, - FileData::MkDir(data) => &data.inode, - FileData::RmDir(data) => &data.inode, - FileData::Unlink(data) => &data.inode, - FileData::Chmod(data) => &data.inner.inode, - FileData::Chown(data) => &data.inner.inode, - FileData::Rename(data) => &data.new.inode, - FileData::SetXattr(data) => &data.inner.inode, - FileData::RemoveXattr(data) => &data.inner.inode, - FileData::AclSet(data) => &data.inner.inode, + FileData::Open(inner) + | FileData::Creation(inner) + | FileData::MkDir(inner) + | FileData::RmDir(inner) + | FileData::Unlink(inner) + | FileData::Chmod(ChmodFileData { inner, .. }) + | FileData::Chown(ChownFileData { inner, .. }) + | FileData::Rename { new: inner, .. } + | FileData::SetXattr(XattrFileData { inner, .. }) + | FileData::RemoveXattr(XattrFileData { inner, .. }) + | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.inode, } } /// Get the parent inode for the file in this event. pub fn get_parent_inode(&self) -> &inode_key_t { match &self.file { - FileData::Open(data) => &data.parent_inode, - FileData::Creation(data) => &data.parent_inode, - FileData::MkDir(data) => &data.parent_inode, - FileData::RmDir(data) => &data.parent_inode, - FileData::Unlink(data) => &data.parent_inode, - FileData::Chmod(data) => &data.inner.parent_inode, - FileData::Chown(data) => &data.inner.parent_inode, - FileData::Rename(data) => &data.new.parent_inode, - FileData::SetXattr(data) => &data.inner.parent_inode, - FileData::RemoveXattr(data) => &data.inner.parent_inode, - FileData::AclSet(data) => &data.inner.parent_inode, + FileData::Open(inner) + | FileData::Creation(inner) + | FileData::MkDir(inner) + | FileData::RmDir(inner) + | FileData::Unlink(inner) + | FileData::Chmod(ChmodFileData { inner, .. }) + | FileData::Chown(ChownFileData { inner, .. }) + | FileData::Rename { new: inner, .. } + | FileData::SetXattr(XattrFileData { inner, .. }) + | FileData::RemoveXattr(XattrFileData { inner, .. }) + | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.parent_inode, } } @@ -201,53 +198,53 @@ impl Event { /// will be returned. pub fn get_old_inode(&self) -> Option<&inode_key_t> { match &self.file { - FileData::Rename(data) => Some(&data.old.inode), + FileData::Rename { old, .. } => Some(&old.inode), _ => None, } } pub fn get_filename(&self) -> &PathBuf { match &self.file { - FileData::Open(data) => &data.filename, - FileData::Creation(data) => &data.filename, - FileData::MkDir(data) => &data.filename, - FileData::RmDir(data) => &data.filename, - FileData::Unlink(data) => &data.filename, - FileData::Chmod(data) => &data.inner.filename, - FileData::Chown(data) => &data.inner.filename, - FileData::Rename(data) => &data.new.filename, - FileData::SetXattr(data) => &data.inner.filename, - FileData::RemoveXattr(data) => &data.inner.filename, - FileData::AclSet(data) => &data.inner.filename, + FileData::Open(inner) + | FileData::Creation(inner) + | FileData::MkDir(inner) + | FileData::RmDir(inner) + | FileData::Unlink(inner) + | FileData::Chmod(ChmodFileData { inner, .. }) + | FileData::Chown(ChownFileData { inner, .. }) + | FileData::Rename { new: inner, .. } + | FileData::SetXattr(XattrFileData { inner, .. }) + | FileData::RemoveXattr(XattrFileData { inner, .. }) + | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.filename, } } pub fn get_old_filename(&self) -> Option<&PathBuf> { match &self.file { - FileData::Rename(data) => Some(&data.old.filename), + FileData::Rename { old, .. } => Some(&old.filename), _ => None, } } pub fn get_host_path(&self) -> &PathBuf { match &self.file { - FileData::Open(data) => &data.host_file, - FileData::Creation(data) => &data.host_file, - FileData::MkDir(data) => &data.host_file, - FileData::RmDir(data) => &data.host_file, - FileData::Unlink(data) => &data.host_file, - FileData::Chmod(data) => &data.inner.host_file, - FileData::Chown(data) => &data.inner.host_file, - FileData::Rename(data) => &data.new.host_file, - FileData::SetXattr(data) => &data.inner.host_file, - FileData::RemoveXattr(data) => &data.inner.host_file, - FileData::AclSet(data) => &data.inner.host_file, + FileData::Open(inner) + | FileData::Creation(inner) + | FileData::MkDir(inner) + | FileData::RmDir(inner) + | FileData::Unlink(inner) + | FileData::Chmod(ChmodFileData { inner, .. }) + | FileData::Chown(ChownFileData { inner, .. }) + | FileData::Rename { new: inner, .. } + | FileData::SetXattr(XattrFileData { inner, .. }) + | FileData::RemoveXattr(XattrFileData { inner, .. }) + | FileData::AclSet(AclSetFileData { inner, .. }) => &inner.host_file, } } pub fn get_old_host_path(&self) -> Option<&PathBuf> { match &self.file { - FileData::Rename(data) => Some(&data.old.host_file), + FileData::Rename { old, .. } => Some(&old.host_file), _ => None, } } @@ -258,47 +255,47 @@ impl Event { /// the 'new' host_file will be set. pub fn set_host_path(&mut self, host_path: PathBuf) { match &mut self.file { - FileData::Open(data) => data.host_file = host_path, - FileData::Creation(data) => data.host_file = host_path, - FileData::MkDir(data) => data.host_file = host_path, - FileData::RmDir(data) => data.host_file = host_path, - FileData::Unlink(data) => data.host_file = host_path, - FileData::Chmod(data) => data.inner.host_file = host_path, - FileData::Chown(data) => data.inner.host_file = host_path, - FileData::Rename(data) => data.new.host_file = host_path, - FileData::SetXattr(data) => data.inner.host_file = host_path, - FileData::RemoveXattr(data) => data.inner.host_file = host_path, - FileData::AclSet(data) => data.inner.host_file = host_path, + FileData::Open(inner) + | FileData::Creation(inner) + | FileData::MkDir(inner) + | FileData::RmDir(inner) + | FileData::Unlink(inner) + | FileData::Chmod(ChmodFileData { inner, .. }) + | FileData::Chown(ChownFileData { inner, .. }) + | FileData::Rename { new: inner, .. } + | FileData::SetXattr(XattrFileData { inner, .. }) + | FileData::RemoveXattr(XattrFileData { inner, .. }) + | FileData::AclSet(AclSetFileData { inner, .. }) => inner.host_file = host_path, } } /// Same as `set_host_path` but setting the 'old' host_file for /// operations that have one, like rename. pub fn set_old_host_path(&mut self, host_path: PathBuf) { - if let FileData::Rename(data) = &mut self.file { - data.old.host_file = host_path + if let FileData::Rename { old, .. } = &mut self.file { + old.host_file = host_path } } pub fn get_monitored(&self) -> monitored_t { match &self.file { - FileData::Open(data) => data.monitored, - FileData::Creation(data) => data.monitored, - FileData::MkDir(data) => data.monitored, - FileData::RmDir(data) => data.monitored, - FileData::Unlink(data) => data.monitored, - FileData::Chmod(data) => data.inner.monitored, - FileData::Chown(data) => data.inner.monitored, - FileData::Rename(data) => data.new.monitored, - FileData::SetXattr(data) => data.inner.monitored, - FileData::RemoveXattr(data) => data.inner.monitored, - FileData::AclSet(data) => data.inner.monitored, + FileData::Open(inner) + | FileData::Creation(inner) + | FileData::MkDir(inner) + | FileData::RmDir(inner) + | FileData::Unlink(inner) + | FileData::Chmod(ChmodFileData { inner, .. }) + | FileData::Chown(ChownFileData { inner, .. }) + | FileData::Rename { new: inner, .. } + | FileData::SetXattr(XattrFileData { inner, .. }) + | FileData::RemoveXattr(XattrFileData { inner, .. }) + | FileData::AclSet(AclSetFileData { inner, .. }) => inner.monitored, } } pub fn get_old_monitored(&self) -> Option { match &self.file { - FileData::Rename(data) => Some(data.old.monitored), + FileData::Rename { old, .. } => Some(old.monitored), _ => None, } } @@ -401,7 +398,10 @@ pub enum FileData { Unlink(BaseFileData), Chmod(ChmodFileData), Chown(ChownFileData), - Rename(RenameFileData), + Rename { + new: BaseFileData, + old: BaseFileData, + }, SetXattr(XattrFileData), RemoveXattr(XattrFileData), AclSet(AclSetFileData), @@ -445,7 +445,7 @@ impl FileData { let old_filename = unsafe { extra_data.rename.filename }; let old_inode = unsafe { extra_data.rename.inode }; let old_monitored = unsafe { extra_data.rename.monitored }; - let data = RenameFileData { + FileData::Rename { new: inner, old: BaseFileData::new( old_filename, @@ -453,8 +453,7 @@ impl FileData { Default::default(), old_monitored, )?, - }; - FileData::Rename(data) + } } file_activity_type_t::FILE_ACTIVITY_SETXATTR => { let xattr_name = slice_to_string( @@ -499,7 +498,7 @@ impl FileData { FileData::Unlink(_) => "unlink", FileData::Chmod(_) => "permission", FileData::Chown(_) => "ownership", - FileData::Rename(_) => "rename", + FileData::Rename { .. } => "rename", FileData::SetXattr(_) => "xattr_set", FileData::RemoveXattr(_) => "xattr_remove", FileData::AclSet(_) => "acl", @@ -547,8 +546,11 @@ impl From for fact_api::file_activity::File { let f_act = fact_api::FileOwnershipChange::from(event); fact_api::file_activity::File::Ownership(f_act) } - FileData::Rename(event) => { - let f_act = fact_api::FileRename::from(event); + FileData::Rename { new, old } => { + let f_act = fact_api::FileRename { + new: Some(new.into()), + old: Some(old.into()), + }; fact_api::file_activity::File::Rename(f_act) } FileData::AclSet(event) => { @@ -571,7 +573,14 @@ impl From for opentelemetry::logs::AnyValue { | FileData::Unlink(data) => AnyValue::from(data), FileData::Chmod(data) => AnyValue::from(data), FileData::Chown(data) => AnyValue::from(data), - FileData::Rename(data) => AnyValue::from(data), + FileData::Rename { new, old } => { + let AnyValue::Map(mut map) = AnyValue::from(new) else { + unreachable!("new value did not serialize to map"); + }; + map.insert("old".into(), AnyValue::from(old)); + + AnyValue::Map(map) + } FileData::SetXattr(data) | FileData::RemoveXattr(data) => AnyValue::from(data), FileData::AclSet(data) => AnyValue::from(data), }) else { @@ -595,7 +604,16 @@ impl PartialEq for FileData { (FileData::Unlink(this), FileData::Unlink(other)) => this == other, (FileData::Chmod(this), FileData::Chmod(other)) => this == other, (FileData::Chown(this), FileData::Chown(other)) => this == other, - (FileData::Rename(this), FileData::Rename(other)) => this == other, + ( + FileData::Rename { + new: l_new, + old: l_old, + }, + FileData::Rename { + new: r_new, + old: r_old, + }, + ) => l_new == r_new && l_old == r_old, (FileData::SetXattr(this), FileData::SetXattr(other)) => this == other, (FileData::RemoveXattr(this), FileData::RemoveXattr(other)) => this == other, (FileData::AclSet(this), FileData::AclSet(other)) => { @@ -768,34 +786,6 @@ impl From for opentelemetry::logs::AnyValue { } } -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct RenameFileData { - new: BaseFileData, - old: BaseFileData, -} - -impl From for fact_api::FileRename { - fn from(RenameFileData { new, old }: RenameFileData) -> Self { - let new = fact_api::FileActivityBase::from(new); - let old = fact_api::FileActivityBase::from(old); - fact_api::FileRename { - old: Some(old), - new: Some(new), - } - } -} - -#[cfg(feature = "otel")] -impl From for opentelemetry::logs::AnyValue { - fn from(value: RenameFileData) -> Self { - let AnyValue::Map(mut map) = value.new.into() else { - unreachable!("new value did not serialize to map"); - }; - map.insert("old".into(), value.old.into()); - AnyValue::Map(map) - } -} - #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] pub enum AclTag { UserObj, @@ -962,13 +952,6 @@ impl From for opentelemetry::logs::AnyValue { } } -#[cfg(test)] -impl PartialEq for RenameFileData { - fn eq(&self, other: &Self) -> bool { - self.new == other.new && self.old == other.old - } -} - #[derive(Debug, Clone, Serialize, Deserialize)] pub struct XattrFileData { inner: BaseFileData,