diff --git a/policy-controller/src/admission.rs b/policy-controller/src/admission.rs index 59bf3269d2b3a..9e0b5eb7fe8c6 100644 --- 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 for Admission { async fn validate( self, - _ns: &str, - _name: &str, + ns: &str, + name: &str, _annotations: &BTreeMap, spec: RateLimitPolicySpec, ) -> Result<()> { @@ -866,6 +866,20 @@ impl Validate for Admission { ); } + let current_server_name = spec.target_ref.name; + let rate_limits = kube::Api::::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 == ¤t_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 index 5e4de499420be..eab77799eeaa8 100644 --- 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::::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::::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 { 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 {