From 2ae636dc1bad4b77eb8334c8e6f21b7532cb127f Mon Sep 17 00:00:00 2001 From: Raymond Kroeker Date: Wed, 27 May 2026 16:41:45 -0700 Subject: [PATCH 1/3] fix(policy): Remove Appropriate AuthN Policy * Delete from the appropriate policy map when handling delete and reset calls. * Mark state as changed when removing items. * Continue with the removal iteration (vs return) in the network authn reset. * Fix copy and paste strings. * Add some test cases. --- .../k8s/index/src/inbound/index.rs | 16 +- .../k8s/index/src/inbound/tests.rs | 78 ++++++++++ .../src/inbound/tests/authorization_policy.rs | 76 ---------- .../inbound/tests/meshtls_authentication.rs | 141 ++++++++++++++++++ .../inbound/tests/network_authentication.rs | 77 ++++++++++ 5 files changed, 306 insertions(+), 82 deletions(-) create mode 100644 policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs create mode 100644 policy-controller/k8s/index/src/inbound/tests/network_authentication.rs diff --git a/policy-controller/k8s/index/src/inbound/index.rs b/policy-controller/k8s/index/src/inbound/index.rs index b80dbd2eac8da..9d981f1df5c35 100644 --- a/policy-controller/k8s/index/src/inbound/index.rs +++ b/policy-controller/k8s/index/src/inbound/index.rs @@ -809,7 +809,7 @@ impl kubert::index::IndexNamespacedResource if let Entry::Occupied(mut ns) = self.authentications.by_ns.entry(ns) { tracing::debug!("Deleting MeshTLSAuthentication"); - ns.get_mut().network.remove(&name); + ns.get_mut().meshtls.remove(&name); if ns.get().is_empty() { ns.remove(); } @@ -848,7 +848,9 @@ impl kubert::index::IndexNamespacedResource for (namespace, names) in deleted.into_iter() { if let Entry::Occupied(mut ns) = self.authentications.by_ns.entry(namespace) { for name in names.into_iter() { - ns.get_mut().meshtls.remove(&name); + if ns.get_mut().meshtls.remove(&name).is_some() { + changed = true; + } } if ns.get().is_empty() { ns.remove(); @@ -885,7 +887,7 @@ impl kubert::index::IndexNamespacedResource let _span = info_span!("delete", %ns, %name).entered(); if let Entry::Occupied(mut ns) = self.authentications.by_ns.entry(ns) { - tracing::debug!("Deleting MeshTLSAuthentication"); + tracing::debug!("Deleting NetworkTLSAuthentication"); ns.get_mut().network.remove(&name); if ns.get().is_empty() { @@ -909,13 +911,13 @@ impl kubert::index::IndexNamespacedResource for authn in authns.into_iter() { let namespace = authn .namespace() - .expect("meshtlsauthentication must be namespaced"); + .expect("networkauthentication must be namespaced"); let name = authn.name_unchecked(); let spec = match network_authentication::Spec::try_from(authn.spec) { Ok(spec) => spec, Err(error) => { tracing::warn!(ns = %namespace, %name, %error, "Invalid NetworkAuthentication"); - return; + continue; } }; changed = self.authentications.update_network(namespace, name, spec) || changed; @@ -923,7 +925,9 @@ impl kubert::index::IndexNamespacedResource for (namespace, names) in deleted.into_iter() { if let Entry::Occupied(mut ns) = self.authentications.by_ns.entry(namespace) { for name in names.into_iter() { - ns.get_mut().meshtls.remove(&name); + if ns.get_mut().network.remove(&name).is_some() { + changed = true; + } } if ns.get().is_empty() { ns.remove(); diff --git a/policy-controller/k8s/index/src/inbound/tests.rs b/policy-controller/k8s/index/src/inbound/tests.rs index 49b422bb6a77e..05359fec81f69 100644 --- a/policy-controller/k8s/index/src/inbound/tests.rs +++ b/policy-controller/k8s/index/src/inbound/tests.rs @@ -2,6 +2,8 @@ mod annotation; mod authorization_policy; mod grpc_routes; mod http_routes; +mod meshtls_authentication; +mod network_authentication; mod ratelimit_policy; mod server_authorization; @@ -68,6 +70,82 @@ const DEFAULTS: [DefaultPolicy; 6] = [ DefaultPolicy::Audit, ]; +fn mk_authorization_policy( + ns: impl ToString, + name: impl ToString, + server: Option, + authns: impl IntoIterator, +) -> k8s::policy::AuthorizationPolicy { + k8s::policy::AuthorizationPolicy { + metadata: k8s::ObjectMeta { + namespace: Some(ns.to_string()), + name: Some(name.to_string()), + ..Default::default() + }, + spec: k8s::policy::AuthorizationPolicySpec { + target_ref: match server { + Some(server) => LocalTargetRef { + group: Some("policy.linkerd.io".to_string()), + kind: "Server".to_string(), + name: server.to_string(), + }, + None => LocalTargetRef { + group: Some("core".to_string()), + kind: "Namespace".to_string(), + name: ns.to_string(), + }, + }, + required_authentication_refs: authns.into_iter().collect(), + }, + } +} + +fn mk_meshtls_authentication( + ns: impl ToString, + name: impl ToString, + identities: impl IntoIterator, + refs: impl IntoIterator, +) -> k8s::policy::MeshTLSAuthentication { + let identities = identities.into_iter().collect::>(); + let identity_refs = refs.into_iter().collect::>(); + k8s::policy::MeshTLSAuthentication { + metadata: k8s::ObjectMeta { + namespace: Some(ns.to_string()), + name: Some(name.to_string()), + ..Default::default() + }, + spec: k8s::policy::MeshTLSAuthenticationSpec { + identities: if identities.is_empty() { + None + } else { + Some(identities) + }, + identity_refs: if identity_refs.is_empty() { + None + } else { + Some(identity_refs) + }, + }, + } +} + +fn mk_network_authentication( + ns: impl ToString, + name: impl ToString, + networks: impl IntoIterator, +) -> k8s::policy::NetworkAuthentication { + k8s::policy::NetworkAuthentication { + metadata: k8s::ObjectMeta { + namespace: Some(ns.to_string()), + name: Some(name.to_string()), + ..Default::default() + }, + spec: k8s::policy::NetworkAuthenticationSpec { + networks: networks.into_iter().collect(), + }, + } +} + pub fn mk_pod_with_containers( ns: impl ToString, name: impl ToString, diff --git a/policy-controller/k8s/index/src/inbound/tests/authorization_policy.rs b/policy-controller/k8s/index/src/inbound/tests/authorization_policy.rs index 7d7cd6410c910..232eea29738a2 100644 --- a/policy-controller/k8s/index/src/inbound/tests/authorization_policy.rs +++ b/policy-controller/k8s/index/src/inbound/tests/authorization_policy.rs @@ -404,82 +404,6 @@ fn authorization_policy_prevents_index_deletion() { ); } -fn mk_authorization_policy( - ns: impl ToString, - name: impl ToString, - server: Option, - authns: impl IntoIterator, -) -> k8s::policy::AuthorizationPolicy { - k8s::policy::AuthorizationPolicy { - metadata: k8s::ObjectMeta { - namespace: Some(ns.to_string()), - name: Some(name.to_string()), - ..Default::default() - }, - spec: k8s::policy::AuthorizationPolicySpec { - target_ref: match server { - Some(server) => LocalTargetRef { - group: Some("policy.linkerd.io".to_string()), - kind: "Server".to_string(), - name: server.to_string(), - }, - None => LocalTargetRef { - group: Some("core".to_string()), - kind: "Namespace".to_string(), - name: ns.to_string(), - }, - }, - required_authentication_refs: authns.into_iter().collect(), - }, - } -} - -fn mk_meshtls_authentication( - ns: impl ToString, - name: impl ToString, - identities: impl IntoIterator, - refs: impl IntoIterator, -) -> k8s::policy::MeshTLSAuthentication { - let identities = identities.into_iter().collect::>(); - let identity_refs = refs.into_iter().collect::>(); - k8s::policy::MeshTLSAuthentication { - metadata: k8s::ObjectMeta { - namespace: Some(ns.to_string()), - name: Some(name.to_string()), - ..Default::default() - }, - spec: k8s::policy::MeshTLSAuthenticationSpec { - identities: if identities.is_empty() { - None - } else { - Some(identities) - }, - identity_refs: if identity_refs.is_empty() { - None - } else { - Some(identity_refs) - }, - }, - } -} - -fn mk_network_authentication( - ns: impl ToString, - name: impl ToString, - networks: impl IntoIterator, -) -> k8s::policy::NetworkAuthentication { - k8s::policy::NetworkAuthentication { - metadata: k8s::ObjectMeta { - namespace: Some(ns.to_string()), - name: Some(name.to_string()), - ..Default::default() - }, - spec: k8s::policy::NetworkAuthenticationSpec { - networks: networks.into_iter().collect(), - }, - } -} - fn mk_http_route( ns: impl ToString, name: impl ToString, diff --git a/policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs b/policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs new file mode 100644 index 0000000000000..221b06cff4634 --- /dev/null +++ b/policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs @@ -0,0 +1,141 @@ +use super::*; +use ahash::AHashSet; + +// === delete === + +#[test] +fn delete_meshtls_authn_removes_authorization() { + let test = TestConfig::default(); + + let mut pod = mk_pod("ns-0", "pod-0", Some(("container-0", None))); + pod.labels_mut() + .insert("app".to_string(), "app-0".to_string()); + test.index.write().apply(pod); + test.index.write().apply(mk_server( + "ns-0", + "srv-8080", + Port::Number(8080.try_into().unwrap()), + None, + Some(("app", "app-0")), + Some(k8s::policy::server::ProxyProtocol::Http1), + )); + test.index.write().apply(mk_authorization_policy( + "ns-0", + "authz-policy-0", + Some("srv-8080"), + vec![NamespacedTargetRef { + group: Some("policy.linkerd.io".to_string()), + kind: "MeshTLSAuthentication".to_string(), + name: "mtls-authn-0".to_string(), + namespace: None, + }], + )); + test.index.write().apply(mk_meshtls_authentication( + "ns-0", + "mtls-authn-0", + vec!["identity-0".to_string()], + vec![], + )); + + let mut rx = test + .index + .write() + .pod_server_rx("ns-0", "pod-0", 8080.try_into().unwrap()) + .expect("pod-0.ns-0 should exist"); + assert!( + rx.borrow_and_update() + .authorizations + .contains_key(&AuthorizationRef::AuthorizationPolicy( + "authz-policy-0".to_string() + )), + "authz-policy-0 should be present before delete" + ); + + >::delete( + &mut test.index.write(), + "ns-0".to_string(), + "mtls-authn-0".to_string(), + ); + + assert!(rx.has_changed().unwrap()); + assert!( + !rx.borrow() + .authorizations + .contains_key(&AuthorizationRef::AuthorizationPolicy( + "authz-policy-0".to_string() + )), + "authz-policy-0 should be absent after MeshTLSAuthentication is deleted" + ); +} + +// === reset === + +#[test] +fn reset_meshtls_authn_with_deleted_entries() { + let test = TestConfig::default(); + + let mut pod = mk_pod("ns-0", "pod-0", Some(("container-0", None))); + pod.labels_mut() + .insert("app".to_string(), "app-0".to_string()); + test.index.write().apply(pod); + test.index.write().apply(mk_server( + "ns-0", + "srv-8080", + Port::Number(8080.try_into().unwrap()), + None, + Some(("app", "app-0")), + Some(k8s::policy::server::ProxyProtocol::Http1), + )); + test.index.write().apply(mk_authorization_policy( + "ns-0", + "authz-policy-0", + Some("srv-8080"), + vec![NamespacedTargetRef { + group: Some("policy.linkerd.io".to_string()), + kind: "MeshTLSAuthentication".to_string(), + name: "mtls-authn-0".to_string(), + namespace: None, + }], + )); + test.index.write().apply(mk_meshtls_authentication( + "ns-0", + "mtls-authn-0", + vec!["identity-0".to_string()], + vec![], + )); + + let mut rx = test + .index + .write() + .pod_server_rx("ns-0", "pod-0", 8080.try_into().unwrap()) + .expect("pod-0.ns-0 should exist"); + assert!( + rx.borrow_and_update() + .authorizations + .contains_key(&AuthorizationRef::AuthorizationPolicy( + "authz-policy-0".to_string() + )), + "authz-policy-0 should be present before reset" + ); + + let mut deleted: HashMap> = HashMap::default(); + deleted.insert( + "ns-0".to_string(), + AHashSet::from_iter(["mtls-authn-0".to_string()]), + ); + >::reset( + &mut test.index.write(), + vec![], + deleted, + ); + + assert!(rx.has_changed().unwrap()); + assert!( + !rx.borrow_and_update() + .authorizations + .contains_key(&AuthorizationRef::AuthorizationPolicy( + "authz-policy-0".to_string() + )), + "authz-policy-0 should be absent after reset removes the MeshTLSAuthentication" + ); +} diff --git a/policy-controller/k8s/index/src/inbound/tests/network_authentication.rs b/policy-controller/k8s/index/src/inbound/tests/network_authentication.rs new file mode 100644 index 0000000000000..5072ccf8a6bb3 --- /dev/null +++ b/policy-controller/k8s/index/src/inbound/tests/network_authentication.rs @@ -0,0 +1,77 @@ +use super::*; +use ahash::AHashSet; + +// === reset === + +#[test] +fn reset_network_authn_with_deleted_entries() { + let test = TestConfig::default(); + + let mut pod = mk_pod("ns-0", "pod-0", Some(("container-0", None))); + pod.labels_mut() + .insert("app".to_string(), "app-0".to_string()); + test.index.write().apply(pod); + test.index.write().apply(mk_server( + "ns-0", + "srv-8080", + Port::Number(8080.try_into().unwrap()), + None, + Some(("app", "app-0")), + Some(k8s::policy::server::ProxyProtocol::Http1), + )); + test.index.write().apply(mk_authorization_policy( + "ns-0", + "authz-policy-0", + Some("srv-8080"), + vec![NamespacedTargetRef { + group: Some("policy.linkerd.io".to_string()), + kind: "NetworkAuthentication".to_string(), + name: "network-authn-0".to_string(), + namespace: None, + }], + )); + test.index.write().apply(mk_network_authentication( + "ns-0", + "network-authn-0", + vec![k8s::policy::network_authentication::Network { + cidr: "10.0.0.0/8".parse().unwrap(), + except: None, + }], + )); + + let mut rx = test + .index + .write() + .pod_server_rx("ns-0", "pod-0", 8080.try_into().unwrap()) + .expect("pod-0.ns-0 should exist"); + assert!( + rx.borrow_and_update() + .authorizations + .contains_key(&AuthorizationRef::AuthorizationPolicy( + "authz-policy-0".to_string() + )), + "authz-foo should be present before reset" + ); + + // reset and delete network authentication policy + let mut deleted: HashMap> = HashMap::default(); + deleted.insert( + "ns-0".to_string(), + AHashSet::from_iter(["network-authn-0".to_string()]), + ); + >::reset( + &mut test.index.write(), + vec![], + deleted, + ); + + assert!(rx.has_changed().unwrap()); + assert!( + !rx.borrow_and_update() + .authorizations + .contains_key(&AuthorizationRef::AuthorizationPolicy( + "authz-policy-0".to_string() + )), + "authz-policy-0 should be absent after reset removes it" + ); +} From ac66d247099229a653e1dc0eddf27d85af04eb0a Mon Sep 17 00:00:00 2001 From: Raymond Kroeker Date: Thu, 28 May 2026 10:01:24 -0700 Subject: [PATCH 2/3] Fix lint issues w/ rust code. --- .../k8s/index/src/inbound/tests/meshtls_authentication.rs | 8 +++----- .../k8s/index/src/inbound/tests/network_authentication.rs | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs b/policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs index 221b06cff4634..8625e23971a1a 100644 --- a/policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs +++ b/policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs @@ -131,11 +131,9 @@ fn reset_meshtls_authn_with_deleted_entries() { assert!(rx.has_changed().unwrap()); assert!( - !rx.borrow_and_update() - .authorizations - .contains_key(&AuthorizationRef::AuthorizationPolicy( - "authz-policy-0".to_string() - )), + !rx.borrow_and_update().authorizations.contains_key( + &AuthorizationRef::AuthorizationPolicy("authz-policy-0".to_string()) + ), "authz-policy-0 should be absent after reset removes the MeshTLSAuthentication" ); } diff --git a/policy-controller/k8s/index/src/inbound/tests/network_authentication.rs b/policy-controller/k8s/index/src/inbound/tests/network_authentication.rs index 5072ccf8a6bb3..d3cc109e86b01 100644 --- a/policy-controller/k8s/index/src/inbound/tests/network_authentication.rs +++ b/policy-controller/k8s/index/src/inbound/tests/network_authentication.rs @@ -67,11 +67,9 @@ fn reset_network_authn_with_deleted_entries() { assert!(rx.has_changed().unwrap()); assert!( - !rx.borrow_and_update() - .authorizations - .contains_key(&AuthorizationRef::AuthorizationPolicy( - "authz-policy-0".to_string() - )), + !rx.borrow_and_update().authorizations.contains_key( + &AuthorizationRef::AuthorizationPolicy("authz-policy-0".to_string()) + ), "authz-policy-0 should be absent after reset removes it" ); } From 56fb180f86802229eede82102a1fc88bbe64f716 Mon Sep 17 00:00:00 2001 From: Raymond Kroeker Date: Fri, 29 May 2026 09:42:03 -0700 Subject: [PATCH 3/3] Drop TLS typo. --- policy-controller/k8s/index/src/inbound/index.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/policy-controller/k8s/index/src/inbound/index.rs b/policy-controller/k8s/index/src/inbound/index.rs index 9d981f1df5c35..e49639f58b5dd 100644 --- a/policy-controller/k8s/index/src/inbound/index.rs +++ b/policy-controller/k8s/index/src/inbound/index.rs @@ -887,7 +887,7 @@ impl kubert::index::IndexNamespacedResource let _span = info_span!("delete", %ns, %name).entered(); if let Entry::Occupied(mut ns) = self.authentications.by_ns.entry(ns) { - tracing::debug!("Deleting NetworkTLSAuthentication"); + tracing::debug!("Deleting NetworkAuthentication"); ns.get_mut().network.remove(&name); if ns.get().is_empty() {