Skip to content

Commit f8a1343

Browse files
authored
fix(policy): ignore route .status.parents ordering (#13328)
The `eq_time_insensitive_route_parent_statuses` considers ordering when comparing statuses, even though ordering doesn't matter. This can result in infite update loops. Add logic to `eq_time_insensitive_route_parent_statuses` to pre-sort statuses before comparison, thus making the result independent of ordering. A unit test was added to validate the fix. Fixes #13327 Signed-off-by: Derek Brown <6845676+DerekTBrown@users.noreply.github.com>
1 parent 8265134 commit f8a1343

File tree

3 files changed

+39
-2
lines changed

3 files changed

+39
-2
lines changed

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,14 +1521,33 @@ pub(crate) fn invalid_backend_kind(message: &str) -> k8s_core_api::Condition {
15211521
}
15221522
}
15231523

1524-
fn eq_time_insensitive_route_parent_statuses(
1524+
pub(crate) fn eq_time_insensitive_route_parent_statuses(
15251525
left: &[k8s_gateway_api::RouteParentStatus],
15261526
right: &[k8s_gateway_api::RouteParentStatus],
15271527
) -> bool {
15281528
if left.len() != right.len() {
15291529
return false;
15301530
}
1531-
left.iter().zip(right.iter()).all(|(l, r)| {
1531+
1532+
// Create sorted versions of the input slices
1533+
let mut left_sorted: Vec<_> = left.to_vec();
1534+
let mut right_sorted: Vec<_> = right.to_vec();
1535+
1536+
left_sorted.sort_by(|a, b| {
1537+
a.controller_name
1538+
.cmp(&b.controller_name)
1539+
.then_with(|| a.parent_ref.name.cmp(&b.parent_ref.name))
1540+
.then_with(|| a.parent_ref.namespace.cmp(&b.parent_ref.namespace))
1541+
});
1542+
right_sorted.sort_by(|a, b| {
1543+
a.controller_name
1544+
.cmp(&b.controller_name)
1545+
.then_with(|| a.parent_ref.name.cmp(&b.parent_ref.name))
1546+
.then_with(|| a.parent_ref.namespace.cmp(&b.parent_ref.namespace))
1547+
});
1548+
1549+
// Compare each element in sorted order
1550+
left_sorted.iter().zip(right_sorted.iter()).all(|(l, r)| {
15321551
l.parent_ref == r.parent_ref
15331552
&& l.controller_name == r.controller_name
15341553
&& eq_time_insensitive_conditions(&l.conditions, &r.conditions)

policy-controller/k8s/status/src/tests/routes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use chrono::{DateTime, Utc};
77
use linkerd_policy_controller_core::POLICY_CONTROLLER_NAME;
88

99
mod grpc;
10+
mod helpers;
1011
mod http;
1112
mod tcp;
1213
mod tls;
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
use super::make_parent_status;
2+
use crate::index::eq_time_insensitive_route_parent_statuses;
3+
4+
#[test]
5+
fn test_eq_time_insensitive_route_parent_statuses_order_sensitive() {
6+
// Create RouteParentStatus instances using make_parent_status helper
7+
let status1 = make_parent_status("ns", "parent1", "Ready", "True", "AllGood");
8+
let status2 = make_parent_status("ns", "parent2", "Ready", "True", "AllGood");
9+
10+
// Create two lists with the same elements in different orders
11+
let list1 = vec![status1.clone(), status2.clone()];
12+
let list2 = vec![status2, status1];
13+
14+
// Assert that eq_time_insensitive_route_parent_statuses returns true
15+
// indicating that it considers the two lists equal
16+
assert!(eq_time_insensitive_route_parent_statuses(&list1, &list2));
17+
}

0 commit comments

Comments
 (0)