Skip to content

Commit

Permalink
policy: limit globally affecting egress networks to a single namespace (
Browse files Browse the repository at this point in the history
linkerd#13246)

This change introduces an `global_external_network_namespace` argument to the policy controller and alters the semantics of `EgressNetwork` matching in a way that: 

- egress networks created in the global egress networks namespace will affect all client workloads in the cluster
- egress networks in the same namespace as the client will always be preferred

Signed-off-by: Zahari Dichev <[email protected]>
  • Loading branch information
zaharidichev authored Oct 31, 2024
1 parent b9ccc99 commit 7bb867b
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 22 deletions.
6 changes: 5 additions & 1 deletion policy-controller/k8s/index/src/cluster_info.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::num::NonZeroU16;
use std::{num::NonZeroU16, sync::Arc};

use crate::{ports::PortSet, DefaultPolicy};
use linkerd_policy_controller_core::IpNet;
Expand Down Expand Up @@ -32,6 +32,10 @@ pub struct ClusterInfo {

/// The networks that probes are expected to be from.
pub probe_networks: Vec<IpNet>,

/// The namespace that is designated for egress configuration
/// affecting all workloads across the cluster
pub global_external_network_namespace: Arc<String>,
}

impl ClusterInfo {
Expand Down
2 changes: 2 additions & 0 deletions policy-controller/k8s/index/src/inbound/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use linkerd_policy_controller_k8s_api::{
ResourceExt,
};
use maplit::*;
use std::sync::Arc;
use tokio::time;

#[test]
Expand Down Expand Up @@ -224,6 +225,7 @@ impl TestConfig {
default_detect_timeout: detect_timeout,
default_opaque_ports: Default::default(),
probe_networks,
global_external_network_namespace: Arc::new("linkerd-external".to_string()),
};
let index = Index::shared(cluster.clone());
Self {
Expand Down
7 changes: 7 additions & 0 deletions policy-controller/k8s/index/src/outbound/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ pub struct Index {
egress_networks_by_ref: HashMap<ResourceRef, EgressNetwork>,
// holds information about resources. currently EgressNetworks and Services
resource_info: HashMap<ResourceRef, ResourceInfo>,

cluster_networks: Vec<linkerd_k8s_api::Cidr>,
global_external_network_namespace: Arc<String>,

// holds a no-op sender to which all clients that have been returned
// a Fallback policy are subsribed. It is used to force these clients
Expand Down Expand Up @@ -403,6 +405,9 @@ impl kubert::index::IndexNamespacedResource<linkerd_k8s_api::EgressNetwork> for
impl Index {
pub fn shared(cluster_info: Arc<ClusterInfo>) -> SharedIndex {
let cluster_networks = cluster_info.networks.clone();
let global_external_network_namespace =
cluster_info.global_external_network_namespace.clone();

let (fallback_polcy_tx, _) = watch::channel(());
Arc::new(RwLock::new(Self {
namespaces: NamespaceIndex {
Expand All @@ -414,6 +419,7 @@ impl Index {
resource_info: HashMap::default(),
cluster_networks: cluster_networks.into_iter().map(Cidr::from).collect(),
fallback_polcy_tx,
global_external_network_namespace,
}))
}

Expand Down Expand Up @@ -489,6 +495,7 @@ impl Index {
egress_network::resolve_egress_network(
addr,
source_namespace,
&self.global_external_network_namespace,
self.egress_networks_by_ref.values(),
)
.map(|r| (r.namespace, r.name))
Expand Down
72 changes: 52 additions & 20 deletions policy-controller/k8s/index/src/outbound/index/egress_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,20 @@ impl EgressNetwork {
// Attempts to find the best matching network for a certain discovery look-up.
// Logic is:
// 1. if there are Egress networks in the source_namespace, only these are considered
// 2. the target IP is matched against the networks of the EgressNetwork
// 3. ambiguity is resolved as by comparing the networks using compare_matched_egress_network
// 2. otherwise only networks from the global egress network namespace are considered
// 3. the target IP is matched against the networks of the EgressNetwork
// 4. ambiguity is resolved as by comparing the networks using compare_matched_egress_network
pub(crate) fn resolve_egress_network<'n>(
addr: IpAddr,
source_namespace: String,
global_external_network_namespace: &str,
nets: impl Iterator<Item = &'n EgressNetwork>,
) -> Option<super::ResourceRef> {
let (same_ns, rest): (Vec<_>, Vec<_>) = nets.partition(|un| un.namespace == source_namespace);
let (same_ns, rest): (Vec<_>, Vec<_>) = nets
.filter(|en| {
en.namespace == source_namespace || en.namespace == *global_external_network_namespace
})
.partition(|un| un.namespace == source_namespace);
let to_pick_from = if !same_ns.is_empty() { same_ns } else { rest };

to_pick_from
Expand Down Expand Up @@ -123,6 +129,8 @@ fn compare_matched_egress_network(
mod test {
use super::*;

const EGRESS_NETS_NS: &str = "linkerd-external";

#[test]
fn test_picks_smallest_cidr() {
let ip_addr = "192.168.0.4".parse().unwrap();
Expand All @@ -133,7 +141,7 @@ mod test {
except: None,
}],
name: "net-1".to_string(),
namespace: "ns".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: None,
traffic_policy: TrafficPolicy::Allow,
},
Expand All @@ -143,13 +151,14 @@ mod test {
except: None,
}],
name: "net-2".to_string(),
namespace: "ns".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: None,
traffic_policy: TrafficPolicy::Allow,
},
];

let resolved = resolve_egress_network(ip_addr, "ns".into(), networks.iter());
let resolved =
resolve_egress_network(ip_addr, "ns".into(), EGRESS_NETS_NS, networks.iter());
assert_eq!(resolved.unwrap().name, "net-2".to_string())
}

Expand All @@ -173,16 +182,36 @@ mod test {
except: None,
}],
name: "net-2".to_string(),
namespace: "ns".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: None,
traffic_policy: TrafficPolicy::Allow,
},
];

let resolved = resolve_egress_network(ip_addr, "ns-1".into(), networks.iter());
let resolved =
resolve_egress_network(ip_addr, "ns-1".into(), EGRESS_NETS_NS, networks.iter());
assert_eq!(resolved.unwrap().name, "net-1".to_string())
}

#[test]
fn does_not_pick_network_in_unralated_ns() {
let ip_addr = "192.168.0.4".parse().unwrap();
let networks = vec![EgressNetwork {
networks: vec![Network {
cidr: "192.168.0.1/16".parse().unwrap(),
except: None,
}],
name: "net-1".to_string(),
namespace: "other-ns".to_string(),
creation_timestamp: None,
traffic_policy: TrafficPolicy::Allow,
}];

let resolved =
resolve_egress_network(ip_addr, "ns-1".into(), EGRESS_NETS_NS, networks.iter());
assert!(resolved.is_none());
}

#[test]
fn test_picks_older_resource() {
let ip_addr = "192.168.0.4".parse().unwrap();
Expand All @@ -193,7 +222,7 @@ mod test {
except: None,
}],
name: "net-1".to_string(),
namespace: "ns".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: Some(DateTime::<Utc>::MAX_UTC),
traffic_policy: TrafficPolicy::Allow,
},
Expand All @@ -203,13 +232,14 @@ mod test {
except: None,
}],
name: "net-2".to_string(),
namespace: "ns".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: Some(DateTime::<Utc>::MIN_UTC),
traffic_policy: TrafficPolicy::Allow,
},
];

let resolved = resolve_egress_network(ip_addr, "ns".into(), networks.iter());
let resolved =
resolve_egress_network(ip_addr, "ns".into(), EGRESS_NETS_NS, networks.iter());
assert_eq!(resolved.unwrap().name, "net-2".to_string())
}

Expand All @@ -222,8 +252,8 @@ mod test {
cidr: "192.168.0.1/16".parse().unwrap(),
except: None,
}],
name: "b".to_string(),
namespace: "a".to_string(),
name: "a".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: None,
traffic_policy: TrafficPolicy::Allow,
},
Expand All @@ -232,15 +262,16 @@ mod test {
cidr: "192.168.0.1/16".parse().unwrap(),
except: None,
}],
name: "d".to_string(),
namespace: "c".to_string(),
name: "b".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: None,
traffic_policy: TrafficPolicy::Allow,
},
];

let resolved = resolve_egress_network(ip_addr, "ns".into(), networks.iter());
assert_eq!(resolved.unwrap().name, "b".to_string())
let resolved =
resolve_egress_network(ip_addr, "ns".into(), EGRESS_NETS_NS, networks.iter());
assert_eq!(resolved.unwrap().name, "a".to_string())
}

#[test]
Expand All @@ -253,7 +284,7 @@ mod test {
except: Some(vec!["192.168.0.4".parse().unwrap()]),
}],
name: "b".to_string(),
namespace: "a".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: None,
traffic_policy: TrafficPolicy::Allow,
},
Expand All @@ -263,13 +294,14 @@ mod test {
except: None,
}],
name: "d".to_string(),
namespace: "c".to_string(),
namespace: EGRESS_NETS_NS.to_string(),
creation_timestamp: None,
traffic_policy: TrafficPolicy::Allow,
},
];

let resolved = resolve_egress_network(ip_addr, "ns".into(), networks.iter());
let resolved =
resolve_egress_network(ip_addr, "ns".into(), EGRESS_NETS_NS, networks.iter());
assert_eq!(resolved.unwrap().name, "d".to_string())
}
}
1 change: 1 addition & 0 deletions policy-controller/k8s/index/src/outbound/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ impl TestConfig {
default_detect_timeout: detect_timeout,
default_opaque_ports: Default::default(),
probe_networks,
global_external_network_namespace: Arc::new("linkerd-external".to_string()),
};
let index = Index::shared(Arc::new(cluster));
Self { index }
Expand Down
7 changes: 6 additions & 1 deletion policy-controller/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ struct Args {

#[clap(long)]
allow_l5d_request_headers: bool,

#[clap(long, default_value = "linkerd-external")]
global_external_network_namespace: String,
}

#[tokio::main]
Expand All @@ -122,6 +125,7 @@ async fn main() -> Result<()> {
default_opaque_ports,
patch_timeout_ms,
allow_l5d_request_headers,
global_external_network_namespace,
} = Args::parse();

let server = if admission_controller_disabled {
Expand All @@ -131,7 +135,7 @@ async fn main() -> Result<()> {
};

let probe_networks = probe_networks.map(|IpNets(nets)| nets).unwrap_or_default();

let global_external_network_namespace = Arc::new(global_external_network_namespace);
let default_opaque_ports = parse_portset(&default_opaque_ports)?;
let cluster_info = Arc::new(ClusterInfo {
networks: cluster_networks.clone(),
Expand All @@ -142,6 +146,7 @@ async fn main() -> Result<()> {
default_detect_timeout: DETECT_TIMEOUT,
default_opaque_ports,
probe_networks,
global_external_network_namespace,
});

// Build the API index data structures which will maintain information
Expand Down
38 changes: 38 additions & 0 deletions policy-test/tests/outbound_api_gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,44 @@ async fn egress_switches_to_fallback() {
.await;
}

#[tokio::test(flavor = "current_thread")]
async fn fallback_switches_to_egress() {
with_temp_ns(|client, ns| async move {
let mut policy_api = grpc::OutboundPolicyClient::port_forwarded(&client).await;
let mut rsp = policy_api.watch_ip(&ns, "1.1.1.1", 80).await.unwrap();

let policy = rsp.next().await.unwrap().unwrap();
let meta = policy.metadata.unwrap();
let expected_meta = meta::Metadata {
kind: Some(meta::metadata::Kind::Default("egress-fallback".to_string())),
};
assert_eq!(meta, expected_meta);

let _egress_net = create_egress_network(&client, &ns, "egress-net").await;
// stream should fall apart now
assert!(rsp.next().await.is_none());

// we should switch to an egress net now
let mut rsp = policy_api.watch_ip(&ns, "1.1.1.1", 80).await.unwrap();
let policy = rsp.next().await.unwrap().unwrap();
let meta = policy.metadata.unwrap();

let expected_meta = meta::Metadata {
kind: Some(meta::metadata::Kind::Resource(meta::Resource {
group: "policy.linkerd.io".to_string(),
port: 80,
kind: "EgressNetwork".to_string(),
name: "egress-net".to_string(),
namespace: ns.clone(),
section: "".to_string(),
})),
};

assert_eq!(meta, expected_meta);
})
.await;
}

#[tokio::test(flavor = "current_thread")]
async fn service_with_no_http_routes() {
with_temp_ns(|client, ns| async move {
Expand Down

0 comments on commit 7bb867b

Please sign in to comment.