From 2c203fdb040f6a9f6d665f81b8bd858299eb35b1 Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Tue, 18 Jul 2023 13:09:50 -0700
Subject: [PATCH] reject duplicate retry filters

---
 policy-controller/src/admission.rs | 54 +++++++++++++++++-------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/policy-controller/src/admission.rs b/policy-controller/src/admission.rs
index 45d04dcd7c733..b3d0143a57e7f 100644
--- a/policy-controller/src/admission.rs
+++ b/policy-controller/src/admission.rs
@@ -452,30 +452,6 @@ impl Validate<HttpRouteSpec> for Admission {
             Ok(())
         }
 
-        fn validate_filter(filter: httproute::HttpRouteFilter) -> Result<()> {
-            match filter {
-                httproute::HttpRouteFilter::RequestHeaderModifier {
-                    request_header_modifier,
-                } => http_route::header_modifier(request_header_modifier).map(|_| ()),
-                httproute::HttpRouteFilter::ResponseHeaderModifier {
-                    response_header_modifier,
-                } => http_route::header_modifier(response_header_modifier).map(|_| ()),
-                httproute::HttpRouteFilter::RequestRedirect { request_redirect } => {
-                    http_route::req_redirect(request_redirect).map(|_| ())
-                }
-                httproute::HttpRouteFilter::ExtensionRef { extension_ref }
-                    if httproute::local_object_ref_targets_kind::<HttpRetryFilter>(
-                        &extension_ref,
-                    ) =>
-                {
-                    Ok(())
-                }
-                httproute::HttpRouteFilter::ExtensionRef { extension_ref } => {
-                    bail!("unsupported extensionRef filter: {extension_ref:?}",)
-                }
-            }
-        }
-
         fn validate_timeouts(timeouts: httproute::HttpRouteTimeouts) -> Result<()> {
             use std::time::Duration;
 
@@ -499,6 +475,36 @@ impl Validate<HttpRouteSpec> for Admission {
             Ok(())
         }
 
+        let mut has_seen_retry_filter = false;
+        let mut validate_filter = move |filter: httproute::HttpRouteFilter| -> Result<()> {
+            match filter {
+                httproute::HttpRouteFilter::RequestHeaderModifier {
+                    request_header_modifier,
+                } => http_route::header_modifier(request_header_modifier).map(|_| ()),
+                httproute::HttpRouteFilter::ResponseHeaderModifier {
+                    response_header_modifier,
+                } => http_route::header_modifier(response_header_modifier).map(|_| ()),
+                httproute::HttpRouteFilter::RequestRedirect { request_redirect } => {
+                    http_route::req_redirect(request_redirect).map(|_| ())
+                }
+                httproute::HttpRouteFilter::ExtensionRef { extension_ref }
+                    if httproute::local_object_ref_targets_kind::<HttpRetryFilter>(
+                        &extension_ref,
+                    ) =>
+                {
+                    ensure!(
+                        !has_seen_retry_filter,
+                        "an HTTPRoute rule may not contain multiple HTTPRetryFilters"
+                    );
+                    has_seen_retry_filter = true;
+                    Ok(())
+                }
+                httproute::HttpRouteFilter::ExtensionRef { extension_ref } => {
+                    bail!("unsupported extensionRef filter: {extension_ref:?}",)
+                }
+            }
+        };
+
         // Validate the rules in this spec.
         // This is essentially equivalent to the indexer's conversion function
         // from `HttpRouteSpec` to `InboundRouteBinding`, except that we don't