Move 'pos' and 'digest' fields out of FirewallRule, since they are not part of the rule definition itself but rather metadata associated with a rule's position in a list.
Introduce FirewallRuleListEntry which wraps a FirewallRule together with its position and digest, for use in list/get API responses. Add a dedicated FIREWALL_RULE_POS_SCHEMA for the rule position field, because we need it several times now. Derive Clone and PartialEq on FirewallRule and FirewallRuleListEntry, because this is a requirement to use it in the yew GUI. Drop Eq from FirewallRuleType since PartialEq is sufficient. Add dev-dependencies (proxmox-router, serde_json) and a test_fake_server_client_api test module with mock create_rule, update_rule, and get_rule API handlers to verify that the types work correctly with the #[api] macro. Signed-off-by: Dietmar Maurer <[email protected]> --- proxmox-firewall-api-types/Cargo.toml | 4 + proxmox-firewall-api-types/src/rule.rs | 130 ++++++++++++++++++++----- 2 files changed, 110 insertions(+), 24 deletions(-) diff --git a/proxmox-firewall-api-types/Cargo.toml b/proxmox-firewall-api-types/Cargo.toml index 2e894606..6685d75c 100644 --- a/proxmox-firewall-api-types/Cargo.toml +++ b/proxmox-firewall-api-types/Cargo.toml @@ -12,6 +12,10 @@ exclude.workspace = true [features] enum-fallback = ["dep:proxmox-fixed-string"] +[dev-dependencies] +proxmox-router = { workspace = true } +serde_json = { workspace = true } + [dependencies] anyhow.workspace = true regex.workspace = true diff --git a/proxmox-firewall-api-types/src/rule.rs b/proxmox-firewall-api-types/src/rule.rs index 48869b25..d067c209 100644 --- a/proxmox-firewall-api-types/src/rule.rs +++ b/proxmox-firewall-api-types/src/rule.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use proxmox_fixed_string::FixedString; use proxmox_schema::api_types::COMMENT_SCHEMA; -use proxmox_schema::{api, const_regex, ApiStringFormat, Updater}; +use proxmox_schema::{api, const_regex, ApiStringFormat, IntegerSchema, Schema, Updater}; use crate::{FirewallAddressMatch, FirewallIcmpType, FirewallPortList}; use crate::{FIREWALL_DPORT_API_SCHEMA, FIREWALL_SPORT_API_SCHEMA}; @@ -16,6 +16,10 @@ const_regex! { FIREWALL_SECURITY_GROUP_RE = r##"^[A-Za-z][A-Za-z0-9\-\_]+$"##; } +const FIREWALL_RULE_POS_SCHEMA: Schema = IntegerSchema::new("Rule position <pos>.") + .minimum(0) + .schema(); + #[api( properties: { action: { @@ -31,9 +35,6 @@ const_regex! { dest: { optional: true, }, - digest: { - optional: true, - }, dport: { optional: true, schema: FIREWALL_DPORT_API_SCHEMA, @@ -61,11 +62,6 @@ const_regex! { optional: true, type: String, }, - pos: { - minimum: 0, - optional: true, - type: Integer, - }, proto: { max_length: 64, // arbitrary limit, much longer than anything in /etc/protocols optional: true, @@ -84,7 +80,7 @@ const_regex! { }, )] /// Firewall Rule. -#[derive(Debug, serde::Deserialize, serde::Serialize, Updater)] +#[derive(Debug, serde::Deserialize, serde::Serialize, Updater, Clone, PartialEq)] pub struct FirewallRule { /// Rule action ('ACCEPT', 'DROP', 'REJECT') or security group name. #[updater(serde(default, skip_serializing_if = "Option::is_none"))] @@ -97,12 +93,6 @@ pub struct FirewallRule { #[serde(default, skip_serializing_if = "Option::is_none")] pub dest: Option<FirewallAddressMatch>, - /// Prevent changes if current configuration file has a different digest. - /// This can be used to prevent concurrent modifications. - #[serde(default, skip_serializing_if = "Option::is_none")] - #[updater(type = "Option<ConfigDigest>")] - pub digest: Option<ConfigDigest>, - /// Restrict TCP/UDP destination port. #[serde(default, skip_serializing_if = "Option::is_none")] pub dport: Option<FirewallPortList>, @@ -132,11 +122,6 @@ pub struct FirewallRule { #[serde(rename = "macro")] pub r#macro: Option<String>, - /// Update rule at position <pos>. - #[serde(deserialize_with = "proxmox_serde::perl::deserialize_u64")] - #[serde(default, skip_serializing_if = "Option::is_none")] - pub pos: Option<u64>, - /// IP protocol. You can use protocol names ('tcp'/'udp') or simple numbers, /// as defined in '/etc/protocols'. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -154,9 +139,30 @@ pub struct FirewallRule { pub ty: FirewallRuleType, } +#[api( + properties: { + pos: { + optional: true, + schema: FIREWALL_RULE_POS_SCHEMA, + }, + rule: { + type: FirewallRule, + flatten: true, + } + }, +)] +#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] +/// Firewall rule list entry. Includes position, digest and the rule itself. +pub struct FirewallRuleListEntry { + pub pos: u64, + pub digest: ConfigDigest, + #[serde(flatten)] + pub rule: FirewallRule, +} + #[api] /// Rule type. -#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)] +#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize)] pub enum FirewallRuleType { #[serde(rename = "in")] /// in. @@ -209,7 +215,6 @@ mod test { // Numeric fields - these are Option<u64> in updater updater.enable = Some(1); - updater.pos = Some(0); // Enum fields - these are Option<EnumType> in updater updater.ty = Some(FirewallRuleType::In); @@ -223,6 +228,83 @@ mod test { updater.dport = Some("80".parse().unwrap()); updater.sport = Some("1024:65535".parse().unwrap()); updater.icmp_type = Some(FirewallIcmpType::Named(FirewallIcmpTypeName::EchoRequest)); - updater.digest = Some(ConfigDigest::from([0u8; 32])); + } +} + +#[cfg(test)] +mod test_fake_server_client_api { + use super::*; + use anyhow::Error; + use proxmox_schema::api; + + #[api( + input: { + properties: { + rule: { + type: FirewallRule, + flatten: true + }, + pos: { + optional: true, + schema: FIREWALL_RULE_POS_SCHEMA, + }, + digest: { + optional: true, + }, + }, + }, + )] + /// create rule + pub fn create_rule( + rule: FirewallRule, + pos: Option<u64>, + digest: Option<ConfigDigest>, + ) -> Result<(), Error> { + unimplemented!(); + } + + #[api( + input: { + properties: { + rule: { + type: FirewallRule, + flatten: true + }, + pos: { + schema: FIREWALL_RULE_POS_SCHEMA, + }, + digest: { + optional: true, + }, + }, + }, + )] + /// update rule + pub fn update_rule( + pos: u64, + rule: FirewallRuleUpdater, + digest: Option<ConfigDigest>, + ) -> Result<(), Error> { + unimplemented!(); + } + + #[api( + input: { + properties: { + pos: { + schema: FIREWALL_RULE_POS_SCHEMA, + }, + digest: { + optional: true, + }, + }, + }, + )] + /// update rule + pub fn get_rule( + pos: u64, + digest: Option<ConfigDigest>, + ) -> Result<FirewallRuleListEntry, Error> { + unimplemented!(); } } -- 2.47.3
