[PATCH proxmox-ve-rs v3 10/45] ve-config: sdn: microseg: add group hierarchy

[Hannes Laimer <h.laimer@proxmox.com>]

A group may name a parent. An interface assigned to a subgroup also
belongs to every group above it. So a rule on a group covers its
subgroups, and a rule on a subgroup covers only that subgroup.

At the same priority the rules with the most specific all or exact
predicates decide, so a rule on a subgroup outranks the same rule on
its parent. Equally specific rules still deny if one of them denies.

Identity allocation is unchanged and gets each assignment's groups
together with their ancestors.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 proxmox-ve-config/src/sdn/microseg/api.rs   |  17 +-
 proxmox-ve-config/src/sdn/microseg/mod.rs   | 393 +++++++++++++++++++-
 proxmox-ve-config/src/sdn/microseg/types.rs |  12 +
 3 files changed, 405 insertions(+), 17 deletions(-)

diff --git a/proxmox-ve-config/src/sdn/microseg/api.rs b/proxmox-ve-config/src/sdn/microseg/api.rs
index fe125c4d4c1b..d9cd22344cb5 100644
--- a/proxmox-ve-config/src/sdn/microseg/api.rs
+++ b/proxmox-ve-config/src/sdn/microseg/api.rs
@@ -11,7 +11,7 @@ use serde::Deserialize;
 use crate::firewall::fw_macros;
 
 use super::types::{
-    Binding, GroupSection, GuestAssignmentSection, GuestInfo, MicrosegEntry, MicrosegId,
+    Binding, GroupName, GroupSection, GuestAssignmentSection, GuestInfo, MicrosegEntry, MicrosegId,
     MicrosegRunningConfig, RegexAssignmentSection, RuleSection, Tag, TagAssignmentSection,
 };
 use super::{PredicateMatch, assigned_nics, assignments, sorted_rules, validate, validate_entry};
@@ -129,6 +129,8 @@ pub enum MicrosegUpdate {
 
 #[derive(Debug, Clone, Deserialize)]
 pub struct GroupUpdate {
+    #[serde(default)]
+    pub parent: Option<GroupName>,
     #[serde(default)]
     pub comment: Option<String>,
     #[serde(default)]
@@ -138,6 +140,7 @@ pub struct GroupUpdate {
 #[derive(Debug, Clone, Copy, Deserialize)]
 #[serde(rename_all = "lowercase")]
 pub enum GroupDeletableProperties {
+    Parent,
     Comment,
 }
 
@@ -510,11 +513,15 @@ pub fn update(
     };
     let position = match (entry, update) {
         (MicrosegEntry::Group(group), MicrosegUpdate::Group(update)) => {
+            if update.parent.is_some() {
+                group.parent = update.parent;
+            }
             if update.comment.is_some() {
                 group.comment = update.comment;
             }
             for property in update.delete {
                 match property {
+                    GroupDeletableProperties::Parent => group.parent = None,
                     GroupDeletableProperties::Comment => group.comment = None,
                 }
             }
@@ -642,7 +649,9 @@ fn describe_referrer(entry: &MicrosegEntry) -> String {
             ),
             assignment.binding.comment.as_deref(),
         ),
-        MicrosegEntry::Group(_) => ("object".to_string(), None),
+        MicrosegEntry::Group(group) => {
+            (format!("subgroup '{}'", group.id), group.comment.as_deref())
+        }
     };
     match comment {
         Some(comment) => format!("{desc} (comment: \"{comment}\")"),
@@ -680,7 +689,9 @@ fn group_referenced_by(entries: &BTreeMap<String, MicrosegEntry>, name: &str) ->
                 rule.src.iter().any(|g| g.as_ref() == name)
                     || rule.dst.iter().any(|g| g.as_ref() == name)
             }
-            MicrosegEntry::Group(_) => false,
+            MicrosegEntry::Group(group) => {
+                group.parent.as_ref().is_some_and(|p| p.as_ref() == name)
+            }
             _ => entry
                 .assignment_groups()
                 .is_some_and(|groups| groups.iter().any(|g| g.as_ref() == name)),
diff --git a/proxmox-ve-config/src/sdn/microseg/mod.rs b/proxmox-ve-config/src/sdn/microseg/mod.rs
index 3aaf299f8169..1744e3e5fc02 100644
--- a/proxmox-ve-config/src/sdn/microseg/mod.rs
+++ b/proxmox-ve-config/src/sdn/microseg/mod.rs
@@ -19,7 +19,7 @@ pub use types::{
 };
 
 use std::cmp::Ordering;
-use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 
 use anyhow::{Context, Error, bail};
 use identity::UNTAGGED_GROUP;
@@ -72,7 +72,25 @@ fn validate_entry(
                 bail!("{subject} has an invalid name regex: {err}");
             }
         }
-        MicrosegEntry::GuestAssignment(_) | MicrosegEntry::Group(_) => {}
+        MicrosegEntry::Group(group) => {
+            if let Some(parent) = &group.parent {
+                known_group(entries, parent.as_ref()).with_context(|| subject.to_string())?;
+                // the walk may start off the cycle, so the error names the group whose parent
+                // link closes the cycle
+                let name = group.id.as_str();
+                let mut seen: HashSet<&str> = HashSet::from([name]);
+                let mut closing = name;
+                let mut current = Some(parent.as_str());
+                while let Some(group) = current {
+                    if !seen.insert(group) {
+                        bail!("group '{closing}' is its own ancestor through '{group}'");
+                    }
+                    closing = group;
+                    current = parent_of(entries, group);
+                }
+            }
+        }
+        MicrosegEntry::GuestAssignment(_) => {}
     }
     if let Some(groups) = entry.assignment_groups() {
         if groups.is_empty() {
@@ -163,6 +181,13 @@ fn is_group(entries: &BTreeMap<String, MicrosegEntry>, name: &str) -> bool {
     matches!(entries.get(name), Some(MicrosegEntry::Group(_)))
 }
 
+fn parent_of<'a>(entries: &'a BTreeMap<String, MicrosegEntry>, name: &str) -> Option<&'a str> {
+    match entries.get(name)? {
+        MicrosegEntry::Group(group) => Some(group.parent.as_ref()?.as_ref()),
+        _ => None,
+    }
+}
+
 fn known_group(entries: &BTreeMap<String, MicrosegEntry>, name: &str) -> Result<(), Error> {
     if name != UNTAGGED_GROUP && !is_group(entries, name) {
         bail!("unknown group '{name}'");
@@ -183,18 +208,65 @@ fn group_set<'a>(
         .collect()
 }
 
+/// The groups with all their ancestors. Membership in a subgroup implies membership in every group
+/// above it.
+fn closed_groups<'a>(
+    entries: &BTreeMap<String, MicrosegEntry>,
+    groups: impl IntoIterator<Item = &'a MicrosegId>,
+) -> Result<identity::GroupSet, Error> {
+    let mut set = identity::GroupSet::new();
+    for group in groups {
+        let mut current = group.as_ref();
+        loop {
+            known_group(entries, current)?;
+            // a group already in the set ends the walk, so a parent cycle that validation rejects
+            // cannot spin here
+            if !set.insert(current.to_string()) {
+                break;
+            }
+            match parent_of(entries, current) {
+                Some(parent) => current = parent,
+                None => break,
+            }
+        }
+    }
+    Ok(set)
+}
+
+/// An `all` or `exact` predicate gets the ancestors of its groups added, like the sets it is
+/// matched against. An `all` fires on the same sets either way, and an `exact` naming a subgroup
+/// could not fire at all without them. An `any` stays as written, since with the ancestors added it
+/// would also fire on sets holding only an ancestor of a group it names.
 fn predicate(
     entries: &BTreeMap<String, MicrosegEntry>,
     match_kind: PredicateMatch,
     groups: &[MicrosegId],
 ) -> Result<identity::Predicate, Error> {
-    let set = group_set(entries, groups)?;
+    let set = match match_kind {
+        PredicateMatch::Any => group_set(entries, groups)?,
+        PredicateMatch::All | PredicateMatch::Exact => closed_groups(entries, groups)?,
+        PredicateMatch::Every => identity::GroupSet::new(),
+    };
     Ok(identity::Predicate {
         kind: match_kind,
         groups: set,
     })
 }
 
+/// How much a predicate pins down, the size of its closed group set for `all` and `exact`, where
+/// every group named is a further condition. An `any` gets weaker the more it names, so it
+/// counts nothing.
+fn predicate_specificity(predicate: &identity::Predicate) -> usize {
+    match predicate.kind {
+        PredicateMatch::Any | PredicateMatch::Every => 0,
+        PredicateMatch::All | PredicateMatch::Exact => predicate.groups.len(),
+    }
+}
+
+fn specificity(src: &identity::Predicate, dst: &identity::Predicate) -> usize {
+    predicate_specificity(src) + predicate_specificity(dst)
+}
+
 fn rule_entries(entries: &BTreeMap<String, MicrosegEntry>) -> impl Iterator<Item = &RuleSection> {
     entries.values().filter_map(|entry| match entry {
         MicrosegEntry::Rule(rule) => Some(rule),
@@ -373,7 +445,8 @@ pub fn rules_by_group_set(
     classes
         .into_iter()
         .map(|class| {
-            let set = group_set(entries, class.iter())?;
+            // the class keeps its written groups, only the matching sees the closure
+            let set = closed_groups(entries, class.iter())?;
             let mut outbound = Vec::new();
             let mut inbound = Vec::new();
             for (rule, policy) in &rules {
@@ -395,7 +468,15 @@ pub fn rules_by_group_set(
         .collect()
 }
 
-/// The distinct group sets the assignments name, the input the identity allocator partitions.
+pub fn rule_specificity(
+    entries: &BTreeMap<String, MicrosegEntry>,
+) -> Result<HashMap<MicrosegId, usize>, Error> {
+    Ok(sorted_policies(entries)?
+        .into_iter()
+        .map(|(rule, policy)| (rule.id.clone(), specificity(&policy.src, &policy.dst)))
+        .collect())
+}
+
 fn assignment_group_sets(
     entries: &BTreeMap<String, MicrosegEntry>,
 ) -> Result<BTreeSet<identity::GroupSet>, Error> {
@@ -403,7 +484,7 @@ fn assignment_group_sets(
         .iter()
         .filter_map(|(id, entry)| Some((id, entry.assignment_groups()?)))
         .map(|(id, groups)| {
-            group_set(entries, groups).with_context(|| format!("assignment '{id}'"))
+            closed_groups(entries, groups).with_context(|| format!("assignment '{id}'"))
         })
         .collect()
 }
@@ -432,6 +513,7 @@ fn evaluation_order(a: &ResolvedRule, b: &ResolvedRule) -> Ordering {
     b.prio
         .cmp(&a.prio)
         .then(b.is_narrowed().cmp(&a.is_narrowed()))
+        .then(b.specificity().cmp(&a.specificity()))
         .then(a.allow.cmp(&b.allow))
         // the id keeps the order stable, and the orientation after it, since both orientations of
         // a bidirectional rule carry the same id
@@ -502,6 +584,10 @@ impl ResolvedRule {
     fn is_narrowed(&self) -> bool {
         !self.protocols.is_empty()
     }
+
+    fn specificity(&self) -> usize {
+        specificity(&self.src, &self.dst)
+    }
 }
 
 fn reversed_protocol(protocol: &Protocol) -> Protocol {
@@ -555,10 +641,11 @@ pub struct Folded<'a> {
     pub allow: bool,
 }
 
-/// Fold the rules that fire on one `(src_class, dst_class)` pair into its verdict and checks. The
-/// verdict comes from the highest priority with a rule that decides the whole pair, and a single
-/// deny among those rules wins. Without such a rule the pair denies. The checks are the narrowed
-/// rules from the top priority down to that one, denies ahead of allows within a priority.
+/// Fold the rules that fire on one `(src_id, dst_id)` pair into its verdict and checks. The
+/// verdict comes from the highest priority with a rule that decides the whole pair. The most
+/// specific of those rules decide, and a single deny among them wins. Without such a rule the pair
+/// denies. The checks are the narrowed rules from the top priority down to that one, the more
+/// specific first and denies ahead of allows within a priority.
 pub fn fold<'a>(firing: impl IntoIterator<Item = &'a ResolvedRule>) -> Folded<'a> {
     let mut firing: Vec<&ResolvedRule> = firing.into_iter().collect();
     firing.sort_by(|a, b| evaluation_order(a, b));
@@ -567,9 +654,11 @@ pub fn fold<'a>(firing: impl IntoIterator<Item = &'a ResolvedRule>) -> Folded<'a
     let mut allow = false;
     for level in firing.chunk_by(|a, b| a.prio == b.prio) {
         checks.extend(level.iter().filter(|rule| rule.is_narrowed()));
-        let mut deciding = level.iter().filter(|rule| !rule.is_narrowed()).peekable();
-        if deciding.peek().is_some() {
-            allow = deciding.all(|rule| rule.allow);
+        let deciding = || level.iter().copied().filter(|rule| !rule.is_narrowed());
+        if let Some(most) = deciding().map(|rule| rule.specificity()).max() {
+            allow = deciding()
+                .filter(|rule| rule.specificity() == most)
+                .all(|rule| rule.allow);
             break;
         }
     }
@@ -601,16 +690,21 @@ mod tests {
         names.iter().map(|n| id(n)).collect()
     }
 
-    fn group(name: &str) -> (String, MicrosegEntry) {
+    fn group_entry(name: &str, parent: Option<&str>) -> (String, MicrosegEntry) {
         (
             name.to_string(),
             MicrosegEntry::Group(GroupSection {
                 id: name.parse().expect("valid group name"),
+                parent: parent.map(|parent| parent.parse().expect("valid group name")),
                 comment: None,
             }),
         )
     }
 
+    fn group(name: &str) -> (String, MicrosegEntry) {
+        group_entry(name, None)
+    }
+
     fn rule(name: &str, src: &[&str], dst: &[&str], allow: bool) -> (String, MicrosegEntry) {
         rule_full(
             name,
@@ -972,4 +1066,275 @@ mod tests {
         assert_eq!(check(&rules, "web", "db"), [tcp(None, Some("443"))]);
         assert_eq!(check(&rules, "db", "web"), [tcp(Some("443"), None)]);
     }
+
+    fn subgroup(name: &str, parent: &str) -> (String, MicrosegEntry) {
+        group_entry(name, Some(parent))
+    }
+
+    #[test]
+    fn subgroup_membership_implies_the_parents() {
+        use PredicateMatch::{All, Any, Exact};
+        let entries: BTreeMap<String, MicrosegEntry> = BTreeMap::from([
+            group("ga"),
+            subgroup("ga-b", "ga"),
+            subgroup("ga-b-c", "ga-b"),
+            group("gx"),
+            rule_full("on-a", &["ga"], All, &["gx"], Any, 0, true),
+        ]);
+        validate(&entries).expect("valid");
+        assert_eq!(
+            closed_groups(&entries, &ids(&["ga-b"])).unwrap(),
+            set(&["ga", "ga-b"])
+        );
+        assert_eq!(
+            closed_groups(&entries, &ids(&["ga-b-c"])).unwrap(),
+            set(&["ga", "ga-b", "ga-b-c"])
+        );
+        let rules = resolved(&entries);
+        assert!(
+            verdict(&rules, &set(&["ga", "ga-b"]), &set(&["gx"])),
+            "a parent rule covers the subgroup"
+        );
+        assert!(verdict(&rules, &set(&["ga"]), &set(&["gx"])));
+
+        let mut sub = entries.clone();
+        sub.extend([rule_full("on-a-b", &["ga-b"], All, &["gx"], Any, 0, false)]);
+        let rules = resolved(&sub);
+        let on_a_b = rules.iter().find(|r| r.id.as_ref() == "on-a-b").unwrap();
+        assert_eq!(on_a_b.src.groups, set(&["ga", "ga-b"]));
+        assert_eq!(on_a_b.specificity(), 2);
+        assert!(
+            !verdict(&rules, &set(&["ga", "ga-b"]), &set(&["gx"])),
+            "the deny on a/b outranks the allow on a"
+        );
+        assert!(
+            verdict(&rules, &set(&["ga"]), &set(&["gx"])),
+            "plain a members are untouched"
+        );
+
+        // exact compares the closed set
+        let exact: BTreeMap<String, MicrosegEntry> = BTreeMap::from([
+            group("ga"),
+            subgroup("ga-b", "ga"),
+            group("gx"),
+            rule_full("just-a", &["ga"], Exact, &["gx"], Any, 0, true),
+            rule_full("just-a-b", &["ga-b"], Exact, &["gx"], Any, 0, true),
+        ]);
+        let rules = resolved(&exact);
+        let just_a = rules.iter().find(|r| r.id.as_ref() == "just-a").unwrap();
+        let just_a_b = rules.iter().find(|r| r.id.as_ref() == "just-a-b").unwrap();
+        assert!(just_a.src.fires(&set(&["ga"])) && !just_a.src.fires(&set(&["ga", "ga-b"])));
+        assert!(just_a_b.src.fires(&set(&["ga", "ga-b"])) && !just_a_b.src.fires(&set(&["ga"])));
+    }
+
+    #[test]
+    fn any_predicates_stay_as_written() {
+        use PredicateMatch::Any;
+        let entries: BTreeMap<String, MicrosegEntry> = BTreeMap::from([
+            group("ga"),
+            subgroup("ga-b", "ga"),
+            group("gx"),
+            rule_full("any-a", &["ga"], Any, &["gx"], Any, 0, true),
+        ]);
+        let rules = resolved(&entries);
+        assert!(
+            verdict(&rules, &set(&["ga", "ga-b"]), &set(&["gx"])),
+            "an any on the parent covers the subgroup"
+        );
+        assert!(verdict(&rules, &set(&["ga"]), &set(&["gx"])));
+
+        let mut sub = entries.clone();
+        sub.remove("any-a");
+        sub.extend([rule_full("any-a-b", &["ga-b"], Any, &["gx"], Any, 0, true)]);
+        let rules = resolved(&sub);
+        assert_eq!(rules[0].src.groups, set(&["ga-b"]));
+        assert_eq!(rules[0].specificity(), 0);
+        assert!(verdict(&rules, &set(&["ga", "ga-b"]), &set(&["gx"])));
+        assert!(
+            !verdict(&rules, &set(&["ga"]), &set(&["gx"])),
+            "plain a members are not covered"
+        );
+
+        let mut with_untagged = entries.clone();
+        with_untagged.extend([rule_full(
+            "any-u",
+            &[UNTAGGED_GROUP, "ga-b"],
+            Any,
+            &["gx"],
+            Any,
+            0,
+            true,
+        )]);
+        let rules = resolved(&with_untagged);
+        let any_u = rules.iter().find(|r| r.id.as_ref() == "any-u").unwrap();
+        assert_eq!(any_u.src.groups, set(&[UNTAGGED_GROUP, "ga-b"]));
+    }
+
+    #[test]
+    fn every_covers_each_group_set_but_untagged() {
+        use PredicateMatch::{Any, Every};
+        let entries: BTreeMap<String, MicrosegEntry> = BTreeMap::from([
+            group("ga"),
+            group("gx"),
+            rule_full("to-x", &[], Every, &["gx"], Any, 0, true),
+        ]);
+        validate(&entries).expect("valid");
+        let rules = resolved(&entries);
+        assert_eq!(rules[0].specificity(), 0);
+        assert!(verdict(&rules, &set(&["ga"]), &set(&["gx"])));
+        assert!(verdict(&rules, &set(&["gx"]), &set(&["gx"])));
+        assert!(
+            !verdict(&rules, &set(&[UNTAGGED_GROUP]), &set(&["gx"])),
+            "unstamped traffic still needs a rule naming untagged"
+        );
+
+        let mut listed = entries.clone();
+        listed.extend([rule_full("bad", &["ga"], Every, &["gx"], Any, 0, true)]);
+        assert!(validate(&listed).is_err(), "every takes no groups");
+    }
+
+    #[test]
+    fn specificity_breaks_priority_ties() {
+        use PredicateMatch::{All, Any};
+        let base: BTreeMap<String, MicrosegEntry> = BTreeMap::from([
+            group("ga"),
+            subgroup("ga-b", "ga"),
+            group("gv"),
+            group("gu"),
+            group("go"),
+            group("gs"),
+            group("gx"),
+            rule_full("deny-vu", &["gv", "gu"], All, &["gs"], All, 5, false),
+            rule_full("allow-ao", &["ga", "go"], All, &["gx"], All, 5, true),
+        ]);
+        let dst = set(&["gs", "gx"]);
+        let plain = set(&["ga", "gv", "gu", "go"]);
+        let sub = set(&["ga", "ga-b", "gv", "gu", "go"]);
+
+        // two unrelated rules of equal specificity tie and deny, for ga and ga-b members alike
+        let rules = resolved(&base);
+        assert!(!verdict(&rules, &plain, &dst));
+        assert!(!verdict(&rules, &sub, &dst));
+
+        // a rule written for ga-b is longer once closed and wins the tie, for ga-b members only
+        let mut more = base.clone();
+        more.extend([rule_full(
+            "allow-abo",
+            &["ga-b", "go"],
+            All,
+            &["gx"],
+            All,
+            5,
+            true,
+        )]);
+        let rules = resolved(&more);
+        assert!(verdict(&rules, &sub, &dst));
+        assert!(!verdict(&rules, &plain, &dst));
+
+        // equally specific rules still AND their verdicts
+        more.extend([rule_full(
+            "deny-abo",
+            &["ga-b", "go"],
+            All,
+            &["gx"],
+            All,
+            5,
+            false,
+        )]);
+        let rules = resolved(&more);
+        assert!(!verdict(&rules, &sub, &dst));
+
+        // an any counts nothing, so an all carve-out beats a broad any deny at the same priority
+        let carve: BTreeMap<String, MicrosegEntry> = BTreeMap::from([
+            group("web"),
+            group("app"),
+            group("prod"),
+            rule_full("broad", &["web"], Any, &["app"], Any, 5, false),
+            rule_full(
+                "carve",
+                &["prod", "web"],
+                All,
+                &["prod", "app"],
+                All,
+                5,
+                true,
+            ),
+        ]);
+        let rules = resolved(&carve);
+        assert!(verdict(
+            &rules,
+            &set(&["web", "prod"]),
+            &set(&["app", "prod"])
+        ));
+        assert!(!verdict(&rules, &set(&["web"]), &set(&["app"])));
+
+        // priority still comes first
+        let mut prio = carve.clone();
+        prio.extend([rule_full("top", &["web"], Any, &["app"], Any, 6, false)]);
+        let rules = resolved(&prio);
+        assert!(!verdict(
+            &rules,
+            &set(&["web", "prod"]),
+            &set(&["app", "prod"])
+        ));
+    }
+
+    #[test]
+    fn a_narrowed_rule_runs_ahead_of_a_more_specific_whole_pair_rule() {
+        use PredicateMatch::All;
+        let mut entries: BTreeMap<String, MicrosegEntry> = BTreeMap::from([
+            group("web"),
+            group("db"),
+            group("prod"),
+            rule_full("whole", &["prod", "web"], All, &["db"], All, 5, false),
+            narrowed("pg", &["web"], &["db"], 5, true, Some("tcp"), Some("5432")),
+        ]);
+        let src = set(&["prod", "web"]);
+        let dst = set(&["db"]);
+
+        // specificity orders within a kind only, so the narrowed allow is checked although the
+        // deny of the same priority names more groups
+        let rules = resolved(&entries);
+        let folded = fold(firing_rules(&rules, &src, &dst));
+        let order: Vec<&str> = folded.checks.iter().map(|r| r.id.as_ref()).collect();
+        assert_eq!(order, ["pg"]);
+        assert!(!folded.allow);
+
+        // a narrowed rule one priority below the whole-pair rule is never reached
+        entries.extend([narrowed(
+            "pg",
+            &["web"],
+            &["db"],
+            4,
+            true,
+            Some("tcp"),
+            Some("5432"),
+        )]);
+        let rules = resolved(&entries);
+        let folded = fold(firing_rules(&rules, &src, &dst));
+        assert!(folded.checks.is_empty());
+        assert!(!folded.allow);
+    }
+
+    #[test]
+    fn validate_rejects_bad_parents() {
+        let with = |entries: Vec<(String, MicrosegEntry)>| validate(&entries.into_iter().collect());
+        with(vec![group("ga"), subgroup("gb", "nope")]).expect_err("an unknown parent");
+        with(vec![subgroup("ga", "ga")]).expect_err("a group as its own parent");
+        with(vec![subgroup("ga", "gb"), subgroup("gb", "ga")]).expect_err("a cycle");
+        with(vec![
+            subgroup("ga", "gb"),
+            subgroup("gb", "gc"),
+            subgroup("gc", "ga"),
+        ])
+        .expect_err("a longer cycle");
+        serde_json::from_str::<GroupSection>(r#"{"id":"ga","parent":"untagged"}"#)
+            .expect_err("untagged as a parent");
+        with(vec![
+            group("ga"),
+            subgroup("gb", "ga"),
+            subgroup("gc", "gb"),
+        ])
+        .expect("a chain");
+    }
 }
diff --git a/proxmox-ve-config/src/sdn/microseg/types.rs b/proxmox-ve-config/src/sdn/microseg/types.rs
index ee0a03beec51..39f80764e5e0 100644
--- a/proxmox-ve-config/src/sdn/microseg/types.rs
+++ b/proxmox-ve-config/src/sdn/microseg/types.rs
@@ -77,6 +77,12 @@ api_string_type! {
 
 #[api(
     properties: {
+        parent: {
+            type: String,
+            optional: true,
+            format: &MICROSEG_GROUP_NAME_FORMAT,
+            description: "The group this one is a subgroup of. Membership in a subgroup implies membership in every group above it.",
+        },
         comment: {
             type: String,
             optional: true,
@@ -87,9 +93,15 @@ api_string_type! {
 )]
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
 /// A microseg group. An interface is assigned a set of groups, which resolves to a wire identity.
+///
+/// Groups may form a hierarchy through `parent`. A member of a subgroup is a member of every
+/// group above it, so an `any` or `all` rule on a group covers its subgroups. An `exact`
+/// rule compares the whole set and does not.
 pub struct GroupSection {
     pub(crate) id: GroupName,
     #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub(crate) parent: Option<GroupName>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
     pub(crate) comment: Option<String>,
 }
 
-- 
2.47.3