diff --git a/benches/throughput.rs b/benches/throughput.rs index d3fa15764d..fc514c4738 100644 --- a/benches/throughput.rs +++ b/benches/throughput.rs @@ -94,6 +94,7 @@ fn create_test_policies() -> Vec { scope: ztunnel::rbac::RbacScope::Global, namespace: "default".into(), rules: rules.clone(), + stats: Default::default(), }); } diff --git a/src/rbac.rs b/src/rbac.rs index e8495ef38b..29019e7ba0 100644 --- a/src/rbac.rs +++ b/src/rbac.rs @@ -17,6 +17,9 @@ use ipnet::IpNet; use std::fmt; use std::fmt::{Display, Formatter}; use std::net::SocketAddr; +use std::sync::Arc; +use std::sync::atomic::AtomicU64; +use std::sync::atomic::Ordering::Relaxed; use tracing::{instrument, trace}; use xds::istio::security::Address as XdsAddress; use xds::istio::security::Authorization as XdsRbac; @@ -31,7 +34,7 @@ use crate::state::workload::{WorkloadError, byte_to_ip}; use crate::strng::Strng; use crate::{strng, xds}; -#[derive(Debug, Hash, Eq, PartialEq, Clone, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Authorization { pub name: Strng, @@ -39,6 +42,24 @@ pub struct Authorization { pub scope: RbacScope, pub action: RbacAction, pub rules: Vec>>, + #[serde(default)] + pub stats: Arc, +} + +#[derive(Debug, Default, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AuthzCounter { + hits: AtomicU64, + misses: AtomicU64, +} + +impl AuthzCounter { + pub fn hit(&self) { + self.hits.fetch_add(1, Relaxed); + } + pub fn miss(&self) { + self.misses.fetch_add(1, Relaxed); + } } #[derive(Debug, Clone, Eq, Hash, Ord, PartialEq, PartialOrd, serde::Serialize)] @@ -360,6 +381,7 @@ impl TryFrom for Authorization { scope: RbacScope::from(xds::istio::security::Scope::try_from(resource.scope)?), action: RbacAction::from(xds::istio::security::Action::try_from(resource.action)?), rules, + stats: Default::default(), }) } } @@ -483,6 +505,7 @@ mod tests { scope: RbacScope::Global, action: RbacAction::Allow, rules, + stats: Default::default(), } } diff --git a/src/state.rs b/src/state.rs index c7efe0418c..fe78cc020b 100644 --- a/src/state.rs +++ b/src/state.rs @@ -545,12 +545,14 @@ impl DemandProxyState { // "If there are any DENY policies that match the request, deny the request." for pol in deny.iter() { if pol.matches(conn) { + pol.stats.hit(); debug!(policy = pol.to_key().as_str(), "deny policy match"); return Err(proxy::AuthorizationRejectionError::ExplicitlyDenied( pol.namespace.to_owned(), pol.name.to_owned(), )); } else { + pol.stats.miss(); trace!(policy = pol.to_key().as_str(), "deny policy does not match"); } } @@ -562,9 +564,11 @@ impl DemandProxyState { // "If any of the ALLOW policies match the request, allow the request." for pol in allow.iter() { if pol.matches(conn) { + pol.stats.hit(); debug!(policy = pol.to_key().as_str(), "allow policy match"); return Ok(()); } else { + pol.stats.miss(); trace!( policy = pol.to_key().as_str(), "allow policy does not match" @@ -1446,6 +1450,7 @@ mod tests { ], ], scope: rbac::RbacScope::Namespace, + stats: Default::default(), }, ); state.policies.insert( @@ -1467,6 +1472,7 @@ mod tests { ], ], scope: rbac::RbacScope::Namespace, + stats: Default::default(), }, ); diff --git a/src/state/policy.rs b/src/state/policy.rs index a759ffa9e4..c1bfbb1299 100644 --- a/src/state/policy.rs +++ b/src/state/policy.rs @@ -56,8 +56,14 @@ impl PolicyStore { .collect() } - pub fn insert(&mut self, xds_name: Strng, rbac: Authorization) { - self.remove(xds_name.clone()); + pub fn insert(&mut self, xds_name: Strng, mut rbac: Authorization) { + if let Some(old) = self.remove(xds_name.clone()) { + // Copy over old stats. There could be some motivation to reset the stats if the policy + // changes, but right now this is getting triggered even for NO-OP changes possibly, so + // resets would be confusing. + rbac.stats = old.stats; + } + match rbac.scope { RbacScope::Global => { self.by_namespace @@ -76,13 +82,11 @@ impl PolicyStore { self.by_key.insert(xds_name.clone(), rbac); } - pub fn remove(&mut self, xds_name: Strng) { - let Some(rbac) = self.by_key.remove(&xds_name) else { - return; - }; - if let Some(key) = match rbac.scope { + pub fn remove(&mut self, xds_name: Strng) -> Option { + let rbac = self.by_key.remove(&xds_name)?; + if let Some(key) = match &rbac.scope { RbacScope::Global => Some(strng::EMPTY), - RbacScope::Namespace => Some(rbac.namespace), + RbacScope::Namespace => Some(rbac.namespace.clone()), RbacScope::WorkloadSelector => None, } { if let Some(pl) = self.by_namespace.get_mut(&key) { @@ -92,6 +96,7 @@ impl PolicyStore { } } } + Some(rbac) } pub fn subscribe(&self) -> watch::Receiver<()> { self.notifier.sender.subscribe() @@ -128,6 +133,7 @@ mod tests { namespaces: vec![StringMatch::Exact("whatever".into())], ..Default::default() }]]], + stats: Default::default(), }; let policy_key = policy.to_key(); // insert this namespace-scoped policy into policystore then assert it is diff --git a/src/xds.rs b/src/xds.rs index fc5fc1cb49..2475394325 100644 --- a/src/xds.rs +++ b/src/xds.rs @@ -399,7 +399,7 @@ pub struct LocalWorkload { pub services: HashMap>, } -#[derive(Default, Debug, Eq, PartialEq, Clone, serde::Serialize, serde::Deserialize)] +#[derive(Default, Debug, Clone, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct LocalConfig { #[serde(default)] diff --git a/tests/namespaced.rs b/tests/namespaced.rs index 7d7d0d4fe0..b76b77e867 100644 --- a/tests/namespaced.rs +++ b/tests/namespaced.rs @@ -1001,6 +1001,7 @@ mod namespaced { )], ..Default::default() }]]], + stats: Default::default(), }) .await?; let _ = manager