Enforce only one RL per Server

linkerd · Nov 7, 2024 · 5baa048 · 5baa048
1 parent 3c34d86
commit 5baa048
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 8 deletions.
diff --git a/policy-controller/src/admission.rs b/policy-controller/src/admission.rs
@@ -854,8 +854,8 @@ fn validate_parent_ref_port_requirements(parent: &k8s_gateway_api::ParentReferen
 impl Validate<RateLimitPolicySpec> for Admission {
     async fn validate(
         self,
-        _ns: &str,
-        _name: &str,
+        ns: &str,
+        name: &str,
         _annotations: &BTreeMap<String, String>,
         spec: RateLimitPolicySpec,
     ) -> Result<()> {
@@ -866,6 +866,20 @@ impl Validate<RateLimitPolicySpec> for Admission {
             );
         }
 
+        let current_server_name = spec.target_ref.name;
+        let rate_limits = kube::Api::<HTTPLocalRateLimitPolicy>::namespaced(self.client, ns)
+            .list(&kube::api::ListParams::default())
+            .await?;
+        for rl in rate_limits.iter() {
+            let server_name = &rl.spec.target_ref.name;
+            if rl.name_unchecked() != name && server_name == &current_server_name {
+                bail!(
+                    "HTTPLocalRateLimitPolicy spec '{}' already exists for Server '{server_name}'",
+                    rl.name_unchecked(),
+                );
+            }
+        }
+
         if let Some(total) = spec.total {
             if total.requests_per_second == 0 {
                 bail!("total.requestsPerSecond must be greater than 0");

diff --git a/policy-test/tests/admit_http_local_ratelimit_policy.rs b/policy-test/tests/admit_http_local_ratelimit_policy.rs
@@ -1,16 +1,24 @@
 use linkerd_policy_controller_k8s_api::{
     self as api,
     policy::{
+        server::{Port, Selector, Server, ServerSpec},
         HTTPLocalRateLimitPolicy, Limit, LocalTargetRef, NamespacedTargetRef, Override,
         RateLimitPolicySpec,
     },
 };
-use linkerd_policy_test::admission;
+use linkerd_policy_test::{admission, with_temp_ns};
 
 #[tokio::test(flavor = "current_thread")]
 async fn accepts_valid() {
     admission::accepts(|ns| {
-        mk_ratelimiter(ns, default_target_ref(), 1000, 100, default_overrides())
+        mk_ratelimiter(
+            ns,
+            "test",
+            default_target_ref(),
+            1000,
+            100,
+            default_overrides(),
+        )
     })
     .await;
 }
@@ -22,13 +30,71 @@ async fn rejects_target_ref_deployment() {
         kind: "Deployment".to_string(),
         name: "api".to_string(),
     };
-    admission::rejects(|ns| mk_ratelimiter(ns, target_ref, 1000, 100, default_overrides())).await;
+    admission::rejects(|ns| mk_ratelimiter(ns, "test", target_ref, 1000, 100, default_overrides()))
+        .await;
+}
+
+#[tokio::test(flavor = "current_thread")]
+async fn rejects_multiple_limits_per_server() {
+    with_temp_ns(|client, ns| async move {
+        let api = kube::Api::<Server>::namespaced(client.clone(), &ns);
+
+        let test0 = Server {
+            metadata: api::ObjectMeta {
+                namespace: Some(ns.clone()),
+                name: Some("api".to_string()),
+                ..Default::default()
+            },
+            spec: ServerSpec {
+                selector: Selector::Pod(api::labels::Selector::from_iter(Some(("app", "test")))),
+                port: Port::Number(80.try_into().unwrap()),
+                proxy_protocol: None,
+                access_policy: None,
+            },
+        };
+        api.create(&kube::api::PostParams::default(), &test0)
+            .await
+            .expect("resource must apply");
+
+        let api = kube::Api::<HTTPLocalRateLimitPolicy>::namespaced(client, &ns);
+        let rl1 = mk_ratelimiter(
+            ns.clone(),
+            "test",
+            default_target_ref(),
+            1000,
+            100,
+            default_overrides(),
+        );
+        api.create(&kube::api::PostParams::default(), &rl1)
+            .await
+            .expect("resource must apply");
+
+        let rl2 = mk_ratelimiter(
+            ns.clone(),
+            "test-2",
+            default_target_ref(),
+            1000,
+            100,
+            default_overrides(),
+        );
+        api.create(&kube::api::PostParams::default(), &rl2)
+            .await
+            .expect_err("resource must not apply");
+    })
+    .await;
 }
 
 #[tokio::test(flavor = "current_thread")]
 async fn rejects_identity_rps_higher_than_total() {
     admission::rejects(|ns| {
-        mk_ratelimiter(ns, default_target_ref(), 1000, 2000, default_overrides())
+        mk_ratelimiter(
+            ns,
+            "test",
+            default_target_ref(),
+            1000,
+            2000,
+            default_overrides(),
+        )
     })
     .await;
 }
@@ -44,7 +110,10 @@ async fn rejects_overrides_rps_higher_than_total() {
             namespace: Some("linkerd".to_string()),
         }],
     }];
-    admission::rejects(|ns| mk_ratelimiter(ns, default_target_ref(), 1000, 2000, overrides)).await;
+    admission::rejects(|ns| {
+        mk_ratelimiter(ns, "test", default_target_ref(), 1000, 2000, overrides)
+    })
+    .await;
 }
 
 fn default_target_ref() -> LocalTargetRef {
@@ -69,6 +138,7 @@ fn default_overrides() -> Vec<Override> {
 
 fn mk_ratelimiter(
     namespace: String,
+    name: &str,
     target_ref: LocalTargetRef,
     total_rps: u32,
     identity_rps: u32,
@@ -77,7 +147,7 @@ fn mk_ratelimiter(
     HTTPLocalRateLimitPolicy {
         metadata: api::ObjectMeta {
             namespace: Some(namespace),
-            name: Some("test".to_string()),
+            name: Some(name.to_string()),
             ..Default::default()
         },
         spec: RateLimitPolicySpec {