fix(policy): Remove Appropriate AuthN Policy

policy-controller/k8s/index/src/inbound/index.rs

@@ -809,7 +809,7 @@ impl kubert::index::IndexNamespacedResource<k8s::policy::MeshTLSAuthentication>
 
         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<k8s::policy::MeshTLSAuthentication>
         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<k8s::policy::NetworkAuthentication>
         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 NetworkAuthentication");
 
             ns.get_mut().network.remove(&name);
             if ns.get().is_empty() {
@@ -909,21 +911,23 @@ impl kubert::index::IndexNamespacedResource<k8s::policy::NetworkAuthentication>
         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;
         }
         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();

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<impl ToString>,
+    authns: impl IntoIterator<Item = NamespacedTargetRef>,
+) -> 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<Item = String>,
+    refs: impl IntoIterator<Item = NamespacedTargetRef>,
+) -> k8s::policy::MeshTLSAuthentication {
+    let identities = identities.into_iter().collect::<Vec<_>>();
+    let identity_refs = refs.into_iter().collect::<Vec<_>>();
+    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<Item = k8s::policy::network_authentication::Network>,
+) -> 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,

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<impl ToString>,
-    authns: impl IntoIterator<Item = NamespacedTargetRef>,
-) -> 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<Item = String>,
-    refs: impl IntoIterator<Item = NamespacedTargetRef>,
-) -> k8s::policy::MeshTLSAuthentication {
-    let identities = identities.into_iter().collect::<Vec<_>>();
-    let identity_refs = refs.into_iter().collect::<Vec<_>>();
-    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<Item = k8s::policy::network_authentication::Network>,
-) -> 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,

policy-controller/k8s/index/src/inbound/tests/meshtls_authentication.rs

@@ -0,0 +1,139 @@
+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"
+    );
+
+    <Index as IndexNamespacedResource<k8s::policy::MeshTLSAuthentication>>::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<String, AHashSet<String>> = HashMap::default();
+    deleted.insert(
+        "ns-0".to_string(),
+        AHashSet::from_iter(["mtls-authn-0".to_string()]),
+    );
+    <Index as IndexNamespacedResource<k8s::policy::MeshTLSAuthentication>>::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"
+    );
+}