From 9b271d81b52f835c669e4eb1b3195adaecf56c0c Mon Sep 17 00:00:00 2001 From: Till Wegmueller Date: Sun, 27 Jul 2025 11:30:24 +0200 Subject: [PATCH] Add support for `User`, `Group`, `Driver`, `Legacy`, and `Transform` actions in `Manifest` parsing - Implement parsing and mapping of new action types: `User`, `Group`, `Driver`, `Legacy`, and `Transform` within `Manifest`. - Introduce comprehensive test coverage for the newly supported actions, including edge cases for `ftpuser` property in manifests. - Refactor to leverage `HashSet` for user `services` to avoid duplicates. - Improve comments and error handling during action parsing for enhanced clarity and robustness. --- libips/src/actions/mod.rs | 321 +++++++++++++++++++++++++- libips/tests/test_manifest_parsing.rs | 59 +++++ libips/tests/test_user_services.rs | 124 ++++++++++ 3 files changed, 497 insertions(+), 7 deletions(-) create mode 100644 libips/tests/test_manifest_parsing.rs create mode 100644 libips/tests/test_user_services.rs diff --git a/libips/src/actions/mod.rs b/libips/src/actions/mod.rs index 8374c17..3ff018c 100644 --- a/libips/src/actions/mod.rs +++ b/libips/src/actions/mod.rs @@ -14,7 +14,7 @@ use pest::Parser; use pest_derive::Parser; use serde::{Deserialize, Serialize}; use std::clone::Clone; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fs::read_to_string; use std::path::Path; use std::result::Result as StdResult; @@ -50,7 +50,7 @@ pub enum ActionError { } pub trait FacetedAction { - // Add a facet to the action if the facet is already present the function returns false. + // Add a facet to the action if the facet is already present, the function returns false. fn add_facet(&mut self, facet: Facet) -> bool; // Remove a facet from the action. @@ -470,6 +470,303 @@ impl From for Link { } } +#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)] +#[diff(attr( + #[derive(Debug, PartialEq)] +))] +pub struct User { + pub username: String, + pub uid: String, + pub group: String, + pub home_dir: String, + pub login_shell: String, + pub password: String, + pub services: HashSet, + pub gcos_field: String, + pub properties: HashMap, + pub facets: HashMap, +} + +impl From for User { + fn from(act: Action) -> Self { + let mut user = User::default(); + let mut props = act.properties; + if !act.payload_string.is_empty() { + let p_str = split_property(act.payload_string); + props.push(Property { + key: p_str.0, + value: p_str.1, + }) + } + for prop in props { + match prop.key.as_str() { + "username" => user.username = prop.value, + "uid" => user.uid = prop.value, + "group" => user.group = prop.value, + "home-dir" => user.home_dir = prop.value, + "login-shell" => user.login_shell = prop.value, + "password" => user.password = prop.value, + "gcos-field" => user.gcos_field = prop.value, + "ftpuser" => { + // Parse ftpuser property into services + match string_to_bool(&prop.value) { + // If it's a boolean value (backward compatibility) + Ok(true) => { user.services.insert("ftp".to_string()); }, + Ok(false) => {}, // No services if false + // If the value not a boolean, treat as a comma-separated list of services + _ => { + for service in prop.value.split(',') { + let service = service.trim(); + if !service.is_empty() { + user.services.insert(service.to_string()); + } + } + } + } + } + _ => { + if is_facet(prop.key.clone()) { + user.add_facet(Facet::from_key_value(prop.key, prop.value)); + } else { + user.properties.insert( + prop.key.clone(), + Property { + key: prop.key, + value: prop.value, + }, + ); + } + } + } + } + user + } +} + +impl FacetedAction for User { + fn add_facet(&mut self, facet: Facet) -> bool { + self.facets.insert(facet.name.clone(), facet).is_none() + } + + fn remove_facet(&mut self, facet: Facet) -> bool { + self.facets.remove(&facet.name) == Some(facet) + } +} + +#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)] +#[diff(attr( + #[derive(Debug, PartialEq)] +))] +pub struct Group { + pub groupname: String, + pub gid: String, + pub properties: HashMap, + pub facets: HashMap, +} + +impl From for Group { + fn from(act: Action) -> Self { + let mut group = Group::default(); + let mut props = act.properties; + if !act.payload_string.is_empty() { + let p_str = split_property(act.payload_string); + props.push(Property { + key: p_str.0, + value: p_str.1, + }) + } + for prop in props { + match prop.key.as_str() { + "groupname" => group.groupname = prop.value, + "gid" => group.gid = prop.value, + _ => { + if is_facet(prop.key.clone()) { + group.add_facet(Facet::from_key_value(prop.key, prop.value)); + } else { + group.properties.insert( + prop.key.clone(), + Property { + key: prop.key, + value: prop.value, + }, + ); + } + } + } + } + group + } +} + +impl FacetedAction for Group { + fn add_facet(&mut self, facet: Facet) -> bool { + self.facets.insert(facet.name.clone(), facet).is_none() + } + + fn remove_facet(&mut self, facet: Facet) -> bool { + self.facets.remove(&facet.name) == Some(facet) + } +} + +#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)] +#[diff(attr( + #[derive(Debug, PartialEq)] +))] +pub struct Driver { + pub name: String, + pub class: String, + pub perms: String, + pub clone_perms: String, + pub alias: String, + pub properties: HashMap, + pub facets: HashMap, +} + +impl From for Driver { + fn from(act: Action) -> Self { + let mut driver = Driver::default(); + let mut props = act.properties; + if !act.payload_string.is_empty() { + let p_str = split_property(act.payload_string); + props.push(Property { + key: p_str.0, + value: p_str.1, + }) + } + for prop in props { + match prop.key.as_str() { + "name" => driver.name = prop.value, + "class" => driver.class = prop.value, + "perms" => driver.perms = prop.value, + "clone_perms" => driver.clone_perms = prop.value, + "alias" => driver.alias = prop.value, + _ => { + if is_facet(prop.key.clone()) { + driver.add_facet(Facet::from_key_value(prop.key, prop.value)); + } else { + driver.properties.insert( + prop.key.clone(), + Property { + key: prop.key, + value: prop.value, + }, + ); + } + } + } + } + driver + } +} + +impl FacetedAction for Driver { + fn add_facet(&mut self, facet: Facet) -> bool { + self.facets.insert(facet.name.clone(), facet).is_none() + } + + fn remove_facet(&mut self, facet: Facet) -> bool { + self.facets.remove(&facet.name) == Some(facet) + } +} + +#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)] +#[diff(attr( + #[derive(Debug, PartialEq)] +))] +pub struct Legacy { + pub arch: String, + pub category: String, + pub desc: String, + pub hotline: String, + pub name: String, + pub pkg: String, + pub vendor: String, + pub version: String, + pub properties: HashMap, +} + +impl From for Legacy { + fn from(act: Action) -> Self { + let mut legacy = Legacy::default(); + let mut props = act.properties; + if !act.payload_string.is_empty() { + let p_str = split_property(act.payload_string); + props.push(Property { + key: p_str.0, + value: p_str.1, + }) + } + for prop in props { + match prop.key.as_str() { + "arch" => legacy.arch = prop.value, + "category" => legacy.category = prop.value, + "desc" => legacy.desc = prop.value, + "hotline" => legacy.hotline = prop.value, + "name" => legacy.name = prop.value, + "pkg" => legacy.pkg = prop.value, + "vendor" => legacy.vendor = prop.value, + "version" => legacy.version = prop.value, + _ => { + legacy.properties.insert( + prop.key.clone(), + Property { + key: prop.key, + value: prop.value, + }, + ); + } + } + } + legacy + } +} + +#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)] +#[diff(attr( + #[derive(Debug, PartialEq)] +))] +pub struct Transform { + pub transform_type: String, + pub pattern: String, + pub match_type: String, + pub operation: String, + pub value: String, + pub properties: HashMap, +} + +impl From for Transform { + fn from(act: Action) -> Self { + let mut transform = Transform::default(); + let mut props = act.properties; + if !act.payload_string.is_empty() { + let p_str = split_property(act.payload_string); + props.push(Property { + key: p_str.0, + value: p_str.1, + }) + } + for prop in props { + match prop.key.as_str() { + "type" => transform.transform_type = prop.value, + "pattern" => transform.pattern = prop.value, + "match_type" => transform.match_type = prop.value, + "operation" => transform.operation = prop.value, + "value" => transform.value = prop.value, + _ => { + transform.properties.insert( + prop.key.clone(), + Property { + key: prop.key, + value: prop.value, + }, + ); + } + } + } + transform + } +} + #[derive(Hash, Eq, PartialEq, Debug, Default, Clone, Deserialize, Serialize, Diff)] #[diff(attr( #[derive(Debug, PartialEq)] @@ -490,6 +787,11 @@ pub struct Manifest { pub dependencies: Vec, pub licenses: Vec, pub links: Vec, + pub users: Vec, + pub groups: Vec, + pub drivers: Vec, + pub legacies: Vec, + pub transforms: Vec, } impl Manifest { @@ -501,6 +803,11 @@ impl Manifest { dependencies: Vec::new(), licenses: Vec::new(), links: Vec::new(), + users: Vec::new(), + groups: Vec::new(), + drivers: Vec::new(), + legacies: Vec::new(), + transforms: Vec::new(), } } @@ -523,13 +830,13 @@ impl Manifest { self.dependencies.push(act.into()); } ActionKind::User => { - todo!() + self.users.push(act.into()); } ActionKind::Group => { - todo!() + self.groups.push(act.into()); } ActionKind::Driver => { - todo!() + self.drivers.push(act.into()); } ActionKind::License => { self.licenses.push(act.into()); @@ -538,10 +845,10 @@ impl Manifest { self.links.push(act.into()); } ActionKind::Legacy => { - todo!() + self.legacies.push(act.into()); } ActionKind::Transform => { - todo!() + self.transforms.push(act.into()); } ActionKind::Unknown { action } => { panic!("action {:?} not known", action) diff --git a/libips/tests/test_manifest_parsing.rs b/libips/tests/test_manifest_parsing.rs new file mode 100644 index 0000000..ecf18c4 --- /dev/null +++ b/libips/tests/test_manifest_parsing.rs @@ -0,0 +1,59 @@ +extern crate libips; + +use libips::actions::Manifest; +use std::path::Path; + +#[test] +fn test_parse_postgre_common_manifest() { + let manifest_path = Path::new("/home/toasty/ws/illumos/ips/pkg6repo/postgre-common.manifest"); + let manifest = Manifest::parse_file(manifest_path).expect("Failed to parse manifest"); + + // Check that the manifest contains the expected actions + assert_eq!(manifest.attributes.len(), 11, "Expected 11 attributes"); + assert_eq!(manifest.directories.len(), 1, "Expected 1 directory"); + assert_eq!(manifest.groups.len(), 1, "Expected 1 group"); + assert_eq!(manifest.users.len(), 1, "Expected 1 user"); + assert_eq!(manifest.licenses.len(), 1, "Expected 1 license"); + + // Check the group action + let group = &manifest.groups[0]; + assert_eq!(group.groupname, "postgres", "Expected groupname to be 'postgres'"); + assert_eq!(group.gid, "90", "Expected gid to be '90'"); + + // Check the user action + let user = &manifest.users[0]; + assert_eq!(user.username, "postgres", "Expected username to be 'postgres'"); + assert_eq!(user.uid, "90", "Expected uid to be '90'"); + assert_eq!(user.group, "postgres", "Expected group to be 'postgres'"); + assert_eq!(user.home_dir, "/var/postgres", "Expected home_dir to be '/var/postgres'"); + assert_eq!(user.login_shell, "/usr/bin/pfksh", "Expected login_shell to be '/usr/bin/pfksh'"); + assert_eq!(user.password, "NP", "Expected password to be 'NP'"); + assert!(user.services.is_empty(), "Expected no services for ftpuser=false"); + assert_eq!(user.gcos_field, "PostgreSQL Reserved UID", "Expected gcos_field to be 'PostgreSQL Reserved UID'"); + + // Check the directory action + let dir = &manifest.directories[0]; + assert_eq!(dir.path, "var/postgres", "Expected path to be 'var/postgres'"); + assert_eq!(dir.group, "postgres", "Expected group to be 'postgres'"); + assert_eq!(dir.owner, "postgres", "Expected owner to be 'postgres'"); + assert_eq!(dir.mode, "0755", "Expected mode to be '0755'"); +} + +#[test] +fn test_parse_pgadmin_manifest() { + let manifest_path = Path::new("/home/toasty/ws/illumos/ips/pkg6repo/pgadmin.manifest"); + let manifest = Manifest::parse_file(manifest_path).expect("Failed to parse manifest"); + + // Check that the manifest contains the expected actions + assert!(manifest.attributes.len() > 0, "Expected attributes"); + assert!(manifest.files.len() > 0, "Expected files"); + assert_eq!(manifest.legacies.len(), 1, "Expected 1 legacy action"); + + // Check the legacy action + let legacy = &manifest.legacies[0]; + assert_eq!(legacy.arch, "i386", "Expected arch to be 'i386'"); + assert_eq!(legacy.category, "system", "Expected category to be 'system'"); + assert_eq!(legacy.pkg, "SUNWpgadmin3", "Expected pkg to be 'SUNWpgadmin3'"); + assert_eq!(legacy.vendor, "Project OpenIndiana", "Expected vendor to be 'Project OpenIndiana'"); + assert_eq!(legacy.version, "11.11.0,REV=2010.05.25.01.00", "Expected version to be '11.11.0,REV=2010.05.25.01.00'"); +} \ No newline at end of file diff --git a/libips/tests/test_user_services.rs b/libips/tests/test_user_services.rs new file mode 100644 index 0000000..6c6964c --- /dev/null +++ b/libips/tests/test_user_services.rs @@ -0,0 +1,124 @@ +extern crate libips; + +use libips::actions::Manifest; + +#[test] +fn test_ftpuser_boolean_true() { + // Create a manifest string with ftpuser=true + let manifest_string = "user ftpuser=true".to_string(); + + // Parse the manifest + let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest"); + + // Get the user + let user = &manifest.users[0]; + + // Check that "ftp" service is added + assert!(user.services.contains("ftp"), "Expected 'ftp' service to be added when ftpuser=true"); + assert_eq!(user.services.len(), 1, "Expected exactly one service"); +} + +#[test] +fn test_ftpuser_boolean_false() { + // Create a manifest string with ftpuser=false + let manifest_string = "user ftpuser=false".to_string(); + + // Parse the manifest + let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest"); + + // Get the user + let user = &manifest.users[0]; + + // Check that no services are added + assert!(user.services.is_empty(), "Expected no services when ftpuser=false"); +} + +#[test] +fn test_ftpuser_services_list() { + // Create a manifest string with ftpuser=ssh,ftp,http + let manifest_string = "user ftpuser=ssh,ftp,http".to_string(); + + // Parse the manifest + let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest"); + + // Get the user + let user = &manifest.users[0]; + + // Check that all services are added + assert!(user.services.contains("ssh"), "Expected 'ssh' service to be added"); + assert!(user.services.contains("ftp"), "Expected 'ftp' service to be added"); + assert!(user.services.contains("http"), "Expected 'http' service to be added"); + assert_eq!(user.services.len(), 3, "Expected exactly three services"); +} + +#[test] +fn test_ftpuser_services_with_whitespace() { + // Create a manifest string with ftpuser=ssh, ftp, http + let manifest_string = "user ftpuser=\"ssh, ftp, http\"".to_string(); + + // Parse the manifest + let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest"); + + // Get the user + let user = &manifest.users[0]; + + // Check that all services are added with whitespace trimmed + assert!(user.services.contains("ssh"), "Expected 'ssh' service to be added"); + assert!(user.services.contains("ftp"), "Expected 'ftp' service to be added"); + assert!(user.services.contains("http"), "Expected 'http' service to be added"); + assert_eq!(user.services.len(), 3, "Expected exactly three services"); +} + +#[test] +fn test_ftpuser_empty_string() { + // Create a manifest string with ftpuser= + let manifest_string = "user ftpuser=".to_string(); + + // Parse the manifest + let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest"); + + // Get the user + let user = &manifest.users[0]; + + // Check that no services are added + assert!(user.services.is_empty(), "Expected no services for empty string"); +} + +#[test] +fn test_ftpuser_with_empty_elements() { + // Create a manifest string with ftpuser=ssh,,http + let manifest_string = "user ftpuser=ssh,,http".to_string(); + + // Parse the manifest + let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest"); + + // Get the user + let user = &manifest.users[0]; + + // Check that only non-empty services are added + assert!(user.services.contains("ssh"), "Expected 'ssh' service to be added"); + assert!(user.services.contains("http"), "Expected 'http' service to be added"); + assert_eq!(user.services.len(), 2, "Expected exactly two services"); +} + +#[test] +fn test_real_world_example() { + // Create a manifest string similar to the one in postgre-common.manifest + let manifest_string = "user username=postgres uid=90 group=postgres home-dir=/var/postgres login-shell=/usr/bin/pfksh password=NP gcos-field=\"PostgreSQL Reserved UID\" ftpuser=false".to_string(); + + // Parse the manifest + let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest"); + + // Get the user + let user = &manifest.users[0]; + + // Check user properties + assert_eq!(user.username, "postgres"); + assert_eq!(user.uid, "90"); + assert_eq!(user.group, "postgres"); + assert_eq!(user.home_dir, "/var/postgres"); + assert_eq!(user.login_shell, "/usr/bin/pfksh"); + assert_eq!(user.password, "NP"); + assert_eq!(user.gcos_field, "PostgreSQL Reserved UID"); + assert!(user.services.is_empty(), "Expected no services for ftpuser=false"); +} \ No newline at end of file